[PATCH v3 0/4] x86: RSBA and RRSBA handling

[PATCH v3 0/4] x86: RSBA and RRSBA handling

[Andrew Cooper]

This series deals with the hanlding of the RSBA and RRSBA bits across all
parts and all mistakes encountered in various microcode versions.

There are only minor changes from v2.  See patches for details.

Andrew Cooper (4):
  x86/spec-ctrl: Use a taint for CET without MSR_SPEC_CTRL
  x86/spec-ctrl: Rename retpoline_safe() to retpoline_calculations()
  x86/spec-ctrl: Fix up the RSBA/RRSBA bits as appropriate
  x86/cpu-policy: Derive RSBA/RRSBA for guest policies

 xen/arch/x86/cpu-policy.c                   |  39 ++++++
 xen/arch/x86/include/asm/cpufeature.h       |   1 +
 xen/arch/x86/spec_ctrl.c                    | 142 +++++++++++++++++---
 xen/common/kernel.c                         |   2 +-
 xen/include/public/arch-x86/cpufeatureset.h |   4 +-
 xen/tools/gen-cpuid.py                      |   5 +-
 6 files changed, 170 insertions(+), 23 deletions(-)

-- 
2.30.2

[PATCH v3 1/4] x86/spec-ctrl: Use a taint for CET without MSR_SPEC_CTRL

[Andrew Cooper]

Reword the comment for 'S' to include an incompatbile set of features on the
same core.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

v3:
 * New
---
 xen/arch/x86/spec_ctrl.c | 3 +++
 xen/common/kernel.c      | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index cd5ea6aa52d9..05b86edf73d3 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -1132,7 +1132,10 @@ void __init init_speculation_mitigations(void)
     if ( read_cr4() & X86_CR4_CET )
     {
         if ( !has_spec_ctrl )
+        {
             printk(XENLOG_WARNING "?!? CET active, but no MSR_SPEC_CTRL?\n");
+            add_taint(TAINT_CPU_OUT_OF_SPEC);
+        }
         else if ( opt_ibrs == -1 )
             opt_ibrs = ibrs = true;
 
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index fd975ae21ebc..719b08d6c76a 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -373,7 +373,7 @@ unsigned int tainted;
  *  'H' - HVM forced emulation prefix is permitted.
  *  'I' - Platform is insecure (usually due to an errata on the platform).
  *  'M' - Machine had a machine check experience.
- *  'S' - Out of spec CPU (One core has a feature incompatible with others).
+ *  'S' - Out of spec CPU (Incompatible features on one or more cores).
  *
  *      The string is overwritten by the next call to print_taint().
  */
-- 
2.30.2

[PATCH v3 2/4] x86/spec-ctrl: Rename retpoline_safe() to retpoline_calculations()

[Andrew Cooper]

This is prep work, split out to simply the diff on the following change.

 * Rename to retpoline_calculations(), and call unconditionally.  It is
   shortly going to synthesise missing enumerations required for guest safety.
 * For the model check switch statement, store the result in a variable and
   break rather than returning directly.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

v2:
 * Extend the 'safe' variable to the entire switch statement.
---
 xen/arch/x86/spec_ctrl.c | 41 +++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 05b86edf73d3..3892ce4d20ba 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -580,9 +580,10 @@ static bool __init check_smt_enabled(void)
 }
 
 /* Calculate whether Retpoline is known-safe on this CPU. */
-static bool __init retpoline_safe(void)
+static bool __init retpoline_calculations(void)
 {
     unsigned int ucode_rev = this_cpu(cpu_sig).rev;
+    bool safe = false;
 
     if ( boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
         return true;
@@ -620,29 +621,31 @@ static bool __init retpoline_safe(void)
     case 0x3f: /* Haswell EX/EP */
     case 0x45: /* Haswell D */
     case 0x46: /* Haswell H */
-        return true;
+        safe = true;
+        break;
 
         /*
          * Broadwell processors are retpoline-safe after specific microcode
          * versions.
          */
     case 0x3d: /* Broadwell */
-        return ucode_rev >= 0x2a;
+        safe = ucode_rev >= 0x2a;      break;
     case 0x47: /* Broadwell H */
-        return ucode_rev >= 0x1d;
+        safe = ucode_rev >= 0x1d;      break;
     case 0x4f: /* Broadwell EP/EX */
-        return ucode_rev >= 0xb000021;
+        safe = ucode_rev >= 0xb000021; break;
     case 0x56: /* Broadwell D */
         switch ( boot_cpu_data.x86_mask )
         {
-        case 2:  return ucode_rev >= 0x15;
-        case 3:  return ucode_rev >= 0x7000012;
-        case 4:  return ucode_rev >= 0xf000011;
-        case 5:  return ucode_rev >= 0xe000009;
+        case 2:  safe = ucode_rev >= 0x15;      break;
+        case 3:  safe = ucode_rev >= 0x7000012; break;
+        case 4:  safe = ucode_rev >= 0xf000011; break;
+        case 5:  safe = ucode_rev >= 0xe000009; break;
         default:
             printk("Unrecognised CPU stepping %#x - assuming not reptpoline safe\n",
                    boot_cpu_data.x86_mask);
-            return false;
+            safe = false;
+            break;
         }
         break;
 
@@ -656,7 +659,8 @@ static bool __init retpoline_safe(void)
     case 0x67: /* Cannonlake? */
     case 0x8e: /* Kabylake M */
     case 0x9e: /* Kabylake D */
-        return false;
+        safe = false;
+        break;
 
         /*
          * Atom processors before Goldmont Plus/Gemini Lake are retpoline-safe.
@@ -675,13 +679,17 @@ static bool __init retpoline_safe(void)
     case 0x5c: /* Goldmont */
     case 0x5f: /* Denverton */
     case 0x85: /* Knights Mill */
-        return true;
+        safe = true;
+        break;
 
     default:
         printk("Unrecognised CPU model %#x - assuming not reptpoline safe\n",
                boot_cpu_data.x86_model);
-        return false;
+        safe = false;
+        break;
     }
+
+    return safe;
 }
 
 /*
@@ -1114,7 +1122,7 @@ void __init init_speculation_mitigations(void)
 {
     enum ind_thunk thunk = THUNK_DEFAULT;
     bool has_spec_ctrl, ibrs = false, hw_smt_enabled;
-    bool cpu_has_bug_taa;
+    bool cpu_has_bug_taa, retpoline_safe;
 
     hw_smt_enabled = check_smt_enabled();
 
@@ -1143,6 +1151,9 @@ void __init init_speculation_mitigations(void)
             thunk = THUNK_JMP;
     }
 
+    /* Determine if retpoline is safe on this CPU. */
+    retpoline_safe = retpoline_calculations();
+
     /*
      * Has the user specified any custom BTI mitigations?  If so, follow their
      * instructions exactly and disable all heuristics.
@@ -1164,7 +1175,7 @@ void __init init_speculation_mitigations(void)
              * On all hardware, we'd like to use retpoline in preference to
              * IBRS, but only if it is safe on this hardware.
              */
-            if ( retpoline_safe() )
+            if ( retpoline_safe )
                 thunk = THUNK_RETPOLINE;
             else if ( has_spec_ctrl )
                 ibrs = true;
-- 
2.30.2

[PATCH v3 3/4] x86/spec-ctrl: Fix up the RSBA/RRSBA bits as appropriate

[Andrew Cooper]

In order to level a VM safely for migration, the toolstack needs to know the
RSBA/RRSBA properties of the CPU, whether or not they happen to be enumerated.

See the code comment for details.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

v3:
 * Add a taint for bad EIBRS vs RSBA/RRSBA.
 * Minor comment improvements.

v2:
 * Rewrite almost from scratch.
---
 xen/arch/x86/include/asm/cpufeature.h |   1 +
 xen/arch/x86/spec_ctrl.c              | 100 ++++++++++++++++++++++++--
 2 files changed, 96 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index ace31e3b1f1a..e2cb8f3cc728 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -193,6 +193,7 @@ static inline bool boot_cpu_has(unsigned int feat)
 #define cpu_has_tsx_ctrl        boot_cpu_has(X86_FEATURE_TSX_CTRL)
 #define cpu_has_taa_no          boot_cpu_has(X86_FEATURE_TAA_NO)
 #define cpu_has_fb_clear        boot_cpu_has(X86_FEATURE_FB_CLEAR)
+#define cpu_has_rrsba           boot_cpu_has(X86_FEATURE_RRSBA)
 
 /* Synthesized. */
 #define cpu_has_arch_perfmon    boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 3892ce4d20ba..fb1b59b4d7e3 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -579,7 +579,10 @@ static bool __init check_smt_enabled(void)
     return false;
 }
 
