Date:   Tue, 7 Mar 2023 15:35:20 -0800
In-Reply-To: <1975308c952236895f2d8f0e56af9db288eaf330.camel@infradead.org>
Mime-Version: 1.0
References: <20230302111227.2102545-1-usama.arif@bytedance.com>
 <faa0eb3bb8ba0326d501516a057ab46eaf1f3c05.camel@infradead.org>
 <effbb6e2-c5a1-af7f-830d-8d7088f57477@amd.com> <269ed38b5eed9c3a259c183d59d4f1eb5128f132.camel@infradead.org>
 <0c56683a-c258-46f6-056e-e85da8a557db@amd.com> <3bfbbd92-b2ed-8189-7b57-0533f6c87ae7@amd.com>
 <1975308c952236895f2d8f0e56af9db288eaf330.camel@infradead.org>
Message-ID: <ZAfDhT97ctXSYiYe@google.com>
Subject: Re: [PATCH v13 00/11] Parallel CPU bringup for x86_64
From:   Sean Christopherson <seanjc@google.com>
To:     David Woodhouse <dwmw2@infradead.org>
Cc:     Tom Lendacky <thomas.lendacky@amd.com>,
        Usama Arif <usama.arif@bytedance.com>, tglx@linutronix.de,
        kim.phillips@amd.com, brgerst@gmail.com,
        Sabin Rapan <sabrapan@amazon.com>, piotrgorski@cachyos.org,
        oleksandr@natalenko.name, arjan@linux.intel.com, mingo@redhat.com,
        bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com,
        x86@kernel.org, pbonzini@redhat.com, paulmck@kernel.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        rcu@vger.kernel.org, mimoja@mimoja.de, hewenliang4@huawei.com,
        pmenzel@molgen.mpg.de, fam.zheng@bytedance.com,
        punit.agrawal@bytedance.com, simon.evans@bytedance.com,
        liangma@liangbit.com
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

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.