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