-/* Calculate whether Retpoline is known-safe on this CPU. */
+/*
+ * Calculate whether Retpoline is known-safe on this CPU.  Fix up the
+ * RSBA/RRSBA bits as necessary.
+ */
 static bool __init retpoline_calculations(void)
 {
     unsigned int ucode_rev = this_cpu(cpu_sig).rev;
@@ -593,15 +596,93 @@ static bool __init retpoline_calculations(void)
         return false;
 
     /*
-     * RSBA may be set by a hypervisor to indicate that we may move to a
-     * processor which isn't retpoline-safe.
+     * The meaning of the RSBA and RRSBA bits have evolved over time.  The
+     * agreed upon meaning at the time of writing (May 2023) is thus:
+     *
+     * - RSBA (RSB Alternative) means that an RSB may fall back to an
+     *   alternative predictor on underflow.  Skylake uarch and later all have
+     *   this property.  Broadwell too, when running microcode versions prior
+     *   to Jan 2018.
+     *
+     * - All eIBRS-capable processors suffer RSBA, but eIBRS also introduces
+     *   tagging of predictions with the mode in which they were learned.  So
+     *   when eIBRS is active, RSBA becomes RRSBA (Restricted RSBA).
+     *
+     * - CPUs are not expected to enumerate both RSBA and RRSBA.
+     *
+     * Some parts (Broadwell) are not expected to ever enumerate this
+     * behaviour directly.  Other parts have differing enumeration with
+     * microcode version.  Fix up Xen's idea, so we can advertise them safely
+     * to guests, and so toolstacks can level a VM safety for migration.
+     *
+     * The following states exist:
+     *
+     * |   | RSBA | EIBRS | RRSBA | Notes              | Action        |
+     * |---+------+-------+-------+--------------------+---------------|
+     * | 1 |    0 |     0 |     0 | OK (older parts)   | Maybe +RSBA   |
+     * | 2 |    0 |     0 |     1 | Broken             | +RSBA, -RRSBA |
+     * | 3 |    0 |     1 |     0 | OK (pre-Aug ucode) | +RRSBA        |
+     * | 4 |    0 |     1 |     1 | OK                 |               |
+     * | 5 |    1 |     0 |     0 | OK                 |               |
+     * | 6 |    1 |     0 |     1 | Broken             | -RRSBA        |
+     * | 7 |    1 |     1 |     0 | Broken             | -RSBA, +RRSBA |
+     * | 8 |    1 |     1 |     1 | Broken             | -RSBA         |
+     *
+     * However, we don't need perfect adherence to the spec.  We only need
+     * RSBA || RRSBA to indicate "alternative predictors potentially in use".
+     * Rows 1 & 3 are fixed up by later logic, as they're known configurations
+     * which exist in the world.
      *
+     * Complain loudly at the broken cases. They're safe for Xen to use (so we
+     * don't attempt to correct), and may or may not exist in reality, but if
+     * we ever encoutner them in practice, something is wrong and needs
+     * further investigation.
+     */
+    if ( cpu_has_eibrs ? cpu_has_rsba  /* Rows 7, 8 */
+                       : cpu_has_rrsba /* Rows 2, 6 */ )
+    {
+        printk(XENLOG_ERR
+               "FIRMWARE BUG: CPU %02x-%02x-%02x, ucode 0x%08x: RSBA %u, EIBRS %u, RRSBA %u\n",
+               boot_cpu_data.x86, boot_cpu_data.x86_model,
+               boot_cpu_data.x86_mask, ucode_rev,
+               cpu_has_rsba, cpu_has_eibrs, cpu_has_rrsba);
+        add_taint(TAINT_CPU_OUT_OF_SPEC);
+    }
+
+    /*
      * Processors offering Enhanced IBRS are not guarenteed to be
      * repoline-safe.
      */
-    if ( cpu_has_rsba || cpu_has_eibrs )
+    if ( cpu_has_eibrs )
+    {
+        /*
+         * Prior to the August 2023 microcode, many eIBRS-capable parts did
+         * not enumerate RRSBA.
+         */
+        if ( !cpu_has_rrsba )
+            setup_force_cpu_cap(X86_FEATURE_RRSBA);
+
+        return false;
+    }
+
+    /*
+     * RSBA is explicitly enumerated in some cases, but may also be set by a
+     * hypervisor to indicate that we may move to a processor which isn't
+     * retpoline-safe.
+     */
+    if ( cpu_has_rsba )
         return false;
 
+    /*
+     * At this point, we've filtered all the legal RSBA || RRSBA cases (or the
+     * known non-ideal cases).  If ARCH_CAPS is visible, trust the absence of
+     * RSBA || RRSBA.  There's no known microcode which advertises ARCH_CAPS
+     * without RSBA or EIBRS, and if we're virtualised we can't rely the model
+     * check anyway.
+     */
+    if ( cpu_has_arch_caps )
+        return true;
+
     switch ( boot_cpu_data.x86_model )
     {
     case 0x17: /* Penryn */
@@ -689,6 +770,15 @@ static bool __init retpoline_calculations(void)
         break;
     }
 
+    if ( !safe )
+    {
+        /*
+         * Note: the eIBRS-capable parts are filtered out earlier, so the
+         * remainder here are the ones which suffer RSBA behaviour.
+         */
+        setup_force_cpu_cap(X86_FEATURE_RSBA);
+    }
+
     return safe;
 }
 
@@ -1151,7 +1241,7 @@ void __init init_speculation_mitigations(void)
             thunk = THUNK_JMP;
     }
 
-    /* Determine if retpoline is safe on this CPU. */
+    /* Determine if retpoline is safe on this CPU.  Fix up RSBA/RRSBA enumerations. */
     retpoline_safe = retpoline_calculations();
 
     /*
-- 
2.30.2

[PATCH v3 4/4] x86/cpu-policy: Derive RSBA/RRSBA for guest policies

[Andrew Cooper]

The RSBA bit, "RSB Alternative", means that the RSB may use alternative
predictors when empty.  From a practical point of view, this mean "Retpoline
not safe".

Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
statement that IBRS is implemented in hardware (as opposed to the form
retrofitted to existing CPUs in microcode).

The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
property that predictions are tagged with the mode in which they were learnt.
Therefore, it means "when eIBRS is active, the RSB may fall back to
alternative predictors but restricted to the current prediction mode".  As
such, it's stronger statement than RSBA, but still means "Retpoline not safe".

CPUs are not expected to enumerate both RSBA and RRSBA.

Add feature dependencies for EIBRS and RRSBA.  While technically they're not
linked, absolutely nothing good can come of letting the guest see RRSBA
without EIBRS.  Nor a guest seeing EIBRS without IBRSB.  Furthermore, we use
this dependency to simplify the max derivation logic.

The max policies gets RSBA and RRSBA unconditionally set (with the EIBRS
dependency maybe hiding RRSBA).  We can run any VM, even if it has been told
"somewhere you might run, Retpoline isn't safe".

The default policies are more complicated.  A guest shouldn't see both bits,
but it needs to see one if the current host suffers from any form of RSBA, and
which bit it needs to see depends on whether eIBRS is visible or not.
Therefore, the calculation must be performed after sanitise_featureset().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

v3:
 * Minor commit message adjustment.
 * Drop changes to recalculate_cpuid_policy().  Deferred to a later series.

v2:
 * Expand/adjust the comment for the max features.
 * Rewrite the default feature derivation in light of new information.
 * Fix up in recalculate_cpuid_policy() too.
---
 xen/arch/x86/cpu-policy.c                   | 39 +++++++++++++++++++++
 xen/include/public/arch-x86/cpufeatureset.h |  4 +--
 xen/tools/gen-cpuid.py                      |  5 ++-
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index ee256ff5a137..cde7f7605c28 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -423,8 +423,17 @@ static void __init guest_common_max_feature_adjustments(uint32_t *fs)
          * Retpoline not safe)", so these need to be visible to a guest in all
          * cases, even when it's only some other server in the pool which
          * suffers the identified behaviour.
+         *
+         * We can always run any VM which has previously (or will
+         * subsequently) run on hardware where Retpoline is not safe.
+         * Note:
+         *  - The dependency logic may hide RRSBA for other reasons.
+         *  - The max policy does not contitute a sensible configuration to
+         *    run a guest in.
          */
         __set_bit(X86_FEATURE_ARCH_CAPS, fs);
+        __set_bit(X86_FEATURE_RSBA, fs);
+        __set_bit(X86_FEATURE_RRSBA, fs);
     }
 }
 
@@ -532,6 +541,21 @@ static void __init calculate_pv_def_policy(void)
     guest_common_default_feature_adjustments(fs);
 
     sanitise_featureset(fs);
