[PATCH v2 3/4] target/i386: Introduce Zhaoxin Shijidadao-Client CPU model

Ewan Hai posted 4 patches 3 months, 2 weeks ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Zhao Liu <zhao1.liu@intel.com>
[PATCH v2 3/4] target/i386: Introduce Zhaoxin Shijidadao-Client CPU model
Posted by Ewan Hai 3 months, 2 weeks ago
Zhaoxin "Shijidadao-Client" hardware has two revisions:

* v1 supports SMAP but lacks XSAVEC, XGETBV1, XSAVES, and VMX-XSAVES
* v2 provides XSAVEC/XGETBV1/XSAVES/VMX-XSAVES but does not support SMAP

This patch leverages QEMU's inherent versioning mechanism, where CPU
models default to '.version = 1'. The v1 definition matches the first
hardware revision, while a new 'version = 2' captures the differences
of the second revision. In this way, both hardware versions are
covered by a single CPU model, with the version property naturally
distinguishing their feature sets.

Signed-off-by: Ewan Hai <ewanhai-oc@zhaoxin.com>
---
 target/i386/cpu.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ffd1c727d5..a2e1b4924a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6769,6 +6769,147 @@ static const X86CPUDefinition builtin_x86_defs[] = {
         .model_id = "AMD EPYC-Turin Processor",
         .cache_info = &epyc_turin_cache_info,
     },
