linux-kernel - Re: [PATCH v13 00/11] Parallel CPU bringup for x86

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 08 Mar 2023 09:04:55 +0000
From:   David Woodhouse <dwmw2@...radead.org>
To:     Tom Lendacky <thomas.lendacky@....com>,
        Usama Arif <usama.arif@...edance.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "Phillips, Kim" <kim.phillips@....com>,
        "brgerst@...il.com" <brgerst@...il.com>,
        "Rapan, Sabin" <sabrapan@...zon.com>
Cc:     "piotrgorski@...hyos.org" <piotrgorski@...hyos.org>,
        "oleksandr@...alenko.name" <oleksandr@...alenko.name>,
        "arjan@...ux.intel.com" <arjan@...ux.intel.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "hpa@...or.com" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "paulmck@...nel.org" <paulmck@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "rcu@...r.kernel.org" <rcu@...r.kernel.org>,
        "mimoja@...oja.de" <mimoja@...oja.de>,
        "hewenliang4@...wei.com" <hewenliang4@...wei.com>,
        "seanjc@...gle.com" <seanjc@...gle.com>,
        "pmenzel@...gen.mpg.de" <pmenzel@...gen.mpg.de>,
        "fam.zheng@...edance.com" <fam.zheng@...edance.com>,
        "punit.agrawal@...edance.com" <punit.agrawal@...edance.com>,
        "simon.evans@...edance.com" <simon.evans@...edance.com>,
        "liangma@...ngbit.com" <liangma@...ngbit.com>
Subject: Re: [PATCH v13 00/11] Parallel CPU bringup for x86_64

On Tue, 2023-03-07 at 16:55 -0600, Tom Lendacky wrote:
> On 3/7/23 16:27, 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.
> 
>  From a baremetal perspective, I think that works. Rome was the first
> generation to support x2apic, and the PPR for Rome states that 0's are 
> returned in all 4 registers for undefined function numbers.
> 
> For virtualization, at least Qemu/KVM, that also looks to be a safe test.

At Sean's suggestion, I've switched it to use the existing
check_extended_topology_leaf() which checks for EBX being non-zero, and
CH being 1 (SMT_TYPE).

I also made it work even if the kernel isn't using x2apic mode (is that
even possible, or does SEV-ES require the MSR-based access anyway?)

It just looked odd handling SEV-ES in the CPUID 0x0B path but not the
CPUID 0x01 case, and I certainly didn't want to implement the asm side
for handling CPUID 0x01 via the GHCB protocol. And this way I can pull
the check for CC_ATTR_GUEST_STATE_ENCRYPT up above. Which I've kept for
now for the reason described in the comment, but I won't die on that
hill.

https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/parallel-6.2-v14

Looks like this:

/*
 * We can do 64-bit AP bringup in parallel if the CPU reports its APIC
 * ID in CPUID (either leaf 0x0B if we need the full APIC ID in X2APIC
 * mode, or leaf 0x01 if 8 bits are sufficient). Otherwise it's too
 * hard.
 */
static bool prepare_parallel_bringup(void)
{
	bool has_sev_es = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT) &&
		static_branch_unlikely(&sev_es_enable_key);

	if (IS_ENABLED(CONFIG_X86_32))
		return false;

	/*
	 * Encrypted guests other than SEV-ES (in the future) will need to
	 * implement an early way of finding the APIC ID, since they will
	 * presumably block direct CPUID too. Be kind to our future selves
	 * by warning here instead of just letting them break. Parallel
	 * startup doesn't have to be in the first round of enabling patches
	 * for any such technology.
	 */
	if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT) || !has_sev_es) {
		pr_info("Disabling parallel bringup due to guest memory encryption\n");
		return false;
	}

	if (x2apic_mode || has_sev_es) {
		if (boot_cpu_data.cpuid_level < 0x0b)
			return false;

		if (check_extended_topology_leaf(0x0b) != 0) {
			pr_info("Disabling parallel bringup because CPUID 0xb looks untrustworthy\n");
			return false;
		}

		if (has_sev_es) {
			pr_debug("Using SEV-ES CPUID 0xb for parallel CPU startup\n");
			smpboot_control = STARTUP_APICID_SEV_ES;
		} else {
			pr_debug("Using CPUID 0xb for parallel CPU startup\n");
			smpboot_control = STARTUP_APICID_CPUID_0B;
		}
	} else {
		/* Without X2APIC, what's in CPUID 0x01 should suffice. */
		if (boot_cpu_data.cpuid_level < 0x01)
			return false;

		pr_debug("Using CPUID 0x1 for parallel CPU startup\n");
		smpboot_control = STARTUP_APICID_CPUID_01;
	}

	cpuhp_setup_state_nocalls(CPUHP_BP_PARALLEL_DYN, "x86/cpu:kick",
				  native_cpu_kick, NULL);
	return true;
}


Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)