[Xen-devel] [PATCH V2] x86, amd_ucode: Support multiple container files appended together

[Aravind Gopalakrishnan <aravind.gopalakrishnan@xxxxxxx>]

This patch adds support for parsing through multiple AMD container
binaries concatenated together. It is a feature already present in Linux.
Link to linux patch:
http://lkml.kernel.org/r/1370463236-2115-3-git-send-email-jacob.shin@xxxxxxx

Other changes introduced are-
 - Define HDR_SIZE's explicitly for code clarity.
 - Define AMD_UCODE_APPLY_SUCCESS so it's clear that we return this
   value if patch application succeeded regardless if there is only one
   container file or multiple ones concatenated.

Changes in V2
 - per Jan suggestion
   - return bool_t from find_equiv_cpu_id
    => drop the respective initializers of equiv_cpu_id in the callers
   - Reduce number of casts by using void * for passing raw binary whenever
     possible
   - Need size_left >= size check in container_fast_forward
   - Use error value returned from container_fast_forward in
     cpu_request_microcode
   - If changing code around 'error = save_error', make sure the behavior
     for single container files does not change

 - per Boris suggestion
   - No need use pointer type for tot_size
    => We can remove this from the caller too as it is unnecessary
   - if ( mpbuf->len + CONT_HDR_SIZE >= *tot_size ) check can be
     removed
 
 -  Fix logic
   - if the first container is corrupted for some reason and returns error
     then we return pre-maturely. Instead, now we at least (try to) forward
     to next container and look for patches there as well
   - Using AMD_MAX_CONT_APPEND as loop condition check simply because I
     wanted to avoid using a while (1) which would also work just as well
   - This check makes some logical sense too as there are only 2 available
     AMD containers out there now. So if we did not find correct patch by then,
     we might as well stop trying.

 - Adding Jan, Boris reviewed-by tags; Dropping Suravee's since logic changed..

Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@xxxxxxx>
Reviewed-by: Jan Beulich <JBeulich@xxxxxxxx>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
---
 xen/arch/x86/microcode_amd.c |  141 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 117 insertions(+), 24 deletions(-)

diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index e83f4b6..9e5fa9a 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -29,6 +29,17 @@
 
 #define pr_debug(x...) ((void)0)
 
+#define CONT_HDR_SIZE           12
+#define SECTION_HDR_SIZE        8
+#define AMD_UCODE_APPLY_SUCCESS 0
+/*
+ * Currently, there are two AMD container files on
+ * 
https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/amd-ucode
+ * So, this is max number of container binaries that can be appended
+ * together and packaged along with initrd.
+ */
+#define AMD_MAX_CONT_APPEND     2
+
 struct __packed equiv_cpu_entry {
     uint32_t installed_cpu;
     uint32_t fixed_errata_mask;
@@ -124,30 +135,43 @@ static bool_t verify_patch_size(uint32_t patch_size)
     return (patch_size <= max_size);
 }
 
-static bool_t microcode_fits(const struct microcode_amd *mc_amd, int cpu)
+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)
 {
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
-    const struct microcode_header_amd *mc_header = mc_amd->mpb;
-    const struct equiv_cpu_entry *equiv_cpu_table = mc_amd->equiv_cpu_table;
-    unsigned int current_cpu_id;
-    unsigned int equiv_cpu_id = 0x0;
     unsigned int i;
+    unsigned int found = 0;
 
-    /* We should bind the task to the CPU */
-    BUG_ON(cpu != raw_smp_processor_id());
-
-    current_cpu_id = cpuid_eax(0x00000001);
+    if ( !equiv_cpu_table )
+        return 0;
 
     for ( i = 0; equiv_cpu_table[i].installed_cpu != 0; i++ )
     {
         if ( current_cpu_id == equiv_cpu_table[i].installed_cpu )
         {
-            equiv_cpu_id = equiv_cpu_table[i].equiv_cpu & 0xffff;
+            *equiv_cpu_id = equiv_cpu_table[i].equiv_cpu & 0xffff;
+            found = 1;
             break;
         }
     }
 
-    if ( !equiv_cpu_id )
+    return found;
+}
+
+static bool_t microcode_fits(const struct microcode_amd *mc_amd, int cpu)
+{
+    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+    const struct microcode_header_amd *mc_header = mc_amd->mpb;
+    const struct equiv_cpu_entry *equiv_cpu_table = mc_amd->equiv_cpu_table;
+    unsigned int current_cpu_id;
+    unsigned int equiv_cpu_id;
+
+    /* We should bind the task to the CPU */
+    BUG_ON(cpu != raw_smp_processor_id());
+
+    current_cpu_id = cpuid_eax(0x00000001);
+
+     if ( !find_equiv_cpu_id(equiv_cpu_table, current_cpu_id, &equiv_cpu_id) )
         return 0;
 
     if ( (mc_header->processor_rev_id) != equiv_cpu_id )
@@ -211,7 +235,7 @@ static int apply_microcode(int cpu)
 
     uci->cpu_sig.rev = rev;
 
-    return 0;
+    return AMD_UCODE_APPLY_SUCCESS;
 }
 
 static int get_ucode_from_buffer_amd(
@@ -236,7 +260,15 @@ static int get_ucode_from_buffer_amd(
     mpbuf = (const struct mpbhdr *)&bufp[off];
     if ( mpbuf->type != UCODE_UCODE_TYPE )
     {
-        printk(KERN_ERR "microcode: Wrong microcode payload type field\n");
+        /*
+         * We will hit this condition only if an update has succeeded
+         * but there is still another container file being parsed.
+         * In that case, there is no need of this ERR message to be
+         * printed.
+         */
+        if ( mpbuf->type != UCODE_MAGIC )
+            printk(KERN_ERR "microcode: Wrong microcode payload type field\n");
+
         return -EINVAL;
     }
 
@@ -260,7 +292,7 @@ static int get_ucode_from_buffer_amd(
     }
     memcpy(mc_amd->mpb, mpbuf->data, mpbuf->len);
 
-    *offset = off + mpbuf->len + 8;
+    *offset = off + mpbuf->len + SECTION_HDR_SIZE;
 
     pr_debug("microcode: CPU%d size %zu, block size %u offset %zu equivID %#x 
rev %#x\n",
              raw_smp_processor_id(), bufsize, mpbuf->len, off,
@@ -272,14 +304,13 @@ static int get_ucode_from_buffer_amd(
 
 static int install_equiv_cpu_table(
     struct microcode_amd *mc_amd,
-    const uint32_t *buf,
-    size_t *offset)
+    const void *data,
+    size_t *curr_offset)
 {
+    uint32_t *buf = (uint32_t *) (data + *curr_offset);
     const struct mpbhdr *mpbuf = (const struct mpbhdr *)&buf[1];
 
-    /* No more data */
-    if ( mpbuf->len + 12 >= *offset )
-        return -EINVAL;
+    *curr_offset += mpbuf->len + CONT_HDR_SIZE;        /* add header length */
 
     if ( mpbuf->type != UCODE_EQUIV_CPU_TABLE_TYPE )
     {
@@ -303,7 +334,30 @@ static int install_equiv_cpu_table(
     memcpy(mc_amd->equiv_cpu_table, mpbuf->data, mpbuf->len);
     mc_amd->equiv_cpu_table_size = mpbuf->len;
 
-    *offset = mpbuf->len + 12; /* add header length */
+    return 0;
+}
+
+static int container_fast_forward(const void *data, size_t size_left, size_t 
*offset)
+{
+    size_t size;
+    uint32_t *header;
+
+    while ( size_left )
+    {
+        header = (uint32_t *) (data + *offset);
+        if ( header[0] == UCODE_MAGIC &&
+             header[1] == UCODE_EQUIV_CPU_TABLE_TYPE )
+            break;
+
+        size = header[1] + SECTION_HDR_SIZE;
+        *offset += size;
+
+        if ( size_left >= size )
+            size_left -= size;
+    }
+
+    if ( !size_left )
+        return -EINVAL;
 
     return 0;
 }
@@ -311,14 +365,19 @@ static int install_equiv_cpu_table(
 static int cpu_request_microcode(int cpu, const void *buf, size_t bufsize)
 {
     struct microcode_amd *mc_amd, *mc_old;
-    size_t offset = bufsize;
+    size_t offset = 0;
     size_t last_offset, applied_offset = 0;
     int error = 0, save_error = 1;
     struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+    unsigned int current_cpu_id;
+    unsigned int equiv_cpu_id;
+    unsigned int containers_processed = 0;
 
     /* We should bind the task to the CPU */
     BUG_ON(cpu != raw_smp_processor_id());
 
+    current_cpu_id = cpuid_eax(0x00000001);
+
     if ( *(const uint32_t *)buf != UCODE_MAGIC )
     {
         printk(KERN_ERR "microcode: Wrong microcode patch file magic\n");
@@ -334,7 +393,33 @@ static int cpu_request_microcode(int cpu, const void *buf, 
size_t bufsize)
         goto out;
     }
 
-    error = install_equiv_cpu_table(mc_amd, buf, &offset);
+    /*
+     * Multiple container file support:
+     * 1. check if this container file has equiv_cpu_id match
+     * 2. If not, fast-fwd to next container file
+     */
+    while ( containers_processed <= AMD_MAX_CONT_APPEND )
+    {
+        containers_processed++;
+        error = install_equiv_cpu_table(mc_amd, buf, &offset);
+
+        if ( !error )
+        {
+             if ( find_equiv_cpu_id(mc_amd->equiv_cpu_table, current_cpu_id,
+                                    &equiv_cpu_id) )
+                break;
+        }
+
+        error = container_fast_forward(buf, (bufsize - offset), &offset);
+        if ( error )
+        {
+            printk(KERN_ERR "microcode: CPU%d incorrect or corrupt container 
file\n"
+                   "microcode: Failed to update patch level. "
+                   "Current lvl:%#x\n", cpu, uci->cpu_sig.rev);
+            goto out;
+        }
+    }
+
     if ( error )
     {
         xfree(mc_amd);
@@ -379,7 +464,15 @@ static int cpu_request_microcode(int cpu, const void *buf, 
size_t bufsize)
         save_error = get_ucode_from_buffer_amd(
             mc_amd, buf, bufsize, &applied_offset);
 
-        if ( save_error )
+        /*
+         * If there happens to be multiple container files and if patch
+         * update succeeded on an earlier container itself, a stale error
+         * val is returned from get_ucode_from_buffer_amd. Since we already
+         * succeeded in patch application, return as success itself
+         */
+        if ( (containers_processed < AMD_MAX_CONT_APPEND) && error )
+            error = AMD_UCODE_APPLY_SUCCESS;
+        else if ( save_error )
             error = save_error;
     }
 
-- 
1.7.9.5


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel