Re: GICv3: Aarch32: Need guidance on the atomic access of "union host_lpi" or if ITS is supported on R52

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 25/10/2022 14:55, Andre Przywara wrote:
> On Tue, 25 Oct 2022 13:25:52 +0100
> Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:
>
> Hi,
Hi Andre,

Many thanks for the explanation.

I need a clarification on the issue of atomic access to 64bit normal 
memory on R52.
> > Hi Andre/All,
> >
> > This came up while porting Xen on R52.
> >
> > Refer "ARM DDI 0568A.cID110520", B1.3.1
> >
> > "A Generic Interrupt Controller (GIC) implemented with an Armv8-R PE
> > must not implement LPI support. "
> >
> > Does this mean ITS is not supported on R52 ? I am assuming yes, please
> > correct me if mistaken.
> An ITS relies on LPIs, so yes: no ITS on a v8-R32 system. I cannot find
> this restriction anymore in the v8-R64 supplement, so it would only apply
> to the R52/AArch32.
>
> > If the answer is no, then my next query is follows :-
> Answering to that anyway ...
>
> > diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
> > index 9ca74bc321..dea60aac0d 100644
> > --- a/xen/arch/arm/gic-v3-lpi.c
> > +++ b/xen/arch/arm/gic-v3-lpi.c
> > @@ -423,7 +423,7 @@ int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
> >        int rc;
> >
> >        /* We rely on the data structure being atomically accessible. */
> > -    BUILD_BUG_ON(sizeof(union host_lpi) > sizeof(unsigned long));
> > +    BUILD_BUG_ON(sizeof(union host_lpi) > sizeof(uint64_t));
> >
> > "unsigned long" on Aarch32 is 32 bits. So this bug gets triggered.
> >
> > Is it fine to change it as above ?
> >
> > Reading"ARM DDI 0487G.bID07202", E2.2.1, "Requirements for single-copy
> > atomicity".
> >
> > "LDRD and STRD accesses to 64-bit aligned locations are 64-bit
> > single-copy atomic as seen by translation table walks and accesses to
> > translation tables"
> This (and the explaining paragraph) indeed suggests that this should
> work architecturally, if you use normal system memory (where you would
> hold page tables). It would be confined to ARMv8 AArch32 and ARMv7 w/
> LPAE, which matches Xen's requirements.
Does it mean that ldrd/strd will not work atomically on AArch32-v8R as 
it uses MPU (not MMU, so no page tables) ?
If so, then is using ldrexd/strexd the solution for this ?

IIUC "Memory accesses caused by LDREXD and STREXD instructions to 
doubleword-aligned locations.", then the answer seems yes.
- Ayan

> But it's only atomic if you are using ldrd/strd, which you cannot know for
> sure in C, because it's up to the compiler to generate the instructions.
>
> This is why we have that test. Changing the unsigned long to uint64_t
> would make the check pointless, since the data structure is 64-bits long,
> so it would always be true.
>
> So given that you don't seem to need it, right now, it would leave the
> test alone.
>
> If you need that on AArch32 anyway, you would need to replace accesses to
> the host_lpis array with inline assembly accessors, to ensure ldrd/strd
> instructions. This seems doable (there are only so many places which
> directly access the array members), but would need a good use case.
>
> Cheers,
> Andre
>
> > Does this imply that atomicity will be retained (with the above change)
> > ? Os will this require ldrexd/strexd as R52 supports MPU (not MMU, so
> > translation tables are irrelevant).
> > Kind regards,
> > Ayan