diff mbox series

Speculative mitigation facilities report wrong status

Message ID 20190731092959.21655-1-jnwang@suse.com (mailing list archive)
State Superseded
Headers show
Series Speculative mitigation facilities report wrong status | expand

Commit Message

Jin Nan Wang July 31, 2019, 9:30 a.m. UTC 
Diff with 'spec-ctrl=no' and without.
====================================================

Comments

Jin Nan Wang July 31, 2019, 10:45 a.m. UTC | #1 
Hi folks,

On 7/31/19 5:44 PM, Andrew Cooper wrote:
> The check for reporting MD_CLEAR must stay as X86_FEATURE_MD_CLEAR,
> because this is a property in microcode which no controls, and nothing
> further to virtualise at Xen's level.

There are two solution, which one would you like?

solution1: make sure set X86_FEATURE_SC_VERW_PV/HVM, under
X86_FEATURE_MD_CLEAR exist.

~ 1084     if ( opt_md_clear_pv && boot_cpu_has(X86_FEATURE_MD_CLEAR))
  1085         setup_force_cpu_cap(X86_FEATURE_SC_VERW_PV);
~ 1086     if ( opt_md_clear_pv || opt_md_clear_hvm &&
boot_cpu_has(X86_FEATURE_MD_CLEAR))
  1087         setup_force_cpu_cap(X86_FEATURE_SC_VERW_IDLE);
~ 1088     if ( opt_md_clear_hvm && !(caps & ARCH_CAPS_SKIP_L1DFL) &&
!opt_l1d_flush && boot_cpu_has(X86_FEATURE_MD_CLEAR))
  1089         setup_force_cpu_cap(X86_FEATURE_SC_VERW_HVM);


Solution2:

   365 #ifdef CONFIG_HVM
   366     printk("  Support for HVM VMs:%s%s%s%s%s\n",
   367            (boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
   368             boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ||
~  369             (boot_cpu_has(X86_FEATURE_MD_CLEAR) &&
boot_cpu_has(X86_FEATURE_SC_VERW_HVM)) ||
   370             opt_eager_fpu)                           ?
""               : " None",
   371            boot_cpu_has(X86_FEATURE_SC_MSR_HVM)      ? "
MSR_SPEC_CTRL" : "",
   372            boot_cpu_has(X86_FEATURE_SC_RSB_HVM)      ? "
RSB"           : "",
   373            opt_eager_fpu                             ? "
EAGER_FPU"     : "",
~  374            (boot_cpu_has(X86_FEATURE_MD_CLEAR) &&
boot_cpu_has(X86_FEATURE_SC_VERW_HVM))     ? " MD_CLEAR"      : "");
   375
   376 #endif
   377 #ifdef CONFIG_PV
   378     printk("  Support for PV VMs:%s%s%s%s%s\n",
   379            (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
   380             boot_cpu_has(X86_FEATURE_SC_RSB_PV) ||
~  381             (boot_cpu_has(X86_FEATURE_MD_CLEAR) &&
boot_cpu_has(X86_FEATURE_SC_VERW_PV)) ||
   382             opt_eager_fpu)                           ?
""               : " None",
   383            boot_cpu_has(X86_FEATURE_SC_MSR_PV)       ? "
MSR_SPEC_CTRL" : "",
   384            boot_cpu_has(X86_FEATURE_SC_RSB_PV)       ? "
RSB"           : "",
   385            opt_eager_fpu                             ? "
EAGER_FPU"     : "",
~  386            (boot_cpu_has(X86_FEATURE_MD_CLEAR) &&
boot_cpu_has(X86_FEATURE_SC_VERW_PV))      ? " MD_CLEAR"      : "");
Andrew Cooper July 31, 2019, 11:04 a.m. UTC | #2 
On 31/07/2019 11:45, Jin Nan Wang wrote:
> Hi folks,
>
> On 7/31/19 5:44 PM, Andrew Cooper wrote:
>> The check for reporting MD_CLEAR must stay as X86_FEATURE_MD_CLEAR,
>> because this is a property in microcode which no controls, and nothing
>> further to virtualise at Xen's level.
> There are two solution, which one would you like?
>
> solution1: make sure set X86_FEATURE_SC_VERW_PV/HVM, under
> X86_FEATURE_MD_CLEAR exist.

No - this is not a solution.  This causes Xen to ignore spec-ctrl=0 and
use VERW itself when it was instructed not to.

The only bug here is in the printed output.  In your example, with
MDS-capable microcode and spec-ctrl=0 on the command line, the correct
output should be

(XEN)   Support for HVM VMs: MD_CLEAR
(XEN)   Support for PV VMs: MD_CLEAR

