diff mbox

[4/5,v3,RESEND] ppc: Add software breakpoint support

Message ID 1403611807-7082-5-git-send-email-Bharat.Bhushan@freescale.com
State New
Headers show

Commit Message

Bharat Bhushan June 24, 2014, 12:10 p.m. UTC
This patch allow insert/remove software breakpoint

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
 target-ppc/kvm.c | 71 +++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 57 insertions(+), 14 deletions(-)

Comments

Alexander Graf June 24, 2014, 1:04 p.m. UTC | #1
On 24.06.14 14:10, Bharat Bhushan wrote:
> This patch allow insert/remove software breakpoint
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> ---
>   target-ppc/kvm.c | 71 +++++++++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 57 insertions(+), 14 deletions(-)
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 5238de7..8e2dbb3 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1317,6 +1317,53 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env, uint32_t dcrn, uint32_t dat
>       return 0;
>   }
>   
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +{
> +    /* Mixed endian case is not handled */
> +    uint32_t sc = debug_inst_opcode;

What if debug_inst_opcode has never been set (thus is 0)? In that case 
we should fail the insert, no?


Alex
Bharat Bhushan June 24, 2014, 1:11 p.m. UTC | #2
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Tuesday, June 24, 2014 6:35 PM
> To: Bhushan Bharat-R65777
> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; maddy@linux.vnet.ibm.com
> Subject: Re: [PATCH 4/5 v3][RESEND] ppc: Add software breakpoint support
> 
> 
> On 24.06.14 14:10, Bharat Bhushan wrote:
> > This patch allow insert/remove software breakpoint
> >
> > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> > ---
> >   target-ppc/kvm.c | 71 +++++++++++++++++++++++++++++++++++++++++++++---------
> --
> >   1 file changed, 57 insertions(+), 14 deletions(-)
> >
> > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index
> > 5238de7..8e2dbb3 100644
> > --- a/target-ppc/kvm.c
> > +++ b/target-ppc/kvm.c
> > @@ -1317,6 +1317,53 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env,
> uint32_t dcrn, uint32_t dat
> >       return 0;
> >   }
> >
> > +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct
> > +kvm_sw_breakpoint *bp) {
> > +    /* Mixed endian case is not handled */
> > +    uint32_t sc = debug_inst_opcode;
> 
> What if debug_inst_opcode has never been set (thus is 0)?

Can "0" be a debug_inst_code ?

> In that case we should fail the insert, no?

Yes, will checking for "0" is sufficient or we need a cap_ also ?

Thanks
-Bharat

> 
> 
> Alex
Alexander Graf June 24, 2014, 1:20 p.m. UTC | #3
On 24.06.14 15:11, Bharat.Bhushan@freescale.com wrote:
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Tuesday, June 24, 2014 6:35 PM
>> To: Bhushan Bharat-R65777
>> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; maddy@linux.vnet.ibm.com
>> Subject: Re: [PATCH 4/5 v3][RESEND] ppc: Add software breakpoint support
>>
>>
>> On 24.06.14 14:10, Bharat Bhushan wrote:
>>> This patch allow insert/remove software breakpoint
>>>
>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
>>> ---
>>>    target-ppc/kvm.c | 71 +++++++++++++++++++++++++++++++++++++++++++++---------
>> --
>>>    1 file changed, 57 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index
>>> 5238de7..8e2dbb3 100644
>>> --- a/target-ppc/kvm.c
>>> +++ b/target-ppc/kvm.c
>>> @@ -1317,6 +1317,53 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env,
>> uint32_t dcrn, uint32_t dat
>>>        return 0;
>>>    }
>>>
>>> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct
>>> +kvm_sw_breakpoint *bp) {
>>> +    /* Mixed endian case is not handled */
>>> +    uint32_t sc = debug_inst_opcode;
>> What if debug_inst_opcode has never been set (thus is 0)?
> Can "0" be a debug_inst_code ?
>
>> In that case we should fail the insert, no?
> Yes, will checking for "0" is sufficient or we need a cap_ also ?

"0" can be a valid debug_inst_code, but I don't think it's a very smart 
one. Either way works for me.


Alex
maddy June 24, 2014, 3:28 p.m. UTC | #4
On Tuesday 24 June 2014 05:40 PM, Bharat Bhushan wrote:
> This patch allow insert/remove software breakpoint
> 
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> ---
>  target-ppc/kvm.c | 71 +++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 57 insertions(+), 14 deletions(-)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 5238de7..8e2dbb3 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1317,6 +1317,53 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env, uint32_t dcrn, uint32_t dat
>      return 0;
>  }
> 
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +{
> +    /* Mixed endian case is not handled */
> +    uint32_t sc = debug_inst_opcode;
> +
> +    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
> +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 1)) {

Instead of hard coding, can we use sizeof ()?

> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +{
> +    uint32_t sc;
> +
> +    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 0) ||
> +        sc != debug_inst_opcode ||
> +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) {
> +        return -EINVAL;
> +    }
> +

