| Message ID | 1566177928-19114-1-git-send-email-chao.gao@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | improve late microcode loading | expand |
Hi Chao, On 19/08/2019 02:25, Chao Gao wrote: > Previous change log: > Changes in version 8: > - block #NMI handling during microcode loading (Patch 16) > - Don't assume that all CPUs in the system have loaded a same ucode. > So when parsing a blob, we attempt to save a patch as long as it matches > with current cpu signature regardless of the revision of the patch. > And also for loading, we only require the patch to be loaded isn't old > than the cached one. > - store an update after the first successful loading on a CPU > - remove the patch that calls wbinvd() unconditionally before microcode> loading. It is under internal discussion. I noticed that you removed the patch which adds wbinvd() back in v8. What was the reasoning behind that and is there any outcome from the internal discussion that you mention here? > - divide two big patches into several patches to improve readability. Thanks, Sergey
On Thu, Aug 22, 2019 at 08:51:43AM +0100, Sergey Dyasli wrote: >Hi Chao, > >On 19/08/2019 02:25, Chao Gao wrote: >> Previous change log: >> Changes in version 8: >> - block #NMI handling during microcode loading (Patch 16) >> - Don't assume that all CPUs in the system have loaded a same ucode. >> So when parsing a blob, we attempt to save a patch as long as it matches >> with current cpu signature regardless of the revision of the patch. >> And also for loading, we only require the patch to be loaded isn't old >> than the cached one. >> - store an update after the first successful loading on a CPU >> - remove the patch that calls wbinvd() unconditionally before microcode> loading. It is under internal discussion. > >I noticed that you removed the patch which adds wbinvd() back in v8. >What was the reasoning behind that and is there any outcome from the >internal discussion that you mention here? Jan (maybe someone else) was concerned about the impact of calling wbinvd() unconditionally, especially with your work to make serial ucode loading an option. To address this concern, I planned to call wbinvd() conditionally. I need to confirm with Intel microcode team whether it is fine and what the condition should be. But I haven't received an answer. I will talk with Ashok again and probably add this patch back in v10. Thanks Chao
Changes in version 9: - add Jan's Reviewed-by - rendevzous threads in NMI handler to disable NMI. Note that NMI can be served as usual on threads that are chosen to initiate ucode loading on each core. - avoid unnecessary memory allocation or copy when creating a microcode patch (patch 12) - rework patch 1 to avoid microcode_update_match() being used to compare two arbitrary updates. - call .end_update in early loading path. Sergey, could you help to test this series on an AMD machine? Regarding changes to AMD side, I didn't do any test for them due to lack of hardware. At least, two basic tests are needed: * do a microcode update after system bootup * don't bring all pCPUs up at bootup by specifying maxcpus option in xen command line and then do a microcode update and online all offlined CPUs via 'xen-hptool'. The intention of this series is to make the late microcode loading more reliable by rendezvousing all cpus in stop_machine context. This idea comes from Ashok. I am porting his linux patch to Xen (see patch 13 more details). This series includes below changes: 1. Patch 1-12: introduce a global microcode cache and some cleanup 2. Patch 13: synchronize late microcode loading 3. Patch 14: support parallel microcodes update on different cores 4. Patch 15: block #NMI handling during microcode loading Currently, late microcode loading does a lot of things including parsing microcode blob, checking the signature/revision and performing update. Putting all of them into stop_machine context is a bad idea because of complexity (one issue I observed is memory allocation triggered one assertion in stop_machine context). To simplify the load process, parsing microcode is moved out of the load process. Remaining parts of load process is put to stop_machine context. Previous change log: Changes in version 8: - block #NMI handling during microcode loading (Patch 16) - Don't assume that all CPUs in the system have loaded a same ucode. So when parsing a blob, we attempt to save a patch as long as it matches with current cpu signature regardless of the revision of the patch. And also for loading, we only require the patch to be loaded isn't old than the cached one. - store an update after the first successful loading on a CPU - remove the patch that calls wbinvd() unconditionally before microcode loading. It is under internal discussion. - divide two big patches into several patches to improve readability. Changes in version 7: - cache one microcode update rather than a list of it. Assuming that all CPUs (including those will be plugged in later) in the system have the same signature, one update matches with one CPU should match with others. Thus, one update is enough for microcode updating during CPU hot-plug and resuming. - To handle load failure, microcode update is cached after it is applied to avoid a broken update overriding a validated one. Unvalidated microcode updates are passed by arguments rather than another global variable, where this series slightly differs from Roger's suggestion in: https://lists.xen.org/archives/html/xen-devel/2019-03/msg00776.html - incorporate Sergey's patch (patch 10) to fix a bug: we maintain a variable to reflect current microcode revision. But in some cases, this variable isn't initialized during system boot time, which results in falsely reporting that processor is susceptible to some known vulnerabilities. - fix issues reported by Sergey: https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg00901.html - Responses to Sergey/Roger/Wei/Ashok's other comments. Major changes in version 6: - run wbinvd before updating microcode (patch 10) - add an userspace tool for late microcode update (patch 1) - scale time to wait by the number of remaining CPUs to respond - remove 'cpu' parameters from some related callbacks and functins - save an ucode patch only if its supported CPU is allowed to mix with current cpu. Changes in version 5: - support parallel microcode updates for all cores (see patch 8) - Address Roger's comments on the last version. Chao Gao (15): microcode/intel: extend microcode_update_match() microcode/amd: fix memory leak microcode/amd: distinguish old and mismatched ucode in microcode_fits() microcode: introduce a global cache of ucode patch microcode: clean up microcode_resume_cpu microcode: remove struct ucode_cpu_info microcode: remove pointless 'cpu' parameter microcode/amd: call svm_host_osvw_init() in common code microcode: pass a patch pointer to apply_microcode() microcode: split out apply_microcode() from cpu_request_microcode() microcode: unify loading update during CPU resuming and AP wakeup microcode: reduce memory allocation and copy when creating a patch x86/microcode: Synchronize late microcode loading microcode: remove microcode_update_lock microcode: block #NMI handling when loading an ucode xen/arch/x86/acpi/power.c | 2 +- xen/arch/x86/apic.c | 2 +- xen/arch/x86/microcode.c | 518 ++++++++++++++++++++++++++++++---------- xen/arch/x86/microcode_amd.c | 267 +++++++++------------ xen/arch/x86/microcode_intel.c | 202 ++++++++-------- xen/arch/x86/smpboot.c | 5 +- xen/arch/x86/spec_ctrl.c | 2 +- xen/include/asm-x86/microcode.h | 43 ++-- xen/include/asm-x86/processor.h | 4 +- 9 files changed, 642 insertions(+), 403 deletions(-)