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

[Julien Grall <julien@xen.org>]

Hi,

On 11/11/2022 15:17, Ayan Kumar Halder wrote:
> One can now use 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."
> 
> By default GICv3 is disabled on AArch32 and enabled on AArch64.
> 
> Updated SUPPORT.md to state that GICv3 on Arm32 is not security supported.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> 
> Changed from :-
> v1 - 1. Remove "ARM_64 || ARM_32" as it is always true.
> 2. Updated SUPPORT.md.
> 
> v2 - 1. GICv3 is enabled by default only on ARM_64.
> 2. Updated SUPPORT.md.
> 
>   SUPPORT.md                            | 7 +++++++
>   xen/arch/arm/Kconfig                  | 9 +++++----
>   xen/arch/arm/include/asm/cpufeature.h | 1 +
>   3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/SUPPORT.md b/SUPPORT.md
> index ab71464cf6..3f16d83191 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md
> @@ -82,6 +82,13 @@ Extension to the GICv3 interrupt controller to support MSI.
>   
>       Status: Experimental
>   
> +### ARM/GICv3
> +
> +GICv3 is an interrupt controller specification designed by Arm.
> +
> +    Status, Arm64: Security supported
> +    Status, Arm32: Supported, not security supported
> +
>   ## Guest Type
>   
>   ### x86/PV
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 1fe5faf847..b90930955b 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -9,6 +9,7 @@ config ARM_64
>   	select 64BIT
>   	select ARM_EFI
>   	select HAS_FAST_MULTIPLY
> +	select GICV3

AFAIU "select" will force the GICV3 to be enabled. IOW, it would not be 
possible to disable it for Arm64 anymore. You want to remove the 
"select" and ...

>   
>   config ARM
>   	def_bool y
> @@ -41,16 +42,16 @@ config ARM_EFI
>   
>   config GICV3
>   	bool "GICv3 driver"
> -	depends on ARM_64 && !NEW_VGIC
> -	default y
> +	depends on !NEW_VGIC
> +	default n

... replace the "default n" with:

         default n if ARM_32
         default y if ARM_64

>   	---help---
>   
>   	  Driver for the ARM Generic Interrupt Controller v3.
> -	  If unsure, say Y
> +	  If unsure, say N

That's technically incorrect. For Arm64, we still want the user to 
select it by default. So I think we need to differentiate 32-bit vs 
64-bit for the "If unsure" part.

Cheers,

-- 
Julien Grall