Re: [Qemu-devel] [PATCH 3/3 v2] ppc debug: Add debug stub support

[Alexander Graf <agraf@suse.de>]

On 24.06.14 13:32, Bharat.Bhushan@freescale.com wrote:
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Tuesday, June 24, 2014 5:02 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 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 <Bharat.Bhushan@freescale.com>
>>>>>>>>> ---
>>>>>>>>> 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.
> V3 version of path is based on this :)

Uh, I don't see a v3 yet? :)


Alex