From:   Thomas Gleixner <tglx@linutronix.de>
To:     David Woodhouse <dwmw2@infradead.org>,
        Usama Arif <usama.arif@bytedance.com>, arjan@linux.intel.com
Cc:     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, thomas.lendacky@amd.com, seanjc@google.com,
        pmenzel@molgen.mpg.de, fam.zheng@bytedance.com,
        punit.agrawal@bytedance.com, simon.evans@bytedance.com,
        liangma@liangbit.com
Subject: Re: [PATCH v6 01/11] x86/apic/x2apic: Fix parallel handling of
 cluster_mask
In-Reply-To: <0460d9cd8cdb7be2fcea00579e80ee683920d66d.camel@infradead.org>
References: <20230202215625.3248306-1-usama.arif@bytedance.com>
 <20230202215625.3248306-2-usama.arif@bytedance.com> <87a61qxtx0.ffs@tglx>
 <d37f3af69df09ff542024ed93a37865b28dfa86e.camel@infradead.org>
 <921cfe295fcd398168e5454e01193045de312688.camel@infradead.org>
 <87v8kdv9i1.ffs@tglx>
 <0460d9cd8cdb7be2fcea00579e80ee683920d66d.camel@infradead.org>
Date:   Tue, 07 Feb 2023 21:58:44 +0100
Message-ID: <87bkm5ur97.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Tue, Feb 07 2023 at 19:53, David Woodhouse wrote:
> On Tue, 2023-02-07 at 15:24 +0100, Thomas Gleixner wrote:
> Thanks. I've reworked and I think I've caught everything. Didn't want
> to elide the credit where Usama had done some of the forward-porting
> work, so I've left those notes and the SoB intact on those patches, on
> the assumption that they will be reposting the series after proper
> testing on hardware again anyway (I'm only spawning it in qemu right
> now).
>
> https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/parallel-6.2-rc7
>
> The only real code change other than what we've discussed here is to
> implement what we talked about for CPUID 0xb vs. 0x1 etc:
>
> 	/*
> 	 * 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.
> 	 */
> 	if (IS_ENABLED(CONFIG_X86_32) || boot_cpu_data.cpuid_level < 1 ||
> 	    (x2apic_mode && boot_cpu_data.cpuid_level < 0xb) ||
> 	    cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
> 		do_parallel_bringup = false;
>
> 	if (do_parallel_bringup && x2apic_mode) {
> 		unsigned int eax, ebx, ecx, edx;
>
> 		/*
> 		 * 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) {
> 			smpboot_control = STARTUP_SECONDARY | STARTUP_APICID_CPUID_0B;
> 		} else {
> 			pr_info("Disabling parallel bringup because CPUID 0xb looks untrustworthy\n");
> 			do_parallel_bringup = false;
> 		}
> 	} else if (do_parallel_bringup) {
> 		/* Without X2APIC, what's in CPUID 0x01 should suffice. */
> 		smpboot_control = STARTUP_SECONDARY | STARTUP_APICID_CPUID_01;
> 	}

Looks good to me!

Thanks,

        tglx