[PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Roger Pau Monne]

Hello,

The following series implements support for MSR_VIRT_SPEC_CTRL
(VIRT_SSBD) on different AMD CPU families.

Note that the support is added backwards, starting with the newer CPUs
that support MSR_SPEC_CTRL and moving to the older ones either using
MSR_VIRT_SPEC_CTRL or the SSBD bit in LS_CFG.

Xen is still free to use it's own SSBD setting, as the selection is
context switched on vm{entry,exit}.

On Zen2 and later, SPEC_CTRL.SSBD should exist and should be used in
preference to VIRT_SPEC_CTRL.SSBD.  However, for migration
compatibility, Xen offers VIRT_SSBD to guests (in the max cpuid policy,
not default) implemented in terms of SPEC_CTRL.SSBD.

On Fam15h thru Zen1, Xen exposes VIRT_SSBD to guests by default to
abstract away the model and/or hypervisor specific differences in
MSR_LS_CFG/MSR_VIRT_SPEC_CTRL.

So the implementation of VIRT_SSBD exposed to HVM guests will use one of
the following underlying mechanisms, in the preference order listed
below:

 * SPEC_CTRL.SSBD: patch 1
 * VIRT_SPEC_CTRL.SSBD: patch 2.
 * Non-architectural way using LS_CFG: patch 3.

NB: patch 3 introduces some logic in GIF=0 context, such logic has been
kept to a minimum due to the special context it's running on.

Roger Pau Monne (3):
  amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL
  amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
  amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD

 CHANGELOG.md                                |   3 +
 xen/arch/x86/cpu/amd.c                      | 121 +++++++++++++++++---
 xen/arch/x86/cpuid.c                        |  21 ++++
 xen/arch/x86/hvm/hvm.c                      |   1 +
 xen/arch/x86/hvm/svm/entry.S                |   8 ++
 xen/arch/x86/hvm/svm/svm.c                  |  39 +++++++
 xen/arch/x86/include/asm/amd.h              |   4 +
 xen/arch/x86/include/asm/cpufeatures.h      |   1 +
 xen/arch/x86/include/asm/msr.h              |  14 +++
 xen/arch/x86/msr.c                          |  26 +++++
 xen/arch/x86/spec_ctrl.c                    |  12 +-
 xen/include/public/arch-x86/cpufeatureset.h |   2 +-
 12 files changed, 233 insertions(+), 19 deletions(-)

-- 
2.36.0

[PATCH v6 1/3] amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL

[Roger Pau Monne]

Use the logic to set shadow SPEC_CTRL values in order to implement
support for VIRT_SPEC_CTRL (signaled by VIRT_SSBD CPUID flag) for HVM
guests. This includes using the spec_ctrl vCPU MSR variable to store
the guest set value of VIRT_SPEC_CTRL.SSBD, which will be OR'ed with
any SPEC_CTRL values being set by the guest.

On hardware having SPEC_CTRL VIRT_SPEC_CTRL will not be offered by
default to guests. VIRT_SPEC_CTRL will only be part of the max CPUID
policy so it can be enabled for compatibility purposes.

Use '!' to annotate the feature in order to express that the presence
of the bit is not directly tied to its value in the host policy.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Changes since v3:
 - Use '!' to annotate the feature.

Changes since v2:
 - Reword reasoning for using '!s'.
 - Trim comment about only setting SSBD bit in spec_ctrl.raw.

Changes since v1:
 - Only expose VIRT_SSBD if AMD_SSBD is available on the host.
 - Revert change to msr-sc= command line option documentation.
 - Only set or clear the SSBD bit of spec_ctrl.
---
 xen/arch/x86/cpuid.c                        |  7 +++++++
 xen/arch/x86/hvm/hvm.c                      |  1 +
 xen/arch/x86/include/asm/msr.h              |  4 ++++
 xen/arch/x86/msr.c                          | 18 ++++++++++++++++++
 xen/arch/x86/spec_ctrl.c                    |  3 ++-
 xen/include/public/arch-x86/cpufeatureset.h |  2 +-
 6 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 7e0b395698..979dcf8164 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -550,6 +550,13 @@ static void __init calculate_hvm_max_policy(void)
         __clear_bit(X86_FEATURE_IBRSB, hvm_featureset);
         __clear_bit(X86_FEATURE_IBRS, hvm_featureset);
     }
