2020-04-22 12:06:25

by John Haxby

[permalink] [raw]
Subject: [PATCH 1/1] x86/fpu: Allow clearcpuid= to clear several bits

Prior to 0c2a3913d6f5, clearcpuid= could be specified several times on
the command line to clear several bits. The old multiple option is a
little anachronistic so change clearcpuid to accept a comma-separated
list of numbers. Up to about eight bits can be cleared.

Fixes: 0c2a3913d6f5 ("x86/fpu: Parse clearcpuid= as early XSAVE argument")
Signed-off-by: John Haxby <[email protected]>
Cc: [email protected]
---
.../admin-guide/kernel-parameters.txt | 24 ++++++++++---------
arch/x86/kernel/fpu/init.c | 18 ++++++++------
2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f2a93c8679e8..f380781be9e0 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -577,18 +577,20 @@
loops can be debugged more effectively on production
systems.

- clearcpuid=BITNUM [X86]
- Disable CPUID feature X for the kernel. See
+ clearcpuid=BITNUM[,BITNUM,...] [X86]
+ Disable CPUID features for the kernel. See
arch/x86/include/asm/cpufeatures.h for the valid bit
- numbers. Note the Linux specific bits are not necessarily
- stable over kernel options, but the vendor specific
- ones should be.
- Also note that user programs calling CPUID directly
- or using the feature without checking anything
- will still see it. This just prevents it from
- being used by the kernel or shown in /proc/cpuinfo.
- Also note the kernel might malfunction if you disable
- some critical bits.
+ numbers. Up to about eight bits can be cleared. Note the
+ Linux specific bits are not necessarily stable over
+ kernel options, but the vendor specific ones should be.
+ Also note that user programs calling CPUID directly or
+ using the feature without checking anything will still
+ see it. This just prevents it from being used by the
+ kernel or shown in /proc/cpuinfo. Also note the kernel
+ might malfunction if you disable some critical bits.
+ Consider using a virtual machine emulating an older CPU
+ type for clearing many bits or for making the cleared
+ bits visible to user programs.

cma=nn[MG]@[start[MG][-end[MG]]]
[ARM,X86,KNL]
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 6ce7e0a23268..8d826505c22e 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -243,8 +243,6 @@ static void __init fpu__init_system_ctx_switch(void)
static void __init fpu__init_parse_early_param(void)
{
char arg[32];
- char *argptr = arg;
- int bit;

#ifdef CONFIG_X86_32
if (cmdline_find_option_bool(boot_command_line, "no387"))
@@ -268,11 +266,17 @@ static void __init fpu__init_parse_early_param(void)
setup_clear_cpu_cap(X86_FEATURE_XSAVES);

if (cmdline_find_option(boot_command_line, "clearcpuid", arg,
- sizeof(arg)) &&
- get_option(&argptr, &bit) &&
- bit >= 0 &&
- bit < NCAPINTS * 32)
- setup_clear_cpu_cap(bit);
+ sizeof(arg))) {
+ /* cpuid bit numbers are mostly three digits */
+ enum { nints = sizeof(arg)/(3+1) + 1 };
+ int i, bits[nints];
+
+ get_options(arg, nints, bits);
+ for (i = 1; i <= bits[0]; i++) {
+ if (bits[i] >= 0 && bits[i] < NCAPINTS * 32)
+ setup_clear_cpu_cap(bits[i]);
+ }
+ }
}

/*
--
2.25.3


2020-04-22 14:41:57

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH 1/1] x86/fpu: Allow clearcpuid= to clear several bits


Thanks good catch.

> if (cmdline_find_option(boot_command_line, "clearcpuid", arg,
> - sizeof(arg)) &&
> - get_option(&argptr, &bit) &&
> - bit >= 0 &&
> - bit < NCAPINTS * 32)
> - setup_clear_cpu_cap(bit);
> + sizeof(arg))) {
> + /* cpuid bit numbers are mostly three digits */
> + enum { nints = sizeof(arg)/(3+1) + 1 };

Not sure what the digits have to do with the stack space of an int array.

We should have enough stack to afford some more than 8.

Would be good to have a warning if the arguments are longer.

Maybe it would be simpler to fix the early arg parser
to allow multiple instances again? That would also avoid the limit,
and keep everything compatible.

-Andi


> + int i, bits[nints];
> +
> + get_options(arg, nints, bits);
> + for (i = 1; i <= bits[0]; i++) {
> + if (bits[i] >= 0 && bits[i] < NCAPINTS * 32)
> + setup_clear_cpu_cap(bits[i]);
> + }
> + }
> }
>
> /*
> --
> 2.25.3
>

2020-04-22 15:24:23

by John Haxby

[permalink] [raw]
Subject: Re: [PATCH 1/1] x86/fpu: Allow clearcpuid= to clear several bits



> On 22 Apr 2020, at 15:35, Andi Kleen <[email protected]> wrote:
>
>
> Thanks good catch.
>
>> if (cmdline_find_option(boot_command_line, "clearcpuid", arg,
>> - sizeof(arg)) &&
>> - get_option(&argptr, &bit) &&
>> - bit >= 0 &&
>> - bit < NCAPINTS * 32)
>> - setup_clear_cpu_cap(bit);
>> + sizeof(arg))) {
>> + /* cpuid bit numbers are mostly three digits */
>> + enum { nints = sizeof(arg)/(3+1) + 1 };
>
> Not sure what the digits have to do with the stack space of an int array.
>
> We should have enough stack to afford some more than 8.

sizeof(arg) == 32; room enough for eight three-digit with their trailing commas. If sizeof(arg) == 1024 instead then there'd be more than enough room to remove every single feature. TBH, 512 is more than enough for the 89 flags I have listed on this machine I'm looking at here. I'll grow sizeof(arg) and nints accordingly.

>
> Would be good to have a warning if the arguments are longer.
>

Yes, I should definitely do that -- coming to a V2 soon.


> Maybe it would be simpler to fix the early arg parser
> to allow multiple instances again? That would also avoid the limit,
> and keep everything compatible.
>

I did wonder about that. However, cmdline_find_option() is specifically documented as

* Find a non-boolean option (i.e. option=argument). In accordance with
* standard Linux practice, if this option is repeated, this returns the
* last instance on the command line.

And since that appeared in 2017 I decided to stick with the new-fangled interface :) This is a little-used feature; I'm not sure it's worth the effort of parsing the command line for the old style. What do you think?

jch


> -Andi
>
>
>> + int i, bits[nints];
>> +
>> + get_options(arg, nints, bits);
>> + for (i = 1; i <= bits[0]; i++) {
>> + if (bits[i] >= 0 && bits[i] < NCAPINTS * 32)
>> + setup_clear_cpu_cap(bits[i]);
>> + }
>> + }
>> }
>>
>> /*
>> --
>> 2.25.3
>>

2020-04-23 01:43:31

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH 1/1] x86/fpu: Allow clearcpuid= to clear several bits

>
> I did wonder about that. However, cmdline_find_option() is specifically documented as
>
> * Find a non-boolean option (i.e. option=argument). In accordance with
> * standard Linux practice, if this option is repeated, this returns the
> * last instance on the command line.

Okay so would need a special version that uses the first and an option
to pass the cmdline string.

>
> And since that appeared in 2017 I decided to stick with the new-fangled interface :) This is a little-used feature; I'm not sure it's worth the effort of parsing the command line for the old style. What do you think?

I'm not sure it's that little used. We use it quite a bit for testing
and workarounds, and it might be that some of those are deployed.

-Andi