Re: [PATCH 10/35] irqchip/gic-v4.1: VPE table (aka GICR_VPROPBASER) allocation

From: Marc Zyngier
Date: Thu Sep 26 2019 - 12:27:25 EST


On 26/09/2019 16:57, Marc Zyngier wrote:
> On 26/09/2019 16:19, Zenghui Yu wrote:
>> Hi Marc,
>>
>> Two more questions below.
>>
>> On 2019/9/25 22:41, Marc Zyngier wrote:
>>> On 25/09/2019 14:04, Zenghui Yu wrote:
>>>> Hi Marc,
>>>>
>>>> Some questions about this patch, mostly to confirm that I would
>>>> understand things here correctly.
>>>>
>>>> On 2019/9/24 2:25, Marc Zyngier wrote:
>>>>> GICv4.1 defines a new VPE table that is potentially shared between
>>>>> both the ITSs and the redistributors, following complicated affinity
>>>>> rules.
>>>>>
>>>>> To make things more confusing, the programming of this table at
>>>>> the redistributor level is reusing the GICv4.0 GICR_VPROPBASER register
>>>>> for something completely different.
>>>>>
>>>>> The code flow is somewhat complexified by the need to respect the
>>>>> affinities required by the HW, meaning that tables can either be
>>>>> inherited from a previously discovered ITS or redistributor.
>>>>>
>>>>> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>> @@ -1962,6 +1965,65 @@ static bool its_parse_indirect_baser(struct its_node *its,
>>>>> return indirect;
>>>>> }
>>>>>
>>>>> +static u32 compute_common_aff(u64 val)
>>>>> +{
>>>>> + u32 aff, clpiaff;
>>>>> +
>>>>> + aff = FIELD_GET(GICR_TYPER_AFFINITY, val);
>>>>> + clpiaff = FIELD_GET(GICR_TYPER_COMMON_LPI_AFF, val);
>>>>> +
>>>>> + return aff & ~(GENMASK(31, 0) >> (clpiaff * 8));
>>>>> +}
>>>>> +
>>>>> +static u32 compute_its_aff(struct its_node *its)
>>>>> +{
>>>>> + u64 val;
>>>>> + u32 svpet;
>>>>> +
>>>>> + /*
>>>>> + * Reencode the ITS SVPET and MPIDR as a GICR_TYPER, and compute
>>>>> + * the resulting affinity. We then use that to see if this match
>>>>> + * our own affinity.
>>>>> + */
>>>>> + svpet = FIELD_GET(GITS_TYPER_SVPET, its->typer);
>>
>> The spec says, ITS does not share vPE table with Redistributors when
>> SVPET==0. It seems that we miss this rule and simply regard SVPET as
>> GICR_TYPER_COMMON_LPI_AFF here. Am I wrong?
>
> Correct. I missed the case where the ITS doesn't share anything. That's
> pretty unlikely though (you loose all the benefit of v4.1, and I don't
> really see how you'd make it work reliably).

Actually, this is already handled...

>
>>
>>>>> + val = FIELD_PREP(GICR_TYPER_COMMON_LPI_AFF, svpet);
>>>>> + val |= FIELD_PREP(GICR_TYPER_AFFINITY, its->mpidr);
>>>>> + return compute_common_aff(val);
>>>>> +}
>>>>> +
>>>>> +static struct its_node *find_sibbling_its(struct its_node *cur_its)
>>>>> +{
>>>>> + struct its_node *its;
>>>>> + u32 aff;
>>>>> +
>>>>> + if (!FIELD_GET(GITS_TYPER_SVPET, cur_its->typer))
>>>>> + return NULL;

... here. If SVPET is 0, there is no sibling, and we'll allocate a VPE
table as usual.

Thanks,

M.
--
Jazz is not dead, it just smells funny...