On Mon, Feb 24, 2025, Tom Lendacky wrote: > On 2/18/25 19:26, Sean Christopherson wrote: > > Never rely on the CPU to restore/load host DR0..DR3 values, even if the > > CPU supports DebugSwap, as there are no guarantees that SNP guests will > > actually enable DebugSwap on APs. E.g. if KVM were to rely on the CPU to > > load DR0..DR3 and skipped them during hw_breakpoint_restore(), KVM would > > run with clobbered-to-zero DRs if an SNP guest created APs without > > DebugSwap enabled. > > > > Update the comment to explain the dangers, and hopefully prevent breaking > > KVM in the future. > > > > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx> > > See comment below about the Type-A vs Type-B thing, but functionally: > > Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx> > > > --- > > arch/x86/kvm/svm/sev.c | 21 ++++++++++++--------- > > 1 file changed, 12 insertions(+), 9 deletions(-) > > > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > > index e3606d072735..6c6d45e13858 100644 > > --- a/arch/x86/kvm/svm/sev.c > > +++ b/arch/x86/kvm/svm/sev.c > > @@ -4594,18 +4594,21 @@ void sev_es_prepare_switch_to_guest(struct vcpu_svm *svm, struct sev_es_save_are > > /* > > * If DebugSwap is enabled, debug registers are loaded but NOT saved by > > * the CPU (Type-B). If DebugSwap is disabled/unsupported, the CPU both > > - * saves and loads debug registers (Type-A). Sadly, on CPUs without > > - * ALLOWED_SEV_FEATURES, KVM can't prevent SNP guests from enabling > > - * DebugSwap on secondary vCPUs without KVM's knowledge via "AP Create", > > - * and so KVM must save DRs if DebugSwap is supported to prevent DRs > > - * from being clobbered by a misbehaving guest. > > + * saves and loads debug registers (Type-A). Sadly, KVM can't prevent > > This mention of Type-A was bothering me, so I did some investigation on > this. If DebugSwap (DebugVirtualization in the latest APM) is > disabled/unsupported, DR0-3 and DR0-3 Mask registers are left alone and > the guest sees the host values, they are not fully restored and fully > saved. When DebugVirtualization is enabled, at that point the registers > become Type-B. Good catch. I completely glossed over that; I'm pretty sure my subconcious simply rejected that statement as wrong. > I'm not sure whether it is best to update the comment here or in the > first patch. Probably first patch.
