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 > --- > 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 softwa
RE: [PATCH 4/8] Added ONE_REG interface for debug instruction
> -Original Message- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Friday, January 25, 2013 5:18 PM > To: Bhushan Bharat-R65777 > Cc: kvm-...@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 > > --- > > 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; >
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:ag...@suse.de] >> Sent: Friday, January 25, 2013 5:18 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-...@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 >>> --- >>> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/8] Added ONE_REG interface for debug instruction
> -Original Message- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Thursday, January 31, 2013 11:23 PM > To: Bhushan Bharat-R65777 > Cc: kvm-...@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:ag...@suse.de] > >> Sent: Friday, January 25, 2013 5:18 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm-...@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 > >>> --- > >>> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] Added ONE_REG interface for debug instruction
On 31.01.2013, at 18:58, Bhushan Bharat-R65777 wrote: > > >> -Original Message- >> From: Alexander Graf [mailto:ag...@suse.de] >> Sent: Thursday, January 31, 2013 11:23 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-...@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:ag...@suse.de] >>>> Sent: Friday, January 25, 2013 5:18 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm-...@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 >>>>> --- >>>>> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] Added ONE_REG interface for debug instruction
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] Added ONE_REG interface for debug instruction
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] Added ONE_REG interface for debug instruction
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html