+    else if ( boot_cpu_has(X86_FEATURE_AMD_SSBD) )
+        /*
+         * If SPEC_CTRL.SSBD is available VIRT_SPEC_CTRL.SSBD can be exposed
+         * and implemented using the former. Expose in the max policy only as
+         * the preference is for guests to use SPEC_CTRL.SSBD if available.
+         */
+        __set_bit(X86_FEATURE_VIRT_SSBD, hvm_featureset);
 
     /*
      * With VT-x, some features are only supported by Xen if dedicated
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 5b16fb4cd8..db8f95ef7c 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1334,6 +1334,7 @@ static const uint32_t msrs_to_send[] = {
     MSR_INTEL_MISC_FEATURES_ENABLES,
     MSR_IA32_BNDCFGS,
     MSR_IA32_XSS,
+    MSR_VIRT_SPEC_CTRL,
     MSR_AMD64_DR0_ADDRESS_MASK,
     MSR_AMD64_DR1_ADDRESS_MASK,
     MSR_AMD64_DR2_ADDRESS_MASK,
diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h
index ce4fe51afe..ab6fbb5051 100644
--- a/xen/arch/x86/include/asm/msr.h
+++ b/xen/arch/x86/include/asm/msr.h
@@ -291,6 +291,7 @@ struct vcpu_msrs
 {
     /*
      * 0x00000048 - MSR_SPEC_CTRL
+     * 0xc001011f - MSR_VIRT_SPEC_CTRL (if X86_FEATURE_AMD_SSBD)
      *
      * For PV guests, this holds the guest kernel value.  It is accessed on
      * every entry/exit path.
@@ -306,6 +307,9 @@ struct vcpu_msrs
      * We must clear/restore Xen's value before/after VMRUN to avoid unduly
      * influencing the guest.  In order to support "behind the guest's back"
      * protections, we load this value (commonly 0) before VMRUN.
+     *
+     * Once of such "behind the guest's back" usages is setting SPEC_CTRL.SSBD
+     * if the guest sets VIRT_SPEC_CTRL.SSBD.
      */
     struct {
         uint32_t raw;
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 01a15857b7..72c175fd8b 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -381,6 +381,13 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
                ? K8_HWCR_TSC_FREQ_SEL : 0;
         break;
 
+    case MSR_VIRT_SPEC_CTRL:
+        if ( !cp->extd.virt_ssbd )
+            goto gp_fault;
+
+        *val = msrs->spec_ctrl.raw & SPEC_CTRL_SSBD;
+        break;
+
     case MSR_AMD64_DE_CFG:
         if ( !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
             goto gp_fault;
@@ -666,6 +673,17 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
             wrmsr_tsc_aux(val);
         break;
 
+    case MSR_VIRT_SPEC_CTRL:
+        if ( !cp->extd.virt_ssbd )
+            goto gp_fault;
+
+        /* Only supports SSBD bit, the rest are ignored. */
+        if ( val & SPEC_CTRL_SSBD )
+            msrs->spec_ctrl.raw |= SPEC_CTRL_SSBD;
+        else
+            msrs->spec_ctrl.raw &= ~SPEC_CTRL_SSBD;
+        break;
+
     case MSR_AMD64_DE_CFG:
         /*
          * OpenBSD 6.7 will panic if writing to DE_CFG triggers a #GP:
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 1408e4c7ab..f338bfe292 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -402,12 +402,13 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
      * mitigation support for guests.
      */
 #ifdef CONFIG_HVM
-    printk("  Support for HVM VMs:%s%s%s%s%s\n",
+    printk("  Support for HVM VMs:%s%s%s%s%s%s\n",
            (boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
             boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ||
             boot_cpu_has(X86_FEATURE_MD_CLEAR)   ||
             opt_eager_fpu)                           ? ""               : " None",
            boot_cpu_has(X86_FEATURE_SC_MSR_HVM)      ? " MSR_SPEC_CTRL" : "",
+           boot_cpu_has(X86_FEATURE_SC_MSR_HVM)      ? " MSR_VIRT_SPEC_CTRL" : "",
            boot_cpu_has(X86_FEATURE_SC_RSB_HVM)      ? " RSB"           : "",
            opt_eager_fpu                             ? " EAGER_FPU"     : "",
            boot_cpu_has(X86_FEATURE_MD_CLEAR)        ? " MD_CLEAR"      : "");
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 9cee4b439e..5aa3c82fc6 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -265,7 +265,7 @@ XEN_CPUFEATURE(IBRS_SAME_MODE, 8*32+19) /*S  IBRS provides same-mode protection
 XEN_CPUFEATURE(NO_LMSL,       8*32+20) /*S  EFER.LMSLE no longer supported. */
 XEN_CPUFEATURE(AMD_PPIN,      8*32+23) /*   Protected Processor Inventory Number */
 XEN_CPUFEATURE(AMD_SSBD,      8*32+24) /*S  MSR_SPEC_CTRL.SSBD available */
-XEN_CPUFEATURE(VIRT_SSBD,     8*32+25) /*   MSR_VIRT_SPEC_CTRL.SSBD */
+XEN_CPUFEATURE(VIRT_SSBD,     8*32+25) /*!  MSR_VIRT_SPEC_CTRL.SSBD */
 XEN_CPUFEATURE(SSB_NO,        8*32+26) /*A  Hardware not vulnerable to SSB */
 XEN_CPUFEATURE(PSFD,          8*32+28) /*S  MSR_SPEC_CTRL.PSFD */
 
-- 
2.36.0

[PATCH v6 2/3] amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests

[Roger Pau Monne]

Allow HVM guests access to MSR_VIRT_SPEC_CTRL if the platform Xen is
running on has support for it.  This requires adding logic in the
vm{entry,exit} paths for SVM in order to context switch between the
hypervisor value and the guest one.  The added handlers for context
switch will also be used for the legacy SSBD support.

Introduce a new synthetic feature leaf (X86_FEATURE_VIRT_SC_MSR_HVM)
to signal whether VIRT_SPEC_CTRL needs to be handled on guest
vm{entry,exit}.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v5:
 - Add comment regarding VIRT_SSBD exposure.
 - Store guest value in 'val' local variable in
   vmentry_virt_spec_ctrl.

Changes since v4:
 - Fix exposing in the max policy.

Changes since v3:
 - Always trap write accesses to VIRT_SPEC_CTRL in order to cache the
   guest setting.
 - Do not use the 'S' annotation for the VIRT_SSBD feature.

Changes since v2:
 - Reword part of the commit message regarding annotation change.
 - Fix MSR intercept.
 - Add handling of VIRT_SPEC_CTRL to guest_{rd,wr}msr when using
   VIRT_SSBD also.

Changes since v1:
 - Introduce virt_spec_ctrl vCPU field.
 - Context switch VIRT_SPEC_CTRL on vmentry/vmexit separately from
   SPEC_CTRL.
---
 xen/arch/x86/cpuid.c                   | 14 +++++++++++
 xen/arch/x86/hvm/svm/entry.S           |  8 ++++++
 xen/arch/x86/hvm/svm/svm.c             | 35 ++++++++++++++++++++++++++
 xen/arch/x86/include/asm/cpufeatures.h |  1 +
 xen/arch/x86/include/asm/msr.h         | 10 ++++++++
 xen/arch/x86/msr.c                     | 16 +++++++++---
 xen/arch/x86/spec_ctrl.c               |  9 ++++++-
 7 files changed, 88 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 979dcf8164..a4a366ad84 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -541,6 +541,13 @@ static void __init calculate_hvm_max_policy(void)
          raw_cpuid_policy.basic.sep )
         __set_bit(X86_FEATURE_SEP, hvm_featureset);
 
+    /*
+     * VIRT_SSBD is exposed in the default policy as a result of
+     * VIRT_SC_MSR_HVM being set, it also needs exposing in the max policy.
+     */
+    if ( boot_cpu_has(X86_FEATURE_VIRT_SC_MSR_HVM) )
+        __set_bit(X86_FEATURE_VIRT_SSBD, hvm_featureset);
+
     /*
      * If Xen isn't virtualising MSR_SPEC_CTRL for HVM guests (functional
      * availability, or admin choice), hide the feature.
@@ -597,6 +604,13 @@ static void __init calculate_hvm_def_policy(void)
     guest_common_feature_adjustments(hvm_featureset);
     guest_common_default_feature_adjustments(hvm_featureset);
 
+    /*
+     * Only expose VIRT_SSBD if AMD_SSBD is not available, and thus
+     * VIRT_SC_MSR_HVM is set.
+     */
+    if ( boot_cpu_has(X86_FEATURE_VIRT_SC_MSR_HVM) )
+        __set_bit(X86_FEATURE_VIRT_SSBD, hvm_featureset);
+
     sanitise_featureset(hvm_featureset);
     cpuid_featureset_to_policy(hvm_featureset, p);
     recalculate_xstate(p);
diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S
index 4ae55a2ef6..2f63a2e3c6 100644
--- a/xen/arch/x86/hvm/svm/entry.S
+++ b/xen/arch/x86/hvm/svm/entry.S
@@ -19,6 +19,8 @@
 
         .file "svm/entry.S"
 
+#include <xen/lib.h>
+
 #include <asm/asm_defns.h>
 #include <asm/page.h>
 
@@ -57,6 +59,9 @@ __UNLIKELY_END(nsvm_hap)
 
         clgi
 
+        ALTERNATIVE "", STR(call vmentry_virt_spec_ctrl), \
+                        X86_FEATURE_VIRT_SC_MSR_HVM
+
         /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
         /* SPEC_CTRL_EXIT_TO_SVM       Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
         .macro svm_vmentry_spec_ctrl
@@ -114,6 +119,9 @@ __UNLIKELY_END(nsvm_hap)
         ALTERNATIVE "", svm_vmexit_spec_ctrl, X86_FEATURE_SC_MSR_HVM
         /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
 
+        ALTERNATIVE "", STR(call vmexit_virt_spec_ctrl), \
+                        X86_FEATURE_VIRT_SC_MSR_HVM
+
         stgi
 GLOBAL(svm_stgi_label)
         mov  %rsp,%rdi
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 2455835eda..c4bdeaff52 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -52,6 +52,7 @@
 #include <asm/hvm/svm/svmdebug.h>
 #include <asm/hvm/svm/nestedsvm.h>
 #include <asm/hvm/nestedhvm.h>
+#include <asm/spec_ctrl.h>
 #include <asm/x86_emulate.h>
 #include <public/sched.h>
 #include <asm/hvm/vpt.h>
@@ -610,6 +611,16 @@ static void cf_check svm_cpuid_policy_changed(struct vcpu *v)
     svm_intercept_msr(v, MSR_SPEC_CTRL,
                       cp->extd.ibrs ? MSR_INTERCEPT_NONE : MSR_INTERCEPT_RW);
 
+    /*
+     * Always trap write accesses to VIRT_SPEC_CTRL in order to cache the guest
+     * setting and avoid having to perform a rdmsr on vmexit to get the guest
+     * setting even if VIRT_SSBD is offered to Xen itself.
+     */
+    svm_intercept_msr(v, MSR_VIRT_SPEC_CTRL,
+                      cp->extd.virt_ssbd && cpu_has_virt_ssbd &&
+                      !cpu_has_amd_ssbd ?
+                      MSR_INTERCEPT_WRITE : MSR_INTERCEPT_RW);
+
     /* Give access to MSR_PRED_CMD if the guest has been told about it. */
     svm_intercept_msr(v, MSR_PRED_CMD,
                       cp->extd.ibpb ? MSR_INTERCEPT_NONE : MSR_INTERCEPT_RW);
@@ -3105,6 +3116,30 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
     vmcb_set_vintr(vmcb, intr);
 }
 
+/* Called with GIF=0. */
+void vmexit_virt_spec_ctrl(void)
+{
+    unsigned int val = opt_ssbd ? SPEC_CTRL_SSBD : 0;
+
+    if ( val == current->arch.msrs->virt_spec_ctrl.raw )
+        return;
+
+    if ( cpu_has_virt_ssbd )
+        wrmsr(MSR_VIRT_SPEC_CTRL, val, 0);
+}
+
+/* Called with GIF=0. */
+void vmentry_virt_spec_ctrl(void)
+{
+    unsigned int val = current->arch.msrs->virt_spec_ctrl.raw;
+
+    if ( val == (opt_ssbd ? SPEC_CTRL_SSBD : 0) )
+        return;
+
+    if ( cpu_has_virt_ssbd )
+        wrmsr(MSR_VIRT_SPEC_CTRL, val, 0);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/include/asm/cpufeatures.h b/xen/arch/x86/include/asm/cpufeatures.h
index 7413febd7a..2240547b64 100644
--- a/xen/arch/x86/include/asm/cpufeatures.h
+++ b/xen/arch/x86/include/asm/cpufeatures.h
@@ -40,6 +40,7 @@ XEN_CPUFEATURE(SC_VERW_HVM,       X86_SYNTH(24)) /* VERW used by Xen for HVM */
 XEN_CPUFEATURE(SC_VERW_IDLE,      X86_SYNTH(25)) /* VERW used by Xen for idle */
 XEN_CPUFEATURE(XEN_SHSTK,         X86_SYNTH(26)) /* Xen uses CET Shadow Stacks */
 XEN_CPUFEATURE(XEN_IBT,           X86_SYNTH(27)) /* Xen uses CET Indirect Branch Tracking */
+XEN_CPUFEATURE(VIRT_SC_MSR_HVM,   X86_SYNTH(28)) /* MSR_VIRT_SPEC_CTRL exposed to HVM */
 
 /* Bug words follow the synthetic words. */
 #define X86_NR_BUG 1
diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h
index ab6fbb5051..de18e90b2e 100644
--- a/xen/arch/x86/include/asm/msr.h
+++ b/xen/arch/x86/include/asm/msr.h
@@ -375,6 +375,16 @@ struct vcpu_msrs
      */
     uint32_t tsc_aux;
 
+    /*
+     * 0xc001011f - MSR_VIRT_SPEC_CTRL (if !X86_FEATURE_AMD_SSBD)
+     *
+     * AMD only. Guest selected value, context switched on guest VM
+     * entry/exit.
+     */
+    struct {
+        uint32_t raw;
+    } virt_spec_ctrl;
+
     /*
      * 0xc00110{27,19-1b} MSR_AMD64_DR{0-3}_ADDRESS_MASK
      *
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 72c175fd8b..a1e268eea9 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -385,7 +385,10 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
         if ( !cp->extd.virt_ssbd )
             goto gp_fault;
 
-        *val = msrs->spec_ctrl.raw & SPEC_CTRL_SSBD;
+        if ( cpu_has_amd_ssbd )
+            *val = msrs->spec_ctrl.raw & SPEC_CTRL_SSBD;
+        else
+            *val = msrs->virt_spec_ctrl.raw;
         break;
 
     case MSR_AMD64_DE_CFG:
@@ -678,10 +681,15 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
             goto gp_fault;
 
         /* Only supports SSBD bit, the rest are ignored. */
-        if ( val & SPEC_CTRL_SSBD )
-            msrs->spec_ctrl.raw |= SPEC_CTRL_SSBD;
+        if ( cpu_has_amd_ssbd )
+        {
+            if ( val & SPEC_CTRL_SSBD )
+                msrs->spec_ctrl.raw |= SPEC_CTRL_SSBD;
+            else
+                msrs->spec_ctrl.raw &= ~SPEC_CTRL_SSBD;
+        }
         else
-            msrs->spec_ctrl.raw &= ~SPEC_CTRL_SSBD;
+            msrs->virt_spec_ctrl.raw = val & SPEC_CTRL_SSBD;
         break;
 
     case MSR_AMD64_DE_CFG:
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index f338bfe292..0d5ec877d1 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -406,9 +406,12 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
            (boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
             boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ||
             boot_cpu_has(X86_FEATURE_MD_CLEAR)   ||
+            boot_cpu_has(X86_FEATURE_VIRT_SC_MSR_HVM) ||
             opt_eager_fpu)                           ? ""               : " None",
            boot_cpu_has(X86_FEATURE_SC_MSR_HVM)      ? " MSR_SPEC_CTRL" : "",
-           boot_cpu_has(X86_FEATURE_SC_MSR_HVM)      ? " MSR_VIRT_SPEC_CTRL" : "",
+           (boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
+            boot_cpu_has(X86_FEATURE_VIRT_SC_MSR_HVM)) ? " MSR_VIRT_SPEC_CTRL"
+                                                       : "",
            boot_cpu_has(X86_FEATURE_SC_RSB_HVM)      ? " RSB"           : "",
            opt_eager_fpu                             ? " EAGER_FPU"     : "",
            boot_cpu_has(X86_FEATURE_MD_CLEAR)        ? " MD_CLEAR"      : "");
@@ -1069,6 +1072,10 @@ void __init init_speculation_mitigations(void)
             setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
     }
 
