| Message ID | 20241009175002.1118178-5-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Fix and harden reg caching from !TASK context | expand |
On Wed, 2024-10-09 at 10:50 -0700, Sean Christopherson wrote: > Explicitly return '0' for guest RIP when handling a PMI VM-Exit for a vCPU > with protected guest state, i.e. when KVM can't read the real RIP. While > there is no "right" value, and profiling a protect guest is rather futile, > returning the last known RIP is worse than returning obviously "bad" data. > E.g. for SEV-ES+, the last known RIP will often point somewhere in the > guest's boot flow. > > Opportunistically add WARNs to effectively assert that the in_kernel() and > get_ip() callbacks are restricted to the common PMI handler, as the return > values for the protected guest state case are largely arbitrary, i.e. only > make any sense whatsoever for PMIs, where the returned values have no > functional impact and thus don't truly matter. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/x86.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 830073294640..516cf6c71567 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -13213,6 +13213,8 @@ void kvm_arch_commit_memory_region(struct kvm *kvm, > > bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu) > { > + WARN_ON_ONCE(!kvm_arch_pmi_in_guest(vcpu)); > + > if (vcpu->arch.guest_state_protected) > return true; > > @@ -13221,6 +13223,11 @@ bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu) > > unsigned long kvm_arch_vcpu_get_ip(struct kvm_vcpu *vcpu) > { > + WARN_ON_ONCE(!kvm_arch_pmi_in_guest(vcpu)); > + > + if (vcpu->arch.guest_state_protected) > + return 0; > + > return kvm_rip_read(vcpu); > } > Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 830073294640..516cf6c71567 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -13213,6 +13213,8 @@ void kvm_arch_commit_memory_region(struct kvm *kvm, bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu) { + WARN_ON_ONCE(!kvm_arch_pmi_in_guest(vcpu)); + if (vcpu->arch.guest_state_protected) return true; @@ -13221,6 +13223,11 @@ bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu) unsigned long kvm_arch_vcpu_get_ip(struct kvm_vcpu *vcpu) { + WARN_ON_ONCE(!kvm_arch_pmi_in_guest(vcpu)); + + if (vcpu->arch.guest_state_protected) + return 0; + return kvm_rip_read(vcpu); }
Explicitly return '0' for guest RIP when handling a PMI VM-Exit for a vCPU with protected guest state, i.e. when KVM can't read the real RIP. While there is no "right" value, and profiling a protect guest is rather futile, returning the last known RIP is worse than returning obviously "bad" data. E.g. for SEV-ES+, the last known RIP will often point somewhere in the guest's boot flow. Opportunistically add WARNs to effectively assert that the in_kernel() and get_ip() callbacks are restricted to the common PMI handler, as the return values for the protected guest state case are largely arbitrary, i.e. only make any sense whatsoever for PMIs, where the returned values have no functional impact and thus don't truly matter. Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/x86.c | 7 +++++++ 1 file changed, 7 insertions(+)