* [PATCH 0/2 v5] powerpc/kvm: support to handle sw breakpoint @ 2014-09-07 16:31 Madhavan Srinivasan 2014-09-07 16:31 ` [PATCH 1/2 " Madhavan Srinivasan 2014-09-07 16:31 ` [PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc Madhavan Srinivasan 0 siblings, 2 replies; 7+ messages in thread From: Madhavan Srinivasan @ 2014-09-07 16:31 UTC (permalink / raw) To: agraf, benh, paulus, mpe; +Cc: Madhavan Srinivasan, linuxppc-dev, kvm-ppc, kvm This patchset adds ppc64 server side support for software breakpoint and extends the use of illegal instruction as software breakpoint across ppc platform. Patch 1, adds kernel side support for software breakpoint. Design is that, by using an illegal instruction, we trap to hypervisor via Emulation Assistance interrupt, where we check for the illegal instruction and accordingly we return to Host or Guest. Patch also adds support for software breakpoint in PR KVM. Patch 2,extends the use of illegal instruction as software breakpoint instruction across the ppc platform. Patch extends booke program interrupt code to support software breakpoint. Patch 2 is only compile tested. Will really help if someone can try it out and let me know comments. Changes v4->v5: Made changes to code comments and commit messages Added debugging active checks for illegal instr comparison Added debug instruction check in emulate code Extended SW breakpoint to booke Changes v3->v4: Made changes to code comments and removed #define of zero opcode Added a new function to handle the debug instruction emulation in book3s_hv Rebased the code to latest upstream source. Changes v2->v3: Changed the debug instructions. Using the all zero opcode in the instruction word as illegal instruction as mentioned in Power ISA instead of ABS Removed reg updated in emulation assist and added a call to kvmppc_emulate_instruction for reg update. Changes v1->v2: Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also share it. Added code to use KVM get one reg infrastructure to get debug opcode. Updated emulate.c to include emulation of debug instruction incase of PR_KVM. Made changes to commit message. Madhavan Srinivasan (2): powerpc/kvm: support to handle sw breakpoint powerpc/kvm: common sw breakpoint instr across ppc arch/powerpc/include/asm/kvm_ppc.h | 6 ++++++ arch/powerpc/kvm/book3s.c | 3 ++- arch/powerpc/kvm/book3s_hv.c | 41 ++++++++++++++++++++++++++++++++++---- arch/powerpc/kvm/book3s_pr.c | 3 +++ arch/powerpc/kvm/booke.c | 18 +++++++++++++++-- arch/powerpc/kvm/emulate.c | 18 +++++++++++++++++ 6 files changed, 82 insertions(+), 7 deletions(-) -- 1.7.11.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2 v5] powerpc/kvm: support to handle sw breakpoint 2014-09-07 16:31 [PATCH 0/2 v5] powerpc/kvm: support to handle sw breakpoint Madhavan Srinivasan @ 2014-09-07 16:31 ` Madhavan Srinivasan 2014-09-08 13:05 ` Alexander Graf 2014-09-07 16:31 ` [PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc Madhavan Srinivasan 1 sibling, 1 reply; 7+ messages in thread From: Madhavan Srinivasan @ 2014-09-07 16:31 UTC (permalink / raw) To: agraf, benh, paulus, mpe; +Cc: Madhavan Srinivasan, linuxppc-dev, kvm-ppc, kvm This patch adds kernel side support for software breakpoint. Design is that, by using an illegal instruction, we trap to hypervisor via Emulation Assistance interrupt, where we check for the illegal instruction and accordingly we return to Host or Guest. Patch also adds support for software breakpoint in PR KVM. Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> --- arch/powerpc/include/asm/kvm_ppc.h | 6 ++++++ arch/powerpc/kvm/book3s.c | 3 ++- arch/powerpc/kvm/book3s_hv.c | 41 ++++++++++++++++++++++++++++++++++---- arch/powerpc/kvm/book3s_pr.c | 3 +++ arch/powerpc/kvm/emulate.c | 18 +++++++++++++++++ 5 files changed, 66 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index fb86a22..dd83c9a 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -38,6 +38,12 @@ #include <asm/paca.h> #endif +/* + * KVMPPC_INST_SW_BREAKPOINT is debug Instruction + * for supporting software breakpoint. + */ +#define KVMPPC_INST_SW_BREAKPOINT 0x00dddd00 + enum emulation_result { EMULATE_DONE, /* no further processing */ EMULATE_DO_MMIO, /* kvm_run filled with MMIO request */ diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c index dd03f6b..00e9c9f 100644 --- a/arch/powerpc/kvm/book3s.c +++ b/arch/powerpc/kvm/book3s.c @@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu, int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) { - return -EINVAL; + vcpu->guest_debug = dbg->control; + return 0; } void kvmppc_decrementer_func(unsigned long data) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index 27cced9..3a2414c 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd) return kvmppc_hcall_impl_hv_realmode(cmd); } +static int kvmppc_emulate_debug_inst(struct kvm_run *run, + struct kvm_vcpu *vcpu) +{ + u32 last_inst; + + if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) != + EMULATE_DONE) { + /* + * Fetch failed, so return to guest and + * try executing it again. + */ + return RESUME_GUEST; + } else { + if (last_inst == KVMPPC_INST_SW_BREAKPOINT) { + run->exit_reason = KVM_EXIT_DEBUG; + run->debug.arch.address = kvmppc_get_pc(vcpu); + return RESUME_HOST; + } else { + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); + return RESUME_GUEST; + } + } +} + static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, struct task_struct *tsk) { @@ -807,12 +831,18 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, break; /* * This occurs if the guest executes an illegal instruction. - * We just generate a program interrupt to the guest, since - * we don't emulate any guest instructions at this stage. + * If the guest debug is disabled, generate a program interrupt + * to the guest. If guest debug is enabled, we need to check + * whether the instruction is a software breakpoint instruction. + * Accordingly return to Guest or Host. */ case BOOK3S_INTERRUPT_H_EMUL_ASSIST: - kvmppc_core_queue_program(vcpu, SRR1_PROGILL); - r = RESUME_GUEST; + if(vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) { + r = kvmppc_emulate_debug_inst(run, vcpu); + } else { + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); + r = RESUME_GUEST; + } break; /* * This occurs if the guest (kernel or userspace), does something that @@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id, long int i; switch (id) { + case KVM_REG_PPC_DEBUG_INST: + *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT); + break; case KVM_REG_PPC_HIOR: *val = get_reg_val(id, 0); break; diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c index faffb27..6d73708 100644 --- a/arch/powerpc/kvm/book3s_pr.c +++ b/arch/powerpc/kvm/book3s_pr.c @@ -1319,6 +1319,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, u64 id, int r = 0; switch (id) { + case KVM_REG_PPC_DEBUG_INST: + *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT); + break; case KVM_REG_PPC_HIOR: *val = get_reg_val(id, to_book3s(vcpu)->hior); break; diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c index e96b50d..30f326b 100644 --- a/arch/powerpc/kvm/emulate.c +++ b/arch/powerpc/kvm/emulate.c @@ -274,6 +274,24 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu) } break; + case 0: + /* + * Instruction with primary opcode 0. Based on PowerISA + * these are illegal instructions. + */ + if(inst == KVMPPC_INST_SW_BREAKPOINT) { + run->exit_reason = KVM_EXIT_DEBUG; + run->debug.arch.address = kvmppc_get_pc(vcpu); + emulated = EMULATE_EXIT_USER; + advance = 0; + printk(KERN_INFO "pr_emulate: debug instr \n"); + } else { + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); + emulated = EMULATE_DONE; + advance = 0; + } + break; + default: emulated = EMULATE_FAIL; } -- 1.7.11.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2 v5] powerpc/kvm: support to handle sw breakpoint 2014-09-07 16:31 ` [PATCH 1/2 " Madhavan Srinivasan @ 2014-09-08 13:05 ` Alexander Graf 2014-09-09 7:31 ` Madhavan Srinivasan 0 siblings, 1 reply; 7+ messages in thread From: Alexander Graf @ 2014-09-08 13:05 UTC (permalink / raw) To: Madhavan Srinivasan, benh, paulus, mpe; +Cc: linuxppc-dev, kvm-ppc, kvm On 07.09.14 18:31, Madhavan Srinivasan wrote: > This patch adds kernel side support for software breakpoint. > Design is that, by using an illegal instruction, we trap to hypervisor > via Emulation Assistance interrupt, where we check for the illegal instruction > and accordingly we return to Host or Guest. Patch also adds support for > software breakpoint in PR KVM. > > Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> > --- > arch/powerpc/include/asm/kvm_ppc.h | 6 ++++++ > arch/powerpc/kvm/book3s.c | 3 ++- > arch/powerpc/kvm/book3s_hv.c | 41 ++++++++++++++++++++++++++++++++++---- > arch/powerpc/kvm/book3s_pr.c | 3 +++ > arch/powerpc/kvm/emulate.c | 18 +++++++++++++++++ > 5 files changed, 66 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h > index fb86a22..dd83c9a 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -38,6 +38,12 @@ > #include <asm/paca.h> > #endif > > +/* > + * KVMPPC_INST_SW_BREAKPOINT is debug Instruction > + * for supporting software breakpoint. > + */ > +#define KVMPPC_INST_SW_BREAKPOINT 0x00dddd00 > + > enum emulation_result { > EMULATE_DONE, /* no further processing */ > EMULATE_DO_MMIO, /* kvm_run filled with MMIO request */ > diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c > index dd03f6b..00e9c9f 100644 > --- a/arch/powerpc/kvm/book3s.c > +++ b/arch/powerpc/kvm/book3s.c > @@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu, > int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, > struct kvm_guest_debug *dbg) > { > - return -EINVAL; > + vcpu->guest_debug = dbg->control; > + return 0; > } > > void kvmppc_decrementer_func(unsigned long data) > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index 27cced9..3a2414c 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd) > return kvmppc_hcall_impl_hv_realmode(cmd); > } > > +static int kvmppc_emulate_debug_inst(struct kvm_run *run, > + struct kvm_vcpu *vcpu) > +{ > + u32 last_inst; > + > + if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) != > + EMULATE_DONE) { > + /* > + * Fetch failed, so return to guest and > + * try executing it again. > + */ > + return RESUME_GUEST; Please end the if() here. In the kernel we usually treat abort situations as separate. > + } else { > + if (last_inst == KVMPPC_INST_SW_BREAKPOINT) { > + run->exit_reason = KVM_EXIT_DEBUG; > + run->debug.arch.address = kvmppc_get_pc(vcpu); > + return RESUME_HOST; > + } else { > + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > + return RESUME_GUEST; > + } > + } > +} > + > static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, > struct task_struct *tsk) > { > @@ -807,12 +831,18 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, > break; > /* > * This occurs if the guest executes an illegal instruction. > - * We just generate a program interrupt to the guest, since > - * we don't emulate any guest instructions at this stage. > + * If the guest debug is disabled, generate a program interrupt > + * to the guest. If guest debug is enabled, we need to check > + * whether the instruction is a software breakpoint instruction. > + * Accordingly return to Guest or Host. > */ > case BOOK3S_INTERRUPT_H_EMUL_ASSIST: > - kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > - r = RESUME_GUEST; > + if(vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) { > + r = kvmppc_emulate_debug_inst(run, vcpu); > + } else { > + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > + r = RESUME_GUEST; > + } > break; > /* > * This occurs if the guest (kernel or userspace), does something that > @@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id, > long int i; > > switch (id) { > + case KVM_REG_PPC_DEBUG_INST: > + *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT); > + break; > case KVM_REG_PPC_HIOR: > *val = get_reg_val(id, 0); > break; > diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c > index faffb27..6d73708 100644 > --- a/arch/powerpc/kvm/book3s_pr.c > +++ b/arch/powerpc/kvm/book3s_pr.c > @@ -1319,6 +1319,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, u64 id, > int r = 0; > > switch (id) { > + case KVM_REG_PPC_DEBUG_INST: > + *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT); > + break; > case KVM_REG_PPC_HIOR: > *val = get_reg_val(id, to_book3s(vcpu)->hior); > break; > diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c > index e96b50d..30f326b 100644 > --- a/arch/powerpc/kvm/emulate.c > +++ b/arch/powerpc/kvm/emulate.c > @@ -274,6 +274,24 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu) > } > break; > > + case 0: > + /* > + * Instruction with primary opcode 0. Based on PowerISA > + * these are illegal instructions. > + */ > + if(inst == KVMPPC_INST_SW_BREAKPOINT) { Here you're missing a space between if and (. Are you sure that you ran checkpatch.pl? > + run->exit_reason = KVM_EXIT_DEBUG; > + run->debug.arch.address = kvmppc_get_pc(vcpu); > + emulated = EMULATE_EXIT_USER; > + advance = 0; > + printk(KERN_INFO "pr_emulate: debug instr \n"); > + } else { > + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > + emulated = EMULATE_DONE; > + advance = 0; Just set emulated = EMULATE_FAIL here, the same as default:. Alex > + } > + break; > + > default: > emulated = EMULATE_FAIL; > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2 v5] powerpc/kvm: support to handle sw breakpoint 2014-09-08 13:05 ` Alexander Graf @ 2014-09-09 7:31 ` Madhavan Srinivasan 0 siblings, 0 replies; 7+ messages in thread From: Madhavan Srinivasan @ 2014-09-09 7:31 UTC (permalink / raw) To: Alexander Graf, benh, paulus, mpe; +Cc: linuxppc-dev, kvm-ppc, kvm On Monday 08 September 2014 06:35 PM, Alexander Graf wrote: > > > On 07.09.14 18:31, Madhavan Srinivasan wrote: >> This patch adds kernel side support for software breakpoint. >> Design is that, by using an illegal instruction, we trap to hypervisor >> via Emulation Assistance interrupt, where we check for the illegal instruction >> and accordingly we return to Host or Guest. Patch also adds support for >> software breakpoint in PR KVM. >> >> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> >> --- >> arch/powerpc/include/asm/kvm_ppc.h | 6 ++++++ >> arch/powerpc/kvm/book3s.c | 3 ++- >> arch/powerpc/kvm/book3s_hv.c | 41 ++++++++++++++++++++++++++++++++++---- >> arch/powerpc/kvm/book3s_pr.c | 3 +++ >> arch/powerpc/kvm/emulate.c | 18 +++++++++++++++++ >> 5 files changed, 66 insertions(+), 5 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h >> index fb86a22..dd83c9a 100644 >> --- a/arch/powerpc/include/asm/kvm_ppc.h >> +++ b/arch/powerpc/include/asm/kvm_ppc.h >> @@ -38,6 +38,12 @@ >> #include <asm/paca.h> >> #endif >> >> +/* >> + * KVMPPC_INST_SW_BREAKPOINT is debug Instruction >> + * for supporting software breakpoint. >> + */ >> +#define KVMPPC_INST_SW_BREAKPOINT 0x00dddd00 >> + >> enum emulation_result { >> EMULATE_DONE, /* no further processing */ >> EMULATE_DO_MMIO, /* kvm_run filled with MMIO request */ >> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c >> index dd03f6b..00e9c9f 100644 >> --- a/arch/powerpc/kvm/book3s.c >> +++ b/arch/powerpc/kvm/book3s.c >> @@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu, >> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, >> struct kvm_guest_debug *dbg) >> { >> - return -EINVAL; >> + vcpu->guest_debug = dbg->control; >> + return 0; >> } >> >> void kvmppc_decrementer_func(unsigned long data) >> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c >> index 27cced9..3a2414c 100644 >> --- a/arch/powerpc/kvm/book3s_hv.c >> +++ b/arch/powerpc/kvm/book3s_hv.c >> @@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd) >> return kvmppc_hcall_impl_hv_realmode(cmd); >> } >> >> +static int kvmppc_emulate_debug_inst(struct kvm_run *run, >> + struct kvm_vcpu *vcpu) >> +{ >> + u32 last_inst; >> + >> + if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) != >> + EMULATE_DONE) { >> + /* >> + * Fetch failed, so return to guest and >> + * try executing it again. >> + */ >> + return RESUME_GUEST; > > Please end the if() here. In the kernel we usually treat abort > situations as separate. > OK. Will change it. >> + } else { >> + if (last_inst == KVMPPC_INST_SW_BREAKPOINT) { >> + run->exit_reason = KVM_EXIT_DEBUG; >> + run->debug.arch.address = kvmppc_get_pc(vcpu); >> + return RESUME_HOST; >> + } else { >> + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); >> + return RESUME_GUEST; >> + } >> + } >> +} >> + >> static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, >> struct task_struct *tsk) >> { >> @@ -807,12 +831,18 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, >> break; >> /* >> * This occurs if the guest executes an illegal instruction. >> - * We just generate a program interrupt to the guest, since >> - * we don't emulate any guest instructions at this stage. >> + * If the guest debug is disabled, generate a program interrupt >> + * to the guest. If guest debug is enabled, we need to check >> + * whether the instruction is a software breakpoint instruction. >> + * Accordingly return to Guest or Host. >> */ >> case BOOK3S_INTERRUPT_H_EMUL_ASSIST: >> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL); >> - r = RESUME_GUEST; >> + if(vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) { >> + r = kvmppc_emulate_debug_inst(run, vcpu); >> + } else { >> + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); >> + r = RESUME_GUEST; >> + } >> break; >> /* >> * This occurs if the guest (kernel or userspace), does something that >> @@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id, >> long int i; >> >> switch (id) { >> + case KVM_REG_PPC_DEBUG_INST: >> + *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT); >> + break; >> case KVM_REG_PPC_HIOR: >> *val = get_reg_val(id, 0); >> break; >> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c >> index faffb27..6d73708 100644 >> --- a/arch/powerpc/kvm/book3s_pr.c >> +++ b/arch/powerpc/kvm/book3s_pr.c >> @@ -1319,6 +1319,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, u64 id, >> int r = 0; >> >> switch (id) { >> + case KVM_REG_PPC_DEBUG_INST: >> + *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT); >> + break; >> case KVM_REG_PPC_HIOR: >> *val = get_reg_val(id, to_book3s(vcpu)->hior); >> break; >> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c >> index e96b50d..30f326b 100644 >> --- a/arch/powerpc/kvm/emulate.c >> +++ b/arch/powerpc/kvm/emulate.c >> @@ -274,6 +274,24 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu) >> } >> break; >> >> + case 0: >> + /* >> + * Instruction with primary opcode 0. Based on PowerISA >> + * these are illegal instructions. >> + */ >> + if(inst == KVMPPC_INST_SW_BREAKPOINT) { > > Here you're missing a space between if and (. Are you sure that you ran > checkpatch.pl? > No. my bad. Did not run checkpatch.pl. Will fix it. > >> + run->exit_reason = KVM_EXIT_DEBUG; >> + run->debug.arch.address = kvmppc_get_pc(vcpu); >> + emulated = EMULATE_EXIT_USER; >> + advance = 0; >> + printk(KERN_INFO "pr_emulate: debug instr \n"); >> + } else { >> + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); >> + emulated = EMULATE_DONE; >> + advance = 0; > > Just set emulated = EMULATE_FAIL here, the same as default:. > > Ok sure will do. Thanks for review. Maddy > Alex > >> + } >> + break; >> + >> default: >> emulated = EMULATE_FAIL; >> } >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc 2014-09-07 16:31 [PATCH 0/2 v5] powerpc/kvm: support to handle sw breakpoint Madhavan Srinivasan 2014-09-07 16:31 ` [PATCH 1/2 " Madhavan Srinivasan @ 2014-09-07 16:31 ` Madhavan Srinivasan 2014-09-08 13:09 ` Alexander Graf 1 sibling, 1 reply; 7+ messages in thread From: Madhavan Srinivasan @ 2014-09-07 16:31 UTC (permalink / raw) To: agraf, benh, paulus, mpe; +Cc: Madhavan Srinivasan, linuxppc-dev, kvm-ppc, kvm This patch extends the use of illegal instruction as software breakpoint instruction across the ppc platform. Patch extends booke program interrupt code to support software breakpoint. Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> --- Patch is only compile tested. Will really help if someone can try it out and let me know comments. arch/powerpc/kvm/booke.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index b4c89fa..1b84853 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -870,6 +870,10 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, case BOOKE_INTERRUPT_HV_PRIV: emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); break; + case BOOKE_INTERRUPT_PROGRAM: + /*SW breakpoints arrive as illegal instructions on HV */ + emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); + break; default: break; } @@ -947,7 +951,17 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; case BOOKE_INTERRUPT_PROGRAM: - if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) { + if ((vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) && + (last_inst == KVMPPC_INST_SW_BREAKPOINT)) { + /* + * We are here because of an SW breakpoint instr, + * so lets return to host to handle. + */ + r = kvmppc_handle_debug(run, vcpu); + run->exit_reason = KVM_EXIT_DEBUG; + kvmppc_account_exit(vcpu, DEBUG_EXITS); + break; + } else { /* * Program traps generated by user-level software must * be handled by the guest kernel. @@ -1505,7 +1519,7 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) val = get_reg_val(reg->id, vcpu->arch.tsr); break; case KVM_REG_PPC_DEBUG_INST: - val = get_reg_val(reg->id, KVMPPC_INST_EHPRIV_DEBUG); + val = get_reg_val(reg->id, KVMPPC_INST_SW_BREAKPOINT); break; case KVM_REG_PPC_VRSAVE: val = get_reg_val(reg->id, vcpu->arch.vrsave); -- 1.7.11.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc 2014-09-07 16:31 ` [PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc Madhavan Srinivasan @ 2014-09-08 13:09 ` Alexander Graf 2014-09-09 7:41 ` Madhavan Srinivasan 0 siblings, 1 reply; 7+ messages in thread From: Alexander Graf @ 2014-09-08 13:09 UTC (permalink / raw) To: Madhavan Srinivasan, benh, paulus, mpe; +Cc: linuxppc-dev, kvm-ppc, kvm On 07.09.14 18:31, Madhavan Srinivasan wrote: > This patch extends the use of illegal instruction as software > breakpoint instruction across the ppc platform. Patch extends > booke program interrupt code to support software breakpoint. > > Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> > --- > > Patch is only compile tested. Will really help if > someone can try it out and let me know comments. > > arch/powerpc/kvm/booke.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index b4c89fa..1b84853 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -870,6 +870,10 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, > case BOOKE_INTERRUPT_HV_PRIV: > emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); > break; > + case BOOKE_INTERRUPT_PROGRAM: > + /*SW breakpoints arrive as illegal instructions on HV */ Is it my email client or is there a space missing again? ;) Also, please only fetch the last instruction if debugging is active. > + emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); > + break; > default: > break; > } > @@ -947,7 +951,17 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, > break; > > case BOOKE_INTERRUPT_PROGRAM: > - if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) { > + if ((vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) && > + (last_inst == KVMPPC_INST_SW_BREAKPOINT)) { I think this is changing the logic from "if the guest is in user mode or we're in HV, deflect" to "if the guest is in user mode or an HV guest and the instruction is a breakpoint, treat it as debug. Otherwise deflect". So you're essentially breaking PR KVM here from what I can tell. Why don't you just split the whole thing out to the beginning of BOOKE_INTERRUPT_PROGRAM and check for a) debug is enabled b) instruction is sw breakpoint instead? > + /* > + * We are here because of an SW breakpoint instr, > + * so lets return to host to handle. > + */ > + r = kvmppc_handle_debug(run, vcpu); > + run->exit_reason = KVM_EXIT_DEBUG; > + kvmppc_account_exit(vcpu, DEBUG_EXITS); > + break; > + } else { > /* > * Program traps generated by user-level software must > * be handled by the guest kernel. > @@ -1505,7 +1519,7 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) > val = get_reg_val(reg->id, vcpu->arch.tsr); > break; > case KVM_REG_PPC_DEBUG_INST: > - val = get_reg_val(reg->id, KVMPPC_INST_EHPRIV_DEBUG); Please also remove the definition of EHPRIV_DEBUG. Alex > + val = get_reg_val(reg->id, KVMPPC_INST_SW_BREAKPOINT); > break; > case KVM_REG_PPC_VRSAVE: > val = get_reg_val(reg->id, vcpu->arch.vrsave); > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc 2014-09-08 13:09 ` Alexander Graf @ 2014-09-09 7:41 ` Madhavan Srinivasan 0 siblings, 0 replies; 7+ messages in thread From: Madhavan Srinivasan @ 2014-09-09 7:41 UTC (permalink / raw) To: Alexander Graf, benh, paulus, mpe; +Cc: linuxppc-dev, kvm-ppc, kvm On Monday 08 September 2014 06:39 PM, Alexander Graf wrote: > > > On 07.09.14 18:31, Madhavan Srinivasan wrote: >> This patch extends the use of illegal instruction as software >> breakpoint instruction across the ppc platform. Patch extends >> booke program interrupt code to support software breakpoint. >> >> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> >> --- >> >> Patch is only compile tested. Will really help if >> someone can try it out and let me know comments. >> >> arch/powerpc/kvm/booke.c | 18 ++++++++++++++++-- >> 1 file changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c >> index b4c89fa..1b84853 100644 >> --- a/arch/powerpc/kvm/booke.c >> +++ b/arch/powerpc/kvm/booke.c >> @@ -870,6 +870,10 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, >> case BOOKE_INTERRUPT_HV_PRIV: >> emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); >> break; >> + case BOOKE_INTERRUPT_PROGRAM: >> + /*SW breakpoints arrive as illegal instructions on HV */ > > Is it my email client or is there a space missing again? ;) > Facepalm. Will fix it. > Also, please only fetch the last instruction if debugging is active. > Will change it. >> + emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); >> + break; >> default: >> break; >> } >> @@ -947,7 +951,17 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, >> break; >> >> case BOOKE_INTERRUPT_PROGRAM: >> - if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) { >> + if ((vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) && >> + (last_inst == KVMPPC_INST_SW_BREAKPOINT)) { > > I think this is changing the logic from "if the guest is in user mode or > we're in HV, deflect" to "if the guest is in user mode or an HV guest > and the instruction is a breakpoint, treat it as debug. Otherwise > deflect". So you're essentially breaking PR KVM here from what I can tell. > > Why don't you just split the whole thing out to the beginning of > BOOKE_INTERRUPT_PROGRAM and check for > > a) debug is enabled > b) instruction is sw breakpoint > This is what we pretty much do for the server side. Will changes it. > instead? > >> + /* >> + * We are here because of an SW breakpoint instr, >> + * so lets return to host to handle. >> + */ >> + r = kvmppc_handle_debug(run, vcpu); >> + run->exit_reason = KVM_EXIT_DEBUG; >> + kvmppc_account_exit(vcpu, DEBUG_EXITS); >> + break; >> + } else { >> /* >> * Program traps generated by user-level software must >> * be handled by the guest kernel. >> @@ -1505,7 +1519,7 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) >> val = get_reg_val(reg->id, vcpu->arch.tsr); >> break; >> case KVM_REG_PPC_DEBUG_INST: >> - val = get_reg_val(reg->id, KVMPPC_INST_EHPRIV_DEBUG); > > Please also remove the definition of EHPRIV_DEBUG. > OK. Will do. Thanks for review Maddy > > Alex > >> + val = get_reg_val(reg->id, KVMPPC_INST_SW_BREAKPOINT); >> break; >> case KVM_REG_PPC_VRSAVE: >> val = get_reg_val(reg->id, vcpu->arch.vrsave); >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-09-09 7:41 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-07 16:31 [PATCH 0/2 v5] powerpc/kvm: support to handle sw breakpoint Madhavan Srinivasan 2014-09-07 16:31 ` [PATCH 1/2 " Madhavan Srinivasan 2014-09-08 13:05 ` Alexander Graf 2014-09-09 7:31 ` Madhavan Srinivasan 2014-09-07 16:31 ` [PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc Madhavan Srinivasan 2014-09-08 13:09 ` Alexander Graf 2014-09-09 7:41 ` Madhavan Srinivasan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).