Hi,

The series is mostly a refactor between everything needed to load microcode and
the bare minimum to probe the current microcode revision.

The Kconfig option keeps the reading of microcode rev data around, as it's very
relevant for security and debuggability in order to deduce which erratas apply
to the current platform.

The idea is to move everything that must still be compiled with !CONFIG_UCODE
onto {,amd-,intel-}base.c, then remove everything else conditionally at the
Makefile level.

Renaming files (e.g: s/base/core/ and s/core/common/) would better reflect
post-series reality, but it'd be annoying for later backports in this general
area.

Cheers,
Alejandro

Alejandro Vallejo (4):
  x86: Split out AMD-specific code to be executed without ucode loading
  x86: Split out Intel-specific code to be executed without ucode
    loading
  x86: Split out early_microcode_load() and microcode_load_one()
  x86: Add Kconfig option to disable microcode loading

 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


base-commit: e00c1673992e07ed31e9c60fefa8d053491abbe6
-- 
2.43.0

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.

~Andrew

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.

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

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.

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.

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.

Cheers,
Alejandro

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

On Thu Nov 13, 2025 at 9:50 AM CET, 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.

Two handfuls when considering both AMD and Intel. But yes, I'd rather not modify
as much as this. However, for our purposes it's important to physically move out
the code we want to Kconfig out, as then coverage reports don't need exceptions.

>
> First, it should be CONFIG_MICROCODE_LOADING. [...]

Sure,  I don't mind it being $FOO or $BAR.

> 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 considered that, but local DCE poses problems for coverage tracking, which
we care about deeply. I'll check if there's some magic we can do to enable that
pattern but as of now, there isn't.

If I can't make it work I'll just take the sledgehammer and drop all ops and
dependent functions.

Cheers,
Alejandro

On 12.11.2025 17:22, Alejandro Vallejo wrote:
> Hi,
> 
> The series is mostly a refactor between everything needed to load microcode and
> the bare minimum to probe the current microcode revision.
> 
> The Kconfig option keeps the reading of microcode rev data around, as it's very
> relevant for security and debuggability in order to deduce which erratas apply
> to the current platform.
> 
> The idea is to move everything that must still be compiled with !CONFIG_UCODE
> onto {,amd-,intel-}base.c, then remove everything else conditionally at the
> Makefile level.
> 
> Renaming files (e.g: s/base/core/ and s/core/common/) would better reflect
> post-series reality, but it'd be annoying for later backports in this general
> area.
> 
> Cheers,
> Alejandro
> 
> Alejandro Vallejo (4):
>   x86: Split out AMD-specific code to be executed without ucode loading
>   x86: Split out Intel-specific code to be executed without ucode
>     loading
>   x86: Split out early_microcode_load() and microcode_load_one()
>   x86: Add Kconfig option to disable microcode loading
> 
>  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

Purely based on this diffstat: A doc update likely is necessary as well, as
the ucode= command line option now becomes only conditionally applicable (aiui,
i.e. without having looked at the patches them selves).

Jan

On Thu Nov 13, 2025 at 8:36 AM CET, Jan Beulich wrote:
> On 12.11.2025 17:22, Alejandro Vallejo wrote:
>> Hi,
>> 
>> The series is mostly a refactor between everything needed to load microcode and
>> the bare minimum to probe the current microcode revision.
>> 
>> The Kconfig option keeps the reading of microcode rev data around, as it's very
>> relevant for security and debuggability in order to deduce which erratas apply
>> to the current platform.
>> 
>> The idea is to move everything that must still be compiled with !CONFIG_UCODE
>> onto {,amd-,intel-}base.c, then remove everything else conditionally at the
>> Makefile level.
>> 
>> Renaming files (e.g: s/base/core/ and s/core/common/) would better reflect
>> post-series reality, but it'd be annoying for later backports in this general
>> area.
>> 
>> Cheers,
>> Alejandro
>> 
>> Alejandro Vallejo (4):
>>   x86: Split out AMD-specific code to be executed without ucode loading
>>   x86: Split out Intel-specific code to be executed without ucode
>>     loading
>>   x86: Split out early_microcode_load() and microcode_load_one()
>>   x86: Add Kconfig option to disable microcode loading
>> 
>>  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
>
> Purely based on this diffstat: A doc update likely is necessary as well, as
> the ucode= command line option now becomes only conditionally applicable (aiui,
> i.e. without having looked at the patches them selves).
>
> Jan

Yes, that sounds sensible.

Cheers,
Alejandro