Hi Ash,
Apologies for the late reply.
On 05/11/2020 18:55, Ash Wilding wrote:
The current Arm boot_cpu_feature64() and boot_cpu_feature32() macros
are hardcoded to only detect features in ID_AA64PFR{0,1}_EL1 and
ID_PFR{0,1} respectively.
This patch replaces these macros with a new macro, boot_cpu_feature(),
which takes an explicit ID register name as an argument.
While we're here, cull cpu_feature64() and cpu_feature32() as they
have no callers (we only ever use the boot CPU features), and update
the printk() messages in setup.c to use the new macro.
Signed-off-by: Ash Wilding <ash.j.wild...@gmail.com>
---
xen/arch/arm/setup.c | 8 +++---
xen/include/asm-arm/cpufeature.h | 44 +++++++++++++++-----------------
2 files changed, 24 insertions(+), 28 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 7fcff9af2a..5121f06fc5 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -134,16 +134,16 @@ static void __init processor_id(void)
cpu_has_gicv3 ? " GICv3-SysReg" : "");
/* Warn user if we find unknown floating-point features */
- if ( cpu_has_fp && (boot_cpu_feature64(fp) >= 2) )
+ if ( cpu_has_fp && (boot_cpu_feature(pfr64, fp) >= 2) )
printk(XENLOG_WARNING "WARNING: Unknown Floating-point ID:%d, "
"this may result in corruption on the platform\n",
- boot_cpu_feature64(fp));
+ boot_cpu_feature(pfr64, fp));
/* Warn user if we find unknown AdvancedSIMD features */
- if ( cpu_has_simd && (boot_cpu_feature64(simd) >= 2) )
+ if ( cpu_has_simd && (boot_cpu_feature(pfr64, simd) >= 2) )
printk(XENLOG_WARNING "WARNING: Unknown AdvancedSIMD ID:%d, "
"this may result in corruption on the platform\n",
- boot_cpu_feature64(simd));
+ boot_cpu_feature(pfr64, simd));
printk(" Debug Features: %016"PRIx64" %016"PRIx64"\n",
boot_cpu_data.dbg64.bits[0], boot_cpu_data.dbg64.bits[1]);
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 10878ead8a..f9281ea343 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -1,39 +1,35 @@
#ifndef __ASM_ARM_CPUFEATURE_H
#define __ASM_ARM_CPUFEATURE_H
+#define boot_cpu_feature(idreg, feat) (boot_cpu_data.idreg.feat)
+
#ifdef CONFIG_ARM_64
-#define cpu_feature64(c, feat) ((c)->pfr64.feat)
-#define boot_cpu_feature64(feat) (boot_cpu_data.pfr64.feat)
-
-#define cpu_has_el0_32 (boot_cpu_feature64(el0) == 2)
-#define cpu_has_el0_64 (boot_cpu_feature64(el0) >= 1)
-#define cpu_has_el1_32 (boot_cpu_feature64(el1) == 2)
-#define cpu_has_el1_64 (boot_cpu_feature64(el1) >= 1)
-#define cpu_has_el2_32 (boot_cpu_feature64(el2) == 2)
-#define cpu_has_el2_64 (boot_cpu_feature64(el2) >= 1)
-#define cpu_has_el3_32 (boot_cpu_feature64(el3) == 2)
-#define cpu_has_el3_64 (boot_cpu_feature64(el3) >= 1)
-#define cpu_has_fp (boot_cpu_feature64(fp) < 8)
-#define cpu_has_simd (boot_cpu_feature64(simd) < 8)
-#define cpu_has_gicv3 (boot_cpu_feature64(gic) == 1)
+#define cpu_has_el0_32 (boot_cpu_feature(pfr64, el0) == 2)
+#define cpu_has_el0_64 (boot_cpu_feature(pfr64, el0) >= 1)
+#define cpu_has_el1_32 (boot_cpu_feature(pfr64, el1) == 2)
+#define cpu_has_el1_64 (boot_cpu_feature(pfr64, el1) >= 1)
+#define cpu_has_el2_32 (boot_cpu_feature(pfr64, el2) == 2)
+#define cpu_has_el2_64 (boot_cpu_feature(pfr64, el2) >= 1)
+#define cpu_has_el3_32 (boot_cpu_feature(pfr64, el3) == 2)
+#define cpu_has_el3_64 (boot_cpu_feature(pfr64, el3) >= 1)
+#define cpu_has_fp (boot_cpu_feature(pfr64, fp) < 8)
+#define cpu_has_simd (boot_cpu_feature(pfr64, simd) < 8)
+#define cpu_has_gicv3 (boot_cpu_feature(pfr64, gic) == 1)
May I ask why the indentation was changed here?
The rest of the patch looks good to me:
Acked-by: Julien Grall <jgr...@amazon.com>
#endif
-#define cpu_feature32(c, feat) ((c)->pfr32.feat)
-#define boot_cpu_feature32(feat) (boot_cpu_data.pfr32.feat)
-
-#define cpu_has_arm (boot_cpu_feature32(arm) == 1)
-#define cpu_has_thumb (boot_cpu_feature32(thumb) >= 1)
-#define cpu_has_thumb2 (boot_cpu_feature32(thumb) >= 3)
-#define cpu_has_jazelle (boot_cpu_feature32(jazelle) > 0)
-#define cpu_has_thumbee (boot_cpu_feature32(thumbee) == 1)
+#define cpu_has_arm (boot_cpu_feature(pfr32, arm) == 1)
+#define cpu_has_thumb (boot_cpu_feature(pfr32, thumb) >= 1)
+#define cpu_has_thumb2 (boot_cpu_feature(pfr32, thumb) >= 3)
+#define cpu_has_jazelle (boot_cpu_feature(pfr32, jazelle) > 0)
+#define cpu_has_thumbee (boot_cpu_feature(pfr32, thumbee) == 1)
#define cpu_has_aarch32 (cpu_has_arm || cpu_has_thumb)
#ifdef CONFIG_ARM_32
-#define cpu_has_gentimer (boot_cpu_feature32(gentimer) == 1)
+#define cpu_has_gentimer (boot_cpu_feature(pfr32, gentimer) == 1)
#else
#define cpu_has_gentimer (1)
#endif
-#define cpu_has_security (boot_cpu_feature32(security) > 0)
+#define cpu_has_security (boot_cpu_feature(pfr32, security) > 0)
#define ARM64_WORKAROUND_CLEAN_CACHE 0
#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE 1
Cheers,
--
Julien Grall
