Re: [PATCH] x86/cet: Support cet=<bool> on the command line

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.04.2022 10:52, Andrew Cooper wrote:
> @@ -283,6 +283,8 @@ CET is incompatible with 32bit PV guests.  If any CET 
> sub-options are active,
>  they will override the `pv=32` boolean to `false`.  Backwards compatibility
>  can be maintained with the pv-shim mechanism.
>  
> +*   An unqualified boolean is shorthand for setting all suboptions at once.

You're the native speaker, but I wonder whether there an "a" missing
before "shorthand".

> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -117,7 +117,20 @@ static int __init cf_check parse_cet(const char *s)
>          if ( !ss )
>              ss = strchr(s, '\0');
>  
> -        if ( (val = parse_boolean("shstk", s, ss)) >= 0 )
> +        if ( (val = parse_bool(s, ss)) >= 0 )
> +        {
> +#ifdef CONFIG_XEN_SHSTK
> +            opt_xen_shstk = val;
> +#else
> +            no_config_param("XEN_SHSTK", "cet", s, ss);
> +#endif
> +#ifdef CONFIG_XEN_IBT
> +            opt_xen_ibt = val;
> +#else
> +            no_config_param("XEN_IBT", "cet", s, ss);
> +#endif

There shouldn't be two invocations of no_config_param() here; imo if
either CONFIG_* is defined, use of the option shouldn't produce any
warning at all.

> +        }
> +        else if ( (val = parse_boolean("shstk", s, ss)) >= 0 )
>          {
>  #ifdef CONFIG_XEN_SHSTK
>              opt_xen_shstk = val;

Having seen Roger's reply, I'd like to make explicit that I don't
mind us allowing strange option combinations to be used, so long as
what we do matches the sequence in which they were provided.

Jan