All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] target/i386: suppress CPUID leaves not defined by the CPU vendor
@ 2021-07-08  0:36 Michael Roth
  2021-07-08 17:06 ` [PATCH] target/i386: Fix cpuid level for AMD Michael Roth
  2021-07-08 21:05 ` [PATCH] target/i386: suppress CPUID leaves not defined by the CPU vendor Eduardo Habkost
  0 siblings, 2 replies; 4+ messages in thread
From: Michael Roth @ 2021-07-08  0:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Eduardo Habkost, Richard Henderson,
	Igor Mammedov, zhenwei pi

Currently all built-in CPUs report cache information via CPUID leaves 2
and 4, but these have never been defined for AMD. In the case of
SEV-SNP this can cause issues with CPUID enforcement. Address this by
allowing CPU types to suppress these via a new "x-vendor-cpuid-only"
CPU property, which is true by default, but switched off for older
machine types to maintain compatibility.

Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: zhenwei pi <pizhenwei@bytedance.com>
Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 hw/i386/pc.c      | 1 +
 target/i386/cpu.c | 6 ++++++
 target/i386/cpu.h | 3 +++
 3 files changed, 10 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8e1220db72..aa79c5e0e6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_0[] = {
     { "qemu64" "-" TYPE_X86_CPU, "family", "6" },
     { "qemu64" "-" TYPE_X86_CPU, "model", "6" },
     { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
+    { TYPE_X86_CPU, "x-vendor-cpuid-only", "off" },
 };
 const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 5f595a0d7e..0b6c921e5a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5155,6 +5155,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         if (cpu->cache_info_passthrough) {
             host_cpuid(index, 0, eax, ebx, ecx, edx);
             break;
+        } else if (cpu->vendor_cpuid_only && IS_AMD_CPU(env)) {
+            *eax = *ebx = *ecx = *edx = 0;
+            break;
         }
         *eax = 1; /* Number of CPUID[EAX=2] calls required */
         *ebx = 0;
@@ -5176,6 +5179,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             if ((*eax & 31) && cs->nr_cores > 1) {
                 *eax |= (cs->nr_cores - 1) << 26;
             }
+        } else if (cpu->vendor_cpuid_only && IS_AMD_CPU(env)) {
+            *eax = *ebx = *ecx = *edx = 0;
         } else {
             *eax = 0;
             switch (count) {
@@ -6647,6 +6652,7 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("full-cpuid-auto-level", X86CPU, full_cpuid_auto_level, true),
     DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor),
     DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
+    DEFINE_PROP_BOOL("x-vendor-cpuid-only", X86CPU, vendor_cpuid_only, true),
     DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false),
     DEFINE_PROP_BOOL("l3-cache", X86CPU, enable_l3_cache, true),
     DEFINE_PROP_BOOL("kvm-no-smi-migration", X86CPU, kvm_no_smi_migration,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 0f7ddbfeae..c4152634e7 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1746,6 +1746,9 @@ struct X86CPU {
     /* Enable auto level-increase for all CPUID leaves */
     bool full_cpuid_auto_level;
 
+    /* Only advertise CPUID leaves defined by the vendor */
+    bool vendor_cpuid_only;
+
     /* Enable auto level-increase for Intel Processor Trace leave */
     bool intel_pt_auto_level;
 
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] target/i386: Fix cpuid level for AMD
  2021-07-08  0:36 [PATCH] target/i386: suppress CPUID leaves not defined by the CPU vendor Michael Roth
@ 2021-07-08 17:06 ` Michael Roth
  2021-07-08 21:05   ` Eduardo Habkost
  2021-07-08 21:05 ` [PATCH] target/i386: suppress CPUID leaves not defined by the CPU vendor Eduardo Habkost
  1 sibling, 1 reply; 4+ messages in thread
From: Michael Roth @ 2021-07-08 17:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: zhenwei pi, Dr. David Alan Gilbert, Eduardo Habkost,
	Richard Henderson, Igor Mammedov

From: zhenwei pi <pizhenwei@bytedance.com>

A AMD server typically has cpuid level 0x10(test on Rome/Milan), it
should not be changed to 0x1f in multi-dies case.

Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: zhenwei pi <pizhenwei@bytedance.com>
Fixes: a94e1428991 (target/i386: Add CPUID.1F generation support for multi-dies PCMachine)
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
* to maintain compatibility with older machine types, only implement
  this change when the CPU's "x-vendor-cpuid-only" property is false
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 target/i386/cpu.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0b6c921e5a..289dd2b1d8 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5950,8 +5950,15 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
             }
         }
 
-        /* CPU topology with multi-dies support requires CPUID[0x1F] */
-        if (env->nr_dies > 1) {
+        /*
+         * Intel CPU topology with multi-dies support requires CPUID[0x1F].
+         * For AMD Rome/Milan, cpuid level is 0x10, and guest OS should detect
+         * extended toplogy by leaf 0xB. Only adjust it for Intel CPU, unless
+         * cpu->vendor_cpuid_only has been unset for compatibility with older
+         * machine types.
+         */
+        if ((env->nr_dies > 1) &&
+            (IS_INTEL_CPU(env) || !cpu->vendor_cpuid_only)) {
             x86_cpu_adjust_level(cpu, &env->cpuid_min_level, 0x1F);
         }
 
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] target/i386: suppress CPUID leaves not defined by the CPU vendor
  2021-07-08  0:36 [PATCH] target/i386: suppress CPUID leaves not defined by the CPU vendor Michael Roth
  2021-07-08 17:06 ` [PATCH] target/i386: Fix cpuid level for AMD Michael Roth
@ 2021-07-08 21:05 ` Eduardo Habkost
  1 sibling, 0 replies; 4+ messages in thread
From: Eduardo Habkost @ 2021-07-08 21:05 UTC (permalink / raw)
  To: Michael Roth
  Cc: Igor Mammedov, Richard Henderson, qemu-devel, zhenwei pi,
	Dr. David Alan Gilbert

On Wed, Jul 07, 2021 at 07:36:23PM -0500, Michael Roth wrote:
> Currently all built-in CPUs report cache information via CPUID leaves 2
> and 4, but these have never been defined for AMD. In the case of
> SEV-SNP this can cause issues with CPUID enforcement. Address this by
> allowing CPU types to suppress these via a new "x-vendor-cpuid-only"
> CPU property, which is true by default, but switched off for older
> machine types to maintain compatibility.
> 
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: zhenwei pi <pizhenwei@bytedance.com>
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>

Queued, thanks!

-- 
Eduardo



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] target/i386: Fix cpuid level for AMD
  2021-07-08 17:06 ` [PATCH] target/i386: Fix cpuid level for AMD Michael Roth
@ 2021-07-08 21:05   ` Eduardo Habkost
  0 siblings, 0 replies; 4+ messages in thread
From: Eduardo Habkost @ 2021-07-08 21:05 UTC (permalink / raw)
  To: Michael Roth
  Cc: Igor Mammedov, Richard Henderson, qemu-devel, zhenwei pi,
	Dr. David Alan Gilbert

On Thu, Jul 08, 2021 at 12:06:41PM -0500, Michael Roth wrote:
> From: zhenwei pi <pizhenwei@bytedance.com>
> 
> A AMD server typically has cpuid level 0x10(test on Rome/Milan), it
> should not be changed to 0x1f in multi-dies case.
> 
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: zhenwei pi <pizhenwei@bytedance.com>
> Fixes: a94e1428991 (target/i386: Add CPUID.1F generation support for multi-dies PCMachine)
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> * to maintain compatibility with older machine types, only implement
>   this change when the CPU's "x-vendor-cpuid-only" property is false
> Signed-off-by: Michael Roth <michael.roth@amd.com>

Queued, thanks!

-- 
Eduardo



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-07-08 21:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-08  0:36 [PATCH] target/i386: suppress CPUID leaves not defined by the CPU vendor Michael Roth
2021-07-08 17:06 ` [PATCH] target/i386: Fix cpuid level for AMD Michael Roth
2021-07-08 21:05   ` Eduardo Habkost
2021-07-08 21:05 ` [PATCH] target/i386: suppress CPUID leaves not defined by the CPU vendor Eduardo Habkost

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.