The actual behaviour of Xen is correct, but the printed message is
confusing.

~Andrew
diff mbox series

Patch

--- xen.dmesg.5.log     2019-07-31 14:55:38.138173874 +0800
+++ xen.dmesg.6.log     2019-07-31 14:59:50.223516313 +0800
@@ -7,7 +7,7 @@ 
 (XEN) Xen version 4.12.0_14-1 (abuild@suse.de) (gcc (SUSE Linux) 4.8.5) debug=n  Mon Jun 17 15:08:33 UTC 2019
 (XEN) Latest ChangeSet:
 (XEN) Bootloader: GRUB2 2.02
-(XEN) Command line: vga=gfx-1024x768x16 crashkernel=251M<4G ucode=scan console=vga,com1 loglvl=all guest_loglvl=all
+(XEN) Command line: vga=gfx-1024x768x16 crashkernel=251M<4G ucode=scan spec-ctrl=no console=vga,com1 loglvl=all guest_loglvl=all
 (XEN) Xen image load base address: 0
 (XEN) Video information:
 (XEN)  VGA is graphics mode 1024x768, 16 bpp
@@ -159,12 +159,12 @@ 
 (XEN) Speculative mitigation facilities:
 (XEN)   Hardware features: IBRS/IBPB STIBP L1D_FLUSH SSBD MD_CLEAR
 (XEN)   Compiled-in support: INDIRECT_THUNK SHADOW_PAGING
-(XEN)   Xen settings: BTI-Thunk JMP, SPEC_CTRL: IBRS+ SSBD-, Other: IBPB L1D_FLUSH VERW
+(XEN)   Xen settings: BTI-Thunk JMP, SPEC_CTRL: IBRS- SSBD-, Other:
 (XEN)   L1TF: believed vulnerable, maxphysaddr L1D 46, CPUID 46, Safe address 300000000000
-(XEN)   Support for HVM VMs: MSR_SPEC_CTRL RSB EAGER_FPU MD_CLEAR
-(XEN)   Support for PV VMs: MSR_SPEC_CTRL RSB EAGER_FPU MD_CLEAR
-(XEN)   XPTI (64-bit PV only): Dom0 enabled, DomU enabled (with PCID)
-(XEN)   PV L1TF shadowing: Dom0 disabled, DomU enabled
+(XEN)   Support for HVM VMs: None MD_CLEAR
+(XEN)   Support for PV VMs: None MD_CLEAR
+(XEN)   XPTI (64-bit PV only): Dom0 disabled, DomU disabled (with PCID)
+(XEN)   PV L1TF shadowing: Dom0 disabled, DomU disabled
 (XEN) Using scheduler: SMP Credit Scheduler rev2 (credit2)
 (XEN) Initializing Credit2 scheduler
 (XEN)  load_precision_shift: 18
==================================================

In "Support for HVM VMs: Support for PV VMs: " lines,
Others feature is reported as "NONE", MD_CLEAR not.

code review:
xen/arch/x86/spec_ctrl.c:
    99         disable_common:
   100             opt_rsb_pv = false;
   101             opt_rsb_hvm = false;
   102             opt_md_clear_pv = 0;   <----- they have been disable when 'spec-ctrl=no'
   103             opt_md_clear_hvm = 0;
   104

X86_FEATURE_SC_VERW_PV, X86_FEATURE_SC_VERW_HVM will not be enabled

 1070     if ( opt_md_clear_pv )
  1071         setup_force_cpu_cap(X86_FEATURE_SC_VERW_PV);
  1072     if ( opt_md_clear_pv || opt_md_clear_hvm )
  1073         setup_force_cpu_cap(X86_FEATURE_SC_VERW_IDLE);
  1074     if ( opt_md_clear_hvm && !(caps & ARCH_CAPS_SKIP_L1DFL) && !opt_l1d_flush )
  1075         setup_force_cpu_cap(X86_FEATURE_SC_VERW_HVM);

