Re: [PATCH 02/21] x86/mm/asi: add X86_FEATURE_ASI and asi=

[Brendan Jackman <jackmanb@google.com>]

On Sat Oct 25, 2025 at 10:06 AM UTC, Borislav Petkov wrote:
> On Wed, Sep 24, 2025 at 02:59:37PM +0000, Brendan Jackman wrote:
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 6c42061ca20e581b5192b66c6f25aba38d4f8ff8..9b8330fc1fe31721af39b08b58b729ced78ba803 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -5324,6 +5324,14 @@
>>  
>>  			Not specifying this option is equivalent to pti=auto.
>>  
>> +	asi=		[X86-64] Control Address Space Isolation (ASI), a
>> +			technology for mitigating CPU vulnerabilities.
>
>> 			ASI is
>> +			not yet ready to provide security guarantees but can be
>> +			enabled for evaluation.
>
> Yeah, no need for such "temporary" statements in the help text since you're
> going to have to touch it again once it becomes a full-fledged feature.

Sure. Per Dave's feedback it is anyway gonna have to be merged in a
state where it already does something useful, so I guess this statement
will need to be updated regardless.

>> +			on   - unconditionally enable
>> +			off  - unconditionally disable
>
> "unconditionally" as opposed to some other setting which is conditional?

My assumption here is that eventually we'll end up with an "auto"
setting or something, where the big startup condiguration dance in
bugs.c decides whether to enable ASI based on your CPU/attack vector
controls/individual mitigation configs.

>> +
>>  	pty.legacy_count=
>>  			[KNL] Number of legacy pty's. Overwrites compiled-in
>>  			default number.
>> diff --git a/arch/x86/include/asm/asi.h b/arch/x86/include/asm/asi.h
>> index 53acdf22fe33efc6ccedbae52b262a904868459a..32a4c04c4be0f6f425c7cbcff4c58f1827a4b4c4 100644
>> --- a/arch/x86/include/asm/asi.h
>> +++ b/arch/x86/include/asm/asi.h
>> @@ -2,4 +2,14 @@
>>  #ifndef _ASM_X86_ASI_H
>>  #define _ASM_X86_ASI_H
>>  
>> +#include <asm/cpufeature.h>
>> +
>> +void asi_check_boottime_disable(void);
>> +
>> +/* Helper for generic code. Arch code just uses cpu_feature_enabled(). */
>> +static inline bool asi_enabled_static(void)
>
> "static" because? There will be a dynamic one too?

Actually... no, probably not - should drop this from the name, thanks.

(At some point I think I was worried about spamming the kernel image
with too many static branches, but this seems like a bridge to cross if
we ever actually come to it).

>> +{
>> +	return cpu_feature_enabled(X86_FEATURE_ASI);
>> +}
>> +
>>  #endif /* _ASM_X86_ASI_H */
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index 4091a776e37aaed67ca93b0a0cd23cc25dbc33d4..3eee24a4cabf3b2131c34596236d8bc8eec05b3b 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -499,6 +499,7 @@
>>  #define X86_FEATURE_IBPB_EXIT_TO_USER	(21*32+14) /* Use IBPB on exit-to-userspace, see VMSCAPE bug */
>>  #define X86_FEATURE_ABMC		(21*32+15) /* Assignable Bandwidth Monitoring Counters */
>>  #define X86_FEATURE_MSR_IMM		(21*32+16) /* MSR immediate form instructions */
>> +#define X86_FEATURE_ASI			(21*32+17) /* Kernel Address Space Isolation */
>
> I think we really will need to show this in /proc/cpuinfo as it is a real, big
> feature which gets proper kernel glue vs some silly CPUID bit.
>
> IOW,
>
> #define X86_FEATURE_ASI			(21*32+17) /* "asi" Kernel Address Space Isolation */
> 						      ^^^^
>
> Not sure, though, when we should make it an ABI - perhaps once the whole pile
> has landed...

Hm yeah, I actually also thought I had some direct feedback from one of
the x86 maintainers saying not to expose it here. I can no longer find
that feedback on Lore so I think I must be misremembering, the flag
was already hidden back in [0].

[0] https://lore.kernel.org/linux-mm/20240712-asi-rfc-24-v1-5-144b319a40d8@google.com/

If that feedback indeed doesn't exist then personally I'd lean towards
exposing it right away, I don't see that much downside in terms of ABI,
since ASI kinda "doesn't do anything", from a SW point of view it's just
a very weird and complicated NOP. It's hard for me to see how userspace
could grow a functional dependency on this flag. Whereas for general
monitoring it's handy.

Still, there's definitely no hill I'd die on here.

>>  /*
>>   * BUG word(s)
>> diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile
>> index 5b9908f13dcfd092897f3778ee56ea4d45bdb868..5ecbff70964f61a903ac96cec3736a7cec1221fd 100644
>> --- a/arch/x86/mm/Makefile
>> +++ b/arch/x86/mm/Makefile
>> @@ -52,6 +52,7 @@ obj-$(CONFIG_ACPI_NUMA)		+= srat.o
>>  obj-$(CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS)	+= pkeys.o
>>  obj-$(CONFIG_RANDOMIZE_MEMORY)			+= kaslr.o
>>  obj-$(CONFIG_MITIGATION_PAGE_TABLE_ISOLATION)	+= pti.o
>> +obj-$(CONFIG_MITIGATION_ADDRESS_SPACE_ISOLATION)	+= asi.o
>>  
>>  obj-$(CONFIG_X86_MEM_ENCRYPT)	+= mem_encrypt.o
>>  obj-$(CONFIG_AMD_MEM_ENCRYPT)	+= mem_encrypt_amd.o
>> diff --git a/arch/x86/mm/asi.c b/arch/x86/mm/asi.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..8c907f3c84f43f66e412ecbfa99e67390d31a66f
>> --- /dev/null
>> +++ b/arch/x86/mm/asi.c
>> @@ -0,0 +1,28 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <linux/asi.h>
>> +#include <linux/init.h>
>> +#include <linux/string.h>
>> +
>> +#include <asm/cmdline.h>
>> +#include <asm/cpufeature.h>
>> +
>> +void __init asi_check_boottime_disable(void)
>> +{
>> +	bool enabled = false;
>> +	char arg[4];
>> +	int ret;
>> +
>> +	ret = cmdline_find_option(boot_command_line, "asi", arg, sizeof(arg));
>> +	if (ret == 3 && !strncmp(arg, "off", 3)) {
>> +		enabled = false;
>> +		pr_info("ASI explicitly disabled by kernel cmdline.\n");
>> +	} else if (ret == 2 && !strncmp(arg, "on", 2)) {
>> +		enabled = true;
>> +		pr_info("ASI enabled.\n");
>
> I'm not sure about those pr_info()s. When it is disabled, you can clear
> X86_FEATURE_ASI so you won't see it in /proc/cpuinfo and then it is disabled.
> And the same when it is enabled.

Agreed, if we expose the CPU flag we don't need the printks.

>> +	} else if (ret) {
>> +		pr_err("Unknown asi= flag '%s', try 'off' or 'on'\n", arg);
>> +	}
>> +
>> +	if (enabled)
>> +		setup_force_cpu_cap(X86_FEATURE_ASI);
>> +}
>
> Not an early_param() ?

Oh this is just for consistency with pti_check_boottime_disable(). But,
I think that function actually exists because of init ordering issues
that aren't relevant here, so early_param() seems fine to me (or, if I
find some reason why it doesn't, work, I'll add a comment in v2 to
explain why we don't use it).

Thanks for taking a look :)