| Message ID | 20200417215345.64800.73351.stgit@localhost.localdomain |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] target/i386: Fix the CPUID leaf CPUID_Fn80000008 | expand |
Hi, thanks for the patch, I tested it in my setup and I'm seeing numbers that make sense. However, I can create a setup which places the value 02h* into the ApicIdSize field, which is reserved. However, I deem this a configuration issue as well. * -cpu EPYC-v2 -smp 4,cores=4 --> 0x8000_0008[ECX] = 0x2003 Cheers, Philipp On 4/17/20 11:55 PM, Babu Moger wrote: > CPUID leaf CPUID_Fn80000008_ECX provides information about the > number of threads supported by the processor. It was found that > the field ApicIdSize(bits 15-12) was not set correctly. > > ApicIdSize is defined as the number of bits required to represent > all the ApicId values within a package. > > Valid Values: Value Description > 3h-0h Reserved. > 4h up to 16 threads. > 5h up to 32 threads. > 6h up to 64 threads. > 7h up to 128 threads. > Fh-8h Reserved. > > Fix the bit appropriately. > > This came up during following thread. > https://lore.kernel.org/qemu-devel/158643709116.17430.15995069125716778943.malonedeb@wampee.canonical.com/#t > > Refer the Processor Programming Reference (PPR) for AMD Family 17h > Model 01h, Revision B1 Processors. The documentation is available > from the bugzilla Link below. > Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537 > > Reported-by: Philipp Eppelt <1871842@bugs.launchpad.net> > Signed-off-by: Babu Moger <babu.moger@amd.com> > --- > v2: > Used env->pkg_offset for bits 15:12 which is already available. > > target/i386/cpu.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 90ffc5f..5e5a605 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -5830,11 +5830,20 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > *eax = cpu->phys_bits; > } > *ebx = env->features[FEAT_8000_0008_EBX]; > - *ecx = 0; > - *edx = 0; > if (cs->nr_cores * cs->nr_threads > 1) { > - *ecx |= (cs->nr_cores * cs->nr_threads) - 1; > + /* > + * Bits 15:12 is "The number of bits in the initial > + * Core::X86::Apic::ApicId[ApicId] value that indicate > + * thread ID within a package". This is already stored at > + * CPUX86State::pkg_offset. > + * Bits 7:0 is "The number of threads in the package is NC+1" > + */ > + *ecx = (env->pkg_offset << 12) | > + ((cs->nr_cores * cs->nr_threads) - 1); > + } else { > + *ecx = 0; > } > + *edx = 0; > break; > case 0x8000000A: > if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) { >
On 17/04/20 23:55, Babu Moger wrote: > CPUID leaf CPUID_Fn80000008_ECX provides information about the > number of threads supported by the processor. It was found that > the field ApicIdSize(bits 15-12) was not set correctly. > > ApicIdSize is defined as the number of bits required to represent > all the ApicId values within a package. > > Valid Values: Value Description > 3h-0h Reserved. > 4h up to 16 threads. > 5h up to 32 threads. > 6h up to 64 threads. > 7h up to 128 threads. > Fh-8h Reserved. > > Fix the bit appropriately. > > This came up during following thread. > https://lore.kernel.org/qemu-devel/158643709116.17430.15995069125716778943.malonedeb@wampee.canonical.com/#t > > Refer the Processor Programming Reference (PPR) for AMD Family 17h > Model 01h, Revision B1 Processors. The documentation is available > from the bugzilla Link below. > Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537 > > Reported-by: Philipp Eppelt <1871842@bugs.launchpad.net> > Signed-off-by: Babu Moger <babu.moger@amd.com> > --- > v2: > Used env->pkg_offset for bits 15:12 which is already available. > > target/i386/cpu.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 90ffc5f..5e5a605 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -5830,11 +5830,20 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > *eax = cpu->phys_bits; > } > *ebx = env->features[FEAT_8000_0008_EBX]; > - *ecx = 0; > - *edx = 0; > if (cs->nr_cores * cs->nr_threads > 1) { > - *ecx |= (cs->nr_cores * cs->nr_threads) - 1; > + /* > + * Bits 15:12 is "The number of bits in the initial > + * Core::X86::Apic::ApicId[ApicId] value that indicate > + * thread ID within a package". This is already stored at > + * CPUX86State::pkg_offset. > + * Bits 7:0 is "The number of threads in the package is NC+1" > + */ > + *ecx = (env->pkg_offset << 12) | > + ((cs->nr_cores * cs->nr_threads) - 1); > + } else { > + *ecx = 0; > } > + *edx = 0; > break; > case 0x8000000A: > if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) { > Queued, thanks. Paolo
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 90ffc5f..5e5a605 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5830,11 +5830,20 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *eax = cpu->phys_bits; } *ebx = env->features[FEAT_8000_0008_EBX]; - *ecx = 0; - *edx = 0; if (cs->nr_cores * cs->nr_threads > 1) { - *ecx |= (cs->nr_cores * cs->nr_threads) - 1; + /* + * Bits 15:12 is "The number of bits in the initial + * Core::X86::Apic::ApicId[ApicId] value that indicate + * thread ID within a package". This is already stored at + * CPUX86State::pkg_offset. + * Bits 7:0 is "The number of threads in the package is NC+1" + */ + *ecx = (env->pkg_offset << 12) | + ((cs->nr_cores * cs->nr_threads) - 1); + } else { + *ecx = 0; } + *edx = 0; break; case 0x8000000A: if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) {
CPUID leaf CPUID_Fn80000008_ECX provides information about the number of threads supported by the processor. It was found that the field ApicIdSize(bits 15-12) was not set correctly. ApicIdSize is defined as the number of bits required to represent all the ApicId values within a package. Valid Values: Value Description 3h-0h Reserved. 4h up to 16 threads. 5h up to 32 threads. 6h up to 64 threads. 7h up to 128 threads. Fh-8h Reserved. Fix the bit appropriately. This came up during following thread. https://lore.kernel.org/qemu-devel/158643709116.17430.15995069125716778943.malonedeb@wampee.canonical.com/#t Refer the Processor Programming Reference (PPR) for AMD Family 17h Model 01h, Revision B1 Processors. The documentation is available from the bugzilla Link below. Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537 Reported-by: Philipp Eppelt <1871842@bugs.launchpad.net> Signed-off-by: Babu Moger <babu.moger@amd.com> --- v2: Used env->pkg_offset for bits 15:12 which is already available. target/i386/cpu.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)