On 31.01.2013, at 15:05, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: kvm-ppc-owner@xxxxxxxxxxxxxxx [mailto:kvm-ppc-owner@xxxxxxxxxxxxxxx] On >> Behalf Of Alexander Graf >> Sent: Thursday, January 31, 2013 6:31 PM >> To: Bhushan Bharat-R65777 >> Cc: Paul Mackerras; kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx >> 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@xxxxxxx] >>>> Sent: Friday, January 25, 2013 5:24 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: Paul Mackerras; kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx >>>> 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@xxxxxxxxx] >>>>>> Sent: Thursday, January 17, 2013 12:53 PM >>>>>> To: Bhushan Bharat-R65777 >>>>>> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; agraf@xxxxxxx; >>>>>> 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" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
