| Message ID | 20250224181315.2376869-1-seanjc@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: SVM: Zero DEBUGCTL before VMRUN if necessary | expand |
On Mon, Feb 24, 2025 at 10:13:12AM -0800, Sean Christopherson wrote: > PeterZ, > > Can you confirm that the last patch (snapshot and restore DEBUGCTL with > IRQs disabled) is actually necessary? I'm 99% certain it is, but I'm > holding out hope that it somehow isn't, because I don't love the idea of > adding a RDMSR to every VM-Entry. I think you're right. I mean, I'd have to go double check and trace the various call paths again, but I'd be very surprised if we can't change DEBUGCTL from NMI context. > Assuming DEBUGCTL can indeed get modified in IRQ context, it probably > makes sense to add a per-CPU cache to eliminate the RDMSR. Unfortunately, > there are quite a few open-coded WRMSRs, so it's not a trivial change. This, I'm surprised we've not yet done that. > On to the main event... > > Fix a long-lurking bug in SVM where KVM runs the guest with the host's > DEBUGCTL if LBR virtualization is disabled. AMD CPUs rather stupidly > context switch DEBUGCTL if and only if LBR virtualization is enabled (not > just supported, but fully enabled). > > The bug has gone unnoticed because until recently, the only bits that > KVM would leave set were things like BTF, which are guest visible but > won't cause functional problems unless guest software is being especially > particular about #DBs. > > The bug was exposed by the addition of BusLockTrap ("Detect" in the kernel), > as the resulting #DBs due to split-lock accesses in guest userspace (lol > Steam) get reflected into the guest by KVM. Hehe, yeah, games. Yeah we ran into that with bus-lock on intel too :-)
PeterZ, Can you confirm that the last patch (snapshot and restore DEBUGCTL with IRQs disabled) is actually necessary? I'm 99% certain it is, but I'm holding out hope that it somehow isn't, because I don't love the idea of adding a RDMSR to every VM-Entry. Assuming DEBUGCTL can indeed get modified in IRQ context, it probably makes sense to add a per-CPU cache to eliminate the RDMSR. Unfortunately, there are quite a few open-coded WRMSRs, so it's not a trivial change. On to the main event... Fix a long-lurking bug in SVM where KVM runs the guest with the host's DEBUGCTL if LBR virtualization is disabled. AMD CPUs rather stupidly context switch DEBUGCTL if and only if LBR virtualization is enabled (not just supported, but fully enabled). The bug has gone unnoticed because until recently, the only bits that KVM would leave set were things like BTF, which are guest visible but won't cause functional problems unless guest software is being especially particular about #DBs. The bug was exposed by the addition of BusLockTrap ("Detect" in the kernel), as the resulting #DBs due to split-lock accesses in guest userspace (lol Steam) get reflected into the guest by KVM. Sean Christopherson (3): KVM: x86: Snapshot the host's DEBUGCTL in common x86 KVM: SVM: Manually zero/restore DEBUGCTL if LBR virtualization is disabled KVM: x86: Snapshot the host's DEBUGCTL after disabling IRQs arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/svm/svm.c | 14 ++++++++++++++ arch/x86/kvm/vmx/vmx.c | 8 ++------ arch/x86/kvm/vmx/vmx.h | 2 -- arch/x86/kvm/x86.c | 2 ++ 5 files changed, 19 insertions(+), 8 deletions(-) base-commit: fed48e2967f402f561d80075a20c5c9e16866e53