[PATCH v2 0/3] x86: load microcode earlier on boot CPU

[PATCH v2 0/3] x86: load microcode earlier on boot CPU

[Sergey Dyasli]

The second version of patches. Changelog is available in each patch.

Sergey Dyasli (3):
  xen/multiboot: add proper struct definitions to typedefs
  x86/ucode: allow cpu_request_microcode() to skip memory allocation
  x86/ucode: load microcode earlier on boot CPU

 xen/arch/x86/cpu/microcode/amd.c     | 13 ++++--
 xen/arch/x86/cpu/microcode/core.c    | 70 +++++++++++++++++++++++-----
 xen/arch/x86/cpu/microcode/intel.c   | 13 ++++--
 xen/arch/x86/cpu/microcode/private.h | 15 ++++--
 xen/arch/x86/include/asm/microcode.h |  7 ++-
 xen/arch/x86/include/asm/setup.h     |  3 --
 xen/arch/x86/setup.c                 | 10 ++--
 xen/include/xen/multiboot.h          | 25 ++++++----
 8 files changed, 115 insertions(+), 41 deletions(-)

-- 
2.17.1

[PATCH v2 1/3] xen/multiboot: add proper struct definitions to typedefs

[Sergey Dyasli]

This allows to use them for forward declaration in other headers.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

---
CC: George Dunlap <george.dunlap@citrix.com>
CC: Julien Grall <julien@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>

v1 --> v2:
- New patch
---
 xen/include/xen/multiboot.h | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/xen/include/xen/multiboot.h b/xen/include/xen/multiboot.h
index d1b43e1183..a541bdf8a8 100644
--- a/xen/include/xen/multiboot.h
+++ b/xen/include/xen/multiboot.h
@@ -46,23 +46,25 @@
 #ifndef __ASSEMBLY__
 
 /* The symbol table for a.out.  */
-typedef struct {
+struct aout_symbol_table {
     u32 tabsize;
     u32 strsize;
     u32 addr;
     u32 reserved;
-} aout_symbol_table_t;
+};
+typedef struct aout_symbol_table aout_symbol_table_t;
 
 /* The section header table for ELF.  */
-typedef struct {
+struct elf_section_header_table{
     u32 num;
     u32 size;
     u32 addr;
     u32 shndx;
-} elf_section_header_table_t;
+};
+typedef struct elf_section_header_table elf_section_header_table_t;
 
 /* The Multiboot information.  */