+    /* Support VIRT_SPEC_CTRL.SSBD if AMD_SSBD is not available. */
+    if ( opt_msr_sc_hvm && !cpu_has_amd_ssbd && cpu_has_virt_ssbd )
+        setup_force_cpu_cap(X86_FEATURE_VIRT_SC_MSR_HVM);
+
     /* If we have IBRS available, see whether we should use it. */
     if ( has_spec_ctrl && ibrs )
         default_xen_spec_ctrl |= SPEC_CTRL_IBRS;
-- 
2.36.0

[PATCH v6 3/3] amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD

[Roger Pau Monne]

Expose VIRT_SSBD to guests if the hardware supports setting SSBD in
the LS_CFG MSR (a.k.a. non-architectural way). Different AMD CPU
families use different bits in LS_CFG, so exposing VIRT_SPEC_CTRL.SSBD
allows for an unified way of exposing SSBD support to guests on AMD
hardware that's compatible migration wise, regardless of what
underlying mechanism is used to set SSBD.

Note that on AMD Family 17h and Hygon Family 18h processors the value
of SSBD in LS_CFG is shared between threads on the same core, so
there's extra logic in order to synchronize the value and have SSBD
set as long as one of the threads in the core requires it to be set.
Such logic also requires extra storage for each thread state, which is
allocated at initialization time.

Do the context switching of the SSBD selection in LS_CFG between
hypervisor and guest in the same handler that's already used to switch
the value of VIRT_SPEC_CTRL.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Changes since v5:
 - Fix one codding style issue.

Changes since v4:
 - Slightly change usage of val/opt_ssbd in
   vm{exit,entry}_virt_spec_ctrl.
 - Pull opt_ssbd outside of the for loop in amd_setup_legacy_ssbd().
 - Fix indentation.
 - Remove ASSERTs/BUG_ONs from GIF=0 context.

Changes since v3:
 - Align ssbd per-core struct to a cache line.
 - Open code a simple spinlock to avoid playing tricks with the lock
   detector.
 - s/ssbd_core/ssbd_ls_cfg/.
 - Fix log message wording.
 - Fix define name and remove comment.
 - Also handle Hygon processors (Fam18h).
 - Add changelog entry.

Changes since v2:
 - Fix codding style issues.
 - Use AMD_ZEN1_MAX_SOCKETS to define the max number of possible
   sockets in Zen1 systems.

Changes since v1:
 - Report legacy SSBD support using a global variable.
 - Use ro_after_init for ssbd_max_cores.
 - Handle boot_cpu_data.x86_num_siblings < 1.
 - Add comment regarding _irqsave usage in amd_set_legacy_ssbd.
