Re: [External] Re: [PATCH v17 6/8] x86/smpboot: Send INIT/SIPI/SIPI to secondary CPUs in parallel

[Usama Arif]

On 30/03/2023 19:17, Thomas Gleixner wrote:
> On Thu, Mar 30 2023 at 19:05, Borislav Petkov wrote:
>
> > On March 30, 2023 6:46:24 PM GMT+02:00, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> > > 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 :)
> >
> > Yeah I don't think we ever enforced this for early loading.
>
> We don't have to so far. CPU bringup is fully serialized so when the
> first sibling comes up the other one is still in wait for SIPI lala
> land. When the second comes up it will see that the microcode is already
> up to date.

A simple solution is to serialize load_ucode_ap by acquiring a spinlock 
at the start of ucode_cpu_init and releasing it at its end.

I guess if we had topology_sibling_cpumask initialized at this point we 
could have a spinlock per core (not thread) and parallelize it, but 
thats set much later in smp_callin.

I can include the below in next version if it makes sense?

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 80a688295ffa..b5e64628a975 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2155,10 +2155,13 @@ static inline void setup_getcpu(int cpu)
 }

 #ifdef CONFIG_X86_64
+static DEFINE_SPINLOCK(ucode_cpu_spinlock);
 static inline void ucode_cpu_init(int cpu)
 {
+       spin_lock(&ucode_cpu_spinlock);
        if (cpu)
                load_ucode_ap();
+       spin_unlock(&ucode_cpu_spinlock);
 }