On 13/06/2023 7:40 am, Jan Beulich wrote: > On 12.06.2023 20:25, Andrew Cooper wrote: >> On 12/06/2023 4:46 pm, Jan Beulich wrote: >>> On 05.06.2023 19:08, Alejandro Vallejo wrote: >>>> @@ -878,5 +887,17 @@ int __init early_microcode_init(unsigned long >>>> *module_map, >>>> if ( ucode_mod.mod_end || ucode_blob.size ) >>>> rc = early_microcode_update_cpu(); >>>> >>>> + early_read_cpuid_7d0(); >>>> + >>>> + /* >>>> + * tsx_init() needs MSR_ARCH_CAPS, but it runs before identify_cpu() >>>> + * populates boot_cpu_data, so we read it here to centralize early >>>> + * CPUID/MSR reads in the same place. >>>> + */ >>>> + if ( cpu_has_arch_caps ) >>>> + rdmsr(MSR_ARCH_CAPABILITIES, >>>> + boot_cpu_data.x86_capability[FEATURESET_m10Al], >>>> + boot_cpu_data.x86_capability[FEATURESET_m10Ah]); >>> ... "centralize" aspect goes away, and hence the comment needs adjusting. >> I find it weird splitting apart the various reads into x86_capability[], >> but in light of the feedback, only the rdmsr() needs to stay. > Hmm, wait: When updating a CPU from a pre-arch-caps ucode level on one > that supports arch-caps, don't we need to re-read 7d0 here? (I.e. the > call to early_read_cpuid_7d0() needs to stay, but for a reason > different from the one presently stated in the description, and > possibly even worth a brief comment.)
Urgh yes. We do have situations where this ucode load will cause MSR_ARCH_CAPS (and features there-within) to appear. I'll rethink the safety here when I've got some breathing room from other tasks. ~Andrew
