| Message ID | 1528498581-131037-2-git-send-email-babu.moger@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | i386: Enable TOPOEXT to support hyperthreading on AMD CPU | expand |
On Fri, Jun 08, 2018 at 06:56:17PM -0400, Babu Moger wrote: > Add support for cpuid leaf CPUID_8000_001E. Build the config that closely > match the underlying hardware. Please refer to the Processor Programming > Reference (PPR) for AMD Family 17h Model for more details. > > Signed-off-by: Babu Moger <babu.moger@amd.com> Queued on x86-next, thanks.
> -----Original Message----- > From: Eduardo Habkost [mailto:ehabkost@redhat.com] > Sent: Monday, June 11, 2018 3:55 PM > To: Moger, Babu <Babu.Moger@amd.com> > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com; > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org; > kvm@vger.kernel.org; kash@tripleback.net; geoff@hostfission.com > Subject: Re: [PATCH v13 1/5] i386: Add support for CPUID_8000_001E for > AMD > > On Fri, Jun 08, 2018 at 06:56:17PM -0400, Babu Moger wrote: > > Add support for cpuid leaf CPUID_8000_001E. Build the config that closely > > match the underlying hardware. Please refer to the Processor > Programming > > Reference (PPR) for AMD Family 17h Model for more details. > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > > Queued on x86-next, thanks. > Thanks > -- > Eduardo
On Fri, Jun 08, 2018 at 06:56:17PM -0400, Babu Moger wrote: > Add support for cpuid leaf CPUID_8000_001E. Build the config that closely > match the underlying hardware. Please refer to the Processor Programming > Reference (PPR) for AMD Family 17h Model for more details. > > Signed-off-by: Babu Moger <babu.moger@amd.com> [...] > + case 0x8000001E: > + assert(cpu->core_id <= 255); It is possible to trigger this assert using: $ qemu-system-x86_64 -machine q35,accel=kvm,kernel-irqchip=split -device intel-iommu,intremap=on,eim=on -smp 1,maxcpus=258,cores=258,threads=1,sockets=1 -cpu qemu64,xlevel=0x8000001e -device qemu64-x86_64-cpu,apic-id=257 qemu-system-x86_64: warning: Number of hotpluggable cpus requested (258) exceeds the recommended cpus supported by KVM (240) qemu-system-x86_64: /home/ehabkost/rh/proj/virt/qemu/target/i386/cpu.c:5888: cpu_x86_cpuid: Assertion `cpu->core_id <= 255' failed. Aborted (core dumped) See bug report and discussion at https://bugzilla.redhat.com/show_bug.cgi?id=1834200 Also, it looks like encode_topo_cpuid8000001e() assumes core_id has only 3 bits, so the existing assert() is not even sufficient. We need to decide what to do if the user requests nr_cores > 8. Probably omitting CPUID[0x8000001E] if the VCPU topology is incompatible with encode_topo_cpuid8000001e() (and printing a warning) is the safest thing to do right now. > + encode_topo_cpuid8000001e(cs, cpu, > + eax, ebx, ecx, edx); > + break; > case 0xC0000000: > *eax = env->cpuid_xlevel2; > *ebx = 0; > -- > 1.8.3.1 >
Started looking at this. Let me know if you have any ideas. Will respond with more details later this week. > -----Original Message----- > From: Eduardo Habkost <ehabkost@redhat.com> > Sent: Tuesday, June 2, 2020 12:52 PM > To: Moger, Babu <Babu.Moger@amd.com> > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com; > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org; > kvm@vger.kernel.org; kash@tripleback.net; geoff@hostfission.com; Dr. David > Alan Gilbert <dgilbert@redhat.com> > Subject: Re: [PATCH v13 1/5] i386: Add support for CPUID_8000_001E for AMD > > On Fri, Jun 08, 2018 at 06:56:17PM -0400, Babu Moger wrote: > > Add support for cpuid leaf CPUID_8000_001E. Build the config that closely > > match the underlying hardware. Please refer to the Processor Programming > > Reference (PPR) for AMD Family 17h Model for more details. > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > [...] > > + case 0x8000001E: > > + assert(cpu->core_id <= 255); > > It is possible to trigger this assert using: > > $ qemu-system-x86_64 -machine q35,accel=kvm,kernel-irqchip=split -device > intel-iommu,intremap=on,eim=on -smp > 1,maxcpus=258,cores=258,threads=1,sockets=1 -cpu > qemu64,xlevel=0x8000001e -device qemu64-x86_64-cpu,apic-id=257 > qemu-system-x86_64: warning: Number of hotpluggable cpus requested (258) > exceeds the recommended cpus supported by KVM (240) > qemu-system-x86_64: > /home/ehabkost/rh/proj/virt/qemu/target/i386/cpu.c:5888: cpu_x86_cpuid: > Assertion `cpu->core_id <= 255' failed. > Aborted (core dumped) > > See bug report and discussion at > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla. > redhat.com%2Fshow_bug.cgi%3Fid%3D1834200&data=02%7C01%7Cbabu. > moger%40amd.com%7C8a2724729b914bc9b53d08d8071db392%7C3dd8961fe4 > 884e608e11a82d994e183d%7C0%7C0%7C637267171438806408&sdata=ib > iGlF%2FF%2FVtYQLf7fe988kxFsLhj4GrRiTOq4LUuOT8%3D&reserved=0 > > Also, it looks like encode_topo_cpuid8000001e() assumes core_id > has only 3 bits, so the existing assert() is not even sufficient. > We need to decide what to do if the user requests nr_cores > 8. > > Probably omitting CPUID[0x8000001E] if the VCPU topology is > incompatible with encode_topo_cpuid8000001e() (and printing a > warning) is the safest thing to do right now. > > > > > + encode_topo_cpuid8000001e(cs, cpu, > > + eax, ebx, ecx, edx); > > + break; > > case 0xC0000000: > > *eax = env->cpuid_xlevel2; > > *ebx = 0; > > -- > > 1.8.3.1 > > > > -- > Eduardo
> -----Original Message----- > From: Eduardo Habkost <ehabkost@redhat.com> > Sent: Tuesday, June 2, 2020 12:52 PM > To: Moger, Babu <Babu.Moger@amd.com> > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com; > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org; > kvm@vger.kernel.org; kash@tripleback.net; geoff@hostfission.com; Dr. David > Alan Gilbert <dgilbert@redhat.com> > Subject: Re: [PATCH v13 1/5] i386: Add support for CPUID_8000_001E for AMD > > On Fri, Jun 08, 2018 at 06:56:17PM -0400, Babu Moger wrote: > > Add support for cpuid leaf CPUID_8000_001E. Build the config that closely > > match the underlying hardware. Please refer to the Processor Programming > > Reference (PPR) for AMD Family 17h Model for more details. > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > [...] > > + case 0x8000001E: > > + assert(cpu->core_id <= 255); > > It is possible to trigger this assert using: > > $ qemu-system-x86_64 -machine q35,accel=kvm,kernel-irqchip=split -device > intel-iommu,intremap=on,eim=on -smp > 1,maxcpus=258,cores=258,threads=1,sockets=1 -cpu > qemu64,xlevel=0x8000001e -device qemu64-x86_64-cpu,apic-id=257 > qemu-system-x86_64: warning: Number of hotpluggable cpus requested (258) > exceeds the recommended cpus supported by KVM (240) > qemu-system-x86_64: > /home/ehabkost/rh/proj/virt/qemu/target/i386/cpu.c:5888: cpu_x86_cpuid: > Assertion `cpu->core_id <= 255' failed. > Aborted (core dumped) > > See bug report and discussion at > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla. > redhat.com%2Fshow_bug.cgi%3Fid%3D1834200&data=02%7C01%7Cbabu. > moger%40amd.com%7C8a2724729b914bc9b53d08d8071db392%7C3dd8961fe4 > 884e608e11a82d994e183d%7C0%7C0%7C637267171438806408&sdata=ib > iGlF%2FF%2FVtYQLf7fe988kxFsLhj4GrRiTOq4LUuOT8%3D&reserved=0 > > Also, it looks like encode_topo_cpuid8000001e() assumes core_id > has only 3 bits, so the existing assert() is not even sufficient. > We need to decide what to do if the user requests nr_cores > 8. > > Probably omitting CPUID[0x8000001E] if the VCPU topology is > incompatible with encode_topo_cpuid8000001e() (and printing a > warning) is the safest thing to do right now. Eduardo, We need to generalize the encode_topo_cpuid8000001e decoding. We will have to remove 3 bit limitation there. It will not scale with latest configurations. I will take a look that. For now, best option I think is to(like you mentioned in bug 1834200), declaring nr_cores > 256 as never supported (or deprecated); and throw warning. What do you think? > > > > > + encode_topo_cpuid8000001e(cs, cpu, > > + eax, ebx, ecx, edx); > > + break; > > case 0xC0000000: > > *eax = env->cpuid_xlevel2; > > *ebx = 0; > > -- > > 1.8.3.1 > > > > -- > Eduardo
On Thu, Jun 04, 2020 at 09:06:27AM -0500, Babu Moger wrote: > > > > -----Original Message----- > > From: Eduardo Habkost <ehabkost@redhat.com> > > Sent: Tuesday, June 2, 2020 12:52 PM > > To: Moger, Babu <Babu.Moger@amd.com> > > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com; > > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org; > > kvm@vger.kernel.org; kash@tripleback.net; geoff@hostfission.com; Dr. David > > Alan Gilbert <dgilbert@redhat.com> > > Subject: Re: [PATCH v13 1/5] i386: Add support for CPUID_8000_001E for AMD > > > > On Fri, Jun 08, 2018 at 06:56:17PM -0400, Babu Moger wrote: > > > Add support for cpuid leaf CPUID_8000_001E. Build the config that closely > > > match the underlying hardware. Please refer to the Processor Programming > > > Reference (PPR) for AMD Family 17h Model for more details. > > > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > > [...] > > > + case 0x8000001E: > > > + assert(cpu->core_id <= 255); > > > > It is possible to trigger this assert using: > > > > $ qemu-system-x86_64 -machine q35,accel=kvm,kernel-irqchip=split -device > > intel-iommu,intremap=on,eim=on -smp > > 1,maxcpus=258,cores=258,threads=1,sockets=1 -cpu > > qemu64,xlevel=0x8000001e -device qemu64-x86_64-cpu,apic-id=257 > > qemu-system-x86_64: warning: Number of hotpluggable cpus requested (258) > > exceeds the recommended cpus supported by KVM (240) > > qemu-system-x86_64: > > /home/ehabkost/rh/proj/virt/qemu/target/i386/cpu.c:5888: cpu_x86_cpuid: > > Assertion `cpu->core_id <= 255' failed. > > Aborted (core dumped) > > > > See bug report and discussion at > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla. > > redhat.com%2Fshow_bug.cgi%3Fid%3D1834200&data=02%7C01%7Cbabu. > > moger%40amd.com%7C8a2724729b914bc9b53d08d8071db392%7C3dd8961fe4 > > 884e608e11a82d994e183d%7C0%7C0%7C637267171438806408&sdata=ib > > iGlF%2FF%2FVtYQLf7fe988kxFsLhj4GrRiTOq4LUuOT8%3D&reserved=0 > > > > Also, it looks like encode_topo_cpuid8000001e() assumes core_id > > has only 3 bits, so the existing assert() is not even sufficient. > > We need to decide what to do if the user requests nr_cores > 8. > > > > Probably omitting CPUID[0x8000001E] if the VCPU topology is > > incompatible with encode_topo_cpuid8000001e() (and printing a > > warning) is the safest thing to do right now. > > Eduardo, We need to generalize the encode_topo_cpuid8000001e decoding. > We will have to remove 3 bit limitation there. It will not scale with > latest configurations. I will take a look that. > > For now, best option I think is to(like you mentioned in bug 1834200), > declaring nr_cores > 256 as never supported (or deprecated); and throw > warning. > > What do you think? I believe we can declare nr_cores > 256 as never supported to address the assert failure. Other CPUID functions also look broken when nr_cores is too large: encode_cache_cpuid4() seems to assume nr_cores is 128 or less. But we still need to make nr_cores > 8 safe while encode_topo_cpuid8000001e() is not generalized yet. > > > > > > > > > + encode_topo_cpuid8000001e(cs, cpu, > > > + eax, ebx, ecx, edx); > > > + break; > > > case 0xC0000000: > > > *eax = env->cpuid_xlevel2; > > > *ebx = 0; > > > -- > > > 1.8.3.1 > > > > > > > -- > > Eduardo >
> -----Original Message----- > From: Eduardo Habkost <ehabkost@redhat.com> > Sent: Thursday, June 4, 2020 3:54 PM > To: Moger, Babu <Babu.Moger@amd.com> > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com; > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org; > kvm@vger.kernel.org; kash@tripleback.net; geoff@hostfission.com; Dr. David > Alan Gilbert <dgilbert@redhat.com> > Subject: Re: [PATCH v13 1/5] i386: Add support for CPUID_8000_001E for AMD > > On Thu, Jun 04, 2020 at 09:06:27AM -0500, Babu Moger wrote: > > > > > > > -----Original Message----- > > > From: Eduardo Habkost <ehabkost@redhat.com> > > > Sent: Tuesday, June 2, 2020 12:52 PM > > > To: Moger, Babu <Babu.Moger@amd.com> > > > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; > pbonzini@redhat.com; > > > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org; > > > kvm@vger.kernel.org; kash@tripleback.net; geoff@hostfission.com; Dr. > David > > > Alan Gilbert <dgilbert@redhat.com> > > > Subject: Re: [PATCH v13 1/5] i386: Add support for CPUID_8000_001E for > AMD > > > > > > On Fri, Jun 08, 2018 at 06:56:17PM -0400, Babu Moger wrote: > > > > Add support for cpuid leaf CPUID_8000_001E. Build the config that closely > > > > match the underlying hardware. Please refer to the Processor > Programming > > > > Reference (PPR) for AMD Family 17h Model for more details. > > > > > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > > > [...] > > > > + case 0x8000001E: > > > > + assert(cpu->core_id <= 255); > > > > > > It is possible to trigger this assert using: > > > > > > $ qemu-system-x86_64 -machine q35,accel=kvm,kernel-irqchip=split -device > > > intel-iommu,intremap=on,eim=on -smp > > > 1,maxcpus=258,cores=258,threads=1,sockets=1 -cpu > > > qemu64,xlevel=0x8000001e -device qemu64-x86_64-cpu,apic-id=257 > > > qemu-system-x86_64: warning: Number of hotpluggable cpus requested > (258) > > > exceeds the recommended cpus supported by KVM (240) > > > qemu-system-x86_64: > > > /home/ehabkost/rh/proj/virt/qemu/target/i386/cpu.c:5888: cpu_x86_cpuid: > > > Assertion `cpu->core_id <= 255' failed. > > > Aborted (core dumped) > > > > > > See bug report and discussion at > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla. > > > > redhat.com%2Fshow_bug.cgi%3Fid%3D1834200&data=02%7C01%7Cbabu. > > > > moger%40amd.com%7C8a2724729b914bc9b53d08d8071db392%7C3dd8961fe4 > > > > 884e608e11a82d994e183d%7C0%7C0%7C637267171438806408&sdata=ib > > > iGlF%2FF%2FVtYQLf7fe988kxFsLhj4GrRiTOq4LUuOT8%3D&reserved=0 > > > > > > Also, it looks like encode_topo_cpuid8000001e() assumes core_id > > > has only 3 bits, so the existing assert() is not even sufficient. > > > We need to decide what to do if the user requests nr_cores > 8. > > > > > > Probably omitting CPUID[0x8000001E] if the VCPU topology is > > > incompatible with encode_topo_cpuid8000001e() (and printing a > > > warning) is the safest thing to do right now. > > > > Eduardo, We need to generalize the encode_topo_cpuid8000001e decoding. > > We will have to remove 3 bit limitation there. It will not scale with > > latest configurations. I will take a look that. > > > > For now, best option I think is to(like you mentioned in bug 1834200), > > declaring nr_cores > 256 as never supported (or deprecated); and throw > > warning. > > > > What do you think? > > I believe we can declare nr_cores > 256 as never supported to > address the assert failure. Other CPUID functions also look > broken when nr_cores is too large: encode_cache_cpuid4() seems to > assume nr_cores is 128 or less. Let me know where to add this check. Or if you want to take care of that that is fine as well. > But we still need to make nr_cores > 8 safe while > encode_topo_cpuid8000001e() is not generalized yet. I am working on a patch to address this now. Will send it soon with other patches(addressing uninitialized node_id). > > > > > > > > > > > > > > + encode_topo_cpuid8000001e(cs, cpu, > > > > + eax, ebx, ecx, edx); > > > > + break; > > > > case 0xC0000000: > > > > *eax = env->cpuid_xlevel2; > > > > *ebx = 0; > > > > -- > > > > 1.8.3.1 > > > > > > > > > > -- > > > Eduardo > > > > -- > Eduardo
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 1e69e68..86fb1a4 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -427,6 +427,87 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs, (cache->complex_indexing ? CACHE_COMPLEX_IDX : 0); } +/* Data structure to hold the configuration info for a given core index */ +struct core_topology { + /* core complex id of the current core index */ + int ccx_id; + /* + * Adjusted core index for this core in the topology + * This can be 0,1,2,3 with max 4 cores in a core complex + */ + int core_id; + /* Node id for this core index */ + int node_id; + /* Number of nodes in this config */ + int num_nodes; +}; + +/* + * Build the configuration closely match the EPYC hardware. Using the EPYC + * hardware configuration values (MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE) + * right now. This could change in future. + * nr_cores : Total number of cores in the config + * core_id : Core index of the current CPU + * topo : Data structure to hold all the config info for this core index + */ +static void build_core_topology(int nr_cores, int core_id, + struct core_topology *topo) +{ + int nodes, cores_in_ccx; + + /* First get the number of nodes required */ + nodes = nodes_in_socket(nr_cores); + + cores_in_ccx = cores_in_core_complex(nr_cores); + + topo->node_id = core_id / (cores_in_ccx * MAX_CCX); + topo->ccx_id = (core_id % (cores_in_ccx * MAX_CCX)) / cores_in_ccx; + topo->core_id = core_id % cores_in_ccx; + topo->num_nodes = nodes; +} + +/* Encode cache info for CPUID[8000001E] */ +static void encode_topo_cpuid8000001e(CPUState *cs, X86CPU *cpu, + uint32_t *eax, uint32_t *ebx, + uint32_t *ecx, uint32_t *edx) +{ + struct core_topology topo = {0}; + + build_core_topology(cs->nr_cores, cpu->core_id, &topo); + *eax = cpu->apic_id; + /* + * CPUID_Fn8000001E_EBX + * 31:16 Reserved + * 15:8 Threads per core (The number of threads per core is + * Threads per core + 1) + * 7:0 Core id (see bit decoding below) + * SMT: + * 4:3 node id + * 2 Core complex id + * 1:0 Core id + * Non SMT: + * 5:4 node id + * 3 Core complex id + * 1:0 Core id + */ + if (cs->nr_threads - 1) { + *ebx = ((cs->nr_threads - 1) << 8) | (topo.node_id << 3) | + (topo.ccx_id << 2) | topo.core_id; + } else { + *ebx = (topo.node_id << 4) | (topo.ccx_id << 3) | topo.core_id; + } + /* + * CPUID_Fn8000001E_ECX + * 31:11 Reserved + * 10:8 Nodes per processor (Nodes per processor is number of nodes + 1) + * 7:0 Node id (see bit decoding below) + * 2 Socket id + * 1:0 Node id + */ + *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << 2) | topo.node_id; + *edx = 0; +} + /* * Definitions of the hardcoded cache entries we expose: * These are legacy cache values. If there is a need to change any @@ -4120,6 +4201,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; } break; + case 0x8000001E: + assert(cpu->core_id <= 255); + encode_topo_cpuid8000001e(cs, cpu, + eax, ebx, ecx, edx); + break; case 0xC0000000: *eax = env->cpuid_xlevel2; *ebx = 0;
Add support for cpuid leaf CPUID_8000_001E. Build the config that closely match the underlying hardware. Please refer to the Processor Programming Reference (PPR) for AMD Family 17h Model for more details. Signed-off-by: Babu Moger <babu.moger@amd.com> --- target/i386/cpu.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+)