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);
}