+    {
+        .name = "Shijidadao-Client",
+        .level = 0x1f,
+        .vendor = CPUID_VENDOR_ZHAOXIN1,
+        .family = 7,
+        .model = 0x6b,
+        .stepping = 1,
+        .cpuid_0x1f = true,
+        /* missing: CPUID_HT, CPUID_TM, CPUID_PBE */
+        .features[FEAT_1_EDX] =
+            CPUID_SS | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+            CPUID_ACPI | CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV |
+            CPUID_MCA | CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC |
+            CPUID_CX8 | CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC |
+            CPUID_PSE | CPUID_DE | CPUID_VME | CPUID_FP87,
+        /*
+         * missing: CPUID_EXT_OSXSAVE, CPUID_EXT_XTPR, CPUID_EXT_TM2,
+         * CPUID_EXT_EST, CPUID_EXT_SMX, CPUID_EXT_VMX
+         */
+        .features[FEAT_1_ECX] =
+            CPUID_EXT_RDRAND | CPUID_EXT_F16C | CPUID_EXT_AVX |
+            CPUID_EXT_XSAVE | CPUID_EXT_AES | CPUID_EXT_TSC_DEADLINE_TIMER |
+            CPUID_EXT_POPCNT | CPUID_EXT_MOVBE | CPUID_EXT_X2APIC |
+            CPUID_EXT_SSE42 | CPUID_EXT_SSE41 | CPUID_EXT_PCID |
+            CPUID_EXT_CX16 | CPUID_EXT_FMA | CPUID_EXT_SSSE3 |
+            CPUID_EXT_MONITOR | CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3,
+        .features[FEAT_7_0_EBX] =
+            CPUID_7_0_EBX_SHA_NI | CPUID_7_0_EBX_ADX | CPUID_7_0_EBX_RDSEED |
+            CPUID_7_0_EBX_INVPCID | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_SMEP |
+            CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_FSGSBASE,
+        /* missing: CPUID_7_0_ECX_OSPKE */
+        .features[FEAT_7_0_ECX] =
+            CPUID_7_0_ECX_RDPID | CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_UMIP,
+        .features[FEAT_7_0_EDX] =
+            CPUID_7_0_EDX_ARCH_CAPABILITIES | CPUID_7_0_EDX_SPEC_CTRL,
+        .features[FEAT_8000_0001_EDX] =
+            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_PDPE1GB |
+            CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
+        .features[FEAT_8000_0001_ECX] =
+            CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_ABM | CPUID_EXT3_LAHF_LM,
+        .features[FEAT_8000_0007_EDX] = CPUID_APM_INVTSC,
+        /*
+         * TODO: When the Linux kernel introduces other existing definitions
+         * for this leaf, remember to update the definitions here.
+         */
+        .features[FEAT_C000_0001_EDX] =
+            CPUID_C000_0001_EDX_PMM_EN | CPUID_C000_0001_EDX_PMM |
+            CPUID_C000_0001_EDX_PHE_EN | CPUID_C000_0001_EDX_PHE |
+            CPUID_C000_0001_EDX_ACE2 |
+            CPUID_C000_0001_EDX_XCRYPT_EN | CPUID_C000_0001_EDX_XCRYPT |
+            CPUID_C000_0001_EDX_XSTORE_EN | CPUID_C000_0001_EDX_XSTORE,
+        .features[FEAT_XSAVE] =
+            CPUID_XSAVE_XSAVEOPT,
+        .features[FEAT_ARCH_CAPABILITIES] =
+            MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY |
+            MSR_ARCH_CAP_MDS_NO | MSR_ARCH_CAP_PSCHANGE_MC_NO |
+            MSR_ARCH_CAP_SSB_NO,
+        .features[FEAT_VMX_PROCBASED_CTLS] =
+            VMX_CPU_BASED_VIRTUAL_INTR_PENDING | VMX_CPU_BASED_HLT_EXITING |
+            VMX_CPU_BASED_USE_TSC_OFFSETING | VMX_CPU_BASED_INVLPG_EXITING |
+            VMX_CPU_BASED_MWAIT_EXITING | VMX_CPU_BASED_RDPMC_EXITING |
+            VMX_CPU_BASED_RDTSC_EXITING | VMX_CPU_BASED_CR3_LOAD_EXITING |
+            VMX_CPU_BASED_CR3_STORE_EXITING | VMX_CPU_BASED_CR8_LOAD_EXITING |
+            VMX_CPU_BASED_CR8_STORE_EXITING | VMX_CPU_BASED_TPR_SHADOW |
+            VMX_CPU_BASED_VIRTUAL_NMI_PENDING | VMX_CPU_BASED_MOV_DR_EXITING |
+            VMX_CPU_BASED_UNCOND_IO_EXITING | VMX_CPU_BASED_USE_IO_BITMAPS |
+            VMX_CPU_BASED_MONITOR_TRAP_FLAG | VMX_CPU_BASED_USE_MSR_BITMAPS |
+            VMX_CPU_BASED_MONITOR_EXITING | VMX_CPU_BASED_PAUSE_EXITING |
+            VMX_CPU_BASED_ACTIVATE_SECONDARY_CONTROLS,
+        /*
+         * missing: VMX_SECONDARY_EXEC_PAUSE_LOOP_EXITING,
+         * VMX_SECONDARY_EXEC_TSC_SCALING
+         */
+        .features[FEAT_VMX_SECONDARY_CTLS] =
+            VMX_SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
+            VMX_SECONDARY_EXEC_ENABLE_EPT | VMX_SECONDARY_EXEC_DESC |
+            VMX_SECONDARY_EXEC_RDTSCP | VMX_SECONDARY_EXEC_ENABLE_VPID |
+            VMX_SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
+            VMX_SECONDARY_EXEC_WBINVD_EXITING |
+            VMX_SECONDARY_EXEC_UNRESTRICTED_GUEST |
+            VMX_SECONDARY_EXEC_APIC_REGISTER_VIRT |
+            VMX_SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
+            VMX_SECONDARY_EXEC_RDRAND_EXITING |
+            VMX_SECONDARY_EXEC_ENABLE_INVPCID |
+            VMX_SECONDARY_EXEC_ENABLE_VMFUNC |
+            VMX_SECONDARY_EXEC_SHADOW_VMCS |
+            VMX_SECONDARY_EXEC_ENABLE_PML,
+        .features[FEAT_VMX_PINBASED_CTLS] =
+            VMX_PIN_BASED_EXT_INTR_MASK | VMX_PIN_BASED_NMI_EXITING |
+            VMX_PIN_BASED_VIRTUAL_NMIS | VMX_PIN_BASED_VMX_PREEMPTION_TIMER |
+            VMX_PIN_BASED_POSTED_INTR,
+        .features[FEAT_VMX_EXIT_CTLS] =
+            VMX_VM_EXIT_SAVE_DEBUG_CONTROLS | VMX_VM_EXIT_HOST_ADDR_SPACE_SIZE |
+            VMX_VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL |
+            VMX_VM_EXIT_ACK_INTR_ON_EXIT | VMX_VM_EXIT_SAVE_IA32_PAT |
+            VMX_VM_EXIT_LOAD_IA32_PAT | VMX_VM_EXIT_SAVE_IA32_EFER |
+            VMX_VM_EXIT_LOAD_IA32_EFER | VMX_VM_EXIT_SAVE_VMX_PREEMPTION_TIMER,
+        /* missing: VMX_VM_ENTRY_SMM, VMX_VM_ENTRY_DEACT_DUAL_MONITOR */
+        .features[FEAT_VMX_ENTRY_CTLS] =
+            VMX_VM_ENTRY_LOAD_DEBUG_CONTROLS | VMX_VM_ENTRY_IA32E_MODE |
+            VMX_VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL |
+            VMX_VM_ENTRY_LOAD_IA32_PAT | VMX_VM_ENTRY_LOAD_IA32_EFER,
+        /*
+         * missing: MSR_VMX_MISC_ACTIVITY_SHUTDOWN,
+         * MSR_VMX_MISC_ACTIVITY_WAIT_SIPI
+         */
+        .features[FEAT_VMX_MISC] =
+            MSR_VMX_MISC_STORE_LMA | MSR_VMX_MISC_ACTIVITY_HLT |
+            MSR_VMX_MISC_VMWRITE_VMEXIT,
+        /* missing: MSR_VMX_EPT_UC */
+        .features[FEAT_VMX_EPT_VPID_CAPS] =
+            MSR_VMX_EPT_EXECONLY | MSR_VMX_EPT_PAGE_WALK_LENGTH_4 |
+            MSR_VMX_EPT_WB | MSR_VMX_EPT_2MB | MSR_VMX_EPT_1GB |
+            MSR_VMX_EPT_INVEPT | MSR_VMX_EPT_AD_BITS |
+            MSR_VMX_EPT_INVEPT_SINGLE_CONTEXT | MSR_VMX_EPT_INVEPT_ALL_CONTEXT |
+            MSR_VMX_EPT_INVVPID_SINGLE_CONTEXT | MSR_VMX_EPT_INVVPID |
+            MSR_VMX_EPT_INVVPID_ALL_CONTEXT | MSR_VMX_EPT_INVVPID_SINGLE_ADDR |
+            MSR_VMX_EPT_INVVPID_SINGLE_CONTEXT_NOGLOBALS,
+        .features[FEAT_VMX_BASIC] =
+            MSR_VMX_BASIC_INS_OUTS | MSR_VMX_BASIC_TRUE_CTLS,
+        .features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
+        .xlevel = 0x80000008,
+        .model_id = "Zhaoxin Shijidadao-Client Processor",
+        .cache_info = &shijidadao_cache_info,
+        .versions = (X86CPUVersionDefinition[]) {
+            { .version = 1 },
+            {
+                .version = 2,
+                .note = "with more XSAVE features",
+                .props = (PropValue[]) {
+                    { "xsavec", "on" },
+                    { "xgetbv1", "on" },
+                    { "xsaves", "on"},
+                    { "vmx-xsaves", "on"},
+                    { "smap", "off" },
+                    { /* end of list */ }
+                },
+            },
+            { /* end of list */ }
+        }
+    },
 };
 
 /*
-- 
2.34.1
Re: [PATCH v2 3/4] target/i386: Introduce Zhaoxin Shijidadao-Client CPU model
Posted by Zhao Liu 2 months, 2 weeks ago
> +        /*
> +         * TODO: When the Linux kernel introduces other existing definitions
> +         * for this leaf, remember to update the definitions here.
> +         */

