On 02/06/2023 2:19 pm, Alejandro Vallejo wrote: > On Thu, Jun 01, 2023 at 11:54:31AM +0100, Andrew Cooper wrote: >> I had something else in mind here. Right now, this will read >> MSR_MCU_CONTROL and emit a printk() on every microcode load, which will >> be every AP, and every time the user uses the xen-ucode tool. > Not every AP. The hypercall would return with an error before the APs are > brought in. It is true that the error on dmesg would appear on every > microcode load attempt though.
I meant every AP on boot, where Xen initiates the ucode load. > >> Instead, I recommend the following: >> >> 1) One patch moving the early-cpuid/msr read from tsx_init() into >> early_microcode_init(), adjusting the comment as it goes. No point >> duplicating that logic, and we need it earlier on boot now. >> 2) This patch, adjusting early_microcode_init() only. Have a printk() >> saying "microcode loading disabled by firmware" and avoid filling in >> ucode_ops. Every other part of ucode handling understands "loading not >> available". > Sure. Going on a tangent though, I do wonder why tsx_init() is preceding > identify_cpu(). It's reading cpuid leaf 7d0 simply because it hasn't been > read yet, but it's not obvious why this rush in invoking tsx_init(). I > can't see any obvious marker that affect the following identify_cpu() call, > and swapping them gets rid of the cpuid read. In __start_xen(), tsx_init(); /* Needs microcode. May change HLE/RTM feature bits. */ If you were to test such a patch, the test-tsx ought to fail on SKL/KBL amongst others. One of the things that tsx_init() does is select TSX_CTRL_CPUID_CLEAR and/or TSX_CPUID_CLEAR, which hides the HLE and RTM bits in regular CPUID, so wants to run before the general CPUID scan. This matters for guest performance - if TSX is actually always aborting, but reported to the guest, then any library using RTM will be less performant than using the non-transactional path. Conversely if the user wants to explicitly re-activate TSX despite the firmware defaults, those bits need clearing before the CPUID scan for anything to work. ~Andrew
