Patchwork [5/8] KVM: PPC: debug stub interface parameter defined

login
register
mail settings
Submitter Bharat Bhushan
Date Jan. 16, 2013, 8:24 a.m.
Message ID <1358324685-30225-4-git-send-email-bharat.bhushan@freescale.com>
Download mbox | patch
Permalink /patch/212433/
State New
Headers show

Comments

Bharat Bhushan - Jan. 16, 2013, 8:24 a.m.
This patch defines the interface parameter for KVM_SET_GUEST_DEBUG
ioctl support. Follow up patches will use this for setting up
hardware breakpoints, watchpoints and software breakpoints.

Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
 arch/powerpc/include/uapi/asm/kvm.h |   23 +++++++++++++++++++++++
 arch/powerpc/kvm/booke.c            |    6 ++++++
 arch/powerpc/kvm/powerpc.c          |    6 ------
 3 files changed, 29 insertions(+), 6 deletions(-)
Paul Mackerras - Jan. 17, 2013, 7:22 a.m.
On Wed, Jan 16, 2013 at 01:54:42PM +0530, Bharat Bhushan wrote:
> This patch defines the interface parameter for KVM_SET_GUEST_DEBUG
> ioctl support. Follow up patches will use this for setting up
> hardware breakpoints, watchpoints and software breakpoints.

[snip]

> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 453a10f..7d5a51c 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -1483,6 +1483,12 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
>  	return r;
>  }
>  
> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> +					 struct kvm_guest_debug *dbg)
> +{
> +	return -EINVAL;
> +}
> +
>  int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
>  {
>  	return -ENOTSUPP;
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 934413c..4c94ca9 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -532,12 +532,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>  #endif
>  }
>  
> -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> -                                        struct kvm_guest_debug *dbg)
> -{
> -	return -EINVAL;
> -}
> -

This will break the build for non-book E machines, since
kvm_arch_vcpu_ioctl_set_guest_debug() is referenced from generic code.
You need to add it to arch/powerpc/kvm/book3s.c as well.

Paul.
--
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
Bharat Bhushan - Jan. 17, 2013, 11:11 a.m.
> -----Original Message-----
> From: Paul Mackerras [mailto:paulus@samba.org]
> Sent: Thursday, January 17, 2013 12:53 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; Bhushan Bharat-
> R65777
> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter defined
> 
> On Wed, Jan 16, 2013 at 01:54:42PM +0530, Bharat Bhushan wrote:
> > This patch defines the interface parameter for KVM_SET_GUEST_DEBUG
> > ioctl support. Follow up patches will use this for setting up hardware
> > breakpoints, watchpoints and software breakpoints.
> 
> [snip]
> 
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
> > 453a10f..7d5a51c 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -1483,6 +1483,12 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
> struct kvm_one_reg *reg)
> >  	return r;
> >  }
> >
> > +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> > +					 struct kvm_guest_debug *dbg)
> > +{
> > +	return -EINVAL;
> > +}
> > +
> >  int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu
> > *fpu)  {
> >  	return -ENOTSUPP;
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index 934413c..4c94ca9 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -532,12 +532,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> > #endif  }
> >
> > -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> > -                                        struct kvm_guest_debug *dbg)
> > -{
> > -	return -EINVAL;
> > -}
> > -
> 
> This will break the build for non-book E machines, since
> kvm_arch_vcpu_ioctl_set_guest_debug() is referenced from generic code.
> You need to add it to arch/powerpc/kvm/book3s.c as well.

right,  I will correct this.

Thanks
-Bharat


--
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
Alexander Graf - Jan. 25, 2013, 11:53 a.m.
On 17.01.2013, at 12:11, Bhushan Bharat-R65777 wrote:

> 
> 
>> -----Original Message-----
>> From: Paul Mackerras [mailto:paulus@samba.org]
>> Sent: Thursday, January 17, 2013 12:53 PM
>> To: Bhushan Bharat-R65777
>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; Bhushan Bharat-
>> R65777
>> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter defined
>> 
>> On Wed, Jan 16, 2013 at 01:54:42PM +0530, Bharat Bhushan wrote:
>>> This patch defines the interface parameter for KVM_SET_GUEST_DEBUG
>>> ioctl support. Follow up patches will use this for setting up hardware
>>> breakpoints, watchpoints and software breakpoints.
>> 
>> [snip]
>> 
>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
>>> 453a10f..7d5a51c 100644
>>> --- a/arch/powerpc/kvm/booke.c
>>> +++ b/arch/powerpc/kvm/booke.c
>>> @@ -1483,6 +1483,12 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
>> struct kvm_one_reg *reg)
>>> 	return r;
>>> }
>>> 
>>> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>> +					 struct kvm_guest_debug *dbg)
>>> +{
>>> +	return -EINVAL;
>>> +}
>>> +
>>> int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu
>>> *fpu)  {
>>> 	return -ENOTSUPP;
>>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>>> index 934413c..4c94ca9 100644
>>> --- a/arch/powerpc/kvm/powerpc.c
>>> +++ b/arch/powerpc/kvm/powerpc.c
>>> @@ -532,12 +532,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>> #endif  }
>>> 
>>> -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>> -                                        struct kvm_guest_debug *dbg)
>>> -{
>>> -	return -EINVAL;
>>> -}
>>> -
>> 
>> This will break the build for non-book E machines, since
>> kvm_arch_vcpu_ioctl_set_guest_debug() is referenced from generic code.
>> You need to add it to arch/powerpc/kvm/book3s.c as well.
> 
> right,  I will correct this.

Would the implementation actually be different on booke vs book3s? My feeling is that powerpc.c is actually the right place for this.


Alex

--
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
Bharat Bhushan - Jan. 30, 2013, 2:15 p.m.
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Friday, January 25, 2013 5:24 PM
> To: Bhushan Bharat-R65777
> Cc: Paul Mackerras; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter defined
> 
> 
> On 17.01.2013, at 12:11, Bhushan Bharat-R65777 wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: Paul Mackerras [mailto:paulus@samba.org]
> >> Sent: Thursday, January 17, 2013 12:53 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de;
> >> Bhushan Bharat-
> >> R65777
> >> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter
> >> defined
> >>
> >> On Wed, Jan 16, 2013 at 01:54:42PM +0530, Bharat Bhushan wrote:
> >>> This patch defines the interface parameter for KVM_SET_GUEST_DEBUG
> >>> ioctl support. Follow up patches will use this for setting up
> >>> hardware breakpoints, watchpoints and software breakpoints.
> >>
> >> [snip]
> >>
> >>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> >>> index 453a10f..7d5a51c 100644
> >>> --- a/arch/powerpc/kvm/booke.c
> >>> +++ b/arch/powerpc/kvm/booke.c
> >>> @@ -1483,6 +1483,12 @@ int kvm_vcpu_ioctl_set_one_reg(struct
> >>> kvm_vcpu *vcpu,
> >> struct kvm_one_reg *reg)
> >>> 	return r;
> >>> }
> >>>
> >>> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> >>> +					 struct kvm_guest_debug *dbg)
> >>> +{
> >>> +	return -EINVAL;
> >>> +}
> >>> +
> >>> int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct
> >>> kvm_fpu
> >>> *fpu)  {
> >>> 	return -ENOTSUPP;
> >>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> >>> index 934413c..4c94ca9 100644
> >>> --- a/arch/powerpc/kvm/powerpc.c
> >>> +++ b/arch/powerpc/kvm/powerpc.c
> >>> @@ -532,12 +532,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> >>> #endif  }
> >>>
> >>> -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> >>> -                                        struct kvm_guest_debug *dbg)
> >>> -{
> >>> -	return -EINVAL;
> >>> -}
> >>> -
> >>
> >> This will break the build for non-book E machines, since
> >> kvm_arch_vcpu_ioctl_set_guest_debug() is referenced from generic code.
> >> You need to add it to arch/powerpc/kvm/book3s.c as well.
> >
> > right,  I will correct this.
> 
> Would the implementation actually be different on booke vs book3s? My feeling is
> that powerpc.c is actually the right place for this.
> 