---
 CHANGELOG.md                   |   3 +
 xen/arch/x86/cpu/amd.c         | 121 ++++++++++++++++++++++++++++-----
 xen/arch/x86/hvm/svm/svm.c     |   4 ++
 xen/arch/x86/include/asm/amd.h |   4 ++
 xen/arch/x86/spec_ctrl.c       |   4 +-
 5 files changed, 118 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6a7755d7b0..9a007e2513 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 ### Removed / support downgraded
  - dropped support for the (x86-only) "vesa-mtrr" and "vesa-remap" command line options
 
+### Added
+ - Support VIRT_SSBD feature for HVM guests on AMD.
+
 ## [4.16.0](https://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=staging) - 2021-12-02
 
 ### Removed
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 4999f8be2b..5f9e734e84 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -48,6 +48,7 @@ boolean_param("allow_unsafe", opt_allow_unsafe);
 
 /* Signal whether the ACPI C1E quirk is required. */
 bool __read_mostly amd_acpi_c1e_quirk;
+bool __ro_after_init amd_legacy_ssbd;
 
 static inline int rdmsr_amd_safe(unsigned int msr, unsigned int *lo,
 				 unsigned int *hi)
@@ -685,23 +686,10 @@ void amd_init_lfence(struct cpuinfo_x86 *c)
  * Refer to the AMD Speculative Store Bypass whitepaper:
  * https://developer.amd.com/wp-content/resources/124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
  */
-void amd_init_ssbd(const struct cpuinfo_x86 *c)
+static bool set_legacy_ssbd(const struct cpuinfo_x86 *c, bool enable)
 {
 	int bit = -1;
 
-	if (cpu_has_ssb_no)
-		return;
-
-	if (cpu_has_amd_ssbd) {
-		/* Handled by common MSR_SPEC_CTRL logic */
-		return;
-	}
-
-	if (cpu_has_virt_ssbd) {
-		wrmsrl(MSR_VIRT_SPEC_CTRL, opt_ssbd ? SPEC_CTRL_SSBD : 0);
-		return;
-	}
-
 	switch (c->x86) {
 	case 0x15: bit = 54; break;
 	case 0x16: bit = 33; break;
@@ -715,20 +703,119 @@ void amd_init_ssbd(const struct cpuinfo_x86 *c)
 		if (rdmsr_safe(MSR_AMD64_LS_CFG, val) ||
 		    ({
 			    val &= ~mask;
-			    if (opt_ssbd)
+			    if (enable)
 				    val |= mask;
 			    false;
 		    }) ||
 		    wrmsr_safe(MSR_AMD64_LS_CFG, val) ||
 		    ({
 			    rdmsrl(MSR_AMD64_LS_CFG, val);
-			    (val & mask) != (opt_ssbd * mask);
+			    (val & mask) != (enable * mask);
 		    }))
 			bit = -1;
 	}
 
-	if (bit < 0)
+	return bit >= 0;
+}
+
+void amd_init_ssbd(const struct cpuinfo_x86 *c)
+{
+	if (cpu_has_ssb_no)
+		return;
+
+	if (cpu_has_amd_ssbd) {
+		/* Handled by common MSR_SPEC_CTRL logic */
+		return;
+	}
+
+	if (cpu_has_virt_ssbd) {
+		wrmsrl(MSR_VIRT_SPEC_CTRL, opt_ssbd ? SPEC_CTRL_SSBD : 0);
+		return;
+	}
+
+	if (!set_legacy_ssbd(c, opt_ssbd)) {
 		printk_once(XENLOG_ERR "No SSBD controls available\n");
+		if (amd_legacy_ssbd)
+			panic("CPU feature mismatch: no legacy SSBD\n");
+	} else if (c == &boot_cpu_data)
+		amd_legacy_ssbd = true;
+}
+
+static struct ssbd_ls_cfg {
+    bool locked;
+    unsigned int count;
+} __cacheline_aligned *ssbd_ls_cfg;
+static unsigned int __ro_after_init ssbd_max_cores;
+#define AMD_FAM17H_MAX_SOCKETS 2
+
+bool __init amd_setup_legacy_ssbd(void)
+{
+	unsigned int i;
+
+	if ((boot_cpu_data.x86 != 0x17 && boot_cpu_data.x86 != 0x18) ||
+	    boot_cpu_data.x86_num_siblings <= 1)
+		return true;
+
+	/*
+	 * One could be forgiven for thinking that c->x86_max_cores is the
+	 * correct value to use here.
+	 *
+	 * However, that value is derived from the current configuration, and
+	 * c->cpu_core_id is sparse on all but the top end CPUs.  Derive
+	 * max_cpus from ApicIdCoreIdSize which will cover any sparseness.
+	 */
+	if (boot_cpu_data.extended_cpuid_level >= 0x80000008) {
+		ssbd_max_cores = 1u << MASK_EXTR(cpuid_ecx(0x80000008), 0xf000);
+		ssbd_max_cores /= boot_cpu_data.x86_num_siblings;
+	}
+	if (!ssbd_max_cores)
+		return false;
+
+	ssbd_ls_cfg = xzalloc_array(struct ssbd_ls_cfg,
+	                            ssbd_max_cores * AMD_FAM17H_MAX_SOCKETS);
+	if (!ssbd_ls_cfg)
+		return false;
+
+	if (opt_ssbd)
+		for (i = 0; i < ssbd_max_cores * AMD_FAM17H_MAX_SOCKETS; i++)
+			/* Set initial state, applies to any (hotplug) CPU. */
+			ssbd_ls_cfg[i].count = boot_cpu_data.x86_num_siblings;
+
+	return true;
+}
+
+/*
+ * Executed from GIF==0 context: avoid using BUG/ASSERT or other functionality
+ * that relies on exceptions as those are not expected to run in GIF==0
+ * context.
+ */
+void amd_set_legacy_ssbd(bool enable)
+{
+	const struct cpuinfo_x86 *c = &current_cpu_data;
+	struct ssbd_ls_cfg *status;
+
+	if ((c->x86 != 0x17 && c->x86 != 0x18) || c->x86_num_siblings <= 1) {
+		set_legacy_ssbd(c, enable);
+		return;
+	}
+
+	status = &ssbd_ls_cfg[c->phys_proc_id * ssbd_max_cores +
+	                      c->cpu_core_id];
+
+	/*
+	 * Open code a very simple spinlock: this function is used with GIF==0
+	 * and different IF values, so would trigger the checklock detector.
+	 * Instead of trying to workaround the detector, use a very simple lock
+	 * implementation: it's better to reduce the amount of code executed
+	 * with GIF==0.
+	 */
+	while (test_and_set_bool(status->locked))
+		cpu_relax();
+	status->count += enable ? 1 : -1;
+	if (enable ? status->count == 1 : !status->count)
+		set_legacy_ssbd(c, enable);
+	barrier();
+	write_atomic(&status->locked, false);
 }
 
 void __init detect_zen2_null_seg_behaviour(void)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index c4bdeaff52..3cc5fcdc44 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -3126,6 +3126,8 @@ void vmexit_virt_spec_ctrl(void)
 
     if ( cpu_has_virt_ssbd )
         wrmsr(MSR_VIRT_SPEC_CTRL, val, 0);
+    else
+        amd_set_legacy_ssbd(val);
 }
 
 /* Called with GIF=0. */
@@ -3138,6 +3140,8 @@ void vmentry_virt_spec_ctrl(void)
 
     if ( cpu_has_virt_ssbd )
         wrmsr(MSR_VIRT_SPEC_CTRL, val, 0);
+    else
+        amd_set_legacy_ssbd(val);
 }
 
 /*
diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h
index a82382e6bf..6a42f68542 100644
--- a/xen/arch/x86/include/asm/amd.h
+++ b/xen/arch/x86/include/asm/amd.h
@@ -151,4 +151,8 @@ void check_enable_amd_mmconf_dmi(void);
 extern bool amd_acpi_c1e_quirk;
 void amd_check_disable_c1e(unsigned int port, u8 value);
 
+extern bool amd_legacy_ssbd;
+bool amd_setup_legacy_ssbd(void);
+void amd_set_legacy_ssbd(bool enable);
+
 #endif /* __AMD_H__ */
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 0d5ec877d1..495e6f9405 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -22,6 +22,7 @@
 #include <xen/param.h>
 #include <xen/warning.h>
 
