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

[Sergey Dyasli <sergey.dyasli@xxxxxxxxx>]

On Thu, Dec 8, 2022 at 3:34 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 08.12.2022 14:59, Andrew Cooper wrote:
> > On 08/12/2022 13:26, Sergey Dyasli wrote:
> >> @@ -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.
>
> Alternatively it demonstrates why xfree() should take const void *,
> just like e.g. unmap_domain_page() or vunmap() already do. We've
> talked about this before, and the argument hasn't changed: Neither
> unmapping nor freeing really alters the contents of the pointed to
> area from the perspective of the caller, as the contents simply
> disappears altogether.

Despite my love of const, const correctness in C is quite a pain. I've
tried to make xfree() take a const pointer but then issues began with
add/strip_padding() functions and I couldn't overcome those without
further (void *) casts which just takes the problem to a different
layer.

I think I'll have to go with Andrew's suggestion and continue to return
non-const pointers from cpu_request_microcode(). This will include
a cast though:

    patch = (struct microcode_patch *)saved;

Sergey