[PATCH] x86/amd: Introduce and use X86_BUG_NULL_SEG

Andrew Cooper posted 1 patch 2 years, 7 months ago
Failed in applying to current master (apply log)
xen/arch/x86/cpu/amd.c            | 7 +++++++
xen/arch/x86/cpu/hygon.c          | 7 +++++++
xen/arch/x86/pv/misc-hypercalls.c | 3 +--
xen/include/asm-x86/cpufeature.h  | 1 +
xen/include/asm-x86/cpufeatures.h | 1 +
5 files changed, 17 insertions(+), 2 deletions(-)
[PATCH] x86/amd: Introduce and use X86_BUG_NULL_SEG
Posted by Andrew Cooper 2 years, 7 months ago
AMD/Hygon processors before the Zen2 microarchitecture don't clear the base or
limit fields when loading a NULL segment.

Express the logic in terms of cpu_bug_null_seg, and adjust the workaround in
do_set_segment_base().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Pu Wen <puwen@hygon.cn>

Since originally posted, most logic dependent on this workaround has been
reworked, but one still remains.  This form of the workaround is more flexible
if other impacted CPUs are discovered.
---
 xen/arch/x86/cpu/amd.c            | 7 +++++++
 xen/arch/x86/cpu/hygon.c          | 7 +++++++
 xen/arch/x86/pv/misc-hypercalls.c | 3 +--
 xen/include/asm-x86/cpufeature.h  | 1 +
 xen/include/asm-x86/cpufeatures.h | 1 +
 5 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index c4d84373a710..379ea1638ca6 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -801,6 +801,13 @@ static void init_amd(struct cpuinfo_x86 *c)
 	    c->x86 == 0x17)
 		detect_zen2_null_seg_behaviour();
 
+	/*
+	 * AMD CPUs before Zen2 don't clear segment bases/limits when loading
+	 * a NULL selector.
+	 */
+        if (c == &boot_cpu_data && !cpu_has_nscb)
+		setup_force_cpu_cap(X86_BUG_NULL_SEG);
+
 	/* MFENCE stops RDTSC speculation */
 	if (!cpu_has_lfence_dispatch)
 		__set_bit(X86_FEATURE_MFENCE_RDTSC, c->x86_capability);
diff --git a/xen/arch/x86/cpu/hygon.c b/xen/arch/x86/cpu/hygon.c
index 429d6601fc13..a2963036b389 100644
--- a/xen/arch/x86/cpu/hygon.c
+++ b/xen/arch/x86/cpu/hygon.c
@@ -40,6 +40,13 @@ static void init_hygon(struct cpuinfo_x86 *c)
 	    c->x86 == 0x18)
 		detect_zen2_null_seg_behaviour();
 
+	/*
+	 * Hygon CPUs before Zen2 don't clear segment bases/limits when
+	 * loading a NULL selector.
+	 */
+        if (c == &boot_cpu_data && !cpu_has_nscb)
+		setup_force_cpu_cap(X86_BUG_NULL_SEG);
+
 	/* MFENCE stops RDTSC speculation */
 	if (!cpu_has_lfence_dispatch)
 		__set_bit(X86_FEATURE_MFENCE_RDTSC, c->x86_capability);
diff --git a/xen/arch/x86/pv/misc-hypercalls.c b/xen/arch/x86/pv/misc-hypercalls.c
index 3a4e4aa4603e..5dade2472687 100644
--- a/xen/arch/x86/pv/misc-hypercalls.c
+++ b/xen/arch/x86/pv/misc-hypercalls.c
@@ -227,8 +227,7 @@ long do_set_segment_base(unsigned int which, unsigned long base)
         if ( sel > 3 )
             /* Fix up RPL for non-NUL selectors. */
             sel |= 3;
-        else if ( boot_cpu_data.x86_vendor &
-                  (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+        else if ( cpu_bug_null_seg )
             /* Work around NUL segment behaviour on AMD hardware. */
             asm volatile ( "mov %[sel], %%gs"
                            :: [sel] "r" (FLAT_USER_DS32) );
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index 94a485f99c22..802d9257b0bf 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -159,6 +159,7 @@
 
 /* Bugs. */
 #define cpu_bug_fpu_ptrs        boot_cpu_has(X86_BUG_FPU_PTRS)
+#define cpu_bug_null_seg        boot_cpu_has(X86_BUG_NULL_SEG)
 
 enum _cache_type {
     CACHE_TYPE_NULL = 0,
diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
index 6c8f432aee4f..72beb7babcce 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -45,6 +45,7 @@ XEN_CPUFEATURE(XEN_SHSTK,         X86_SYNTH(26)) /* Xen uses CET Shadow Stacks *
 #define X86_BUG(x) ((FSCAPINTS + X86_NR_SYNTH) * 32 + (x))
 
 #define X86_BUG_FPU_PTRS          X86_BUG( 0) /* (F)X{SAVE,RSTOR} doesn't save/restore FOP/FIP/FDP. */
+#define X86_BUG_NULL_SEG          X86_BUG( 1) /* NULL-ing a selector preserves the base and limit. */
 
 /* Total number of capability words, inc synth and bug words. */
 #define NCAPINTS (FSCAPINTS + X86_NR_SYNTH + X86_NR_BUG) /* N 32-bit words worth of info */
-- 
2.11.0


Re: [PATCH] x86/amd: Introduce and use X86_BUG_NULL_SEG
Posted by Jan Beulich 2 years, 7 months ago
On 08.09.2021 15:59, Andrew Cooper wrote:
> AMD/Hygon processors before the Zen2 microarchitecture don't clear the base or
> limit fields when loading a NULL segment.
> 
> Express the logic in terms of cpu_bug_null_seg, and adjust the workaround in
> do_set_segment_base().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
albeit with a question (perhaps to Pu):

> --- a/xen/arch/x86/cpu/hygon.c
> +++ b/xen/arch/x86/cpu/hygon.c
> @@ -40,6 +40,13 @@ static void init_hygon(struct cpuinfo_x86 *c)
>  	    c->x86 == 0x18)
>  		detect_zen2_null_seg_behaviour();
>  
> +	/*
> +	 * Hygon CPUs before Zen2 don't clear segment bases/limits when
> +	 * loading a NULL selector.
> +	 */
> +        if (c == &boot_cpu_data && !cpu_has_nscb)
> +		setup_force_cpu_cap(X86_BUG_NULL_SEG);

Are there any affected CPUs in reality?

And actually (only noticing when seeing the malformed context
above) there look to be indentation issues in the changes to
both this file and amd.c.

Jan


Re: [PATCH] x86/amd: Introduce and use X86_BUG_NULL_SEG
Posted by Andrew Cooper 2 years, 7 months ago
On 08/09/2021 16:06, Jan Beulich wrote:
> On 08.09.2021 15:59, Andrew Cooper wrote:
>> AMD/Hygon processors before the Zen2 microarchitecture don't clear the base or
>> limit fields when loading a NULL segment.
>>
>> Express the logic in terms of cpu_bug_null_seg, and adjust the workaround in
>> do_set_segment_base().
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

> albeit with a question (perhaps to Pu):
>
>> --- a/xen/arch/x86/cpu/hygon.c
>> +++ b/xen/arch/x86/cpu/hygon.c
>> @@ -40,6 +40,13 @@ static void init_hygon(struct cpuinfo_x86 *c)
>>  	    c->x86 == 0x18)
>>  		detect_zen2_null_seg_behaviour();
>>  
>> +	/*
>> +	 * Hygon CPUs before Zen2 don't clear segment bases/limits when
>> +	 * loading a NULL selector.
>> +	 */
>> +        if (c == &boot_cpu_data && !cpu_has_nscb)
>> +		setup_force_cpu_cap(X86_BUG_NULL_SEG);
> Are there any affected CPUs in reality?

Yes - the Hygon Zen1 uarch CPUs are affected.

>
> And actually (only noticing when seeing the malformed context
> above) there look to be indentation issues in the changes to
> both this file and amd.c.

Ah yes - I'll fix on commit.

~Andrew