+
+    /*
+     * If the host suffers from RSBA of any form, and the guest can see
+     * MSR_ARCH_CAPS, reflect the appropriate RSBA/RRSBA property to the guest
+     * depending on the visibility of eIBRS.
+     */
+    if ( test_bit(X86_FEATURE_ARCH_CAPS, fs) &&
+         (cpu_has_rsba || cpu_has_rrsba) )
+    {
+        bool eibrs = test_bit(X86_FEATURE_EIBRS, fs);
+
+        __set_bit(eibrs ? X86_FEATURE_RRSBA
+                        : X86_FEATURE_RSBA, fs);
+    }
+
     x86_cpu_featureset_to_policy(fs, p);
     recalculate_xstate(p);
 }
@@ -664,6 +688,21 @@ static void __init calculate_hvm_def_policy(void)
         __set_bit(X86_FEATURE_VIRT_SSBD, fs);
 
     sanitise_featureset(fs);
+
+    /*
+     * If the host suffers from RSBA of any form, and the guest can see
+     * MSR_ARCH_CAPS, reflect the appropriate RSBA/RRSBA property to the guest
+     * depending on the visibility of eIBRS.
+     */
+    if ( test_bit(X86_FEATURE_ARCH_CAPS, fs) &&
+         (cpu_has_rsba || cpu_has_rrsba) )
+    {
+        bool eibrs = test_bit(X86_FEATURE_EIBRS, fs);
+
+        __set_bit(eibrs ? X86_FEATURE_RRSBA
+                        : X86_FEATURE_RSBA, fs);
+    }
+
     x86_cpu_featureset_to_policy(fs, p);
     recalculate_xstate(p);
 }
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index ea779c29879e..ce7407d6a10c 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -311,7 +311,7 @@ XEN_CPUFEATURE(CET_SSS,            15*32+18) /*   CET Supervisor Shadow Stacks s
 /* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.eax, word 16 */
 XEN_CPUFEATURE(RDCL_NO,            16*32+ 0) /*A  No Rogue Data Cache Load (Meltdown) */
 XEN_CPUFEATURE(EIBRS,              16*32+ 1) /*A  Enhanced IBRS */
-XEN_CPUFEATURE(RSBA,               16*32+ 2) /*!A RSB Alternative (Retpoline not safe) */
+XEN_CPUFEATURE(RSBA,               16*32+ 2) /*!  RSB Alternative (Retpoline not safe) */
 XEN_CPUFEATURE(SKIP_L1DFL,         16*32+ 3) /*   Don't need to flush L1D on VMEntry */
 XEN_CPUFEATURE(INTEL_SSB_NO,       16*32+ 4) /*A  No Speculative Store Bypass */
 XEN_CPUFEATURE(MDS_NO,             16*32+ 5) /*A  No Microarchitectural Data Sampling */
@@ -327,7 +327,7 @@ XEN_CPUFEATURE(FBSDP_NO,           16*32+14) /*A  No Fill Buffer Stale Data Prop
 XEN_CPUFEATURE(PSDP_NO,            16*32+15) /*A  No Primary Stale Data Propagation */
 XEN_CPUFEATURE(FB_CLEAR,           16*32+17) /*A  Fill Buffers cleared by VERW */
 XEN_CPUFEATURE(FB_CLEAR_CTRL,      16*32+18) /*   MSR_OPT_CPU_CTRL.FB_CLEAR_DIS */
-XEN_CPUFEATURE(RRSBA,              16*32+19) /*!A Restricted RSB Alternative */
+XEN_CPUFEATURE(RRSBA,              16*32+19) /*!  Restricted RSB Alternative */
 XEN_CPUFEATURE(BHI_NO,             16*32+20) /*A  No Branch History Injection  */
 XEN_CPUFEATURE(XAPIC_STATUS,       16*32+21) /*   MSR_XAPIC_DISABLE_STATUS */
 XEN_CPUFEATURE(OVRCLK_STATUS,      16*32+23) /*   MSR_OVERCLOCKING_STATUS */
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index 973fcc1c64e8..72cf11654ba9 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -318,7 +318,7 @@ def crunch_numbers(state):
         # IBRSB/IBRS, and we pass this MSR directly to guests.  Treating them
         # as dependent features simplifies Xen's logic, and prevents the guest
         # from seeing implausible configurations.
-        IBRSB: [STIBP, SSBD, INTEL_PSFD],
+        IBRSB: [STIBP, SSBD, INTEL_PSFD, EIBRS],
         IBRS: [AMD_STIBP, AMD_SSBD, PSFD, AUTO_IBRS,
                IBRS_ALWAYS, IBRS_FAST, IBRS_SAME_MODE],
         AMD_STIBP: [STIBP_ALWAYS],
@@ -328,6 +328,9 @@ def crunch_numbers(state):
 
         # The ARCH_CAPS CPUID bit enumerates the availability of the whole register.
         ARCH_CAPS: list(range(RDCL_NO, RDCL_NO + 64)),
+
+        # The behaviour described by RRSBA depend on eIBRS being active.
+        EIBRS: [RRSBA],
     }
 
     deep_features = tuple(sorted(deps.keys()))
-- 
2.30.2

Re: [PATCH v3 1/4] x86/spec-ctrl: Use a taint for CET without MSR_SPEC_CTRL

[Jan Beulich]

On 12.06.2023 18:13, Andrew Cooper wrote:
> Reword the comment for 'S' to include an incompatbile set of features on the
> same core.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v3 3/4] x86/spec-ctrl: Fix up the RSBA/RRSBA bits as appropriate

[Jan Beulich]

On 12.06.2023 18:13, Andrew Cooper wrote:
> @@ -593,15 +596,93 @@ static bool __init retpoline_calculations(void)
>          return false;
>  
>      /*
> -     * RSBA may be set by a hypervisor to indicate that we may move to a
> -     * processor which isn't retpoline-safe.
> +     * The meaning of the RSBA and RRSBA bits have evolved over time.  The
> +     * agreed upon meaning at the time of writing (May 2023) is thus:
> +     *
> +     * - RSBA (RSB Alternative) means that an RSB may fall back to an
> +     *   alternative predictor on underflow.  Skylake uarch and later all have
> +     *   this property.  Broadwell too, when running microcode versions prior
> +     *   to Jan 2018.
> +     *
> +     * - All eIBRS-capable processors suffer RSBA, but eIBRS also introduces
> +     *   tagging of predictions with the mode in which they were learned.  So
> +     *   when eIBRS is active, RSBA becomes RRSBA (Restricted RSBA).
> +     *
> +     * - CPUs are not expected to enumerate both RSBA and RRSBA.
> +     *
> +     * Some parts (Broadwell) are not expected to ever enumerate this
> +     * behaviour directly.  Other parts have differing enumeration with
> +     * microcode version.  Fix up Xen's idea, so we can advertise them safely
> +     * to guests, and so toolstacks can level a VM safety for migration.
> +     *
> +     * The following states exist:
> +     *
> +     * |   | RSBA | EIBRS | RRSBA | Notes              | Action        |
> +     * |---+------+-------+-------+--------------------+---------------|
> +     * | 1 |    0 |     0 |     0 | OK (older parts)   | Maybe +RSBA   |
> +     * | 2 |    0 |     0 |     1 | Broken             | +RSBA, -RRSBA |
> +     * | 3 |    0 |     1 |     0 | OK (pre-Aug ucode) | +RRSBA        |
> +     * | 4 |    0 |     1 |     1 | OK                 |               |
> +     * | 5 |    1 |     0 |     0 | OK                 |               |
> +     * | 6 |    1 |     0 |     1 | Broken             | -RRSBA        |
> +     * | 7 |    1 |     1 |     0 | Broken             | -RSBA, +RRSBA |
> +     * | 8 |    1 |     1 |     1 | Broken             | -RSBA         |

You've kept the Action column as you had it originally, despite no longer
applying all the fixups. Wouldn't it make sense to mark those we don't do,
e.g. by enclosing in parentheses?

> +     * However, we don't need perfect adherence to the spec.  We only need
> +     * RSBA || RRSBA to indicate "alternative predictors potentially in use".
> +     * Rows 1 & 3 are fixed up by later logic, as they're known configurations
> +     * which exist in the world.
>       *
> +     * Complain loudly at the broken cases. They're safe for Xen to use (so we
> +     * don't attempt to correct), and may or may not exist in reality, but if
> +     * we ever encoutner them in practice, something is wrong and needs

Nit: "encounter"

> +     * further investigation.
> +     */
> +    if ( cpu_has_eibrs ? cpu_has_rsba  /* Rows 7, 8 */
> +                       : cpu_has_rrsba /* Rows 2, 6 */ )
> +    {
> +        printk(XENLOG_ERR
> +               "FIRMWARE BUG: CPU %02x-%02x-%02x, ucode 0x%08x: RSBA %u, EIBRS %u, RRSBA %u\n",
> +               boot_cpu_data.x86, boot_cpu_data.x86_model,
> +               boot_cpu_data.x86_mask, ucode_rev,
> +               cpu_has_rsba, cpu_has_eibrs, cpu_has_rrsba);

Perhaps with adjustments (as you deem them sensible)
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH v3 4/4] x86/cpu-policy: Derive RSBA/RRSBA for guest policies

