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

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

[Jan Beulich]

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@suse.com>
---
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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Jan Beulich]

>>> On 21.08.18 at 12:44,  wrote:
> 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@suse.com>
> ---
> 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;

Obviously with this corrected, as pointed out by Jürgen.

Jan

> +            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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Andrew Cooper]

On 21/08/18 11:44, Jan Beulich wrote:
> 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".

That was accidental, but the end result is consistent with other options.

As with spec-ctrl, if someone wants to start making fine-grain control,
they should specify everything.  There is a reason why the

**WARNING: Any use of this option may interfere with heuristics.  Use
with extreme care.**

disclaimer exists.

>  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.

The code below is getting unmanageably complicated.  Given that its all
slowpath operations, I think switching to 4 separate int8_t's would be
better than trying to multiplex several tristates into the same byte. 
It would also remove all of the constants.

>
> 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.

The earlier change did do what the patch claimed, to the best of my
knowledge.  Can you explain what is apparently still broken, because its
not clear from this description?

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

No.  Having "default" was a mistake for xpti= I would have objected to
if I'd noticed it.

The default is not specifying the option in the first place. 
"foo=default" is entirely redundant, and worse, in combination with your
"Make "spec-ctrl=no" a global disable of all mitigations" patch:

  "spec-ctrl=0 $FOO=default" and
  "$FOO=default spec-ctrl=0"

now result in different things happening.

~Andrew

>
> 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@suse.com>
> ---
> 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.
>
>


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

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

[Jan Beulich]

>>> On 29.08.18 at 14:36, <andrew.cooper3@citrix.com> wrote:
> On 21/08/18 11:44, Jan Beulich wrote:
>> 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".
> 
> That was accidental, but the end result is consistent with other options.
> 
> As with spec-ctrl, if someone wants to start making fine-grain control,
> they should specify everything.  There is a reason why the
> 
> **WARNING: Any use of this option may interfere with heuristics.  Use
> with extreme care.**
> 
> disclaimer exists.

As said ...

>>  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, the current behavior is pretty counterintuitive. I'm also
curious to know how this is "consistent with other options" - can
you give two or three examples at least?

>> 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.
> 
> The code below is getting unmanageably complicated.  Given that its all
> slowpath operations, I think switching to 4 separate int8_t's would be
> better than trying to multiplex several tristates into the same byte. 
> It would also remove all of the constants.

I can do that; it simply seemed more intrusive a change that way.

>> 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.
> 
> The earlier change did do what the patch claimed, to the best of my
> knowledge.  Can you explain what is apparently still broken, because its
> not clear from this description?

The earlier commit claims the log message went away, which is not
the case according to the testing that I had done with various
option combinations while putting together this change. Hence this

-            else
+            else if ( *s )
                 rc = -EINVAL;
             break;

change in both of the handlers

>> Finally also support a "default" sub-option for "pv-l1tf=", just like
>> "xpti=" does.
> 
> No.  Having "default" was a mistake for xpti= I would have objected to
> if I'd noticed it.
> 
> The default is not specifying the option in the first place. 
> "foo=default" is entirely redundant,

It is not. I've said so a number of times before: You need a way
to go back to the default when you want to override a stored
portion of the command line that you can't edit while booting (as
is minimally the case for xen.efi, which takes the command line
out of a config file).

>and worse, in combination with your
> "Make "spec-ctrl=no" a global disable of all mitigations" patch:
> 
>   "spec-ctrl=0 $FOO=default" and
>   "$FOO=default spec-ctrl=0"
> 
> now result in different things happening.

And validly so: Order of options matters.

Jan



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

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

[Jan Beulich]

>>> On 29.08.18 at 14:36, <andrew.cooper3@citrix.com> wrote:
> On 21/08/18 11:44, Jan Beulich wrote:
>> 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".
> 
> That was accidental, but the end result is consistent with other options.
> 
> As with spec-ctrl, if someone wants to start making fine-grain control,
> they should specify everything.  There is a reason why the
> 
> **WARNING: Any use of this option may interfere with heuristics.  Use
> with extreme care.**
> 
> disclaimer exists.

I've looked again: Such a disclaimer does not exist for xpti= nor
pv-l1tf=, and it shouldn't, as use of these options does not in fact
interfere with any (other) heuristics (they're separate options for
reasons beyond syntax issues that would result if they were folded
into spec-ctrl= ).

If the sole remaining change request was to split the variables into
separate booleans, I can do that (although, as said, I'm not
convinced this is helpful). But there were other open points, and
I'd prefer to either commit v1 with the one copy-and-paste bug
fixed, or send a v2 which has a chance of being accepted (i.e.
with all open points addressed verbally or by code changes).

Jan



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

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

[Jan Beulich]

Ping?