+#include <asm/amd.h>
 #include <asm/hvm/svm/svm.h>
 #include <asm/microcode.h>
 #include <asm/msr.h>
@@ -1073,7 +1074,8 @@ void __init init_speculation_mitigations(void)
     }
 
     /* Support VIRT_SPEC_CTRL.SSBD if AMD_SSBD is not available. */
-    if ( opt_msr_sc_hvm && !cpu_has_amd_ssbd && cpu_has_virt_ssbd )
+    if ( opt_msr_sc_hvm && !cpu_has_amd_ssbd &&
+         (cpu_has_virt_ssbd || (amd_legacy_ssbd && amd_setup_legacy_ssbd())) )
         setup_force_cpu_cap(X86_FEATURE_VIRT_SC_MSR_HVM);
 
     /* If we have IBRS available, see whether we should use it. */
-- 
2.36.0

Re: [PATCH v6 2/3] amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests

[Jan Beulich]

On 17.05.2022 17:31, Roger Pau Monne wrote:
> Allow HVM guests access to MSR_VIRT_SPEC_CTRL if the platform Xen is
> running on has support for it.  This requires adding logic in the
> vm{entry,exit} paths for SVM in order to context switch between the
> hypervisor value and the guest one.  The added handlers for context
> switch will also be used for the legacy SSBD support.
> 
> Introduce a new synthetic feature leaf (X86_FEATURE_VIRT_SC_MSR_HVM)
> to signal whether VIRT_SPEC_CTRL needs to be handled on guest
> vm{entry,exit}.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

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

Re: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Jan Beulich]

On 17.05.2022 17:31, Roger Pau Monne wrote:
> Hello,
> 
> The following series implements support for MSR_VIRT_SPEC_CTRL
> (VIRT_SSBD) on different AMD CPU families.
> 
> Note that the support is added backwards, starting with the newer CPUs
> that support MSR_SPEC_CTRL and moving to the older ones either using
> MSR_VIRT_SPEC_CTRL or the SSBD bit in LS_CFG.
> 
> Xen is still free to use it's own SSBD setting, as the selection is
> context switched on vm{entry,exit}.
> 
> On Zen2 and later, SPEC_CTRL.SSBD should exist and should be used in
> preference to VIRT_SPEC_CTRL.SSBD.  However, for migration
> compatibility, Xen offers VIRT_SSBD to guests (in the max cpuid policy,
> not default) implemented in terms of SPEC_CTRL.SSBD.
> 
> On Fam15h thru Zen1, Xen exposes VIRT_SSBD to guests by default to
> abstract away the model and/or hypervisor specific differences in
> MSR_LS_CFG/MSR_VIRT_SPEC_CTRL.
> 
> So the implementation of VIRT_SSBD exposed to HVM guests will use one of
> the following underlying mechanisms, in the preference order listed
> below:
> 
>  * SPEC_CTRL.SSBD: patch 1
>  * VIRT_SPEC_CTRL.SSBD: patch 2.
>  * Non-architectural way using LS_CFG: patch 3.
> 
> NB: patch 3 introduces some logic in GIF=0 context, such logic has been
> kept to a minimum due to the special context it's running on.
> 
> Roger Pau Monne (3):
>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL
>   amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD

FTAOD, while besides a missing ack for ...

>  CHANGELOG.md                                |   3 +

... this addition the series would now look to be ready to go in,
I'd like to have some form of confirmation by you, Andrew, that
you now view this as meeting the comments you gave on an earlier
version.

Jan

RE: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Henry Wang]

Hi Jan, Roger and Andrew,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> 
> On 17.05.2022 17:31, Roger Pau Monne wrote:
> > Hello,
> >
> > The following series implements support for MSR_VIRT_SPEC_CTRL
> > (VIRT_SSBD) on different AMD CPU families.
> >
> > Note that the support is added backwards, starting with the newer CPUs
> > that support MSR_SPEC_CTRL and moving to the older ones either using
> > MSR_VIRT_SPEC_CTRL or the SSBD bit in LS_CFG.
> >
> > Xen is still free to use it's own SSBD setting, as the selection is
> > context switched on vm{entry,exit}.
> >
> > On Zen2 and later, SPEC_CTRL.SSBD should exist and should be used in
> > preference to VIRT_SPEC_CTRL.SSBD.  However, for migration
> > compatibility, Xen offers VIRT_SSBD to guests (in the max cpuid policy,
> > not default) implemented in terms of SPEC_CTRL.SSBD.
> >
> > On Fam15h thru Zen1, Xen exposes VIRT_SSBD to guests by default to
> > abstract away the model and/or hypervisor specific differences in
> > MSR_LS_CFG/MSR_VIRT_SPEC_CTRL.
> >
> > So the implementation of VIRT_SSBD exposed to HVM guests will use one
> of
> > the following underlying mechanisms, in the preference order listed
> > below:
> >
> >  * SPEC_CTRL.SSBD: patch 1
> >  * VIRT_SPEC_CTRL.SSBD: patch 2.
> >  * Non-architectural way using LS_CFG: patch 3.
> >
> > NB: patch 3 introduces some logic in GIF=0 context, such logic has been
> > kept to a minimum due to the special context it's running on.
> >
> > Roger Pau Monne (3):
> >   amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of
> SPEC_CTRL
> >   amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
> >   amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD
> 
> FTAOD, while besides a missing ack for ...
> 
> >  CHANGELOG.md                                |   3 +
> 
> ... this addition the series would now look to be ready to go in,
> I'd like to have some form of confirmation by you, Andrew, that
> you now view this as meeting the comments you gave on an earlier
> version.

Not completely sure if I am proper to do that but for the CHANGELOG.md
change:

Acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry

> 
> Jan

Re: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Jan Beulich]

On 18.05.2022 11:51, Henry Wang wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>>
>> On 17.05.2022 17:31, Roger Pau Monne wrote:
>>> Roger Pau Monne (3):
>>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of
>> SPEC_CTRL
>>>   amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
>>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD
>>
>> FTAOD, while besides a missing ack for ...
>>
>>>  CHANGELOG.md                                |   3 +
>>
>> ... this addition the series would now look to be ready to go in,
>> I'd like to have some form of confirmation by you, Andrew, that
>> you now view this as meeting the comments you gave on an earlier
>> version.
> 
> Not completely sure if I am proper to do that but for the CHANGELOG.md
> change:

Well, no-one except you actually can ack changes to this file, as per
./MAINTAINERS.

> Acked-by: Henry Wang <Henry.Wang@arm.com>

Thanks.

Jan

RE: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Henry Wang]

Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> 
> On 18.05.2022 11:51, Henry Wang wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >>
> >> On 17.05.2022 17:31, Roger Pau Monne wrote:
> >>> Roger Pau Monne (3):
> >>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of
> >> SPEC_CTRL
> >>>   amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
> >>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy
> SSBD
> >>
> >> FTAOD, while besides a missing ack for ...
> >>
> >>>  CHANGELOG.md                                |   3 +
> >>
> >> ... this addition the series would now look to be ready to go in,
> >> I'd like to have some form of confirmation by you, Andrew, that
> >> you now view this as meeting the comments you gave on an earlier
> >> version.
> >
> > Not completely sure if I am proper to do that but for the CHANGELOG.md
> > change:
> 
> Well, no-one except you actually can ack changes to this file, as per
> ./MAINTAINERS.

Thanks for confirming and sending the reminder to help me to understand
that I should ack the changes in CHANGELOG.md for this series, I will keep
the information in mind and I guess I am gradually acquiring experiences :)

Kind regards,
Henry

> 
> > Acked-by: Henry Wang <Henry.Wang@arm.com>
> 
> Thanks.
> 
> Jan

RE: [PATCH v6 3/3] amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD

[Henry Wang]

Hi,

It seems that this series [1] has been stale for more than a month and also
this series seems to be properly reviewed and acked already. 

From what Jan has replied to Roger and Andrew:
"... this addition the series would now look to be ready to go in,
I'd like to have some form of confirmation by you, Andrew, that
you now view this as meeting the comments you gave on an earlier
version."