[Jan Beulich]

On 12.06.2023 18:13, Andrew Cooper wrote:
> The RSBA bit, "RSB Alternative", means that the RSB may use alternative
> predictors when empty.  From a practical point of view, this mean "Retpoline
> not safe".
> 
> Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
> statement that IBRS is implemented in hardware (as opposed to the form
> retrofitted to existing CPUs in microcode).
> 
> The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
> property that predictions are tagged with the mode in which they were learnt.
> Therefore, it means "when eIBRS is active, the RSB may fall back to
> alternative predictors but restricted to the current prediction mode".  As
> such, it's stronger statement than RSBA, but still means "Retpoline not safe".
> 
> CPUs are not expected to enumerate both RSBA and RRSBA.
> 
> Add feature dependencies for EIBRS and RRSBA.  While technically they're not
> linked, absolutely nothing good can come of letting the guest see RRSBA
> without EIBRS.  Nor a guest seeing EIBRS without IBRSB.  Furthermore, we use
> this dependency to simplify the max derivation logic.
> 
> The max policies gets RSBA and RRSBA unconditionally set (with the EIBRS
> dependency maybe hiding RRSBA).  We can run any VM, even if it has been told
> "somewhere you might run, Retpoline isn't safe".
> 
> The default policies are more complicated.  A guest shouldn't see both bits,
> but it needs to see one if the current host suffers from any form of RSBA, and
> which bit it needs to see depends on whether eIBRS is visible or not.
> Therefore, the calculation must be performed after sanitise_featureset().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> 
> v3:
>  * Minor commit message adjustment.
>  * Drop changes to recalculate_cpuid_policy().  Deferred to a later series.

With this dropped, with the title not saying "max/default", and with
the description also not mentioning "live" policies at all, I don't
think this patch is self-consistent (meaning in particular: leaving
aside the fact that there's no way right now to requests e.g. both
RSBA and RRSBA for a guest; aiui it is possible for Dom0).

As you may imagine I'm also curious why you decided to drop this.

Jan

Re: [PATCH v3 3/4] x86/spec-ctrl: Fix up the RSBA/RRSBA bits as appropriate

[Andrew Cooper]