This TODO seems a bit vague; it's best to explicitly list the existing
features that are currently missing. Otherwise, maintainers won't be
able to understand or clean up this TODO either.

> +        .features[FEAT_C000_0001_EDX] =
> +            CPUID_C000_0001_EDX_PMM_EN | CPUID_C000_0001_EDX_PMM |
> +            CPUID_C000_0001_EDX_PHE_EN | CPUID_C000_0001_EDX_PHE |
> +            CPUID_C000_0001_EDX_ACE2 |
> +            CPUID_C000_0001_EDX_XCRYPT_EN | CPUID_C000_0001_EDX_XCRYPT |
> +            CPUID_C000_0001_EDX_XSTORE_EN | CPUID_C000_0001_EDX_XSTORE,

...

> +        .model_id = "Zhaoxin Shijidadao-Client Processor",
> +        .cache_info = &shijidadao_cache_info,
> +        .versions = (X86CPUVersionDefinition[]) {
> +            { .version = 1 },
> +            {
> +                .version = 2,
> +                .note = "with more XSAVE features",

it's better to mention "without smap" as well.

(Based based on my personal experience, the absence of SMAP seems a bit
odd. Could it be a hardware bug in a specific stepping?)

> +                .props = (PropValue[]) {
> +                    { "xsavec", "on" },
> +                    { "xgetbv1", "on" },
> +                    { "xsaves", "on"},
> +                    { "vmx-xsaves", "on"},
> +                    { "smap", "off" },
> +                    { /* end of list */ }
> +                },
> +            },

BTW, if the differences aren't too significant, is it possible to merge
the server and client models? :)

