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

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



On 06/19/2014 11:14 AM, Aravind Gopalakrishnan wrote:
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

While at it, define HDR_SIZES and use these for clarity.

Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@xxxxxxx>
Reviewed-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@xxxxxxx>
---
  xen/arch/x86/microcode_amd.c |  120 ++++++++++++++++++++++++++++++++++++------
  1 file changed, 103 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index e83f4b6..6478dda 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -29,6 +29,9 @@
#define pr_debug(x...) ((void)0) +#define CONT_HDR_SIZE 12
+#define SECTION_HDR_SIZE    8
+
  struct __packed equiv_cpu_entry {
      uint32_t installed_cpu;
      uint32_t fixed_errata_mask;
@@ -124,6 +127,25 @@ static bool_t verify_patch_size(uint32_t patch_size)
      return (patch_size <= max_size);
  }
+static void find_equiv_cpu_id(const struct equiv_cpu_entry *equiv_cpu_table,
+                              unsigned int current_cpu_id,
+                              unsigned int *equiv_cpu_id)
+{
+    unsigned int i;
+
+    if ( !equiv_cpu_table )
+        return;
+
+    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;
+            break;
+        }
+    }
+}
+
  static bool_t microcode_fits(const struct microcode_amd *mc_amd, int cpu)
  {
      struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
@@ -131,21 +153,13 @@ static bool_t microcode_fits(const struct microcode_amd 
*mc_amd, int cpu)
      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;
/* We should bind the task to the CPU */
      BUG_ON(cpu != raw_smp_processor_id());
current_cpu_id = cpuid_eax(0x00000001); - 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;
-            break;
-        }
-    }
+    find_equiv_cpu_id(equiv_cpu_table, current_cpu_id, &equiv_cpu_id);
if ( !equiv_cpu_id )
          return 0;
@@ -236,7 +250,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 +282,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,13 +294,16 @@ 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 uint8_t *data,
+    size_t *tot_size,

Why is this a pointer? This routine does not modify the value.

+    size_t *curr_offset)
  {
+    size_t off = *curr_offset;
+    uint32_t *buf = (uint32_t *) &data[off];
      const struct mpbhdr *mpbuf = (const struct mpbhdr *)&buf[1];
/* No more data */
-    if ( mpbuf->len + 12 >= *offset )
+    if ( mpbuf->len + CONT_HDR_SIZE >= *tot_size )
          return -EINVAL;
if ( mpbuf->type != UCODE_EQUIV_CPU_TABLE_TYPE )
@@ -303,7 +328,32 @@ 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 */
+    *curr_offset += mpbuf->len + CONT_HDR_SIZE;     /* add header length */
+
+    return 0;
+}
+
+static int container_fast_forward(const uint8_t *data, size_t size_left, 
size_t *offset)
+{
+    size_t size, off;
+    uint32_t *header;
+
+    while ( size_left )
+    {
+        off = *offset;
+        header = (uint32_t *) &data[off];
+        if ( header[0] == UCODE_MAGIC &&
+             header[1] == UCODE_EQUIV_CPU_TABLE_TYPE )
+            break;
+
+        size = header[1] + SECTION_HDR_SIZE;
+        *offset += size;
+        size_left -= size;
+
+    }
+
+    if ( !size_left )
+        return -EINVAL;
return 0;
  }
@@ -311,14 +361,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 tot_size = bufsize;
      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 = 0x0;
/* 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 +389,30 @@ 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 ( (error = install_equiv_cpu_table(mc_amd, buf, &tot_size,
+                                             &offset)) == 0 )

install_equiv_cpu_table() checks whether container size is larger than the buffer size. If we have multiple containers merged I think tot_size is the size of the merged file, not of an individual container. And this makes the first check in install_equiv_cpu_table() not especially meaningful.

-boris

+    {
+        find_equiv_cpu_id(mc_amd->equiv_cpu_table, current_cpu_id,
+                          &equiv_cpu_id);
+        if ( 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);
+            error = -EINVAL;
+            goto out;
+        }
+    }
+
      if ( error )
      {
          xfree(mc_amd);
@@ -379,8 +457,16 @@ 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 there happens to be multiple container files and if patch
+         * update succeeded on first container itself, a stale error val
+         * is returned from get_ucode_from_buffer_amd. So, force
+         * error = 0 here as we have already succeeded in the update.
+         */
          if ( save_error )
              error = save_error;
+        else
+            error = 0;
      }
if ( save_error )


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


 


Rackspace

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