| Message ID | 20221031151326.22634-13-ayankuma@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Arm: Enable GICv3 for AArch32 (v8R) | expand |
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@amd.com> > --- > > 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
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@amd.com> >> --- >> >> 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
On Fri, 4 Nov 2022 at 12:09, Ayan Kumar Halder <ayankuma@amd.com> 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@amd.com> > >> --- > >> > >> 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 >
On 04/11/2022 12:30, Julien Grall wrote: > > > > > > On Fri, 4 Nov 2022 at 12:09, Ayan Kumar Halder <ayankuma@amd.com <mailto:ayankuma@amd.com>> 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@amd.com <mailto:ayankuma@amd.com>> > >> --- > >> > >> 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. +1 ~Michal
Hi Ayan, On 31/10/2022 15: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. s/enable/use/ > 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@amd.com> > --- > > 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 The general apprpoach in SUPPORT.MD is to name the feature and then describe per arch the exact support. For this case it would be: ## ARM/GICv3 GICv3 is an interrupt controller specification designed by Arm. Status, Arm64: Security supported Status, Arm32: Supported, not security supported Cheers,
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) + + 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) #define cpu_has_gentimer (boot_cpu_feature32(gentimer) == 1) /* * On Armv7, the value 0 is used to indicate that PMUv2 is not
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@amd.com> --- 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(-)