Re: Why do we mark vpending table as non-shareable in GICR_VPENDBASER?

[Heyi Guo]

After thinking about it for a while, I feel it difficult to image how non shareable plus normal cacheable works for vpending table. Supposing the shareability bits are to direct the corresponding GICR to read/write table memory, if a vPE is first scheduled on pCPU0 with GICR0 and an VLPI is triggered, so a pending bit in vpending table is set by GICR0 (or ITS?); before the interrupt is activated, the vPE is then scheduled on pCPU1 with GICR1, could GICR1 still be guaranteed to see the pending bit in vpending table when the shareability is non-shareable?

It seems more reasonable for physical pending table to be non-shareable, for it is pinned with one GICR. Even with this assumption, the code will still fall back to non-cacheable cacheability when we have no more choice for shareability attribute other than non-shareable, as the comment says:

        /*
         * The HW reports non-shareable, we must remove the
         * cacheability attributes as well.
         */

Did I miss something?

Thanks,
Heyi

On 2019/5/9 15:58, Marc Zyngier wrote:
> On Thu, 09 May 2019 08:10:09 +0100,
> Heyi Guo <guoheyi@xxxxxxxxxx> wrote:
> > Hi Marc,
> >
> > We can see in its_vpe_schedule() the shareability bits of
> > GICR_VPENDBASER are set as non-shareable, But we set physical
> > PENDBASER as inner-shareable. Is there any special reason for doing
> > this? If it is because the vpending table is GICR specific, why
> > don't we do the same for physical pending table?
> That's a good question. They should have similar attributes.
>
> > We have not seen function issue with this setting, but a special
> > detector in our hardware warns us that there are non-shareable
> > requests sent out while some inner shareable cache entries still
> > present in the cache, and it may cause data inconsistent.
> The main issue with the inner-shareable attributes and the GIC is that
> nothing in the spec says that CPUs and GIC have to be in the same
> inner-shareable domain, as the system can have as many as you want.
>
> You obviously have built it with GICR in the same inner-shareability
> domain as the CPU. I'm happy to change the VPENDBASER attributes,
> given that the CPU has a mapping to that memory already, and that
> shouldn't affect systems where GICR isn't in the same inner shareable
> domain anyway.
>
> Thanks,
>
> 	M.