Thanks,
Zhao
Re: [PATCH v2 3/4] target/i386: Introduce Zhaoxin Shijidadao-Client CPU model
Posted by Ewan Hai 2 months, 2 weeks ago
On 11/25/25 3:35 PM, Zhao Liu wrote:
> 
> 
>> +        /*
>> +         * TODO: When the Linux kernel introduces other existing definitions
>> +         * for this leaf, remember to update the definitions here.
>> +         */
> 
> This TODO seems a bit vague; it's best to explicitly list the existing
> features that are currently missing. Otherwise, maintainers won't be
> able to understand or clean up this TODO either.
> 

I agree. The same problem also exists in the YongFeng vCPU model. For this
series, I can drop the vague TODO and instead add a more explicit comment that
documents which CPUID.C000_0001.EDX bits are intentionally missing today. In
addition, I can post a small follow-up cleanup patch to fix the YongFeng model
in the same way, so the two Zhaoxin models stay consistent. If you prefer, I can
also fold the YongFeng comment update into this series as an extra patch.

As background, current Zhaoxin CPUs implement several CPUID.(EAX=0xC0000001,
ECX=0):EDX feature bits that are not yet defined in the Linux kernel, for
example SM2/SM2_EN, SM3/SM4 and their enable bits, PARALLAX/PARALLAX_EN,
TM3/TM3_EN, RNG2/RNG2_EN, PHE2/PHE2_EN, and RSA/RSA_EN.

