On 15.06.2023 17:48, Alejandro Vallejo wrote: > --- a/xen/arch/x86/cpu/microcode/core.c > +++ b/xen/arch/x86/cpu/microcode/core.c > @@ -879,5 +879,18 @@ int __init early_microcode_init(unsigned long > *module_map, > if ( ucode_mod.mod_end || ucode_blob.size ) > rc = early_microcode_update_cpu(); > > + /* > + * We might have exposed MSR_ARCH_CAPS after the microcode update.
I'm struggling a little with this sentence, but not being a native speaker it may be me, not the sentence. I would perhaps have said "MSR_ARCH_CAPS may have appeared with the microcode update." > + * Reload relevant fields in boot_cpu_data if so because they are > + * needed in tsx_init() Nit: Missing full stop. I also wonder whether you wouldn't want to insert "e.g.", since iirc with the next patch tsx_init() isn't going to be the only user anymore. > + */ > + if ( boot_cpu_data.cpuid_level >= 7 ) > + boot_cpu_data.x86_capability[FEATURESET_7d0] > + = cpuid_count_edx(7, 0); I take it we assume the maximum CPUID level won't go from below 7 to 7 or higher with the ucode update? > --- a/xen/arch/x86/tsx.c > +++ b/xen/arch/x86/tsx.c > @@ -39,9 +39,9 @@ void tsx_init(void) > static bool __read_mostly once; > > /* > - * This function is first called between microcode being loaded, and > CPUID > - * being scanned generally. Read into boot_cpu_data.x86_capability[] for > - * the cpu_has_* bits we care about using here. > + * This function is first called between microcode being loaded, and > + * CPUID being scanned generally. early_microcode_init() has already > + * prepared the feature bits needed here after the microcode update. Is this true in all cases? early_microcode_init() may have bailed early, so I think you need to further transform early_microcode_init() (and as a personal request of mine preferably without goto). Jan
