Re: [PATCH v3 5/5] x86/microcode: Disable microcode update handler if DIS_MCU_UPDATE is set

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.06.2023 17:48, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -352,6 +352,11 @@ void __init early_cpu_init(void)
>                           &c->x86_capability[FEATURESET_7c0],
>                           &c->x86_capability[FEATURESET_7d0]);
>  
> +             if (test_bit(X86_FEATURE_ARCH_CAPS, c->x86_capability))
> +                     rdmsr(MSR_ARCH_CAPABILITIES,
> +                           c->x86_capability[FEATURESET_m10Al],
> +                           c->x86_capability[FEATURESET_m10Ah]);

Is this still going to be needed if early_microcode_init() uniformly
does this, for things to be correct for tsx_init() (as pointed out
in the other patch)?

> --- a/xen/arch/x86/cpu/microcode/intel.c
> +++ b/xen/arch/x86/cpu/microcode/intel.c
> @@ -387,8 +387,22 @@ static struct microcode_patch *cf_check 
> cpu_request_microcode(
>  
>  void __init intel_get_ucode_ops(struct microcode_ops *ops)
>  {
> +    uint64_t mcu_ctrl;
> +
>      ops->cpu_request_microcode = cpu_request_microcode;
>      ops->collect_cpu_info      = collect_cpu_info;
>      ops->apply_microcode       = apply_microcode;
>      ops->compare_patch         = compare_patch;
> +
> +    if ( cpu_has_mcu_ctrl )
> +    {
> +        rdmsrl(MSR_MCU_CONTROL, mcu_ctrl);
> +        /*
> +         * If DIS_MCU_LOAD is set applying microcode updates won't work. We
> +         * can still query the current version and things like that, so
> +         * we'll leave the other handlers in place.
> +         */
> +        if ( mcu_ctrl & MCU_CONTROL_DIS_MCU_LOAD )
> +            ops->apply_microcode = NULL;
> +    }

While this won't address Andrew's request (afaict), but only the
cf_clobber requirement, I think you want to drop removing of the struct
instances from patch 2, return pointers from the new per-vendor
functions, and simply introduce another static instance of struct
microcode_ops here, with the one hook simply left unset. This lives in
init data, so the size increase is of no major concern.

Jan