But when we report the status of MD_CLEAR, we use X86_FEATURE_MD_CLEAR to check.
it seems not good.

   360     printk("  Support for HVM VMs:%s%s%s%s%s\n",
   361            (boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
   362             boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ||
   363             opt_eager_fpu)                           ? ""               : " None",
   364            boot_cpu_has(X86_FEATURE_SC_MSR_HVM)      ? " MSR_SPEC_CTRL" : "",
   365            boot_cpu_has(X86_FEATURE_SC_RSB_HVM)      ? " RSB"           : "",
   366            opt_eager_fpu                             ? " EAGER_FPU"     : "",
   367    ---->   boot_cpu_has(X86_FEATURE_MD_CLEAR)        ? " MD_CLEAR"      : "");
   368
   369 #endif
   370 #ifdef CONFIG_PV
   371     printk("  Support for PV VMs:%s%s%s%s%s\n",
   372            (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
   373             boot_cpu_has(X86_FEATURE_SC_RSB_PV) ||
   374             opt_eager_fpu)                           ? ""               : " None",
   375            boot_cpu_has(X86_FEATURE_SC_MSR_PV)       ? " MSR_SPEC_CTRL" : "",
   376            boot_cpu_has(X86_FEATURE_SC_RSB_PV)       ? " RSB"           : "",
   377            opt_eager_fpu                             ? " EAGER_FPU"     : "",
   378    ---->   boot_cpu_has(X86_FEATURE_MD_CLEAR)        ? " MD_CLEAR"      : "");

There is a patch for this issue.

diff -Nurp xen-4.12.0-testing.orig/xen/arch/x86/spec_ctrl.c xen-4.12.0-testing/xen/arch/x86/spec_ctrl.c
--- xen-4.12.0-testing.orig/xen/arch/x86/spec_ctrl.c	2019-07-31 13:49:41.755568027 +0800
+++ xen-4.12.0-testing/xen/arch/x86/spec_ctrl.c	2019-07-31 15:08:10.158994444 +0800
@@ -360,22 +360,24 @@  static void __init print_details(enum in
     printk("  Support for HVM VMs:%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_SC_VERW_HVM) ||
             opt_eager_fpu)                           ? ""               : " None",
            boot_cpu_has(X86_FEATURE_SC_MSR_HVM)      ? " MSR_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"      : "");
+           boot_cpu_has(X86_FEATURE_SC_VERW_HVM)     ? " MD_CLEAR"      : "");

 #endif
 #ifdef CONFIG_PV
     printk("  Support for PV VMs:%s%s%s%s%s\n",
            (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
             boot_cpu_has(X86_FEATURE_SC_RSB_PV) ||
+            boot_cpu_has(X86_FEATURE_SC_VERW_PV) ||
             opt_eager_fpu)                           ? ""               : " None",
            boot_cpu_has(X86_FEATURE_SC_MSR_PV)       ? " MSR_SPEC_CTRL" : "",
            boot_cpu_has(X86_FEATURE_SC_RSB_PV)       ? " RSB"           : "",
            opt_eager_fpu                             ? " EAGER_FPU"     : "",
-           boot_cpu_has(X86_FEATURE_MD_CLEAR)        ? " MD_CLEAR"      : "");
+           boot_cpu_has(X86_FEATURE_SC_VERW_PV)      ? " MD_CLEAR"      : "");

     printk("  XPTI (64-bit PV only): Dom0 %s, DomU %s (with%s PCID)\n",
            opt_xpti_hwdom ? "enabled" : "disabled",

Signed-off-by: James Wang <jnwang@suse.com>
---
 xen/arch/x86/spec_ctrl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index cada9a058e..759eee452d 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -366,22 +366,24 @@  static void __init print_details(enum ind_thunk thunk, uint64_t caps)
     printk("  Support for HVM VMs:%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_SC_VERW_HVM) ||
             opt_eager_fpu)                           ? ""               : " None",
            boot_cpu_has(X86_FEATURE_SC_MSR_HVM)      ? " MSR_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"      : "");
+           boot_cpu_has(X86_FEATURE_SC_VERW_HVM)     ? " MD_CLEAR"      : "");
 
 #endif
 #ifdef CONFIG_PV
     printk("  Support for PV VMs:%s%s%s%s%s\n",
            (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
             boot_cpu_has(X86_FEATURE_SC_RSB_PV) ||
+            boot_cpu_has(X86_FEATURE_SC_VERW_PV) ||
             opt_eager_fpu)                           ? ""               : " None",
            boot_cpu_has(X86_FEATURE_SC_MSR_PV)       ? " MSR_SPEC_CTRL" : "",
            boot_cpu_has(X86_FEATURE_SC_RSB_PV)       ? " RSB"           : "",
            opt_eager_fpu                             ? " EAGER_FPU"     : "",
-           boot_cpu_has(X86_FEATURE_MD_CLEAR)        ? " MD_CLEAR"      : "");
+           boot_cpu_has(X86_FEATURE_SC_VERW_PV)      ? " MD_CLEAR"      : "");
 
     printk("  XPTI (64-bit PV only): Dom0 %s, DomU %s (with%s PCID)\n",
            opt_xpti_hwdom ? "enabled" : "disabled",