Date:   Fri, 17 Dec 2021 11:09:06 +0100
From:   Igor Mammedov <imammedo@redhat.com>
To:     David Woodhouse <dwmw2@infradead.org>
Cc:     Tom Lendacky <thomas.lendacky@amd.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        "Paul E . McKenney" <paulmck@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "rcu@vger.kernel.org" <rcu@vger.kernel.org>,
        "mimoja@mimoja.de" <mimoja@mimoja.de>,
        "hewenliang4@huawei.com" <hewenliang4@huawei.com>,
        "hushiyuan@huawei.com" <hushiyuan@huawei.com>,
        "luolongjun@huawei.com" <luolongjun@huawei.com>,
        "hejingxian@huawei.com" <hejingxian@huawei.com>
Subject: Re: [PATCH v3 0/9] Parallel CPU bringup for x86_64
Message-ID: <20211217110906.5c38fe7b@redhat.com>
In-Reply-To: <721484e0fa719e99f9b8f13e67de05033dd7cc86.camel@infradead.org>
References: <20211215145633.5238-1-dwmw2@infradead.org>
        <761c1552-0ca0-403b-3461-8426198180d0@amd.com>
        <ca0751c864570015ffe4d8cccdc94e0a5ef3086d.camel@infradead.org>
        <b13eac6c-ea87-aef9-437f-7266be2e2031@amd.com>
        <721484e0fa719e99f9b8f13e67de05033dd7cc86.camel@infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 17 Dec 2021 00:13:16 +0000
David Woodhouse <dwmw2@infradead.org> wrote:

> On Thu, 2021-12-16 at 16:52 -0600, Tom Lendacky wrote:
> > On baremetal, I haven't seen an issue. This only seems to have a problem 
> > with Qemu/KVM.
> > 
> > With 191f08997577 I could boot without issues with and without the 
> > no_parallel_bringup. Only after I applied e78fa57dd642 did the failure happen.
> > 
> > With e78fa57dd642 I could boot 64 vCPUs pretty consistently, but when I 
> > jumped to 128 vCPUs it failed again. When I moved the series to 
> > df9726cb7178, then 64 vCPUs also failed pretty consistently.
> > 
> > Strange thing is it is random. Sometimes (rarely) it works on the first 
> > boot and then sometimes it doesn't, at which point it will reset and 
> > reboot 3 or 4 times and then make it past the failure and fully boot.  
> 
> Hm, some of that is just artifacts of timing, I'm sure. But now I'm
that's most likely the case (there is a race somewhere left).
To trigger CPU bringup (hotplug) races, I used to run QEMU guest with
heavy vCPU overcommit. It helps to induce unexpected delays at CPU bringup
time.


> staring at the way that early_setup_idt() can run in parallel on all
> CPUs, rewriting bringup_idt_descr and loading it.
> 
> To start with, let's try unlocking the trampoline_lock much later,
> after cpu_init_exception_handling() has loaded the real IDT. 
> 
> I think we can probably make secondaries load the real IDT early and
> never use bringup_idt_descr at all, can't we? But let's see if this
> makes it go away, to start with...
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 0cd6373bc3f2..2307f7575ab4 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -59,7 +59,7 @@
>  #include <asm/cpu_device_id.h>
>  #include <asm/uv/uv.h>
>  #include <asm/sigframe.h>
> -
> +#include <asm/realmode.h>
>  #include "cpu.h"
>  
>  u32 elf_hwcap2 __read_mostly;
> @@ -2060,6 +2060,7 @@ void cpu_init_secondary(void)
>  	 * on this CPU in cpu_init_exception_handling().
>  	 */
>  	cpu_init_exception_handling();
> +	clear_bit(0, (unsigned long *)trampoline_lock);
>  	cpu_init();
>  }
>  #endif
> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> index 3e4c3c416bce..db01b56574cd 100644
> --- a/arch/x86/kernel/head_64.S
> +++ b/arch/x86/kernel/head_64.S
> @@ -273,14 +273,6 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
>  	 */
>  	movq initial_stack(%rip), %rsp
>  
> -	/* Drop the realmode protection. For the boot CPU the pointer is NULL! */
> -	movq	trampoline_lock(%rip), %rax
> -	testq	%rax, %rax
> -	jz	.Lsetup_idt
> -	lock
> -	btrl	$0, (%rax)
> -
> -.Lsetup_idt:
>  	/* Setup and Load IDT */
>  	pushq	%rsi
>  	call	early_setup_idt