| Message ID | 1358324685-30225-3-git-send-email-bharat.bhushan@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16.01.2013, at 09:24, Bharat Bhushan wrote: > This patch adds the one_reg interface to get the special instruction > to be used for setting software breakpoint from userspace. > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > --- > Documentation/virtual/kvm/api.txt | 1 + > arch/powerpc/include/asm/kvm_ppc.h | 1 + > arch/powerpc/include/uapi/asm/kvm.h | 3 +++ > arch/powerpc/kvm/44x.c | 5 +++++ > arch/powerpc/kvm/booke.c | 10 ++++++++++ > arch/powerpc/kvm/e500.c | 5 +++++ > arch/powerpc/kvm/e500.h | 9 +++++++++ > arch/powerpc/kvm/e500mc.c | 5 +++++ > 8 files changed, 39 insertions(+), 0 deletions(-) > > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt > index 09905cb..7e8be9e 100644 > --- a/Documentation/virtual/kvm/api.txt > +++ b/Documentation/virtual/kvm/api.txt > @@ -1775,6 +1775,7 @@ registers, find a list below: > PPC | KVM_REG_PPC_VPA_DTL | 128 > PPC | KVM_REG_PPC_EPCR | 32 > PPC | KVM_REG_PPC_EPR | 32 > + PPC | KVM_REG_PPC_DEBUG_INST| 32 > > 4.69 KVM_GET_ONE_REG > > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h > index 44a657a..b3c481e 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -235,6 +235,7 @@ union kvmppc_one_reg { > > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); > int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); > +u32 kvmppc_core_debug_inst_op(void); > > void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); > int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); > diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h > index 16064d0..e81ae5b 100644 > --- a/arch/powerpc/include/uapi/asm/kvm.h > +++ b/arch/powerpc/include/uapi/asm/kvm.h > @@ -417,4 +417,7 @@ struct kvm_get_htab_header { > #define KVM_REG_PPC_EPCR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85) > #define KVM_REG_PPC_EPR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86) > > +/* Debugging: Special instruction for software breakpoint */ > +#define KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x87) > + > #endif /* __LINUX_KVM_POWERPC_H */ > diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c > index 3d7fd21..41501be 100644 > --- a/arch/powerpc/kvm/44x.c > +++ b/arch/powerpc/kvm/44x.c > @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu, > return 0; > } > > +u32 kvmppc_core_debug_inst_op(void) > +{ > + return -1; > +} > + > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs) > { > kvmppc_get_sregs_ivor(vcpu, sregs); > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index d2f502d..453a10f 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c Please provide the DEBUG_INST on a more global level - across all ppc subarchs. > @@ -1424,6 +1424,12 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) > r = put_user(vcpu->arch.epcr, (u32 __user *)(long)reg->addr); > break; > #endif > + case KVM_REG_PPC_DEBUG_INST: { > + u32 opcode = kvmppc_core_debug_inst_op(); > + r = copy_to_user((u32 __user *)(long)reg->addr, > + &opcode, sizeof(u32)); > + break; > + } > default: > break; > } > @@ -1467,6 +1473,10 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) > break; > } > #endif > + case KVM_REG_PPC_DEBUG_INST: > + /* This is read only, so write to this is nop*/ > + r = 0; > + break; Just don't support set_one_reg on this reg. > default: > break; > } > diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c > index 6dd4de7..d8a5e8e 100644 > --- a/arch/powerpc/kvm/e500.c > +++ b/arch/powerpc/kvm/e500.c > @@ -367,6 +367,11 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) > return 0; > } > > +u32 kvmppc_core_debug_inst_op(void) > +{ > + return KVMPPC_INST_GUEST_GDB; > +} > + > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs) > { > struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); > diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h > index c70d37e..17942d2 100644 > --- a/arch/powerpc/kvm/e500.h > +++ b/arch/powerpc/kvm/e500.h > @@ -302,4 +302,13 @@ static inline unsigned int get_tlbmiss_tid(struct kvm_vcpu *vcpu) > #define get_tlb_sts(gtlbe) (MAS1_TS) > #endif /* !BOOKE_HV */ > > +/* When setting software breakpoint, Change the software breakpoint > + * instruction to special trap/invalid instruction and set > + * KVM_GUESTDBG_USE_SW_BP flag in kvm_guest_debug->control. KVM does > + * keep track of software breakpoints. So when KVM_GUESTDBG_USE_SW_BP > + * flag is set and special trap instruction is executed by guest then > + * exit to userspace. This comment chunk no apply to define. Also please fix English ;). Alex > + */ > +#define KVMPPC_INST_GUEST_GDB 0x7C00021C /* ehpriv OC=0 */ > + > #endif /* KVM_E500_H */ > diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c > index 1f89d26..dead142 100644 > --- a/arch/powerpc/kvm/e500mc.c > +++ b/arch/powerpc/kvm/e500mc.c > @@ -199,6 +199,11 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) > return 0; > } > > +u32 kvmppc_core_debug_inst_op(void) > +{ > + return KVMPPC_INST_GUEST_GDB; > +} > + > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs) > { > struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); > -- > 1.7.0.4 > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Friday, January 25, 2013 5:18 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777 > Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug instruction > > > On 16.01.2013, at 09:24, Bharat Bhushan wrote: > > > This patch adds the one_reg interface to get the special instruction > > to be used for setting software breakpoint from userspace. > > > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > > --- > > Documentation/virtual/kvm/api.txt | 1 + > > arch/powerpc/include/asm/kvm_ppc.h | 1 + > > arch/powerpc/include/uapi/asm/kvm.h | 3 +++ > > arch/powerpc/kvm/44x.c | 5 +++++ > > arch/powerpc/kvm/booke.c | 10 ++++++++++ > > arch/powerpc/kvm/e500.c | 5 +++++ > > arch/powerpc/kvm/e500.h | 9 +++++++++ > > arch/powerpc/kvm/e500mc.c | 5 +++++ > > 8 files changed, 39 insertions(+), 0 deletions(-) > > > > diff --git a/Documentation/virtual/kvm/api.txt > > b/Documentation/virtual/kvm/api.txt > > index 09905cb..7e8be9e 100644 > > --- a/Documentation/virtual/kvm/api.txt > > +++ b/Documentation/virtual/kvm/api.txt > > @@ -1775,6 +1775,7 @@ registers, find a list below: > > PPC | KVM_REG_PPC_VPA_DTL | 128 > > PPC | KVM_REG_PPC_EPCR | 32 > > PPC | KVM_REG_PPC_EPR | 32 > > + PPC | KVM_REG_PPC_DEBUG_INST| 32 > > > > 4.69 KVM_GET_ONE_REG > > > > diff --git a/arch/powerpc/include/asm/kvm_ppc.h > > b/arch/powerpc/include/asm/kvm_ppc.h > > index 44a657a..b3c481e 100644 > > --- a/arch/powerpc/include/asm/kvm_ppc.h > > +++ b/arch/powerpc/include/asm/kvm_ppc.h > > @@ -235,6 +235,7 @@ union kvmppc_one_reg { > > > > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs > > *sregs); int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct > > kvm_sregs *sregs); > > +u32 kvmppc_core_debug_inst_op(void); > > > > void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs > > *sregs); int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct > > kvm_sregs *sregs); diff --git a/arch/powerpc/include/uapi/asm/kvm.h > > b/arch/powerpc/include/uapi/asm/kvm.h > > index 16064d0..e81ae5b 100644 > > --- a/arch/powerpc/include/uapi/asm/kvm.h > > +++ b/arch/powerpc/include/uapi/asm/kvm.h > > @@ -417,4 +417,7 @@ struct kvm_get_htab_header { > > #define KVM_REG_PPC_EPCR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85) > > #define KVM_REG_PPC_EPR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86) > > > > +/* Debugging: Special instruction for software breakpoint */ #define > > +KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x87) > > + > > #endif /* __LINUX_KVM_POWERPC_H */ > > diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c index > > 3d7fd21..41501be 100644 > > --- a/arch/powerpc/kvm/44x.c > > +++ b/arch/powerpc/kvm/44x.c > > @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu, > > return 0; > > } > > > > +u32 kvmppc_core_debug_inst_op(void) > > +{ > > + return -1; > > +} > > + > > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs > > *sregs) { > > kvmppc_get_sregs_ivor(vcpu, sregs); > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index > > d2f502d..453a10f 100644 > > --- a/arch/powerpc/kvm/booke.c > > +++ b/arch/powerpc/kvm/booke.c > > Please provide the DEBUG_INST on a more global level - across all ppc subarchs. Do you mean defining in powerpc.c ? We are using one_reg for DEBUG_INST and one_reg_ioctl and defined in respective subarchs (booke and books have their separate handler). So how you want this to be defined in more common way for all subarchs? Thanks -Bharat > > > @@ -1424,6 +1424,12 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, > struct kvm_one_reg *reg) > > r = put_user(vcpu->arch.epcr, (u32 __user *)(long)reg->addr); > > break; > > #endif > > + case KVM_REG_PPC_DEBUG_INST: { > > + u32 opcode = kvmppc_core_debug_inst_op(); > > + r = copy_to_user((u32 __user *)(long)reg->addr, > > + &opcode, sizeof(u32)); > > + break; > > + } > > default: > > break; > > } > > @@ -1467,6 +1473,10 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, > struct kvm_one_reg *reg) > > break; > > } > > #endif > > + case KVM_REG_PPC_DEBUG_INST: > > + /* This is read only, so write to this is nop*/ > > + r = 0; > > + break; > > Just don't support set_one_reg on this reg. > > > default: > > break; > > } > > diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c index > > 6dd4de7..d8a5e8e 100644 > > --- a/arch/powerpc/kvm/e500.c > > +++ b/arch/powerpc/kvm/e500.c > > @@ -367,6 +367,11 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) > > return 0; > > } > > > > +u32 kvmppc_core_debug_inst_op(void) > > +{ > > + return KVMPPC_INST_GUEST_GDB; > > +} > > + > > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs > > *sregs) { > > struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); diff --git > > a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h index > > c70d37e..17942d2 100644 > > --- a/arch/powerpc/kvm/e500.h > > +++ b/arch/powerpc/kvm/e500.h > > @@ -302,4 +302,13 @@ static inline unsigned int get_tlbmiss_tid(struct > kvm_vcpu *vcpu) > > #define get_tlb_sts(gtlbe) (MAS1_TS) > > #endif /* !BOOKE_HV */ > > > > +/* When setting software breakpoint, Change the software breakpoint > > + * instruction to special trap/invalid instruction and set > > + * KVM_GUESTDBG_USE_SW_BP flag in kvm_guest_debug->control. KVM does > > + * keep track of software breakpoints. So when KVM_GUESTDBG_USE_SW_BP > > + * flag is set and special trap instruction is executed by guest then > > + * exit to userspace. > > This comment chunk no apply to define. Also please fix English ;). > > > Alex > > > + */ > > +#define KVMPPC_INST_GUEST_GDB 0x7C00021C /* ehpriv OC=0 */ > > + > > #endif /* KVM_E500_H */ > > diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c > > index 1f89d26..dead142 100644 > > --- a/arch/powerpc/kvm/e500mc.c > > +++ b/arch/powerpc/kvm/e500mc.c > > @@ -199,6 +199,11 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) > > return 0; > > } > > > > +u32 kvmppc_core_debug_inst_op(void) > > +{ > > + return KVMPPC_INST_GUEST_GDB; > > +} > > + > > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs > > *sregs) { > > struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); > > -- > > 1.7.0.4 > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > > the body of a message to majordomo@vger.kernel.org More majordomo info > > at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31.01.2013, at 18:44, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Friday, January 25, 2013 5:18 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777 >> Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug instruction >> >> >> On 16.01.2013, at 09:24, Bharat Bhushan wrote: >> >>> This patch adds the one_reg interface to get the special instruction >>> to be used for setting software breakpoint from userspace. >>> >>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> >>> --- >>> Documentation/virtual/kvm/api.txt | 1 + >>> arch/powerpc/include/asm/kvm_ppc.h | 1 + >>> arch/powerpc/include/uapi/asm/kvm.h | 3 +++ >>> arch/powerpc/kvm/44x.c | 5 +++++ >>> arch/powerpc/kvm/booke.c | 10 ++++++++++ >>> arch/powerpc/kvm/e500.c | 5 +++++ >>> arch/powerpc/kvm/e500.h | 9 +++++++++ >>> arch/powerpc/kvm/e500mc.c | 5 +++++ >>> 8 files changed, 39 insertions(+), 0 deletions(-) >>> >>> diff --git a/Documentation/virtual/kvm/api.txt >>> b/Documentation/virtual/kvm/api.txt >>> index 09905cb..7e8be9e 100644 >>> --- a/Documentation/virtual/kvm/api.txt >>> +++ b/Documentation/virtual/kvm/api.txt >>> @@ -1775,6 +1775,7 @@ registers, find a list below: >>> PPC | KVM_REG_PPC_VPA_DTL | 128 >>> PPC | KVM_REG_PPC_EPCR | 32 >>> PPC | KVM_REG_PPC_EPR | 32 >>> + PPC | KVM_REG_PPC_DEBUG_INST| 32 >>> >>> 4.69 KVM_GET_ONE_REG >>> >>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h >>> b/arch/powerpc/include/asm/kvm_ppc.h >>> index 44a657a..b3c481e 100644 >>> --- a/arch/powerpc/include/asm/kvm_ppc.h >>> +++ b/arch/powerpc/include/asm/kvm_ppc.h >>> @@ -235,6 +235,7 @@ union kvmppc_one_reg { >>> >>> void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs >>> *sregs); int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct >>> kvm_sregs *sregs); >>> +u32 kvmppc_core_debug_inst_op(void); >>> >>> void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs >>> *sregs); int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct >>> kvm_sregs *sregs); diff --git a/arch/powerpc/include/uapi/asm/kvm.h >>> b/arch/powerpc/include/uapi/asm/kvm.h >>> index 16064d0..e81ae5b 100644 >>> --- a/arch/powerpc/include/uapi/asm/kvm.h >>> +++ b/arch/powerpc/include/uapi/asm/kvm.h >>> @@ -417,4 +417,7 @@ struct kvm_get_htab_header { >>> #define KVM_REG_PPC_EPCR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85) >>> #define KVM_REG_PPC_EPR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86) >>> >>> +/* Debugging: Special instruction for software breakpoint */ #define >>> +KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x87) >>> + >>> #endif /* __LINUX_KVM_POWERPC_H */ >>> diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c index >>> 3d7fd21..41501be 100644 >>> --- a/arch/powerpc/kvm/44x.c >>> +++ b/arch/powerpc/kvm/44x.c >>> @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu, >>> return 0; >>> } >>> >>> +u32 kvmppc_core_debug_inst_op(void) >>> +{ >>> + return -1; The way you handle it here this needs to be an int kvmppc_core_debug_inst_op(u32 *inst) so you can return an error for 440. I don't think it's worth to worry about a case where we don't know about the inst though. Just return the same as what we use on e500v2 here. >>> +} >>> + >>> void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs >>> *sregs) { >>> kvmppc_get_sregs_ivor(vcpu, sregs); >>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index >>> d2f502d..453a10f 100644 >>> --- a/arch/powerpc/kvm/booke.c >>> +++ b/arch/powerpc/kvm/booke.c >> >> Please provide the DEBUG_INST on a more global level - across all ppc subarchs. > > Do you mean defining in powerpc.c ? > > We are using one_reg for DEBUG_INST and one_reg_ioctl and defined in respective subarchs (booke and books have their separate handler). So how you want this to be defined in more common way for all subarchs? Just add it to all subarch's one_reg handlers. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Thursday, January 31, 2013 11:23 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org > Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug instruction > > > On 31.01.2013, at 18:44, Bhushan Bharat-R65777 wrote: > > > > > > >> -----Original Message----- > >> From: Alexander Graf [mailto:agraf@suse.de] > >> Sent: Friday, January 25, 2013 5:18 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan > >> Bharat-R65777 > >> Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug > >> instruction > >> > >> > >> On 16.01.2013, at 09:24, Bharat Bhushan wrote: > >> > >>> This patch adds the one_reg interface to get the special instruction > >>> to be used for setting software breakpoint from userspace. > >>> > >>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > >>> --- > >>> Documentation/virtual/kvm/api.txt | 1 + > >>> arch/powerpc/include/asm/kvm_ppc.h | 1 + > >>> arch/powerpc/include/uapi/asm/kvm.h | 3 +++ > >>> arch/powerpc/kvm/44x.c | 5 +++++ > >>> arch/powerpc/kvm/booke.c | 10 ++++++++++ > >>> arch/powerpc/kvm/e500.c | 5 +++++ > >>> arch/powerpc/kvm/e500.h | 9 +++++++++ > >>> arch/powerpc/kvm/e500mc.c | 5 +++++ > >>> 8 files changed, 39 insertions(+), 0 deletions(-) > >>> > >>> diff --git a/Documentation/virtual/kvm/api.txt > >>> b/Documentation/virtual/kvm/api.txt > >>> index 09905cb..7e8be9e 100644 > >>> --- a/Documentation/virtual/kvm/api.txt > >>> +++ b/Documentation/virtual/kvm/api.txt > >>> @@ -1775,6 +1775,7 @@ registers, find a list below: > >>> PPC | KVM_REG_PPC_VPA_DTL | 128 > >>> PPC | KVM_REG_PPC_EPCR | 32 > >>> PPC | KVM_REG_PPC_EPR | 32 > >>> + PPC | KVM_REG_PPC_DEBUG_INST| 32 > >>> > >>> 4.69 KVM_GET_ONE_REG > >>> > >>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h > >>> b/arch/powerpc/include/asm/kvm_ppc.h > >>> index 44a657a..b3c481e 100644 > >>> --- a/arch/powerpc/include/asm/kvm_ppc.h > >>> +++ b/arch/powerpc/include/asm/kvm_ppc.h > >>> @@ -235,6 +235,7 @@ union kvmppc_one_reg { > >>> > >>> void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs > >>> *sregs); int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct > >>> kvm_sregs *sregs); > >>> +u32 kvmppc_core_debug_inst_op(void); > >>> > >>> void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs > >>> *sregs); int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct > >>> kvm_sregs *sregs); diff --git a/arch/powerpc/include/uapi/asm/kvm.h > >>> b/arch/powerpc/include/uapi/asm/kvm.h > >>> index 16064d0..e81ae5b 100644 > >>> --- a/arch/powerpc/include/uapi/asm/kvm.h > >>> +++ b/arch/powerpc/include/uapi/asm/kvm.h > >>> @@ -417,4 +417,7 @@ struct kvm_get_htab_header { > >>> #define KVM_REG_PPC_EPCR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85) > >>> #define KVM_REG_PPC_EPR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86) > >>> > >>> +/* Debugging: Special instruction for software breakpoint */ > >>> +#define KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | > >>> +0x87) > >>> + > >>> #endif /* __LINUX_KVM_POWERPC_H */ > >>> diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c index > >>> 3d7fd21..41501be 100644 > >>> --- a/arch/powerpc/kvm/44x.c > >>> +++ b/arch/powerpc/kvm/44x.c > >>> @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu, > >>> return 0; > >>> } > >>> > >>> +u32 kvmppc_core_debug_inst_op(void) { > >>> + return -1; > > The way you handle it here this needs to be an int > kvmppc_core_debug_inst_op(u32 *inst) so you can return an error for 440. I don't > think it's worth to worry about a case where we don't know about the inst > though. Just return the same as what we use on e500v2 here. > > >>> +} > >>> + > >>> void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs > >>> *sregs) { > >>> kvmppc_get_sregs_ivor(vcpu, sregs); diff --git > >>> a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index > >>> d2f502d..453a10f 100644 > >>> --- a/arch/powerpc/kvm/booke.c > >>> +++ b/arch/powerpc/kvm/booke.c > >> > >> Please provide the DEBUG_INST on a more global level - across all ppc > subarchs. > > > > Do you mean defining in powerpc.c ? > > > > We are using one_reg for DEBUG_INST and one_reg_ioctl and defined in > respective subarchs (booke and books have their separate handler). So how you > want this to be defined in more common way for all subarchs? > > Just add it to all subarch's one_reg handlers. And what book3s etc should return? -1 ? Thanks -Bharat -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31.01.2013, at 18:58, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Thursday, January 31, 2013 11:23 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >> Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug instruction >> >> >> On 31.01.2013, at 18:44, Bhushan Bharat-R65777 wrote: >> >>> >>> >>>> -----Original Message----- >>>> From: Alexander Graf [mailto:agraf@suse.de] >>>> Sent: Friday, January 25, 2013 5:18 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan >>>> Bharat-R65777 >>>> Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug >>>> instruction >>>> >>>> >>>> On 16.01.2013, at 09:24, Bharat Bhushan wrote: >>>> >>>>> This patch adds the one_reg interface to get the special instruction >>>>> to be used for setting software breakpoint from userspace. >>>>> >>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> >>>>> --- >>>>> Documentation/virtual/kvm/api.txt | 1 + >>>>> arch/powerpc/include/asm/kvm_ppc.h | 1 + >>>>> arch/powerpc/include/uapi/asm/kvm.h | 3 +++ >>>>> arch/powerpc/kvm/44x.c | 5 +++++ >>>>> arch/powerpc/kvm/booke.c | 10 ++++++++++ >>>>> arch/powerpc/kvm/e500.c | 5 +++++ >>>>> arch/powerpc/kvm/e500.h | 9 +++++++++ >>>>> arch/powerpc/kvm/e500mc.c | 5 +++++ >>>>> 8 files changed, 39 insertions(+), 0 deletions(-) >>>>> >>>>> diff --git a/Documentation/virtual/kvm/api.txt >>>>> b/Documentation/virtual/kvm/api.txt >>>>> index 09905cb..7e8be9e 100644 >>>>> --- a/Documentation/virtual/kvm/api.txt >>>>> +++ b/Documentation/virtual/kvm/api.txt >>>>> @@ -1775,6 +1775,7 @@ registers, find a list below: >>>>> PPC | KVM_REG_PPC_VPA_DTL | 128 >>>>> PPC | KVM_REG_PPC_EPCR | 32 >>>>> PPC | KVM_REG_PPC_EPR | 32 >>>>> + PPC | KVM_REG_PPC_DEBUG_INST| 32 >>>>> >>>>> 4.69 KVM_GET_ONE_REG >>>>> >>>>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h >>>>> b/arch/powerpc/include/asm/kvm_ppc.h >>>>> index 44a657a..b3c481e 100644 >>>>> --- a/arch/powerpc/include/asm/kvm_ppc.h >>>>> +++ b/arch/powerpc/include/asm/kvm_ppc.h >>>>> @@ -235,6 +235,7 @@ union kvmppc_one_reg { >>>>> >>>>> void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs >>>>> *sregs); int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct >>>>> kvm_sregs *sregs); >>>>> +u32 kvmppc_core_debug_inst_op(void); >>>>> >>>>> void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs >>>>> *sregs); int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct >>>>> kvm_sregs *sregs); diff --git a/arch/powerpc/include/uapi/asm/kvm.h >>>>> b/arch/powerpc/include/uapi/asm/kvm.h >>>>> index 16064d0..e81ae5b 100644 >>>>> --- a/arch/powerpc/include/uapi/asm/kvm.h >>>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h >>>>> @@ -417,4 +417,7 @@ struct kvm_get_htab_header { >>>>> #define KVM_REG_PPC_EPCR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85) >>>>> #define KVM_REG_PPC_EPR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86) >>>>> >>>>> +/* Debugging: Special instruction for software breakpoint */ >>>>> +#define KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | >>>>> +0x87) >>>>> + >>>>> #endif /* __LINUX_KVM_POWERPC_H */ >>>>> diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c index >>>>> 3d7fd21..41501be 100644 >>>>> --- a/arch/powerpc/kvm/44x.c >>>>> +++ b/arch/powerpc/kvm/44x.c >>>>> @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu, >>>>> return 0; >>>>> } >>>>> >>>>> +u32 kvmppc_core_debug_inst_op(void) { >>>>> + return -1; >> >> The way you handle it here this needs to be an int >> kvmppc_core_debug_inst_op(u32 *inst) so you can return an error for 440. I don't >> think it's worth to worry about a case where we don't know about the inst >> though. Just return the same as what we use on e500v2 here. >> >>>>> +} >>>>> + >>>>> void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs >>>>> *sregs) { >>>>> kvmppc_get_sregs_ivor(vcpu, sregs); diff --git >>>>> a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index >>>>> d2f502d..453a10f 100644 >>>>> --- a/arch/powerpc/kvm/booke.c >>>>> +++ b/arch/powerpc/kvm/booke.c >>>> >>>> Please provide the DEBUG_INST on a more global level - across all ppc >> subarchs. >>> >>> Do you mean defining in powerpc.c ? >>> >>> We are using one_reg for DEBUG_INST and one_reg_ioctl and defined in >> respective subarchs (booke and books have their separate handler). So how you >> want this to be defined in more common way for all subarchs? >> >> Just add it to all subarch's one_reg handlers. > > And what book3s etc should return? > > -1 ? trap maybe? Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 16, 2013 at 01:54:41PM +0530, Bharat Bhushan wrote: > This patch adds the one_reg interface to get the special instruction > to be used for setting software breakpoint from userspace. Since this presumably is constant for any given platform, wouldn't a capability be more appropriate? Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04.02.2013, at 01:41, Paul Mackerras wrote: > On Wed, Jan 16, 2013 at 01:54:41PM +0530, Bharat Bhushan wrote: >> This patch adds the one_reg interface to get the special instruction >> to be used for setting software breakpoint from userspace. > > Since this presumably is constant for any given platform, wouldn't > a capability be more appropriate? How so? A capability only tells you "I can do debug". Or "I can do debug on e500". I don't want to reteach QEMU how to do debug for every core we implement. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 07, 2013 at 03:29:50PM +0100, Alexander Graf wrote: > > On 04.02.2013, at 01:41, Paul Mackerras wrote: > > > On Wed, Jan 16, 2013 at 01:54:41PM +0530, Bharat Bhushan wrote: > >> This patch adds the one_reg interface to get the special instruction > >> to be used for setting software breakpoint from userspace. > > > > Since this presumably is constant for any given platform, wouldn't > > a capability be more appropriate? > > How so? A capability only tells you "I can do debug". Or "I can do debug on e500". I don't want to reteach QEMU how to do debug for every core we implement. Capabilities aren't just binary - the get-capability (check_extension) ioctl returns a 32-bit value, and we already have some that return values other than 0 or 1. So I was thinking that we could add a capability which when queried returns the special instruction. It's up to you whether you want to do it that way or not. Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index 09905cb..7e8be9e 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -1775,6 +1775,7 @@ registers, find a list below: PPC | KVM_REG_PPC_VPA_DTL | 128 PPC | KVM_REG_PPC_EPCR | 32 PPC | KVM_REG_PPC_EPR | 32 + PPC | KVM_REG_PPC_DEBUG_INST| 32 4.69 KVM_GET_ONE_REG diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index 44a657a..b3c481e 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -235,6 +235,7 @@ union kvmppc_one_reg { void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); +u32 kvmppc_core_debug_inst_op(void); void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index 16064d0..e81ae5b 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -417,4 +417,7 @@ struct kvm_get_htab_header { #define KVM_REG_PPC_EPCR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85) #define KVM_REG_PPC_EPR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86) +/* Debugging: Special instruction for software breakpoint */ +#define KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x87) + #endif /* __LINUX_KVM_POWERPC_H */ diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c index 3d7fd21..41501be 100644 --- a/arch/powerpc/kvm/44x.c +++ b/arch/powerpc/kvm/44x.c @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu, return 0; } +u32 kvmppc_core_debug_inst_op(void) +{ + return -1; +} + void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs) { kvmppc_get_sregs_ivor(vcpu, sregs); diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index d2f502d..453a10f 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -1424,6 +1424,12 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) r = put_user(vcpu->arch.epcr, (u32 __user *)(long)reg->addr); break; #endif + case KVM_REG_PPC_DEBUG_INST: { + u32 opcode = kvmppc_core_debug_inst_op(); + r = copy_to_user((u32 __user *)(long)reg->addr, + &opcode, sizeof(u32)); + break; + } default: break; } @@ -1467,6 +1473,10 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) break; } #endif + case KVM_REG_PPC_DEBUG_INST: + /* This is read only, so write to this is nop*/ + r = 0; + break; default: break; } diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c index 6dd4de7..d8a5e8e 100644 --- a/arch/powerpc/kvm/e500.c +++ b/arch/powerpc/kvm/e500.c @@ -367,6 +367,11 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) return 0; } +u32 kvmppc_core_debug_inst_op(void) +{ + return KVMPPC_INST_GUEST_GDB; +} + void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs) { struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h index c70d37e..17942d2 100644 --- a/arch/powerpc/kvm/e500.h +++ b/arch/powerpc/kvm/e500.h @@ -302,4 +302,13 @@ static inline unsigned int get_tlbmiss_tid(struct kvm_vcpu *vcpu) #define get_tlb_sts(gtlbe) (MAS1_TS) #endif /* !BOOKE_HV */ +/* When setting software breakpoint, Change the software breakpoint + * instruction to special trap/invalid instruction and set + * KVM_GUESTDBG_USE_SW_BP flag in kvm_guest_debug->control. KVM does + * keep track of software breakpoints. So when KVM_GUESTDBG_USE_SW_BP + * flag is set and special trap instruction is executed by guest then + * exit to userspace. + */ +#define KVMPPC_INST_GUEST_GDB 0x7C00021C /* ehpriv OC=0 */ + #endif /* KVM_E500_H */ diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 1f89d26..dead142 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -199,6 +199,11 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) return 0; } +u32 kvmppc_core_debug_inst_op(void) +{ + return KVMPPC_INST_GUEST_GDB; +} + void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs) { struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
This patch adds the one_reg interface to get the special instruction to be used for setting software breakpoint from userspace. Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> --- Documentation/virtual/kvm/api.txt | 1 + arch/powerpc/include/asm/kvm_ppc.h | 1 + arch/powerpc/include/uapi/asm/kvm.h | 3 +++ arch/powerpc/kvm/44x.c | 5 +++++ arch/powerpc/kvm/booke.c | 10 ++++++++++ arch/powerpc/kvm/e500.c | 5 +++++ arch/powerpc/kvm/e500.h | 9 +++++++++ arch/powerpc/kvm/e500mc.c | 5 +++++ 8 files changed, 39 insertions(+), 0 deletions(-)