On 13/06/2023 10:30 am, Jan Beulich wrote:
> On 12.06.2023 18:13, Andrew Cooper wrote:
>> @@ -593,15 +596,93 @@ static bool __init retpoline_calculations(void)
>>          return false;
>>  
>>      /*
>> -     * RSBA may be set by a hypervisor to indicate that we may move to a
>> -     * processor which isn't retpoline-safe.
>> +     * The meaning of the RSBA and RRSBA bits have evolved over time.  The
>> +     * agreed upon meaning at the time of writing (May 2023) is thus:
>> +     *
>> +     * - RSBA (RSB Alternative) means that an RSB may fall back to an
>> +     *   alternative predictor on underflow.  Skylake uarch and later all have
>> +     *   this property.  Broadwell too, when running microcode versions prior
>> +     *   to Jan 2018.
>> +     *
>> +     * - All eIBRS-capable processors suffer RSBA, but eIBRS also introduces
>> +     *   tagging of predictions with the mode in which they were learned.  So
>> +     *   when eIBRS is active, RSBA becomes RRSBA (Restricted RSBA).
>> +     *
>> +     * - CPUs are not expected to enumerate both RSBA and RRSBA.
>> +     *
>> +     * Some parts (Broadwell) are not expected to ever enumerate this
>> +     * behaviour directly.  Other parts have differing enumeration with
>> +     * microcode version.  Fix up Xen's idea, so we can advertise them safely
>> +     * to guests, and so toolstacks can level a VM safety for migration.
>> +     *
>> +     * The following states exist:
>> +     *
>> +     * |   | RSBA | EIBRS | RRSBA | Notes              | Action        |
>> +     * |---+------+-------+-------+--------------------+---------------|
>> +     * | 1 |    0 |     0 |     0 | OK (older parts)   | Maybe +RSBA   |
>> +     * | 2 |    0 |     0 |     1 | Broken             | +RSBA, -RRSBA |
>> +     * | 3 |    0 |     1 |     0 | OK (pre-Aug ucode) | +RRSBA        |
>> +     * | 4 |    0 |     1 |     1 | OK                 |               |
>> +     * | 5 |    1 |     0 |     0 | OK                 |               |
>> +     * | 6 |    1 |     0 |     1 | Broken             | -RRSBA        |
>> +     * | 7 |    1 |     1 |     0 | Broken             | -RSBA, +RRSBA |
>> +     * | 8 |    1 |     1 |     1 | Broken             | -RSBA         |
> You've kept the Action column as you had it originally, despite no longer
> applying all the fixups. Wouldn't it make sense to mark those we don't do,
> e.g. by enclosing in parentheses?

Hmm, yes.  How does this look?

|   | RSBA | EIBRS | RRSBA | Notes              | Action (in principle) |
|---+------+-------+-------+--------------------+-----------------------|
| 1 |    0 |     0 |     0 | OK (older parts)   | Maybe +RSBA           |
| 2 |    0 |     0 |     1 | Broken             | (+RSBA, -RRSBA)       |
| 3 |    0 |     1 |     0 | OK (pre-Aug ucode) | +RRSBA                |
| 4 |    0 |     1 |     1 | OK                 |                       |
| 5 |    1 |     0 |     0 | OK                 |                       |
| 6 |    1 |     0 |     1 | Broken             | (-RRSBA)              |
| 7 |    1 |     1 |     0 | Broken             | (-RSBA, +RRSBA)       |
| 8 |    1 |     1 |     1 | Broken             | (-RSBA)               |


>> +     * further investigation.
>> +     */
>> +    if ( cpu_has_eibrs ? cpu_has_rsba  /* Rows 7, 8 */
>> +                       : cpu_has_rrsba /* Rows 2, 6 */ )
>> +    {
>> +        printk(XENLOG_ERR
>> +               "FIRMWARE BUG: CPU %02x-%02x-%02x, ucode 0x%08x: RSBA %u, EIBRS %u, RRSBA %u\n",
>> +               boot_cpu_data.x86, boot_cpu_data.x86_model,
>> +               boot_cpu_data.x86_mask, ucode_rev,
>> +               cpu_has_rsba, cpu_has_eibrs, cpu_has_rrsba);
> Perhaps with adjustments (as you deem them sensible)
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

~Andrew

Re: [PATCH v3 4/4] x86/cpu-policy: Derive RSBA/RRSBA for guest policies

[Andrew Cooper]

On 13/06/2023 10:59 am, Jan Beulich wrote:
> On 12.06.2023 18:13, Andrew Cooper wrote:
>> The RSBA bit, "RSB Alternative", means that the RSB may use alternative
>> predictors when empty.  From a practical point of view, this mean "Retpoline
>> not safe".
>>
>> Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
>> statement that IBRS is implemented in hardware (as opposed to the form
>> retrofitted to existing CPUs in microcode).
>>
>> The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
>> property that predictions are tagged with the mode in which they were learnt.
>> Therefore, it means "when eIBRS is active, the RSB may fall back to
>> alternative predictors but restricted to the current prediction mode".  As
>> such, it's stronger statement than RSBA, but still means "Retpoline not safe".
>>
>> CPUs are not expected to enumerate both RSBA and RRSBA.
>>
>> Add feature dependencies for EIBRS and RRSBA.  While technically they're not
>> linked, absolutely nothing good can come of letting the guest see RRSBA
>> without EIBRS.  Nor a guest seeing EIBRS without IBRSB.  Furthermore, we use
>> this dependency to simplify the max derivation logic.
>>
>> The max policies gets RSBA and RRSBA unconditionally set (with the EIBRS
>> dependency maybe hiding RRSBA).  We can run any VM, even if it has been told
>> "somewhere you might run, Retpoline isn't safe".
>>
>> The default policies are more complicated.  A guest shouldn't see both bits,
>> but it needs to see one if the current host suffers from any form of RSBA, and
>> which bit it needs to see depends on whether eIBRS is visible or not.
>> Therefore, the calculation must be performed after sanitise_featureset().
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Wei Liu <wl@xen.org>
>>
>> v3:
>>  * Minor commit message adjustment.
>>  * Drop changes to recalculate_cpuid_policy().  Deferred to a later series.
> With this dropped, with the title not saying "max/default", and with
> the description also not mentioning "live" policies at all, I don't
> think this patch is self-consistent (meaning in particular: leaving
> aside the fact that there's no way right now to requests e.g. both
> RSBA and RRSBA for a guest; aiui it is possible for Dom0).
>
> As you may imagine I'm also curious why you decided to drop this.

Because when I tried doing levelling in Xapi, I remembered why I did it
the way I did in v1, and why the v2 way was wrong.

Xen cannot safely edit what the toolstack provides, so must not. 
Instead, failing the set_policy() call is an option, and is what we want
to do longterm, but also happens to be wrong too in this case. An admin
may know that a VM isn't using retpoline, and may need to migrate it
anyway for a number of reasons, so any safety checks need to be in the
toolstack, and need to be overrideable with something like --force.


I don't really associate "derive policies" with anything other than the
system policies.  Domain construction isn't any kind of derivation -
it's simply doing what the toolstack asks.

~Andrew

Re: [PATCH v3 3/4] x86/spec-ctrl: Fix up the RSBA/RRSBA bits as appropriate

[Jan Beulich]

On 14.06.2023 19:25, Andrew Cooper wrote:
> On 13/06/2023 10:30 am, Jan Beulich wrote:
>> On 12.06.2023 18:13, Andrew Cooper wrote:
>>> @@ -593,15 +596,93 @@ static bool __init retpoline_calculations(void)
>>>          return false;
>>>  
>>>      /*
>>> -     * RSBA may be set by a hypervisor to indicate that we may move to a
>>> -     * processor which isn't retpoline-safe.
>>> +     * The meaning of the RSBA and RRSBA bits have evolved over time.  The
>>> +     * agreed upon meaning at the time of writing (May 2023) is thus:
>>> +     *
>>> +     * - RSBA (RSB Alternative) means that an RSB may fall back to an
>>> +     *   alternative predictor on underflow.  Skylake uarch and later all have
>>> +     *   this property.  Broadwell too, when running microcode versions prior
>>> +     *   to Jan 2018.
>>> +     *
>>> +     * - All eIBRS-capable processors suffer RSBA, but eIBRS also introduces
>>> +     *   tagging of predictions with the mode in which they were learned.  So
>>> +     *   when eIBRS is active, RSBA becomes RRSBA (Restricted RSBA).
>>> +     *
>>> +     * - CPUs are not expected to enumerate both RSBA and RRSBA.
>>> +     *
>>> +     * Some parts (Broadwell) are not expected to ever enumerate this
>>> +     * behaviour directly.  Other parts have differing enumeration with
>>> +     * microcode version.  Fix up Xen's idea, so we can advertise them safely
>>> +     * to guests, and so toolstacks can level a VM safety for migration.
>>> +     *
>>> +     * The following states exist:
>>> +     *
>>> +     * |   | RSBA | EIBRS | RRSBA | Notes              | Action        |
>>> +     * |---+------+-------+-------+--------------------+---------------|
>>> +     * | 1 |    0 |     0 |     0 | OK (older parts)   | Maybe +RSBA   |
>>> +     * | 2 |    0 |     0 |     1 | Broken             | +RSBA, -RRSBA |
>>> +     * | 3 |    0 |     1 |     0 | OK (pre-Aug ucode) | +RRSBA        |
>>> +     * | 4 |    0 |     1 |     1 | OK                 |               |
>>> +     * | 5 |    1 |     0 |     0 | OK                 |               |
>>> +     * | 6 |    1 |     0 |     1 | Broken             | -RRSBA        |
>>> +     * | 7 |    1 |     1 |     0 | Broken             | -RSBA, +RRSBA |
>>> +     * | 8 |    1 |     1 |     1 | Broken             | -RSBA         |
>> You've kept the Action column as you had it originally, despite no longer
>> applying all the fixups. Wouldn't it make sense to mark those we don't do,
>> e.g. by enclosing in parentheses?
> 
> Hmm, yes.  How does this look?
> 
> |   | RSBA | EIBRS | RRSBA | Notes              | Action (in principle) |
> |---+------+-------+-------+--------------------+-----------------------|
> | 1 |    0 |     0 |     0 | OK (older parts)   | Maybe +RSBA           |
> | 2 |    0 |     0 |     1 | Broken             | (+RSBA, -RRSBA)       |
> | 3 |    0 |     1 |     0 | OK (pre-Aug ucode) | +RRSBA                |
> | 4 |    0 |     1 |     1 | OK                 |                       |
> | 5 |    1 |     0 |     0 | OK                 |                       |
> | 6 |    1 |     0 |     1 | Broken             | (-RRSBA)              |
> | 7 |    1 |     1 |     0 | Broken             | (-RSBA, +RRSBA)       |
> | 8 |    1 |     1 |     1 | Broken             | (-RSBA)               |

Yes, I think it's better to have it this way, thanks.

Jan

Re: [PATCH v3 4/4] x86/cpu-policy: Derive RSBA/RRSBA for guest policies

[Jan Beulich]

On 14.06.2023 20:12, Andrew Cooper wrote:
> On 13/06/2023 10:59 am, Jan Beulich wrote:
>> On 12.06.2023 18:13, Andrew Cooper wrote:
>>> The RSBA bit, "RSB Alternative", means that the RSB may use alternative
>>> predictors when empty.  From a practical point of view, this mean "Retpoline
>>> not safe".
>>>
>>> Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
>>> statement that IBRS is implemented in hardware (as opposed to the form
>>> retrofitted to existing CPUs in microcode).
>>>
>>> The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
>>> property that predictions are tagged with the mode in which they were learnt.
>>> Therefore, it means "when eIBRS is active, the RSB may fall back to
>>> alternative predictors but restricted to the current prediction mode".  As
>>> such, it's stronger statement than RSBA, but still means "Retpoline not safe".
>>>
>>> CPUs are not expected to enumerate both RSBA and RRSBA.
>>>
>>> Add feature dependencies for EIBRS and RRSBA.  While technically they're not
>>> linked, absolutely nothing good can come of letting the guest see RRSBA
>>> without EIBRS.  Nor a guest seeing EIBRS without IBRSB.  Furthermore, we use
>>> this dependency to simplify the max derivation logic.
>>>
>>> The max policies gets RSBA and RRSBA unconditionally set (with the EIBRS
>>> dependency maybe hiding RRSBA).  We can run any VM, even if it has been told
>>> "somewhere you might run, Retpoline isn't safe".
>>>
>>> The default policies are more complicated.  A guest shouldn't see both bits,
>>> but it needs to see one if the current host suffers from any form of RSBA, and
>>> which bit it needs to see depends on whether eIBRS is visible or not.
>>> Therefore, the calculation must be performed after sanitise_featureset().
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Jan Beulich <JBeulich@suse.com>
>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>> CC: Wei Liu <wl@xen.org>
>>>
>>> v3:
>>>  * Minor commit message adjustment.
>>>  * Drop changes to recalculate_cpuid_policy().  Deferred to a later series.
>> With this dropped, with the title not saying "max/default", and with
>> the description also not mentioning "live" policies at all, I don't
>> think this patch is self-consistent (meaning in particular: leaving
>> aside the fact that there's no way right now to requests e.g. both
>> RSBA and RRSBA for a guest; aiui it is possible for Dom0).
>>
>> As you may imagine I'm also curious why you decided to drop this.
> 
> Because when I tried doing levelling in Xapi, I remembered why I did it
> the way I did in v1, and why the v2 way was wrong.
> 
> Xen cannot safely edit what the toolstack provides, so must not. 

And this is the part I don't understand: Why can't we correct the
(EIBRS,RSBA,RRSBA) tuple to a combination that is "legal"? At least
as long as ...

> Instead, failing the set_policy() call is an option, and is what we want
> to do longterm,

... we aren't there.

> but also happens to be wrong too in this case. An admin
> may know that a VM isn't using retpoline, and may need to migrate it
> anyway for a number of reasons, so any safety checks need to be in the
> toolstack, and need to be overrideable with something like --force.

Possibly leading to an inconsistent policy exposed to a guest? I
guess this may be the only option when we can't really resolve an
ambiguity, but that isn't the case here, is it?

> I don't really associate "derive policies" with anything other than the
> system policies.  Domain construction isn't any kind of derivation -
> it's simply doing what the toolstack asks.

Hmm, I see. To me, since we do certain adjustments, "derive" still
fits there as well. But I'm not going to insist on a subject
adjustment then, given that imo both ways of looking at things make
some sense.

Jan

Re: [PATCH v3 4/4] x86/cpu-policy: Derive RSBA/RRSBA for guest policies

[Andrew Cooper]

On 15/06/2023 9:30 am, Jan Beulich wrote:
> On 14.06.2023 20:12, Andrew Cooper wrote:
>> On 13/06/2023 10:59 am, Jan Beulich wrote:
>>> On 12.06.2023 18:13, Andrew Cooper wrote:
>>>> The RSBA bit, "RSB Alternative", means that the RSB may use alternative
>>>> predictors when empty.  From a practical point of view, this mean "Retpoline
>>>> not safe".
>>>>
>>>> Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
>>>> statement that IBRS is implemented in hardware (as opposed to the form
>>>> retrofitted to existing CPUs in microcode).
>>>>
>>>> The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
>>>> property that predictions are tagged with the mode in which they were learnt.
>>>> Therefore, it means "when eIBRS is active, the RSB may fall back to
>>>> alternative predictors but restricted to the current prediction mode".  As
>>>> such, it's stronger statement than RSBA, but still means "Retpoline not safe".
>>>>
>>>> CPUs are not expected to enumerate both RSBA and RRSBA.
>>>>
>>>> Add feature dependencies for EIBRS and RRSBA.  While technically they're not
>>>> linked, absolutely nothing good can come of letting the guest see RRSBA
>>>> without EIBRS.  Nor a guest seeing EIBRS without IBRSB.  Furthermore, we use
>>>> this dependency to simplify the max derivation logic.
>>>>
>>>> The max policies gets RSBA and RRSBA unconditionally set (with the EIBRS
>>>> dependency maybe hiding RRSBA).  We can run any VM, even if it has been told
>>>> "somewhere you might run, Retpoline isn't safe".
>>>>
>>>> The default policies are more complicated.  A guest shouldn't see both bits,
>>>> but it needs to see one if the current host suffers from any form of RSBA, and
>>>> which bit it needs to see depends on whether eIBRS is visible or not.
>>>> Therefore, the calculation must be performed after sanitise_featureset().
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> ---
>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>>> CC: Wei Liu <wl@xen.org>
>>>>
>>>> v3:
>>>>  * Minor commit message adjustment.
>>>>  * Drop changes to recalculate_cpuid_policy().  Deferred to a later series.
>>> With this dropped, with the title not saying "max/default", and with
>>> the description also not mentioning "live" policies at all, I don't
>>> think this patch is self-consistent (meaning in particular: leaving
>>> aside the fact that there's no way right now to requests e.g. both
>>> RSBA and RRSBA for a guest; aiui it is possible for Dom0).
>>>
>>> As you may imagine I'm also curious why you decided to drop this.
>> Because when I tried doing levelling in Xapi, I remembered why I did it
>> the way I did in v1, and why the v2 way was wrong.
>>
>> Xen cannot safely edit what the toolstack provides, so must not. 
> And this is the part I don't understand: Why can't we correct the
> (EIBRS,RSBA,RRSBA) tuple to a combination that is "legal"? At least
> as long as ...
>
>> Instead, failing the set_policy() call is an option, and is what we want
>> to do longterm,
> ... we aren't there.
>
>> but also happens to be wrong too in this case. An admin
>> may know that a VM isn't using retpoline, and may need to migrate it
>> anyway for a number of reasons, so any safety checks need to be in the
>> toolstack, and need to be overrideable with something like --force.
> Possibly leading to an inconsistent policy exposed to a guest? I
> guess this may be the only option when we can't really resolve an
> ambiguity, but that isn't the case here, is it?

Wrong.  Xen does not have any knowledge of other hosts the VM might
migrate to.

So while Xen can spot problem combinations *on this host*, which way to
correct the problem combination depends on where the VM might migrate to.

Xen cannot safely correct a problem combination even if you don't wish
to allow the admin the ability to override the safety check.

>
>> I don't really associate "derive policies" with anything other than the
>> system policies.  Domain construction isn't any kind of derivation -
>> it's simply doing what the toolstack asks.
> Hmm, I see. To me, since we do certain adjustments, "derive" still
> fits there as well. But I'm not going to insist on a subject
> adjustment then, given that imo both ways of looking at things make
> some sense.

It's a problem that Xen ever made adjustments behind the toolstack's
back, and this decade of technical debt has been extremely difficult to
address.  I guess I still view it in terms of the end properties, not
the intermediate mess.

~Andrew

Re: [PATCH v3 4/4] x86/cpu-policy: Derive RSBA/RRSBA for guest policies

[Jan Beulich]

On 15.06.2023 12:41, Andrew Cooper wrote:
> On 15/06/2023 9:30 am, Jan Beulich wrote:
>> On 14.06.2023 20:12, Andrew Cooper wrote:
>>> On 13/06/2023 10:59 am, Jan Beulich wrote:
>>>> On 12.06.2023 18:13, Andrew Cooper wrote:
>>>>> The RSBA bit, "RSB Alternative", means that the RSB may use alternative
>>>>> predictors when empty.  From a practical point of view, this mean "Retpoline
>>>>> not safe".
>>>>>
>>>>> Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
>>>>> statement that IBRS is implemented in hardware (as opposed to the form
>>>>> retrofitted to existing CPUs in microcode).
>>>>>
>>>>> The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
>>>>> property that predictions are tagged with the mode in which they were learnt.
>>>>> Therefore, it means "when eIBRS is active, the RSB may fall back to
>>>>> alternative predictors but restricted to the current prediction mode".  As
>>>>> such, it's stronger statement than RSBA, but still means "Retpoline not safe".
>>>>>
>>>>> CPUs are not expected to enumerate both RSBA and RRSBA.
>>>>>
>>>>> Add feature dependencies for EIBRS and RRSBA.  While technically they're not
>>>>> linked, absolutely nothing good can come of letting the guest see RRSBA
>>>>> without EIBRS.  Nor a guest seeing EIBRS without IBRSB.  Furthermore, we use
>>>>> this dependency to simplify the max derivation logic.
>>>>>
>>>>> The max policies gets RSBA and RRSBA unconditionally set (with the EIBRS
>>>>> dependency maybe hiding RRSBA).  We can run any VM, even if it has been told
>>>>> "somewhere you might run, Retpoline isn't safe".
>>>>>
>>>>> The default policies are more complicated.  A guest shouldn't see both bits,
>>>>> but it needs to see one if the current host suffers from any form of RSBA, and
>>>>> which bit it needs to see depends on whether eIBRS is visible or not.
>>>>> Therefore, the calculation must be performed after sanitise_featureset().
>>>>>
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> ---
>>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>>>> CC: Wei Liu <wl@xen.org>
>>>>>
>>>>> v3:
>>>>>  * Minor commit message adjustment.
>>>>>  * Drop changes to recalculate_cpuid_policy().  Deferred to a later series.
>>>> With this dropped, with the title not saying "max/default", and with
>>>> the description also not mentioning "live" policies at all, I don't
>>>> think this patch is self-consistent (meaning in particular: leaving
>>>> aside the fact that there's no way right now to requests e.g. both
>>>> RSBA and RRSBA for a guest; aiui it is possible for Dom0).
>>>>
>>>> As you may imagine I'm also curious why you decided to drop this.
>>> Because when I tried doing levelling in Xapi, I remembered why I did it
>>> the way I did in v1, and why the v2 way was wrong.
>>>
>>> Xen cannot safely edit what the toolstack provides, so must not. 
>> And this is the part I don't understand: Why can't we correct the
>> (EIBRS,RSBA,RRSBA) tuple to a combination that is "legal"? At least
>> as long as ...
>>
>>> Instead, failing the set_policy() call is an option, and is what we want
>>> to do longterm,
>> ... we aren't there.
>>
>>> but also happens to be wrong too in this case. An admin
>>> may know that a VM isn't using retpoline, and may need to migrate it
>>> anyway for a number of reasons, so any safety checks need to be in the
>>> toolstack, and need to be overrideable with something like --force.
>> Possibly leading to an inconsistent policy exposed to a guest? I
>> guess this may be the only option when we can't really resolve an
>> ambiguity, but that isn't the case here, is it?
> 
> Wrong.  Xen does not have any knowledge of other hosts the VM might
> migrate to.
> 
> So while Xen can spot problem combinations *on this host*, which way to
> correct the problem combination depends on where the VM might migrate to.

I actually view this as two different levels: With a flawed policy, the
guest is liable to not work correctly at all. No point thinking about
it being able to migrate. With a fixed up policy it may fail to migrate,
but it'll at least work otherwise.

Jan

Re: [PATCH v3 4/4] x86/cpu-policy: Derive RSBA/RRSBA for guest policies

[Andrew Cooper]

On 15/06/2023 1:13 pm, Jan Beulich wrote:
> On 15.06.2023 12:41, Andrew Cooper wrote:
>> On 15/06/2023 9:30 am, Jan Beulich wrote:
>>> On 14.06.2023 20:12, Andrew Cooper wrote:
>>>> On 13/06/2023 10:59 am, Jan Beulich wrote:
>>>>> On 12.06.2023 18:13, Andrew Cooper wrote:
>>>>>> The RSBA bit, "RSB Alternative", means that the RSB may use alternative
>>>>>> predictors when empty.  From a practical point of view, this mean "Retpoline
>>>>>> not safe".
>>>>>>
>>>>>> Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
>>>>>> statement that IBRS is implemented in hardware (as opposed to the form
>>>>>> retrofitted to existing CPUs in microcode).
>>>>>>
>>>>>> The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
>>>>>> property that predictions are tagged with the mode in which they were learnt.
>>>>>> Therefore, it means "when eIBRS is active, the RSB may fall back to
>>>>>> alternative predictors but restricted to the current prediction mode".  As
>>>>>> such, it's stronger statement than RSBA, but still means "Retpoline not safe".
>>>>>>
>>>>>> CPUs are not expected to enumerate both RSBA and RRSBA.
>>>>>>
>>>>>> Add feature dependencies for EIBRS and RRSBA.  While technically they're not
>>>>>> linked, absolutely nothing good can come of letting the guest see RRSBA
>>>>>> without EIBRS.  Nor a guest seeing EIBRS without IBRSB.  Furthermore, we use
>>>>>> this dependency to simplify the max derivation logic.
>>>>>>
>>>>>> The max policies gets RSBA and RRSBA unconditionally set (with the EIBRS
>>>>>> dependency maybe hiding RRSBA).  We can run any VM, even if it has been told
>>>>>> "somewhere you might run, Retpoline isn't safe".
>>>>>>
>>>>>> The default policies are more complicated.  A guest shouldn't see both bits,
>>>>>> but it needs to see one if the current host suffers from any form of RSBA, and
>>>>>> which bit it needs to see depends on whether eIBRS is visible or not.
>>>>>> Therefore, the calculation must be performed after sanitise_featureset().
>>>>>>
>>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>>> ---
>>>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>>>>> CC: Wei Liu <wl@xen.org>
>>>>>>
>>>>>> v3:
>>>>>>  * Minor commit message adjustment.
>>>>>>  * Drop changes to recalculate_cpuid_policy().  Deferred to a later series.
>>>>> With this dropped, with the title not saying "max/default", and with
>>>>> the description also not mentioning "live" policies at all, I don't
>>>>> think this patch is self-consistent (meaning in particular: leaving
>>>>> aside the fact that there's no way right now to requests e.g. both
>>>>> RSBA and RRSBA for a guest; aiui it is possible for Dom0).
>>>>>
>>>>> As you may imagine I'm also curious why you decided to drop this.
>>>> Because when I tried doing levelling in Xapi, I remembered why I did it
>>>> the way I did in v1, and why the v2 way was wrong.
>>>>
>>>> Xen cannot safely edit what the toolstack provides, so must not. 
>>> And this is the part I don't understand: Why can't we correct the
>>> (EIBRS,RSBA,RRSBA) tuple to a combination that is "legal"? At least
>>> as long as ...
>>>
>>>> Instead, failing the set_policy() call is an option, and is what we want
>>>> to do longterm,
>>> ... we aren't there.
>>>
>>>> but also happens to be wrong too in this case. An admin
>>>> may know that a VM isn't using retpoline, and may need to migrate it
>>>> anyway for a number of reasons, so any safety checks need to be in the
>>>> toolstack, and need to be overrideable with something like --force.
>>> Possibly leading to an inconsistent policy exposed to a guest? I
>>> guess this may be the only option when we can't really resolve an
>>> ambiguity, but that isn't the case here, is it?
>> Wrong.  Xen does not have any knowledge of other hosts the VM might
>> migrate to.
>>
>> So while Xen can spot problem combinations *on this host*, which way to
>> correct the problem combination depends on where the VM might migrate to.
> I actually view this as two different levels: With a flawed policy, the
> guest is liable to not work correctly at all. No point thinking about
> it being able to migrate. With a fixed up policy it may fail to migrate,
> but it'll at least work otherwise.

If you get RSBA and/or RRSBA wrong, nothing is going to malfunction in
the guest, even if you migrate it.

The consequence of getting RSBA and/or RRSBA wrong is the guest *might*
think retpoline is safe to use, and *might* end up vulnerable to
speculative attacks on this or other hardware.

And the admin might know that they overrode the default settings and
forced the use of some other protection mechanism, so the guest is in
fact safe despite having wrong RSBA/RRSBA settings.


I don't know how to put it any more plainly.  Xen *does not* have the
information necessary to make a safety judgement in this matter.  Only
the toolstack (as a proxy for the admin) has the necessary information.

~Andrew

Re: [PATCH v3 4/4] x86/cpu-policy: Derive RSBA/RRSBA for guest policies

[Jan Beulich]

On 15.06.2023 20:17, Andrew Cooper wrote:
> On 15/06/2023 1:13 pm, Jan Beulich wrote:
>> On 15.06.2023 12:41, Andrew Cooper wrote:
>>> On 15/06/2023 9:30 am, Jan Beulich wrote:
>>>> On 14.06.2023 20:12, Andrew Cooper wrote:
>>>>> On 13/06/2023 10:59 am, Jan Beulich wrote:
>>>>>> On 12.06.2023 18:13, Andrew Cooper wrote:
>>>>>>> The RSBA bit, "RSB Alternative", means that the RSB may use alternative
>>>>>>> predictors when empty.  From a practical point of view, this mean "Retpoline
>>>>>>> not safe".
>>>>>>>
>>>>>>> Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
>>>>>>> statement that IBRS is implemented in hardware (as opposed to the form
>>>>>>> retrofitted to existing CPUs in microcode).
>>>>>>>
>>>>>>> The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
>>>>>>> property that predictions are tagged with the mode in which they were learnt.
>>>>>>> Therefore, it means "when eIBRS is active, the RSB may fall back to
>>>>>>> alternative predictors but restricted to the current prediction mode".  As
>>>>>>> such, it's stronger statement than RSBA, but still means "Retpoline not safe".
>>>>>>>
>>>>>>> CPUs are not expected to enumerate both RSBA and RRSBA.
>>>>>>>
>>>>>>> Add feature dependencies for EIBRS and RRSBA.  While technically they're not
>>>>>>> linked, absolutely nothing good can come of letting the guest see RRSBA
>>>>>>> without EIBRS.  Nor a guest seeing EIBRS without IBRSB.  Furthermore, we use
>>>>>>> this dependency to simplify the max derivation logic.
>>>>>>>
>>>>>>> The max policies gets RSBA and RRSBA unconditionally set (with the EIBRS
>>>>>>> dependency maybe hiding RRSBA).  We can run any VM, even if it has been told
>>>>>>> "somewhere you might run, Retpoline isn't safe".
>>>>>>>
>>>>>>> The default policies are more complicated.  A guest shouldn't see both bits,
>>>>>>> but it needs to see one if the current host suffers from any form of RSBA, and
>>>>>>> which bit it needs to see depends on whether eIBRS is visible or not.
>>>>>>> Therefore, the calculation must be performed after sanitise_featureset().
>>>>>>>
>>>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>>>> ---
>>>>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>>>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>>>>>> CC: Wei Liu <wl@xen.org>
>>>>>>>
>>>>>>> v3:
>>>>>>>  * Minor commit message adjustment.
>>>>>>>  * Drop changes to recalculate_cpuid_policy().  Deferred to a later series.
>>>>>> With this dropped, with the title not saying "max/default", and with
>>>>>> the description also not mentioning "live" policies at all, I don't
>>>>>> think this patch is self-consistent (meaning in particular: leaving
>>>>>> aside the fact that there's no way right now to requests e.g. both
>>>>>> RSBA and RRSBA for a guest; aiui it is possible for Dom0).
>>>>>>
>>>>>> As you may imagine I'm also curious why you decided to drop this.
>>>>> Because when I tried doing levelling in Xapi, I remembered why I did it
>>>>> the way I did in v1, and why the v2 way was wrong.
>>>>>
>>>>> Xen cannot safely edit what the toolstack provides, so must not. 
>>>> And this is the part I don't understand: Why can't we correct the
>>>> (EIBRS,RSBA,RRSBA) tuple to a combination that is "legal"? At least
>>>> as long as ...
>>>>
>>>>> Instead, failing the set_policy() call is an option, and is what we want
>>>>> to do longterm,
>>>> ... we aren't there.
>>>>
>>>>> but also happens to be wrong too in this case. An admin
>>>>> may know that a VM isn't using retpoline, and may need to migrate it
>>>>> anyway for a number of reasons, so any safety checks need to be in the
>>>>> toolstack, and need to be overrideable with something like --force.
>>>> Possibly leading to an inconsistent policy exposed to a guest? I
>>>> guess this may be the only option when we can't really resolve an
>>>> ambiguity, but that isn't the case here, is it?
>>> Wrong.  Xen does not have any knowledge of other hosts the VM might
>>> migrate to.
>>>
>>> So while Xen can spot problem combinations *on this host*, which way to
>>> correct the problem combination depends on where the VM might migrate to.
>> I actually view this as two different levels: With a flawed policy, the
>> guest is liable to not work correctly at all. No point thinking about
>> it being able to migrate. With a fixed up policy it may fail to migrate,
>> but it'll at least work otherwise.
> 
> If you get RSBA and/or RRSBA wrong, nothing is going to malfunction in
> the guest, even if you migrate it.
> 
> The consequence of getting RSBA and/or RRSBA wrong is the guest *might*
> think retpoline is safe to use, and *might* end up vulnerable to
> speculative attacks on this or other hardware.

Isn't that some sort of "malfunction"?

> And the admin might know that they overrode the default settings and
> forced the use of some other protection mechanism, so the guest is in
> fact safe despite having wrong RSBA/RRSBA settings.

But then the guest would also be safe with adjusted settings, wouldn't it?

> I don't know how to put it any more plainly.  Xen *does not* have the
> information necessary to make a safety judgement in this matter.  Only
> the toolstack (as a proxy for the admin) has the necessary information.

I'm not looking at it as Xen making things safe by adjusting bogus
settings. I'm merely looking at it as not letting a guest run that way.
For the safety aspect I agree it needs a wider view than Xen has.

Anyway, I don't think either of us is going to convince the other of
there being only one way of looking at things vs there being at least
two possible ways, so in order to allow things to progress
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH v3 4/4] x86/cpu-policy: Derive RSBA/RRSBA for guest policies

[Andrew Cooper]

On 16/06/2023 1:12 pm, Jan Beulich wrote:
> On 15.06.2023 20:17, Andrew Cooper wrote:
>> On 15/06/2023 1:13 pm, Jan Beulich wrote:
>>> On 15.06.2023 12:41, Andrew Cooper wrote:
>>>> On 15/06/2023 9:30 am, Jan Beulich wrote:
>>>>> On 14.06.2023 20:12, Andrew Cooper wrote:
>>>>>> On 13/06/2023 10:59 am, Jan Beulich wrote:
>>>>>>> On 12.06.2023 18:13, Andrew Cooper wrote:
>>>>>>>> The RSBA bit, "RSB Alternative", means that the RSB may use alternative
>>>>>>>> predictors when empty.  From a practical point of view, this mean "Retpoline
>>>>>>>> not safe".
>>>>>>>>
>>>>>>>> Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
>>>>>>>> statement that IBRS is implemented in hardware (as opposed to the form
>>>>>>>> retrofitted to existing CPUs in microcode).
>>>>>>>>
>>>>>>>> The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
>>>>>>>> property that predictions are tagged with the mode in which they were learnt.
>>>>>>>> Therefore, it means "when eIBRS is active, the RSB may fall back to
>>>>>>>> alternative predictors but restricted to the current prediction mode".  As
>>>>>>>> such, it's stronger statement than RSBA, but still means "Retpoline not safe".
>>>>>>>>
>>>>>>>> CPUs are not expected to enumerate both RSBA and RRSBA.
>>>>>>>>
>>>>>>>> Add feature dependencies for EIBRS and RRSBA.  While technically they're not
>>>>>>>> linked, absolutely nothing good can come of letting the guest see RRSBA
>>>>>>>> without EIBRS.  Nor a guest seeing EIBRS without IBRSB.  Furthermore, we use
>>>>>>>> this dependency to simplify the max derivation logic.
>>>>>>>>
>>>>>>>> The max policies gets RSBA and RRSBA unconditionally set (with the EIBRS
>>>>>>>> dependency maybe hiding RRSBA).  We can run any VM, even if it has been told
>>>>>>>> "somewhere you might run, Retpoline isn't safe".
>>>>>>>>
>>>>>>>> The default policies are more complicated.  A guest shouldn't see both bits,
>>>>>>>> but it needs to see one if the current host suffers from any form of RSBA, and
>>>>>>>> which bit it needs to see depends on whether eIBRS is visible or not.
>>>>>>>> Therefore, the calculation must be performed after sanitise_featureset().
>>>>>>>>
>>>>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>>>>> ---
>>>>>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>>>>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>>>>>>> CC: Wei Liu <wl@xen.org>
>>>>>>>>
>>>>>>>> v3:
>>>>>>>>  * Minor commit message adjustment.
>>>>>>>>  * Drop changes to recalculate_cpuid_policy().  Deferred to a later series.
>>>>>>> With this dropped, with the title not saying "max/default", and with
>>>>>>> the description also not mentioning "live" policies at all, I don't
>>>>>>> think this patch is self-consistent (meaning in particular: leaving
>>>>>>> aside the fact that there's no way right now to requests e.g. both
>>>>>>> RSBA and RRSBA for a guest; aiui it is possible for Dom0).
>>>>>>>
>>>>>>> As you may imagine I'm also curious why you decided to drop this.
>>>>>> Because when I tried doing levelling in Xapi, I remembered why I did it
>>>>>> the way I did in v1, and why the v2 way was wrong.
>>>>>>
>>>>>> Xen cannot safely edit what the toolstack provides, so must not. 
>>>>> And this is the part I don't understand: Why can't we correct the
>>>>> (EIBRS,RSBA,RRSBA) tuple to a combination that is "legal"? At least
>>>>> as long as ...
>>>>>
>>>>>> Instead, failing the set_policy() call is an option, and is what we want
>>>>>> to do longterm,
>>>>> ... we aren't there.
>>>>>
>>>>>> but also happens to be wrong too in this case. An admin
>>>>>> may know that a VM isn't using retpoline, and may need to migrate it
>>>>>> anyway for a number of reasons, so any safety checks need to be in the
>>>>>> toolstack, and need to be overrideable with something like --force.
>>>>> Possibly leading to an inconsistent policy exposed to a guest? I
>>>>> guess this may be the only option when we can't really resolve an
>>>>> ambiguity, but that isn't the case here, is it?
>>>> Wrong.  Xen does not have any knowledge of other hosts the VM might
>>>> migrate to.
>>>>
>>>> So while Xen can spot problem combinations *on this host*, which way to
>>>> correct the problem combination depends on where the VM might migrate to.
>>> I actually view this as two different levels: With a flawed policy, the
>>> guest is liable to not work correctly at all. No point thinking about
>>> it being able to migrate. With a fixed up policy it may fail to migrate,
>>> but it'll at least work otherwise.
>> If you get RSBA and/or RRSBA wrong, nothing is going to malfunction in
>> the guest, even if you migrate it.
>>
>> The consequence of getting RSBA and/or RRSBA wrong is the guest *might*
>> think retpoline is safe to use, and *might* end up vulnerable to
>> speculative attacks on this or other hardware.
> Isn't that some sort of "malfunction"?

Perhaps, there's a difference between "it will likely crash hard" and
"you won't notice the difference".

>
>> And the admin might know that they overrode the default settings and
>> forced the use of some other protection mechanism, so the guest is in
>> fact safe despite having wrong RSBA/RRSBA settings.
> But then the guest would also be safe with adjusted settings, wouldn't it?

It doesn't mean the guest is going to tolerate having features change
underfoot.

>
>> I don't know how to put it any more plainly.  Xen *does not* have the
>> information necessary to make a safety judgement in this matter.  Only
>> the toolstack (as a proxy for the admin) has the necessary information.
> I'm not looking at it as Xen making things safe by adjusting bogus
> settings. I'm merely looking at it as not letting a guest run that way.
> For the safety aspect I agree it needs a wider view than Xen has.
>
> Anyway, I don't think either of us is going to convince the other of
> there being only one way of looking at things vs there being at least
> two possible ways, so in order to allow things to progress
> Acked-by: Jan Beulich <jbeulich@suse.com>

Thankyou.

To be clear, we are planning to put checks in place.  We definitely
don't want the admin to end up in this corner case accidentally.

~Andrew