We previously tried to upstream all these extra feature bits in one
patch(https://lore.kernel.org/all/20230414095334.8743-1-TonyWWang-oc@zhaoxin.com/),
but the maintainer rejected it because there was no in-tree code using these
features yet. So our current plan is to add the CPUID bits together with real
kernel users step by step.

>> +        .features[FEAT_C000_0001_EDX] =
>> +            CPUID_C000_0001_EDX_PMM_EN | CPUID_C000_0001_EDX_PMM |
>> +            CPUID_C000_0001_EDX_PHE_EN | CPUID_C000_0001_EDX_PHE |
>> +            CPUID_C000_0001_EDX_ACE2 |
>> +            CPUID_C000_0001_EDX_XCRYPT_EN | CPUID_C000_0001_EDX_XCRYPT |
>> +            CPUID_C000_0001_EDX_XSTORE_EN | CPUID_C000_0001_EDX_XSTORE,
> 
> ...
> 
>> +        .model_id = "Zhaoxin Shijidadao-Client Processor",
>> +        .cache_info = &shijidadao_cache_info,
>> +        .versions = (X86CPUVersionDefinition[]) {
>> +            { .version = 1 },
>> +            {
>> +                .version = 2,
>> +                .note = "with more XSAVE features",
> 
> it's better to mention "without smap" as well.
>

Sure, I will update the note to say "with more XSAVE features but without SMAP".

> (Based on my personal experience, the absence of SMAP seems a bit
> odd. Could it be a hardware bug in a specific stepping?)
> 

This is not a stepping-specific silicon bug. For this product family, SMAP
support was intentionally not enabled in the final product because our internal
performance evaluation showed an unacceptable performance impact in certain
workloads. The v2 CPU model therefore keeps "smap" off to reflect the actual
shipped behavior, while the v1 definition with SMAP enabled is kept for
customers who need to model early v1 silicon where SMAP is still available.

>> +                .props = (PropValue[]) {
>> +                    { "xsavec", "on" },
>> +                    { "xgetbv1", "on" },
>> +                    { "xsaves", "on"},
>> +                    { "vmx-xsaves", "on"},
>> +                    { "smap", "off" },
>> +                    { /* end of list */ }
>> +                },
>> +            },
> 
> BTW, if the differences aren't too significant, is it possible to merge
> the server and client models? :)
> 

From the user point of view, I slightly prefer keeping separate
Shijidadao-Client and Shijidadao-Server models.

The main reason is that customers who want a "full-feature" vCPU that behaves
very close to a specific physical product can simply pick the corresponding
model name, without having to remember a set of extra "-cpu ..., +-feature"
overrides. If we merge everything into a single Shijidadao model that
corresponds to a more restricted baseline, users who want the full configuration
would need to explicitly enable multiple features (such as the additional XSAVE
bits) on the command line, which is easier to get wrong and less user-friendly.

This is also aligned with how QEMU models other vendors' micro-architectures
where client and server products have slightly different feature sets.

Thanks again for the review!

Best regards,
Ewan.
Re: [PATCH v2 3/4] target/i386: Introduce Zhaoxin Shijidadao-Client CPU model
Posted by Zhao Liu 1 month, 3 weeks ago
On Tue, Nov 25, 2025 at 04:57:04PM +0800, Ewan Hai wrote:
> Date: Tue, 25 Nov 2025 16:57:04 +0800
> From: Ewan Hai <ewanhai-oc@zhaoxin.com>
> Subject: Re: [PATCH v2 3/4] target/i386: Introduce Zhaoxin
>  Shijidadao-Client CPU model
> 
> On 11/25/25 3:35 PM, Zhao Liu wrote:
> > 
> > 
> >> +        /*
> >> +         * TODO: When the Linux kernel introduces other existing definitions
> >> +         * for this leaf, remember to update the definitions here.
> >> +         */
> > 
> > This TODO seems a bit vague; it's best to explicitly list the existing
> > features that are currently missing. Otherwise, maintainers won't be
> > able to understand or clean up this TODO either.
> > 
> 
> I agree. The same problem also exists in the YongFeng vCPU model. For this
> series, I can drop the vague TODO and instead add a more explicit comment that
> documents which CPUID.C000_0001.EDX bits are intentionally missing today. In
> addition, I can post a small follow-up cleanup patch to fix the YongFeng model
> in the same way, so the two Zhaoxin models stay consistent. If you prefer, I can
> also fold the YongFeng comment update into this series as an extra patch.

Yes, it's good to make everything clear and I think it's better to
include your extra patch into this series to help maintainer review/pick
in one goes.

> As background, current Zhaoxin CPUs implement several CPUID.(EAX=0xC0000001,
> ECX=0):EDX feature bits that are not yet defined in the Linux kernel, for
> example SM2/SM2_EN, SM3/SM4 and their enable bits, PARALLAX/PARALLAX_EN,
> TM3/TM3_EN, RNG2/RNG2_EN, PHE2/PHE2_EN, and RSA/RSA_EN.
> 
> We previously tried to upstream all these extra feature bits in one
> patch(https://lore.kernel.org/all/20230414095334.8743-1-TonyWWang-oc@zhaoxin.com/),
> but the maintainer rejected it because there was no in-tree code using these
> features yet. So our current plan is to add the CPUID bits together with real
> kernel users step by step.

I see. I think it's enough to document missing CPUIDs in comment.

...

> > (Based on my personal experience, the absence of SMAP seems a bit
> > odd. Could it be a hardware bug in a specific stepping?)
> > 
> 
> This is not a stepping-specific silicon bug. For this product family, SMAP
> support was intentionally not enabled in the final product because our internal
> performance evaluation showed an unacceptable performance impact in certain
> workloads. The v2 CPU model therefore keeps "smap" off to reflect the actual
> shipped behavior, while the v1 definition with SMAP enabled is kept for
> customers who need to model early v1 silicon where SMAP is still available.

v1 is not the final product, then I think it's not necessary to upstream
it. For example, these Intel CPU models are basically all targeted at
the final products. But unluckily, engineering samples may have bugs so
we have to add or remove features based on what the final products
support. So if the final product is clear from the beginning, there's no
need to take intermediate steps.

BTW, even with v2, user can still enable smap by +smap.

> >> +                .props = (PropValue[]) {
> >> +                    { "xsavec", "on" },
> >> +                    { "xgetbv1", "on" },
> >> +                    { "xsaves", "on"},
> >> +                    { "vmx-xsaves", "on"},
> >> +                    { "smap", "off" },
> >> +                    { /* end of list */ }
> >> +                },
> >> +            },
> > 
> > BTW, if the differences aren't too significant, is it possible to merge
> > the server and client models? :)
> > 
> 
> From the user point of view, I slightly prefer keeping separate
> Shijidadao-Client and Shijidadao-Server models.
> 
> The main reason is that customers who want a "full-feature" vCPU that behaves
> very close to a specific physical product can simply pick the corresponding
> model name, without having to remember a set of extra "-cpu ..., +-feature"
> overrides. If we merge everything into a single Shijidadao model that
> corresponds to a more restricted baseline, users who want the full configuration
> would need to explicitly enable multiple features (such as the additional XSAVE
> bits) on the command line, which is easier to get wrong and less user-friendly.

