Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Usama Arif <usama.arif@bytedance.com>, dwmw2@infradead.org,
        kim.phillips@amd.com, brgerst@gmail.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,
        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,
        gpiccoli@igalia.com, David Woodhouse <dwmw@amazon.co.uk>,
        Usama Arif <usama.arif@bytedance.com>
Subject: Re: [PATCH v17 6/8] x86/smpboot: Send INIT/SIPI/SIPI to secondary
 CPUs in parallel
In-Reply-To: <20230328195758.1049469-7-usama.arif@bytedance.com>
References: <20230328195758.1049469-1-usama.arif@bytedance.com>
 <20230328195758.1049469-7-usama.arif@bytedance.com>
Date:   Thu, 30 Mar 2023 18:46:24 +0200
Message-ID: <87v8iirxun.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Tue, Mar 28 2023 at 20:57, Usama Arif wrote:
> The APs will then take turns through the real mode code (which has its
> own bitlock for exclusion) until they make it to their own stack, then
> proceed through the first few lines of start_secondary() and execute
> these parts in parallel:
>
>  start_secondary()
>     -> cr4_init()
>     -> (some 32-bit only stuff so not in the parallel cases)
>     -> cpu_init_secondary()
>        -> cpu_init_exception_handling()
>        -> cpu_init()
>           -> wait_for_master_cpu()
>
> At this point they wait for the BSP to set their bit in cpu_callout_mask
> (from do_wait_cpu_initialized()), and release them to continue through
> the rest of cpu_init() and beyond.

That's actually broken on SMT enabled machines when microcode needs to
be updated.

Lets look at a 2 core, 4 thread system, where CPU0/2 and CPU1/3 are the
sibling pairs.

CPU 0:                         	CPU1		CPU2		CPU3

for_each_present_cpu(cpu)
    cpu_up(cpu, KICK_AP_ALIVE);
                                startup()       
                                wait()
                                
			                        startup()
                                                wait()

Release CPU1
                                load_ucode()                    startup()
                                                                wait()

So that violates the rules of microcode loading that the sibling must be
in a state where it does not execute anything which might be affected by
the microcode update. The fragile startup code does not really qualify
as such a state :)

Thanks,

        tglx