Hi Bertrand,
On 31/05/2022 11:43, Bertrand Marquis wrote:
diff --git a/xen/arch/arm/include/asm/cpufeature.h
b/xen/arch/arm/include/asm/cpufeature.h
index f7368766c0..9649a7afee 100644
--- a/xen/arch/arm/include/asm/cpufeature.h
+++ b/xen/arch/arm/include/asm/cpufeature.h
@@ -67,8 +67,9 @@
#define ARM_WORKAROUND_BHB_LOOP_24 13
#define ARM_WORKAROUND_BHB_LOOP_32 14
#define ARM_WORKAROUND_BHB_SMCC_3 15
+#define ARM64_HAS_SB 16
The feature is for both 32-bit and 64-bit. So I would prefer if it is
called ARM_HAS_SB.
-#define ARM_NCAPS 16
+#define ARM_NCAPS 17
#ifndef __ASSEMBLY__
@@ -78,6 +79,9 @@
extern DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
+void check_local_cpu_features(void);
+void enable_cpu_features(void);
+
static inline bool cpus_have_cap(unsigned int num)
{
if ( num >= ARM_NCAPS )
diff --git a/xen/arch/arm/include/asm/macros.h
b/xen/arch/arm/include/asm/macros.h
index 1aa373760f..33e863d982 100644
--- a/xen/arch/arm/include/asm/macros.h
+++ b/xen/arch/arm/include/asm/macros.h
@@ -5,14 +5,7 @@
# error "This file should only be included in assembly file"
#endif
- /*
- * Speculative barrier
- * XXX: Add support for the 'sb' instruction
- */
- .macro sb
- dsb nsh
- isb
- .endm
Looking at the patch bcab2ac84931 "xen/arm64: Place a speculation
barrier following an ret instruction", the macro was defined before
including <asm/arm*/macros.h> so 'sb' could be used in macros defined by
the headers.
I can't remember whether I chose the order because I had a failure on
some compilers. However, I couldn't find anything in the assembler
documentation suggesting that a macro A could use B before it is used.
So I would rather avoid to move the macro if there are no strong
argument for it.
+#include <asm/alternative.h>
#if defined (CONFIG_ARM_32)
# include <asm/arm32/macros.h>
@@ -29,4 +22,28 @@
.endr
.endm
+ /*
+ * Speculative barrier
+ */
+ .macro sb
+alternative_if_not ARM64_HAS_SB
+ dsb nsh
+ isb
+alternative_else
+ /*
+ * SB encoding in hexadecimal to prevent recursive macro.
+ * extra nop is required to keep same number of instructions on both sides
+ * of the alternative.
+ */
+#if defined(CONFIG_ARM_32)
+ .inst 0xf57ff070
+#elif defined(CONFIG_ARM_64)
+ .inst 0xd50330ff
+#else
+# error "missing sb encoding for ARM variant"
+#endif
+ nop
+alternative_endif
+ .endm
+
#endif /* __ASM_ARM_MACROS_H */
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index ea1f5ee3d3..b44494c9a9 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -961,6 +961,8 @@ void __init start_xen(unsigned long boot_phys_offset,
*/
check_local_cpu_errata();
+ check_local_cpu_features();
+
init_xen_time();
gic_init();
@@ -1030,6 +1032,7 @@ void __init start_xen(unsigned long boot_phys_offset,
*/
apply_alternatives_all();
enable_errata_workarounds();
+ enable_cpu_features();
/* Create initial domain 0. */
if ( !is_dom0less_mode() )
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 9bb32a301a..fb7cc43a93 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -389,6 +389,7 @@ void start_secondary(void)
local_abort_enable();
check_local_cpu_errata();
+ check_local_cpu_features();
printk(XENLOG_DEBUG "CPU %u booted.\n", smp_processor_id());
Cheers,
--
Julien Grall