Could we make Shijidadao-Client as a v2 of Shijidadao-Server, and create an
alias for this v2?

.alias = "Shijidadao-Client"

Then we could rename Shijidadao-Server to Shijidadao, and its v2 is for
client.

> This is also aligned with how QEMU models other vendors' micro-architectures
> where client and server products have slightly different feature sets.

The main use case for CPU models is to easy migration across mixed CPU
clusters [*]. So, IMO, not all products require a model.

[*]: https://lore.kernel.org/qemu-devel/ZJrJYG1dICHjKx09@redhat.com/

> Thanks again for the review!

Thanks for your patience and sorry for my late reply.

Regards,
Zhao
Re: [PATCH v2 3/4] target/i386: Introduce Zhaoxin Shijidadao-Client CPU model
Posted by Ewan Hai 1 month ago
On 12/15/25 5:03 PM, Zhao Liu wrote:
> 
> 
> On Tue, Nov 25, 2025 at 04:57:04PM +0800, Ewan Hai wrote:
>> Date: Tue, 25 Nov 2025 16:57:04 +0800
>> From: Ewan Hai <ewanhai-oc@zhaoxin.com>
>> Subject: Re: [PATCH v2 3/4] target/i386: Introduce Zhaoxin
>>  Shijidadao-Client CPU model
>>
>> On 11/25/25 3:35 PM, Zhao Liu wrote:
>>>
>>>
>>>> +        /*
>>>> +         * TODO: When the Linux kernel introduces other existing definitions
>>>> +         * for this leaf, remember to update the definitions here.
>>>> +         */
>>>
>>> This TODO seems a bit vague; it's best to explicitly list the existing
>>> features that are currently missing. Otherwise, maintainers won't be
>>> able to understand or clean up this TODO either.
>>>
>>
>> I agree. The same problem also exists in the YongFeng vCPU model. For this
>> series, I can drop the vague TODO and instead add a more explicit comment that
>> documents which CPUID.C000_0001.EDX bits are intentionally missing today. In
>> addition, I can post a small follow-up cleanup patch to fix the YongFeng model
>> in the same way, so the two Zhaoxin models stay consistent. If you prefer, I can
>> also fold the YongFeng comment update into this series as an extra patch.
> 
> Yes, it's good to make everything clear and I think it's better to
> include your extra patch into this series to help maintainer review/pick
> in one goes.

Got it.

