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

[Julien Grall <julien.grall.oss@xxxxxxxxx>]

On Fri, 4 Nov 2022 at 12:09, Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:

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.

The same could be said for Xen on Aarch32 Armv8. This is not officially supported but works with some tweak in the lookup function.

I should have introduced a new macro AArch32_v8R so that GICV3 is
defined for it.

I would rather not have such config. There are no issue to allow someone to build it for 32-bit because Xen is perfectly capable to detect which GIC version is in use.

Instead we could simply disable GICv3 by default for arm32.

  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