On 08/12/2022 13:26, Sergey Dyasli wrote:
> diff --git a/xen/arch/x86/cpu/microcode/core.c
> b/xen/arch/x86/cpu/microcode/core.c
> index 452a7ca773..924a2bd7b5 100644
> --- a/xen/arch/x86/cpu/microcode/core.c
> +++ b/xen/arch/x86/cpu/microcode/core.c
> @@ -99,7 +99,7 @@ static bool ucode_in_nmi = true;
> bool __read_mostly opt_ucode_allow_same;
>
> /* Protected by microcode_mutex */
> -static struct microcode_patch *microcode_cache;
> +static const struct microcode_patch *microcode_cache;
>
> void __init microcode_set_module(unsigned int idx)
> {
> @@ -240,20 +240,20 @@ static const struct microcode_patch *nmi_patch =
> ZERO_BLOCK_PTR;
> * patch is found and an error occurs during the parsing process. Otherwise
> * return NULL.
> */
> -static struct microcode_patch *parse_blob(const char *buf, size_t len)
> +static const 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)
> +static void microcode_free_patch(const struct microcode_patch *patch)
> {
> - xfree(patch);
> + xfree((void *)patch);
This hunk demonstrates why the hook wants to return a non-const
pointer. Keeping it non-const will shrink this patch quite a bit.
Given that make_copy=false is a special case for the BSP, it can cope
with knowing that it shouldn't mutate the pointer it gets given back.
I do have a plan for the future to reduce the complexity here, but it
depends on getting a few more details out of AMD first. (All of this
mess is because their ucode doesn't have an embedded length field.)
In the short term, handing the BSP a non-const pointer to a const buffer
for early loading is going to be a lesser evil than in the common case
giving the caller a const pointer that they're expected to call xfree() on.
If this is the only issue, then I'm happy to adjust on commit.
~Andrew
