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