Re: [PATCH 02/10] x86/ucode: Delete the microcode_init() initcall

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 10/28/24 05:18, Andrew Cooper wrote:
> The comment highlights just how bogus this really is.  Being an initcall, the
> boot allocator is long gone, and bootstrap_unmap() is a no-op.
>
> The fact there is nothing to do should be a giant red flag about the validity
> of the mappings "being freed".  Indeed, they both constitute use-after-frees.
>
> We could move the size/data/end clobbering into microcode_init_cache() which
> is the final consumer of the information, but we're intending to delete these
> static variables entirely, and it's less churn to just leave them dangling for
> a few patches.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
>
> After the $N'th rearranging, this could actually be merged into "x86/ucode:
> Drop ucode_mod and ucode_blob" with no effect on the intermediate patches.
> ---
Not sure if this helps, but for clarification sake, the module itself is 
freed by discard_initial_images() and ucode_mod is __initdata and will 
be cleaned up with the rest of _initdata. This function is called after 
the last user of ucode_mod and the call to bootstrap_unmap() perpetuates 
the assumption that some how the underlying module has remained mapped. 
There is not any condition where this function does anything that would 
affect the execution of the hypervisor. It frees nothing and sets the 
value of a unit local variable that is no longer used shortly before the 
backing memory is freed.
As to patch ordering, I have no opinion.

Reviewed-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>