Same. Can we use sizeof?

> +    return 0;
> +}
> +
> +void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
> +{
> +    /* Software Breakpoint updates */
> +    if (kvm_sw_breakpoints_active(cs)) {
> +        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
> +    }
> +}
> +
> +static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run)
> +{
> +    CPUState *cs = CPU(cpu);
> +    struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
> +    int handle = 0;
> +
> +    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
> +        handle = 1;
> +    }
> +
> +    return handle;
> +}
> +
>  int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
>  {
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
> @@ -1357,6 +1404,16 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
>          ret = 0;
>          break;
> 
> +    case KVM_EXIT_DEBUG:
> +        DPRINTF("handle debug exception\n");
> +        if (kvm_handle_debug(cpu, run)) {
> +            ret = EXCP_DEBUG;
> +            break;
> +        }
> +        /* re-enter, this exception was guest-internal */

Kindly can you explain when this will happen?

> +        ret = 0;
> +        break;
> +
>      default:
>          fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
>          ret = -1;
> @@ -2044,16 +2101,6 @@ void kvm_arch_init_irq_routing(KVMState *s)
>  {
>  }
> 
> -int kvm_arch_insert_sw_breakpoint(CPUState *cpu, struct kvm_sw_breakpoint *bp)
> -{
> -    return -EINVAL;
> -}
> -
> -int kvm_arch_remove_sw_breakpoint(CPUState *cpu, struct kvm_sw_breakpoint *bp)
> -{
> -    return -EINVAL;
> -}
> -
>  int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong len, int type)
>  {
>      return -EINVAL;
> @@ -2068,10 +2115,6 @@ void kvm_arch_remove_all_hw_breakpoints(void)
>  {
>  }
> 
> -void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
> -{
> -}
> -
>  struct kvm_get_htab_buf {
>      struct kvm_get_htab_header header;
>      /*
>
Bharat Bhushan June 24, 2014, 5:06 p.m. UTC | #5
> -----Original Message-----
> From: Madhavan Srinivasan [mailto:maddy@linux.vnet.ibm.com]
> Sent: Tuesday, June 24, 2014 8:59 PM
> To: Bhushan Bharat-R65777; agraf@suse.de
> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
> Subject: Re: [PATCH 4/5 v3][RESEND] ppc: Add software breakpoint support
> 
> On Tuesday 24 June 2014 05:40 PM, Bharat Bhushan wrote:
> > This patch allow insert/remove software breakpoint
> >
> > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> > ---
> >  target-ppc/kvm.c | 71
> > +++++++++++++++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 57 insertions(+), 14 deletions(-)
> >
> > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index
> > 5238de7..8e2dbb3 100644
> > --- a/target-ppc/kvm.c
> > +++ b/target-ppc/kvm.c
> > @@ -1317,6 +1317,53 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env,
> uint32_t dcrn, uint32_t dat
> >      return 0;
> >  }
> >
> > +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct
> > +kvm_sw_breakpoint *bp) {
> > +    /* Mixed endian case is not handled */
> > +    uint32_t sc = debug_inst_opcode;
> > +
> > +    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
> > +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 1)) {
> 
> Instead of hard coding, can we use sizeof ()?

Yes

> 
> > +        return -EINVAL;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct
> > +kvm_sw_breakpoint *bp) {
> > +    uint32_t sc;
> > +
> > +    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 0) ||
> > +        sc != debug_inst_opcode ||
> > +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) {
> > +        return -EINVAL;
> > +    }
> > +
> 
> Same. Can we use sizeof?