So I guess this can be merged. Sending this as a gentle reminder for
possible actions from Roger and Andrew. Thanks!

Also, not sure why my acked-by for the CHANGELOG.md is missing in
patchwork, just in case - for the change in CHANGELOG.md in patch#3:

Acked-by: Henry Wang <Henry.Wang@arm.com>

[1] https://patchwork.kernel.org/project/xen-devel/list/?series=642413

Kind regards,
Henry

> -----Original Message-----
> From: Roger Pau Monne <roger.pau@citrix.com>
> Subject: [PATCH v6 3/3] amd/msr: implement VIRT_SPEC_CTRL for HVM
> guests using legacy SSBD
> 
> Expose VIRT_SSBD to guests if the hardware supports setting SSBD in
> the LS_CFG MSR (a.k.a. non-architectural way). Different AMD CPU
> families use different bits in LS_CFG, so exposing VIRT_SPEC_CTRL.SSBD
> allows for an unified way of exposing SSBD support to guests on AMD
> hardware that's compatible migration wise, regardless of what
> underlying mechanism is used to set SSBD.
> 
> Note that on AMD Family 17h and Hygon Family 18h processors the value
> of SSBD in LS_CFG is shared between threads on the same core, so
> there's extra logic in order to synchronize the value and have SSBD
> set as long as one of the threads in the core requires it to be set.
> Such logic also requires extra storage for each thread state, which is
> allocated at initialization time.
> 
> Do the context switching of the SSBD selection in LS_CFG between
> hypervisor and guest in the same handler that's already used to switch
> the value of VIRT_SPEC_CTRL.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> ---
> Changes since v5:
>  - Fix one codding style issue.
> 
> Changes since v4:
>  - Slightly change usage of val/opt_ssbd in
>    vm{exit,entry}_virt_spec_ctrl.
>  - Pull opt_ssbd outside of the for loop in amd_setup_legacy_ssbd().
>  - Fix indentation.
>  - Remove ASSERTs/BUG_ONs from GIF=0 context.
> 
> Changes since v3:
>  - Align ssbd per-core struct to a cache line.
>  - Open code a simple spinlock to avoid playing tricks with the lock
>    detector.
>  - s/ssbd_core/ssbd_ls_cfg/.
>  - Fix log message wording.
>  - Fix define name and remove comment.
>  - Also handle Hygon processors (Fam18h).
>  - Add changelog entry.
> 
> Changes since v2:
>  - Fix codding style issues.
>  - Use AMD_ZEN1_MAX_SOCKETS to define the max number of possible
>    sockets in Zen1 systems.
> 
> Changes since v1:
>  - Report legacy SSBD support using a global variable.
>  - Use ro_after_init for ssbd_max_cores.
>  - Handle boot_cpu_data.x86_num_siblings < 1.
>  - Add comment regarding _irqsave usage in amd_set_legacy_ssbd.
> ---
>  CHANGELOG.md                   |   3 +
>  xen/arch/x86/cpu/amd.c         | 121 ++++++++++++++++++++++++++++-----
>  xen/arch/x86/hvm/svm/svm.c     |   4 ++
>  xen/arch/x86/include/asm/amd.h |   4 ++
>  xen/arch/x86/spec_ctrl.c       |   4 +-
>  5 files changed, 118 insertions(+), 18 deletions(-)
> 
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index 6a7755d7b0..9a007e2513 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -13,6 +13,9 @@ The format is based on [Keep a
> Changelog](https://keepachangelog.com/en/1.0.0/)
>  ### Removed / support downgraded
>   - dropped support for the (x86-only) "vesa-mtrr" and "vesa-remap"
> command line options
> 
> +### Added
> + - Support VIRT_SSBD feature for HVM guests on AMD.
> +
>  ## [4.16.0](https://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=staging)
> - 2021-12-02
> 
>  ### Removed
> diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
> index 4999f8be2b..5f9e734e84 100644
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -48,6 +48,7 @@ boolean_param("allow_unsafe", opt_allow_unsafe);
> 
>  /* Signal whether the ACPI C1E quirk is required. */
>  bool __read_mostly amd_acpi_c1e_quirk;
> +bool __ro_after_init amd_legacy_ssbd;
> 
>  static inline int rdmsr_amd_safe(unsigned int msr, unsigned int *lo,
>  				 unsigned int *hi)
> @@ -685,23 +686,10 @@ void amd_init_lfence(struct cpuinfo_x86 *c)
>   * Refer to the AMD Speculative Store Bypass whitepaper:
>   * https://developer.amd.com/wp-
> content/resources/124441_AMD64_SpeculativeStoreBypassDisable_White
> paper_final.pdf
>   */
> -void amd_init_ssbd(const struct cpuinfo_x86 *c)
> +static bool set_legacy_ssbd(const struct cpuinfo_x86 *c, bool enable)
>  {
>  	int bit = -1;
> 
> -	if (cpu_has_ssb_no)
> -		return;
> -
> -	if (cpu_has_amd_ssbd) {
> -		/* Handled by common MSR_SPEC_CTRL logic */
> -		return;
> -	}
> -
> -	if (cpu_has_virt_ssbd) {
> -		wrmsrl(MSR_VIRT_SPEC_CTRL, opt_ssbd ?
> SPEC_CTRL_SSBD : 0);
> -		return;
> -	}
> -
>  	switch (c->x86) {
>  	case 0x15: bit = 54; break;
>  	case 0x16: bit = 33; break;
> @@ -715,20 +703,119 @@ void amd_init_ssbd(const struct cpuinfo_x86 *c)
>  		if (rdmsr_safe(MSR_AMD64_LS_CFG, val) ||
>  		    ({
>  			    val &= ~mask;
> -			    if (opt_ssbd)
> +			    if (enable)
>  				    val |= mask;
>  			    false;
>  		    }) ||
>  		    wrmsr_safe(MSR_AMD64_LS_CFG, val) ||
>  		    ({
>  			    rdmsrl(MSR_AMD64_LS_CFG, val);
> -			    (val & mask) != (opt_ssbd * mask);
> +			    (val & mask) != (enable * mask);
>  		    }))
>  			bit = -1;
>  	}
> 
> -	if (bit < 0)
> +	return bit >= 0;
> +}
> +
> +void amd_init_ssbd(const struct cpuinfo_x86 *c)
> +{
> +	if (cpu_has_ssb_no)
> +		return;
> +
> +	if (cpu_has_amd_ssbd) {
> +		/* Handled by common MSR_SPEC_CTRL logic */
> +		return;
> +	}
> +
> +	if (cpu_has_virt_ssbd) {
> +		wrmsrl(MSR_VIRT_SPEC_CTRL, opt_ssbd ?
> SPEC_CTRL_SSBD : 0);
> +		return;
> +	}
> +
> +	if (!set_legacy_ssbd(c, opt_ssbd)) {
>  		printk_once(XENLOG_ERR "No SSBD controls available\n");
> +		if (amd_legacy_ssbd)
> +			panic("CPU feature mismatch: no legacy SSBD\n");
> +	} else if (c == &boot_cpu_data)
> +		amd_legacy_ssbd = true;
> +}
> +
> +static struct ssbd_ls_cfg {
> +    bool locked;
> +    unsigned int count;
> +} __cacheline_aligned *ssbd_ls_cfg;
> +static unsigned int __ro_after_init ssbd_max_cores;
> +#define AMD_FAM17H_MAX_SOCKETS 2
> +
> +bool __init amd_setup_legacy_ssbd(void)
> +{
> +	unsigned int i;
> +
> +	if ((boot_cpu_data.x86 != 0x17 && boot_cpu_data.x86 != 0x18) ||
> +	    boot_cpu_data.x86_num_siblings <= 1)
> +		return true;
> +
> +	/*
> +	 * One could be forgiven for thinking that c->x86_max_cores is the
> +	 * correct value to use here.
> +	 *
> +	 * However, that value is derived from the current configuration,
> and
> +	 * c->cpu_core_id is sparse on all but the top end CPUs.  Derive
> +	 * max_cpus from ApicIdCoreIdSize which will cover any sparseness.
> +	 */
> +	if (boot_cpu_data.extended_cpuid_level >= 0x80000008) {
> +		ssbd_max_cores = 1u <<
> MASK_EXTR(cpuid_ecx(0x80000008), 0xf000);
> +		ssbd_max_cores /= boot_cpu_data.x86_num_siblings;
> +	}
> +	if (!ssbd_max_cores)
> +		return false;
> +
> +	ssbd_ls_cfg = xzalloc_array(struct ssbd_ls_cfg,
> +	                            ssbd_max_cores * AMD_FAM17H_MAX_SOCKETS);
> +	if (!ssbd_ls_cfg)
> +		return false;
> +
> +	if (opt_ssbd)
> +		for (i = 0; i < ssbd_max_cores *
> AMD_FAM17H_MAX_SOCKETS; i++)
> +			/* Set initial state, applies to any (hotplug) CPU. */
> +			ssbd_ls_cfg[i].count =
> boot_cpu_data.x86_num_siblings;
> +
> +	return true;
> +}
> +
> +/*
> + * Executed from GIF==0 context: avoid using BUG/ASSERT or other
> functionality
> + * that relies on exceptions as those are not expected to run in GIF==0
> + * context.
> + */
> +void amd_set_legacy_ssbd(bool enable)
> +{
> +	const struct cpuinfo_x86 *c = &current_cpu_data;
> +	struct ssbd_ls_cfg *status;
> +
> +	if ((c->x86 != 0x17 && c->x86 != 0x18) || c->x86_num_siblings <= 1)
> {
> +		set_legacy_ssbd(c, enable);
> +		return;
> +	}
> +
> +	status = &ssbd_ls_cfg[c->phys_proc_id * ssbd_max_cores +
> +	                      c->cpu_core_id];
> +
> +	/*
> +	 * Open code a very simple spinlock: this function is used with
> GIF==0
> +	 * and different IF values, so would trigger the checklock detector.
> +	 * Instead of trying to workaround the detector, use a very simple
> lock
> +	 * implementation: it's better to reduce the amount of code
> executed
> +	 * with GIF==0.
> +	 */
> +	while (test_and_set_bool(status->locked))
> +		cpu_relax();
> +	status->count += enable ? 1 : -1;
> +	if (enable ? status->count == 1 : !status->count)
> +		set_legacy_ssbd(c, enable);
> +	barrier();
> +	write_atomic(&status->locked, false);
>  }
> 
>  void __init detect_zen2_null_seg_behaviour(void)
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index c4bdeaff52..3cc5fcdc44 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -3126,6 +3126,8 @@ void vmexit_virt_spec_ctrl(void)
> 
>      if ( cpu_has_virt_ssbd )
>          wrmsr(MSR_VIRT_SPEC_CTRL, val, 0);
> +    else
> +        amd_set_legacy_ssbd(val);
>  }
> 
>  /* Called with GIF=0. */
> @@ -3138,6 +3140,8 @@ void vmentry_virt_spec_ctrl(void)
> 
>      if ( cpu_has_virt_ssbd )
>          wrmsr(MSR_VIRT_SPEC_CTRL, val, 0);
> +    else
> +        amd_set_legacy_ssbd(val);
>  }
> 
>  /*
> diff --git a/xen/arch/x86/include/asm/amd.h
> b/xen/arch/x86/include/asm/amd.h
> index a82382e6bf..6a42f68542 100644
> --- a/xen/arch/x86/include/asm/amd.h
> +++ b/xen/arch/x86/include/asm/amd.h
> @@ -151,4 +151,8 @@ void check_enable_amd_mmconf_dmi(void);
>  extern bool amd_acpi_c1e_quirk;
>  void amd_check_disable_c1e(unsigned int port, u8 value);
> 
> +extern bool amd_legacy_ssbd;
> +bool amd_setup_legacy_ssbd(void);
> +void amd_set_legacy_ssbd(bool enable);
> +
>  #endif /* __AMD_H__ */
> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
> index 0d5ec877d1..495e6f9405 100644
> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -22,6 +22,7 @@
>  #include <xen/param.h>
>  #include <xen/warning.h>
> 
> +#include <asm/amd.h>
>  #include <asm/hvm/svm/svm.h>
>  #include <asm/microcode.h>
>  #include <asm/msr.h>
> @@ -1073,7 +1074,8 @@ void __init init_speculation_mitigations(void)
>      }
> 
>      /* Support VIRT_SPEC_CTRL.SSBD if AMD_SSBD is not available. */
> -    if ( opt_msr_sc_hvm && !cpu_has_amd_ssbd && cpu_has_virt_ssbd )
> +    if ( opt_msr_sc_hvm && !cpu_has_amd_ssbd &&
> +         (cpu_has_virt_ssbd || (amd_legacy_ssbd &&
> amd_setup_legacy_ssbd())) )
>          setup_force_cpu_cap(X86_FEATURE_VIRT_SC_MSR_HVM);
> 
>      /* If we have IBRS available, see whether we should use it. */
> --
> 2.36.0

