[02/11] x86/ucode/amd: Move check_final_patch_levels() to apply_microcode()

Commit Message

Andrew Cooper March 31, 2020, 10:05 a.m. UTC

The microcode revision of whichever CPU runs cpu_request_microcode() is not
necessarily applicable to other CPUs.

If the BIOS left us with asymmetric microcode, rejecting updates in
cpu_request_microcode() would prevent us levelling the system even if only up
to the final level.  Also, failing to cache microcode misses an opportunity to
get beyond the final level via the S3 path.

Move check_final_patch_levels() earlier and use it in apply_microcode().
Reword the error message to be more informative, and use -ENXIO as this corner
case has nothing to do with permissions.

Signed-off-by: Andrew Cooper <[e-mail]>
---
CC: Jan Beulich <[e-mail]>
CC: Wei Liu <[e-mail]>
CC: Roger Pau Monné <[e-mail]>
---
 xen/arch/x86/cpu/microcode/amd.c | 83 ++++++++++++++++++----------------------
 1 file changed, 38 insertions(+), 45 deletions(-)

Comments

Jan Beulich March 31, 2020, 2:27 p.m. UTC | #1

On 31.03.2020 12:05, Andrew Cooper wrote:
> The microcode revision of whichever CPU runs cpu_request_microcode() is not
> necessarily applicable to other CPUs.
> 
> If the BIOS left us with asymmetric microcode, rejecting updates in
> cpu_request_microcode() would prevent us levelling the system even if only up
> to the final level.  Also, failing to cache microcode misses an opportunity to
> get beyond the final level via the S3 path.
> 
> Move check_final_patch_levels() earlier and use it in apply_microcode().
> Reword the error message to be more informative, and use -ENXIO as this corner
> case has nothing to do with permissions.
> 
> Signed-off-by: Andrew Cooper <[e-mail]>

Reviewed-by: Jan Beulich <[e-mail]>

Patch

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 796745e928..4245dc13bb 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -119,6 +119,36 @@  static bool_t verify_patch_size(uint32_t patch_size)
     return (patch_size <= max_size);
 }
 
+static bool check_final_patch_levels(const struct cpu_signature *sig)
+{
+    /*
+     * The 'final_levels' of patch ids have been obtained empirically.
+     * Refer bug https://bugzilla.suse.com/show_bug.cgi?id=913996
+     * for details of the issue. The short version is that people
+     * using certain Fam10h systems noticed system hang issues when
+     * trying to update microcode levels beyond the patch IDs below.
+     * From internal discussions, we gathered that OS/hypervisor
+     * cannot reliably perform microcode updates beyond these levels
+     * due to hardware issues. Therefore, we need to abort microcode
+     * update process if we hit any of these levels.
+     */
+    static const unsigned int final_levels[] = {
+        0x01000098,
+        0x0100009f,
+        0x010000af,
+    };
+    unsigned int i;
+
+    if ( boot_cpu_data.x86 != 0x10 )
+        return false;
+
+    for ( i = 0; i < ARRAY_SIZE(final_levels); i++ )
+        if ( sig->rev == final_levels[i] )
+            return true;
+
+    return false;
+}
+
 static bool_t find_equiv_cpu_id(const struct equiv_cpu_entry *equiv_cpu_table,
                                 unsigned int current_cpu_id,
                                 unsigned int *equiv_cpu_id)
@@ -229,6 +259,14 @@  static int apply_microcode(const struct microcode_patch *patch)
     if ( !match_cpu(patch) )
         return -EINVAL;
 
+    if ( check_final_patch_levels(sig) )
+    {
+        printk(XENLOG_ERR
+               "microcode: CPU%u current rev %#x unsafe to update\n",
+               cpu, sig->rev);
+        return -ENXIO;
+    }
+
     hdr = patch->mpb;
 
     hw_err = wrmsr_safe(MSR_AMD_PATCHLOADER, (unsigned long)hdr);
@@ -374,43 +412,6 @@  static int container_fast_forward(const void *data, size_t size_left, size_t *of
     return 0;
 }
 
-/*
- * The 'final_levels' of patch ids have been obtained empirically.
- * Refer bug https://bugzilla.suse.com/show_bug.cgi?id=913996 
- * for details of the issue. The short version is that people
- * using certain Fam10h systems noticed system hang issues when
- * trying to update microcode levels beyond the patch IDs below.
- * From internal discussions, we gathered that OS/hypervisor
- * cannot reliably perform microcode updates beyond these levels
- * due to hardware issues. Therefore, we need to abort microcode
- * update process if we hit any of these levels.
- */
-static const unsigned int final_levels[] = {
-    0x01000098,
-    0x0100009f,
-    0x010000af
-};
-
-static bool_t check_final_patch_levels(unsigned int cpu)
-{
-    /*
-     * Check the current patch levels on the cpu. If they are equal to
-     * any of the 'final_levels', then we should not update the microcode
-     * patch on the cpu as system will hang otherwise.
-     */
-    const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
-    unsigned int i;
-
-    if ( boot_cpu_data.x86 != 0x10 )
-        return 0;
-
-    for ( i = 0; i < ARRAY_SIZE(final_levels); i++ )
-        if ( sig->rev == final_levels[i] )
-            return 1;
-
-    return 0;
-}
-
 static struct microcode_patch *cpu_request_microcode(const void *buf,
                                                      size_t bufsize)
 {
@@ -434,14 +435,6 @@  static struct microcode_patch *cpu_request_microcode(const void *buf,
         goto out;
     }
 
-    if ( check_final_patch_levels(cpu) )
-    {
-        printk(XENLOG_INFO
-               "microcode: Cannot update microcode patch on the cpu as we hit a final level\n");
-        error = -EPERM;
-        goto out;
-    }
-
     mc_amd = xzalloc(struct microcode_amd);
     if ( !mc_amd )
     {