linux-kernel - Re: [PATCH] KVM: x86: expose CPUID 0xC000

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ed29e030-63f2-493f-af74-d1d0e1fb09e4@zhaoxin.com>
Date: Wed, 13 Aug 2025 04:38:26 -0400
From: Ewan Hai <ewanhai-oc@...oxin.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: <pbonzini@...hat.com>, <tglx@...utronix.de>, <mingo@...hat.com>,
	<bp@...en8.de>, <dave.hansen@...ux.intel.com>, <hpa@...or.com>,
	<x86@...nel.org>, <kvm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<ewanhai@...oxin.com>, <cobechen@...oxin.com>, <leoliu@...oxin.com>,
	<lyleli@...oxin.com>
Subject: Re: [PATCH] KVM: x86: expose CPUID 0xC000_0000 for Zhaoxin "Shanghai"
 vendor



On 8/12/25 11:07, Sean Christopherson wrote:
> On Sun, Aug 10, 2025, Ewan Hai wrote:
>> rename the local constant CENTAUR_CPUID_SIGNATURE to ZHAOXIN_CPUID_SIGNATURE.
> Why?  I'm not inclined to rename any of the Centaur references, as I don't see
> any point in effectively rewriting history.  If we elect to rename things, then
> it needs to be done in a separate patch, there needs to be proper justification,
> and _all_ references should be converted, e.g. converting just this one macro
> creates discrepancies even with cpuid.c, as there are multiple comments that
> specifically talk about Centaur CPUID leaves.
>
Okay, it seems I oversimplified the situation.

My initial thought was that, since there will no longer be separate handling for
"Centaurhauls," nearly all new software and hardware features will be applied to
both "Centaurhauls" and "  Shanghai  " vendors in parallel. This would gradually
lead to more and more occurrences of if (vendor == centaur || vendor ==
shanghai) in the kernel code. In that case, introducing an is_zhaoxin_vendor()
helper could significantly reduce the number of repetitive if (xx || yy) checks.

However, it appears that this "duplication issue" is not a real concern for now.
We can revisit it later when it becomes a practical problem.

For the current matter, there are two possible approaches. Which one do you
prefer? Or, if you have other suggestions, please let me know and I will
incorporate your recommendation into the v2 patch.

## Version 1 ##
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1820,7 +1820,8 @@ static int get_cpuid_func(struct kvm_cpuid_array *array, u32 func,
         int r;

         if (func == CENTAUR_CPUID_SIGNATURE &&
-           boot_cpu_data.x86_vendor != X86_VENDOR_CENTAUR)
+           boot_cpu_data.x86_vendor != X86_VENDOR_CENTAUR &&
+           boot_cpu_data.x86_vendor != X86_VENDOR_ZHAOXIN)
                 return 0;

         r = do_cpuid_func(array, func, type);

## Version 2 ##
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1812,6 +1812,7 @@ static int do_cpuid_func(struct kvm_cpuid_array *array, u32 func,
  }

  #define CENTAUR_CPUID_SIGNATURE 0xC0000000
+#define ZHAOXIN_CPUID_SIGNATURE 0xC0000000

  static int get_cpuid_func(struct kvm_cpuid_array *array, u32 func,
                           unsigned int type)
@@ -1819,8 +1820,10 @@ static int get_cpuid_func(struct kvm_cpuid_array *array, u32 func,
         u32 limit;
         int r;

-       if (func == CENTAUR_CPUID_SIGNATURE &&
-           boot_cpu_data.x86_vendor != X86_VENDOR_CENTAUR)
+       if ((func == CENTAUR_CPUID_SIGNATURE &&
+            boot_cpu_data.x86_vendor != X86_VENDOR_CENTAUR) ||
+           (func == ZHAOXIN_CPUID_SIGNATURE &&
+            boot_cpu_data.x86_vendor != X86_VENDOR_ZHAOXIN))
                 return 0;

         r = do_cpuid_func(array, func, type);

>> The constant is used only inside cpuid.c, so the rename is NFC outside this
>> file.
>>
>> Signed-off-by: Ewan Hai <ewanhai-oc@...oxin.com>
>> ---
>>   arch/x86/kvm/cpuid.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> index e2836a255b16..beb83eaa1868 100644
>> --- a/arch/x86/kvm/cpuid.c
>> +++ b/arch/x86/kvm/cpuid.c
>> @@ -1811,7 +1811,7 @@ static int do_cpuid_func(struct kvm_cpuid_array *array, u32 func,
>>        return __do_cpuid_func(array, func);
>>   }
>>
>> -#define CENTAUR_CPUID_SIGNATURE 0xC0000000
>> +#define ZHAOXIN_CPUID_SIGNATURE 0xC0000000
>>
>>   static int get_cpuid_func(struct kvm_cpuid_array *array, u32 func,
>>                          unsigned int type)
>> @@ -1819,8 +1819,9 @@ static int get_cpuid_func(struct kvm_cpuid_array *array, u32 func,
>>        u32 limit;
>>        int r;
>>
>> -     if (func == CENTAUR_CPUID_SIGNATURE &&
>> -         boot_cpu_data.x86_vendor != X86_VENDOR_CENTAUR)
>> +     if (func == ZHAOXIN_CPUID_SIGNATURE &&
>> +             boot_cpu_data.x86_vendor != X86_VENDOR_CENTAUR &&
>> +             boot_cpu_data.x86_vendor != X86_VENDOR_ZHAOXIN)
> Align indentation.

Will fix in v2.

>
>          if (func == CENTAUR_CPUID_SIGNATURE &&
>              boot_cpu_data.x86_vendor != X86_VENDOR_CENTAUR &&
>              boot_cpu_data.x86_vendor != X86_VENDOR_ZHAOXIN)
>                  return 0;
>
>>                return 0;
>>
>>        r = do_cpuid_func(array, func, type);
>> @@ -1869,7 +1870,7 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
>>                            unsigned int type)
>>   {
>>        static const u32 funcs[] = {
>> -             0, 0x80000000, CENTAUR_CPUID_SIGNATURE, KVM_CPUID_SIGNATURE,
>> +             0, 0x80000000, ZHAOXIN_CPUID_SIGNATURE, KVM_CPUID_SIGNATURE,
>>        };
>>
>>        struct kvm_cpuid_array array = {
>> --
>> 2.34.1
>>