LASS is enabled via setting a CR4 bit if the platform
supports the feature.
LASS may be disabled in early boot time, for example,
by command line parameter clearcpuid=lass/390 or
vsyscall flag. In such cases, the CPU feature and CR4
bits will be cleared.
Signed-off-by: Yian Chen <[email protected]>
Reviewed-by: Tony Luck <[email protected]>
---
arch/x86/kernel/cpu/common.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 9cfca3d7d0e2..efc7c7623968 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -412,6 +412,23 @@ static __always_inline void setup_umip(struct cpuinfo_x86 *c)
cr4_clear_bits(X86_CR4_UMIP);
}
+static __always_inline void setup_lass(struct cpuinfo_x86 *c)
+{
+ if (cpu_feature_enabled(X86_FEATURE_LASS)) {
+ cr4_set_bits(X86_CR4_LASS);
+ } else {
+ /*
+ * only clear the feature and cr4 bits when hardware
+ * supports LASS, in case it was enabled in a previous
+ * boot (e.g., via kexec)
+ */
+ if (cpu_has(c, X86_FEATURE_LASS)) {
+ cr4_clear_bits(X86_CR4_LASS);
+ clear_cpu_cap(c, X86_FEATURE_LASS);
+ }
+ }
+}
+
/* These bits should not change their value after CPU init is finished. */
static const unsigned long cr4_pinned_mask =
X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_UMIP |
@@ -1848,6 +1865,7 @@ static void identify_cpu(struct cpuinfo_x86 *c)
setup_smep(c);
setup_smap(c);
setup_umip(c);
+ setup_lass(c);
/* Enable FSGSBASE instructions if available. */
if (cpu_has(c, X86_FEATURE_FSGSBASE)) {
--
2.34.1
> +static __always_inline void setup_lass(struct cpuinfo_x86 *c)
> +{
> + if (cpu_feature_enabled(X86_FEATURE_LASS)) {
> + cr4_set_bits(X86_CR4_LASS);
> + } else {
> + /*
> + * only clear the feature and cr4 bits when hardware
> + * supports LASS, in case it was enabled in a previous
> + * boot (e.g., via kexec)
> + */
> + if (cpu_has(c, X86_FEATURE_LASS)) {
> + cr4_clear_bits(X86_CR4_LASS);
> + clear_cpu_cap(c, X86_FEATURE_LASS);
> + }
> + }
> +}
I am quite confused by the "else" code flow. Can you please help
understand how this code path would be exercised?
Also, why don't other features such as SMAP or SMEP need this type of
handling? I see something on similar lines for UMIP.
Also, how does the CR4 pinning code in the following patch play into
this? Could it flag a warning when cr4_clear_bits() is called above?
> +
> /* These bits should not change their value after CPU init is finished. */
> static const unsigned long cr4_pinned_mask =
> X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_UMIP |
> @@ -1848,6 +1865,7 @@ static void identify_cpu(struct cpuinfo_x86 *c)
> setup_smep(c);
> setup_smap(c);
> setup_umip(c);
> + setup_lass(c);
>
On 1/11/2023 2:22 PM, Sohil Mehta wrote:
>> +static __always_inline void setup_lass(struct cpuinfo_x86 *c)
>> +{
>> + if (cpu_feature_enabled(X86_FEATURE_LASS)) {
>> + cr4_set_bits(X86_CR4_LASS);
>> + } else {
>> + /*
>> + * only clear the feature and cr4 bits when hardware
>> + * supports LASS, in case it was enabled in a previous
>> + * boot (e.g., via kexec)
>> + */
>> + if (cpu_has(c, X86_FEATURE_LASS)) {
>> + cr4_clear_bits(X86_CR4_LASS);
>> + clear_cpu_cap(c, X86_FEATURE_LASS);
>> + }
>> + }
>> +}
>
> I am quite confused by the "else" code flow. Can you please help
> understand how this code path would be exercised?
>
The "else" branch is to disable LASS for every cpu. Addition to clear
the CR4 bit, it would be better to clear cpu feature id for consistent
result in every CPU as well, otherwise, we could see the feature id is
cleared in the boot-cpu only, for example, if user enables vsyscall.
> Also, why don't other features such as SMAP or SMEP need this type of
> handling?
It seems there is no need to have such if-else for SMAP/SMEP since the
X86_SMAP/SMEP was gone from arch/x86/Kconfig.
I see something on similar lines for UMIP.
>
> Also, how does the CR4 pinning code in the following patch play into
> this?
I did not test if pinning code works as commented: "/* These bits
should not change their value after CPU init is finished. */"
Could it flag a warning when cr4_clear_bits() is called above?
I did not observed a warning for cr4_clear_bits(), this should be okay
since it is called during init.
>
>> +
>> /* These bits should not change their value after CPU init is
>> finished. */
>> static const unsigned long cr4_pinned_mask =
>> X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_UMIP |
>> @@ -1848,6 +1865,7 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>> setup_smep(c);
>> setup_smap(c);
>> setup_umip(c);
>> + setup_lass(c);
On 1/9/23 21:52, Yian Chen wrote:
> +static __always_inline void setup_lass(struct cpuinfo_x86 *c)
> +{
> + if (cpu_feature_enabled(X86_FEATURE_LASS)) {
> + cr4_set_bits(X86_CR4_LASS);
> + } else {
> + /*
> + * only clear the feature and cr4 bits when hardware
> + * supports LASS, in case it was enabled in a previous
> + * boot (e.g., via kexec)
> + */
> + if (cpu_has(c, X86_FEATURE_LASS)) {
> + cr4_clear_bits(X86_CR4_LASS);
> + clear_cpu_cap(c, X86_FEATURE_LASS);
> + }
> + }
> +}
Could you try testing this, please?
Please remember that there are three things in play here:
1. disabled-features.h. Makes cpu_feature_enabled(X86_FEATURE_LASS)==0
when CONFIG_X86_LASS=n.
2. The X86_FEATURE_LASS software feature bit itself. clearcpuid=lass
would clear it.
3. The actual CPU enumeration of X86_FEATURE_LASS
The else{} is handling the case where X86_FEATURE_LASS is compiled out
but where the CPU supports LASS. It doesn't do anything when the CPU
lacks LASS support *OR* when clearcpuid=lass is used.
In the end, want X86_CR4_LASS set when the kernel wants LASS and clear
in *ALL* other cases. That would be simply:
if (cpu_feature_enabled(X86_FEATURE_LASS)) {
cr4_set_bits(X86_CR4_LASS);
} else {
cr4_clear_bits(X86_CR4_LASS);
}
The cr4_clear_bits(X86_CR4_LASS) should be safe regardless of CPU or
kernel support for LASS.
As for the:
clear_cpu_cap(c, X86_FEATURE_LASS);
It really only matters for kernels where CONFIG_X86_LASS=n but where the
CPU supports it. I'm not clear on what specifically you expect to gain
from it, though.
I'm also wondering if we even want a Kconfig option. Is anyone
realistically going to be compiling this support out?
On 1/12/2023 10:17 AM, Dave Hansen wrote:
> In the end, want X86_CR4_LASS set when the kernel wants LASS and clear
> in *ALL* other cases. That would be simply:
>
> if (cpu_feature_enabled(X86_FEATURE_LASS)) {
> cr4_set_bits(X86_CR4_LASS);
> } else {
> cr4_clear_bits(X86_CR4_LASS);
> }
>
Thanks for the explanation. This is very helpful.
> I'm also wondering if we even want a Kconfig option. Is anyone
> realistically going to be compiling this support out?
I was initially thinking we should leave the Kconfig option for users
that really *need* vsyscall support. But, thinking about it more, we
don't need an LASS Kconfig option for that.
We can make CONFIG_LEGACY_VSYSCALL_NONE as the default instead of the
current CONFIG_LEGACY_VSYSCALL_XONLY. The kernel can disable LASS at
boot time if a user/admin has shown explicit preference by selecting
CONFIG_LEGACY_VSYSCALL_XONLY or by providing command line parameter
vsyscall=xonly/emulate.
Thoughts?
Adding folks from the previous discussion when a similar suggestion was
made by Kees. There didn't seem to be any major objection that time.
https://lore.kernel.org/lkml/202205111424.DEB0E3418@keescook/
On 1/12/2023 5:17 PM, Sohil Mehta wrote:
> We can make CONFIG_LEGACY_VSYSCALL_NONE as the default instead of the
> current CONFIG_LEGACY_VSYSCALL_XONLY. The kernel can disable LASS at
> boot time if a user/admin has shown explicit preference by selecting
> CONFIG_LEGACY_VSYSCALL_XONLY or by providing command line parameter
> vsyscall=xonly/emulate.
>
> Thoughts?