QEMU-Devel Archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: Babu Moger <babu.moger@amd.com>,
	qemu-stable@nongnu.org, Zhao Liu <zhao1.liu@intel.com>
Subject: [PULL 01/26] target/i386: Fix CPUID encoding of Fn8000001E_ECX
Date: Tue,  7 May 2024 12:55:13 +0200	[thread overview]
Message-ID: <20240507105538.180704-2-pbonzini@redhat.com> (raw)
In-Reply-To: <20240507105538.180704-1-pbonzini@redhat.com>

From: Babu Moger <babu.moger@amd.com>

Observed the following failure while booting the SEV-SNP guest and the
guest fails to boot with the smp parameters:
"-smp 192,sockets=1,dies=12,cores=8,threads=2".

qemu-system-x86_64: sev_snp_launch_update: SNP_LAUNCH_UPDATE ret=-5 fw_error=22 'Invalid parameter'
qemu-system-x86_64: SEV-SNP: CPUID validation failed for function 0x8000001e, index: 0x0.
provided: eax:0x00000000, ebx: 0x00000100, ecx: 0x00000b00, edx: 0x00000000
expected: eax:0x00000000, ebx: 0x00000100, ecx: 0x00000300, edx: 0x00000000
qemu-system-x86_64: SEV-SNP: failed update CPUID page

Reason for the failure is due to overflowing of bits used for "Node per
processor" in CPUID Fn8000001E_ECX. This field's width is 3 bits wide and
can hold maximum value 0x7. With dies=12 (0xB), it overflows and spills
over into the reserved bits. In the case of SEV-SNP, this causes CPUID
enforcement failure and guest fails to boot.

The PPR documentation for CPUID_Fn8000001E_ECX [Node Identifiers]
=================================================================
Bits    Description
31:11   Reserved.

10:8    NodesPerProcessor: Node per processor. Read-only.
        ValidValues:
        Value   Description
        0h      1 node per processor.
        7h-1h   Reserved.

7:0     NodeId: Node ID. Read-only. Reset: Fixed,XXh.
=================================================================

As in the spec, the valid value for "node per processor" is 0 and rest
are reserved.

Looking back at the history of decoding of CPUID_Fn8000001E_ECX, noticed
that there were cases where "node per processor" can be more than 1. It
is valid only for pre-F17h (pre-EPYC) architectures. For EPYC or later
CPUs, the linux kernel does not use this information to build the L3
topology.

Also noted that the CPUID Function 0x8000001E_ECX is available only when
TOPOEXT feature is enabled. This feature is enabled only for EPYC(F17h)
or later processors. So, previous generation of processors do not not
enumerate 0x8000001E_ECX leaf.

There could be some corner cases where the older guests could enable the
TOPOEXT feature by running with -cpu host, in which case legacy guests
might notice the topology change. To address those cases introduced a
new CPU property "legacy-multi-node". It will be true for older machine
types to maintain compatibility. By default, it will be false, so new
decoding will be used going forward.

The documentation is taken from Preliminary Processor Programming
Reference (PPR) for AMD Family 19h Model 11h, Revision B1 Processors 55901
Rev 0.25 - Oct 6, 2022.

