[Xen-devel] [PATCH v5 8/8] microcode: update microcode on cores in parallel

[Chao Gao <chao.gao@xxxxxxxxx>]

Currently, microcode_update_lock and microcode_mutex prevent cores
from updating microcode in parallel. Below changes are made to support
parallel microcode update on cores.

microcode_update_lock is removed. The purpose of this lock is to
prevent logic threads of a same core from updating microcode at the
same time. But due to using a global lock, it also prevents parallel
microcode updating on different cores. The original purpose of
microcode_update_lock is already enforced at the level of
apply_microcode()'s caller:
1. For late microcode update, only one sibiling thread of a core will
call the apply_microcode().
2. For microcode update during system startup or CPU-hotplug, each
logical thread is woken up one-by-one.
3. get/put_cpu_bitmaps() prevents the concurrency of CPU-hotplug and
late microcode update.

microcode_mutex is replaced by a rwlock. microcode_mutex was used to
prevent concurrent accesses to 'uci' and microcode_cache. Now the
per-cpu variable, 'uci', won't be accessed by remote cpus after most
fields in 'uci' have been removed; The only shared resource which
needs to be protected is the microcode_cache. A rwlock allows multiple
readers (one thread of each core) to access the global cache and
update microcode simultaneously. Because the rwlock may be held in
stop_machine context, where interrupt is disabled, irq{save, restore}
variants are used to get/release the rwlock.

Note that printk in apply_microcode() and svm_host_osvm_init() (for AMD
only) are still processed sequentially.

Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
---
Changes in v5:
 - newly add
---
 xen/arch/x86/microcode.c       | 51 ++++++++++++++++++++++++++++++------------
 xen/arch/x86/microcode_amd.c   |  9 +-------
 xen/arch/x86/microcode_intel.c |  9 +-------
 3 files changed, 39 insertions(+), 30 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index b7b20cf..8de9f0e 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -192,7 +192,7 @@ scan:
 
 const struct microcode_ops *microcode_ops;
 
-static DEFINE_SPINLOCK(microcode_mutex);
+static DEFINE_RWLOCK(cache_rwlock);
 
 DEFINE_PER_CPU(struct ucode_cpu_info, ucode_cpu_info);
 
@@ -206,6 +206,7 @@ static void microcode_fini_cpu(unsigned int cpu)
 {
     struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
 
+    ASSERT(cpu == smp_processor_id());
     memset(uci, 0, sizeof(*uci));
 }
 
@@ -213,21 +214,25 @@ static void microcode_fini_cpu(unsigned int cpu)
 bool save_patch(struct microcode_patch *new_patch)
 {
     struct microcode_patch *microcode_patch;
+    enum microcode_match_result result = MIS_UCODE;
+    bool ret;
+    unsigned long flag;
+
+    write_lock_irqsave(&cache_rwlock, flag);
 
     list_for_each_entry(microcode_patch, &microcode_cache, list)
     {
-        enum microcode_match_result result =
-            microcode_ops->replace_patch(new_patch, microcode_patch);
+        result = microcode_ops->replace_patch(new_patch, microcode_patch);
 
         switch ( result )
         {
         case OLD_UCODE:
-            microcode_ops->free_patch(new_patch);
-            return false;
+            ret = false;
+            goto out;
 
         case NEW_UCODE:
-            microcode_ops->free_patch(microcode_patch);
-            return true;
+            ret = true;
+            goto out;
 
         case MIS_UCODE:
             continue;
@@ -238,7 +243,27 @@ bool save_patch(struct microcode_patch *new_patch)
         }
     }
     list_add_tail(&new_patch->list, &microcode_cache);
-    return true;
+    ret = true;
+
+ out:
+    write_unlock_irqrestore(&cache_rwlock, flag);
+
+    /* free useless patches after interrupt enabled */
+    switch ( result )
+    {
+    case OLD_UCODE:
+        microcode_ops->free_patch(new_patch);
+        break;
+
+    case NEW_UCODE:
+        microcode_ops->free_patch(microcode_patch);
+        break;
+
+    default:
+        break;
+    }
+
+    return ret;
 }
 
 /* Find a ucode patch who has newer revision than the one in use */
