On 4/14/25 2:44 PM, Xiaoyao Li wrote:

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


Got it. Thanks for the explanation.
I'll send a new fix patch as soon as possible.


Reply via email to