Re: [PATCH v6 3/3] amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD

[Jan Beulich]

On 17.06.2022 05:26, Henry Wang wrote:
> It seems that this series [1] has been stale for more than a month and also
> this series seems to be properly reviewed and acked already. 
> 
> From what Jan has replied to Roger and Andrew:
> "... this addition the series would now look to be ready to go in,
> I'd like to have some form of confirmation by you, Andrew, that
> you now view this as meeting the comments you gave on an earlier
> version."
> 
> So I guess this can be merged. Sending this as a gentle reminder for
> possible actions from Roger and Andrew. Thanks!

My view here remains as before - I'd prefer to avoid merging this
without at least informal agreement by Andrew.

> Also, not sure why my acked-by for the CHANGELOG.md is missing in
> patchwork, just in case - for the change in CHANGELOG.md in patch#3:
> 
> Acked-by: Henry Wang <Henry.Wang@arm.com>

At a guess that might be because that earlier reply that you did send
was to 0/3, not 3/3.

Jan

RE: [PATCH v6 3/3] amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD

[Henry Wang]

Hi Jan (and Andrew),

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> On 17.06.2022 05:26, Henry Wang wrote:
> > It seems that this series [1] has been stale for more than a month and also
> > this series seems to be properly reviewed and acked already.
> >
> > From what Jan has replied to Roger and Andrew:
> > "... this addition the series would now look to be ready to go in,
> > I'd like to have some form of confirmation by you, Andrew, that
> > you now view this as meeting the comments you gave on an earlier
> > version."
> >
> > So I guess this can be merged. Sending this as a gentle reminder for
> > possible actions from Roger and Andrew. Thanks!
> 
> My view here remains as before - I'd prefer to avoid merging this
> without at least informal agreement by Andrew.

Sure, then I would route this email to Andrew (by directly "To:" him) so
that he can take a look when he gets some free time.

> 
> > Also, not sure why my acked-by for the CHANGELOG.md is missing in
> > patchwork, just in case - for the change in CHANGELOG.md in patch#3:
> >
> > Acked-by: Henry Wang <Henry.Wang@arm.com>
> 
> At a guess that might be because that earlier reply that you did send
> was to 0/3, not 3/3.

Yep that should be the reason - now this patch in patchwork has my
acked-by. Thanks for the information and I will keep this in mind in
the future :)

Kind regards,
Henry

> 
> Jan

PING: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Jan Beulich]

On 17.05.2022 17:31, Roger Pau Monne wrote:
> Roger Pau Monne (3):
>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL
>   amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD

While, somewhat different from Jürgen's series, here the delay is more
voluntary (in that I had told Roger that I'd prefer to commit this only
with your (perhaps informal) agreement, I think we also can't wait much
longer. I'm willing to give it until the end of next week, so I guess
I'd move forward with committing early the week after, unless I hear
substantial arguments against doing so (at which point the two of us
would likely need to decide who's going to pick up this work while
Roger is away).

Once again thanks for your understanding,
Jan

Re: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Jan Beulich]