Cc: qemu-stable@nongnu.org
Fixes: 31ada106d891 ("Simplify CPUID_8000_001E for AMD")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
Message-ID: <0ee4b0a8293188a53970a2b0e4f4ef713425055e.1714757834.git.babu.moger@amd.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.h |  6 ++++++
 hw/i386/pc.c      |  1 +
 target/i386/cpu.c | 18 ++++++++++--------
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 565c7a98c37..1e0d2c915f5 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1994,6 +1994,12 @@ struct ArchCPU {
      */
     bool legacy_cache;
 
+    /* Compatibility bits for old machine types.
+     * If true decode the CPUID Function 0x8000001E_ECX to support multiple
+     * nodes per processor
+     */
+    bool legacy_multi_node;
+
     /* Compatibility bits for old machine types: */
     bool enable_cpuid_0xb;
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 08c7de416fd..46235466d7a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -81,6 +81,7 @@
 GlobalProperty pc_compat_9_0[] = {
     { TYPE_X86_CPU, "guest-phys-bits", "0" },
     { "sev-guest", "legacy-vm-type", "true" },
+    { TYPE_X86_CPU, "legacy-multi-node", "on" },
 };
 const size_t pc_compat_9_0_len = G_N_ELEMENTS(pc_compat_9_0);
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3ef30a765c1..1058b6803fd 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -398,12 +398,9 @@ static void encode_topo_cpuid8000001e(X86CPU *cpu, X86CPUTopoInfo *topo_info,
      * 31:11 Reserved.
      * 10:8 NodesPerProcessor: Node per processor. Read-only. Reset: XXXb.
      *      ValidValues:
-     *      Value Description
-     *      000b  1 node per processor.
-     *      001b  2 nodes per processor.
-     *      010b Reserved.
-     *      011b 4 nodes per processor.
-     *      111b-100b Reserved.
+     *      Value   Description
+     *      0h      1 node per processor.
+     *      7h-1h   Reserved.
      *  7:0 NodeId: Node ID. Read-only. Reset: XXh.
      *
      * NOTE: Hardware reserves 3 bits for number of nodes per processor.
@@ -412,8 +409,12 @@ static void encode_topo_cpuid8000001e(X86CPU *cpu, X86CPUTopoInfo *topo_info,
      * NodeId is combination of node and socket_id which is already decoded
      * in apic_id. Just use it by shifting.
      */
-    *ecx = ((topo_info->dies_per_pkg - 1) << 8) |
-           ((cpu->apic_id >> apicid_die_offset(topo_info)) & 0xFF);
+    if (cpu->legacy_multi_node) {
+        *ecx = ((topo_info->dies_per_pkg - 1) << 8) |
+               ((cpu->apic_id >> apicid_die_offset(topo_info)) & 0xFF);
+    } else {
+        *ecx = (cpu->apic_id >> apicid_pkg_offset(topo_info)) & 0xFF;
+    }
 
     *edx = 0;
 }
@@ -8084,6 +8085,7 @@ static Property x86_cpu_properties[] = {
      * own cache information (see x86_cpu_load_def()).
      */
     DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true),
+    DEFINE_PROP_BOOL("legacy-multi-node", X86CPU, legacy_multi_node, false),
     DEFINE_PROP_BOOL("xen-vapic", X86CPU, xen_vapic, false),
 
     /*
-- 
2.45.0



  reply	other threads:[~2024-05-07 10:56 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-07 10:55 [PULL 00/26] target/i386 changes for 2024-05-07 Paolo Bonzini
2024-05-07 10:55 ` Paolo Bonzini [this message]
2024-05-07 10:55 ` [PULL 02/26] target/i386: use TSTEQ/TSTNE to test low bits Paolo Bonzini
2024-05-07 10:55 ` [PULL 03/26] target/i386: use TSTEQ/TSTNE to check flags Paolo Bonzini
2024-05-07 10:55 ` [PULL 04/26] target/i386: remove mask from CCPrepare Paolo Bonzini
2024-05-07 10:55 ` [PULL 05/26] target/i386: cc_op is not dynamic in gen_jcc1 Paolo Bonzini
2024-05-07 10:55 ` [PULL 06/26] target/i386: cleanup cc_op changes for REP/REPZ/REPNZ Paolo Bonzini
2024-05-07 10:55 ` [PULL 07/26] target/i386: pull cc_op update to callers of gen_jmp_rel{, _csize} Paolo Bonzini
2024-05-07 10:55 ` [PULL 08/26] target/i386: extend cc_* when using them to compute flags Paolo Bonzini
2024-05-07 10:55 ` [PULL 09/26] target/i386: do not use s->T0 and s->T1 as scratch registers for CCPrepare Paolo Bonzini
2024-05-07 10:55 ` [PULL 10/26] target/i386: clarify the "reg" argument of functions returning CCPrepare Paolo Bonzini
2024-05-07 10:55 ` [PULL 11/26] target/i386: cleanup *gen_eob* Paolo Bonzini
2024-05-07 10:55 ` [PULL 12/26] target/i386: reintroduce debugging mechanism Paolo Bonzini
2024-05-07 10:55 ` [PULL 13/26] target/i386: move 00-5F opcodes to new decoder Paolo Bonzini
2024-05-07 10:55 ` [PULL 14/26] target/i386: extract gen_far_call/jmp, reordering temporaries Paolo Bonzini
2024-05-07 10:55 ` [PULL 15/26] target/i386: allow instructions with more than one immediate Paolo Bonzini
2024-05-07 10:55 ` [PULL 16/26] target/i386: move 60-BF opcodes to new decoder Paolo Bonzini
2024-05-07 10:55 ` [PULL 17/26] target/i386: generalize gen_movl_seg_T0 Paolo Bonzini
2024-05-07 10:55 ` [PULL 18/26] target/i386: move C0-FF opcodes to new decoder (except for x87) Paolo Bonzini
2024-05-07 10:55 ` [PULL 19/26] target/i386: merge and enlarge a few ranges for call to disas_insn_new Paolo Bonzini
2024-05-07 10:55 ` [PULL 20/26] target/i386: move remaining conditional operations to new decoder Paolo Bonzini
2024-05-07 10:55 ` [PULL 21/26] target/i386: move BSWAP " Paolo Bonzini
2024-05-07 10:55 ` [PULL 22/26] target/i386: port extensions of one-byte opcodes " Paolo Bonzini
2024-05-07 10:55 ` [PULL 23/26] target/i386: remove now-converted opcodes from old decoder Paolo Bonzini
2024-05-07 10:55 ` [PULL 24/26] target/i386: decode x87 instructions in a separate function Paolo Bonzini
2024-05-07 10:55 ` [PULL 25/26] target/i386: split legacy decoder into " Paolo Bonzini
2024-05-07 10:55 ` [PULL 26/26] target/i386: remove duplicate prefix decoding Paolo Bonzini
2024-05-07 18:27 ` [PULL 00/26] target/i386 changes for 2024-05-07 Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240507105538.180704-2-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=babu.moger@amd.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=zhao1.liu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).