On 13.01.2026 13:21, Alejandro Vallejo wrote:
> @@ -469,7 +471,7 @@ struct ucode_buf {
> char buffer[];
> };
>
> -static long cf_check ucode_update_hcall_cont(void *data)
> +static long cf_check __maybe_unused ucode_update_hcall_cont(void *data)
> {
> struct microcode_patch *patch = NULL;
> int ret, result;
Why this change when ...
> @@ -613,6 +615,7 @@ static long cf_check ucode_update_hcall_cont(void *data)
> return ret;
> }
>
> +#ifdef CONFIG_MICROCODE_LOADING
... this can simply be moved up accordingly? After all ...
> int ucode_update_hcall(XEN_GUEST_HANDLE(const_void) buf,
> unsigned long len, unsigned int flags)
> {
> @@ -645,6 +648,7 @@ int ucode_update_hcall(XEN_GUEST_HANDLE(const_void) buf,
> */
> return continue_hypercall_on_cpu(0, ucode_update_hcall_cont, buffer);
... this is the only user of that other function.
> --- a/xen/arch/x86/cpu/microcode/intel.c
> +++ b/xen/arch/x86/cpu/microcode/intel.c
> @@ -408,17 +408,20 @@ static const char __initconst intel_cpio_path[] =
> "kernel/x86/microcode/GenuineIntel.bin";
>
> static const struct microcode_ops __initconst_cf_clobber intel_ucode_ops = {
> - .cpu_request_microcode = cpu_request_microcode,
> + .cpu_request_microcode = MICROCODE_OP(cpu_request_microcode),
> .collect_cpu_info = collect_cpu_info,
> - .apply_microcode = apply_microcode,
> - .compare = intel_compare,
> - .cpio_path = intel_cpio_path,
> + .apply_microcode = MICROCODE_OP(apply_microcode),
> + .compare = MICROCODE_OP(intel_compare),
> + .cpio_path = MICROCODE_OP(intel_cpio_path),
> };
While I appreciate the intention with MICROCODE_OP(), I'm not really happy
with function pointer members left in place just for them to be NULL
everywhere. What if a call site remains unguarded? With PV guests that
would be a privilege escalation XSA.
Jan
