Re: [XEN v2 12/12] xen/Arm: GICv3: Enable GICv3 for AArch32

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 04/11/2022 09:55, Michal Orzel wrote:
> Hi Ayan,
Hi Michal,
> On 31/10/2022 16:13, Ayan Kumar Halder wrote:
> > Refer ARM DDI 0487G.b ID072021,
> > D13.2.86 -
> > ID_PFR1_EL1, AArch32 Processor Feature Register 1
> >
> > GIC, bits[31:28] == 0b0001 for GIC3.0 on Aarch32
> >
> > One can now enable GICv3 on AArch32 systems. However, ITS is not supported.
> > The reason being currently we are trying to validate GICv3 on an AArch32_v8R
> > system. Refer ARM DDI 0568A.c ID110520, B1.3.1,
> > "A Generic Interrupt Controller (GIC) implemented with an Armv8-R PE must not
> > implement LPI support."
> >
> > Updated SUPPORT.md.
> >
> > Signed-off-by: Ayan Kumar Halder <ayankuma@xxxxxxx>
> > ---
> >
> > Changed from :-
> > v1 - 1. Remove "ARM_64 || ARM_32" as it is always true.
> > 2. Updated SUPPORT.md.
> >
> >   SUPPORT.md                            | 6 ++++++
> >   xen/arch/arm/Kconfig                  | 4 ++--
> >   xen/arch/arm/include/asm/cpufeature.h | 1 +
> >   3 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/SUPPORT.md b/SUPPORT.md
> > index cf2ddfacaf..0137855c66 100644
> > --- a/SUPPORT.md
> > +++ b/SUPPORT.md
> > @@ -82,6 +82,12 @@ Extension to the GICv3 interrupt controller to support MSI.
> >
> >       Status: Experimental
> >
> > +### ARM/GICv3 + AArch32 ARM v8
> > +
> > +GICv3 is supported on AArch32 ARMv8 (besides AArch64)
> Looking at the CONFIG_GICV3, it can be enabled on arm32, which at the moment
> supports only ARMv7 (see __lookup_processor_type -> proc-v7.S).
> What will prevent the user from enabling GICv3 for ARMv7 based CPU?
Yes, this is my mistake.

ARMv7 does not support GICv3.

I should have introduced a new macro AArch32_v8R so that GICV3 is 
defined for it.
 config GICV3
        bool "GICv3 driver"
-       depends on ARM_64 && !NEW_VGIC
+       depends on ARM_64 || AArch32_v8R !NEW_VGIC
        default y
        ---help---

> > +
> > +    Status: Supported, not security supported
> > +
> >   ## Guest Type
> >
> >   ### x86/PV
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index 1fe5faf847..7c3c6eb3bd 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -41,7 +41,7 @@ config ARM_EFI
> >
> >   config GICV3
> >          bool "GICv3 driver"
> > -       depends on ARM_64 && !NEW_VGIC
> > +       depends on !NEW_VGIC
> >          default y
> >          ---help---
> >
> > @@ -50,7 +50,7 @@ config GICV3
> >
> >   config HAS_ITS
> >           bool "GICv3 ITS MSI controller support (UNSUPPORTED)" if UNSUPPORTED
> > -        depends on GICV3 && !NEW_VGIC
> > +        depends on GICV3 && !NEW_VGIC && !ARM_32
> >
> >   config HVM
> >           def_bool y
> > diff --git a/xen/arch/arm/include/asm/cpufeature.h 
> > b/xen/arch/arm/include/asm/cpufeature.h
> > index c86a2e7f29..c62cf6293f 100644
> > --- a/xen/arch/arm/include/asm/cpufeature.h
> > +++ b/xen/arch/arm/include/asm/cpufeature.h
> > @@ -33,6 +33,7 @@
> >   #define cpu_has_aarch32   (cpu_has_arm || cpu_has_thumb)
> >
> >   #ifdef CONFIG_ARM_32
> > +#define cpu_has_gicv3     (boot_cpu_feature32(gic) >= 1)
> What is the purpose of defining this macro if it is not used?
It is used in xen/arch/arm/gic-v3.c, gicv3_init().

- Ayan

> >   #define cpu_has_gentimer  (boot_cpu_feature32(gentimer) == 1)
> >   /*
> >    * On Armv7, the value 0 is used to indicate that PMUv2 is not
> > --
> > 2.17.1
> ~Michal