Re: [PATCH 0/4] Add Kconfig option to remove microcode loading support

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/11/2025 10:19 am, Alejandro Vallejo wrote:
> On Mon Nov 17, 2025 at 5:55 PM CET, Jan Beulich wrote:
>> On 13.11.2025 09:50, Andrew Cooper wrote:
>>> On 12/11/2025 4:22 pm, Alejandro Vallejo wrote:
>>>>  xen/arch/x86/Kconfig                    | 12 ++++
>>>>  xen/arch/x86/cpu/microcode/Makefile     |  9 ++-
>>>>  xen/arch/x86/cpu/microcode/amd-base.c   | 55 +++++++++++++++++++
>>>>  xen/arch/x86/cpu/microcode/amd.c        | 55 ++-----------------
>>>>  xen/arch/x86/cpu/microcode/amd.h        | 15 +++++
>>>>  xen/arch/x86/cpu/microcode/base.c       | 73 +++++++++++++++++++++++++
>>>>  xen/arch/x86/cpu/microcode/core.c       | 58 +-------------------
>>>>  xen/arch/x86/cpu/microcode/intel-base.c | 50 +++++++++++++++++
>>>>  xen/arch/x86/cpu/microcode/intel.c      | 56 +++----------------
>>>>  xen/arch/x86/cpu/microcode/intel.h      | 16 ++++++
>>>>  xen/arch/x86/cpu/microcode/private.h    | 14 +++++
>>>>  xen/arch/x86/efi/efi-boot.h             |  2 +-
>>>>  xen/arch/x86/platform_hypercall.c       |  2 +
>>>>  13 files changed, 259 insertions(+), 158 deletions(-)
>>>>  create mode 100644 xen/arch/x86/cpu/microcode/amd-base.c
>>>>  create mode 100644 xen/arch/x86/cpu/microcode/amd.h
>>>>  create mode 100644 xen/arch/x86/cpu/microcode/base.c
>>>>  create mode 100644 xen/arch/x86/cpu/microcode/intel-base.c
>>>>  create mode 100644 xen/arch/x86/cpu/microcode/intel.h
>>> This is awfully invasive for something that ultimately drops only a
>>> handful of lines of code.
>>>
>>> First, it should be CONFIG_MICROCODE_LOADING.  We're not getting rid of
>>> all microcode capabilities.  Also, of all the places where UCODE needs
>>> expanding properly, it's Kconfig.
>>>
>>> Next, annotate the functions that you conditionally don't reference in
>>> {amd,intel}_ucode_ops with __maybe_unused, and dead code elimination
>>> should do the rest.
> I've done a few tests to see how something along those lines would pan out for
> our needs. Our coverage tool correctly ignores ellided functions, so I'll be
> sending a v2 shortly.
>
>> Are you, btw, sure this would be Misra-compliant? Right now we solely
>> deviate __maybe_unused when used on labels which may really be unused,
>> afaics.
>>
>> Jan
> Rather than appending an unconditional __maybe_unused (that's, imo, a bad 
> idea),
> I'll be creating a local __ucode_loading attribute in private.h that maps to
> __maybe_unused when CONFIG_MICROCODE_LOADING is not set and to nothing when it
> is set.

__maybe_unused literally exists for this purpose.  See it's comment.

Wrapping in another condition just adds complexity for no gain.  This
case is unlike livepatch_or_$FOO because it's not choice between two
different things.

>
> However, I'm tentatively keeping the movement from core.c to base.c, as 
> there's
> just way too many functions with external linkage to ifdef. It'd be an utterly
> confusing file otherwise.

There are 4 functions with external linkage, only one of which you can
fully elide.

(I think) you can do everything you need here with 4 IS_ENABLED()'s (two
in do_platform_op(), one in early_microcode_init() and
microcode_update_one()), one ifdef around ucode_update_hcall(), and one
__maybe_unused for ucode_update_hcall().

I'm going to save you some time, and insist that core.c is not split;
I'm not willing to take that kind of disruption into logic this
complicated.  The result is far nicer not split than split.

>
> Plus, I'll be conditionally getting rid of earlycpio.c too, which is 
> something I
> neglected to do in v1 even if it's only used for microcode loading.

This is fine, but cpio should be lib-ified like sha2 so it's simply
dropped if not referenced.

In fact, the one ifdef mentioned above could be dropped if someone were
to get --gc-sections working, but I suppose that is a task for a
different day.

~Andrew