>> As background, current Zhaoxin CPUs implement several CPUID.(EAX=0xC0000001,
>> ECX=0):EDX feature bits that are not yet defined in the Linux kernel, for
>> example SM2/SM2_EN, SM3/SM4 and their enable bits, PARALLAX/PARALLAX_EN,
>> TM3/TM3_EN, RNG2/RNG2_EN, PHE2/PHE2_EN, and RSA/RSA_EN.
>>
>> We previously tried to upstream all these extra feature bits in one
>> patch(https://lore.kernel.org/all/20230414095334.8743-1-TonyWWang-oc@zhaoxin.com/),
>> but the maintainer rejected it because there was no in-tree code using these
>> features yet. So our current plan is to add the CPUID bits together with real
>> kernel users step by step.
> 
> I see. I think it's enough to document missing CPUIDs in comment.
> 

Would the following comment be acceptable?

/*
 * missing: SM2/SM2_EN, CCS/CCS_EN, PARALLAX/PARALLAX_EN,
 * TM3/TM3_EN, RNG2/RNG2_EN, PHE2/PHE2_EN, RSA/RSA_EN
 */

Do you think I should also include the lore link in the commit message/cover
letter for additional context?

> ...
> 
>>> (Based on my personal experience, the absence of SMAP seems a bit
>>> odd. Could it be a hardware bug in a specific stepping?)
>>>
>>
>> This is not a stepping-specific silicon bug. For this product family, SMAP
>> support was intentionally not enabled in the final product because our internal
>> performance evaluation showed an unacceptable performance impact in certain
>> workloads. The v2 CPU model therefore keeps "smap" off to reflect the actual
>> shipped behavior, while the v1 definition with SMAP enabled is kept for
>> customers who need to model early v1 silicon where SMAP is still available.
> 
> v1 is not the final product, then I think it's not necessary to upstream
> it. For example, these Intel CPU models are basically all targeted at
> the final products. But unluckily, engineering samples may have bugs so
> we have to add or remove features based on what the final products
> support. So if the final product is clear from the beginning, there's no
> need to take intermediate steps.
> 
> BTW, even with v2, user can still enable smap by +smap.
> 
>>>> +                .props = (PropValue[]) {
>>>> +                    { "xsavec", "on" },
>>>> +                    { "xgetbv1", "on" },
>>>> +                    { "xsaves", "on"},
>>>> +                    { "vmx-xsaves", "on"},
>>>> +                    { "smap", "off" },
>>>> +                    { /* end of list */ }
>>>> +                },
>>>> +            },
>>>
>>> BTW, if the differences aren't too significant, is it possible to merge
>>> the server and client models? :)
>>>
>>
>> From the user point of view, I slightly prefer keeping separate
>> Shijidadao-Client and Shijidadao-Server models.
>>
>> The main reason is that customers who want a "full-feature" vCPU that behaves
>> very close to a specific physical product can simply pick the corresponding
>> model name, without having to remember a set of extra "-cpu ..., +-feature"
>> overrides. If we merge everything into a single Shijidadao model that
>> corresponds to a more restricted baseline, users who want the full configuration
>> would need to explicitly enable multiple features (such as the additional XSAVE
>> bits) on the command line, which is easier to get wrong and less user-friendly.
> 
> Could we make Shijidadao-Client as a v2 of Shijidadao-Server, and create an
> alias for this v2?
> 
> .alias = "Shijidadao-Client"
> 
> Then we could rename Shijidadao-Server to Shijidadao, and its v2 is for
> client.
> 
>> This is also aligned with how QEMU models other vendors' micro-architectures
>> where client and server products have slightly different feature sets.
> 
> The main use case for CPU models is to easy migration across mixed CPU
> clusters [*]. So, IMO, not all products require a model.

For the CPU model naming/versioning, my plan is:
The current Shijidadao will be equivalent to the old Shijidadao-Client-v2, drop
the old Shijidadao-Client-v1 according to your advice, Shijidadao-v1 will have
the alias Shijidadao-Client, and Shijidadao-v2 will have the alias
Shijidadao-Server.

A key code sketch would look like:

    {
        .name = "Shijidadao",
        .level = 0x1f,
        .vendor = CPUID_VENDOR_ZHAOXIN1,
        .family = 7,
        .model = 0x6b,
        .stepping = 1,
...

        .model_id = "Zhaoxin Shijidadao-Client Processor",
        .cache_info = &shijidadao_cache_info,
        .versions = (X86CPUVersionDefinition[]) {
            { .version = 1, .alias = "Shijidadao-Client" },
            {
                .version = 2,
                .alias = "Shijidadao-Server",
                .note = "server variant",
                .props = (PropValue[]) {
                    { "model", "0x7b" },
                    { "stepping", "0" },
                    { "core-capability", "on" },
                    { "split-lock-detect", "on" },
                    { "model-id",
                      "Zhaoxin Shijidadao-Server Processor" },
                    { /* end of list */ }
                },
            },
            { /* end of list */ }
        }
    },

Does this mapping look acceptable to you?


