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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Feb 26, 2024 at 01:50:46PM +0100, Jan Beulich wrote:
> 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)?

TBH I'm not overly worried about those 10 extra characters, but if I
had to choose I prefer the first option (so `s - 10`).

Thanks, Roger.