[PATCH v2 0/3] i386: Minor fixes of building CPUIDs

[PATCH v2 0/3] i386: Minor fixes of building CPUIDs

[Xiaoyao Li]

This v2 adds patch 3 to fix the build of CPUID leaf 7.

The issue that fixed by Patch 1 looks fatal though it doesn't appear on
KVM because KVM always searches with assending order and hit with the
correct cpuid leaf 0.

Patch 2 removes the wrong constraint on CPUID leaf 1f.

Changes in v2:
- Add Patch 3;
- rebase to latest master branch
v1: https://lore.kernel.org/qemu-devel/20220712021249.3227256-1-xiaoyao.li@intel.com/

Xiaoyao Li (3):
  i386/cpuid: Decrease cpuid_i when skipping CPUID leaf 1F
  i386/cpuid: Remove subleaf constraint on CPUID leaf 1F
  i386/cpuid: Move leaf 7 to correct group

 target/i386/kvm/kvm.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)


base-commit: fdd0df5340a8ebc8de88078387ebc85c5af7b40f
-- 
2.34.1

[PATCH v2 1/3] i386/cpuid: Decrease cpuid_i when skipping CPUID leaf 1F

[Xiaoyao Li]

Decrease array index cpuid_i when CPUID leaf 1F is skipped, otherwise it
will get an all zero'ed CPUID entry with leaf 0 and subleaf 0. It
conflicts with correct leaf 0.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/kvm/kvm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index de531842f6b1..afa97799d89a 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1956,6 +1956,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
         }
         case 0x1f:
             if (env->nr_dies < 2) {
+                cpuid_i--;
                 break;
             }
             /* fallthrough */
-- 
2.34.1

Re: [PATCH v2 1/3] i386/cpuid: Decrease cpuid_i when skipping CPUID leaf 1F

[Yang, Weijiang]

On 6/13/2023 9:19 PM, Xiaoyao Li wrote:
> Decrease array index cpuid_i when CPUID leaf 1F is skipped, otherwise it
> will get an all zero'ed CPUID entry with leaf 0 and subleaf 0. It
> conflicts with correct leaf 0.

Maybe change the commit log like this:

Exiting code misses a decrement of cpuid_i when skip left 0x1F, so 
there's a blank CPUID

entry(with all fields stuffed 0s) left in the CPUID array.  Fix the 
issue to avoid the blank slot.

Reviewed-by:Yang Weijiang <weijiang.yang@intel.com>

>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>   target/i386/kvm/kvm.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index de531842f6b1..afa97799d89a 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -1956,6 +1956,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>           }
>           case 0x1f:
>               if (env->nr_dies < 2) {
> +                cpuid_i--;
>                   break;
>               }
>               /* fallthrough */

[PATCH v2 2/3] i386/cpuid: Remove subleaf constraint on CPUID leaf 1F

[Xiaoyao Li]

No such constraint that subleaf index needs to be less than 64.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/kvm/kvm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index afa97799d89a..d7e235ce35a6 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1968,10 +1968,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
                     break;
                 }
 
-                if (i == 0x1f && j == 64) {
-                    break;
-                }
-
                 c->function = i;
                 c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
                 c->index = j;
-- 
2.34.1

Re: [PATCH v2 2/3] i386/cpuid: Remove subleaf constraint on CPUID leaf 1F

[Yang, Weijiang]

On 6/13/2023 9:19 PM, Xiaoyao Li wrote:

> No such constraint that subleaf index needs to be less than 64.
>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>   target/i386/kvm/kvm.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index afa97799d89a..d7e235ce35a6 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -1968,10 +1968,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
>                       break;
>                   }
>   
> -                if (i == 0x1f && j == 64) {
> -                    break;
> -                }
> -
>                   c->function = i;
>                   c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>                   c->index = j;
Reviewed-by:Yang Weijiang <weijiang.yang@intel.com>

[PATCH v2 3/3] i386/cpuid: Move leaf 7 to correct group

[Xiaoyao Li]

CPUID leaf 7 was grouped together with SGX leaf 0x12 by commit
b9edbadefb9e ("i386: Propagate SGX CPUID sub-leafs to KVM") by mistake.

SGX leaf 0x12 has its specific logic to check if subleaf (starting from 2)
is valid or not by checking the bit 0:3 of corresponding EAX is 1 or
not.

Leaf 7 follows the logic that EAX of subleaf 0 enumerates the maximum
valid subleaf.

Fixes: b9edbadefb9e ("i386: Propagate SGX CPUID sub-leafs to KVM")
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/kvm/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index d7e235ce35a6..86aab9ca4ba2 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1993,7 +1993,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
                 c = &cpuid_data.entries[cpuid_i++];
             }
             break;
-        case 0x7:
         case 0x12:
             for (j = 0; ; j++) {
                 c->function = i;
@@ -2013,6 +2012,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
                 c = &cpuid_data.entries[cpuid_i++];
             }
             break;
+        case 0x7:
         case 0x14:
         case 0x1d:
         case 0x1e: {
-- 
2.34.1

Re: [PATCH v2 3/3] i386/cpuid: Move leaf 7 to correct group

[Yang, Weijiang]

On 6/13/2023 9:19 PM, Xiaoyao Li wrote:
> CPUID leaf 7 was grouped together with SGX leaf 0x12 by commit
> b9edbadefb9e ("i386: Propagate SGX CPUID sub-leafs to KVM") by mistake.
>
> SGX leaf 0x12 has its specific logic to check if subleaf (starting from 2)
> is valid or not by checking the bit 0:3 of corresponding EAX is 1 or
> not.
>
> Leaf 7 follows the logic that EAX of subleaf 0 enumerates the maximum
> valid subleaf.
>
> Fixes: b9edbadefb9e ("i386: Propagate SGX CPUID sub-leafs to KVM")
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>   target/i386/kvm/kvm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index d7e235ce35a6..86aab9ca4ba2 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -1993,7 +1993,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
>                   c = &cpuid_data.entries[cpuid_i++];
>               }
>               break;
> -        case 0x7:
>           case 0x12:
>               for (j = 0; ; j++) {
>                   c->function = i;
> @@ -2013,6 +2012,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>                   c = &cpuid_data.entries[cpuid_i++];
>               }
>               break;
> +        case 0x7:
>           case 0x14:
>           case 0x1d:
>           case 0x1e: {

Reviewed-by:Yang Weijiang <weijiang.yang@intel.com>