[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 4/4] x86/microcode: Prevent attempting updates if DIS_MCU_LOAD is set
On 05.06.2023 19:08, Alejandro Vallejo wrote: > --- a/xen/arch/x86/cpu/microcode/core.c > +++ b/xen/arch/x86/cpu/microcode/core.c > @@ -749,11 +749,12 @@ __initcall(microcode_init); > /* Load a cached update to current cpu */ > int microcode_update_one(void) > { > + if ( ucode_ops.collect_cpu_info ) > + alternative_vcall(ucode_ops.collect_cpu_info); > + > if ( !ucode_ops.apply_microcode ) > return -EOPNOTSUPP; > > - alternative_vcall(ucode_ops.collect_cpu_info); > - > return microcode_update_cpu(NULL); > } This adjustment (and related logic below) doesn't really fit the purpose that the title states. I wonder if the two things wouldn't better be split. > @@ -849,12 +850,25 @@ static void __init early_read_cpuid_7d0(void) > = cpuid_count_edx(7, 0); > } > > +static bool __init this_cpu_can_install_update(void) > +{ > + uint64_t mcu_ctrl; > + > + if ( !cpu_has_mcu_ctrl ) > + return true; > + > + rdmsrl(MSR_MCU_CONTROL, mcu_ctrl); > + return !(mcu_ctrl & MCU_CONTROL_DIS_MCU_LOAD); > +} As Andrew says, in principle AMD could have a means as well. Surely that would be a different one, so I wonder if this shouldn't be a new hook. > @@ -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 }; Similarly I'm not happy to see a direct reference to some vendor specific field. I think it wants to be the hook to return such an override struct. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |