[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v8 16/16] microcode: block #NMI handling when loading an ucode



On 07.08.2019 09:59, Chao Gao wrote:
On Mon, Aug 05, 2019 at 12:11:01PM +0000, Jan Beulich wrote:
On 01.08.2019 12:22, Chao Gao wrote:
@@ -439,12 +440,37 @@ static int do_microcode_update(void *patch)
       return ret;
   }
+static int microcode_nmi_callback(const struct cpu_user_regs *regs, int cpu)
+{
+    bool print = false;
+
+    /* The first thread of a core is to load an update. Don't block it. */
+    if ( cpu == cpumask_first(per_cpu(cpu_sibling_mask, cpu)) )
+        return 0;
+
+    if ( loading_state == LOADING_ENTERED )
+    {
+        cpumask_set_cpu(cpu, &cpu_callin_map);
+        printk(XENLOG_DEBUG "CPU%u enters %s\n", smp_processor_id(), __func__);

Here  and ...

+        print = true;
+    }
+
+    while ( loading_state == LOADING_ENTERED )
+        rep_nop();
+
+    if ( print )
+        printk(XENLOG_DEBUG "CPU%u exits %s\n", smp_processor_id(), __func__);

... here - why smp_processor_id() when you can use "cpu"? And what
use is __func__ here?

The rep_nop() above also presumably wants to be cpu_relax() again.

But on the whole I was really hoping for more aggressive disabling
of NMI handling, more like (but of course not quite as heavy as)
the crash path wiring the IDT entry to trap_nop().

I agree with you that it should be more aggressive. This patch is
problematic considering there is a lot of code before reaching this
callback (especially, SPEC_CTRL_ENTRY_FROM_INTR_IST which may write
MSR_SPEC_CTRL).

In my mind, we have two options to solve the issue:
1. Wire the IDT entry to trap_nop() like the crash path.

As said, this is not directly an option - at the very least a thread
should record the fact that there was an NMI, such that it can
handle it after the ucode update has completed.

2. Enhance this patch: A thread which is not going to load an update
is forced to send an #NMI to itself to enter the callback, do
busy-loop until completion of loading ucode on all cores.
With this method, no #NMI delivery, or MSR write would happen on this
threads during microcode update.

This sounds doable at the first glance; iirc the CPU would latch
another NMI while NMIs are blocked due to there not having been an
IRET yet after the last one was raised. There would still be some
ambiguity in case the self-NMI and an actual one arrived at about
the same time, but I guess we need to live with this small window.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.