Re: [Xen-devel] [PATCH] Speculative mitigation facilities report wrong status

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 31/07/2019 10:30, Jin Nan Wang wrote:
> Diff with 'spec-ctrl=no' and without.
> ====================================================
> --- 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@xxxxxxx) (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.

Thankyou for the report.  However, the patch is only half correct.

It should only be the first part, adding the extra check to avoid "None".

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.

I.e. even with spec-ctrl=no, if the microcode is new enough, the guest
gets to see the MD_CLEAR CPUID bit, because all it means is "this
preexisting instruction now has a new side effect".

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel