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

[Xen-devel] [PATCH] x86: fix "xpti=" and "pv-l1tf=" yet again



While commit 2a3b34ec47 ("x86/spec-ctrl: Yet more fixes for xpti=
parsing") indeed fixed "xpti=dom0", it broke "xpti=no-dom0", in that
this then became equivalent to "xpti=no". In particular, the presence
of "xpti=" alone on the command line means nothing as to which
default is to be overridden; "xpti=no-dom0" ought to have no effect
for DomU-s (and vice versa), as this is distinct from both
"xpti=no-dom0,domu" and "xpti=no-dom0,no-domu".

Here as well as for "pv-l1tf=" I think there's no way around tracking
the "use default" state separately for Dom0 and DomU-s. Introduce
individual bits for this, and convert the variables' types (back) to
uint8_t.

Additionally the earlier change claimed to have got rid of the
'parameter "xpti" has invalid value "", rc=-22!' log message for "xpti"
alone on the command line, which wasn't the case (the option took effect
nevertheless). Fix this as well.

Finally also support a "default" sub-option for "pv-l1tf=", just like
"xpti=" does.

It is perhaps worth to note that OPT_<what>_DOM<which>_DEFAULT set
implies OPT_<what>_DOM<which> clear, which is being utilized in a number
of places (we effectively want to hold two tristates in a single
variable, which means the fourth state is impossible).

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
Seeing the redundancy between OPT_XPTI_* and OPT_PV_L1TF_*, I wonder
whether it wouldn't be worthwhile to fold the constants. Which option
they apply to is easily seen from the variable they get used with.

--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1563,7 +1563,7 @@ certain you don't plan on having PV gues
 turning it off can reduce the attack surface.
 
 ### pv-l1tf (x86)
-> `= List of [ <bool>, dom0=<bool>, domu=<bool> ]`
+> `= List of [ default, <bool>, dom0=<bool>, domu=<bool> ]`
 
 > Default: `false` on believed-unaffected hardware, or in pv-shim mode.
 >          `domu`  on believed-affected hardware.
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -134,15 +134,12 @@ static int __init parse_spec_ctrl(const
 
             opt_eager_fpu = 0;
 
-            if ( opt_xpti < 0 )
-                opt_xpti = 0;
+            opt_xpti &= ~(OPT_XPTI_DOM0_DEFAULT | OPT_XPTI_DOMU_DEFAULT);
+            opt_pv_l1tf &= ~OPT_PV_L1TF_DOMU_DEFAULT;
 
             if ( opt_smt < 0 )
                 opt_smt = 1;
 
-            if ( opt_pv_l1tf < 0 )
-                opt_pv_l1tf = 0;
-
         disable_common:
             opt_rsb_pv = false;
             opt_rsb_hvm = false;
@@ -219,17 +216,13 @@ static int __init parse_spec_ctrl(const
 }
 custom_param("spec-ctrl", parse_spec_ctrl);
 
-int8_t __read_mostly opt_pv_l1tf = -1;
+uint8_t __read_mostly opt_pv_l1tf = OPT_PV_L1TF_DOMU_DEFAULT;
 
 static __init int parse_pv_l1tf(const char *s)
 {
     const char *ss;
     int val, rc = 0;
 
-    /* Inhibit the defaults as an explicit choice has been given. */
-    if ( opt_pv_l1tf == -1 )
-        opt_pv_l1tf = 0;
-
     /* Interpret 'pv-l1tf' alone in its positive boolean form. */
     if ( *s == '\0' )
         opt_pv_l1tf = OPT_PV_L1TF_DOM0 | OPT_PV_L1TF_DOMU;
@@ -250,13 +243,16 @@ static __init int parse_pv_l1tf(const ch
             break;
 
         default:
-            if ( (val = parse_boolean("dom0", s, ss)) >= 0 )
+            if ( !strcmp(s, "default") )
+                opt_xpti = OPT_PV_L1TF_DOMU_DEFAULT;
+            else if ( (val = parse_boolean("dom0", s, ss)) >= 0 )
                 opt_pv_l1tf = ((opt_pv_l1tf & ~OPT_PV_L1TF_DOM0) |
                                (val ? OPT_PV_L1TF_DOM0 : 0));
             else if ( (val = parse_boolean("domu", s, ss)) >= 0 )
-                opt_pv_l1tf = ((opt_pv_l1tf & ~OPT_PV_L1TF_DOMU) |
+                opt_pv_l1tf = ((opt_pv_l1tf & ~(OPT_PV_L1TF_DOMU_DEFAULT |
+                                                OPT_PV_L1TF_DOMU)) |
                                (val ? OPT_PV_L1TF_DOMU : 0));
-            else
+            else if ( *s )
                 rc = -EINVAL;
             break;
         }
@@ -657,17 +653,22 @@ static __init void l1tf_calculations(uin
                                             : (3ul << (paddr_bits - 2))));
 }
 
-int8_t __read_mostly opt_xpti = -1;
+uint8_t __read_mostly opt_xpti = OPT_XPTI_DOM0_DEFAULT | OPT_XPTI_DOMU_DEFAULT;
 
 static __init void xpti_init_default(uint64_t caps)
 {
     if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
         caps = ARCH_CAPABILITIES_RDCL_NO;
 
-    if ( caps & ARCH_CAPABILITIES_RDCL_NO )
-        opt_xpti = 0;
-    else
-        opt_xpti = OPT_XPTI_DOM0 | OPT_XPTI_DOMU;
+    if ( !(caps & ARCH_CAPABILITIES_RDCL_NO) )
+    {
+        if ( opt_xpti & OPT_XPTI_DOM0_DEFAULT )
+            opt_xpti |= OPT_XPTI_DOM0;
+        if ( opt_xpti & OPT_XPTI_DOMU_DEFAULT )
+            opt_xpti |= OPT_XPTI_DOMU;
+    }
+
+    opt_xpti &= ~(OPT_XPTI_DOM0_DEFAULT | OPT_XPTI_DOMU_DEFAULT);
 }
 
 static __init int parse_xpti(const char *s)
@@ -675,10 +676,6 @@ static __init int parse_xpti(const char
     const char *ss;
     int val, rc = 0;
 
-    /* Inhibit the defaults as an explicit choice has been given. */
-    if ( opt_xpti == -1 )
-        opt_xpti = 0;
-
     /* Interpret 'xpti' alone in its positive boolean form. */
     if ( *s == '\0' )
         opt_xpti = OPT_XPTI_DOM0 | OPT_XPTI_DOMU;
@@ -700,14 +697,16 @@ static __init int parse_xpti(const char
 
         default:
             if ( !strcmp(s, "default") )
-                opt_xpti = -1;
+                opt_xpti = OPT_XPTI_DOM0_DEFAULT | OPT_XPTI_DOMU_DEFAULT;
             else if ( (val = parse_boolean("dom0", s, ss)) >= 0 )
-                opt_xpti = (opt_xpti & ~OPT_XPTI_DOM0) |
+                opt_xpti = (opt_xpti & ~(OPT_XPTI_DOM0_DEFAULT |
+                                         OPT_XPTI_DOM0)) |
                            (val ? OPT_XPTI_DOM0 : 0);
             else if ( (val = parse_boolean("domu", s, ss)) >= 0 )
-                opt_xpti = (opt_xpti & ~OPT_XPTI_DOMU) |
+                opt_xpti = (opt_xpti & ~(OPT_XPTI_DOMU_DEFAULT |
+                                         OPT_XPTI_DOMU)) |
                            (val ? OPT_XPTI_DOMU : 0);
-            else
+            else if ( *s )
                 rc = -EINVAL;
             break;
         }
@@ -862,8 +861,7 @@ void __init init_speculation_mitigations
     if ( default_xen_spec_ctrl )
         setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
 
-    if ( opt_xpti == -1 )
-        xpti_init_default(caps);
+    xpti_init_default(caps);
 
     if ( opt_xpti == 0 )
         setup_force_cpu_cap(X86_FEATURE_NO_XPTI);
@@ -879,13 +877,11 @@ void __init init_speculation_mitigations
      * In shim mode, SHADOW is expected to be compiled out, and a malicious
      * guest kernel can only attack the shim Xen, not the host Xen.
      */
-    if ( opt_pv_l1tf == -1 )
-    {
-        if ( pv_shim || !cpu_has_bug_l1tf )
-            opt_pv_l1tf = 0;
-        else
-            opt_pv_l1tf = OPT_PV_L1TF_DOMU;
-    }
+    if ( (opt_pv_l1tf & OPT_PV_L1TF_DOMU_DEFAULT) &&
+         !pv_shim && cpu_has_bug_l1tf )
+        opt_pv_l1tf |= OPT_PV_L1TF_DOMU;
+
+    opt_pv_l1tf &= ~OPT_PV_L1TF_DOMU_DEFAULT;
 
     /*
      * By default, enable L1D_FLUSH on L1TF-vulnerable hardware, unless
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -35,13 +35,16 @@ extern bool bsp_delay_spec_ctrl;
 extern uint8_t default_xen_spec_ctrl;
 extern uint8_t default_spec_ctrl_flags;
 
-extern int8_t opt_xpti;
+extern uint8_t opt_xpti;
 #define OPT_XPTI_DOM0  0x01
 #define OPT_XPTI_DOMU  0x02
+#define OPT_XPTI_DOM0_DEFAULT 0x10
+#define OPT_XPTI_DOMU_DEFAULT 0x20
 
-extern int8_t opt_pv_l1tf;
+extern uint8_t opt_pv_l1tf;
 #define OPT_PV_L1TF_DOM0  0x01
 #define OPT_PV_L1TF_DOMU  0x02
+#define OPT_PV_L1TF_DOMU_DEFAULT 0x20
 
 /*
  * The L1D address mask, which might be wider than reported in CPUID, and the




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

 


Rackspace

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