On Tue, 2023-03-07 at 15:35 -0800, Sean Christopherson wrote:
> On Tue, Mar 07, 2023, David Woodhouse wrote:
> > On Tue, 2023-03-07 at 16:22 -0600, Tom Lendacky wrote:
> > >
> > > I did some Qemu/KVM testing. One thing I noticed is that on AMD, CPUID 0xB
> > > EAX will be non-zero only if SMT is enabled. So just booting some guests
> > > without CPU topology never did parallel booting ("smpboot: Disabling
> > > parallel bringup because CPUID 0xb looks untrustworthy"). I would imagine
> > > a bare-metal system that has diabled SMT will not do parallel booting, too
> > > (but I haven't had time to test that).
> >
> > Interesting, thanks. Should I change to checking for *both* EAX and EBX
> > being zero? That's what I did first, after reading only the Intel SDM.
> > But I changed to only EAX because the AMD doc only says that EAX will
> > be zero for unsupported leaves.
>
> LOL, nice. '0' is a prefectly valid output for the shift if there's exactly one
> CPU at the current level, which is why Intel states that both EAX and EBX are
> cleared. I assume/hope this is effectively a documentation goof, in that the APM
> assumes that all "real" CPUs that support CPUID 0xB also support sub-function 0.
>
> Nit, the AMD says EAX will be zero for unsupported _levels_. The distinction
> matters because if the entire leaf is unsupported, AMD behavior is to zero out
> all output registers, even if the input leaf is above the max supported leaf.
>
> Anyways, the kernel already sanity checks the outputs on all x86 CPUs, just
> piggyback that logic, e.g. expose detect_extended_topology_leaf() or so. If AMD
> or a VMM is doing something crazy, the kernel is doomed anyways.
Well, we have to be somewhat careful with that assumption. Turns out
that if CPUID 0xb doesn't have valid data, the kernel *wasn't* doomed
anyway. A bunch of our early "WTF doesn't it work on AMD?" issue with
this series were due to precisely that.
But yeah, check_extended_topology_leaf() does check for EBX being non-
zero, and also for SMT_TYPE being in CH. I'll look at just calling
that; it'll make the mess of if/else in the prepare_parallel_bringup()
function a bit cleaner.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
