From: Xiaoyao Li <xiaoyao.li@intel.com>
Currently, QEMU exposes CPUID 0x1f to guest only when necessary, i.e.,
when topology level that cannot be enumerated by leaf 0xB, e.g., die or
module level, are configured for the guest, e.g., -smp xx,dies=2.
However, TDX architecture forces to require CPUID 0x1f to configure CPU
topology.
Introduce a bool flag, enable_cpuid_0x1f, in CPU for the case that
requires CPUID leaf 0x1f to be exposed to guest.
Introduce a new function x86_has_cpuid_0x1f(), which is the warpper of
cpu->enable_cpuid_0x1f and x86_has_extended_topo() to check if it needs
to enable cpuid leaf 0x1f for the guest.
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
target/i386/cpu.c | 4 ++--
target/i386/cpu.h | 9 +++++++++
target/i386/kvm/kvm.c | 2 +-
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d90e048d48f2..e0716dbe5934 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7292,7 +7292,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
break;
case 0x1F:
/* V2 Extended Topology Enumeration Leaf */
- if (!x86_has_extended_topo(env->avail_cpu_topo)) {
+ if (!x86_has_cpuid_0x1f(cpu)) {
*eax = *ebx = *ecx = *edx = 0;
break;
}
@@ -8178,7 +8178,7 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
* cpu->vendor_cpuid_only has been unset for compatibility with older
* machine types.
*/
- if (x86_has_extended_topo(env->avail_cpu_topo) &&
+ if (x86_has_cpuid_0x1f(cpu) &&
(IS_INTEL_CPU(env) || !cpu->vendor_cpuid_only)) {
x86_cpu_adjust_level(cpu, &env->cpuid_min_level, 0x1F);
}
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 76f24446a55d..3910b488f775 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2251,6 +2251,9 @@ struct ArchCPU {
/* Compatibility bits for old machine types: */
bool enable_cpuid_0xb;
+ /* Force to enable cpuid 0x1f */
+ bool enable_cpuid_0x1f;
+
/* Enable auto level-increase for all CPUID leaves */
bool full_cpuid_auto_level;
@@ -2513,6 +2516,12 @@ void host_cpuid(uint32_t function, uint32_t count,
uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
bool cpu_has_x2apic_feature(CPUX86State *env);
+static inline bool x86_has_cpuid_0x1f(X86CPU *cpu)
+{
+ return cpu->enable_cpuid_0x1f ||
+ x86_has_extended_topo(cpu->env.avail_cpu_topo);
+}
+
/* helper.c */
void x86_cpu_set_a20(X86CPU *cpu, int a20_state);
void cpu_sync_avx_hflag(CPUX86State *env);
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 6c749d4ee812..23b8de308525 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1863,7 +1863,7 @@ static uint32_t kvm_x86_build_cpuid(CPUX86State *env,
break;
}
case 0x1f:
- if (!x86_has_extended_topo(env->avail_cpu_topo)) {
+ if (!x86_has_cpuid_0x1f(env_archcpu(env))) {
cpuid_i--;
break;
}
--
2.34.1On Wed, 23 Apr 2025 19:46:58 +0800
Zhao Liu <zhao1.liu@intel.com> wrote:
> From: Xiaoyao Li <xiaoyao.li@intel.com>
>
> Currently, QEMU exposes CPUID 0x1f to guest only when necessary, i.e.,
> when topology level that cannot be enumerated by leaf 0xB, e.g., die or
> module level, are configured for the guest, e.g., -smp xx,dies=2.
>
> However, TDX architecture forces to require CPUID 0x1f to configure CPU
> topology.
>
> Introduce a bool flag, enable_cpuid_0x1f, in CPU for the case that
> requires CPUID leaf 0x1f to be exposed to guest.
>
> Introduce a new function x86_has_cpuid_0x1f(), which is the warpper of
> cpu->enable_cpuid_0x1f and x86_has_extended_topo() to check if it needs
> to enable cpuid leaf 0x1f for the guest.
that reminds me about recent attempt to remove enable_cpuid_0xb,
So is enable_cpuid_0x1f inteded to be used by external users or
it's internal only knob for TDX sake?
I'd push for it being marked as internal|unstable with the means
we currently have (i.e. adding 'x-' prefix)
and also enable_ is not right here, the leaf is enabled when
topology requires it.
perhaps s/enable_/force_/
>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> target/i386/cpu.c | 4 ++--
> target/i386/cpu.h | 9 +++++++++
> target/i386/kvm/kvm.c | 2 +-
> 3 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index d90e048d48f2..e0716dbe5934 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7292,7 +7292,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> break;
> case 0x1F:
> /* V2 Extended Topology Enumeration Leaf */
> - if (!x86_has_extended_topo(env->avail_cpu_topo)) {
> + if (!x86_has_cpuid_0x1f(cpu)) {
> *eax = *ebx = *ecx = *edx = 0;
> break;
> }
> @@ -8178,7 +8178,7 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
> * cpu->vendor_cpuid_only has been unset for compatibility with older
> * machine types.
> */
> - if (x86_has_extended_topo(env->avail_cpu_topo) &&
> + if (x86_has_cpuid_0x1f(cpu) &&
> (IS_INTEL_CPU(env) || !cpu->vendor_cpuid_only)) {
> x86_cpu_adjust_level(cpu, &env->cpuid_min_level, 0x1F);
> }
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 76f24446a55d..3910b488f775 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2251,6 +2251,9 @@ struct ArchCPU {
> /* Compatibility bits for old machine types: */
> bool enable_cpuid_0xb;
>
> + /* Force to enable cpuid 0x1f */
> + bool enable_cpuid_0x1f;
> +
> /* Enable auto level-increase for all CPUID leaves */
> bool full_cpuid_auto_level;
>
> @@ -2513,6 +2516,12 @@ void host_cpuid(uint32_t function, uint32_t count,
> uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
> bool cpu_has_x2apic_feature(CPUX86State *env);
>
> +static inline bool x86_has_cpuid_0x1f(X86CPU *cpu)
> +{
> + return cpu->enable_cpuid_0x1f ||
> + x86_has_extended_topo(cpu->env.avail_cpu_topo);
> +}
> +
> /* helper.c */
> void x86_cpu_set_a20(X86CPU *cpu, int a20_state);
> void cpu_sync_avx_hflag(CPUX86State *env);
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 6c749d4ee812..23b8de308525 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -1863,7 +1863,7 @@ static uint32_t kvm_x86_build_cpuid(CPUX86State *env,
> break;
> }
> case 0x1f:
> - if (!x86_has_extended_topo(env->avail_cpu_topo)) {
> + if (!x86_has_cpuid_0x1f(env_archcpu(env))) {
> cpuid_i--;
> break;
> }
Hi Igor, thanks for your review! On Tue, May 13, 2025 at 02:45:15PM +0200, Igor Mammedov wrote: > Date: Tue, 13 May 2025 14:45:15 +0200 > From: Igor Mammedov <imammedo@redhat.com> > Subject: Re: [RFC 06/10] i386/cpu: Introduce enable_cpuid_0x1f to force > exposing CPUID 0x1f > X-Mailer: Claws Mail 4.3.1 (GTK 3.24.43; x86_64-redhat-linux-gnu) > > On Wed, 23 Apr 2025 19:46:58 +0800 > Zhao Liu <zhao1.liu@intel.com> wrote: > > > From: Xiaoyao Li <xiaoyao.li@intel.com> > > > > Currently, QEMU exposes CPUID 0x1f to guest only when necessary, i.e., > > when topology level that cannot be enumerated by leaf 0xB, e.g., die or > > module level, are configured for the guest, e.g., -smp xx,dies=2. > > > > However, TDX architecture forces to require CPUID 0x1f to configure CPU > > topology. > > > > Introduce a bool flag, enable_cpuid_0x1f, in CPU for the case that > > requires CPUID leaf 0x1f to be exposed to guest. > > > > Introduce a new function x86_has_cpuid_0x1f(), which is the warpper of > > cpu->enable_cpuid_0x1f and x86_has_extended_topo() to check if it needs > > to enable cpuid leaf 0x1f for the guest. > > that reminds me about recent attempt to remove enable_cpuid_0xb, > > So is enable_cpuid_0x1f inteded to be used by external users or > it's internal only knob for TDX sake? TDX needs this and I also try to apply this to named CPU models. For max/host CPUs, there are no explicit use cases. I think it's enough to make named CPU models have 0x1f. Then this should be only used internally. > I'd push for it being marked as internal|unstable with the means > we currently have (i.e. adding 'x-' prefix) Sure, 'x-' is good. (If there is the internal property in the future, I can also convert this unsatble property into internal one.) This patch is picked from the TDX series, and in this patch Xiaoyao doesn't make 0x1f enabling as property. In the next patch (RFC patch 7), I add the "cpuid-0x1f" property. I'll rename that property as "x-cpuid-0x1f". > and also enable_ is not right here, the leaf is enabled when > topology requires it. > perhaps s/enable_/force_/ Thanks, I agree force_cpuid_0x1f is a better name. @Xiaoyao, do you agree with this idea? But probably TDX won't have v10 though, I can rename the field in my v2 after TDX. Regards, Zhao
On 5/14/2025 11:23 PM, Zhao Liu wrote: > Hi Igor, thanks for your review! > > On Tue, May 13, 2025 at 02:45:15PM +0200, Igor Mammedov wrote: >> Date: Tue, 13 May 2025 14:45:15 +0200 >> From: Igor Mammedov <imammedo@redhat.com> >> Subject: Re: [RFC 06/10] i386/cpu: Introduce enable_cpuid_0x1f to force >> exposing CPUID 0x1f >> X-Mailer: Claws Mail 4.3.1 (GTK 3.24.43; x86_64-redhat-linux-gnu) >> >> On Wed, 23 Apr 2025 19:46:58 +0800 >> Zhao Liu <zhao1.liu@intel.com> wrote: >> >>> From: Xiaoyao Li <xiaoyao.li@intel.com> >>> >>> Currently, QEMU exposes CPUID 0x1f to guest only when necessary, i.e., >>> when topology level that cannot be enumerated by leaf 0xB, e.g., die or >>> module level, are configured for the guest, e.g., -smp xx,dies=2. >>> >>> However, TDX architecture forces to require CPUID 0x1f to configure CPU >>> topology. >>> >>> Introduce a bool flag, enable_cpuid_0x1f, in CPU for the case that >>> requires CPUID leaf 0x1f to be exposed to guest. >>> >>> Introduce a new function x86_has_cpuid_0x1f(), which is the warpper of >>> cpu->enable_cpuid_0x1f and x86_has_extended_topo() to check if it needs >>> to enable cpuid leaf 0x1f for the guest. >> >> that reminds me about recent attempt to remove enable_cpuid_0xb, >> >> So is enable_cpuid_0x1f inteded to be used by external users or >> it's internal only knob for TDX sake? > > TDX needs this and I also try to apply this to named CPU models. For > max/host CPUs, there are no explicit use cases. I think it's enough to > make named CPU models have 0x1f. > > Then this should be only used internally. > >> I'd push for it being marked as internal|unstable with the means >> we currently have (i.e. adding 'x-' prefix) > > Sure, 'x-' is good. (If there is the internal property in the future, > I can also convert this unsatble property into internal one.) > > This patch is picked from the TDX series, and in this patch Xiaoyao > doesn't make 0x1f enabling as property. In the next patch (RFC patch 7), > I add the "cpuid-0x1f" property. I'll rename that property as > "x-cpuid-0x1f". > >> and also enable_ is not right here, the leaf is enabled when >> topology requires it. >> perhaps s/enable_/force_/ > > Thanks, I agree force_cpuid_0x1f is a better name. > > @Xiaoyao, do you agree with this idea? > > But probably TDX won't have v10 though, I can rename the field in my v2 > after TDX. I'm OK with it. The TDX series won't be merged by Paolo soon. It has to be after the KVM part being in linux v6.16-rc1, i.e., about one month later. And there are rebase conflicts when I rebase the TDX-QEMU series against upstream QEMU master daily. It seems to have a newer version of TDX-QEMU when it's going to be picked up by Paolo for Paolo's convenience. If a v10 needed, I can rename in it. Let's see how it goes. > Regards, > Zhao >
© 2016 - 2025 Red Hat, Inc.