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/[email protected]/), 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.
