RE: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, April 02, 2013 11:30 PM To: Bhushan Bharat-R65777 Cc: Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott- B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 04/02/2013 09:09:34 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 1:57 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support How does the normal debug register switching code work in Linux? Can't we just reuse that? Or rely on it to restore working state when another process gets scheduled in? Good point, I can see debug registers loading in function __switch_to()- switch_booke_debug_regs() in file arch/powerpc/kernel/process.c. So as long as assume that host will not use debug resources we can rely on this restore. But I am not sure that this is a fare assumption. As Scott earlier mentioned someone can use debug resource for kernel debugging also. Someone in the kernel can also use floating point registers. But then it's his responsibility to clean up the mess he leaves behind. I am neither convinced by what you said and nor even have much reason to oppose :) Scott, I remember you mentioned that host can use debug resources, you comment on this ? I thought the conclusion we reached was that it was OK as long as KVM waits until it actually needs the debug resources to mess with the registers. Right, Are we also agreeing on that KVM will not save/restore host debug context on vcpu_load/vcpu_put()? KVM will load its context in vcpu_load() if needed and on vcpu_put() it will clear DBCR0 and DBSR. Thanks -Bharat -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
Am 03.04.2013 um 12:03 schrieb Bhushan Bharat-R65777 r65...@freescale.com: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, April 02, 2013 11:30 PM To: Bhushan Bharat-R65777 Cc: Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott- B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 04/02/2013 09:09:34 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 1:57 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support How does the normal debug register switching code work in Linux? Can't we just reuse that? Or rely on it to restore working state when another process gets scheduled in? Good point, I can see debug registers loading in function __switch_to()- switch_booke_debug_regs() in file arch/powerpc/kernel/process.c. So as long as assume that host will not use debug resources we can rely on this restore. But I am not sure that this is a fare assumption. As Scott earlier mentioned someone can use debug resource for kernel debugging also. Someone in the kernel can also use floating point registers. But then it's his responsibility to clean up the mess he leaves behind. I am neither convinced by what you said and nor even have much reason to oppose :) Scott, I remember you mentioned that host can use debug resources, you comment on this ? I thought the conclusion we reached was that it was OK as long as KVM waits until it actually needs the debug resources to mess with the registers. Right, Are we also agreeing on that KVM will not save/restore host debug context on vcpu_load/vcpu_put()? KVM will load its context in vcpu_load() if needed and on vcpu_put() it will clear DBCR0 and DBSR. That depends on whether the kernel restores the debug registers. Please double-check that. Also, someone could want to gdb QEMU, so the debug registers might have to get restored on a heavy weight exit. I'd hope Linux just provides helpers to restore a process's debug state that we can call here. Alex Thanks -Bharat -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
On 03.04.2013, at 15:50, Bhushan Bharat-R65777 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Wednesday, April 03, 2013 3:58 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support Am 03.04.2013 um 12:03 schrieb Bhushan Bharat-R65777 r65...@freescale.com: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, April 02, 2013 11:30 PM To: Bhushan Bharat-R65777 Cc: Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott- B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 04/02/2013 09:09:34 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 1:57 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support How does the normal debug register switching code work in Linux? Can't we just reuse that? Or rely on it to restore working state when another process gets scheduled in? Good point, I can see debug registers loading in function __switch_to()- switch_booke_debug_regs() in file arch/powerpc/kernel/process.c. So as long as assume that host will not use debug resources we can rely on this restore. But I am not sure that this is a fare assumption. As Scott earlier mentioned someone can use debug resource for kernel debugging also. Someone in the kernel can also use floating point registers. But then it's his responsibility to clean up the mess he leaves behind. I am neither convinced by what you said and nor even have much reason to oppose :) Scott, I remember you mentioned that host can use debug resources, you comment on this ? I thought the conclusion we reached was that it was OK as long as KVM waits until it actually needs the debug resources to mess with the registers. Right, Are we also agreeing on that KVM will not save/restore host debug context on vcpu_load/vcpu_put()? KVM will load its context in vcpu_load() if needed and on vcpu_put() it will clear DBCR0 and DBSR. That depends on whether the kernel restores the debug registers. Please double- check that. Currently the kernel code restore the debug state of new schedule process in context_switch(). switch_booke_debug_regs() from __switch_to() and defined as : /* * Unless neither the old or new thread are making use of the * debug registers, set the debug registers from the values * stored in the new thread. */ static void switch_booke_debug_regs(struct thread_struct *new_thread) { if ((current-thread.dbcr0 DBCR0_IDM) || (new_thread-dbcr0 DBCR0_IDM)) prime_debug_regs(new_thread); } static void prime_debug_regs(struct thread_struct *thread) { mtspr(SPRN_IAC1, thread-iac1); mtspr(SPRN_IAC2, thread-iac2); #if CONFIG_PPC_ADV_DEBUG_IACS 2 mtspr(SPRN_IAC3, thread-iac3); mtspr(SPRN_IAC4, thread-iac4); #endif mtspr(SPRN_DAC1, thread-dac1); mtspr(SPRN_DAC2, thread-dac2); #if CONFIG_PPC_ADV_DEBUG_DVCS 0 mtspr(SPRN_DVC1, thread-dvc1); mtspr(SPRN_DVC2, thread-dvc2); #endif mtspr(SPRN_DBCR0, thread-dbcr0); mtspr(SPRN_DBCR1, thread-dbcr1); #ifdef CONFIG_BOOKE mtspr(SPRN_DBCR2, thread-dbcr2); #endif } This is analogous to moving from guest to/from QEMU. so we can make prime_debug_regs() available to kvm code for heavyweight_exit. And vcpu_load() will load guest state and save host state (update thread-debug_registers). I don't think we need to do anything on vcpu_load if we just swap the thread-debug_registers. Just make sure to restore them before you return from a heavy weight exit. And the kernel exception handling code clear DBSR and load DBCR0 with 0 (global_dbcr0 variable, which is zero) in transfer_to_handler in entry_32.S This is analogous to switching from KVM to kernel. But I do not same (clearing DBCR0 and DBSR) in 64bit exception handler. Is this a problem or I am missing something. That one's up for the normal ppc ML to answer :). Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, April 03, 2013 7:39 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 03.04.2013, at 15:50, Bhushan Bharat-R65777 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Wednesday, April 03, 2013 3:58 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support Am 03.04.2013 um 12:03 schrieb Bhushan Bharat-R65777 r65...@freescale.com: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, April 02, 2013 11:30 PM To: Bhushan Bharat-R65777 Cc: Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott- B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 04/02/2013 09:09:34 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 1:57 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support How does the normal debug register switching code work in Linux? Can't we just reuse that? Or rely on it to restore working state when another process gets scheduled in? Good point, I can see debug registers loading in function __switch_to()- switch_booke_debug_regs() in file arch/powerpc/kernel/process.c. So as long as assume that host will not use debug resources we can rely on this restore. But I am not sure that this is a fare assumption. As Scott earlier mentioned someone can use debug resource for kernel debugging also. Someone in the kernel can also use floating point registers. But then it's his responsibility to clean up the mess he leaves behind. I am neither convinced by what you said and nor even have much reason to oppose :) Scott, I remember you mentioned that host can use debug resources, you comment on this ? I thought the conclusion we reached was that it was OK as long as KVM waits until it actually needs the debug resources to mess with the registers. Right, Are we also agreeing on that KVM will not save/restore host debug context on vcpu_load/vcpu_put()? KVM will load its context in vcpu_load() if needed and on vcpu_put() it will clear DBCR0 and DBSR. That depends on whether the kernel restores the debug registers. Please double- check that. Currently the kernel code restore the debug state of new schedule process in context_switch(). switch_booke_debug_regs() from __switch_to() and defined as : /* * Unless neither the old or new thread are making use of the * debug registers, set the debug registers from the values * stored in the new thread. */ static void switch_booke_debug_regs(struct thread_struct *new_thread) { if ((current-thread.dbcr0 DBCR0_IDM) || (new_thread-dbcr0 DBCR0_IDM)) prime_debug_regs(new_thread); } static void prime_debug_regs(struct thread_struct *thread) { mtspr(SPRN_IAC1, thread-iac1); mtspr(SPRN_IAC2, thread-iac2); #if CONFIG_PPC_ADV_DEBUG_IACS 2 mtspr(SPRN_IAC3, thread-iac3); mtspr(SPRN_IAC4, thread-iac4); #endif mtspr(SPRN_DAC1, thread-dac1); mtspr(SPRN_DAC2, thread-dac2); #if CONFIG_PPC_ADV_DEBUG_DVCS 0 mtspr(SPRN_DVC1, thread-dvc1); mtspr(SPRN_DVC2, thread-dvc2); #endif mtspr(SPRN_DBCR0, thread-dbcr0); mtspr(SPRN_DBCR1, thread-dbcr1); #ifdef CONFIG_BOOKE mtspr(SPRN_DBCR2, thread-dbcr2); #endif } This is analogous to moving from guest to/from QEMU. so we can make prime_debug_regs() available to kvm code for heavyweight_exit. And vcpu_load() will load guest state and save host state (update thread-debug_registers). I don't think we need to do anything on vcpu_load if we just swap the thread- debug_registers. Just make sure to restore them before you return from a heavy weight exit. My understanding is : 1) When VCPU is running - h/w debug registers have vcpu-arch.debug_registers Goes for heavyweight_exit - h/w debug registers are loaded with thread-debug_registers Return from
Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
On 03.04.2013, at 17:18, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, April 03, 2013 7:39 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 03.04.2013, at 15:50, Bhushan Bharat-R65777 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Wednesday, April 03, 2013 3:58 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support Am 03.04.2013 um 12:03 schrieb Bhushan Bharat-R65777 r65...@freescale.com: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, April 02, 2013 11:30 PM To: Bhushan Bharat-R65777 Cc: Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott- B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 04/02/2013 09:09:34 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 1:57 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support How does the normal debug register switching code work in Linux? Can't we just reuse that? Or rely on it to restore working state when another process gets scheduled in? Good point, I can see debug registers loading in function __switch_to()- switch_booke_debug_regs() in file arch/powerpc/kernel/process.c. So as long as assume that host will not use debug resources we can rely on this restore. But I am not sure that this is a fare assumption. As Scott earlier mentioned someone can use debug resource for kernel debugging also. Someone in the kernel can also use floating point registers. But then it's his responsibility to clean up the mess he leaves behind. I am neither convinced by what you said and nor even have much reason to oppose :) Scott, I remember you mentioned that host can use debug resources, you comment on this ? I thought the conclusion we reached was that it was OK as long as KVM waits until it actually needs the debug resources to mess with the registers. Right, Are we also agreeing on that KVM will not save/restore host debug context on vcpu_load/vcpu_put()? KVM will load its context in vcpu_load() if needed and on vcpu_put() it will clear DBCR0 and DBSR. That depends on whether the kernel restores the debug registers. Please double- check that. Currently the kernel code restore the debug state of new schedule process in context_switch(). switch_booke_debug_regs() from __switch_to() and defined as : /* * Unless neither the old or new thread are making use of the * debug registers, set the debug registers from the values * stored in the new thread. */ static void switch_booke_debug_regs(struct thread_struct *new_thread) { if ((current-thread.dbcr0 DBCR0_IDM) || (new_thread-dbcr0 DBCR0_IDM)) prime_debug_regs(new_thread); } static void prime_debug_regs(struct thread_struct *thread) { mtspr(SPRN_IAC1, thread-iac1); mtspr(SPRN_IAC2, thread-iac2); #if CONFIG_PPC_ADV_DEBUG_IACS 2 mtspr(SPRN_IAC3, thread-iac3); mtspr(SPRN_IAC4, thread-iac4); #endif mtspr(SPRN_DAC1, thread-dac1); mtspr(SPRN_DAC2, thread-dac2); #if CONFIG_PPC_ADV_DEBUG_DVCS 0 mtspr(SPRN_DVC1, thread-dvc1); mtspr(SPRN_DVC2, thread-dvc2); #endif mtspr(SPRN_DBCR0, thread-dbcr0); mtspr(SPRN_DBCR1, thread-dbcr1); #ifdef CONFIG_BOOKE mtspr(SPRN_DBCR2, thread-dbcr2); #endif } This is analogous to moving from guest to/from QEMU. so we can make prime_debug_regs() available to kvm code for heavyweight_exit. And vcpu_load() will load guest state and save host state (update thread-debug_registers). I don't think we need to do anything on vcpu_load if we just swap the thread- debug_registers. Just make sure to restore them before you return from a heavy weight exit. My understanding is : 1) When VCPU is running - h/w debug registers have vcpu-arch.debug_registers Goes for heavyweight_exit - h/w debug registers are loaded with thread-debug_registers Return from heavyweight_exit and run vcpu - h/w debug registers
RE: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 9:11 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 04/02/2013 04:09 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 1:57 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 21.03.2013, at 07:25, Bharat Bhushan wrote: From: Bharat Bhushanbharat.bhus...@freescale.com This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. Debug registers are saved/restored on vcpu_put()/vcpu_get(). Also the debug registers are saved restored only if guest is using debug resources. Signed-off-by: Bharat Bhushanbharat.bhus...@freescale.com --- v2: - save/restore in vcpu_get()/vcpu_put() - some more minor cleanup based on review comments. arch/powerpc/include/asm/kvm_host.h | 10 ++ arch/powerpc/include/uapi/asm/kvm.h | 22 +++- arch/powerpc/kvm/booke.c| 252 - -- arch/powerpc/kvm/e500_emulate.c | 10 ++ 4 files changed, 272 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index f4ba881..8571952 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -504,7 +504,17 @@ struct kvm_vcpu_arch { u32 mmucfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct kvmppc_booke_debug_reg dbg_reg; + /* shadow debug registers */ + struct kvmppc_booke_debug_reg shadow_dbg_reg; + /* host debug registers*/ + struct kvmppc_booke_debug_reg host_dbg_reg; + /* +* Flag indicating that debug registers are used by guest +* and requires save restore. + */ + bool debug_save_restore; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index 15f9a00..d7ce449 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -25,6 +25,7 @@ /* Select powerpc specific features inlinux/kvm.h */ #define __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; @@ -267,7 +268,24 @@ struct kvm_fpu { __u64 fpr[32]; }; +/* + * Defines for h/w breakpoint, watchpoint (read, write or both) +and + * software breakpoint. + * These are used as type in KVM_SET_GUEST_DEBUG ioctl and status + * for KVM_DEBUG_EXIT. + */ +#define KVMPPC_DEBUG_NONE 0x0 +#define KVMPPC_DEBUG_BREAKPOINT(1UL 1) +#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) +#define KVMPPC_DEBUG_WATCH_READ(1UL 3) struct kvm_debug_exit_arch { + __u64 address; + /* +* exiting to userspace because of h/w breakpoint, watchpoint +* (read, write or both) and software breakpoint. +*/ + __u32 status; + __u32 reserved; }; /* for KVM_SET_GUEST_DEBUG */ @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { * Type denotes h/w breakpoint, read watchpoint, write * watchpoint or watchpoint (both read and write). */ -#define KVMPPC_DEBUG_NOTYPE0x0 -#define KVMPPC_DEBUG_BREAKPOINT(1UL 1) -#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) -#define KVMPPC_DEBUG_WATCH_READ(1UL 3) __u32 type; __u32 reserved; } bp[16]; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1de93a8..bf20056 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { + /* Synchronize guest's desire to get debug interrupts into +shadow MSR */ #ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr= ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr
Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
On 03.04.2013, at 19:14, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 9:11 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 04/02/2013 04:09 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 1:57 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 21.03.2013, at 07:25, Bharat Bhushan wrote: From: Bharat Bhushanbharat.bhus...@freescale.com This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. Debug registers are saved/restored on vcpu_put()/vcpu_get(). Also the debug registers are saved restored only if guest is using debug resources. Signed-off-by: Bharat Bhushanbharat.bhus...@freescale.com --- v2: - save/restore in vcpu_get()/vcpu_put() - some more minor cleanup based on review comments. arch/powerpc/include/asm/kvm_host.h | 10 ++ arch/powerpc/include/uapi/asm/kvm.h | 22 +++- arch/powerpc/kvm/booke.c| 252 - -- arch/powerpc/kvm/e500_emulate.c | 10 ++ 4 files changed, 272 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index f4ba881..8571952 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -504,7 +504,17 @@ struct kvm_vcpu_arch { u32 mmucfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct kvmppc_booke_debug_reg dbg_reg; + /* shadow debug registers */ + struct kvmppc_booke_debug_reg shadow_dbg_reg; + /* host debug registers*/ + struct kvmppc_booke_debug_reg host_dbg_reg; + /* +* Flag indicating that debug registers are used by guest +* and requires save restore. + */ + bool debug_save_restore; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index 15f9a00..d7ce449 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -25,6 +25,7 @@ /* Select powerpc specific features inlinux/kvm.h */ #define __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; @@ -267,7 +268,24 @@ struct kvm_fpu { __u64 fpr[32]; }; +/* + * Defines for h/w breakpoint, watchpoint (read, write or both) +and + * software breakpoint. + * These are used as type in KVM_SET_GUEST_DEBUG ioctl and status + * for KVM_DEBUG_EXIT. + */ +#define KVMPPC_DEBUG_NONE 0x0 +#define KVMPPC_DEBUG_BREAKPOINT(1UL 1) +#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) +#define KVMPPC_DEBUG_WATCH_READ(1UL 3) struct kvm_debug_exit_arch { + __u64 address; + /* +* exiting to userspace because of h/w breakpoint, watchpoint +* (read, write or both) and software breakpoint. +*/ + __u32 status; + __u32 reserved; }; /* for KVM_SET_GUEST_DEBUG */ @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { * Type denotes h/w breakpoint, read watchpoint, write * watchpoint or watchpoint (both read and write). */ -#define KVMPPC_DEBUG_NOTYPE0x0 -#define KVMPPC_DEBUG_BREAKPOINT(1UL 1) -#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) -#define KVMPPC_DEBUG_WATCH_READ(1UL 3) __u32 type; __u32 reserved; } bp[16]; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1de93a8..bf20056 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { + /* Synchronize guest's desire to get debug interrupts into +shadow MSR */ #ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr= ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; #endif + + /* Force enable debug interrupts
RE: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
+ dbg_reg =(vcpu-arch.shadow_dbg_reg); + + /* + * On BOOKE (e500v2); Set DBCR1 and DBCR2 to allow debug events + * to occur when MSR.PR is set. + * On BOOKE-HV (e500mc+); MSR.PR = 0 when guest is running. So we + * should clear DBCR1 and DBCR2. + */ +#ifdef CONFIG_KVM_BOOKE_HV + dbg_reg-dbcr1 = 0; + dbg_reg-dbcr2 = 0; Does that mean we can't debug guest user space? Yes This is wrong. Really, So far I am assuming qemu debug stub is not mean for debugging guest application. Ok, let me rephrase: This is confusing. You do trap in PR mode on e500v2. IIRC x86 also traps in kernel and user space. I don't see why e500 hv should be different. I am sorry, I think did not read the document correctly. DBCR1 = 0 ; means the 00 IAC1 debug conditions unaffected by MSR[PR],MSR[GS]. Similarly for dbcr2. So yes the guest user space can be debugged. So why is this conditional on BOOKE_HV then? Wouldn't it make things easier to treat HV and PR identical? On BOOKE-HV we have to keep these to 0, so guest and guest application both can be debugged. Also on HV we have EPCR.DUVD to control that debug events will not come in hypervisor (GS = 0). On BOOKE; guest and guest application both runs in PR = 1 and hypervisor in PR = 0. So with dbcr1/dbcr2 on booke we control debug exception not to come in hypervisor mode still allow guest and its application debugging. Thanks -Bharat -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
Am 03.04.2013 um 19:47 schrieb Bhushan Bharat-R65777 r65...@freescale.com: +dbg_reg =(vcpu-arch.shadow_dbg_reg); + +/* + * On BOOKE (e500v2); Set DBCR1 and DBCR2 to allow debug events + * to occur when MSR.PR is set. + * On BOOKE-HV (e500mc+); MSR.PR = 0 when guest is running. So we + * should clear DBCR1 and DBCR2. + */ +#ifdef CONFIG_KVM_BOOKE_HV +dbg_reg-dbcr1 = 0; +dbg_reg-dbcr2 = 0; Does that mean we can't debug guest user space? Yes This is wrong. Really, So far I am assuming qemu debug stub is not mean for debugging guest application. Ok, let me rephrase: This is confusing. You do trap in PR mode on e500v2. IIRC x86 also traps in kernel and user space. I don't see why e500 hv should be different. I am sorry, I think did not read the document correctly. DBCR1 = 0 ; means the 00 IAC1 debug conditions unaffected by MSR[PR],MSR[GS]. Similarly for dbcr2. So yes the guest user space can be debugged. So why is this conditional on BOOKE_HV then? Wouldn't it make things easier to treat HV and PR identical? On BOOKE-HV we have to keep these to 0, so guest and guest application both can be debugged. Also on HV we have EPCR.DUVD to control that debug events will not come in hypervisor (GS = 0). On BOOKE; guest and guest application both runs in PR = 1 and hypervisor in PR = 0. So with dbcr1/dbcr2 on booke we control debug exception not to come in hypervisor mode still allow guest and its application debugging. Ah, can we group these 2 overrides next to each other with an #ifdef ... #else to make this obvious from the code? Alex Thanks -Bharat -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 21.03.2013, at 07:25, Bharat Bhushan wrote: From: Bharat Bhushan bharat.bhus...@freescale.com This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. Debug registers are saved/restored on vcpu_put()/vcpu_get(). Also the debug registers are saved restored only if guest is using debug resources. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- v2: - save/restore in vcpu_get()/vcpu_put() - some more minor cleanup based on review comments. arch/powerpc/include/asm/kvm_host.h | 10 ++ arch/powerpc/include/uapi/asm/kvm.h | 22 +++- arch/powerpc/kvm/booke.c| 252 --- arch/powerpc/kvm/e500_emulate.c | 10 ++ 4 files changed, 272 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index f4ba881..8571952 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -504,7 +504,17 @@ struct kvm_vcpu_arch { u32 mmucfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct kvmppc_booke_debug_reg dbg_reg; + /* shadow debug registers */ + struct kvmppc_booke_debug_reg shadow_dbg_reg; + /* host debug registers*/ + struct kvmppc_booke_debug_reg host_dbg_reg; + /* +* Flag indicating that debug registers are used by guest +* and requires save restore. + */ + bool debug_save_restore; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index 15f9a00..d7ce449 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -25,6 +25,7 @@ /* Select powerpc specific features in linux/kvm.h */ #define __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; @@ -267,7 +268,24 @@ struct kvm_fpu { __u64 fpr[32]; }; +/* + * Defines for h/w breakpoint, watchpoint (read, write or both) and + * software breakpoint. + * These are used as type in KVM_SET_GUEST_DEBUG ioctl and status + * for KVM_DEBUG_EXIT. + */ +#define KVMPPC_DEBUG_NONE 0x0 +#define KVMPPC_DEBUG_BREAKPOINT(1UL 1) +#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) +#define KVMPPC_DEBUG_WATCH_READ(1UL 3) struct kvm_debug_exit_arch { + __u64 address; + /* +* exiting to userspace because of h/w breakpoint, watchpoint +* (read, write or both) and software breakpoint. +*/ + __u32 status; + __u32 reserved; }; /* for KVM_SET_GUEST_DEBUG */ @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { * Type denotes h/w breakpoint, read watchpoint, write * watchpoint or watchpoint (both read and write). */ -#define KVMPPC_DEBUG_NOTYPE0x0 -#define KVMPPC_DEBUG_BREAKPOINT(1UL 1) -#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) -#define KVMPPC_DEBUG_WATCH_READ(1UL 3) __u32 type; __u32 reserved; } bp[16]; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1de93a8..bf20056 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { + /* Synchronize guest's desire to get debug interrupts into shadow +MSR */ #ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr = ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; #endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu-guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* +* Since there is no shadow MSR, sync MSR_DE into the guest +* visible MSR. Do not allow guest to change MSR[DE]. +*/ + vcpu-arch.shared-msr |= MSR_DE; + mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP); This mtspr should really just be a bit or in shadow_mspr when guest_debug gets enabled. It should automatically get synchronized as soon as the next vpcu_load() happens. I think this is not required here as shadow_dbsr already have MSRP_DEP set. Will setup shadow_msrp when setting guest_debug and clear shadow_msrp when guest_debug is cleared. But that will also not be sufficient as it not sure when vcpu_load() will be called after the shadow_msrp is changed. So
RE: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 1:57 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 21.03.2013, at 07:25, Bharat Bhushan wrote: From: Bharat Bhushan bharat.bhus...@freescale.com This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. Debug registers are saved/restored on vcpu_put()/vcpu_get(). Also the debug registers are saved restored only if guest is using debug resources. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- v2: - save/restore in vcpu_get()/vcpu_put() - some more minor cleanup based on review comments. arch/powerpc/include/asm/kvm_host.h | 10 ++ arch/powerpc/include/uapi/asm/kvm.h | 22 +++- arch/powerpc/kvm/booke.c| 252 - -- arch/powerpc/kvm/e500_emulate.c | 10 ++ 4 files changed, 272 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index f4ba881..8571952 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -504,7 +504,17 @@ struct kvm_vcpu_arch { u32 mmucfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct kvmppc_booke_debug_reg dbg_reg; + /* shadow debug registers */ + struct kvmppc_booke_debug_reg shadow_dbg_reg; + /* host debug registers*/ + struct kvmppc_booke_debug_reg host_dbg_reg; + /* + * Flag indicating that debug registers are used by guest + * and requires save restore. + */ + bool debug_save_restore; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index 15f9a00..d7ce449 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -25,6 +25,7 @@ /* Select powerpc specific features in linux/kvm.h */ #define __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; @@ -267,7 +268,24 @@ struct kvm_fpu { __u64 fpr[32]; }; +/* + * Defines for h/w breakpoint, watchpoint (read, write or both) and + * software breakpoint. + * These are used as type in KVM_SET_GUEST_DEBUG ioctl and status + * for KVM_DEBUG_EXIT. + */ +#define KVMPPC_DEBUG_NONE0x0 +#define KVMPPC_DEBUG_BREAKPOINT (1UL 1) +#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) +#define KVMPPC_DEBUG_WATCH_READ (1UL 3) struct kvm_debug_exit_arch { + __u64 address; + /* + * exiting to userspace because of h/w breakpoint, watchpoint + * (read, write or both) and software breakpoint. + */ + __u32 status; + __u32 reserved; }; /* for KVM_SET_GUEST_DEBUG */ @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { * Type denotes h/w breakpoint, read watchpoint, write * watchpoint or watchpoint (both read and write). */ -#define KVMPPC_DEBUG_NOTYPE 0x0 -#define KVMPPC_DEBUG_BREAKPOINT (1UL 1) -#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) -#define KVMPPC_DEBUG_WATCH_READ (1UL 3) __u32 type; __u32 reserved; } bp[16]; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1de93a8..bf20056 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { + /* Synchronize guest's desire to get debug interrupts into shadow +MSR */ #ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr = ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; #endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu-guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* + * Since there is no shadow MSR, sync MSR_DE into the guest + * visible MSR. Do not allow guest to change MSR[DE]. + */ + vcpu-arch.shared-msr |= MSR_DE; + mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP); This mtspr should really just be a bit or in shadow_mspr when guest_debug gets enabled. It should automatically get synchronized as soon as the next
Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
On 04/02/2013 04:09 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 1:57 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 21.03.2013, at 07:25, Bharat Bhushan wrote: From: Bharat Bhushanbharat.bhus...@freescale.com This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. Debug registers are saved/restored on vcpu_put()/vcpu_get(). Also the debug registers are saved restored only if guest is using debug resources. Signed-off-by: Bharat Bhushanbharat.bhus...@freescale.com --- v2: - save/restore in vcpu_get()/vcpu_put() - some more minor cleanup based on review comments. arch/powerpc/include/asm/kvm_host.h | 10 ++ arch/powerpc/include/uapi/asm/kvm.h | 22 +++- arch/powerpc/kvm/booke.c| 252 - -- arch/powerpc/kvm/e500_emulate.c | 10 ++ 4 files changed, 272 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index f4ba881..8571952 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -504,7 +504,17 @@ struct kvm_vcpu_arch { u32 mmucfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct kvmppc_booke_debug_reg dbg_reg; + /* shadow debug registers */ + struct kvmppc_booke_debug_reg shadow_dbg_reg; + /* host debug registers*/ + struct kvmppc_booke_debug_reg host_dbg_reg; + /* +* Flag indicating that debug registers are used by guest +* and requires save restore. + */ + bool debug_save_restore; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index 15f9a00..d7ce449 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -25,6 +25,7 @@ /* Select powerpc specific features inlinux/kvm.h */ #define __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; @@ -267,7 +268,24 @@ struct kvm_fpu { __u64 fpr[32]; }; +/* + * Defines for h/w breakpoint, watchpoint (read, write or both) and + * software breakpoint. + * These are used as type in KVM_SET_GUEST_DEBUG ioctl and status + * for KVM_DEBUG_EXIT. + */ +#define KVMPPC_DEBUG_NONE 0x0 +#define KVMPPC_DEBUG_BREAKPOINT(1UL 1) +#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) +#define KVMPPC_DEBUG_WATCH_READ(1UL 3) struct kvm_debug_exit_arch { + __u64 address; + /* +* exiting to userspace because of h/w breakpoint, watchpoint +* (read, write or both) and software breakpoint. +*/ + __u32 status; + __u32 reserved; }; /* for KVM_SET_GUEST_DEBUG */ @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { * Type denotes h/w breakpoint, read watchpoint, write * watchpoint or watchpoint (both read and write). */ -#define KVMPPC_DEBUG_NOTYPE0x0 -#define KVMPPC_DEBUG_BREAKPOINT(1UL 1) -#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) -#define KVMPPC_DEBUG_WATCH_READ(1UL 3) __u32 type; __u32 reserved; } bp[16]; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1de93a8..bf20056 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { + /* Synchronize guest's desire to get debug interrupts into shadow +MSR */ #ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr= ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; #endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu-guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* +* Since there is no shadow MSR, sync MSR_DE into the guest +* visible MSR. Do not allow guest to change MSR[DE]. +*/ + vcpu-arch.shared-msr |= MSR_DE; + mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP); This mtspr should really just be a bit or in shadow_mspr when guest_debug gets
Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
On 04/02/2013 09:09:34 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, April 02, 2013 1:57 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support How does the normal debug register switching code work in Linux? Can't we just reuse that? Or rely on it to restore working state when another process gets scheduled in? Good point, I can see debug registers loading in function __switch_to()- switch_booke_debug_regs() in file arch/powerpc/kernel/process.c. So as long as assume that host will not use debug resources we can rely on this restore. But I am not sure that this is a fare assumption. As Scott earlier mentioned someone can use debug resource for kernel debugging also. Someone in the kernel can also use floating point registers. But then it's his responsibility to clean up the mess he leaves behind. I am neither convinced by what you said and nor even have much reason to oppose :) Scott, I remember you mentioned that host can use debug resources, you comment on this ? I thought the conclusion we reached was that it was OK as long as KVM waits until it actually needs the debug resources to mess with the registers. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 28, 2013 10:06 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support On 21.03.2013, at 07:25, Bharat Bhushan wrote: From: Bharat Bhushan bharat.bhus...@freescale.com This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. Debug registers are saved/restored on vcpu_put()/vcpu_get(). Also the debug registers are saved restored only if guest is using debug resources. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- v2: - save/restore in vcpu_get()/vcpu_put() - some more minor cleanup based on review comments. arch/powerpc/include/asm/kvm_host.h | 10 ++ arch/powerpc/include/uapi/asm/kvm.h | 22 +++- arch/powerpc/kvm/booke.c| 252 --- arch/powerpc/kvm/e500_emulate.c | 10 ++ 4 files changed, 272 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index f4ba881..8571952 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -504,7 +504,17 @@ struct kvm_vcpu_arch { u32 mmucfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct kvmppc_booke_debug_reg dbg_reg; + /* shadow debug registers */ + struct kvmppc_booke_debug_reg shadow_dbg_reg; + /* host debug registers*/ + struct kvmppc_booke_debug_reg host_dbg_reg; + /* +* Flag indicating that debug registers are used by guest +* and requires save restore. + */ + bool debug_save_restore; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index 15f9a00..d7ce449 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -25,6 +25,7 @@ /* Select powerpc specific features in linux/kvm.h */ #define __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; @@ -267,7 +268,24 @@ struct kvm_fpu { __u64 fpr[32]; }; +/* + * Defines for h/w breakpoint, watchpoint (read, write or both) and + * software breakpoint. + * These are used as type in KVM_SET_GUEST_DEBUG ioctl and status + * for KVM_DEBUG_EXIT. + */ +#define KVMPPC_DEBUG_NONE 0x0 +#define KVMPPC_DEBUG_BREAKPOINT(1UL 1) +#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) +#define KVMPPC_DEBUG_WATCH_READ(1UL 3) struct kvm_debug_exit_arch { + __u64 address; + /* +* exiting to userspace because of h/w breakpoint, watchpoint +* (read, write or both) and software breakpoint. +*/ + __u32 status; + __u32 reserved; }; /* for KVM_SET_GUEST_DEBUG */ @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { * Type denotes h/w breakpoint, read watchpoint, write * watchpoint or watchpoint (both read and write). */ -#define KVMPPC_DEBUG_NOTYPE0x0 -#define KVMPPC_DEBUG_BREAKPOINT(1UL 1) -#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) -#define KVMPPC_DEBUG_WATCH_READ(1UL 3) __u32 type; __u32 reserved; } bp[16]; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1de93a8..bf20056 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { + /* Synchronize guest's desire to get debug interrupts into shadow +MSR */ #ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr = ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; #endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu-guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* +* Since there is no shadow MSR, sync MSR_DE into the guest +* visible MSR. Do not allow guest to change MSR[DE]. +*/ + vcpu-arch.shared-msr |= MSR_DE; + mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP); This mtspr should really just be a bit or in shadow_mspr when guest_debug gets enabled. It should automatically get synchronized as soon as the next vpcu_load() happens. I think this is not required here as shadow_dbsr already have MSRP_DEP set. Will setup shadow_msrp when setting guest_debug and clear shadow_msrp when guest_debug is cleared. But that will also not be sufficient as it not sure when vcpu_load
Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support
On 21.03.2013, at 07:25, Bharat Bhushan wrote: From: Bharat Bhushan bharat.bhus...@freescale.com This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. Debug registers are saved/restored on vcpu_put()/vcpu_get(). Also the debug registers are saved restored only if guest is using debug resources. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- v2: - save/restore in vcpu_get()/vcpu_put() - some more minor cleanup based on review comments. arch/powerpc/include/asm/kvm_host.h | 10 ++ arch/powerpc/include/uapi/asm/kvm.h | 22 +++- arch/powerpc/kvm/booke.c| 252 --- arch/powerpc/kvm/e500_emulate.c | 10 ++ 4 files changed, 272 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index f4ba881..8571952 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -504,7 +504,17 @@ struct kvm_vcpu_arch { u32 mmucfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct kvmppc_booke_debug_reg dbg_reg; + /* shadow debug registers */ + struct kvmppc_booke_debug_reg shadow_dbg_reg; + /* host debug registers*/ + struct kvmppc_booke_debug_reg host_dbg_reg; + /* + * Flag indicating that debug registers are used by guest + * and requires save restore. + */ + bool debug_save_restore; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index 15f9a00..d7ce449 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -25,6 +25,7 @@ /* Select powerpc specific features in linux/kvm.h */ #define __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; @@ -267,7 +268,24 @@ struct kvm_fpu { __u64 fpr[32]; }; +/* + * Defines for h/w breakpoint, watchpoint (read, write or both) and + * software breakpoint. + * These are used as type in KVM_SET_GUEST_DEBUG ioctl and status + * for KVM_DEBUG_EXIT. + */ +#define KVMPPC_DEBUG_NONE0x0 +#define KVMPPC_DEBUG_BREAKPOINT (1UL 1) +#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) +#define KVMPPC_DEBUG_WATCH_READ (1UL 3) struct kvm_debug_exit_arch { + __u64 address; + /* + * exiting to userspace because of h/w breakpoint, watchpoint + * (read, write or both) and software breakpoint. + */ + __u32 status; + __u32 reserved; }; /* for KVM_SET_GUEST_DEBUG */ @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { * Type denotes h/w breakpoint, read watchpoint, write * watchpoint or watchpoint (both read and write). */ -#define KVMPPC_DEBUG_NOTYPE 0x0 -#define KVMPPC_DEBUG_BREAKPOINT (1UL 1) -#define KVMPPC_DEBUG_WATCH_WRITE (1UL 2) -#define KVMPPC_DEBUG_WATCH_READ (1UL 3) __u32 type; __u32 reserved; } bp[16]; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1de93a8..bf20056 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) +{ + /* Synchronize guest's desire to get debug interrupts into shadow MSR */ +#ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr = ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; +#endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu-guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* + * Since there is no shadow MSR, sync MSR_DE into the guest + * visible MSR. Do not allow guest to change MSR[DE]. + */ + vcpu-arch.shared-msr |= MSR_DE; + mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP); This mtspr should really just be a bit or in shadow_mspr when guest_debug gets enabled. It should automatically get synchronized as soon as the next vpcu_load() happens. Also, what happens when user space disables guest_debug? +#else + vcpu-arch.shadow_msr |= MSR_DE; + vcpu-arch.shared-msr = ~MSR_DE; +#endif + } +} + /* * Helper function for full MSR writes. No need to call this if only * EE/CE/ME/DE/RI are changing. @@ -150,6 +174,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) kvmppc_mmu_msr_notify(vcpu, old_msr); kvmppc_vcpu_sync_spe(vcpu); kvmppc_vcpu_sync_fpu(vcpu); + kvmppc_vcpu_sync_debug(vcpu); } static void kvmppc_booke_queue_irqprio(struct kvm_vcpu