On 03.09.2010, at 14:21, Roedel, Joerg wrote: > On Thu, Sep 02, 2010 at 11:29:47AM -0400, Joerg Roedel wrote: >> This patch changes the rip handling in the vmrun emulation >> path from using next_rip to the generic kvm register access >> functions. >> >> Signed-off-by: Joerg Roedel <[email protected]> >> --- >> arch/x86/kvm/svm.c | 6 +++--- >> 1 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index ecd4e58..1643f30 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -2069,7 +2069,7 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm) >> return false; >> } >> >> - trace_kvm_nested_vmrun(svm->vmcb->save.rip - 3, vmcb_gpa, >> + trace_kvm_nested_vmrun(svm->vmcb->save.rip, vmcb_gpa, >> nested_vmcb->save.rip, >> nested_vmcb->control.int_ctl, >> nested_vmcb->control.event_inj, >> @@ -2270,8 +2270,8 @@ static int vmrun_interception(struct vcpu_svm *svm) >> if (nested_svm_check_permissions(svm)) >> return 1; >> >> - svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; >> - skip_emulated_instruction(&svm->vcpu); >> + /* Save rip after vmrun instruction */ >> + kvm_rip_write(&svm->vcpu, kvm_rip_read(&svm->vcpu) + 3); >> >> if (!nested_svm_vmrun(svm)) >> return 1; > > Argh, in my interactive commit I forgot one part of this patch. Please > apply the attached one instead. > > > From 42450df2b72c23538d61616834dbdf1b53deafd7 Mon Sep 17 00:00:00 2001 > From: Joerg Roedel <[email protected]> > Date: Thu, 2 Sep 2010 17:12:18 +0200 > Subject: [PATCH 3/3] KVM: SVM: Clean up rip handling in vmrun emulation > > This patch changes the rip handling in the vmrun emulation > path from using next_rip to the generic kvm register access > functions. > > Signed-off-by: Joerg Roedel <[email protected]> > --- > arch/x86/kvm/svm.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index ecd4e58..6808f64 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -2069,7 +2069,7 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm) > return false; > } > > - trace_kvm_nested_vmrun(svm->vmcb->save.rip - 3, vmcb_gpa, > + trace_kvm_nested_vmrun(svm->vmcb->save.rip, vmcb_gpa, > nested_vmcb->save.rip, > nested_vmcb->control.int_ctl, > nested_vmcb->control.event_inj, > @@ -2098,7 +2098,7 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm) > hsave->save.cr0 = kvm_read_cr0(&svm->vcpu); > hsave->save.cr4 = svm->vcpu.arch.cr4; > hsave->save.rflags = vmcb->save.rflags; > - hsave->save.rip = svm->next_rip; > + hsave->save.rip = kvm_rip_read(&svm->vcpu); > hsave->save.rsp = vmcb->save.rsp; > hsave->save.rax = vmcb->save.rax; > if (npt_enabled) > @@ -2270,8 +2270,8 @@ static int vmrun_interception(struct vcpu_svm *svm) > if (nested_svm_check_permissions(svm)) > return 1; > > - svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; > - skip_emulated_instruction(&svm->vcpu); > + /* Save rip after vmrun instruction */ > + kvm_rip_write(&svm->vcpu, kvm_rip_read(&svm->vcpu) + 3);
Any reason we can't use the next_rip information here? A hypervisor could potentially do badness and put a prefix here, thus break all the logic, right? (yes, I know, I wrote that code, but still ...) Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
