Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E for AMD

-----Original Message-----
From: Radim Krčmář [mailto:rkrcmar@redhat.com]
Sent: Thursday, March 1, 2018 1:57 PM
To: Moger, Babu <Babu.Moger@amd.com>
Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
pixo@polepetko.eu; Hook, Gary [off-list ref]
Subject: Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E
for AMD

2018-02-28 22:18+0000, Moger, Babu:

quoted

quoted

-----Original Message-----
From: Radim Krčmář [mailto:rkrcmar@redhat.com]
Sent: Wednesday, February 28, 2018 12:24 PM
To: Moger, Babu <Babu.Moger@amd.com>
Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
pixo@polepetko.eu; Hook, Gary [off-list ref]
Subject: Re: [PATCH v2 3/5] target/i386: Add support for

CPUID_8000_001E

quoted

quoted

for AMD

2018-02-23 21:30-0500, Babu Moger:

quoted

From: Stanislav Lanci <redacted>

Populate threads/core_id/apic_ids/socket_id when

CPUID_EXT3_TOPOEXT

quoted

quoted

quoted

feature is supported. This is required to support hyperthreading
feature on AMD CPUS. These are supported via CPUID_8000_001E

extended

quoted

functions.

Signed-off-by: Stanislav Lanci <redacted>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 target/i386/cpu.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a5a480e..191e850 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c

@@ -3666,6 +3666,14 @@ void cpu_x86_cpuid(CPUX86State *env,

uint32_t index, uint32_t count,

quoted

             *edx = 0;
         }
         break;
+    case 0x8000001E:
+        if (env->features[FEAT_8000_0001_ECX] &

CPUID_EXT3_TOPOEXT) {

quoted

quoted

quoted

+            *eax = cpu->apic_id;
+            *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;

Do we somewhere assert that AMD cannot have cpu->core_id > 255?
(qemu does allow weird configurations.)

I don't see specific assert on core_id.   But, I see that qemu does not allow

nr_cores more than 255.

quoted

Also I see that core_id is iterated based on nr_cores.  If you strongly

believe we need to add assert here, I will add it.

quoted

Let me know.

A user can emulate AMD with x2apic and intel-iommu to allow more than
255 vcpus and all of them can be a separate core (or its own socket),
which would overflow this (or the next counter).

Forbidding that crazy configuration may be preferable, but an assert
won't hurt now,

Ok. Sure. Will add assert.

thanks.

quoted

quoted

Thanks.

quoted

+            *ecx = cpu->socket_id;
+            *edx = 0;
+        }
+        break;
     case 0xC0000000:
         *eax = env->cpuid_xlevel2;
         *ebx = 0;
--
1.8.3.1