Yes

> 
> > +    return 0;
> > +}
> > +
> > +void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug
> > +*dbg) {
> > +    /* Software Breakpoint updates */
> > +    if (kvm_sw_breakpoints_active(cs)) {
> > +        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
> > +    }
> > +}
> > +
> > +static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run) {
> > +    CPUState *cs = CPU(cpu);
> > +    struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
> > +    int handle = 0;
> > +
> > +    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
> > +        handle = 1;
> > +    }
> > +
> > +    return handle;
> > +}
> > +
> >  int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)  {
> >      PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -1357,6 +1404,16 @@ int
> > kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> >          ret = 0;
> >          break;
> >
> > +    case KVM_EXIT_DEBUG:
> > +        DPRINTF("handle debug exception\n");
> > +        if (kvm_handle_debug(cpu, run)) {
> > +            ret = EXCP_DEBUG;
> > +            break;
> > +        }
> > +        /* re-enter, this exception was guest-internal */
> 
> Kindly can you explain when this will happen?

If the debug interrupt condition (breakpoint/watchpoint etc) is not set by qemu, i.e that is set by guest. 

Thanks
-Bharat

> 
> > +        ret = 0;
> > +        break;
> > +
> >      default:
> >          fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
> >          ret = -1;
> > @@ -2044,16 +2101,6 @@ void kvm_arch_init_irq_routing(KVMState *s)  {
> > }
> >
> > -int kvm_arch_insert_sw_breakpoint(CPUState *cpu, struct
> > kvm_sw_breakpoint *bp) -{
> > -    return -EINVAL;
> > -}
> > -
> > -int kvm_arch_remove_sw_breakpoint(CPUState *cpu, struct
> > kvm_sw_breakpoint *bp) -{
> > -    return -EINVAL;
> > -}
> > -
> >  int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong
> > len, int type)  {
> >      return -EINVAL;
> > @@ -2068,10 +2115,6 @@ void kvm_arch_remove_all_hw_breakpoints(void)
> >  {
> >  }
> >
> > -void kvm_arch_update_guest_debug(CPUState *cpu, struct
> > kvm_guest_debug *dbg) -{ -}
> > -
> >  struct kvm_get_htab_buf {
> >      struct kvm_get_htab_header header;
> >      /*
> >
maddy June 24, 2014, 5:59 p.m. UTC | #6
On Tuesday 24 June 2014 10:36 PM, Bharat.Bhushan@freescale.com wrote:
> 
> 
>> -----Original Message-----
>> From: Madhavan Srinivasan [mailto:maddy@linux.vnet.ibm.com]
>> Sent: Tuesday, June 24, 2014 8:59 PM
>> To: Bhushan Bharat-R65777; agraf@suse.de
>> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
>> Subject: Re: [PATCH 4/5 v3][RESEND] ppc: Add software breakpoint support
>>
>> On Tuesday 24 June 2014 05:40 PM, Bharat Bhushan wrote:
>>> This patch allow insert/remove software breakpoint
>>>
>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
>>> ---
>>>  target-ppc/kvm.c | 71
>>> +++++++++++++++++++++++++++++++++++++++++++++-----------
>>>  1 file changed, 57 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index
>>> 5238de7..8e2dbb3 100644
>>> --- a/target-ppc/kvm.c
>>> +++ b/target-ppc/kvm.c
>>> @@ -1317,6 +1317,53 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env,
>> uint32_t dcrn, uint32_t dat
>>>      return 0;
>>>  }
>>>
>>> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct
>>> +kvm_sw_breakpoint *bp) {
>>> +    /* Mixed endian case is not handled */
>>> +    uint32_t sc = debug_inst_opcode;
>>> +
>>> +    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
>>> +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 1)) {
>>
>> Instead of hard coding, can we use sizeof ()?
> 
> Yes
> 
>>
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct
>>> +kvm_sw_breakpoint *bp) {
>>> +    uint32_t sc;
>>> +
>>> +    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 0) ||
>>> +        sc != debug_inst_opcode ||
>>> +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) {
>>> +        return -EINVAL;
>>> +    }
>>> +
>>
>> Same. Can we use sizeof?
> 
> Yes
> 
>>
>>> +    return 0;
>>> +}
>>> +
>>> +void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug
>>> +*dbg) {
>>> +    /* Software Breakpoint updates */
>>> +    if (kvm_sw_breakpoints_active(cs)) {
>>> +        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
>>> +    }
>>> +}
>>> +
>>> +static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run) {
>>> +    CPUState *cs = CPU(cpu);
>>> +    struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
>>> +    int handle = 0;
>>> +
>>> +    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
>>> +        handle = 1;
>>> +    }
>>> +
>>> +    return handle;
>>> +}
>>> +
>>>  int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)  {
>>>      PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -1357,6 +1404,16 @@ int
>>> kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
>>>          ret = 0;
>>>          break;
>>>
>>> +    case KVM_EXIT_DEBUG:
>>> +        DPRINTF("handle debug exception\n");
>>> +        if (kvm_handle_debug(cpu, run)) {
>>> +            ret = EXCP_DEBUG;
>>> +            break;
>>> +        }
>>> +        /* re-enter, this exception was guest-internal */
>>
>> Kindly can you explain when this will happen?
> 
> If the debug interrupt condition (breakpoint/watchpoint etc) is not set by qemu, i.e that is set by guest. 
> 

OK. This is my understanding. Kindly correct if it is wrong.
If we are here without any breakpoint from qemu, are we not suppose to
pass it on to guest with an interrupt inject?

Also, can we make it as if-else with single break for KVM_EXIT_DEBUG case.

Regards
Maddy

> Thanks
> -Bharat
> 
>>
>>> +        ret = 0;
>>> +        break;
>>> +
>>>      default:
>>>          fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
>>>          ret = -1;
>>> @@ -2044,16 +2101,6 @@ void kvm_arch_init_irq_routing(KVMState *s)  {
>>> }
>>>
>>> -int kvm_arch_insert_sw_breakpoint(CPUState *cpu, struct
>>> kvm_sw_breakpoint *bp) -{
>>> -    return -EINVAL;
>>> -}
>>> -
>>> -int kvm_arch_remove_sw_breakpoint(CPUState *cpu, struct
>>> kvm_sw_breakpoint *bp) -{
>>> -    return -EINVAL;
>>> -}
>>> -
>>>  int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong
>>> len, int type)  {
>>>      return -EINVAL;
>>> @@ -2068,10 +2115,6 @@ void kvm_arch_remove_all_hw_breakpoints(void)
>>>  {
>>>  }
>>>
>>> -void kvm_arch_update_guest_debug(CPUState *cpu, struct
>>> kvm_guest_debug *dbg) -{ -}
>>> -
>>>  struct kvm_get_htab_buf {
>>>      struct kvm_get_htab_header header;
>>>      /*
>>>
>
Alexander Graf June 24, 2014, 10:48 p.m. UTC | #7
On 24.06.14 19:59, Madhavan Srinivasan wrote:
> On Tuesday 24 June 2014 10:36 PM, Bharat.Bhushan@freescale.com wrote:
>>
>>> -----Original Message-----
>>> From: Madhavan Srinivasan [mailto:maddy@linux.vnet.ibm.com]
>>> Sent: Tuesday, June 24, 2014 8:59 PM
>>> To: Bhushan Bharat-R65777; agraf@suse.de
>>> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
>>> Subject: Re: [PATCH 4/5 v3][RESEND] ppc: Add software breakpoint support
>>>
>>> On Tuesday 24 June 2014 05:40 PM, Bharat Bhushan wrote:
>>>> This patch allow insert/remove software breakpoint
>>>>
>>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
>>>> ---
>>>>   target-ppc/kvm.c | 71
>>>> +++++++++++++++++++++++++++++++++++++++++++++-----------
>>>>   1 file changed, 57 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index
>>>> 5238de7..8e2dbb3 100644
>>>> --- a/target-ppc/kvm.c
>>>> +++ b/target-ppc/kvm.c
>>>> @@ -1317,6 +1317,53 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env,
>>> uint32_t dcrn, uint32_t dat
>>>>       return 0;
>>>>   }
>>>>
>>>> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct
>>>> +kvm_sw_breakpoint *bp) {
>>>> +    /* Mixed endian case is not handled */
>>>> +    uint32_t sc = debug_inst_opcode;
>>>> +
>>>> +    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
>>>> +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 1)) {
>>> Instead of hard coding, can we use sizeof ()?
>> Yes
>>
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct
>>>> +kvm_sw_breakpoint *bp) {
>>>> +    uint32_t sc;
>>>> +
>>>> +    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 0) ||
>>>> +        sc != debug_inst_opcode ||
>>>> +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) {
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>> Same. Can we use sizeof?
>> Yes
>>
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug
>>>> +*dbg) {
>>>> +    /* Software Breakpoint updates */
>>>> +    if (kvm_sw_breakpoints_active(cs)) {
>>>> +        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
>>>> +    }
>>>> +}
>>>> +
>>>> +static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run) {
>>>> +    CPUState *cs = CPU(cpu);
>>>> +    struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
>>>> +    int handle = 0;
>>>> +
>>>> +    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
>>>> +        handle = 1;
>>>> +    }
>>>> +
>>>> +    return handle;
>>>> +}
>>>> +
>>>>   int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)  {
>>>>       PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -1357,6 +1404,16 @@ int
>>>> kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
>>>>           ret = 0;
>>>>           break;
>>>>
>>>> +    case KVM_EXIT_DEBUG:
>>>> +        DPRINTF("handle debug exception\n");
>>>> +        if (kvm_handle_debug(cpu, run)) {
>>>> +            ret = EXCP_DEBUG;
>>>> +            break;
>>>> +        }
>>>> +        /* re-enter, this exception was guest-internal */
>>> Kindly can you explain when this will happen?
>> If the debug interrupt condition (breakpoint/watchpoint etc) is not set by qemu, i.e that is set by guest.
>>
> OK. This is my understanding. Kindly correct if it is wrong.
> If we are here without any breakpoint from qemu, are we not suppose to
> pass it on to guest with an interrupt inject?

Yes. If the guest issued that instruction itself we need to pass in the 
interrupt that the guest would have received. I think in the book3s case 
this would be a PROGRAM interrupt rather than a DEBUG interrupt.


Alex
diff mbox

Patch

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 5238de7..8e2dbb3 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1317,6 +1317,53 @@  static int kvmppc_handle_dcr_write(CPUPPCState *env, uint32_t dcrn, uint32_t dat
     return 0;
 }
 
+int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+{
+    /* Mixed endian case is not handled */
+    uint32_t sc = debug_inst_opcode;
+
+    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
+        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 1)) {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+{
+    uint32_t sc;
+
+    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 0) ||
+        sc != debug_inst_opcode ||
+        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
+{
+    /* Software Breakpoint updates */
+    if (kvm_sw_breakpoints_active(cs)) {
+        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
+    }
+}
+
+static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run)
+{
+    CPUState *cs = CPU(cpu);
+    struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
+    int handle = 0;
+
+    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
+        handle = 1;
+    }
+
+    return handle;
+}
+
 int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -1357,6 +1404,16 @@  int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
         ret = 0;
         break;
 
+    case KVM_EXIT_DEBUG:
+        DPRINTF("handle debug exception\n");
+        if (kvm_handle_debug(cpu, run)) {
+            ret = EXCP_DEBUG;
+            break;
+        }
+        /* re-enter, this exception was guest-internal */
+        ret = 0;
+        break;
+
     default:
         fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
         ret = -1;
@@ -2044,16 +2101,6 @@  void kvm_arch_init_irq_routing(KVMState *s)
 {
 }
 
-int kvm_arch_insert_sw_breakpoint(CPUState *cpu, struct kvm_sw_breakpoint *bp)
-{
-    return -EINVAL;
-}
-
-int kvm_arch_remove_sw_breakpoint(CPUState *cpu, struct kvm_sw_breakpoint *bp)
-{
-    return -EINVAL;
-}
-
 int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong len, int type)
 {
     return -EINVAL;
@@ -2068,10 +2115,6 @@  void kvm_arch_remove_all_hw_breakpoints(void)
 {
 }
 
-void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
-{
-}
-
 struct kvm_get_htab_buf {
     struct kvm_get_htab_header header;
     /*