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
