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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.04.2022 12:13, Andrew Cooper wrote:
> On 28/04/2022 11:13, Jan Beulich wrote:
>>> --- 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.
> 
> It's this, or:
> 
>         if ( (val = parse_bool(s, ss)) >= 0 )
>         {
> #if !defined(CONFIG_XEN_SHSTK) && !defined(CONFIG_XEN_IBT)
>             no_config_param("XEN_{SHSTK,IBT}", "cet", s, ss);
> #endif
> #ifdef CONFIG_XEN_SHSTK
>             opt_xen_shstk = val;
> #endif
> #ifdef CONFIG_XEN_IBT
>             opt_xen_ibt = val;
> #endif
>         }
> 
> I'm not terribly fussed.

I'd prefer the alternative variant; hopefully Roger doesn't strongly
prefer the other one. And then
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan