Re: [PATCH] x86/cpu/centaur: Disable X86_FEATURE_FSGSBASE on Zhaoxin C4600
From: Tony W Wang-oc
Date: Tue Mar 17 2026 - 23:44:47 EST
On 2026/3/17 23:21, Dave Hansen wrote:
Sorry, The previous patch didn't consider the generality of the newly added zhaoxin.h. The intention was to minimize modifications to common.c.--- /dev/nullHi Tony,
+++ b/arch/x86/include/asm/zhaoxin.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_ZHAOXIN_H
+#define _ASM_X86_ZHAOXIN_H
+
+#include <asm/cpu_device_id.h>
+#include <asm/microcode.h>
+
+#define ZHAOXIN_MODEL_ZXC VFM_MAKE(X86_VENDOR_ZHAOXIN, 6, 25)
+#define CENTAUR_MODEL_ZXC VFM_MAKE(X86_VENDOR_CENTAUR, 6, 15)
+
+struct x86_cpu_id naughty_list[] = {
+ X86_MATCH_VFM_STEPS(ZHAOXIN_MODEL_ZXC, 0, 3, 0),
+ X86_MATCH_VFM_STEPS(CENTAUR_MODEL_ZXC, 14, 15, 0),
+ {}
+};
This is headed in the right direction, in a way.
However, I think you might have missed a few things. Did you notice that
this structure is in a .h file? We generally don't define data
structures and variables in header files. You might want to take a quick
look around the tree.
Then, go try and #include this header in two different places. See what
happens.
+void check_fsgsbase_bugs(void);Generally, compiler warnings are good things. They tell you that you've
+
+void check_fsgsbase_bugs(void)
+{
done something wrong. Simply throwing code in to silence them isn't a
great practice.
Remember the compiler warning you got without the function declaration?
That was there to tell you that something is wrong. You placed
definitions in a header, not declarations.
But, adding a declaration before the definition made the compiler quiet.
+ u32 chip_pf, dummy, fixed_ucode;This is whitespace damaged, btw.
I also prefer one variable per line
u32 fixed_ucode;
u32 chip_pf;
u32 dummy;
+ if (!cpu_feature_enabled(X86_FEATURE_FSGSBASE))Heh, also I was joking about 'naughty_list'. It would be best to give it
+ return;
+
+ if (!x86_match_cpu(naughty_list))
+ return;
a good symbolic, meaningful name.
+ native_rdmsr(MSR_ZHAOXIN_MFGID, dummy, chip_pf);This at least need commenting. What prevents this code from getting
called on other vendors' CPUs? What about models of Zhaoxin CPUs that
don't have this MSR?
+ /* chip_pf represents product version flag */Please use the GENMASK macros here.
+ chip_pf = (chip_pf >> 15) & 0x7;
+ if (chip_pf == 0)It's probably worth commenting why this is calling an "intel"_ function.
+ fixed_ucode = 0x20e;
+ if (chip_pf == 1)
+ fixed_ucode = 0x208;
+
+ if (intel_get_microcode_revision() >= fixed_ucode)
+ return;
+ pr_warn_once("Broken FSGSBASE support, clearing feature\n");
+ setup_clear_cpu_cap(X86_FEATURE_FSGSBASE);
+}
+
+#endif
The revised patch is provided below, please review it again. Thank you.
iff --git a/MAINTAINERS b/MAINTAINERS
index 364f0bec8748..42093a794056 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -29168,6 +29168,7 @@ ZHAOXIN PROCESSOR SUPPORT
M: Tony W Wang-oc <TonyWWang-oc@xxxxxxxxxxx>
L: linux-kernel@xxxxxxxxxxxxxxx
S: Maintained
+F: arch/x86/include/asm/zhaoxin.h
F: arch/x86/kernel/cpu/zhaoxin.c
ZONED BLOCK DEVICE (BLOCK LAYER)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index be3e3cc963b2..dc71a4adc776 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -1306,5 +1306,7 @@
* disabling x2APIC will cause
* a #GP
*/
+/* ZHAOXIN defined MSRs*/
+#define MSR_ZHAOXIN_MFGID 0x00001232
#endif /* _ASM_X86_MSR_INDEX_H */
diff --git a/arch/x86/include/asm/zhaoxin.h b/arch/x86/include/asm/zhaoxin.h
new file mode 100644
index 000000000000..e7f380b678dc
--- /dev/null
+++ b/arch/x86/include/asm/zhaoxin.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_ZHAOXIN_H
+#define _ASM_X86_ZHAOXIN_H
+
+#include <asm/cpu_device_id.h>
+#include <asm/microcode.h>
+
+#define ZHAOXIN_MODEL_ZXC VFM_MAKE(X86_VENDOR_ZHAOXIN, 6, 25)
+#define CENTAUR_MODEL_ZXC VFM_MAKE(X86_VENDOR_CENTAUR, 6, 15)
+
+extern struct x86_cpu_id fsgsbase_bugs_list[];
+extern void check_fsgsbase_bugs(void);
+
+#endif
+
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 1c3261cae40c..49e58b55c414 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -73,6 +73,7 @@
#include <asm/tdx.h>
#include <asm/posted_intr.h>
#include <asm/runtime-const.h>
+#include <asm/zhaoxin.h>
#include "cpu.h"
@@ -1940,6 +1941,50 @@ void check_null_seg_clears_base(struct cpuinfo_x86 *c)
set_cpu_bug(c, X86_BUG_NULL_SEG);
}
+struct x86_cpu_id fsgsbase_bugs_list[] = {
+ X86_MATCH_VFM_STEPS(ZHAOXIN_MODEL_ZXC, 0, 3, 0),
+ X86_MATCH_VFM_STEPS(CENTAUR_MODEL_ZXC, 14, 15, 0),
+ {}
+};
+
+void check_fsgsbase_bugs(void)
+{
+ u32 chip_pf;
+ u32 dummy;
+ u32 fixed_ucode;
+
+ if (!cpu_feature_enabled(X86_FEATURE_FSGSBASE))
+ return;
+
+ if (!x86_match_cpu(fsgsbase_bugs_list))
+ return;
+
+ /*
+ * All Zhaoxin CPUs use MSR_ZHAOXIN_MFGID to represent
+ * manufacturing information. Please note that this MSR
+ * may have different meanings in other vendors' CPUs.
+ */
+ native_rdmsr(MSR_ZHAOXIN_MFGID, dummy, chip_pf);
+
+ /* chip_pf represents product version flag */
+ chip_pf = (chip_pf & GENMASK(17, 15)) >> 15;
+
+ if (chip_pf == 0)
+ fixed_ucode = 0x20e;
+ if (chip_pf == 1)
+ fixed_ucode = 0x208;
+
+ /*
+ * Zhaoxin ucode version retrieval method is compatible
+ * with Intel.
+ */
+ if (intel_get_microcode_revision() >= fixed_ucode)
+ return;
+
+ pr_warn_once("Broken FSGSBASE support, clearing feature\n");
+ setup_clear_cpu_cap(X86_FEATURE_FSGSBASE);
+}
+
static void generic_identify(struct cpuinfo_x86 *c)
{
c->extended_cpuid_level = 0;
@@ -2047,6 +2092,8 @@ static void identify_cpu(struct cpuinfo_x86 *c)
setup_umip(c);
setup_lass(c);
+ check_fsgsbase_bugs();
+
/* Enable FSGSBASE instructions if available. */
if (cpu_has(c, X86_FEATURE_FSGSBASE)) {
cr4_set_bits(X86_CR4_FSGSBASE);