lkml.org 
[lkml]   [2010]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 3/3] KVM: SVM: Clean up rip handling in vmrun emulation
From
Date

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 <joerg.roedel@amd.com>
>> ---
>> 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 <joerg.roedel@amd.com>
> 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 <joerg.roedel@amd.com>
> ---
> 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



\
 
 \ /
  Last update: 2010-09-03 23:31    [W:0.119 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site