[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 04/13] xen/arm: Add ARCH_WORKAROUND_2 probing


On 05/23/2018 10:57 PM, Stefano Stabellini wrote:
On Tue, 22 May 2018, Julien Grall wrote:
As for Spectre variant-2, we rely on SMCCC 1.1 to provide the discovery
mechanism for detecting the SSBD mitigation.

A new capability is also allocated for that purpose, and a config

This is part of XSA-263.

Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
  xen/arch/arm/Kconfig             | 10 ++++++++++
  xen/arch/arm/cpuerrata.c         | 39 +++++++++++++++++++++++++++++++++++++++
  xen/include/asm-arm/cpuerrata.h  | 21 +++++++++++++++++++++
  xen/include/asm-arm/cpufeature.h |  3 ++-
  xen/include/asm-arm/smccc.h      |  6 ++++++
  5 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 8174c0c635..0e2d027060 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -73,6 +73,16 @@ config SBSA_VUART_CONSOLE
          Allows a guest to use SBSA Generic UART as a console. The
          SBSA Generic UART implements a subset of ARM PL011 UART.
+config ARM_SSBD
+       bool "Speculative Store Bypass Disable" if EXPERT = "y"
+       depends on HAS_ALTERNATIVE
+       default y
+       help
+         This enables mitigation of bypassing of previous stores by speculative
+         loads.

I would add a reference to spectre v4. What do you think of:

   This enables the mitigation of Spectre v4 attacks based on bypassing
   of previous memory stores by speculative loads.

Well, the real name is SSBD (Speculative Store Bypass Disable). AFAIK, Spectre only refers to variant 1 and 2 so far. This one has no fancy name and the specifications is using SSBD.

+         If unsure, say Y.
menu "ARM errata workaround via the alternative framework"
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 1baa20654b..bcea2eb6e5 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -235,6 +235,39 @@ static int enable_ic_inv_hardening(void *data)
#endif +#ifdef CONFIG_ARM_SSBD
+ * Assembly code may use the variable directly, so we need to make sure
+ * it fits in a register.
+ */
+DEFINE_PER_CPU_READ_MOSTLY(register_t, ssbd_callback_required);
+static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry)
+    struct arm_smccc_res res;
+    bool supported = true;
+    if ( smccc_ver < SMCCC_VERSION(1, 1) )
+        return false;
+    /*
+     * The probe function return value is either negative (unsupported
+     * or mitigated), positive (unaffected), or zero (requires
+     * mitigation). We only need to do anything in the last case.
+     */
+    arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FID,
+                      ARM_SMCCC_ARCH_WORKAROUND_2_FID, &res);
+    if ( (int)res.a0 != 0 )
+        supported = false;
+    if ( supported )
+        this_cpu(ssbd_callback_required) = 1;
+    return supported;
  #define MIDR_RANGE(model, min, max)     \
      .matches = is_affected_midr_range,  \
      .midr_model = model,                \
@@ -336,6 +369,12 @@ static const struct arm_cpu_capabilities arm_errata[] = {
          .enable = enable_ic_inv_hardening,
+    {
+        .capability = ARM_SSBD,
+        .matches = has_ssbd_mitigation,
+    },
diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
index 4e45b237c8..e628d3ff56 100644
--- a/xen/include/asm-arm/cpuerrata.h
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -27,9 +27,30 @@ static inline bool check_workaround_##erratum(void)          
+#include <asm/current.h>
+DECLARE_PER_CPU(register_t, ssbd_callback_required);

It is becoming more common to have per-cpu capabilities and workarounds
(or at least per MPIDR).

Really? This is the first place where we need an ad-hoc boolean per-CPU. For the hardening branch predictor, we have to store the vector pointer.

Instead of adding this add-hoc variable, should
we make cpu_hwcaps per-cpu, then implement this check with
cpus_have_cap (that would become per-cpu as well)?

It looks like the code would be simpler.

I don't see any benefits for that. Most of the workaround/features are platform wide because they either use alternative or set/clear a bit in the system registers.

Furthermore, as I wrote above the declaration, this is going to be used in assembly code and we need something that can be tested in the less possible number of instructions because The smccc function ARM_ARCH_WORKAROUND_2 is going to be called very often.

Lastly, after the next patch, ssbd_callback_required and ARM_SSBD have different meaning. The former indicates that runtime mitigation is required, while the latter just indicate that the mitigation is present (either runtime or forced enable).


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.