-typedef struct {
+struct multiboot_info {
     u32 flags;
 
     /* Valid if flags sets MBI_MEMLIMITS */
@@ -101,26 +103,29 @@ typedef struct {
 
     /* Valid if flags sets MBI_APM */
     u32 apm_table;
-} multiboot_info_t;
+};
+typedef struct multiboot_info multiboot_info_t;
 
 /* The module structure.  */
-typedef struct {
+struct module {
     u32 mod_start;
     u32 mod_end;
     u32 string;
     u32 reserved;
-} module_t;
+};
+typedef struct module module_t;
 
 /* The memory map. Be careful that the offset 0 is base_addr_low
    but no size.  */
-typedef struct {
+struct memory_map {
     u32 size;
     u32 base_addr_low;
     u32 base_addr_high;
     u32 length_low;
     u32 length_high;
     u32 type;
-} memory_map_t;
+};
+typedef struct memory_map memory_map_t;
 
 
 #endif /* __ASSEMBLY__ */
-- 
2.17.1

[PATCH v2 2/3] x86/ucode: allow cpu_request_microcode() to skip memory allocation

[Sergey Dyasli]

This is a preparatory step in order to do earlier microcode loading on
the boot CPU when the domain heap has not been initialized yet and
xmalloc still unavailable.

Add make_copy argument which will allow to load microcode directly from
the blob bypassing microcode_cache.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

---
v1 --> v2:
- Don't add extra consts
---
 xen/arch/x86/cpu/microcode/amd.c     | 13 +++++++++----
 xen/arch/x86/cpu/microcode/core.c    |  2 +-
 xen/arch/x86/cpu/microcode/intel.c   | 13 +++++++++----
 xen/arch/x86/cpu/microcode/private.h | 15 +++++++++++----
 4 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 8195707ee1..4b097187a0 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -300,7 +300,7 @@ static int scan_equiv_cpu_table(const struct container_equiv_table *et)
 }
 
 static struct microcode_patch *cf_check cpu_request_microcode(
-    const void *buf, size_t size)
+    const void *buf, size_t size, bool make_copy)
 {
     const struct microcode_patch *saved = NULL;
     struct microcode_patch *patch = NULL;
@@ -411,9 +411,14 @@ static struct microcode_patch *cf_check cpu_request_microcode(
 
     if ( saved )
     {
-        patch = xmemdup_bytes(saved, saved_size);
-        if ( !patch )
-            error = -ENOMEM;
+        if ( make_copy )
+        {
+            patch = xmemdup_bytes(saved, saved_size);
+            if ( !patch )
+                error = -ENOMEM;
+        }
+        else
+            patch = (struct microcode_patch *)saved;
     }
 
     if ( error && !patch )
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 452a7ca773..85c05e480d 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -244,7 +244,7 @@ static struct microcode_patch *parse_blob(const char *buf, size_t len)
 {
     alternative_vcall(ucode_ops.collect_cpu_info);
 
-    return alternative_call(ucode_ops.cpu_request_microcode, buf, len);
+    return alternative_call(ucode_ops.cpu_request_microcode, buf, len, true);
 }
 
 static void microcode_free_patch(struct microcode_patch *patch)
diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c
index f5ba6d76d7..f7fec4b4ed 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -324,7 +324,7 @@ static int cf_check apply_microcode(const struct microcode_patch *patch)
 }
 
 static struct microcode_patch *cf_check cpu_request_microcode(
-    const void *buf, size_t size)
+    const void *buf, size_t size, bool make_copy)
 {
     int error = 0;
     const struct microcode_patch *saved = NULL;
@@ -364,10 +364,15 @@ static struct microcode_patch *cf_check cpu_request_microcode(
 
     if ( saved )
     {
-        patch = xmemdup_bytes(saved, get_totalsize(saved));
+        if ( make_copy )
+        {
+            patch = xmemdup_bytes(saved, get_totalsize(saved));
 
-        if ( !patch )
-            error = -ENOMEM;
+            if ( !patch )
+                error = -ENOMEM;
+        }
+        else
+            patch = (struct microcode_patch *)saved;
     }
 
     if ( error && !patch )
diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h
index c085a10268..58c5dffd7b 100644
--- a/xen/arch/x86/cpu/microcode/private.h
+++ b/xen/arch/x86/cpu/microcode/private.h
@@ -23,15 +23,22 @@ struct microcode_ops {
      * older that what is running in the CPU.  This is a feature, to better
      * cope with corner cases from buggy firmware.)
      *
-     * If one is found, allocate and return a struct microcode_patch
-     * encapsulating the appropriate microcode patch.  Does not alias the
-     * original buffer.  Must be suitable to be freed with a single xfree().
+     * If one is found, behaviour depends on the make_copy argument:
+     *
+     *     true: allocate and return a struct microcode_patch encapsulating
+     *           the appropriate microcode patch.  Does not alias the original
+     *           buffer.  Must be suitable to be freed with a single xfree().
+     *
+     *    false: return a pointer to the patch within the original buffer.
+     *           This is useful for early microcode loading when xmalloc might
+     *           not be available yet.
      *
      * If one is not found, (nothing matches the current CPU), return NULL.
      * Also may return ERR_PTR(-err), e.g. bad container, out of memory.
      */
     struct microcode_patch *(*cpu_request_microcode)(const void *buf,
-                                                     size_t size);
+                                                     size_t size,
+                                                     bool make_copy);
 
     /*
      * Obtain microcode-relevant details for the current CPU.  Results in
-- 
2.17.1

[PATCH v2 3/3] x86/ucode: load microcode earlier on boot CPU

[Sergey Dyasli]

Call early_microcode_init() straight after multiboot modules become
accessible. Modify it to load the ucode directly from the blob bypassing
populating microcode_cache because xmalloc is still not available at
that point during Xen boot.

Introduce early_microcode_init_cache() for populating microcode_cache.
It needs to rescan the modules in order to find the new virtual address
of the ucode blob because it changes during the boot process, e.g.
from 0x00000000010802fc to 0xffff83204dac52fc.

While at it, drop alternative_vcall() from early_microcode_init() since
it's not useful in an __init fuction.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

---
v1 --> v2:
- Don't call microcode_grab_module() the second time, use
  microcode_scan_module() instead
- Use forward declaration of struct multiboot_info
- Don't use alternative calls
- Rename early_microcode_update_cache() to early_update_cache() and
  move it around a bit
---
 xen/arch/x86/cpu/microcode/core.c    | 66 +++++++++++++++++++++++-----
 xen/arch/x86/include/asm/microcode.h |  7 ++-
 xen/arch/x86/include/asm/setup.h     |  3 --
 xen/arch/x86/setup.c                 | 10 +++--
 4 files changed, 68 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 85c05e480d..04b5d346ab 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -27,6 +27,7 @@
 #include <xen/err.h>
 #include <xen/guest_access.h>
 #include <xen/init.h>
+#include <xen/multiboot.h>
 #include <xen/param.h>
 #include <xen/spinlock.h>
 #include <xen/stop_machine.h>
@@ -198,7 +199,8 @@ void __init microcode_scan_module(
         bootstrap_map(NULL);
     }
 }
-void __init microcode_grab_module(
+
+static void __init microcode_grab_module(
     unsigned long *module_map,
     const multiboot_info_t *mbi)
 {
@@ -732,10 +734,54 @@ int microcode_update_one(void)
     return microcode_update_cpu(NULL);
 }
 
+static int __init early_update_cache(const void *data, size_t len)
+{
+    int rc = 0;
+    struct microcode_patch *patch;
+
+    if ( !data )
+        return -ENOMEM;
+
+    patch = parse_blob(data, len);
+    if ( IS_ERR(patch) )
+    {
+        printk(XENLOG_WARNING "Parsing microcode blob error %ld\n",
+               PTR_ERR(patch));
+        return PTR_ERR(patch);
+    }
+
+    if ( !patch )
+        return -ENOENT;
+
+    spin_lock(&microcode_mutex);
+    rc = microcode_update_cache(patch);
+    spin_unlock(&microcode_mutex);
+    ASSERT(rc);
+
+    return rc;
+}
+
+int __init early_microcode_init_cache(unsigned long *module_map,
+                                      const struct multiboot_info *mbi)
+{
+    int rc = 0;
+
+    if ( ucode_scan )
+        /* Need to rescan the modules because they might have been relocated */
+        microcode_scan_module(module_map, mbi);
+
+    if ( ucode_mod.mod_end )
+        rc = early_update_cache(bootstrap_map(&ucode_mod),
+                                ucode_mod.mod_end);
+    else if ( ucode_blob.size )
+        rc = early_update_cache(ucode_blob.data, ucode_blob.size);
+
+    return rc;
+}
+
 /* BSP calls this function to parse ucode blob and then apply an update. */
 static int __init early_microcode_update_cpu(void)
 {
-    int rc = 0;
     const void *data = NULL;
     size_t len;
     struct microcode_patch *patch;
@@ -754,7 +800,7 @@ static int __init early_microcode_update_cpu(void)
     if ( !data )
         return -ENOMEM;
 
-    patch = parse_blob(data, len);
+    patch = ucode_ops.cpu_request_microcode(data, len, false);
     if ( IS_ERR(patch) )
     {
         printk(XENLOG_WARNING "Parsing microcode blob error %ld\n",
@@ -765,15 +811,11 @@ static int __init early_microcode_update_cpu(void)
     if ( !patch )
         return -ENOENT;
 
-    spin_lock(&microcode_mutex);
-    rc = microcode_update_cache(patch);
-    spin_unlock(&microcode_mutex);
-    ASSERT(rc);
-
-    return microcode_update_one();
+    return microcode_update_cpu(patch);
 }
 
-int __init early_microcode_init(void)
+int __init early_microcode_init(unsigned long *module_map,
+                                const struct multiboot_info *mbi)
 {
     const struct cpuinfo_x86 *c = &boot_cpu_data;
     int rc = 0;
@@ -797,7 +839,9 @@ int __init early_microcode_init(void)
         return -ENODEV;
     }
 
-    alternative_vcall(ucode_ops.collect_cpu_info);
+    microcode_grab_module(module_map, mbi);
+
+    ucode_ops.collect_cpu_info();
 
     if ( ucode_mod.mod_end || ucode_blob.size )
         rc = early_microcode_update_cpu();
diff --git a/xen/arch/x86/include/asm/microcode.h b/xen/arch/x86/include/asm/microcode.h
index 3b0234e9fa..170481d257 100644
--- a/xen/arch/x86/include/asm/microcode.h
+++ b/xen/arch/x86/include/asm/microcode.h
@@ -6,6 +6,8 @@
 
 #include <public/xen.h>
 
+struct multiboot_info;
+
 struct cpu_signature {
     /* CPU signature (CPUID.1.EAX). */
     unsigned int sig;
@@ -21,7 +23,10 @@ DECLARE_PER_CPU(struct cpu_signature, cpu_sig);
 
 void microcode_set_module(unsigned int idx);
 int microcode_update(XEN_GUEST_HANDLE(const_void), unsigned long len);
-int early_microcode_init(void);
+int early_microcode_init(unsigned long *module_map,
+                         const struct multiboot_info *mbi);
+int early_microcode_init_cache(unsigned long *module_map,
+                               const struct multiboot_info *mbi);
 int microcode_update_one(void);
 
 #endif /* ASM_X86__MICROCODE_H */
diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index ae470ea12f..ae0dd3915a 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -44,9 +44,6 @@ void *bootstrap_map(const module_t *mod);
 
 int xen_in_range(unsigned long mfn);
 
-void microcode_grab_module(
-    unsigned long *, const multiboot_info_t *);
-
 extern uint8_t kbd_shift_flags;
 
 #ifdef NDEBUG
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 6bb5bc7c84..2d7c815e0a 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1178,6 +1178,12 @@ void __init noreturn __start_xen(unsigned long mbi_p)
         mod[i].reserved = 0;
     }
 
+    /*
+     * TODO: load ucode earlier once multiboot modules become accessible
+     * at an earlier stage.
+     */
+    early_microcode_init(module_map, mbi);
+
     if ( xen_phys_start )
     {
         relocated = true;
@@ -1762,11 +1768,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     init_IRQ();
 
-    microcode_grab_module(module_map, mbi);
-
     timer_init();
 
-    early_microcode_init();
+    early_microcode_init_cache(module_map, mbi);
 
     tsx_init(); /* Needs microcode.  May change HLE/RTM feature bits. */
 
-- 
2.17.1

Re: [PATCH v2 3/3] x86/ucode: load microcode earlier on boot CPU

[Julien Grall]

Hi Sergey,

On 19/12/2022 14:45, Sergey Dyasli wrote:
> Call early_microcode_init() straight after multiboot modules become
> accessible. Modify it to load the ucode directly from the blob bypassing
> populating microcode_cache because xmalloc is still not available at
> that point during Xen boot.
> 
> Introduce early_microcode_init_cache() for populating microcode_cache.
> It needs to rescan the modules in order to find the new virtual address
> of the ucode blob because it changes during the boot process, e.g.
> from 0x00000000010802fc to 0xffff83204dac52fc.
> 
> While at it, drop alternative_vcall() from early_microcode_init() since
> it's not useful in an __init fuction.

Can you explain in the commit message why you need to load the microcode 
early? E.g. is it a nice feature to have or a real issue?

If the latter, then I think we will need to consider the patches for 
backport.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 3/3] x86/ucode: load microcode earlier on boot CPU

[Andrew Cooper]

On 19/12/2022 4:57 pm, Julien Grall wrote:
> Hi Sergey,
>
> On 19/12/2022 14:45, Sergey Dyasli wrote:
>> Call early_microcode_init() straight after multiboot modules become
>> accessible. Modify it to load the ucode directly from the blob bypassing
>> populating microcode_cache because xmalloc is still not available at
>> that point during Xen boot.
>>
>> Introduce early_microcode_init_cache() for populating microcode_cache.
>> It needs to rescan the modules in order to find the new virtual address
>> of the ucode blob because it changes during the boot process, e.g.
>> from 0x00000000010802fc to 0xffff83204dac52fc.
>>
>> While at it, drop alternative_vcall() from early_microcode_init() since
>> it's not useful in an __init fuction.
>
> Can you explain in the commit message why you need to load the
> microcode early? E.g. is it a nice feature to have or a real issue?
>
> If the latter, then I think we will need to consider the patches for
> backport.

Microcode loading should be as early as possible.  Linux does it even
before setting up the console (which is a bit too early IMO).

Xen currently loads microcode half way through BSP boot, because there's
a inappropriate dependency on needing xmalloc().  This is what Sergey is
addressing with this series.

I'm working on addressing the TODO in the penultimate hunk of this patch
(resolving some major abuse with with the multiboot module structures),
which will let us load microcode even earlier.

A consequence of this (relatively) late loading is that we've got a
tangle of feature enumeration logic where cpu_has_* doesn't fully work
before ucode load, and we've got a lot of ad-hoc logic which is fragile.


So no - there's not a specific bug driving this, but a lot of cleanup
that I've been wanting to do since before speculation came along.

~Andrew

Re: [PATCH v2 3/3] x86/ucode: load microcode earlier on boot CPU

[Julien Grall]

On 19/12/2022 17:33, Andrew Cooper wrote:
> On 19/12/2022 4:57 pm, Julien Grall wrote:
>> Hi Sergey,
>>
>> On 19/12/2022 14:45, Sergey Dyasli wrote:
>>> Call early_microcode_init() straight after multiboot modules become
>>> accessible. Modify it to load the ucode directly from the blob bypassing
>>> populating microcode_cache because xmalloc is still not available at
>>> that point during Xen boot.
>>>
>>> Introduce early_microcode_init_cache() for populating microcode_cache.
>>> It needs to rescan the modules in order to find the new virtual address
>>> of the ucode blob because it changes during the boot process, e.g.
>>> from 0x00000000010802fc to 0xffff83204dac52fc.
>>>
>>> While at it, drop alternative_vcall() from early_microcode_init() since
>>> it's not useful in an __init fuction.
>>
>> Can you explain in the commit message why you need to load the
>> microcode early? E.g. is it a nice feature to have or a real issue?
>>
>> If the latter, then I think we will need to consider the patches for
>> backport.
> 
> Microcode loading should be as early as possible.  Linux does it even
> before setting up the console (which is a bit too early IMO).
> 
> Xen currently loads microcode half way through BSP boot, because there's
> a inappropriate dependency on needing xmalloc().  This is what Sergey is
> addressing with this series.
> 
> I'm working on addressing the TODO in the penultimate hunk of this patch
> (resolving some major abuse with with the multiboot module structures),
> which will let us load microcode even earlier.
> 
> A consequence of this (relatively) late loading is that we've got a
> tangle of feature enumeration logic where cpu_has_* doesn't fully work
> before ucode load, and we've got a lot of ad-hoc logic which is fragile.
> 
> 
> So no - there's not a specific bug driving this, but a lot of cleanup
> that I've been wanting to do since before speculation came along.

Thanks for the clarification!

Cheers,

-- 
Julien Grall

Re: [PATCH v2 1/3] xen/multiboot: add proper struct definitions to typedefs

[Jan Beulich]

On 19.12.2022 15:45, Sergey Dyasli wrote:
> This allows to use them for forward declaration in other headers.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with ...

> --- a/xen/include/xen/multiboot.h
> +++ b/xen/include/xen/multiboot.h
> @@ -46,23 +46,25 @@
>  #ifndef __ASSEMBLY__
>  
>  /* The symbol table for a.out.  */
> -typedef struct {
> +struct aout_symbol_table {
>      u32 tabsize;
>      u32 strsize;
>      u32 addr;
>      u32 reserved;
> -} aout_symbol_table_t;
> +};
> +typedef struct aout_symbol_table aout_symbol_table_t;
>  
>  /* The section header table for ELF.  */
> -typedef struct {
> +struct elf_section_header_table{

... the missing blank added here.

I also have to admit that I would have preferred a variant with less
code churn: The typedef re-arrangement really isn't needed to fulfill
the goal pf the change.

Jan

Re: [PATCH v2 2/3] x86/ucode: allow cpu_request_microcode() to skip memory allocation

[Jan Beulich]

On 19.12.2022 15:45, Sergey Dyasli wrote:
> This is a preparatory step in order to do earlier microcode loading on
> the boot CPU when the domain heap has not been initialized yet and
> xmalloc still unavailable.
> 
> Add make_copy argument which will allow to load microcode directly from
> the blob bypassing microcode_cache.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v2 3/3] x86/ucode: load microcode earlier on boot CPU

[Jan Beulich]

On 19.12.2022 15:45, Sergey Dyasli wrote:
> Call early_microcode_init() straight after multiboot modules become
> accessible. Modify it to load the ucode directly from the blob bypassing
> populating microcode_cache because xmalloc is still not available at
> that point during Xen boot.
> 
> Introduce early_microcode_init_cache() for populating microcode_cache.
> It needs to rescan the modules in order to find the new virtual address
> of the ucode blob because it changes during the boot process, e.g.
> from 0x00000000010802fc to 0xffff83204dac52fc.
> 
> While at it, drop alternative_vcall() from early_microcode_init() since
> it's not useful in an __init fuction.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v2 3/3] x86/ucode: load microcode earlier on boot CPU

[Andrew Cooper]

On 19/12/2022 2:45 pm, Sergey Dyasli wrote:
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 6bb5bc7c84..2d7c815e0a 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
>          relocated = true;
> @@ -1762,11 +1768,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  
>      init_IRQ();
>  
> -    microcode_grab_module(module_map, mbi);
> -
>      timer_init();
>  
> -    early_microcode_init();
> +    early_microcode_init_cache(module_map, mbi);

microcode_init_cache(module_map, mbi); /* Needs xmalloc() */

Can fix on commit.

~Andrew

Re: [PATCH v2 3/3] x86/ucode: load microcode earlier on boot CPU

[Jan Beulich]

On 20.12.2022 15:50, Andrew Cooper wrote:
> On 19/12/2022 2:45 pm, Sergey Dyasli wrote:
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 6bb5bc7c84..2d7c815e0a 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>>          relocated = true;
>> @@ -1762,11 +1768,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>  
>>      init_IRQ();
>>  
>> -    microcode_grab_module(module_map, mbi);
>> -
>>      timer_init();
>>  
>> -    early_microcode_init();
>> +    early_microcode_init_cache(module_map, mbi);
> 
> microcode_init_cache(module_map, mbi); /* Needs xmalloc() */
> 
> Can fix on commit.

Are you merely after the added comment, or is the omission of the early_
prefix also meaningful in some way?

Jan

Re: [PATCH v2 3/3] x86/ucode: load microcode earlier on boot CPU

[Andrew Cooper]

On 20/12/2022 3:18 pm, Jan Beulich wrote:
> On 20.12.2022 15:50, Andrew Cooper wrote:
>> On 19/12/2022 2:45 pm, Sergey Dyasli wrote:
>>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>>> index 6bb5bc7c84..2d7c815e0a 100644
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>>          relocated = true;
>>> @@ -1762,11 +1768,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>>  
>>>      init_IRQ();
>>>  
>>> -    microcode_grab_module(module_map, mbi);
>>> -
>>>      timer_init();
>>>  
>>> -    early_microcode_init();
>>> +    early_microcode_init_cache(module_map, mbi);
>> microcode_init_cache(module_map, mbi); /* Needs xmalloc() */
>>
>> Can fix on commit.
> Are you merely after the added comment, or is the omission of the early_
> prefix also meaningful in some way?

This isn't "early_microcode" and frankly wasn't "early" to begin with.

Caching the blob can happen at any time after the heap is set up, so
should not have anything like "early" in its name.

The comment is just to make it easier in the future to figure out how to
rearrange __start_xen().

~Andrew