On 05/06/2023 6:08 pm, Alejandro Vallejo wrote:
> diff --git a/xen/arch/x86/cpu/microcode/core.c
> b/xen/arch/x86/cpu/microcode/core.c
> index 4f60d96d98..a4c123118b 100644
> --- a/xen/arch/x86/cpu/microcode/core.c
> +++ b/xen/arch/x86/cpu/microcode/core.c
> @@ -871,6 +885,15 @@ int __init early_microcode_init(unsigned long
> *module_map,
> * present.
> */
> ucode_ops = intel_ucode_ops;
> +
> + /*
> + * In the case where microcode updates are blocked by the
> + * DIS_MCU_LOAD bit we can still read the microcode version even if
> + * we can't change it.
> + */
> + if ( !this_cpu_can_install_update() )
> + ucode_ops = (struct microcode_ops){ .collect_cpu_info =
> + intel_ucode_ops.collect_cpu_info };
I don't see how this (the logic in this_cpu_can_install_update()) can
work, as ...
> break;
> }
>
> @@ -900,6 +923,10 @@ int __init early_microcode_init(unsigned long
> *module_map,
> if ( ucode_mod.mod_end || ucode_blob.size )
> rc = early_microcode_update_cpu();
>
> + /*
> + * We just updated microcode so we must reload the boot_cpu_data bits
> + * we read before because they might be stale after the updata.
> + */
> early_read_cpuid_7d0();
>
> /*
... MSR_ARCH_CAPS is read out-of-context down here.
In hindsight, I think swapping patch 2 and 3 might be wise. The rev ==
~0 case doesn't need any of the cpu_has_* shuffling, and it already
starts to build up the if/else chain of cases where we decide to clobber
the apply_microcode() hook.
The call to this_cpu_can_install_update() should be lower down. In
principle it's not Intel-specific.
~Andrew
