2017-08-02 16:03:43

by Paolo Bonzini

Subject: [PATCH] KVM: svm: support single stepping over emulated instructions

Use kvm_skip_emulated_instruction so that debug exceptions are
properly injected. This fixes debug.flat on AMD machines.

Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/x86/kvm/svm.c | 50 +++++++++++++++++++++-----------------------------
arch/x86/kvm/x86.c | 1 +
2 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ba9891ac5c56..0c8b9cb715a3 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2245,6 +2245,7 @@ static int io_interception(struct vcpu_svm *svm)
u32 io_info = svm->vmcb->control.exit_info_1; /* address size bug? */
int size, in, string;
unsigned port;
+ int ret;

++svm->vcpu.stat.io_exits;
string = (io_info & SVM_IOIO_STR_MASK) != 0;
@@ -2255,10 +2256,10 @@ static int io_interception(struct vcpu_svm *svm)
port = io_info >> 16;
size = (io_info & SVM_IOIO_SIZE_MASK) >> SVM_IOIO_SIZE_SHIFT;
svm->next_rip = svm->vmcb->control.exit_info_2;
- skip_emulated_instruction(&svm->vcpu);
+ ret = kvm_skip_emulated_instruction(&svm->vcpu);

- return in ? kvm_fast_pio_in(vcpu, size, port)
- : kvm_fast_pio_out(vcpu, size, port);
+ return (in ? kvm_fast_pio_in(vcpu, size, port)
+ : kvm_fast_pio_out(vcpu, size, port)) && ret;
}

static int nmi_interception(struct vcpu_svm *svm)
@@ -3002,6 +3003,7 @@ static int vmload_interception(struct vcpu_svm *svm)
{
struct vmcb *nested_vmcb;
struct page *page;
+ int ret;

if (nested_svm_check_permissions(svm))
return 1;
@@ -3011,18 +3013,18 @@ static int vmload_interception(struct vcpu_svm *svm)
return 1;

svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
- skip_emulated_instruction(&svm->vcpu);
+ ret = kvm_skip_emulated_instruction(&svm->vcpu);

nested_svm_vmloadsave(nested_vmcb, svm->vmcb);
nested_svm_unmap(page);
-
- return 1;
+ return ret;
}

static int vmsave_interception(struct vcpu_svm *svm)
{
struct vmcb *nested_vmcb;
struct page *page;
+ int ret;

if (nested_svm_check_permissions(svm))
return 1;
@@ -3032,12 +3034,11 @@ static int vmsave_interception(struct vcpu_svm *svm)
return 1;

svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
- skip_emulated_instruction(&svm->vcpu);
+ ret = kvm_skip_emulated_instruction(&svm->vcpu);

nested_svm_vmloadsave(svm->vmcb, nested_vmcb);
nested_svm_unmap(page);
-
- return 1;
+ return ret;
}

static int vmrun_interception(struct vcpu_svm *svm)
@@ -3074,12 +3075,10 @@ static int stgi_interception(struct vcpu_svm *svm)
return 1;

svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
- skip_emulated_instruction(&svm->vcpu);
kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
-
enable_gif(svm);

- return 1;
+ return kvm_skip_emulated_instruction(&svm->vcpu);
}

static int clgi_interception(struct vcpu_svm *svm)
@@ -3088,8 +3087,6 @@ static int clgi_interception(struct vcpu_svm *svm)
return 1;

svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
- skip_emulated_instruction(&svm->vcpu);
-
disable_gif(svm);

/* After a CLGI no interrupts should come */
@@ -3099,7 +3096,7 @@ static int clgi_interception(struct vcpu_svm *svm)
mark_dirty(svm->vmcb, VMCB_INTR);
}

- return 1;
+ return kvm_skip_emulated_instruction(&svm->vcpu);
}

static int invlpga_interception(struct vcpu_svm *svm)
@@ -3113,8 +3110,7 @@ static int invlpga_interception(struct vcpu_svm *svm)
kvm_mmu_invlpg(vcpu, kvm_register_read(&svm->vcpu, VCPU_REGS_RAX));

svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
- skip_emulated_instruction(&svm->vcpu);
- return 1;
+ return kvm_skip_emulated_instruction(&svm->vcpu);
}

static int skinit_interception(struct vcpu_svm *svm)
@@ -3137,7 +3133,7 @@ static int xsetbv_interception(struct vcpu_svm *svm)

if (kvm_set_xcr(&svm->vcpu, index, new_bv) == 0) {
svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
- skip_emulated_instruction(&svm->vcpu);
+ return kvm_skip_emulated_instruction(&svm->vcpu);
}

return 1;
@@ -3233,8 +3229,7 @@ static int invlpg_interception(struct vcpu_svm *svm)
return emulate_instruction(&svm->vcpu, 0) == EMULATE_DONE;