Best wishes,
Ewan.
Re: [PATCH v2 3/4] target/i386: Introduce Zhaoxin Shijidadao-Client CPU model
Posted by Zhao Liu 1 month ago
> >> As background, current Zhaoxin CPUs implement several CPUID.(EAX=0xC0000001,
> >> ECX=0):EDX feature bits that are not yet defined in the Linux kernel, for
> >> example SM2/SM2_EN, SM3/SM4 and their enable bits, PARALLAX/PARALLAX_EN,
> >> TM3/TM3_EN, RNG2/RNG2_EN, PHE2/PHE2_EN, and RSA/RSA_EN.
> >>
> >> We previously tried to upstream all these extra feature bits in one
> >> patch(https://lore.kernel.org/all/20230414095334.8743-1-TonyWWang-oc@zhaoxin.com/),
> >> but the maintainer rejected it because there was no in-tree code using these
> >> features yet. So our current plan is to add the CPUID bits together with real
> >> kernel users step by step.
> > 
> > I see. I think it's enough to document missing CPUIDs in comment.
> > 
> 
> Would the following comment be acceptable?
> 
> /*
>  * missing: SM2/SM2_EN, CCS/CCS_EN, PARALLAX/PARALLAX_EN,
>  * TM3/TM3_EN, RNG2/RNG2_EN, PHE2/PHE2_EN, RSA/RSA_EN
>  */

Yes, look good to me.

> Do you think I should also include the lore link in the commit message/cover
> letter for additional context?

Yes, mentioning the link in commit message is good. More information is
helpful.

> > Could we make Shijidadao-Client as a v2 of Shijidadao-Server, and create an
> > alias for this v2?
> > 
> > .alias = "Shijidadao-Client"
> > 
> > Then we could rename Shijidadao-Server to Shijidadao, and its v2 is for
> > client.
> > 
> >> This is also aligned with how QEMU models other vendors' micro-architectures
> >> where client and server products have slightly different feature sets.
> > 
> > The main use case for CPU models is to easy migration across mixed CPU
> > clusters [*]. So, IMO, not all products require a model.
> 
> For the CPU model naming/versioning, my plan is:
> The current Shijidadao will be equivalent to the old Shijidadao-Client-v2, drop
> the old Shijidadao-Client-v1 according to your advice, Shijidadao-v1 will have
> the alias Shijidadao-Client, and Shijidadao-v2 will have the alias
> Shijidadao-Server.

Migration should have more use cases for the server. Personally, I feel
using the server version as the base model might be more convenient?
Anyway, it's up to you. Overall, these are fine for me.

Thanks,
Zhao
Re: [PATCH v2 3/4] target/i386: Introduce Zhaoxin Shijidadao-Client CPU model
Posted by Ewan Hai 1 month ago
On 1/7/26 11:55 AM, Zhao Liu wrote:
> 
> 
>>>> As background, current Zhaoxin CPUs implement several CPUID.(EAX=0xC0000001,
>>>> ECX=0):EDX feature bits that are not yet defined in the Linux kernel, for
>>>> example SM2/SM2_EN, SM3/SM4 and their enable bits, PARALLAX/PARALLAX_EN,
>>>> TM3/TM3_EN, RNG2/RNG2_EN, PHE2/PHE2_EN, and RSA/RSA_EN.
>>>>
>>>> We previously tried to upstream all these extra feature bits in one
>>>> patch(https://lore.kernel.org/all/20230414095334.8743-1-TonyWWang-oc@zhaoxin.com/),
>>>> but the maintainer rejected it because there was no in-tree code using these
>>>> features yet. So our current plan is to add the CPUID bits together with real
>>>> kernel users step by step.
>>>
>>> I see. I think it's enough to document missing CPUIDs in comment.
>>>
>>
>> Would the following comment be acceptable?
>>
>> /*
>>  * missing: SM2/SM2_EN, CCS/CCS_EN, PARALLAX/PARALLAX_EN,
>>  * TM3/TM3_EN, RNG2/RNG2_EN, PHE2/PHE2_EN, RSA/RSA_EN
>>  */
> 
> Yes, look good to me.
> 
>> Do you think I should also include the lore link in the commit message/cover
>> letter for additional context?
> 
> Yes, mentioning the link in commit message is good. More information is
> helpful.
> 
>>> Could we make Shijidadao-Client as a v2 of Shijidadao-Server, and create an
>>> alias for this v2?
>>>
>>> .alias = "Shijidadao-Client"
>>>
>>> Then we could rename Shijidadao-Server to Shijidadao, and its v2 is for
>>> client.
>>>
>>>> This is also aligned with how QEMU models other vendors' micro-architectures
>>>> where client and server products have slightly different feature sets.
>>>
>>> The main use case for CPU models is to easy migration across mixed CPU
>>> clusters [*]. So, IMO, not all products require a model.
>>
>> For the CPU model naming/versioning, my plan is:
>> The current Shijidadao will be equivalent to the old Shijidadao-Client-v2, drop
>> the old Shijidadao-Client-v1 according to your advice, Shijidadao-v1 will have
>> the alias Shijidadao-Client, and Shijidadao-v2 will have the alias
>> Shijidadao-Server.
> 
> Migration should have more use cases for the server. Personally, I feel
> using the server version as the base model might be more convenient?
> Anyway, it's up to you. Overall, these are fine for me.

I hadn't considered that server-side migration is a more common use case, thanks
a lot. See you in v3.

Best wishes,
Ewan.