I am not sure there will be anything common between book3s and booke. Should we define the cpu specific function something like kvm_ppc_vcpu_ioctl_set_guest_debug() for booke and book3s and call this new defined function from kvm_arch_vcpu_ioctl_set_guest_debug() in powerpc.c ?

Thanks
-Bharat



--
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
Alexander Graf - Jan. 31, 2013, 1:01 p.m.
On 30.01.2013, at 15:15, Bhushan Bharat-R65777 wrote:

> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Friday, January 25, 2013 5:24 PM
>> To: Bhushan Bharat-R65777
>> Cc: Paul Mackerras; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter defined
>> 
>> 
>> On 17.01.2013, at 12:11, Bhushan Bharat-R65777 wrote:
>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: Paul Mackerras [mailto:paulus@samba.org]
>>>> Sent: Thursday, January 17, 2013 12:53 PM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de;
>>>> Bhushan Bharat-
>>>> R65777
>>>> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter
>>>> defined
>>>> 
>>>> On Wed, Jan 16, 2013 at 01:54:42PM +0530, Bharat Bhushan wrote:
>>>>> This patch defines the interface parameter for KVM_SET_GUEST_DEBUG
>>>>> ioctl support. Follow up patches will use this for setting up
>>>>> hardware breakpoints, watchpoints and software breakpoints.
>>>> 
>>>> [snip]
>>>> 
>>>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
>>>>> index 453a10f..7d5a51c 100644
>>>>> --- a/arch/powerpc/kvm/booke.c
>>>>> +++ b/arch/powerpc/kvm/booke.c
>>>>> @@ -1483,6 +1483,12 @@ int kvm_vcpu_ioctl_set_one_reg(struct
>>>>> kvm_vcpu *vcpu,
>>>> struct kvm_one_reg *reg)
>>>>> 	return r;
>>>>> }
>>>>> 
>>>>> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>>>> +					 struct kvm_guest_debug *dbg)
>>>>> +{
>>>>> +	return -EINVAL;
>>>>> +}
>>>>> +
>>>>> int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct
>>>>> kvm_fpu
>>>>> *fpu)  {
>>>>> 	return -ENOTSUPP;
>>>>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>>>>> index 934413c..4c94ca9 100644
>>>>> --- a/arch/powerpc/kvm/powerpc.c
>>>>> +++ b/arch/powerpc/kvm/powerpc.c
>>>>> @@ -532,12 +532,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>>>> #endif  }
>>>>> 
>>>>> -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>>>> -                                        struct kvm_guest_debug *dbg)
>>>>> -{
>>>>> -	return -EINVAL;
>>>>> -}
>>>>> -
>>>> 
>>>> This will break the build for non-book E machines, since
>>>> kvm_arch_vcpu_ioctl_set_guest_debug() is referenced from generic code.
>>>> You need to add it to arch/powerpc/kvm/book3s.c as well.
>>> 
>>> right,  I will correct this.
>> 
>> Would the implementation actually be different on booke vs book3s? My feeling is
>> that powerpc.c is actually the right place for this.
>> 
> 
> I am not sure there will be anything common between book3s and booke. Should we define the cpu specific function something like kvm_ppc_vcpu_ioctl_set_guest_debug() for booke and book3s and call this new defined function from kvm_arch_vcpu_ioctl_set_guest_debug() in powerpc.c ?

No, just put it into the subarch directories then :). No need to overengineer anything for now.


Alex

