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

[Zenghui Yu]

On 2019/9/27 0:27, Marc Zyngier wrote:
> 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.

Yes, I see.  So we can safely encode the non-zero SVPET as
COMMON_LPI_AFF in a GICR_TYPER when computing the affinity.


Thanks,
zenghui