KVM: Improved guest debugging / debug register emulation
cover KVM: Improved guest debugging / debug register emulation
This is the kernel part of the guest debugging / debug register emulation patch series.
Changes since last round:
- Reinject both soft *and* hard exceptions in vmx_complete_interrupts()
Find the patches also at git://git.kiszka.org/linux-kvm.git gdb-queue
Jan
--
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux
PATCH 1/5 VMX: Support for injecting software exceptions
VMX differentiates between processor and software generated exceptions when injecting them into the guest. Extend vmx_queue_exception accordingly (and refactor related constants) so that we can use this service reliably for the new guest debugging framework.
---
arch/x86/include/asm/vmx.h | 3 ++- arch/x86/kvm/vmx.c | 35 ++++++++++++++++++++--------------- 2 files changed, 22 insertions(+), 16 deletions(-)
arch/x86/include/asm/vmx.h +2 -1
@@ -270,8 +270,9 @@ enum vmcs_field {#define INTR_TYPE_EXT_INTR (0 << 8)/* external interrupt */ #define INTR_TYPE_NMI_INTR (2 << 8)/* NMI */ -#define INTR_TYPE_EXCEPTION (3 << 8)/* processor exception */ +#define INTR_TYPE_HARD_EXCEPTION (3 << 8)/* processor exception */ #define INTR_TYPE_SOFT_INTR (4 << 8)/* software interrupt */ +#define INTR_TYPE_SOFT_EXCEPTION (6 << 8)/* software exception */ /* GUEST_INTERRUPTIBILITY_INFO flags. */ #define GUEST_INTR_STATE_STI 0x00000001
arch/x86/kvm/vmx.c +20 -15
@@ -189,21 +189,21 @@ static inline int is_page_fault(u32 intr_info) {return (intr_info & (INTR_INFO_INTR_TYPE_MASK |INTR_INFO_VECTOR_MASK |INTR_INFO_VALID_MASK )) == - (INTR_TYPE_EXCEPTION |PF_VECTOR |INTR_INFO_VALID_MASK ); + (INTR_TYPE_HARD_EXCEPTION |PF_VECTOR |INTR_INFO_VALID_MASK ); }static inline int is_no_device (u32 intr_info) {return (intr_info & (INTR_INFO_INTR_TYPE_MASK |INTR_INFO_VECTOR_MASK |INTR_INFO_VALID_MASK )) == - (INTR_TYPE_EXCEPTION |NM_VECTOR |INTR_INFO_VALID_MASK ); + (INTR_TYPE_HARD_EXCEPTION |NM_VECTOR |INTR_INFO_VALID_MASK ); }static inline int is_invalid_opcode (u32 intr_info) {return (intr_info & (INTR_INFO_INTR_TYPE_MASK |INTR_INFO_VECTOR_MASK |INTR_INFO_VALID_MASK )) == - (INTR_TYPE_EXCEPTION |UD_VECTOR |INTR_INFO_VALID_MASK ); + (INTR_TYPE_HARD_EXCEPTION |UD_VECTOR |INTR_INFO_VALID_MASK ); }static inline int is_external_interrupt (u32 intr_info)
@@ -747,29 +747,33 @@ static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr, bool has_error_code, u32 error_code) {struct vcpu_vmx *vmx =to_vmx (vcpu); +u32 intr_info = nr |INTR_INFO_VALID_MASK ; -if (has_error_code) +if (has_error_code) {vmcs_write32 (VM_ENTRY_EXCEPTION_ERROR_CODE , error_code); + intr_info |=INTR_INFO_DELIVER_CODE_MASK ; + }if (vcpu->arch.rmode.active) { vmx->rmode.irq.pending = true; vmx->rmode.irq.vector = nr; vmx->rmode.irq.rip =kvm_rip_read (vcpu); -if (nr ==BP_VECTOR ) +if (nr ==BP_VECTOR || nr ==OF_VECTOR ) vmx->rmode.irq.rip++; -vmcs_write32 (VM_ENTRY_INTR_INFO_FIELD , - nr |INTR_TYPE_SOFT_INTR - | (has_error_code ?INTR_INFO_DELIVER_CODE_MASK :0 ) - |INTR_INFO_VALID_MASK ); + intr_info |=INTR_TYPE_SOFT_INTR ; +vmcs_write32 (VM_ENTRY_INTR_INFO_FIELD , intr_info);vmcs_write32 (VM_ENTRY_INSTRUCTION_LEN ,1 );kvm_rip_write (vcpu, vmx->rmode.irq.rip -1 );return ; } -vmcs_write32 (VM_ENTRY_INTR_INFO_FIELD , - nr |INTR_TYPE_EXCEPTION - | (has_error_code ?INTR_INFO_DELIVER_CODE_MASK :0 ) - |INTR_INFO_VALID_MASK ); +if (nr ==BP_VECTOR || nr ==OF_VECTOR ) { +vmcs_write32 (VM_ENTRY_INSTRUCTION_LEN ,1 ); + intr_info |=INTR_TYPE_SOFT_EXCEPTION ; + }else + intr_info |=INTR_TYPE_HARD_EXCEPTION ; + +vmcs_write32 (VM_ENTRY_INTR_INFO_FIELD , intr_info); }static bool vmx_exception_injected (struct kvm_vcpu *vcpu)
@@ -2649,7 +2653,7 @@ static int handle_exception(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) }if ((intr_info & (INTR_INFO_INTR_TYPE_MASK |INTR_INFO_VECTOR_MASK )) == - (INTR_TYPE_EXCEPTION |1 )) { + (INTR_TYPE_HARD_EXCEPTION |1 )) { kvm_run->exit_reason =KVM_EXIT_DEBUG ;return 0 ; }
@@ -3255,7 +3259,8 @@ static void vmx_complete_interrupts(struct vcpu_vmx *vmx) vmx->vcpu.arch.nmi_injected = false; }kvm_clear_exception_queue (&vmx->vcpu); -if (idtv_info_valid && type ==INTR_TYPE_EXCEPTION ) { +if (idtv_info_valid && (type ==INTR_TYPE_HARD_EXCEPTION || + type ==INTR_TYPE_SOFT_EXCEPTION )) {if (idt_vectoring_info &VECTORING_INFO_DELIVER_CODE_MASK ) { error =vmcs_read32 (IDT_VECTORING_ERROR_CODE );kvm_queue_exception_e (&vmx->vcpu, vector, error);
PATCH 2/5 KVM: New guest debug interface
In reply to Jan KiszkaThis rips out the support for KVM_DEBUG_GUEST and introduces a new IOCTL
instead: KVM_SET_GUEST_DEBUG. The IOCTL payload consists of a generic part, controlling the "main switch" and the single-step feature. The arch specific part adds an x86 interface for intercepting both types of debug exceptions separately and re-injecting them when the host was not interested. Moveover, the foundation for guest debugging via debug registers is layed.
To signal breakpoint events properly back to userland, an arch-specific data block is now returned along KVM_EXIT_DEBUG. For x86, the arch block contains the PC, the debug exception, and relevant debug registers to tell debug events properly apart.
The availability of this new interface is signaled by KVM_CAP_SET_GUEST_DEBUG. Empty stubs for not yet supported archs are provided.
Note that both SVM and VTX are supported, but only the latter was tested yet. Based on the experience with all those VTX corner case, I would be fairly surprised if SVM will work out of the box.
---
arch/ia64/include/asm/kvm.h | 7 +++ arch/ia64/kvm/kvm-ia64.c | 4 +- arch/powerpc/include/asm/kvm.h | 7 +++ arch/powerpc/kvm/powerpc.c | 4 +- arch/s390/include/asm/kvm.h | 7 +++ arch/s390/kvm/kvm-s390.c | 4 +- arch/x86/include/asm/kvm.h | 18 ++++++++ arch/x86/include/asm/kvm_host.h | 9 ---- arch/x86/kvm/svm.c | 50 ++++++++++++++++++++- arch/x86/kvm/vmx.c | 93 +++++++++++++++------------------------ arch/x86/kvm/x86.c | 14 +++--- include/linux/kvm.h | 51 +++++++++++++++------ include/linux/kvm_host.h | 6 +-- virt/kvm/kvm_main.c | 6 +-- 14 files changed, 179 insertions(+), 101 deletions(-)
arch/ia64/include/asm/kvm.h +7 -0
@@ -208,4 +208,11 @@ struct kvm_sregs {struct kvm_fpu { }; +struct kvm_debug_exit_arch { +}; + +/* for KVM_SET_GUEST_DEBUG */ +struct kvm_guest_debug_arch { +}; +#endif
arch/ia64/kvm/kvm-ia64.c +2 -2
@@ -1316,8 +1316,8 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)return -EINVAL ; } -int kvm_arch_vcpu_ioctl_debug_guest (struct kvm_vcpu *vcpu, -struct kvm_debug_guest *dbg) +int kvm_arch_vcpu_ioctl_set_guest_debug (struct kvm_vcpu *vcpu, +struct kvm_guest_debug *dbg) {return -EINVAL ; }
arch/powerpc/include/asm/kvm.h +7 -0
@@ -52,4 +52,11 @@ struct kvm_fpu { __u64 fpr[32]; }; +struct kvm_debug_exit_arch { +}; + +/* for KVM_SET_GUEST_DEBUG */ +struct kvm_guest_debug_arch { +}; +#endif /* __LINUX_KVM_POWERPC_H */
arch/powerpc/kvm/powerpc.c +2 -2
@@ -229,8 +229,8 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)kvmppc_core_vcpu_put (vcpu); } -int kvm_arch_vcpu_ioctl_debug_guest (struct kvm_vcpu *vcpu, -struct kvm_debug_guest *dbg) +int kvm_arch_vcpu_ioctl_set_guest_debug (struct kvm_vcpu *vcpu, +struct kvm_guest_debug *dbg) {int i;
arch/s390/include/asm/kvm.h +7 -0
@@ -42,4 +42,11 @@ struct kvm_fpu { __u64 fprs[16]; }; +struct kvm_debug_exit_arch { +}; + +/* for KVM_SET_GUEST_DEBUG */ +struct kvm_guest_debug_arch { +}; +#endif
arch/s390/kvm/kvm-s390.c +2 -2
@@ -413,8 +413,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,return -EINVAL ;/* not implemented yet */ } -int kvm_arch_vcpu_ioctl_debug_guest (struct kvm_vcpu *vcpu, -struct kvm_debug_guest *dbg) +int kvm_arch_vcpu_ioctl_set_guest_debug (struct kvm_vcpu *vcpu, +struct kvm_guest_debug *dbg) {return -EINVAL ;/* not implemented yet */ }
arch/x86/include/asm/kvm.h +18 -0
@@ -205,6 +205,24 @@ struct kvm_pit_channel_state {__s64 count_load_time; }; +struct kvm_debug_exit_arch { +__u32 exception; +__u32 pad; +__u64 pc; +__u64 dr6; +__u64 dr7; +}; + +#define KVM_GUESTDBG_USE_SW_BP 0x00010000 +#define KVM_GUESTDBG_USE_HW_BP 0x00020000 +#define KVM_GUESTDBG_INJECT_DB 0x00040000 +#define KVM_GUESTDBG_INJECT_BP 0x00080000 + +/* for KVM_SET_GUEST_DEBUG */ +struct kvm_guest_debug_arch { +__u64 debugreg[8 ]; +}; +struct kvm_pit_state {struct kvm_pit_channel_state channels[3 ]; };
arch/x86/include/asm/kvm_host.h +1 -8
@@ -134,12 +134,6 @@ enum {#define KVM_NR_MEM_OBJS 40 -struct kvm_guest_debug { -int enabled; -unsigned long bp[4 ]; -int singlestep; -}; -/* * We don't want allocation failures within the mmu code, so we preallocate * enough memory for a single page fault in a cache.
@@ -441,8 +435,7 @@ struct kvm_x86_ops {void (*vcpu_put)(struct kvm_vcpu *vcpu);int (*set_guest_debug)(struct kvm_vcpu *vcpu, -struct kvm_debug_guest *dbg); -void (*guest_debug_pre)(struct kvm_vcpu *vcpu); +struct kvm_guest_debug *dbg);int (*get_msr)(struct kvm_vcpu *vcpu,u32 msr_index,u64 *pdata);int (*set_msr)(struct kvm_vcpu *vcpu,u32 msr_index,u64 data);u64 (*get_segment_base)(struct kvm_vcpu *vcpu,int seg);
arch/x86/kvm/svm.c +48 -2
69 lines not on this page · raw message
arch/x86/kvm/vmx.c +36 -57
145 lines not on this page · raw message
arch/x86/kvm/x86.c +8 -6
@@ -2993,9 +2993,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) goto out; } -if (vcpu->guest_debug.enabled) - kvm_x86_ops->guest_debug_pre (vcpu); - vcpu->guest_mode =1 ;/* * Make sure that guest_mode assignment won't happen after
@@ -3211,7 +3208,7 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)/* * Don't leak debug flags in case they were set for guest debugging */ -if (vcpu->guest_debug.enabled && vcpu->guest_debug.singlestep) +if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) regs->rflags &= ~(X86_EFLAGS_TF |X86_EFLAGS_RF );vcpu_put (vcpu);
@@ -3830,8 +3827,8 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,return 0 ; } -int kvm_arch_vcpu_ioctl_debug_guest (struct kvm_vcpu *vcpu, -struct kvm_debug_guest *dbg) +int kvm_arch_vcpu_ioctl_set_guest_debug (struct kvm_vcpu *vcpu, +struct kvm_guest_debug *dbg) {int r;
@@ -3839,6 +3836,11 @@ int kvm_arch_vcpu_ioctl_debug_guest(struct kvm_vcpu *vcpu, r = kvm_x86_ops->set_guest_debug (vcpu, dbg); +if (dbg->control &KVM_GUESTDBG_INJECT_DB ) +kvm_queue_exception (vcpu,DB_VECTOR ); +else if (dbg->control &KVM_GUESTDBG_INJECT_BP ) +kvm_queue_exception (vcpu,BP_VECTOR ); +vcpu_put (vcpu);return r;
include/linux/kvm.h +35 -16
87 lines not on this page · raw message
include/linux/kvm_host.h +3 -3
@@ -73,7 +73,7 @@ struct kvm_vcpu {struct kvm_run *run;int guest_mode;unsigned long requests; -struct kvm_guest_debug guest_debug; +unsigned long guest_debug;int fpu_active;int guest_fpu_loaded;wait_queue_head_t wq;
@@ -255,8 +255,8 @@ int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu, struct kvm_mp_state *mp_state);int kvm_arch_vcpu_ioctl_set_mpstate (struct kvm_vcpu *vcpu,struct kvm_mp_state *mp_state); -int kvm_arch_vcpu_ioctl_debug_guest (struct kvm_vcpu *vcpu, -struct kvm_debug_guest *dbg); +int kvm_arch_vcpu_ioctl_set_guest_debug (struct kvm_vcpu *vcpu, +struct kvm_guest_debug *dbg);int kvm_arch_vcpu_ioctl_run (struct kvm_vcpu *vcpu,struct kvm_run *kvm_run);int kvm_arch_init (void *opaque);
virt/kvm/kvm_main.c +3 -3
@@ -1672,13 +1672,13 @@ out_free2: r =0 ;break ; } -case KVM_DEBUG_GUEST : { -struct kvm_debug_guest dbg; +case KVM_SET_GUEST_DEBUG : { +struct kvm_guest_debug dbg; r = -EFAULT ;if (copy_from_user (&dbg, argp,sizeof dbg)) goto out; - r =kvm_arch_vcpu_ioctl_debug_guest (vcpu, &dbg); + r =kvm_arch_vcpu_ioctl_set_guest_debug (vcpu, &dbg);if (r) goto out; r =0 ;
Re: [PATCH 2/5] KVM: New guest debug interface
Quoted 8 lines · Jan Kiszka
This rips out the support for
KVM_DEBUG_GUESTand introduces a new IOCTL
instead:KVM_SET_GUEST_DEBUG. The IOCTL payload consists of a generic part, controlling the "main switch" and the single-step feature. The arch specific part adds an x86 interface for intercepting both types of debug exceptions separately and re-injecting them when the host was not interested. Moveover, the foundation for guest debugging via debug registers is layed.
Have you tested compile-time compatibility with older userspace?
Quoted 5 lines
Note that both SVM and VTX are supported, but only the latter was tested yet. Based on the experience with all those VTX corner case, I would be fairly surprised if SVM will work out of the box.
I'd like svm to work before applying.
Quoted 11 lines
include/linux/kvm.h +2
-1@@ -439,7 +437,8 @@ struct kvm_trace_rec {#define KVM_SET_SREGS _IOW(KVMIO, 0x84, struct kvm_sregs)#define KVM_TRANSLATE _IOWR(KVMIO, 0x85, struct kvm_translation)#define KVM_INTERRUPT _IOW(KVMIO, 0x86, struct kvm_interrupt) -#define KVM_DEBUG_GUEST _IOW(KVMIO, 0x87, struct kvm_debug_guest) +/* KVM_DEBUG_GUEST is no longer supported, use KVM_SET_GUEST_DEBUG instead */ +#define KVM_DEBUG_GUEST __KVM_DEPRECATED_DEBUG_GUEST#define KVM_GET_MSRS _IOWR(KVMIO, 0x88, struct kvm_msrs)#define KVM_SET_MSRS _IOW(KVMIO, 0x89, struct kvm_msrs)#define KVM_SET_CPUID _IOW(KVMIO, 0x8a, struct kvm_cpuid)
include/linux/kvm.h struct kvm_trace_rec { · line 473
Quoted 7 lines
include/linux/kvm.h +3
-0@@ -468,6 +467,26 @@ struct kvm_trace_rec {#define KVM_SET_MP_STATE _IOW(KVMIO, 0x99, struct kvm_mp_state)/* Available with KVM_CAP_NMI */ #define KVM_NMI _IO(KVMIO, 0x9a) +/* Available with KVM_CAP_SET_GUEST_DEBUG */ +#define KVM_SET_GUEST_DEBUG _IOW(KVMIO, 0x9a, struct kvm_guest_debug) +
0x9b...
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH 2/5] KVM: New guest debug interface
Quoted 11 lines · Avi Kivity
Quoted 8 lines · Jan Kiszka
This rips out the support for
KVM_DEBUG_GUESTand introduces a new IOCTL
instead:KVM_SET_GUEST_DEBUG. The IOCTL payload consists of a generic part, controlling the "main switch" and the single-step feature. The arch specific part adds an x86 interface for intercepting both types of debug exceptions separately and re-injecting them when the host was not interested. Moveover, the foundation for guest debugging via debug registers is layed.Have you tested compile-time compatibility with older userspace?
Yes, works.
Quoted 8 lines
Quoted 6 lines
Note that both SVM and VTX are supported, but only the latter was tested yet. Based on the experience with all those VTX corner case, I would be fairly surprised if SVM will work out of the box.
I'd like svm to work before applying.
To validate the design?
I will see if I can organize an SVM box, but I can't promise when I'll be able to do the testing. Anyone willing to contribute time on this would be warmly welcome!
Quoted 19 lines
Quoted 19 lines
(quoted diff) +1
-1@@ -439,7 +437,8 @@ struct kvm_trace_rec { #define KVM_SET_SREGS _IOW(KVMIO, 0x84, struct kvm_sregs) #define KVM_TRANSLATE _IOWR(KVMIO, 0x85, struct kvm_translation) #define KVM_INTERRUPT _IOW(KVMIO, 0x86, struct kvm_interrupt) -#define KVM_DEBUG_GUEST _IOW(KVMIO, 0x87, struct kvm_debug_guest) +/* KVM_DEBUG_GUEST is no longer supported, use KVM_SET_GUEST_DEBUGinstead */
include/linux/kvm.h +1
-0+#define KVM_DEBUG_GUEST __KVM_DEPRECATED_DEBUG_GUEST#define KVM_GET_MSRS _IOWR(KVMIO, 0x88, struct kvm_msrs)#define KVM_SET_MSRS _IOW(KVMIO, 0x89, struct kvm_msrs)#define KVM_SET_CPUID _IOW(KVMIO, 0x8a, struct kvm_cpuid)@@ -468,6 +467,26 @@ struct kvm_trace_rec {#define KVM_SET_MP_STATE _IOW(KVMIO, 0x99, struct kvm_mp_state)/* Available with KVM_CAP_NMI */
Quoted 8 lines
include/linux/kvm.h struct kvm_trace_rec { · line 490
Quoted 6 lines
include/linux/kvm.h +3
-0#define KVM_NMI _IO(KVMIO, 0x9a) +/* Available with KVM_CAP_SET_GUEST_DEBUG */ +#define KVM_SET_GUEST_DEBUG _IOW(KVMIO, 0x9a, struct kvm_guest_debug) +
0x9b...
Oh, rebased a bit too often.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 26 Corporate Competence Center Embedded Linux
Re: [PATCH 2/5] KVM: New guest debug interface
Quoted 6 lines · Jan Kiszka
Quoted 6 lines
Quoted 6 lines
Note that both SVM and VTX are supported, but only the latter was tested yet. Based on the experience with all those VTX corner case, I would be fairly surprised if SVM will work out of the box.
Quoted 6 lines
Quoted 2 lines
I'd like svm to work before applying.
To validate the design?
To make sure it doesn't oops, and to make sure it works.
Quoted 5 lines
I will see if I can organize an SVM box, but I can't promise when I'll be able to do the testing. Anyone willing to contribute time on this would be warmly welcome!
I happen to have an svm box. How do you test this? play with gdb inside and outside the guest?
You could also try qemu's svm emulation, but it may be a little slow, and probably hasn't been well tested in these areas.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [PATCH 2/5] KVM: New guest debug interface
Quoted 16 lines · Avi Kivity
Quoted 6 lines · Jan Kiszka
Quoted 6 lines
Quoted 6 lines
Note that both SVM and VTX are supported, but only the latter was tested
yet. Based on the experience with all those VTX corner case, I would be fairly surprised if SVM will work out of the box.Quoted 6 lines
Quoted 2 lines
I'd like svm to work before applying.
To validate the design?
To make sure it doesn't oops, and to make sure it works.
Quoted 8 lines
Quoted 5 lines
I will see if I can organize an SVM box, but I can't promise when I'll be able to do the testing. Anyone willing to contribute time on this would be warmly welcome!
I happen to have an svm box. How do you test this? play with gdb inside and outside the guest?
For inside the box, I also use the kgdb testsuite (kgbts).
For guest debugging check, some manual interaction is required: Set a breakpoint on a prominent symbol (my favorite is sys_execve), do some single stepping, check that smp works (breakpoint hits on all vcpus), add some watchpoint and check if it triggers at reasonable spots (I recently did a short demo with first trapping hrtimer_start, grabbing &timer->state and then set a watchpoint on that address to catch changes).
Quoted 4 lines
You could also try qemu's svm emulation, but it may be a little slow, and probably hasn't been well tested in these areas.
Yeah, could be a supplemental test.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 26 Corporate Competence Center Embedded Linux
Re: [PATCH 2/5] KVM: New guest debug interface
In reply to Avi KivityQuoted 22 lines · Avi Kivity
Quoted 6 lines · Jan Kiszka
Quoted 6 lines
Quoted 6 lines
Note that both SVM and VTX are supported, but only the latter was tested
yet. Based on the experience with all those VTX corner case, I would be fairly surprised if SVM will work out of the box.Quoted 6 lines
Quoted 2 lines
I'd like svm to work before applying.
To validate the design?
To make sure it doesn't oops, and to make sure it works.
Quoted 5 lines
I will see if I can organize an SVM box, but I can't promise when I'll be able to do the testing. Anyone willing to contribute time on this would be warmly welcome!
Quoted 5 lines
I happen to have an svm box. How do you test this? play with gdb inside and outside the guest?
You could also try qemu's svm emulation, but it may be a little slow, and probably hasn't been well tested in these areas.
FYI: I just passed the kgdbts test with some Linux guest on an Opteron 2216. Also using hbreak with gdb inside the guest appears to work. There are just a few to-be-analyzed guest lockups/kernel panics that may relate to the fact that the kernels were build for Intel CPUs. On the other hand, it would be far too simple if it worked already - compared to the pain I went through with VMX...
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 26 Corporate Competence Center Embedded Linux
Re: [PATCH 2/5] KVM: New guest debug interface
Quoted 7 lines · Jan Kiszka
FYI: I just passed the kgdbts test with some Linux guest on an Opteron 2216. Also using hbreak with gdb inside the guest appears to work. There are just a few to-be-analyzed guest lockups/kernel panics that may relate to the fact that the kernels were build for Intel CPUs. On the other hand, it would be far too simple if it worked already - compared to the pain I went through with VMX...
So long as it doesn't crash, this functionality is far enough from mainstream use.
btw, is it possible to emulate the old interface on top of the new implementation? I feel unconfortable with dropping an interface (albeit an unused one) without even a warning.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH 2/5] KVM: New guest debug interface
Quoted 11 lines · Avi Kivity
Quoted 7 lines · Jan Kiszka
FYI: I just passed the kgdbts test with some Linux guest on an Opteron 2216. Also using hbreak with gdb inside the guest appears to work. There are just a few to-be-analyzed guest lockups/kernel panics that may relate to the fact that the kernels were build for Intel CPUs. On the other hand, it would be far too simple if it worked already - compared to the pain I went through with VMX...
So long as it doesn't crash, this functionality is far enough from mainstream use.
I would love to dig a bit deeper, doing more SMP tests, maybe also adding userspace NMI support to SVM at this chance, but I was just called away again. Hope to find some time next week, we'll see.
Quoted 4 lines
btw, is it possible to emulate the old interface on top of the new implementation? I feel unconfortable with dropping an interface (albeit an unused one) without even a warning.
Would be possible, I guess - but do we really have to care? It only worked for VMX, only on UP, qemu has no support for it (I'm planning to submit the new interface once it's merged and settled), and I don't think many people actually touched the old one so far. And those who may throw the old IOCTL at a new kernel will get a proper error on return. Is this problematic?
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 26 Corporate Competence Center Embedded Linux
Re: [PATCH 2/5] KVM: New guest debug interface
Quoted 5 lines · Jan Kiszka
Quoted 4 lines
btw, is it possible to emulate the old interface on top of the new implementation? I feel unconfortable with dropping an interface (albeit an unused one) without even a warning.
Quoted 7 lines
Would be possible, I guess - but do we really have to care? It only worked for VMX, only on UP, qemu has no support for it (I'm planning to submit the new interface once it's merged and settled), and I don't think many people actually touched the old one so far. And those who may throw the old IOCTL at a new kernel will get a proper error on return. Is this problematic?
No, just exploring our options. I'm okay with dropping the old implementation.
Signature · 1 line
error compiling committee.c: too many arguments to function
PATCH 3/5 KVM: VMX: Ensure interruptibility when single-stepping
In reply to Jan KiszkaWhen single-stepping, we have to ensure that the INT1 can make it through even if the guest itself is uninterruptible due to MOV SS or STI. VMENTRY will fail otherwise.
---
arch/x86/kvm/vmx.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)
arch/x86/kvm/vmx.c +8 -2
@@ -1010,6 +1010,7 @@ static void vmx_cache_reg(struct kvm_vcpu *vcpu, enum kvm_reg reg)static int set_guest_debug (struct kvm_vcpu *vcpu,struct kvm_guest_debug *dbg) {int old_debug = vcpu->guest_debug; +u32 interruptibility;unsigned long flags; vcpu->guest_debug = dbg->control;
@@ -1017,9 +1018,14 @@ static int set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) vcpu->guest_debug =0 ; flags =vmcs_readl (GUEST_RFLAGS ); -if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) +if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) { flags |=X86_EFLAGS_TF |X86_EFLAGS_RF ; -else if (old_debug &KVM_GUESTDBG_SINGLESTEP ) +/* We must be interruptible when single-stepping */ + interruptibility =vmcs_read32 (GUEST_INTERRUPTIBILITY_INFO ); +if (interruptibility &3 ) +vmcs_write32 (GUEST_INTERRUPTIBILITY_INFO , + interruptibility & ~3 ); + }else if (old_debug &KVM_GUESTDBG_SINGLESTEP ) flags &= ~(X86_EFLAGS_TF |X86_EFLAGS_RF );vmcs_writel (GUEST_RFLAGS , flags);
Re: [PATCH 3/5] KVM: VMX: Ensure interruptibility when single-stepping
Quoted 26 lines · Jan Kiszka
When single-stepping, we have to ensure that the INT1 can make it through even if the guest itself is uninterruptible due to MOV SS or STI. VMENTRY will fail otherwise.
Signed-off-by Jan Kiszka <[email protected]>---
arch/x86/kvm/vmx.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)arch/x86/kvm/vmx.c +1
-0@@ -1010,6 +1010,7 @@ static void vmx_cache_reg(struct kvm_vcpu *vcpu, enum kvm_reg reg)static int set_guest_debug (struct kvm_vcpu *vcpu,struct kvm_guest_debug *dbg) {int old_debug = vcpu->guest_debug; +u32 interruptibility;unsigned long flags; vcpu->guest_debug = dbg->control;@@ -1017,9 +1018,14 @@ static int set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) vcpu->guest_debug =0 ; flags =vmcs_readl (GUEST_RFLAGS );
arch/x86/kvm/vmx.c static int set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) · line 1028
Quoted 10 lines
arch/x86/kvm/vmx.c +6
-2-if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) +if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) { flags |=X86_EFLAGS_TF |X86_EFLAGS_RF ; -else if (old_debug &KVM_GUESTDBG_SINGLESTEP ) +/* We must be interruptible when single-stepping */ + interruptibility =vmcs_read32 (GUEST_INTERRUPTIBILITY_INFO ); +if (interruptibility &3 ) +vmcs_write32 (GUEST_INTERRUPTIBILITY_INFO , + interruptibility & ~3 );
Could just write unconditionally - it's not like the write has any effect on speed. vmcs_clear_bits() will do it cleanly.
But I'm worried about correctness. Suppose we're singlestepping a sti; hlt sequence. While we'll clear interruptibility, probably receive the debug trap (since that's a high priority exception), but then inject the interrupt before the hlt, hanging the guest. So we probably need to restore interruptibility on exit.
This looks like a good candidate for a test case.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH 3/5] KVM: VMX: Ensure interruptibility when single-stepping
Quoted 29 lines · Avi Kivity
Quoted 28 lines · Jan Kiszka
When single-stepping, we have to ensure that the INT1 can make it through even if the guest itself is uninterruptible due to MOV SS or STI. VMENTRY will fail otherwise.
Signed-off-by Jan Kiszka <[email protected]>---
arch/x86/kvm/vmx.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)arch/x86/kvm/vmx.c +1
-0@@ -1010,6 +1010,7 @@ static void vmx_cache_reg(struct kvm_vcpu *vcpu, enum kvm_reg reg)static int set_guest_debug (struct kvm_vcpu *vcpu,struct kvm_guest_debug *dbg) {int old_debug = vcpu->guest_debug; +u32 interruptibility;unsigned long flags;vcpu->guest_debug = dbg->control;arch/x86/kvm/vmx.c +0
-0@@ -1017,9 +1018,14 @@ static int set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) vcpu->guest_debug =0 ;
Quoted 14 lines
arch/x86/kvm/vmx.c static int set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) · line 1031
Quoted 11 lines
arch/x86/kvm/vmx.c +6
-2flags =vmcs_readl (GUEST_RFLAGS ); -if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) +if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) { flags |=X86_EFLAGS_TF |X86_EFLAGS_RF ; -else if (old_debug &KVM_GUESTDBG_SINGLESTEP ) +/* We must be interruptible when single-stepping */ + interruptibility =vmcs_read32 (GUEST_INTERRUPTIBILITY_INFO ); +if (interruptibility &3 ) +vmcs_write32 (GUEST_INTERRUPTIBILITY_INFO , + interruptibility & ~3 );Could just write unconditionally - it's not like the write has any effect on speed.
vmcs_clear_bits()will do it cleanly.
OK.
Quoted 8 lines
But I'm worried about correctness. Suppose we're singlestepping a sti; hlt sequence. While we'll clear interruptibility, probably receive the debug trap (since that's a high priority exception), but then inject the interrupt before the hlt, hanging the guest. So we probably need to restore interruptibility on exit.
This looks like a good candidate for a test case.
You're concern might be valid. Will dig into the test infrastructure. Maybe it is a good idea to have something like the kgdb testsuite as part of the kvm tests to check debugger regressions, not just this special case.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 26 Corporate Competence Center Embedded Linux
PATCH 1/1 KVM: VMX: Allow single-stepping when interruptible
In reply to Avi KivityQuoted 44 lines · Avi Kivity
Quoted 29 lines · Jan Kiszka
When single-stepping, we have to ensure that the INT1 can make it through even if the guest itself is uninterruptible due to MOV SS or STI. VMENTRY will fail otherwise.
Signed-off-by Jan Kiszka <[email protected]>---
arch/x86/kvm/vmx.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)arch/x86/kvm/vmx.c +1
-0@@ -1010,6 +1010,7 @@ static void vmx_cache_reg(struct kvm_vcpu *vcpu, enum kvm_reg reg)static int set_guest_debug (struct kvm_vcpu *vcpu,struct kvm_guest_debug *dbg) {int old_debug = vcpu->guest_debug; +u32 interruptibility;unsigned long flags;vcpu->guest_debug = dbg->control;arch/x86/kvm/vmx.c +0
-0@@ -1017,9 +1018,14 @@ static int set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) vcpu->guest_debug =0 ; flags =vmcs_readl (GUEST_RFLAGS );arch/x86/kvm/vmx.c static int set_guest_debug(struct kvm_vcpu · line 1029
Quoted 10 lines
arch/x86/kvm/vmx.c +6
-2-if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) +if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) { flags |=X86_EFLAGS_TF |X86_EFLAGS_RF ; -else if (old_debug &KVM_GUESTDBG_SINGLESTEP ) +/* We must be interruptible when single-stepping */ + interruptibility =vmcs_read32 (GUEST_INTERRUPTIBILITY_INFO ); +if (interruptibility &3 ) +vmcs_write32 (GUEST_INTERRUPTIBILITY_INFO , + interruptibility & ~3 );Could just write unconditionally - it's not like the write has any effect on speed.
vmcs_clear_bits()will do it cleanly.
Quoted 6 lines
But I'm worried about correctness. Suppose we're singlestepping a sti; hlt sequence. While we'll clear interruptibility, probably receive the debug trap (since that's a high priority exception), but then inject the interrupt before the hlt, hanging the guest. So we probably need to restore interruptibility on exit.
There was some issue with the original patch, but I think I have a safe version now that also works as good as the old one. Please see below, including comments. I'm still open to further concerns or better approaches. Sheng, maybe you can provide some more details on how one is supposed to handle this hairy case with VMX.
Quoted 2 lines
This looks like a good candidate for a test case.
This will be more complicated as I'm currently able to handle: kvmctl would have to be extended to interact with the guest debug interface of kvm, setting appropriate breakpoints and handling the callbacks.
Jan
----------->
When single-stepping over STI and MOV SS, we must clear the corresponding interruptibility bits in the guest state. Otherwise vmentry fails as it then expects bit 14 (BS) in pending debug exceptions being set, but that's not correct for the guest debugging case.
Note that clearing those bits is safe as we check for interruptibility based on the original state and do not inject interrupts or NMIs if guest interruptibility was blocked.
---
arch/x86/kvm/vmx.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
arch/x86/kvm/vmx.c +10 -0
@@ -2477,6 +2477,11 @@ static void do_interrupt_requests(struct kvm_vcpu *vcpu, {vmx_update_window_states (vcpu); +if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) +vmcs_clear_bits (GUEST_INTERRUPTIBILITY_INFO , +GUEST_INTR_STATE_STI | +GUEST_INTR_STATE_MOV_SS ); +if (vcpu->arch.nmi_pending && !vcpu->arch.nmi_injected) {if (vcpu->arch.interrupt.pending) {enable_nmi_window (vcpu);
@@ -3263,6 +3268,11 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)vmx_update_window_states (vcpu); +if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) +vmcs_clear_bits (GUEST_INTERRUPTIBILITY_INFO , +GUEST_INTR_STATE_STI | +GUEST_INTR_STATE_MOV_SS ); +if (vcpu->arch.nmi_pending && !vcpu->arch.nmi_injected) {if (vcpu->arch.interrupt.pending) {enable_nmi_window (vcpu);
PATCH 4/5 KVM: x86: Virtualize debug registers
In reply to Jan KiszkaSo far KVM only had basic x86 debug register support, once introduced to realize guest debugging that way. The guest itself was not able to use those registers.
This patch now adds (almost) full support for guest self-debugging via hardware registers. It refactors the code, moving generic parts out of SVM (VMX was already cleaned up by the KVM_SET_GUEST_DEBUG patches), and it ensures that the registers are properly switched between host and guest.
This patch also prepares debug register usage by the host. The latter will (once wired-up by the following patch) allow for hardware breakpoints/watchpoints in guest code. If this is enabled, the guest will only see faked debug registers without functionality, but with content reflecting the guest's modifications.
Tested on Intel only, but SVM /should/ work as well, but who knows...
Known limitations: Trapping on tss switch won't work - most probably on Intel.
Credits also go to Joerg Roedel - I used his once posted debugging series as platform for this patch.
---
arch/x86/include/asm/kvm_host.h | 22 +++++++ arch/x86/include/asm/vmx.h | 2 - arch/x86/kvm/kvm_svm.h | 6 -- arch/x86/kvm/svm.c | 116 ++++++++++++++------------------------- arch/x86/kvm/vmx.c | 114 +++++++++++++++++++++++++++++++++----- arch/x86/kvm/x86.c | 29 ++++++++++ 6 files changed, 193 insertions(+), 96 deletions(-)
arch/x86/include/asm/kvm_host.h +22 -0
@@ -134,6 +134,19 @@ enum {#define KVM_NR_MEM_OBJS 40 +#define KVM_NR_DB_REGS 4 + +#define DR6_BD (1 << 13) +#define DR6_BS (1 << 14) +#define DR6_FIXED_1 0xffff0ff0 +#define DR6_VOLATILE 0x0000e00f + +#define DR7_BP_EN_MASK 0x000000ff +#define DR7_GE (1 << 9) +#define DR7_GD (1 << 13) +#define DR7_FIXED_1 0x00000400 +#define DR7_VOLATILE 0xffff23ff +/* * We don't want allocation failures within the mmu code, so we preallocate * enough memory for a single page fault in a cache.
@@ -329,6 +342,15 @@ struct kvm_vcpu_arch {struct mtrr_state_type mtrr_state;u32 pat; + +int switch_db_regs; +unsigned long host_db[KVM_NR_DB_REGS ]; +unsigned long host_dr6; +unsigned long host_dr7; +unsigned long db[KVM_NR_DB_REGS ]; +unsigned long dr6; +unsigned long dr7; +unsigned long eff_db[KVM_NR_DB_REGS ]; };struct kvm_mem_alias {
arch/x86/include/asm/vmx.h +1 -1
@@ -312,7 +312,7 @@ enum vmcs_field {#define DEBUG_REG_ACCESS_TYPE 0x10/* 4, direction of access */ #define TYPE_MOV_TO_DR (0 << 4)#define TYPE_MOV_FROM_DR (1 << 4) -#define DEBUG_REG_ACCESS_REG 0xf00/* 11:8, general purpose reg. */ +#define DEBUG_REG_ACCESS_REG (eq) (((eq) >> 8) & 0xf)/* 11:8, general purpose reg. */ /* segment AR */
arch/x86/kvm/kvm_svm.h +0 -6
@@ -18,7 +18,6 @@ static const u32 host_save_user_msrs[] = { };#define NR_HOST_SAVE_USER_MSRS ARRAY_SIZE(host_save_user_msrs) -#define NUM_DB_REGS 4struct kvm_vcpu ;
@@ -29,16 +28,11 @@ struct vcpu_svm {struct svm_cpu_data *svm_data;uint64_t asid_generation; -unsigned long db_regs[NUM_DB_REGS ]; -u64 next_rip;u64 host_user_msrs[NR_HOST_SAVE_USER_MSRS ];u64 host_gs_base;unsigned long host_cr2; -unsigned long host_db_regs[NUM_DB_REGS ]; -unsigned long host_dr6; -unsigned long host_dr7;u32 *msrpm; };
arch/x86/kvm/svm.c +43 -73
188 lines not on this page · raw message
arch/x86/kvm/vmx.c +98 -16
173 lines not on this page · raw message
arch/x86/kvm/x86.c +29 -0
@@ -3013,10 +3013,34 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)kvm_guest_enter (); +get_debugreg (vcpu->arch.host_dr6,6 ); +get_debugreg (vcpu->arch.host_dr7,7 ); +if (unlikely (vcpu->arch.switch_db_regs)) { +get_debugreg (vcpu->arch.host_db[0 ],0 ); +get_debugreg (vcpu->arch.host_db[1 ],1 ); +get_debugreg (vcpu->arch.host_db[2 ],2 ); +get_debugreg (vcpu->arch.host_db[3 ],3 ); + +set_debugreg (0 ,7 ); +set_debugreg (vcpu->arch.eff_db[0 ],0 ); +set_debugreg (vcpu->arch.eff_db[1 ],1 ); +set_debugreg (vcpu->arch.eff_db[2 ],2 ); +set_debugreg (vcpu->arch.eff_db[3 ],3 ); + }KVMTRACE_0D (VMENTRY , vcpu, entryexit); kvm_x86_ops->run (vcpu, kvm_run); +if (unlikely (vcpu->arch.switch_db_regs)) { +set_debugreg (0 ,7 ); +set_debugreg (vcpu->arch.host_db[0 ],0 ); +set_debugreg (vcpu->arch.host_db[1 ],1 ); +set_debugreg (vcpu->arch.host_db[2 ],2 ); +set_debugreg (vcpu->arch.host_db[3 ],3 ); + } +set_debugreg (vcpu->arch.host_dr6,6 ); +set_debugreg (vcpu->arch.host_dr7,7 ); + vcpu->guest_mode =0 ;local_irq_enable ();
@@ -4028,6 +4052,11 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu) vcpu->arch.nmi_pending = false; vcpu->arch.nmi_injected = false; + vcpu->arch.switch_db_regs =0 ; +memset (vcpu->arch.db,0 ,sizeof (vcpu->arch.db)); + vcpu->arch.dr6 =DR6_FIXED_1 ; + vcpu->arch.dr7 =DR7_FIXED_1 ; +return kvm_x86_ops->vcpu_reset (vcpu); }
PATCH 5/5 KVM: x86: Wire-up hardware breakpoints for guest debugging
In reply to Jan KiszkaAdd the remaining bits to make use of debug registers also for guest debugging, thus enabling the use of hardware breakpoints and watchpoints.
---
arch/x86/kvm/svm.c | 5 +++++ arch/x86/kvm/vmx.c | 5 +++++ arch/x86/kvm/x86.c | 14 +++++++++++++- 3 files changed, 23 insertions(+), 1 deletions(-)
arch/x86/kvm/svm.c +5 -0
@@ -895,6 +895,11 @@ static int svm_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) }else vcpu->guest_debug =0 ; +if (vcpu->guest_debug &KVM_GUESTDBG_USE_HW_BP ) + svm->vmcb->save.dr7 = dbg->arch.debugreg[7 ]; +else + svm->vmcb->save.dr7 = vcpu->arch.dr7; +if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) svm->vmcb->save.rflags |=X86_EFLAGS_TF |X86_EFLAGS_RF ;else if (old_debug &KVM_GUESTDBG_SINGLESTEP )
arch/x86/kvm/vmx.c +5 -0
@@ -1017,6 +1017,11 @@ static int set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg)if (!(vcpu->guest_debug &KVM_GUESTDBG_ENABLE )) vcpu->guest_debug =0 ; +if (vcpu->guest_debug &KVM_GUESTDBG_USE_HW_BP ) +vmcs_writel (GUEST_DR7 , dbg->arch.debugreg[7 ]); +else +vmcs_writel (GUEST_DR7 , vcpu->arch.dr7); + flags =vmcs_readl (GUEST_RFLAGS );if (vcpu->guest_debug &KVM_GUESTDBG_SINGLESTEP ) { flags |=X86_EFLAGS_TF |X86_EFLAGS_RF ;
arch/x86/kvm/x86.c +13 -1
@@ -3854,10 +3854,22 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,int kvm_arch_vcpu_ioctl_set_guest_debug (struct kvm_vcpu *vcpu,struct kvm_guest_debug *dbg) { -int r; +int i, r;vcpu_load (vcpu); +if ((dbg->control & (KVM_GUESTDBG_ENABLE |KVM_GUESTDBG_USE_HW_BP )) == + (KVM_GUESTDBG_ENABLE |KVM_GUESTDBG_USE_HW_BP )) { +for (i =0 ; i <KVM_NR_DB_REGS ; ++i) + vcpu->arch.eff_db[i] = dbg->arch.debugreg[i]; + vcpu->arch.switch_db_regs = + (dbg->arch.debugreg[7 ] &DR7_BP_EN_MASK ); + }else { +for (i =0 ; i <KVM_NR_DB_REGS ; i++) + vcpu->arch.eff_db[i] = vcpu->arch.db[i]; + vcpu->arch.switch_db_regs = (vcpu->arch.dr7 &DR7_BP_EN_MASK ); + } + r = kvm_x86_ops->set_guest_debug (vcpu, dbg);if (dbg->control &KVM_GUESTDBG_INJECT_DB )