Re: [Xen-devel] [PATCH v2] x86/cpuid: Extend the cpuid= option to support all named features

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.09.2019 21:49, Andrew Cooper wrote:
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -21,45 +21,62 @@ static const uint32_t deep_features[] = 
> INIT_DEEP_FEATURES;
>  
>  static int __init parse_xen_cpuid(const char *s)
>  {
> +    static const struct feature {
> +        const char *name;
> +        unsigned int bit;
> +    } features[] __initconst = INIT_FEATURE_NAMES, *lhs, *mid, *rhs;

The pointer field want this to use __initconstrel. And I don't think
you mean lhs, mid, and rhs to also be static? Albeit ...

>      const char *ss;
>      int val, rc = 0;
>  
>      do {
> +        const char *feat;
> +
>          ss = strchr(s, ',');
>          if ( !ss )
>              ss = strchr(s, '\0');
>  
> -        if ( (val = parse_boolean("md-clear", s, ss)) >= 0 )
> -        {
> -            if ( !val )
> -                setup_clear_cpu_cap(X86_FEATURE_MD_CLEAR);
> -        }
> -        else if ( (val = parse_boolean("ibpb", s, ss)) >= 0 )
> -        {
> -            if ( !val )
> -                setup_clear_cpu_cap(X86_FEATURE_IBPB);
> -        }
> -        else if ( (val = parse_boolean("ibrsb", s, ss)) >= 0 )
> -        {
> -            if ( !val )
> -                setup_clear_cpu_cap(X86_FEATURE_IBRSB);
> -        }
> -        else if ( (val = parse_boolean("stibp", s, ss)) >= 0 )
> -        {
> -            if ( !val )
> -                setup_clear_cpu_cap(X86_FEATURE_STIBP);
> -        }
> -        else if ( (val = parse_boolean("l1d-flush", s, ss)) >= 0 )
> -        {
> -            if ( !val )
> -                setup_clear_cpu_cap(X86_FEATURE_L1D_FLUSH);
> -        }
> -        else if ( (val = parse_boolean("ssbd", s, ss)) >= 0 )
> +        /* Skip the 'no-' prefix for name comparisons. */
> +        feat = s;
> +        if ( strncmp(s, "no-", 3) == 0 )
> +            feat += 3;
> +
> +        /* (Re)initalise lhs and rhs for binary search. */
> +        lhs = features;
> +        rhs = features + ARRAY_SIZE(features);

... the comment here suggests you do, but I don't currently see why.
I'd like to understand this though before giving an ack.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel