Re: [PATCH v2] target/i386: Fix model number of Zhaoxin YongFeng vCPU template

[Xiaoyao Li]

On 4/11/2025 3:42 PM, Ewan Hai wrote:


On 4/11/25 11:22 AM, Zhao Liu wrote:

On Thu, Apr 10, 2025 at 10:07:15PM +0800, Ewan Hai wrote:
Date: Thu, 10 Apr 2025 22:07:15 +0800
From: Ewan Hai <ewanhai...@zhaoxin.com>
Subject: Re: [PATCH v2] target/i386: Fix model number of Zhaoxin 
YongFeng
  vCPU template

On 4/10/25 8:22 PM, Paolo Bonzini wrote:

On 4/7/25 04:07, Ewan Hai wrote:
The model number was mistakenly set to 0x0b (11) in commit ff04bc1ac4.
The correct value is 0x5b. This mistake occurred because the extended
model bits in cpuid[eax=0x1].eax were overlooked, and only the base
model was used.

This patch corrects the model field.

Hi, please follow commit e0013791b9326945ccd09b5b602437beb322cab8 to
define a new version of the CPU.

I’ve noticed that in the QEMU repository at commit
e0013791b9326945ccd09b5b602437beb322cab8 (as HEAD), the following 
patches I
previously submitted (which the Zhaoxin YongFeng vCPU model depends 
on) are
not included:

:-) e0013791b9326945ccd09b5b602437beb322cab8 is an example case to show
how to fix model id.

- 5d20aa540b6991c0dbeef933d2055e5372f52e0e: "target/i386: Add support 
for
Zhaoxin CPU vendor identification"
- c0799e8b003713e07b546faba600363eccd179ee: "target/i386: Add CPUID leaf
0xC000_0001 EDX definitions"
- ff04bc1ac478656e5d6a255bf4069edb3f55bc58: "target/i386: Introduce 
Zhaoxin
Yongfeng CPU model" (this is the main patch that needs to be fixed)
- a4e749780bd20593c0c386612a51bf4d64a80132: "target/i386: Mask 
CMPLegacy bit
in CPUID[0x80000001].ECX for Zhaoxin CPUs"

Should I resend the entire patchset, or would it be sufficient to 
just send
a revised version of the “target/i386: Introduce Zhaoxin Yongfeng CPU 
model”
patch?

IIUC, because this fix is planning to land in v10.1 (next release
cycle), current CPU model (will be released in v10.0) can't be modified
directly. It is only possible to directly modify an unreleased CPU model
during the same release cycle.

Thus it's enough to just introduce a v2 and correct your model id like
this:

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1b64ceaaba46..1ca1c3a729e8 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5621,6 +5621,17 @@ static const X86CPUDefinition 
builtin_x86_defs[] = {
          .features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
          .xlevel = 0x80000008,
          .model_id = "Zhaoxin YongFeng Processor",
+        .versions = (X86CPUVersionDefinition[]) {
+            { .version = 1 },
+            {
+                .version = 2,
+                .props = (PropValue[]) {
+                    { "model", "0x5b" },
+                    { /* end of list */ }
+                }
+            },
+            { /* end of list */ }
+        }
      },
  };


Thanks again for your patience and explanation.

I'm not entirely sure if this is the best approach. I have one thought, 
and I'd like your help to confirm whether I'm on the right track or not. 
From what I can tell, most other vCPU definitions that use the .versions 
mechanism do so incrementally: for instance, they add new features in 
v2, v3, etc., but each of those versions (v1, v2, v3) remains valid for 
practical use.

However, in our specific case, the v1 version of the Zhaoxin vCPU 
definition has an incorrect .model value, which breaks the Linux guest's 
vPMU functionality. That makes me uncertain whether using new version 
definitions to fix this issue is really the best solution. After all, v1 
itself would remain problematic.

Do you have any thoughts on whether it might be better to correct the 
existing definition, or do you think the versioned approach is still the 
recommended path? I appreciate any input or guidance you can provide.


If changing the @model value directly, it will introduce user visible 
change. E.g., live migrating from old QEMU to new QEMU, the guest will 
find the model number changes.

That's why versioned CPU model was introduced. It becomes the fact 
already that YongFeng-v1 has model id 11 and broken vpmu. If user create 
vcpu with YongFeng-v1, user has to accept it. If user wants a working 
vpmu, go and use YongFeng-v2