Re: [PATCH v3 3/4] x86/spec: fix INDIRECT_THUNK option to only be set when build-enabled

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.02.2024 12:07, Roger Pau Monne wrote:
> Attempt to provide a more helpful error message when the user attempts to set
> spec-ctrl=bti-thunk option but the support is build-time disabled.
> 
> While there also adjust the command line documentation to mention
> CONFIG_INDIRECT_THUNK instead of INDIRECT_THUNK.
> 
> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with one minor remark:

> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -241,7 +241,12 @@ static int __init cf_check parse_spec_ctrl(const char *s)
>          {
>              s += 10;
>  
> -            if ( !cmdline_strcmp(s, "retpoline") )
> +            if ( !IS_ENABLED(CONFIG_INDIRECT_THUNK) )
> +            {
> +                no_config_param("INDIRECT_THUNK", "spec-ctrl=bti-thunk", s, 
> ss);
> +                rc = -EINVAL;
> +            }
> +            else if ( !cmdline_strcmp(s, "retpoline") )
>                  opt_thunk = THUNK_RETPOLINE;
>              else if ( !cmdline_strcmp(s, "lfence") )
>                  opt_thunk = THUNK_LFENCE;

How about

            if ( !IS_ENABLED(CONFIG_INDIRECT_THUNK) )
            {
                no_config_param("INDIRECT_THUNK", "spec-ctrl", s - 10, ss);
                rc = -EINVAL;
            }
            else if ( !cmdline_strcmp(s, "retpoline") )

or (likely less liked by you and Andrew) "s += 10;" dropped and then

            if ( !IS_ENABLED(CONFIG_INDIRECT_THUNK) )
            {
                no_config_param("INDIRECT_THUNK", "spec-ctrl", s, ss);
                rc = -EINVAL;
            }
            else if ( !cmdline_strcmp(s += 10, "retpoline") )

conserving a little on string literal space (sadly, despite the function
being __init, string literals remain post-init due to living in .rodata)?

Jan