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