* [PATCH 1/3] KVM: x86: rename EMULATE_DO_MMIO
2013-07-30 15:11 [PATCH 0/2] KVM: x86: minimal debugging support during emulation Paolo Bonzini
@ 2013-07-30 15:11 ` Paolo Bonzini
2013-07-31 9:40 ` Gleb Natapov
2013-07-30 15:11 ` [PATCH 2/3] KVM: x86: handle hardware breakpoints during emulation Paolo Bonzini
2013-07-30 15:11 ` [PATCH 3/3] KVM: x86: handle singlestep " Paolo Bonzini
2 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2013-07-30 15:11 UTC (permalink / raw)
To: linux-kernel; +Cc: gnatapov, kvm
The next patch will reuse it for other userspace exits than MMIO,
namely debug events.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/include/asm/kvm_host.h | 4 ++--
arch/x86/kvm/mmu.c | 2 +-
arch/x86/kvm/vmx.c | 2 +-
arch/x86/kvm/x86.c | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1f98c1b..b33e9dc 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -795,8 +795,8 @@ extern u32 kvm_min_guest_tsc_khz;
extern u32 kvm_max_guest_tsc_khz;
enum emulation_result {
- EMULATE_DONE, /* no further processing */
- EMULATE_DO_MMIO, /* kvm_run filled with mmio request */
+ EMULATE_DONE, /* no further processing */
+ EMULATE_USER_EXIT, /* kvm_run ready for userspace exit */
EMULATE_FAIL, /* can't emulate this instruction */
};
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 6941fa7..7ee30d4 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -4096,7 +4096,7 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u32 error_code,
switch (er) {
case EMULATE_DONE:
return 1;
- case EMULATE_DO_MMIO:
+ case EMULATE_USER_EXIT:
++vcpu->stat.mmio_exits;
/* fall through */
case EMULATE_FAIL:
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 260a919..0c87015 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5422,7 +5422,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
err = emulate_instruction(vcpu, EMULTYPE_NO_REEXECUTE);
- if (err == EMULATE_DO_MMIO) {
+ if (err == EMULATE_USER_EXIT) {
ret = 0;
goto out;
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 737c804..c2a0674 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5038,11 +5038,11 @@ restart:
writeback = false;
vcpu->arch.complete_userspace_io = complete_emulated_pio;
}
- r = EMULATE_DO_MMIO;
+ r = EMULATE_USER_EXIT;
} else if (vcpu->mmio_needed) {
if (!vcpu->mmio_is_write)
writeback = false;
- r = EMULATE_DO_MMIO;
+ r = EMULATE_USER_EXIT;
vcpu->arch.complete_userspace_io = complete_emulated_mmio;
} else if (r == EMULATION_RESTART)
goto restart;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] KVM: x86: rename EMULATE_DO_MMIO
2013-07-30 15:11 ` [PATCH 1/3] KVM: x86: rename EMULATE_DO_MMIO Paolo Bonzini
@ 2013-07-31 9:40 ` Gleb Natapov
0 siblings, 0 replies; 9+ messages in thread
From: Gleb Natapov @ 2013-07-31 9:40 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm
On Tue, Jul 30, 2013 at 05:11:34PM +0200, Paolo Bonzini wrote:
> The next patch will reuse it for other userspace exits than MMIO,
> namely debug events.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> arch/x86/include/asm/kvm_host.h | 4 ++--
> arch/x86/kvm/mmu.c | 2 +-
> arch/x86/kvm/vmx.c | 2 +-
> arch/x86/kvm/x86.c | 4 ++--
> 4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 1f98c1b..b33e9dc 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -795,8 +795,8 @@ extern u32 kvm_min_guest_tsc_khz;
> extern u32 kvm_max_guest_tsc_khz;
>
> enum emulation_result {
> - EMULATE_DONE, /* no further processing */
> - EMULATE_DO_MMIO, /* kvm_run filled with mmio request */
> + EMULATE_DONE, /* no further processing */
> + EMULATE_USER_EXIT, /* kvm_run ready for userspace exit */
> EMULATE_FAIL, /* can't emulate this instruction */
> };
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 6941fa7..7ee30d4 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -4096,7 +4096,7 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u32 error_code,
> switch (er) {
> case EMULATE_DONE:
> return 1;
> - case EMULATE_DO_MMIO:
> + case EMULATE_USER_EXIT:
> ++vcpu->stat.mmio_exits;
> /* fall through */
> case EMULATE_FAIL:
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 260a919..0c87015 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5422,7 +5422,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
>
> err = emulate_instruction(vcpu, EMULTYPE_NO_REEXECUTE);
>
> - if (err == EMULATE_DO_MMIO) {
> + if (err == EMULATE_USER_EXIT) {
Not related to the patch, but here we need ++vcpu->stat.mmio_exits too.
And mmio_exits is not really about mmios any more, oh well.
The patch itself looks good.
> ret = 0;
> goto out;
> }
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 737c804..c2a0674 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5038,11 +5038,11 @@ restart:
> writeback = false;
> vcpu->arch.complete_userspace_io = complete_emulated_pio;
> }
> - r = EMULATE_DO_MMIO;
> + r = EMULATE_USER_EXIT;
> } else if (vcpu->mmio_needed) {
> if (!vcpu->mmio_is_write)
> writeback = false;
> - r = EMULATE_DO_MMIO;
> + r = EMULATE_USER_EXIT;
> vcpu->arch.complete_userspace_io = complete_emulated_mmio;
> } else if (r == EMULATION_RESTART)
> goto restart;
> --
> 1.8.1.4
>
--
Gleb.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] KVM: x86: handle hardware breakpoints during emulation
2013-07-30 15:11 [PATCH 0/2] KVM: x86: minimal debugging support during emulation Paolo Bonzini
2013-07-30 15:11 ` [PATCH 1/3] KVM: x86: rename EMULATE_DO_MMIO Paolo Bonzini
@ 2013-07-30 15:11 ` Paolo Bonzini
2013-07-31 10:00 ` Gleb Natapov
2013-07-30 15:11 ` [PATCH 3/3] KVM: x86: handle singlestep " Paolo Bonzini
2 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2013-07-30 15:11 UTC (permalink / raw)
To: linux-kernel; +Cc: gnatapov, kvm
This lets debugging work better during emulation of invalid
guest state.
The check is done before emulating the instruction, and (in the case
of guest debugging) reuses EMULATE_DO_MMIO to exit with KVM_EXIT_DEBUG.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/x86.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c2a0674..1368cf5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4956,6 +4956,62 @@ static bool retry_instruction(struct x86_emulate_ctxt *ctxt,
static int complete_emulated_mmio(struct kvm_vcpu *vcpu);
static int complete_emulated_pio(struct kvm_vcpu *vcpu);
+static int kvm_vcpu_check_hw_bp(unsigned long addr, u32 type, u32 dr7,
+ unsigned long *db)
+{
+ u32 dr6 = 0;
+ int i;
+ u32 enable, rwlen;
+
+ enable = dr7;
+ rwlen = dr7 >> 16;
+ for (i = 0; i < 4; i++, enable >>= 2, rwlen >>= 4)
+ if ((enable & 3) && (rwlen & 15) == type && db[i] == addr)
+ dr6 |= (1 << i);
+ return dr6;
+}
+
+static bool kvm_vcpu_check_breakpoint(struct kvm_vcpu *vcpu, int *r)
+{
+ struct kvm_run *kvm_run = vcpu->run;
+ unsigned long eip = vcpu->arch.emulate_ctxt.eip;
+ u32 dr6 = 0;
+
+ if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) &&
+ (vcpu->arch.guest_debug_dr7 & DR7_BP_EN_MASK)) {
+ dr6 = kvm_vcpu_check_hw_bp(eip, 0,
+ vcpu->arch.guest_debug_dr7,
+ vcpu->arch.eff_db);
+
+ if (dr6 != 0) {
+ kvm_run->debug.arch.dr6 = dr6 | DR6_FIXED_1;
+ kvm_run->debug.arch.pc = kvm_rip_read(vcpu) +
+ get_segment_base(vcpu, VCPU_SREG_CS);
+
+ kvm_run->debug.arch.exception = DB_VECTOR;
+ kvm_run->exit_reason = KVM_EXIT_DEBUG;
+ *r = EMULATE_USER_EXIT;
+ return true;
+ }
+ }
+
+ if (unlikely(vcpu->arch.dr7 & DR7_BP_EN_MASK)) {
+ dr6 = kvm_vcpu_check_hw_bp(eip, 0,
+ vcpu->arch.dr7,
+ vcpu->arch.db);
+
+ if (dr6 != 0) {
+ vcpu->arch.dr6 &= ~15;
+ vcpu->arch.dr6 |= dr6;
+ kvm_queue_exception(vcpu, DB_VECTOR);
+ *r = EMULATE_DONE;
+ return true;
+ }
+ }
+
+ return false;
+}
+
int x86_emulate_instruction(struct kvm_vcpu *vcpu,
unsigned long cr2,
int emulation_type,
@@ -4976,6 +5032,16 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
if (!(emulation_type & EMULTYPE_NO_DECODE)) {
init_emulate_ctxt(vcpu);
+
+ /*
+ * We will reenter on the same instruction since
+ * we do not set complete_userspace_io. This does not
+ * handle watchpoints yet, those would be handled in
+ * the emulate_ops.
+ */
+ if (kvm_vcpu_check_breakpoint(vcpu, &r))
+ return r;
+
ctxt->interruptibility = 0;
ctxt->have_exception = false;
ctxt->perm_ok = false;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] KVM: x86: handle hardware breakpoints during emulation
2013-07-30 15:11 ` [PATCH 2/3] KVM: x86: handle hardware breakpoints during emulation Paolo Bonzini
@ 2013-07-31 10:00 ` Gleb Natapov
0 siblings, 0 replies; 9+ messages in thread
From: Gleb Natapov @ 2013-07-31 10:00 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm
On Tue, Jul 30, 2013 at 05:11:35PM +0200, Paolo Bonzini wrote:
> This lets debugging work better during emulation of invalid
> guest state.
>
> The check is done before emulating the instruction, and (in the case
> of guest debugging) reuses EMULATE_DO_MMIO to exit with KVM_EXIT_DEBUG.
>
Not EMULATE_DO_MMIO any more. Otherwise looks good.
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> arch/x86/kvm/x86.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c2a0674..1368cf5 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4956,6 +4956,62 @@ static bool retry_instruction(struct x86_emulate_ctxt *ctxt,
> static int complete_emulated_mmio(struct kvm_vcpu *vcpu);
> static int complete_emulated_pio(struct kvm_vcpu *vcpu);
>
> +static int kvm_vcpu_check_hw_bp(unsigned long addr, u32 type, u32 dr7,
> + unsigned long *db)
> +{
> + u32 dr6 = 0;
> + int i;
> + u32 enable, rwlen;
> +
> + enable = dr7;
> + rwlen = dr7 >> 16;
> + for (i = 0; i < 4; i++, enable >>= 2, rwlen >>= 4)
> + if ((enable & 3) && (rwlen & 15) == type && db[i] == addr)
> + dr6 |= (1 << i);
> + return dr6;
> +}
> +
> +static bool kvm_vcpu_check_breakpoint(struct kvm_vcpu *vcpu, int *r)
> +{
> + struct kvm_run *kvm_run = vcpu->run;
> + unsigned long eip = vcpu->arch.emulate_ctxt.eip;
> + u32 dr6 = 0;
> +
> + if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) &&
> + (vcpu->arch.guest_debug_dr7 & DR7_BP_EN_MASK)) {
> + dr6 = kvm_vcpu_check_hw_bp(eip, 0,
> + vcpu->arch.guest_debug_dr7,
> + vcpu->arch.eff_db);
> +
> + if (dr6 != 0) {
> + kvm_run->debug.arch.dr6 = dr6 | DR6_FIXED_1;
> + kvm_run->debug.arch.pc = kvm_rip_read(vcpu) +
> + get_segment_base(vcpu, VCPU_SREG_CS);
> +
> + kvm_run->debug.arch.exception = DB_VECTOR;
> + kvm_run->exit_reason = KVM_EXIT_DEBUG;
> + *r = EMULATE_USER_EXIT;
> + return true;
> + }
> + }
> +
> + if (unlikely(vcpu->arch.dr7 & DR7_BP_EN_MASK)) {
> + dr6 = kvm_vcpu_check_hw_bp(eip, 0,
> + vcpu->arch.dr7,
> + vcpu->arch.db);
> +
> + if (dr6 != 0) {
> + vcpu->arch.dr6 &= ~15;
> + vcpu->arch.dr6 |= dr6;
> + kvm_queue_exception(vcpu, DB_VECTOR);
> + *r = EMULATE_DONE;
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> int x86_emulate_instruction(struct kvm_vcpu *vcpu,
> unsigned long cr2,
> int emulation_type,
> @@ -4976,6 +5032,16 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
>
> if (!(emulation_type & EMULTYPE_NO_DECODE)) {
> init_emulate_ctxt(vcpu);
> +
> + /*
> + * We will reenter on the same instruction since
> + * we do not set complete_userspace_io. This does not
> + * handle watchpoints yet, those would be handled in
> + * the emulate_ops.
> + */
> + if (kvm_vcpu_check_breakpoint(vcpu, &r))
> + return r;
> +
> ctxt->interruptibility = 0;
> ctxt->have_exception = false;
> ctxt->perm_ok = false;
> --
> 1.8.1.4
>
--
Gleb.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] KVM: x86: handle singlestep during emulation
2013-07-30 15:11 [PATCH 0/2] KVM: x86: minimal debugging support during emulation Paolo Bonzini
2013-07-30 15:11 ` [PATCH 1/3] KVM: x86: rename EMULATE_DO_MMIO Paolo Bonzini
2013-07-30 15:11 ` [PATCH 2/3] KVM: x86: handle hardware breakpoints during emulation Paolo Bonzini
@ 2013-07-30 15:11 ` Paolo Bonzini
2013-07-31 9:46 ` Gleb Natapov
2 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2013-07-30 15:11 UTC (permalink / raw)
To: linux-kernel; +Cc: gnatapov, kvm
This lets debugging work better during emulation of invalid
guest state.
This time the check is done after emulation, but before writeback
of the flags; we need to check the flags *before* execution of the
instruction, we cannot check singlestep_rip because the CS base may
have already been modified.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/x86.c | 39 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1368cf5..9805cfd 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4971,6 +4971,41 @@ static int kvm_vcpu_check_hw_bp(unsigned long addr, u32 type, u32 dr7,
return dr6;
}
+static void kvm_vcpu_check_singlestep(struct kvm_vcpu *vcpu, int *r)
+{
+ struct kvm_run *kvm_run = vcpu->run;
+
+ /*
+ * Use the "raw" value to see if TF was passed to the processor.
+ * Note that the new value of the flags has not been saved yet.
+ *
+ * This is correct even for TF set by the guest, because "the
+ * processor will not generate this exception after the instruction
+ * that sets the TF flag".
+ */
+ unsigned long rflags = kvm_x86_ops->get_rflags(vcpu);
+
+ if (unlikely(rflags & X86_EFLAGS_TF)) {
+ if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
+ kvm_run->debug.arch.dr6 = DR6_BS | DR6_FIXED_1;
+ kvm_run->debug.arch.pc = vcpu->arch.singlestep_rip;
+ kvm_run->debug.arch.exception = DB_VECTOR;
+ kvm_run->exit_reason = KVM_EXIT_DEBUG;
+ *r = EMULATE_USER_EXIT;
+ } else {
+ vcpu->arch.emulate_ctxt.eflags &= ~X86_EFLAGS_TF;
+ /*
+ * "Certain debug exceptions may clear bit 0-3. The
+ * remaining contents of the DR6 register are never
+ * cleared by the processor".
+ */
+ vcpu->arch.dr6 &= ~15;
+ vcpu->arch.dr6 |= DR6_BS;
+ kvm_queue_exception(vcpu, DB_VECTOR);
+ }
+ }
+}
+
static bool kvm_vcpu_check_breakpoint(struct kvm_vcpu *vcpu, int *r)
{
struct kvm_run *kvm_run = vcpu->run;
@@ -5117,10 +5152,12 @@ restart:
if (writeback) {
toggle_interruptibility(vcpu, ctxt->interruptibility);
- kvm_set_rflags(vcpu, ctxt->eflags);
kvm_make_request(KVM_REQ_EVENT, vcpu);
vcpu->arch.emulate_regs_need_sync_to_vcpu = false;
kvm_rip_write(vcpu, ctxt->eip);
+ if (r == EMULATE_DONE)
+ kvm_vcpu_check_singlestep(vcpu, &r);
+ kvm_set_rflags(vcpu, ctxt->eflags);
} else
vcpu->arch.emulate_regs_need_sync_to_vcpu = true;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] KVM: x86: handle singlestep during emulation
2013-07-30 15:11 ` [PATCH 3/3] KVM: x86: handle singlestep " Paolo Bonzini
@ 2013-07-31 9:46 ` Gleb Natapov
2013-07-31 10:30 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Gleb Natapov @ 2013-07-31 9:46 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm
On Tue, Jul 30, 2013 at 05:11:36PM +0200, Paolo Bonzini wrote:
> This lets debugging work better during emulation of invalid
> guest state.
>
> This time the check is done after emulation, but before writeback
> of the flags; we need to check the flags *before* execution of the
> instruction, we cannot check singlestep_rip because the CS base may
> have already been modified.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> arch/x86/kvm/x86.c | 39 ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1368cf5..9805cfd 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4971,6 +4971,41 @@ static int kvm_vcpu_check_hw_bp(unsigned long addr, u32 type, u32 dr7,
> return dr6;
> }
>
> +static void kvm_vcpu_check_singlestep(struct kvm_vcpu *vcpu, int *r)
> +{
> + struct kvm_run *kvm_run = vcpu->run;
> +
> + /*
> + * Use the "raw" value to see if TF was passed to the processor.
> + * Note that the new value of the flags has not been saved yet.
> + *
> + * This is correct even for TF set by the guest, because "the
> + * processor will not generate this exception after the instruction
> + * that sets the TF flag".
> + */
> + unsigned long rflags = kvm_x86_ops->get_rflags(vcpu);
> +
> + if (unlikely(rflags & X86_EFLAGS_TF)) {
> + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
> + kvm_run->debug.arch.dr6 = DR6_BS | DR6_FIXED_1;
> + kvm_run->debug.arch.pc = vcpu->arch.singlestep_rip;
> + kvm_run->debug.arch.exception = DB_VECTOR;
> + kvm_run->exit_reason = KVM_EXIT_DEBUG;
> + *r = EMULATE_USER_EXIT;
> + } else {
> + vcpu->arch.emulate_ctxt.eflags &= ~X86_EFLAGS_TF;
> + /*
> + * "Certain debug exceptions may clear bit 0-3. The
> + * remaining contents of the DR6 register are never
> + * cleared by the processor".
> + */
> + vcpu->arch.dr6 &= ~15;
> + vcpu->arch.dr6 |= DR6_BS;
> + kvm_queue_exception(vcpu, DB_VECTOR);
> + }
> + }
> +}
> +
> static bool kvm_vcpu_check_breakpoint(struct kvm_vcpu *vcpu, int *r)
> {
> struct kvm_run *kvm_run = vcpu->run;
> @@ -5117,10 +5152,12 @@ restart:
>
> if (writeback) {
> toggle_interruptibility(vcpu, ctxt->interruptibility);
> - kvm_set_rflags(vcpu, ctxt->eflags);
> kvm_make_request(KVM_REQ_EVENT, vcpu);
> vcpu->arch.emulate_regs_need_sync_to_vcpu = false;
> kvm_rip_write(vcpu, ctxt->eip);
> + if (r == EMULATE_DONE)
Single step will not work for mmio write and pio out, we never return
into emulator for those instructions.
> + kvm_vcpu_check_singlestep(vcpu, &r);
> + kvm_set_rflags(vcpu, ctxt->eflags);
> } else
> vcpu->arch.emulate_regs_need_sync_to_vcpu = true;
>
> --
> 1.8.1.4
--
Gleb.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] KVM: x86: handle singlestep during emulation
2013-07-31 9:46 ` Gleb Natapov
@ 2013-07-31 10:30 ` Paolo Bonzini
2013-07-31 10:40 ` Gleb Natapov
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2013-07-31 10:30 UTC (permalink / raw)
To: Gleb Natapov; +Cc: linux-kernel, kvm
----- Gleb Natapov <gleb@redhat.com> wrote:
> On Tue, Jul 30, 2013 at 05:11:36PM +0200, Paolo Bonzini wrote:
> > This lets debugging work better during emulation of invalid
> > guest state.
> >
> > This time the check is done after emulation, but before writeback
> > of the flags; we need to check the flags *before* execution of the
> > instruction, we cannot check singlestep_rip because the CS base may
> > have already been modified.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > arch/x86/kvm/x86.c | 39 ++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 1368cf5..9805cfd 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -4971,6 +4971,41 @@ static int kvm_vcpu_check_hw_bp(unsigned long addr, u32 type, u32 dr7,
> > return dr6;
> > }
> >
> > +static void kvm_vcpu_check_singlestep(struct kvm_vcpu *vcpu, int *r)
> > +{
> > + struct kvm_run *kvm_run = vcpu->run;
> > +
> > + /*
> > + * Use the "raw" value to see if TF was passed to the processor.
> > + * Note that the new value of the flags has not been saved yet.
> > + *
> > + * This is correct even for TF set by the guest, because "the
> > + * processor will not generate this exception after the instruction
> > + * that sets the TF flag".
> > + */
> > + unsigned long rflags = kvm_x86_ops->get_rflags(vcpu);
> > +
> > + if (unlikely(rflags & X86_EFLAGS_TF)) {
> > + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
> > + kvm_run->debug.arch.dr6 = DR6_BS | DR6_FIXED_1;
> > + kvm_run->debug.arch.pc = vcpu->arch.singlestep_rip;
> > + kvm_run->debug.arch.exception = DB_VECTOR;
> > + kvm_run->exit_reason = KVM_EXIT_DEBUG;
> > + *r = EMULATE_USER_EXIT;
> > + } else {
> > + vcpu->arch.emulate_ctxt.eflags &= ~X86_EFLAGS_TF;
> > + /*
> > + * "Certain debug exceptions may clear bit 0-3. The
> > + * remaining contents of the DR6 register are never
> > + * cleared by the processor".
> > + */
> > + vcpu->arch.dr6 &= ~15;
> > + vcpu->arch.dr6 |= DR6_BS;
> > + kvm_queue_exception(vcpu, DB_VECTOR);
> > + }
> > + }
> > +}
> > +
> > static bool kvm_vcpu_check_breakpoint(struct kvm_vcpu *vcpu, int *r)
> > {
> > struct kvm_run *kvm_run = vcpu->run;
> > @@ -5117,10 +5152,12 @@ restart:
> >
> > if (writeback) {
> > toggle_interruptibility(vcpu, ctxt->interruptibility);
> > - kvm_set_rflags(vcpu, ctxt->eflags);
> > kvm_make_request(KVM_REQ_EVENT, vcpu);
> > vcpu->arch.emulate_regs_need_sync_to_vcpu = false;
> > kvm_rip_write(vcpu, ctxt->eip);
> > + if (r == EMULATE_DONE)
> Single step will not work for mmio write and pio out, we never return
> into emulator for those instructions.
Ok to apply the patch as is and work it out later (I suppose I need to
check for NULL complete_userspace_io, and if so set my function)? It
is already a huge improvement in usability.
Paolo
> > + kvm_vcpu_check_singlestep(vcpu, &r);
> > + kvm_set_rflags(vcpu, ctxt->eflags);
> > } else
> > vcpu->arch.emulate_regs_need_sync_to_vcpu = true;
> >
> > --
> > 1.8.1.4
>
> --
> Gleb.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] KVM: x86: handle singlestep during emulation
2013-07-31 10:30 ` Paolo Bonzini
@ 2013-07-31 10:40 ` Gleb Natapov
0 siblings, 0 replies; 9+ messages in thread
From: Gleb Natapov @ 2013-07-31 10:40 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm
On Wed, Jul 31, 2013 at 06:30:31AM -0400, Paolo Bonzini wrote:
>
> ----- Gleb Natapov <gleb@redhat.com> wrote:
> > On Tue, Jul 30, 2013 at 05:11:36PM +0200, Paolo Bonzini wrote:
> > > This lets debugging work better during emulation of invalid
> > > guest state.
> > >
> > > This time the check is done after emulation, but before writeback
> > > of the flags; we need to check the flags *before* execution of the
> > > instruction, we cannot check singlestep_rip because the CS base may
> > > have already been modified.
> > >
> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > > ---
> > > arch/x86/kvm/x86.c | 39 ++++++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 38 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index 1368cf5..9805cfd 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -4971,6 +4971,41 @@ static int kvm_vcpu_check_hw_bp(unsigned long addr, u32 type, u32 dr7,
> > > return dr6;
> > > }
> > >
> > > +static void kvm_vcpu_check_singlestep(struct kvm_vcpu *vcpu, int *r)
> > > +{
> > > + struct kvm_run *kvm_run = vcpu->run;
> > > +
> > > + /*
> > > + * Use the "raw" value to see if TF was passed to the processor.
> > > + * Note that the new value of the flags has not been saved yet.
> > > + *
> > > + * This is correct even for TF set by the guest, because "the
> > > + * processor will not generate this exception after the instruction
> > > + * that sets the TF flag".
> > > + */
> > > + unsigned long rflags = kvm_x86_ops->get_rflags(vcpu);
> > > +
> > > + if (unlikely(rflags & X86_EFLAGS_TF)) {
> > > + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
> > > + kvm_run->debug.arch.dr6 = DR6_BS | DR6_FIXED_1;
> > > + kvm_run->debug.arch.pc = vcpu->arch.singlestep_rip;
> > > + kvm_run->debug.arch.exception = DB_VECTOR;
> > > + kvm_run->exit_reason = KVM_EXIT_DEBUG;
> > > + *r = EMULATE_USER_EXIT;
> > > + } else {
> > > + vcpu->arch.emulate_ctxt.eflags &= ~X86_EFLAGS_TF;
> > > + /*
> > > + * "Certain debug exceptions may clear bit 0-3. The
> > > + * remaining contents of the DR6 register are never
> > > + * cleared by the processor".
> > > + */
> > > + vcpu->arch.dr6 &= ~15;
> > > + vcpu->arch.dr6 |= DR6_BS;
> > > + kvm_queue_exception(vcpu, DB_VECTOR);
> > > + }
> > > + }
> > > +}
> > > +
> > > static bool kvm_vcpu_check_breakpoint(struct kvm_vcpu *vcpu, int *r)
> > > {
> > > struct kvm_run *kvm_run = vcpu->run;
> > > @@ -5117,10 +5152,12 @@ restart:
> > >
> > > if (writeback) {
> > > toggle_interruptibility(vcpu, ctxt->interruptibility);
> > > - kvm_set_rflags(vcpu, ctxt->eflags);
> > > kvm_make_request(KVM_REQ_EVENT, vcpu);
> > > vcpu->arch.emulate_regs_need_sync_to_vcpu = false;
> > > kvm_rip_write(vcpu, ctxt->eip);
> > > + if (r == EMULATE_DONE)
> > Single step will not work for mmio write and pio out, we never return
> > into emulator for those instructions.
>
> Ok to apply the patch as is and work it out later (I suppose I need to
> check for NULL complete_userspace_io, and if so set my function)? It
> is already a huge improvement in usability.
>
This is not worse from what we have now, so lets apply it, but please add
comment here that write case is not handled properly yet to not forget
about it.
complete_userspace_io is not null for write MMIO but the execution
never returns to emulator, so this is not so simple. May be set
vcpu->arch.io_singlestep and check it in complete_userspace_io.
--
Gleb.
^ permalink raw reply [flat|nested] 9+ messages in thread