@@ -269,15 +294,12 @@ static int parse_microcode_blob(const void *buffer, 
size_t len)
 {
     unsigned int cpu = smp_processor_id();
     struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
-    int ret;
+    int ret = microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig);
 
-    spin_lock(&microcode_mutex);
-    ret = microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig);
     if ( likely(!ret) )
         ret = microcode_ops->cpu_request_microcode(cpu, buffer, len);
     else
         microcode_fini_cpu(cpu);
-    spin_unlock(&microcode_mutex);
 
     return ret;
 }
@@ -285,10 +307,11 @@ static int parse_microcode_blob(const void *buffer, 
size_t len)
 static int microcode_update_cpu(void)
 {
     int ret;
+    unsigned long flag;
 
-    spin_lock(&microcode_mutex);
+    read_lock_irqsave(&cache_rwlock, flag);
     ret = microcode_ops->apply_microcode(smp_processor_id());
-    spin_unlock(&microcode_mutex);
+    read_unlock_irqrestore(&cache_rwlock, flag);
 
     return ret;
 }
diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index 80e274e..73df708 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -74,9 +74,6 @@ struct mpbhdr {
     uint8_t data[];
 };
 
-/* serialize access to the physical write */
-static DEFINE_SPINLOCK(microcode_update_lock);
-
 /* See comment in start_update() for cases when this routine fails */
 static int collect_cpu_info(unsigned int cpu, struct cpu_signature *csig)
 {
@@ -258,7 +255,6 @@ static enum microcode_match_result replace_patch(struct 
microcode_patch *new,
 
 static int apply_microcode(unsigned int cpu)
 {
-    unsigned long flags;
     struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
     uint32_t rev;
     struct microcode_header_amd *hdr;
@@ -274,16 +270,13 @@ static int apply_microcode(unsigned int cpu)
 
     hdr = patch->data;
     BUG_ON(!hdr);
-
-    spin_lock_irqsave(&microcode_update_lock, flags);
+    BUG_ON(local_irq_is_enabled());
 
     hw_err = wrmsr_safe(MSR_AMD_PATCHLOADER, (unsigned long)hdr);
 
     /* get patch id after patching */
     rdmsrl(MSR_AMD_PATCHLEVEL, rev);
 
-    spin_unlock_irqrestore(&microcode_update_lock, flags);
-
     /* check current patch id and patch's id for match */
     if ( hw_err || (rev != hdr->patch_id) )
     {
diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
index 02fc5a0..11440d1 100644
--- a/xen/arch/x86/microcode_intel.c
+++ b/xen/arch/x86/microcode_intel.c
@@ -86,9 +86,6 @@ struct extended_sigtable {
 
 #define exttable_size(et) ((et)->count * EXT_SIGNATURE_SIZE + EXT_HEADER_SIZE)
 
-/* serialize access to the physical write to MSR 0x79 */
-static DEFINE_SPINLOCK(microcode_update_lock);
-
 static int collect_cpu_info(unsigned int cpu_num, struct cpu_signature *csig)
 {
     struct cpuinfo_x86 *c = &cpu_data[cpu_num];
@@ -297,7 +294,6 @@ static int get_matching_microcode(const void *mc, unsigned 
int cpu)
 
 static int apply_microcode(unsigned int cpu)
 {
-    unsigned long flags;
     uint64_t msr_content;
     unsigned int val[2];
     unsigned int cpu_num = raw_smp_processor_id();
@@ -314,9 +310,7 @@ static int apply_microcode(unsigned int cpu)
 
     mc_intel = patch->data;
     BUG_ON(!mc_intel);
-
-    /* serialize access to the physical write to MSR 0x79 */
-    spin_lock_irqsave(&microcode_update_lock, flags);
+    BUG_ON(local_irq_is_enabled());
 
     /* write microcode via MSR 0x79 */
     wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->bits);
@@ -329,7 +323,6 @@ static int apply_microcode(unsigned int cpu)
     rdmsrl(MSR_IA32_UCODE_REV, msr_content);
     val[1] = (uint32_t)(msr_content >> 32);
 
-    spin_unlock_irqrestore(&microcode_update_lock, flags);
     if ( val[1] != mc_intel->hdr.rev )
     {
         printk(KERN_ERR "microcode: CPU%d update from revision "
-- 
1.8.3.1


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