1. Patchew
  2. QEMU
  3. [PATCH 0/4] Add support for Zhaoxin Yongfeng CPU model and other improvements
  4. View patch

[PATCH 4/4] target/i386: Update CMPLegacy handling for Zhaoxin and VIA CPUs

EwanHai posted 4 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH 4/4] target/i386: Update CMPLegacy handling for Zhaoxin and VIA CPUs
Posted by EwanHai 2 months, 3 weeks ago 
Zhaoxin and VIA CPUs handle the CMPLegacy bit in the same way
as Intel CPUs. This patch simplifies the existing logic by
using the IS_XXX_CPU macro and includes checks for Zhaoxin
and VIA vendors to align their behavior with Intel.

Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
---
 target/i386/cpu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 50edff077e..0836416617 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6945,9 +6945,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
          * So don't set it here for Intel to make Linux guests happy.
          */
         if (threads_per_pkg > 1) {
-            if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 ||
-                env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 ||
-                env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) {
+            if (!IS_INTEL_CPU(env) &&
+                !IS_ZHAOXIN_CPU(env) &&
+                !IS_VIA_CPU(env)) {
                 *ecx |= 1 << 1;    /* CmpLegacy bit */
             }
         }
-- 
2.34.1
Re: [PATCH 4/4] target/i386: Update CMPLegacy handling for Zhaoxin and VIA CPUs
Posted by Xiaoyao Li 2 months, 2 weeks ago 
On 6/25/2024 5:19 PM, EwanHai wrote:
> Zhaoxin and VIA CPUs handle the CMPLegacy bit in the same way
> as Intel CPUs. This patch simplifies the existing logic by
> using the IS_XXX_CPU macro and includes checks for Zhaoxin
> and VIA vendors to align their behavior with Intel.
> 
> Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
> ---
>   target/i386/cpu.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 50edff077e..0836416617 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6945,9 +6945,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>            * So don't set it here for Intel to make Linux guests happy.
>            */
>           if (threads_per_pkg > 1) {
> -            if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 ||
> -                env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 ||
> -                env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) {
> +            if (!IS_INTEL_CPU(env) &&
> +                !IS_ZHAOXIN_CPU(env) &&
> +                !IS_VIA_CPU(env)) {

it seems you added ! by mistake.

>                   *ecx |= 1 << 1;    /* CmpLegacy bit */
>               }
>           }
Re: [PATCH 4/4] target/i386: Update CMPLegacy handling for Zhaoxin and VIA CPUs
Posted by Ewan Hai 2 months, 2 weeks ago 
On 7/3/24 10:49, Xiaoyao Li wrote:
>
> On 6/25/2024 5:19 PM, EwanHai wrote:
>> Zhaoxin and VIA CPUs handle the CMPLegacy bit in the same way
>> as Intel CPUs. This patch simplifies the existing logic by
>> using the IS_XXX_CPU macro and includes checks for Zhaoxin
>> and VIA vendors to align their behavior with Intel.
>>
>> Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
>> ---
>>   target/i386/cpu.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 50edff077e..0836416617 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -6945,9 +6945,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t 
>> index, uint32_t count,
>>            * So don't set it here for Intel to make Linux guests happy.
>>            */
>>           if (threads_per_pkg > 1) {
>> -            if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 ||
>> -                env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 ||
>> -                env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) {
>> +            if (!IS_INTEL_CPU(env) &&
>> +                !IS_ZHAOXIN_CPU(env) &&
>> +                !IS_VIA_CPU(env)) {
>
> it seems you added ! by mistake.
>
>>                   *ecx |= 1 << 1; /* CmpLegacy bit */
>>               }
>>           }
>
For CPUID leaf 0x80000001 ECX bit 1, Intel defines it as "Bits 04-01: 
Reserved,"
whereas AMD defines it as "CmpLegacy, Core multi-processing legacy 
mode." For Intel
CPUs and those following Intel's behavior, this bit should not be set to 
1. Therefore,
I believe the "!" here is correct.
Re: [PATCH 4/4] target/i386: Update CMPLegacy handling for Zhaoxin and VIA CPUs
Posted by Xiaoyao Li 2 months, 2 weeks ago 
On 7/4/2024 11:14 AM, Ewan Hai wrote:
> On 7/3/24 10:49, Xiaoyao Li wrote:
>>
>> On 6/25/2024 5:19 PM, EwanHai wrote:
>>> Zhaoxin and VIA CPUs handle the CMPLegacy bit in the same way
>>> as Intel CPUs. This patch simplifies the existing logic by
>>> using the IS_XXX_CPU macro and includes checks for Zhaoxin
>>> and VIA vendors to align their behavior with Intel.
>>>
>>> Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
>>> ---
>>>   target/i386/cpu.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>>> index 50edff077e..0836416617 100644
>>> --- a/target/i386/cpu.c
>>> +++ b/target/i386/cpu.c
>>> @@ -6945,9 +6945,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t 
>>> index, uint32_t count,
>>>            * So don't set it here for Intel to make Linux guests happy.
>>>            */
>>>           if (threads_per_pkg > 1) {
>>> -            if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 ||
>>> -                env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 ||
>>> -                env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) {
>>> +            if (!IS_INTEL_CPU(env) &&
>>> +                !IS_ZHAOXIN_CPU(env) &&
>>> +                !IS_VIA_CPU(env)) {
>>
>> it seems you added ! by mistake.
>>
>>>                   *ecx |= 1 << 1; /* CmpLegacy bit */
>>>               }
>>>           }
>>
> For CPUID leaf 0x80000001 ECX bit 1, Intel defines it as "Bits 04-01: 
> Reserved,"
> whereas AMD defines it as "CmpLegacy, Core multi-processing legacy 
> mode." For Intel
> CPUs and those following Intel's behavior, this bit should not be set to 
> 1. Therefore,
> I believe the "!" here is correct.
> 

