Re: [PATCH] target/i386: Fix reporting of CPU dies when nr_cores=nr_threads=1

[‍小太]

On Thu, 27 Jul 2023 at 11:25, Xiaoyao Li <xiaoyao.li@intel.com> wrote:
>
> On 7/24/2023 2:59 AM, 小太 wrote:
> > When QEMU is started with `-smp D,sockets=1,dies=D,cores=1,threads=1` (that
> > is, 1 socket with D dies but each die contains just a single thread), both
> > Linux and Windows guests incorrectly interprets the system as having D
> > sockets with 1 die each
> >
> > Ultimately this is caused by various CPUID leaves not being die-aware in
> > their "threads per socket" calculations, so this patch fixes that
> >
> > These changes are referenced to the AMD PPR for Family 19h Model 01h (Milan)
> > and Family 17h Model 01h (Naples) manuals:
> >   - CPUID_Fn00000001_EBX[23:16]: Number of threads in the processor
> >                                  (Core::X86::Cpuid::SizeId[NC] + 1)
> >   - CPUID_Fn0000000B_EBX_x01[15:0]: Number of logical cores in processor
> >                                     socket (not present until Rome)
> >   - CPUID_Fn80000001_ECX[1]: Multi core product
> >                              (Core::X86::Cpuid::SizeId[NC] != 0)
> >   - CPUID_Fn80000008_ECX[7:0]: The number of threads in the package - 1
> >                                (Core::X86::Cpuid::SizeId[NC])
> >
> > Note there are two remaining occurences that I didn't touch:
> >   - CPUID_Fn8000001E_ECX[10:8]: Always 0 (1 node per processor) for Milan.
> >                                 But for Naples, it can also be 2 or 4 nodes
> >                                 where each node is defined as one or two
> >                                 CCXes (CCD?). But Milan also has multiple
> >                                 CCXes, so clearly the definition of a node 
> > is
> >                                 different from model to model, so I've left
> >                                 it untouched. (QEMU seems to use the Naples
> >                                 definition)
> >   - MSR_CORE_THREAD_COUNT: This MSR doesn't exist on Milan or Naples
>
> Is this patch specific to AMD CPU type? what's situation for Intel CPU?

I don't have a MCM Intel CPU to confirm against, but according to "Intel
Architectures Software Developer’s Manual":
 - 01h EBX[23:16]: Maximum number of addressable IDs for logical processors in
                   this physical package
 - 0Bh EBX[15:0]: The number of logical processors across all instances of this
                  domain within the next higher-scoped domain. Note: The 0Bh
                  leaf caps at the "core" domain, so Intel prefers using the 1Fh
                  leaf instead which supports modules/tiles/dies/sockets. But
                  AMD doesn't support the 1Fh leaf yet
 - 80000001h ECX[1]: Reserved
 - 80000008h ECX: Reserved = 0
 - 8000001Eh: Unsupported
 - MSR_CORE_THREAD_COUNT[31:16]: The number of processor cores that are
                                 currently enabled in the physical package

So the changes seem compatible with Intel, though MSR_CORE_THREAD_COUNT should
be updated to support dies as well. But in the absence of a MCM Intel CPU to
test with, I don't feel comfortable making that change