Re: [PATCH] irq/gic-its: gicv4: set VPENDING table as inner-shareable

[Guoheyi]

在 2019/12/2 2:04, Marc Zyngier 写道:
> On Sat, 30 Nov 2019 15:38:49 +0800
> Heyi Guo <guoheyi@xxxxxxxxxx> wrote:
>
> > There is no special reason to set virtual LPI pending table as
> > non-shareable. If we choose to hard code the shareability without
> > probing, inner-shareable will be a better choice, for all the other
> > ITS/GICR tables prefer to be inner-shareable.
> One of the issues is that we have strictly no idea what the caches are
> Inner Shareable with (I've been asking for such clarification for years
> without getting anywhere). You can have as many disconnected inner
> shareable domains as you want!

Hisilicon HIP07 and HIP08 are compliant with ARM SBSA and have only one 
inner shareable domain in the whole system.

What will happen if a system has multiple inner shareable domains? Will 
Linux still work on such system? Can we declare that Linux only supports 
one single inner shareable domain?

> I suspect that in the grand scheme of things, the redistributors
> ought to be in the same inner shareable domain, and that with a bit of
> luck, the CPUs are there as well. Still, that's a massive guess.
>
> > What's more, on Hisilicon hip08 it will trigger some kind of bus
> > warning when mixing use of different shareabilities.
> Do you have more information about what the bus is complaining about?
> Is that because the CPUs have these pages mapped as inner shareable?

Actually HIP08 L3 Cache will complain on any non-shareable cache entry, 
for the data coherence cannot be guarenteed for such configuration. This 
also implies VPENDING table will be allocated and snooped in L3 cache.

> I'll give it a go on D05 (HIP07) to find out what changes there.

Thanks,

HG

> Thanks,
>
> 	M.
>
> > Signed-off-by: Heyi Guo <guoheyi@xxxxxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Cc: Jason Cooper <jason@xxxxxxxxxxxxxx>
> > Cc: Marc Zyngier <maz@xxxxxxxxxx>
> > ---
> >   drivers/irqchip/irq-gic-v3-its.c   | 2 +-
> >   include/linux/irqchip/arm-gic-v3.h | 3 +++
> >   2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> > index 787e8eec9a7f..d31e863bc9ef 100644
> > --- a/drivers/irqchip/irq-gic-v3-its.c
> > +++ b/drivers/irqchip/irq-gic-v3-its.c
> > @@ -2831,7 +2831,7 @@ static void its_vpe_schedule(struct its_vpe *vpe)
> >   	val  = virt_to_phys(page_address(vpe->vpt_page)) &
> >   		GENMASK_ULL(51, 16);
> >   	val |= GICR_VPENDBASER_RaWaWb;
> > -	val |= GICR_VPENDBASER_NonShareable;
> > +	val |= GICR_VPENDBASER_InnerShareable;
> >   	/*
> >   	 * There is no good way of finding out if the pending table is
> >   	 * empty as we can race against the doorbell interrupt very
> > diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
> > index 5cc10cf7cb3e..a184f875e451 100644
> > --- a/include/linux/irqchip/arm-gic-v3.h
> > +++ b/include/linux/irqchip/arm-gic-v3.h
> > @@ -289,6 +289,9 @@
> >   #define GICR_VPENDBASER_NonShareable					\
> >   	GIC_BASER_SHAREABILITY(GICR_VPENDBASER, NonShareable)
> >   
> > +#define GICR_VPENDBASER_InnerShareable					\
> > +	GIC_BASER_SHAREABILITY(GICR_VPENDBASER, InnerShareable)
> > +
> >   #define GICR_VPENDBASER_nCnB	GIC_BASER_CACHEABILITY(GICR_VPENDBASER, INNER, nCnB)
> >   #define GICR_VPENDBASER_nC 	GIC_BASER_CACHEABILITY(GICR_VPENDBASER, INNER, nC)
> >   #define GICR_VPENDBASER_RaWt	GIC_BASER_CACHEABILITY(GICR_VPENDBASER, INNER, RaWt)