--
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
Bharat Bhushan - Jan. 31, 2013, 2:05 p.m.
> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On
> Behalf Of Alexander Graf
> Sent: Thursday, January 31, 2013 6:31 PM
> To: Bhushan Bharat-R65777
> Cc: Paul Mackerras; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter defined
> 
> 
> On 30.01.2013, at 15:15, Bhushan Bharat-R65777 wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:agraf@suse.de]
> >> Sent: Friday, January 25, 2013 5:24 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: Paul Mackerras; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> >> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter
> >> defined
> >>
> >>
> >> On 17.01.2013, at 12:11, Bhushan Bharat-R65777 wrote:
> >>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Paul Mackerras [mailto:paulus@samba.org]
> >>>> Sent: Thursday, January 17, 2013 12:53 PM
> >>>> To: Bhushan Bharat-R65777
> >>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de;
> >>>> Bhushan Bharat-
> >>>> R65777
> >>>> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter
> >>>> defined
> >>>>
> >>>> On Wed, Jan 16, 2013 at 01:54:42PM +0530, Bharat Bhushan wrote:
> >>>>> This patch defines the interface parameter for KVM_SET_GUEST_DEBUG
> >>>>> ioctl support. Follow up patches will use this for setting up
> >>>>> hardware breakpoints, watchpoints and software breakpoints.
> >>>>
> >>>> [snip]
> >>>>
> >>>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> >>>>> index 453a10f..7d5a51c 100644
> >>>>> --- a/arch/powerpc/kvm/booke.c
> >>>>> +++ b/arch/powerpc/kvm/booke.c
> >>>>> @@ -1483,6 +1483,12 @@ int kvm_vcpu_ioctl_set_one_reg(struct
> >>>>> kvm_vcpu *vcpu,
> >>>> struct kvm_one_reg *reg)
> >>>>> 	return r;
> >>>>> }
> >>>>>
> >>>>> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> >>>>> +					 struct kvm_guest_debug *dbg) {
> >>>>> +	return -EINVAL;
> >>>>> +}
> >>>>> +
> >>>>> int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct
> >>>>> kvm_fpu
> >>>>> *fpu)  {
> >>>>> 	return -ENOTSUPP;
> >>>>> diff --git a/arch/powerpc/kvm/powerpc.c
> >>>>> b/arch/powerpc/kvm/powerpc.c index 934413c..4c94ca9 100644
> >>>>> --- a/arch/powerpc/kvm/powerpc.c
> >>>>> +++ b/arch/powerpc/kvm/powerpc.c
> >>>>> @@ -532,12 +532,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> >>>>> #endif  }
> >>>>>
> >>>>> -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> >>>>> -                                        struct kvm_guest_debug *dbg)
> >>>>> -{
> >>>>> -	return -EINVAL;
> >>>>> -}
> >>>>> -
> >>>>
> >>>> This will break the build for non-book E machines, since
> >>>> kvm_arch_vcpu_ioctl_set_guest_debug() is referenced from generic code.
> >>>> You need to add it to arch/powerpc/kvm/book3s.c as well.
> >>>
> >>> right,  I will correct this.
> >>
> >> Would the implementation actually be different on booke vs book3s? My
> >> feeling is that powerpc.c is actually the right place for this.
> >>
> >
> > I am not sure there will be anything common between book3s and booke. Should
> we define the cpu specific function something like
> kvm_ppc_vcpu_ioctl_set_guest_debug() for booke and book3s and call this new
> defined function from kvm_arch_vcpu_ioctl_set_guest_debug() in powerpc.c ?
> 
> No, just put it into the subarch directories then :). No need to overengineer
> anything for now.

What you mean by subarch?  Above you mentioned that powerpc.c is right place? 
Is not this patch is doing partially :)

Thanks
-Bharat


--
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
Alexander Graf - Jan. 31, 2013, 2:27 p.m.
On 31.01.2013, at 15:05, Bhushan Bharat-R65777 wrote:

