From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59445) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzOww-0006O0-QR for qemu-devel@nongnu.org; Tue, 24 Jun 2014 07:31:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WzOwq-0005DL-QQ for qemu-devel@nongnu.org; Tue, 24 Jun 2014 07:31:42 -0400 Message-ID: <53A96196.4000408@suse.de> Date: Tue, 24 Jun 2014 13:31:34 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1402988887-30418-1-git-send-email-Bharat.Bhushan@freescale.com> <1402988887-30418-4-git-send-email-Bharat.Bhushan@freescale.com> <539FF926.4020300@suse.de> <53A00F35.6000909@suse.de> <53A01BD9.50207@suse.de> <6d8d17beba1245569435295cdb6e2b13@DM2PR03MB574.namprd03.prod.outlook.com> In-Reply-To: <6d8d17beba1245569435295cdb6e2b13@DM2PR03MB574.namprd03.prod.outlook.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/3 v2] ppc debug: Add debug stub support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Bharat.Bhushan@freescale.com" , "qemu-ppc@nongnu.org" , "qemu-devel@nongnu.org" On 18.06.14 06:39, Bharat.Bhushan@freescale.com wrote: > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Tuesday, June 17, 2014 4:14 PM >> To: Bhushan Bharat-R65777; qemu-ppc@nongnu.org; qemu-devel@nongnu.org >> Subject: Re: [PATCH 3/3 v2] ppc debug: Add debug stub support >> >> >> On 17.06.14 12:40, Bharat.Bhushan@freescale.com wrote: >>>> -----Original Message----- >>>> From: Alexander Graf [mailto:agraf@suse.de] >>>> Sent: Tuesday, June 17, 2014 3:20 PM >>>> To: Bhushan Bharat-R65777; qemu-ppc@nongnu.org; qemu-devel@nongnu.org >>>> Subject: Re: [PATCH 3/3 v2] ppc debug: Add debug stub support >>>> >>>> >>>> On 17.06.14 11:14, Bharat.Bhushan@freescale.com wrote: >>>>>> -----Original Message----- >>>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>>> Sent: Tuesday, June 17, 2014 1:46 PM >>>>>> To: Bhushan Bharat-R65777; qemu-ppc@nongnu.org; >>>>>> qemu-devel@nongnu.org >>>>>> Subject: Re: [PATCH 3/3 v2] ppc debug: Add debug stub support >>>>>> >>>>>> >>>>>> On 17.06.14 09:08, Bharat Bhushan wrote: >>>>>>> This patch adds software breakpoint, hardware breakpoint and >>>>>>> hardware watchpoint support for ppc. If the debug interrupt is not >>>>>>> handled then this is injected to guest. >>>>>>> >>>>>>> Signed-off-by: Bharat Bhushan >>>>>>> --- >>>>>>> v1->v2: >>>>>>> - factored out e500 specific code based on exception model >>>>>> POWERPC_EXCP_BOOKE. >>>>>>> - Not supporting ppc440 >>>>>>> >>>>>>> hw/ppc/e500.c | 3 + >>>>>>> target-ppc/kvm.c | 355 >>>> ++++++++++++++++++++++++++++++++++++++++++++++--- >>>>>> -- >>>>>>> target-ppc/kvm_ppc.h | 1 + >>>>>>> 3 files changed, 330 insertions(+), 29 deletions(-) >>>>>>> >>>>>>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index a973c18..47caa84 >>>>>>> 100644 >>>>>>> --- a/hw/ppc/e500.c >>>>>>> +++ b/hw/ppc/e500.c >>>>>>> @@ -853,6 +853,9 @@ void ppce500_init(MachineState *machine, >>>>>>> PPCE500Params >>>>>> *params) >>>>>>> if (kvm_enabled()) { >>>>>>> kvmppc_init(); >>>>>>> } >>>>>>> + >>>>>>> + /* E500 supports 2 h/w breakpoints and 2 watchpoints */ >>>>>>> + kvmppc_hw_breakpoint_init(2, 2); >>>>>> This does not belong into the machine file. >>>>> What about calling this from init_proc_e500() in target-ppc/translate_init.c >> ? >>>> I think it makes sense to leave it in KVM land. Why not do it lazily >>>> on insert_hw_breakpoint? >>> You mean setting in kvm_arch_insert_hw_breakpoint() when called first time; >> something like: >>> static bool init = 0; >>> >>> if (!init) { >>> if (env->excp_model == POWERPC_EXCP_BOOKE) { >>> max_hw_breakpoint = 2; >>> max_hw_watchpoint = 2; >>> } else >>> // Add for book3s max_hw_watchpoint = 1; >>> } >>> init = 1; >>> } >> I would probably reuse max_hw_breakpoint as a hint whether it's initialized and >> put all of this into a small function, but yes :). > Ahh, we cannot do this in kvm_arch_insert_hw_breakpoint() as we can not get "env" reference in this function. Prototype of this is: > int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong len, int type); Just use first_cpu? :) > > I will suggest that we initialize this from kvm_arch_init_vcpu(). This way we are still in KVM zone. That works too, yes. Alex