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

[Ewan Hai <ewanhai-oc@zhaoxin.com>]

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.