Message-ID: <cb82069c-cd81-1799-91c7-dea79916ab1a@bytedance.com>
Date:   Tue, 7 Mar 2023 21:00:47 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [External] Re: [PATCH v13 00/11] Parallel CPU bringup for x86_64
Content-Language: en-US
To:     David Woodhouse <dwmw2@infradead.org>,
        Tom Lendacky <thomas.lendacky@amd.com>, tglx@linutronix.de,
        kim.phillips@amd.com, brgerst@gmail.com,
        "Rapan, Sabin" <sabrapan@amazon.com>
Cc:     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,
        seanjc@google.com, pmenzel@molgen.mpg.de, fam.zheng@bytedance.com,
        punit.agrawal@bytedance.com, simon.evans@bytedance.com,
        liangma@liangbit.com
References: <20230302111227.2102545-1-usama.arif@bytedance.com>
 <faa0eb3bb8ba0326d501516a057ab46eaf1f3c05.camel@infradead.org>
 <effbb6e2-c5a1-af7f-830d-8d7088f57477@amd.com>
 <269ed38b5eed9c3a259c183d59d4f1eb5128f132.camel@infradead.org>
From:   Usama Arif <usama.arif@bytedance.com>
In-Reply-To: <269ed38b5eed9c3a259c183d59d4f1eb5128f132.camel@infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 9d956571ecc1..d194c4ffeef8 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1510,6 +1510,71 @@ void __init smp_prepare_cpus_common(void)
>   	set_cpu_sibling_map(0);
>   }
>   
> +
> +/*
> + * 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. And not for SEV-ES guests because they can't use CPUID that
> + * early.
> + */
> +static bool __init prepare_parallel_bringup(void)
> +{
> +	if (IS_ENABLED(CONFIG_X86_32) || boot_cpu_data.cpuid_level < 1)
> +		return false;
> +
> +	if (x2apic_mode) {
> +		unsigned int eax, ebx, ecx, edx;
> +
> +		if (boot_cpu_data.cpuid_level < 0xb)
> +			return false;
> +
> +		/*
> +		 * To support parallel bringup in x2apic mode, the AP will need
> +		 * to obtain its APIC ID from CPUID 0x0B, since CPUID 0x01 has
> +		 * only 8 bits. Check that it is present and seems correct.
> +		 */
> +		cpuid_count(0xb, 0, &eax, &ebx, &ecx, &edx);
> +
> +		/*
> +		 * AMD says that if executed with an umimplemented level in
> +		 * ECX, then it will return all zeroes in EAX. Intel says it
> +		 * will return zeroes in both EAX and EBX. Checking only EAX
> +		 * should be sufficient.
> +		 */
> +		if (!eax) {
> +			pr_info("Disabling parallel bringup because CPUID 0xb looks untrustworthy\n");
> +			return false;
> +		}
> +
> +		if (IS_ENABLED(AMD_MEM_ENCRYPT) && static_branch_unlikely(&sev_es_enable_key)) {
> +			pr_debug("Using SEV-ES CPUID 0xb for parallel CPU startup\n");
> +			smpboot_control = STARTUP_APICID_SEV_ES;
> +		} else if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)) {
> +			/*
> +			 * Other forms of memory encryption need to implement a way of
> +			 * finding the APs' APIC IDs that early.
> +			 */
> +			return false;
> +		} else {
> +			pr_debug("Using CPUID 0xb for parallel CPU startup\n");
> +			smpboot_control = STARTUP_APICID_CPUID_0B;

I believe TDX guests with x2apic mode will end up here and enable 
parallel smp if Sean was correct in this 
(https://lore.kernel.org/all/Y91PoIfc2jdRv0WG@google.com/). i.e. "TDX 
guest state is also encrypted, but TDX doesn't return true 
CC_ATTR_GUEST_STATE_ENCRYPT.".

So I believe the above else if 
(cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)) is not useful as thats 
set for just SEV-ES guests? which is covered in the if part.

Thanks,
Usama


> +		}
> +	} else {
> +		if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
> +			return false;
> +
> +		/* Without X2APIC, what's in CPUID 0x01 should suffice. */
> +		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;
> +}
> +
>   /*
>    * Prepare for SMP bootup.