Messages in this thread | | | From | Arvind Sankar <> | Date | Sun, 20 Sep 2020 12:16:28 -0400 | Subject | Re: [RFC PATCH] Use feature bit names in clearcpuid= |
| |
On Sun, Sep 20, 2020 at 05:42:28PM +0200, Borislav Petkov wrote: > Hi, > > so tglx hates this clearcpuid= interface where you have to give the > X86_FEATURE array indices in order to disable a feature bit for testing. > Below is a first attempt (lightly tested in a VM only) to accept the bit > names from /proc/cpuinfo too. > > I say "too" because not all feature bits have names and we would still > have to support the numbers. Yeah, yuck. > > An exemplary cmdline would then be something like: > > clearcpuid=de,440,smca,succory,bmi1,3dnow ("succory" is wrong on > purpose). > > and it says: > > [ 0.000000] Clearing CPUID bits: de 13:24 smca bmi1 3dnow > > Also, I'm thinking we should taint the kernel when this option is used. > > Thoughts?
I like it. Allowing 13:24 as input would be icing on the cake :)
Small comments below.
> @@ -273,21 +273,45 @@ static void __init fpu__init_parse_early_param(void) > return; > > pr_info("Clearing CPUID bits:"); > - do { > - res = get_option(&argptr, &bit); > - if (res == 0 || res == 3) > - break; > - > - /* If the argument was too long, the last bit may be cut off */ > - if (res == 1 && arglen >= sizeof(arg)) > - break; > - > - if (bit >= 0 && bit < NCAPINTS * 32) { > - pr_cont(" " X86_CAP_FMT, x86_cap_flag(bit)); > - setup_clear_cpu_cap(bit); > + > + while (argptr) { > + int i; > + > + opt = (strsep(&argptr, ",")); > + if (!opt) > + continue;
The !opt check is unnecessary: strsep() returns NULL iff argptr is NULL on entry. The parentheses around strsep() also look odd.
> + > + if (!kstrtoint(opt, 10, &bit)) {
Could make bit unsigned and use kstrtouint().
> + if (bit >= 0 && bit < NCAPINTS * 32) { > + if (!x86_cap_flag(bit)) > + pr_cont(" " X86_CAP_FMT_BARE, x86_cap_flag_bare(bit)); > + else > + pr_cont(" " X86_CAP_FMT, x86_cap_flag(bit)); > + > + setup_clear_cpu_cap(bit); > + taint++; > + continue; > + }
Could always continue if it was a number, even if it was invalid, since that shouldn't match a name in any case?
> } > - } while (res == 2); > + > +#ifdef CONFIG_X86_FEATURE_NAMES > + for (i = 0; i < 32 * NCAPINTS; i++) { > + if (!x86_cap_flags[i]) > + continue; > + > + if (strcmp(x86_cap_flags[i], opt)) > + continue; > + > + pr_cont(" %s", opt); > + setup_clear_cpu_cap(i); > + taint++;
We could break out of the loop here -- we can't have multiple bits with the same name, right?
> + } > +#endif > + } > pr_cont("\n"); > + > + if (taint) > + add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK); > } > > /* > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
| |