Sorry, I misread the original code.

I think maybe we can just use is_AMD_CPU(). But I'm not sure if any 
magic use case with customized VENDOR ID relies on it. So you code looks 
good to me.
Re: [PATCH 4/4] target/i386: Update CMPLegacy handling for Zhaoxin and VIA CPUs
Posted by Ewan Hai 2 months, 2 weeks ago 

On 7/3/24 23:19, Xiaoyao Li wrote:
> On 7/4/2024 11:14 AM, Ewan Hai wrote:
>> On 7/3/24 10:49, Xiaoyao Li wrote:
>>> On 6/25/2024 5:19 PM, EwanHai wrote:
>>>> Zhaoxin and VIA CPUs handle the CMPLegacy bit in the same way
>>>> as Intel CPUs. This patch simplifies the existing logic by
>>>> using the IS_XXX_CPU macro and includes checks for Zhaoxin
>>>> and VIA vendors to align their behavior with Intel.
>>>>
>>>> Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
>>>> ---
>>>>   target/i386/cpu.c | 6 +++---
>>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>>>> index 50edff077e..0836416617 100644
>>>> --- a/target/i386/cpu.c
>>>> +++ b/target/i386/cpu.c
>>>> @@ -6945,9 +6945,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
>>>> index, uint32_t count,
>>>>            * So don't set it here for Intel to make Linux guests 
>>>> happy.
>>>>            */
>>>>           if (threads_per_pkg > 1) {
>>>> -            if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 ||
>>>> -                env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 ||
>>>> -                env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) {
>>>> +            if (!IS_INTEL_CPU(env) &&
>>>> +                !IS_ZHAOXIN_CPU(env) &&
>>>> +                !IS_VIA_CPU(env)) {
>>>
>>> it seems you added ! by mistake.
>>>
>>>>                   *ecx |= 1 << 1; /* CmpLegacy bit */
>>>>               }
>>>>           }
>>>
>> For CPUID leaf 0x80000001 ECX bit 1, Intel defines it as "Bits 04-01:
>> Reserved,"
>> whereas AMD defines it as "CmpLegacy, Core multi-processing legacy
>> mode." For Intel
>> CPUs and those following Intel's behavior, this bit should not be set to
>> 1. Therefore,
>> I believe the "!" here is correct.
>>
>
> Sorry, I misread the original code.
>
> I think maybe we can just use is_AMD_CPU(). But I'm not sure if any
> magic use case with customized VENDOR ID relies on it. So you code looks
> good to me.
Ok, thanks. Additionally, in this patch series, I used some VIA terms, which
might cause confusion. I will remove all VIA references in the description
of the next version of the patch.
Currently, the “Centaurhauls” Vendor ID belongs to Zhaoxin CPUs.

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.