kvm_mmu_invlpg(&svm->vcpu, svm->vmcb->control.exit_info_1);
- skip_emulated_instruction(&svm->vcpu);
- return 1;
+ return kvm_skip_emulated_instruction(&svm->vcpu);
}

static int emulate_on_interception(struct vcpu_svm *svm)
@@ -3384,9 +3379,7 @@ static int dr_interception(struct vcpu_svm *svm)
kvm_register_write(&svm->vcpu, reg, val);
}

- skip_emulated_instruction(&svm->vcpu);
-
- return 1;
+ return kvm_skip_emulated_instruction(&svm->vcpu);
}

static int cr8_write_interception(struct vcpu_svm *svm)
@@ -3509,6 +3502,7 @@ static int rdmsr_interception(struct vcpu_svm *svm)
if (svm_get_msr(&svm->vcpu, &msr_info)) {
trace_kvm_msr_read_ex(ecx);
kvm_inject_gp(&svm->vcpu, 0);
+ return 1;
} else {
trace_kvm_msr_read(ecx, msr_info.data);

@@ -3517,9 +3511,8 @@ static int rdmsr_interception(struct vcpu_svm *svm)
kvm_register_write(&svm->vcpu, VCPU_REGS_RDX,
msr_info.data >> 32);
svm->next_rip = kvm_rip_read(&svm->vcpu) + 2;
- skip_emulated_instruction(&svm->vcpu);
+ return kvm_skip_emulated_instruction(&svm->vcpu);
}
- return 1;
}

static int svm_set_vm_cr(struct kvm_vcpu *vcpu, u64 data)
@@ -3645,11 +3638,11 @@ static int wrmsr_interception(struct vcpu_svm *svm)
if (kvm_set_msr(&svm->vcpu, &msr)) {
trace_kvm_msr_write_ex(ecx, data);
kvm_inject_gp(&svm->vcpu, 0);
+ return 1;
} else {
trace_kvm_msr_write(ecx, data);
- skip_emulated_instruction(&svm->vcpu);
+ return kvm_skip_emulated_instruction(&svm->vcpu);
}
- return 1;
}

static int msr_interception(struct vcpu_svm *svm)
@@ -3678,8 +3671,7 @@ static int pause_interception(struct vcpu_svm *svm)

static int nop_interception(struct vcpu_svm *svm)
{
- skip_emulated_instruction(&(svm->vcpu));
- return 1;
+ return kvm_skip_emulated_instruction(&(svm->vcpu));
}

static int monitor_interception(struct vcpu_svm *svm)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0e846f0cb83b..931ba449456e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5548,6 +5548,7 @@ static void kvm_vcpu_do_singlestep(struct kvm_vcpu *vcpu, int *r)
*/
vcpu->arch.dr6 &= ~15;
vcpu->arch.dr6 |= DR6_BS | DR6_RTM;
+ kvm_update_dr6(vcpu);
kvm_queue_exception(vcpu, DB_VECTOR);
}
}
--
1.8.3.1

2017-08-02 20:13:47

by Radim Krčmář

Subject: Re: [PATCH] KVM: svm: support single stepping over emulated instructions

2017-08-02 18:03+0200, Paolo Bonzini:
> Use kvm_skip_emulated_instruction so that debug exceptions are
> properly injected. This fixes debug.flat on AMD machines.
>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---

We already have b742c1e6e79d ("KVM: SVM: handle singlestep exception
when skipping emulated instructions"), so the only applicable part of
this patch is

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0e846f0cb83b..931ba449456e 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5548,6 +5548,7 @@ static void kvm_vcpu_do_singlestep(struct kvm_vcpu *vcpu, int *r)
> */
> vcpu->arch.dr6 &= ~15;
> vcpu->arch.dr6 |= DR6_BS | DR6_RTM;
> + kvm_update_dr6(vcpu);
> kvm_queue_exception(vcpu, DB_VECTOR);
> }
> }

which makes sense. And shouldn't we do the same for
kvm_vcpu_check_breakpoint()?

Thanks.

2017-08-03 05:30:21

by Paolo Bonzini

Subject: Re: [PATCH] KVM: svm: support single stepping over emulated instructions

> We already have b742c1e6e79d ("KVM: SVM: handle singlestep exception
> when skipping emulated instructions"), so the only applicable part of
> this patch is

Doh. :)

> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 0e846f0cb83b..931ba449456e 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -5548,6 +5548,7 @@ static void kvm_vcpu_do_singlestep(struct kvm_vcpu
> > *vcpu, int *r)
> > */
> > vcpu->arch.dr6 &= ~15;
> > vcpu->arch.dr6 |= DR6_BS | DR6_RTM;
> > + kvm_update_dr6(vcpu);
> > kvm_queue_exception(vcpu, DB_VECTOR);
> > }
> > }
>
> which makes sense. And shouldn't we do the same for
> kvm_vcpu_check_breakpoint()?

Yes, will send v2.

Paolo