[Xen-devel] [RFC PATCH v6 13/12] microcode: add sequential application policy

[Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>]

Hi Chao,

Updating microcode in parallel by default should be fine, but I think
there are no guarantees that a parallel application will be fine for
all future microcodes. To retain the ability to update microcode on cores
sequentially and give some choice to a user, I developed the below patch.

Could you consider including something like this into v7?

(The patch is for Xen 4.11 but forward-porting should be trivial)

---
 tools/misc/xen-microcode.c        | 17 +++++++++++++--
 xen/arch/x86/microcode.c          | 35 +++++++++++++++++++++++++++++--
 xen/arch/x86/platform_hypercall.c |  3 ++-
 xen/include/asm-x86/processor.h   |  3 ++-
 xen/include/public/platform.h     |  3 +++
 5 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/tools/misc/xen-microcode.c b/tools/misc/xen-microcode.c
index 2c1b2e362b..55d7d2f5df 100644
--- a/tools/misc/xen-microcode.c
+++ b/tools/misc/xen-microcode.c
@@ -46,7 +46,7 @@ void show_help(void)
 {
     fprintf(stderr,
             "xen-microcode: Xen microcode updating tool\n"
-            "Usage: xen-microcode <microcode blob>\n");
+            "Usage: xen-microcode <microcode blob> <parallel|sequential>\n");

     get_cpu_family(cpuid_eax(1));
     fprintf(stderr,
@@ -62,13 +62,25 @@ int main(int argc, char *argv[])
     struct xen_platform_op op;
     xc_interface *xch;
     DECLARE_HYPERCALL_BUFFER(struct xenpf_microcode_update, uc);
+    uint32_t strategy;

-    if (argc < 2)
+    if (argc < 3)
     {
         show_help();
         return 0;
     }

+    if (!strcmp(argv[2], "parallel"))
+        strategy = XENPF_microcode_parallel;
+    else if (!strcmp(argv[2], "sequential"))
+        strategy = XENPF_microcode_sequential;
+    else
+    {
+        show_help();
+        return 0;
+    }
+
+
     filename = argv[1];
     fd = open(filename, O_RDONLY);
     if (fd < 0) {
@@ -107,6 +119,7 @@ int main(int argc, char *argv[])
     op.cmd = XENPF_microcode_update;
     op.interface_version = XENPF_INTERFACE_VERSION;
     op.u.microcode.length = len;
+    op.u.microcode.strategy = strategy;
     ret = xc_platform_op(xch, &op);
     if ( ret )
         fprintf(stderr, "Failed to update microcode. (err: %d)\n", ret);
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 95e39c33bc..63b01b571f 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -37,6 +37,8 @@
 #include <xen/earlycpio.h>
 #include <xen/watchdog.h>

+#include <public/platform.h>
+
 #include <asm/delay.h>
 #include <asm/msr.h>
 #include <asm/processor.h>
@@ -216,6 +218,10 @@ DEFINE_PER_CPU(struct cpu_signature, cpu_sig);
  */
 static atomic_t cpu_in, cpu_out;

+static uint32_t application_strategy;
+/* The next CPU to perform a ucode update */
+static int next_cpu;
+
 /*
  * Save an ucode patch to the global cache list.
  *
@@ -351,6 +357,16 @@ static int do_microcode_update(void *unused)
     if ( ret )
         return ret;

+    while ( application_strategy == XENPF_microcode_sequential &&
+            cpu != next_cpu )
+    {
+        finished = atomic_read(&cpu_out);
+        if ( wait_for_cpus(&cpu_out, finished + 1,
+                           MICROCODE_UPDATE_TIMEOUT_US) )
+            panic("Timeout during sequential microcode update (finished 
%d/%d)",
+                  finished, cpu_nr);
+    }
+
     /*
      * Initiate an update on all processors which don't have an online sibling
      * thread with a lower thread id. Other sibling threads just await the
@@ -358,6 +374,10 @@ static int do_microcode_update(void *unused)
      */
     if ( cpu == cpumask_first(per_cpu(cpu_sibling_mask, cpu)) )
         ret = microcode_update_cpu();
+
+    if ( application_strategy == XENPF_microcode_sequential )
+        next_cpu = cpumask_next(next_cpu, &cpu_online_map);
+
     /*
      * Increase the wait timeout to a safe value here since we're serializing
      * the microcode update and that could take a while on a large number of
@@ -393,7 +413,8 @@ static int do_microcode_update(void *unused)
     return ret;
 }

-int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
+int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len,
+                     uint32_t strategy)
 {
     int ret;
     void *buffer;
@@ -405,6 +426,10 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
     if ( microcode_ops == NULL )
         return -EINVAL;

+    if ( strategy != XENPF_microcode_parallel &&
+         strategy != XENPF_microcode_sequential )
+        return -EINVAL;
+
     buffer = xmalloc_bytes(len);
     if ( !buffer )
     {
@@ -449,13 +474,19 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
         if ( cpu == cpumask_first(per_cpu(cpu_sibling_mask, cpu)) )
             nr_cores++;

-    printk(XENLOG_INFO "%d cores are to update their microcode\n", nr_cores);
+    printk(XENLOG_INFO "%d cores are to update their microcode %s\n", nr_cores,
+           strategy == XENPF_microcode_parallel ? "in parallel" :
+                                                  "sequentially");

     /*
      * We intend to disable interrupt for long time, which may lead to
      * watchdog timeout.
      */
     watchdog_disable();
+
+    application_strategy = strategy;
+    if ( strategy == XENPF_microcode_sequential )
+        next_cpu = cpumask_first(&cpu_online_map);
     /*
      * Late loading dance. Why the heavy-handed stop_machine effort?
      *
diff --git a/xen/arch/x86/platform_hypercall.c 
b/xen/arch/x86/platform_hypercall.c
index ea18c3215a..b5d2c5790b 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -296,7 +296,8 @@ ret_t 
do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)

         ret = microcode_update(
                 guest_handle_to_param(data, const_void),
-                op->u.microcode.length);
+                op->u.microcode.length,
+                op->u.microcode.strategy);
         spin_unlock(&vcpu_alloc_lock);
     }
     break;
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 0a62dfa0a3..612b786e81 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -573,7 +573,8 @@ int rdmsr_hypervisor_regs(uint32_t idx, uint64_t *val);
 int wrmsr_hypervisor_regs(uint32_t idx, uint64_t val);

 void microcode_set_module(unsigned int);
-int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len);
+int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len,
+                     uint32_t strategy);
 int early_microcode_update_cpu(void);
 int early_microcode_init(void);
 int microcode_init_intel(void);
diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h
index 94dbc3feb4..4dea94b84d 100644
--- a/xen/include/public/platform.h
+++ b/xen/include/public/platform.h
@@ -114,6 +114,9 @@ struct xenpf_microcode_update {
     /* IN variables. */
     XEN_GUEST_HANDLE(const_void) data;/* Pointer to microcode data */
     uint32_t length;                  /* Length of microcode data. */
+#define XENPF_microcode_parallel   0
+#define XENPF_microcode_sequential 1
+    uint32_t strategy;                /* Application strategy.     */
 };
 typedef struct xenpf_microcode_update xenpf_microcode_update_t;
 DEFINE_XEN_GUEST_HANDLE(xenpf_microcode_update_t);

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