> 
> 
>> -----Original Message-----
>> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On
>> Behalf Of Alexander Graf
>> Sent: Thursday, January 31, 2013 6:31 PM
>> To: Bhushan Bharat-R65777
>> Cc: Paul Mackerras; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter defined
>> 
>> 
>> On 30.01.2013, at 15:15, Bhushan Bharat-R65777 wrote:
>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>> Sent: Friday, January 25, 2013 5:24 PM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: Paul Mackerras; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>>>> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter
>>>> defined
>>>> 
>>>> 
>>>> On 17.01.2013, at 12:11, Bhushan Bharat-R65777 wrote:
>>>> 
>>>>> 
>>>>> 
>>>>>> -----Original Message-----
>>>>>> From: Paul Mackerras [mailto:paulus@samba.org]
>>>>>> Sent: Thursday, January 17, 2013 12:53 PM
>>>>>> To: Bhushan Bharat-R65777
>>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de;
>>>>>> Bhushan Bharat-
>>>>>> R65777
>>>>>> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter
>>>>>> defined
>>>>>> 
>>>>>> On Wed, Jan 16, 2013 at 01:54:42PM +0530, Bharat Bhushan wrote:
>>>>>>> This patch defines the interface parameter for KVM_SET_GUEST_DEBUG
>>>>>>> ioctl support. Follow up patches will use this for setting up
>>>>>>> hardware breakpoints, watchpoints and software breakpoints.
>>>>>> 
>>>>>> [snip]
>>>>>> 
>>>>>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
>>>>>>> index 453a10f..7d5a51c 100644
>>>>>>> --- a/arch/powerpc/kvm/booke.c
>>>>>>> +++ b/arch/powerpc/kvm/booke.c
>>>>>>> @@ -1483,6 +1483,12 @@ int kvm_vcpu_ioctl_set_one_reg(struct
>>>>>>> kvm_vcpu *vcpu,
>>>>>> struct kvm_one_reg *reg)
>>>>>>> 	return r;
>>>>>>> }
>>>>>>> 
>>>>>>> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>>>>>> +					 struct kvm_guest_debug *dbg) {
>>>>>>> +	return -EINVAL;
>>>>>>> +}
>>>>>>> +
>>>>>>> int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct
>>>>>>> kvm_fpu
>>>>>>> *fpu)  {
>>>>>>> 	return -ENOTSUPP;
>>>>>>> diff --git a/arch/powerpc/kvm/powerpc.c
>>>>>>> b/arch/powerpc/kvm/powerpc.c index 934413c..4c94ca9 100644
>>>>>>> --- a/arch/powerpc/kvm/powerpc.c
>>>>>>> +++ b/arch/powerpc/kvm/powerpc.c
>>>>>>> @@ -532,12 +532,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>>>>>> #endif  }
>>>>>>> 
>>>>>>> -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>>>>>> -                                        struct kvm_guest_debug *dbg)
>>>>>>> -{
>>>>>>> -	return -EINVAL;
>>>>>>> -}
>>>>>>> -
>>>>>> 
>>>>>> This will break the build for non-book E machines, since
>>>>>> kvm_arch_vcpu_ioctl_set_guest_debug() is referenced from generic code.
>>>>>> You need to add it to arch/powerpc/kvm/book3s.c as well.
>>>>> 
>>>>> right,  I will correct this.
>>>> 
>>>> Would the implementation actually be different on booke vs book3s? My
>>>> feeling is that powerpc.c is actually the right place for this.
>>>> 
>>> 
>>> I am not sure there will be anything common between book3s and booke. Should
>> we define the cpu specific function something like
>> kvm_ppc_vcpu_ioctl_set_guest_debug() for booke and book3s and call this new
>> defined function from kvm_arch_vcpu_ioctl_set_guest_debug() in powerpc.c ?
>> 
>> No, just put it into the subarch directories then :). No need to overengineer
>> anything for now.
> 
> What you mean by subarch?  Above you mentioned that powerpc.c is right place? 
> Is not this patch is doing partially :)

If the code in powerpc.c only says

void kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) {
    kvmppc_core_set_guest_debug(vcpu, dbg);
}

then doing it in powerpc.c is obviously moot. Since there is no other debug implementation, it's ok if we try and find (and create) commonalities later. So yes, it's ok if you put it into booke.c or even e500.c. Just make sure to not break any other archs (440, book3s_pr, book3s_hv).


Alex

--
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
Bharat Bhushan - Jan. 31, 2013, 2:44 p.m.
> -----Original Message-----
> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On Behalf Of
> Alexander Graf
> Sent: Thursday, January 31, 2013 7:58 PM
> To: Bhushan Bharat-R65777
> Cc: Paul Mackerras; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter defined
> 
> 
> On 31.01.2013, at 15:05, Bhushan Bharat-R65777 wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: kvm-ppc-owner@vger.kernel.org
> >> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf
> >> Sent: Thursday, January 31, 2013 6:31 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: Paul Mackerras; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> >> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter
> >> defined
> >>
> >>
> >> On 30.01.2013, at 15:15, Bhushan Bharat-R65777 wrote:
> >>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Alexander Graf [mailto:agraf@suse.de]
> >>>> Sent: Friday, January 25, 2013 5:24 PM
> >>>> To: Bhushan Bharat-R65777
> >>>> Cc: Paul Mackerras; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> >>>> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter
> >>>> defined
> >>>>
> >>>>
> >>>> On 17.01.2013, at 12:11, Bhushan Bharat-R65777 wrote:
> >>>>
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Paul Mackerras [mailto:paulus@samba.org]
> >>>>>> Sent: Thursday, January 17, 2013 12:53 PM
> >>>>>> To: Bhushan Bharat-R65777
> >>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de;
> >>>>>> Bhushan Bharat-
> >>>>>> R65777
> >>>>>> Subject: Re: [PATCH 5/8] KVM: PPC: debug stub interface parameter
> >>>>>> defined
> >>>>>>
> >>>>>> On Wed, Jan 16, 2013 at 01:54:42PM +0530, Bharat Bhushan wrote:
> >>>>>>> This patch defines the interface parameter for
> >>>>>>> KVM_SET_GUEST_DEBUG ioctl support. Follow up patches will use
> >>>>>>> this for setting up hardware breakpoints, watchpoints and software
> breakpoints.
> >>>>>>
> >>>>>> [snip]
> >>>>>>
> >>>>>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> >>>>>>> index 453a10f..7d5a51c 100644
> >>>>>>> --- a/arch/powerpc/kvm/booke.c
> >>>>>>> +++ b/arch/powerpc/kvm/booke.c
> >>>>>>> @@ -1483,6 +1483,12 @@ int kvm_vcpu_ioctl_set_one_reg(struct
> >>>>>>> kvm_vcpu *vcpu,
> >>>>>> struct kvm_one_reg *reg)
> >>>>>>> 	return r;
> >>>>>>> }
> >>>>>>>
> >>>>>>> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> >>>>>>> +					 struct kvm_guest_debug *dbg) {
> >>>>>>> +	return -EINVAL;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct
> >>>>>>> kvm_fpu
> >>>>>>> *fpu)  {
> >>>>>>> 	return -ENOTSUPP;
> >>>>>>> diff --git a/arch/powerpc/kvm/powerpc.c
> >>>>>>> b/arch/powerpc/kvm/powerpc.c index 934413c..4c94ca9 100644
> >>>>>>> --- a/arch/powerpc/kvm/powerpc.c
> >>>>>>> +++ b/arch/powerpc/kvm/powerpc.c
> >>>>>>> @@ -532,12 +532,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu
> >>>>>>> *vcpu) #endif  }
> >>>>>>>
> >>>>>>> -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> >>>>>>> -                                        struct kvm_guest_debug *dbg)
> >>>>>>> -{
> >>>>>>> -	return -EINVAL;
> >>>>>>> -}
> >>>>>>> -
> >>>>>>
> >>>>>> This will break the build for non-book E machines, since
> >>>>>> kvm_arch_vcpu_ioctl_set_guest_debug() is referenced from generic code.
> >>>>>> You need to add it to arch/powerpc/kvm/book3s.c as well.
> >>>>>
> >>>>> right,  I will correct this.
> >>>>
> >>>> Would the implementation actually be different on booke vs book3s?
> >>>> My feeling is that powerpc.c is actually the right place for this.
> >>>>
> >>>
> >>> I am not sure there will be anything common between book3s and
> >>> booke. Should
> >> we define the cpu specific function something like
> >> kvm_ppc_vcpu_ioctl_set_guest_debug() for booke and book3s and call
> >> this new defined function from kvm_arch_vcpu_ioctl_set_guest_debug() in
> powerpc.c ?
> >>
> >> No, just put it into the subarch directories then :). No need to
> >> overengineer anything for now.
> >
> > What you mean by subarch?  Above you mentioned that powerpc.c is right place?
> > Is not this patch is doing partially :)
> 
> If the code in powerpc.c only says
> 
> void kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, struct
> kvm_guest_debug *dbg) {
>     kvmppc_core_set_guest_debug(vcpu, dbg); }
> 
> then doing it in powerpc.c is obviously moot. Since there is no other debug
> implementation, it's ok if we try and find (and create) commonalities later.
> So
> yes, it's ok if you put it into booke.c or even e500.c. Just make sure to not
> break any other archs (440, book3s_pr, book3s_hv).

Right, yes I will correct that it compiles for all archs.

Thanks.
-Bharat

> 
> 
> 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


--
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

Patch

diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
index e81ae5b..e8842ed 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -272,8 +272,31 @@  struct kvm_debug_exit_arch {
 
 /* for KVM_SET_GUEST_DEBUG */
 struct kvm_guest_debug_arch {
+	struct {
+		/* H/W breakpoint/watchpoint address */
+		__u64 addr;
+		/*
+		 * Type denotes h/w breakpoint, read watchpoint, write
+		 * watchpoint or watchpoint (both read and write).
+		 */
+#define KVMPPC_DEBUG_NOTYPE		0x0
+#define KVMPPC_DEBUG_BREAKPOINT		(1UL << 1)
+#define KVMPPC_DEBUG_WATCH_WRITE	(1UL << 2)
+#define KVMPPC_DEBUG_WATCH_READ		(1UL << 3)
+		__u32 type;
+		__u32 reserved;
+	} bp[16];
 };
 
+/* Debug related defines */
+/*
+ * kvm_guest_debug->control is a 32 bit field. The lower 16 bits are generic
+ * and upper 16 bits are architecture specific. Architecture specific defines
+ * that ioctl is for setting hardware breakpoint or software breakpoint.
+ */
+#define KVM_GUESTDBG_USE_SW_BP		0x00010000
+#define KVM_GUESTDBG_USE_HW_BP		0x00020000
+
 /* definition of registers in kvm_run */
 struct kvm_sync_regs {
 };
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 453a10f..7d5a51c 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1483,6 +1483,12 @@  int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
 	return r;
 }
 
+int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
+					 struct kvm_guest_debug *dbg)
+{
+	return -EINVAL;
+}
+
 int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
 {
 	return -ENOTSUPP;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 934413c..4c94ca9 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -532,12 +532,6 @@  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 #endif
 }
 
-int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
-                                        struct kvm_guest_debug *dbg)
-{
-	return -EINVAL;
-}
-
 static void kvmppc_complete_dcr_load(struct kvm_vcpu *vcpu,
                                      struct kvm_run *run)
 {