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

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Ayan,

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?

> +
> +    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?

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