On 17.05.2022 17:31, Roger Pau Monne wrote:
> Roger Pau Monne (3):
>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL
>   amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD

I came to realize that I had announced that I would commit this about a
month ago. I've done so now, but there was quite a bit of re-basing
necessary, to a fair degree because of this delay that I did introduce
by oversight. I hope I didn't screw up anywhere.

Jan

Re: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Andrew Cooper]

On 15/08/2022 09:01, Jan Beulich wrote:
> On 17.05.2022 17:31, Roger Pau Monne wrote:
>> Roger Pau Monne (3):
>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL
>>   amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD
> I came to realize that I had announced that I would commit this about a
> month ago. I've done so now, but there was quite a bit of re-basing
> necessary, to a fair degree because of this delay that I did introduce
> by oversight. I hope I didn't screw up anywhere.

Revert them, or I will.

There has not been adequate review or testing.

Patch 2 in particular is firmly nacked, because the only thing I have
ever suggested in that area is deleting the patch.

~Andrew

Re: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Jan Beulich]

On 15.08.2022 10:15, Andrew Cooper wrote:
> On 15/08/2022 09:01, Jan Beulich wrote:
>> On 17.05.2022 17:31, Roger Pau Monne wrote:
>>> Roger Pau Monne (3):
>>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL
>>>   amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
>>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD
>> I came to realize that I had announced that I would commit this about a
>> month ago. I've done so now, but there was quite a bit of re-basing
>> necessary, to a fair degree because of this delay that I did introduce
>> by oversight. I hope I didn't screw up anywhere.
> 
> Revert them, or I will.

There is no basis for reverting, at this point at least. May I remind
you of the fact that Xen is a community project? I know you've done
reverts in the past without even waiting for a discussion to settle,
but this wasn't okay back then and is not going to be okay this time
round, nor going forward. If you see issues with a series, and in
particular one which is otherwise fully qualified for committing,
you ought to voice these concerns. You cannot expect people to guess
that you're still not happy with the adjustments which were made in
an attempt to address earlier voiced concerns.

> There has not been adequate review or testing.

No adequate review? Am I a 2nd class citizen all of the sudden? In my
reviews I've tried hard to account for the few comments you gave (or
should I say that Roger was able to shake out of you)? Plus I've said
more than once that I would prefer to not commit this without you
having given it a (perhaps just informal) look over. Yet no feedback
ever surfaced. I don't recall you going ...

> Patch 2 in particular is firmly nacked, because the only thing I have
> ever suggested in that area is deleting the patch.

... this far (and in particular not for the later versions of the
series), but I do recall Roger re-working the patch to (try to)
address your concerns. Going from just my mailbox (which goes back
only to v3) I see no replies from you _at all_ on this patch. There
was a longish reply to 0/3, but nothing on v4 or newer, despite
pings which were sent your way.

Knowing this has happened in the past - is your reaction based on v6
or rather on the last version you've actually looked at (presumably
v3)?

Jan

Re: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Andrew Cooper]

On 15/08/2022 10:14, Jan Beulich wrote:
> On 15.08.2022 10:15, Andrew Cooper wrote:
>> On 15/08/2022 09:01, Jan Beulich wrote:
>>> On 17.05.2022 17:31, Roger Pau Monne wrote:
>>>> Roger Pau Monne (3):
>>>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL
>>>>   amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
>>>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD
>>> I came to realize that I had announced that I would commit this about a
>>> month ago. I've done so now, but there was quite a bit of re-basing
>>> necessary, to a fair degree because of this delay that I did introduce
>>> by oversight. I hope I didn't screw up anywhere.
>> Revert them, or I will.
> There is no basis for reverting

You have falsified tags from me, which is a consequence of the series
not having been reviewed correctly.

~Andrew

Re: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Jan Beulich]

On 15.08.2022 20:49, Andrew Cooper wrote:
> On 15/08/2022 10:14, Jan Beulich wrote:
>> On 15.08.2022 10:15, Andrew Cooper wrote:
>>> On 15/08/2022 09:01, Jan Beulich wrote:
>>>> On 17.05.2022 17:31, Roger Pau Monne wrote:
>>>>> Roger Pau Monne (3):
>>>>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL
>>>>>   amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
>>>>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD
>>>> I came to realize that I had announced that I would commit this about a
>>>> month ago. I've done so now, but there was quite a bit of re-basing
>>>> necessary, to a fair degree because of this delay that I did introduce
>>>> by oversight. I hope I didn't screw up anywhere.
>>> Revert them, or I will.
>> There is no basis for reverting
> 
> You have falsified tags from me, which is a consequence of the series
> not been reviewed correctly.

Andrew, please, come on. I have not added any tags from you, hence I
cannot possibly have falsified any. The Suggested-by tags had been
there all the time. I also cannot see any evidence of "the series not
[having] been reviewed correctly". Please can you support your claims
by actual facts?

As to the series itself and its possible reverting (or fixing) - can
you please reply with technical comments on the problematic patch(es)
of the most recent version?

Henry - you may want to add this to your list of things to monitor.
The situation will need resolving one way or another, but obviously
we first need to determine the most suitable way.

Jan

Re: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Roger Pau Monné]

On Mon, Aug 15, 2022 at 06:49:08PM +0000, Andrew Cooper wrote:
> On 15/08/2022 10:14, Jan Beulich wrote:
> > On 15.08.2022 10:15, Andrew Cooper wrote:
> >> On 15/08/2022 09:01, Jan Beulich wrote:
> >>> On 17.05.2022 17:31, Roger Pau Monne wrote:
> >>>> Roger Pau Monne (3):
> >>>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL
> >>>>   amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
> >>>>   amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD
> >>> I came to realize that I had announced that I would commit this about a
> >>> month ago. I've done so now, but there was quite a bit of re-basing
> >>> necessary, to a fair degree because of this delay that I did introduce
> >>> by oversight. I hope I didn't screw up anywhere.
> >> Revert them, or I will.
> > There is no basis for reverting
> 
> You have falsified tags from me, which is a consequence of the series
> not having been reviewed correctly.

Seeing the changes done to the commits, I guess the problem was the
'Suggested-by' tag.  This was added by me, and has been there since v1
because it was you who suggested to do this work, and additionally
provided guidance on how the implementation should look like in:

https://lore.kernel.org/xen-devel/4457dcd5-6a64-355a-b794-6b404cf90335@citrix.com/

I'm sorry if this turned out to not look like you expected/wanted.
It's possible we had informal conversations about this where we
discussed changes, but TBH I have quite a big queue of patches, so
it's likely I've forgotten about.

I'm happy to make any further adjustments to the code, but I will need
to be pointed out at issues.

Thanks, Roger.

Re: [PATCH v6 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[George Dunlap]

[-- Attachment #1.1: Type: text/plain, Size: 1677 bytes --]

Hello all,

This thread was brought to the attention of the Code of Conduct team because it contains some fairly serious accusations. Having looked into the matter, and given the people involved a chance to respond, we’d like to set the record straight (to the best of our current knowledge).

First of all, we’d like to acknowledge that the issue addressed in this series has been a long-term source of frustration: Andrew first mentioned the need for this functionality in 2018, and from then until January 2022 repeatedly mentioned it as something critical to get in but which he simply didn’t have time to personally address.

That said, while his frustration is understandable, there were a number of accusations made which, as far as we can tell, were unfounded; so we want to set the record straight. In particular, having looked into the history of this series and had discussions with all the parties, we have concluded the following:

* Roger attempted to address all of the feedback Andrew gave; and not only was the minimum check-in process followed, but Jan and Roger made every effort to get Andrew's feedback before checking in the patch.

* The "Suggested-by" tags were reasonable for Roger to have put in, and reasonable for Jan to have retained.

This is not the kind of community we strive to be. In particular, we’d like to exhort people to avoid charged language and attempt to lay out the facts — with references — such that they speak for themselves.

We are also attempting to improve communication and shared understanding, to hopefully keep frustration levels low in the future.

Thanks,

 -The Xen Project Conduct Team

[-- Attachment #1.2: Type: text/html, Size: 2409 bytes --]

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 488 bytes --]