On Fri, Oct 01, 2021 at 02:33:49PM +0100, Jane Malalane wrote:
Subject: Re: [PATCH] x86/cpu: Fix migration safety with X86_BUG_NULL_SEL ... Currently, Linux probes for X86_BUG_NULL_SEL unconditionally which makes it unsafe to migrate in a virtualised environment as the properties across the migration pool might differ.
Sorry but you need to explain "migration safety" in greater detail - we're not all virtualizers.
Zen3 adds the NullSelectorClearsBase bit to indicate that loading a NULL segment selector zeroes the base and limit fields, as well as just attributes. Zen2 also has this behaviour but doesn't have the NSCB bit.
I'm guessing you can detect Zen2 too, without the CPUID bit?
When virtualised, NSCB might be cleared for migration safety, therefore we must not probe. Always honour the NSCB bit in this case,
Who is "we"?
as the hypervisor is expected to synthesize it in the Zen2 case.
Signed-off-by: Jane Malalane jane.malalane@citrix.com
CC: x86@kernel.org CC: Thomas Gleixner tglx@linutronix.de CC: Ingo Molnar mingo@redhat.com CC: Borislav Petkov bp@alien8.de CC: "H. Peter Anvin" hpa@zytor.com CC: Pu Wen puwen@hygon.cn CC: Paolo Bonzini pbonzini@redhat.com CC: Sean Christopherson seanjc@google.com CC: Peter Zijlstra peterz@infradead.org CC: Andrew Cooper andrew.cooper3@citrix.com CC: Yazen Ghannam Yazen.Ghannam@amd.com CC: Brijesh Singh brijesh.singh@amd.com CC: Huang Rui ray.huang@amd.com CC: Andy Lutomirski luto@kernel.org CC: Kim Phillips kim.phillips@amd.com CC: stable@vger.kernel.org
arch/x86/include/asm/cpufeatures.h | 2 +- arch/x86/kernel/cpu/amd.c | 23 +++++++++++++++++++++++ arch/x86/kernel/cpu/common.c | 6 ++---- arch/x86/kernel/cpu/cpu.h | 1 + arch/x86/kernel/cpu/hygon.c | 23 +++++++++++++++++++++++ 5 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index d0ce5cfd3ac1..f571e4f6fe83 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -96,7 +96,7 @@ #define X86_FEATURE_SYSCALL32 ( 3*32+14) /* "" syscall in IA32 userspace */ #define X86_FEATURE_SYSENTER32 ( 3*32+15) /* "" sysenter in IA32 userspace */ #define X86_FEATURE_REP_GOOD ( 3*32+16) /* REP microcode works well */ -/* FREE! ( 3*32+17) */ +#define X86_FEATURE_NSCB ( 3*32+17) /* Null Selector Clears Base */
You don't really need that bit definition - you check CPUID in the early_init_amd/hygon() functions and that is enough. IOW, based on that CPUID bit, you can call detect_null_seg_behavior() and set (or not) X86_BUG_NULL_SEG.
...
diff --git a/arch/x86/kernel/cpu/hygon.c b/arch/x86/kernel/cpu/hygon.c index 6d50136f7ab9..765f1556d964 100644 --- a/arch/x86/kernel/cpu/hygon.c +++ b/arch/x86/kernel/cpu/hygon.c @@ -264,6 +264,29 @@ static void early_init_hygon(struct cpuinfo_x86 *c) if (c->x86_power & BIT(14)) set_cpu_cap(c, X86_FEATURE_RAPL);
- /*
* Zen1 and earlier CPUs don't clear segment base/limits when* loading a NULL selector. This has been designated* X86_BUG_NULL_SEG.** Zen3 CPUs advertise Null Selector Clears Base in CPUID.* Zen2 CPUs also have this behaviour, but no CPUID bit.** A hypervisor may sythesize the bit, but may also hide it* for migration safety, so we must not probe for model* specific behaviour when virtualised.*/- if (c->extended_cpuid_level >= 0x80000021 &&
cpuid_eax(0x80000021) & BIT(6))set_cpu_cap(c, X86_FEATURE_NSCB);- if (!cpu_has(c, X86_FEATURE_HYPERVISOR) && !cpu_has(c, X86_FEATURE_NSCB) &&
c->x86 == 0x18)detect_null_seg_behavior(c);- if (!cpu_has(c, X86_FEATURE_NSCB))
set_cpu_bug(c, X86_BUG_NULL_SEG);
Please integrate scripts/checkpatch.pl into your patch creation workflow. Some of the warnings/errors *actually* make sense.
I'll show you that there's whitespace damage here:
ERROR: code indent should use tabs where possible #238: FILE: arch/x86/kernel/cpu/hygon.c:281: +^I set_cpu_cap(c, X86_FEATURE_NSCB);$
ERROR: code indent should use tabs where possible #242: FILE: arch/x86/kernel/cpu/hygon.c:285: + detect_null_seg_behavior(c);$
WARNING: please, no spaces at the start of a line #242: FILE: arch/x86/kernel/cpu/hygon.c:285: + detect_null_seg_behavior(c);$