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

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

[Ewan Hai]

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.

Fixes: ff04bc1ac4 ("target/i386: Introduce Zhaoxin Yongfeng CPU model")
Signed-off-by: Ewan Hai <ewanhai-oc@zhaoxin.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1b64ceaaba..0dd9788a68 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5503,7 +5503,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
         .level = 0x1F,
         .vendor = CPUID_VENDOR_ZHAOXIN1,
         .family = 7,
-        .model = 11,
+        .model = 0x5b,
         .stepping = 3,
         /* missing: CPUID_HT, CPUID_TM, CPUID_PBE */
         .features[FEAT_1_EDX] =
-- 
2.34.1

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

[Paolo Bonzini]

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.

Paolo

> Fixes: ff04bc1ac4 ("target/i386: Introduce Zhaoxin Yongfeng CPU model")
> Signed-off-by: Ewan Hai <ewanhai-oc@zhaoxin.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   target/i386/cpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 1b64ceaaba..0dd9788a68 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5503,7 +5503,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
>           .level = 0x1F,
>           .vendor = CPUID_VENDOR_ZHAOXIN1,
>           .family = 7,
> -        .model = 11,
> +        .model = 0x5b,
>           .stepping = 3,
>           /* missing: CPUID_HT, CPUID_TM, CPUID_PBE */
>           .features[FEAT_1_EDX] =

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

[Ewan Hai]

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:
- 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?

Thank you for your guidance.

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

[Zhao Liu]

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-oc@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 */ }
+        }
     },
 };

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

[Ewan Hai]

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-oc@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.

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-oc@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

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

[Ewan Hai]

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-oc@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.