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

[Andre Przywara <andre.przywara@xxxxxxx>]

On Tue, 25 Oct 2022 13:25:52 +0100
Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:

Hi,

> 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.

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
>