Re: [Qemu-devel] [PATCH 4/5 v3][RESEND] ppc: Add software breakpoint support

[Alexander Graf <agraf@suse.de>]

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