[Xen-changelog] [xen staging] x86/microcode: avoid unnecessary xmalloc/memcpy of ucode data

[patchbot@xxxxxxx]

commit e6eb81a028ba610de43bc966ced5d95bafe8911b
Author:     Eslam Elnikety <elnikety@xxxxxxxxxx>
AuthorDate: Fri Jan 24 10:31:21 2020 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Jan 24 10:31:21 2020 +0100

    x86/microcode: avoid unnecessary xmalloc/memcpy of ucode data
    
    When using `ucode=scan` and if a matching module is found, the microcode
    payload is maintained in an xmalloc()'d region. This is unnecessary since
    the bootmap would just do. Remove the xmalloc and xfree on the microcode
    module scan path.
    
    This commit also does away with the restriction on the microcode module
    size limit. The concern that a large microcode module would consume too
    much memory preventing guests launch is misplaced since this is all the
    init path. While having such safeguards is valuable, this should apply
    across the board for all early/late microcode loading. Having it just on
    the `scan` path is confusing.
    
    Looking forward, we are a bit closer (i.e., one xmalloc down) to pulling
    the early microcode loading of the BSP a bit earlier in the early boot
    process. This commit is the low hanging fruit. There is still a sizable
    amount of work to get there as there are still a handful of xmalloc in
    microcode_{amd,intel}.c.
    
    First, there are xmallocs on the path of finding a matching microcode
    update. Similar to the commit at hand, searching through the microcode
    blob can be done on the already present buffer with no need to xmalloc
    any further. Even better, do the filtering in microcode.c before
    requesting the microcode update on all CPUs. The latter requires careful
    restructuring and exposing the arch-specific logic for iterating over
    patches and declaring a match.
    
    Second, there are xmallocs for the microcode cache. Here, we would need
    to ensure that the cache corresponding to the BSP gets xmalloc()'d and
    populated after the fact.
    
    Signed-off-by: Eslam Elnikety <elnikety@xxxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/microcode.c | 32 ++++----------------------------
 1 file changed, 4 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 6ced293d88..844a33bbd6 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -147,11 +147,6 @@ static int __init parse_ucode(const char *s)
 }
 custom_param("ucode", parse_ucode);
 
-/*
- * 8MB ought to be enough.
- */
-#define MAX_EARLY_CPIO_MICROCODE (8 << 20)
-
 void __init microcode_scan_module(
     unsigned long *module_map,
     const multiboot_info_t *mbi)
@@ -196,31 +191,12 @@ void __init microcode_scan_module(
         cd = find_cpio_data(p, _blob_start, _blob_size, &offset /* ignore */);
         if ( cd.data )
         {
-                /*
-                 * This is an arbitrary check - it would be sad if the blob
-                 * consumed most of the memory and did not allow guests
-                 * to launch.
-                 */
-                if ( cd.size > MAX_EARLY_CPIO_MICROCODE )
-                {
-                    printk("Multiboot %d microcode payload too big! (%ld, we 
can do %d)\n",
-                           i, cd.size, MAX_EARLY_CPIO_MICROCODE);
-                    goto err;
-                }
-                ucode_blob.size = cd.size;
-                ucode_blob.data = xmalloc_bytes(cd.size);
-                if ( !ucode_blob.data )
-                    cd.data = NULL;
-                else
-                    memcpy(ucode_blob.data, cd.data, cd.size);
+            ucode_blob.size = cd.size;
+            ucode_blob.data = cd.data;
+            break;
         }
         bootstrap_map(NULL);
-        if ( cd.data )
-            break;
     }
-    return;
-err:
-    bootstrap_map(NULL);
 }
 void __init microcode_grab_module(
     unsigned long *module_map,
@@ -730,7 +706,7 @@ static int __init microcode_init(void)
      */
     if ( ucode_blob.size )
     {
-        xfree(ucode_blob.data);
+        bootstrap_map(NULL);
         ucode_blob.size = 0;
         ucode_blob.data = NULL;
     }
--
generated by git-patchbot for /home/xen/git/xen.git#staging

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog