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