>>> On 11.09.18 at 10:20, <JBeulich@suse.com> wrote:
>>>> On 29.08.18 at 14:36, <andrew.cooper3@citrix.com> wrote:
>> On 21/08/18 11:44, Jan Beulich wrote:
>>> 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".
>> 
>> That was accidental, but the end result is consistent with other options.
>> 
>> As with spec-ctrl, if someone wants to start making fine-grain control,
>> they should specify everything.  There is a reason why the
>> 
>> **WARNING: Any use of this option may interfere with heuristics.  Use
>> with extreme care.**
>> 
>> disclaimer exists.
> 
> I've looked again: Such a disclaimer does not exist for xpti= nor
> pv-l1tf=, and it shouldn't, as use of these options does not in fact
> interfere with any (other) heuristics (they're separate options for
> reasons beyond syntax issues that would result if they were folded
> into spec-ctrl= ).
> 
> If the sole remaining change request was to split the variables into
> separate booleans, I can do that (although, as said, I'm not
> convinced this is helpful). But there were other open points, and
> I'd prefer to either commit v1 with the one copy-and-paste bug
> fixed, or send a v2 which has a chance of being accepted (i.e.
> with all open points addressed verbally or by code changes).
> 
> Jan



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

Re: Ping: [PATCH] x86: fix "xpti=" and "pv-l1tf=" yet again

[Andrew Cooper]

On 18/09/18 13:41, Jan Beulich wrote:
> Ping?

You're very unlikely to get any reply when I'm

>
>>>> On 11.09.18 at 10:20, <JBeulich@suse.com> wrote:
>>>>> On 29.08.18 at 14:36, <andrew.cooper3@citrix.com> wrote:
>>> On 21/08/18 11:44, Jan Beulich wrote:
>>>> 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".
>>> That was accidental, but the end result is consistent with other options.
>>>
>>> As with spec-ctrl, if someone wants to start making fine-grain control,
>>> they should specify everything.  There is a reason why the
>>>
>>> **WARNING: Any use of this option may interfere with heuristics.  Use
>>> with extreme care.**
>>>
>>> disclaimer exists.
>> I've looked again: Such a disclaimer does not exist for xpti= nor
>> pv-l1tf=, and it shouldn't, as use of these options does not in fact
>> interfere with any (other) heuristics (they're separate options for
>> reasons beyond syntax issues that would result if they were folded
>> into spec-ctrl= ).
>>
>> If the sole remaining change request was to split the variables into
>> separate booleans, I can do that (although, as said, I'm not
>> convinced this is helpful).

How can you possibly justify this comment?  Even in your proposed patch,
you note that the current scheme is overloaded and won't work for an
extra boolean.

Further to that, the code is almost impossible to parse.

>>  But there were other open points, and
>> I'd prefer to either commit v1 with the one copy-and-paste bug
>> fixed, or send a v2 which has a chance of being accepted (i.e.
>> with all open points addressed verbally or by code changes).

I'm still not happy about the addition of default.  The ordering of
options causing different results is the dealbreaker (as a side effect
of your spec_ctrl=0 extention), but I'm also not inclined to take bodges
to work around broken boot software configurations.

~Andrew

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

Re: Ping: [PATCH] x86: fix "xpti=" and "pv-l1tf=" yet again

[Jan Beulich]

>>> On 25.09.18 at 18:43, <andrew.cooper3@citrix.com> wrote:
> On 18/09/18 13:41, Jan Beulich wrote:
>> Ping?
> 
> You're very unlikely to get any reply when I'm

I certainly did realize that I wouldn't get an immediate answer, but I
also didn't want to defer the pings any longer.

>>>>> On 11.09.18 at 10:20, <JBeulich@suse.com> wrote:
>>>>>> On 29.08.18 at 14:36, <andrew.cooper3@citrix.com> wrote:
>>>> On 21/08/18 11:44, Jan Beulich wrote:
>>>>> 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".
>>>> That was accidental, but the end result is consistent with other options.
>>>>
>>>> As with spec-ctrl, if someone wants to start making fine-grain control,
>>>> they should specify everything.  There is a reason why the
>>>>
>>>> **WARNING: Any use of this option may interfere with heuristics.  Use
>>>> with extreme care.**
>>>>
>>>> disclaimer exists.
>>> I've looked again: Such a disclaimer does not exist for xpti= nor
>>> pv-l1tf=, and it shouldn't, as use of these options does not in fact
>>> interfere with any (other) heuristics (they're separate options for
>>> reasons beyond syntax issues that would result if they were folded
>>> into spec-ctrl= ).
>>>
>>> If the sole remaining change request was to split the variables into
>>> separate booleans, I can do that (although, as said, I'm not
>>> convinced this is helpful).
> 
> How can you possibly justify this comment?  Even in your proposed patch,
> you note that the current scheme is overloaded and won't work for an
> extra boolean.

Hmm, I don't see me noting anything like this. The only remark that
I think you may possibly mean is the post-commit-message one about
perhaps folding OPT_XPTI_* and OPT_PV_L1TF_*.

> Further to that, the code is almost impossible to parse.

Subjective again, and this patch isn't really making things any worse.
If you felt this was the wrong model, why did you mirror the XPTI
parsing into your L1TF patches?

>>>  But there were other open points, and
>>> I'd prefer to either commit v1 with the one copy-and-paste bug
>>> fixed, or send a v2 which has a chance of being accepted (i.e.
>>> with all open points addressed verbally or by code changes).
> 
> I'm still not happy about the addition of default.  The ordering of
> options causing different results is the dealbreaker (as a side effect
> of your spec_ctrl=0 extention),

The ordering of options being meaningful is there, and has been
there before any of this year's security work. I.e. it also affects
other options, perhaps just not as noticably as is the case here. See
the difference between "cpuidle no-cpuidle" and "no-cpuidle cpuidle",
or "apic=bigsmp apic=default" and "apic=default apic=bigsmp"? Their
behavior is all well defined, afaict.

In fact things like "cpuidle" lack a way to switch back to default mode,
in my opinion. But it's certainly the case that for more complex cases
it is more important to have a way to switch back to default mode;
when it's just a yes/no thing you can as well say "yes" or "no" right
on the command line.

> but I'm also not inclined to take bodges
> to work around broken boot software configurations.

I'm afraid I don't understand this part.

Bottom line: I'm afraid I still don't see a way forward here, i.e. what
I _need_ to change to make the patch acceptable. I'm open to
making changes in how things are done (but I'd like to be told clearly),
but I'm not really open to leaving (parts of) the current broken
behavior broken. In any event I'll break out the hopefully
uncontroversial part of silencing the false error messages.

Jan



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

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

[Jan Beulich]

>>> On 21.08.18 at 15:59, <jgross@suse.com> wrote:
> On 21/08/18 14:40, Jan Beulich wrote:
>>>>> On 21.08.18 at 14:13, <jgross@suse.com> wrote:
>>> On 21/08/18 12:44, Jan Beulich wrote:
>>>> @@ -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;
>>>
>>> Wouldn't setting the default value (DOMU) here be enough? Same for
>>> xpti below?
>> 
>> No, because we want to defer default processing until we've
>> actually obtained the necessary data. While parsing we don't
>> know yet whether "default" means "on" or "off".
>> 
>> Or perhaps I don't understand what you mean?
> 
> I meant:
> 
>      if ( opt_pv_l1tf == -1 )
> -        opt_pv_l1tf = 0;
> +        opt_pv_l1tf = OPT_PV_L1TF_DOMU;
> 
> This starts at the default setting and then applies the settings of the
> sub-options on top of it, instead of starting at "everything off".

Well, as said - this would get in the way of

    if ( (opt_pv_l1tf & OPT_PV_L1TF_DOMU_DEFAULT) &&
         !pv_shim && cpu_has_bug_l1tf )
        opt_pv_l1tf |= OPT_PV_L1TF_DOMU;

close to the end of the patch. IOW "pv-l1tf=dom0" as well
as "pv-l1tf=no-dom0" ought to leave DomU-s running
without the workaround on fixed hardware.

Jan



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

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

[Juergen Gross]

On 21/08/18 14:40, Jan Beulich wrote:
>>>> On 21.08.18 at 14:13, <jgross@suse.com> wrote:
>> On 21/08/18 12:44, Jan Beulich wrote:
>>> @@ -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;
>>
>> Wouldn't setting the default value (DOMU) here be enough? Same for
>> xpti below?
> 
> No, because we want to defer default processing until we've
> actually obtained the necessary data. While parsing we don't
> know yet whether "default" means "on" or "off".
> 
> Or perhaps I don't understand what you mean?

I meant:

     if ( opt_pv_l1tf == -1 )
-        opt_pv_l1tf = 0;
+        opt_pv_l1tf = OPT_PV_L1TF_DOMU;

This starts at the default setting and then applies the settings of the
sub-options on top of it, instead of starting at "everything off".


Juergen

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

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

[Jan Beulich]

>>> On 21.08.18 at 14:13, <jgross@suse.com> wrote:
> On 21/08/18 12:44, Jan Beulich wrote:
>> @@ -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;
> 
> Wouldn't setting the default value (DOMU) here be enough? Same for
> xpti below?

No, because we want to defer default processing until we've
actually obtained the necessary data. While parsing we don't
know yet whether "default" means "on" or "off".

Or perhaps I don't understand what you mean?

>> @@ -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;
> 
> opt_pv_l1tf

Argh, again. And I've tried to be super careful... Thanks for
noticing!

Jan



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

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

[Juergen Gross]

On 21/08/18 14:04, Juergen Gross wrote:
> On 21/08/18 12:44, Jan Beulich wrote:
>> 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).
> 
> Another possibility would be to have two local variables holding the
> bits to clear and to set and to apply those after each sub-option
> parsed.

Ignore please, hit send instead of cancel.


Juergen

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

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

[Juergen Gross]

On 21/08/18 12:44, Jan Beulich wrote:
> 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@suse.com>
> ---
> 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;

Wouldn't setting the default value (DOMU) here be enough? Same for
xpti below?

> -
>      /* 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;

opt_pv_l1tf


Juergen

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

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

[Juergen Gross]

On 21/08/18 12:44, Jan Beulich wrote:
> 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).

Another possibility would be to have two local variables holding the
bits to clear and to set and to apply those after each sub-option
parsed.


Juergen

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