Re: [Qemu-ppc] [PATCH 3/4 v7] ppc: Add software breakpoint support

[Madhavan Srinivasan]

On Thursday 10 July 2014 07:49 PM, Bharat Bhushan wrote:
> This patch allow insert/remove software breakpoint.
> 
> When QEMU is not able to handle debug exception then we inject program
> exception to guest because for software breakpoint QEMU uses a ehpriv-1
> instruction;
> So there cannot be any reason that we are in qemu with exit reason
> KVM_EXIT_DEBUG  for guest set debug exception, only possibility is
> guest executed ehpriv-1 privilege instruction and that's why we are
> injecting program exception.
> 
> Signed-off-by: Bharat Bhushan <address@hidden>
> ---
> v6->v7
>  - Moved exception injection to this patch
>  - Inject the fault directly
> 
>  target-ppc/kvm.c | 87 
> +++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 73 insertions(+), 14 deletions(-)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index e00a20f..afa2291 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1275,6 +1275,69 @@ 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,
> +                            sizeof(sc), 0) ||
> +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, sizeof(sc), 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, sizeof(sc), 0) ||
> +        sc != debug_inst_opcode ||
> +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
> +                            sizeof(sc), 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);
> +    CPUPPCState *env = &cpu->env;
> +    struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
> +    int handle = 0;
> +
> +    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
> +        handle = 1;
> +    } else {
> +        /* QEMU is not able to handle debug exception, so inject
> +         * program exception to guest;
> +         * Yes program exception NOT debug exception !!
> +         * For software breakpoint QEMU uses a ehpriv-1 instruction;
> +         * So there cannot be any reason that we are here for guest
> +         * set debug exception, only possibility is guest executed a
> +         * privilege instruction and that's why we are injecting
> +         * program exception.
> +         */
> +         cs->exception_index = POWERPC_EXCP_PROGRAM;
> +         env->error_code = POWERPC_EXCP_INVAL;
> +         ppc_cpu_do_interrupt(cs);
> +    }
> +

Excellent. This is the change I had as part of server side patch for no
sw breakpoint case. Also have one more addition to this, which I found
in the debug.

Only issue in here (using TCG for injecting) is that, KVM gives us PC,
but incase of TCG, it uses nip. So nip gets decremented in
ppc_cpu_do_interrupt function ending up sending the wrong pc to guest.
So Alex suggested to increment the nip by 4 before calling the
ppc_cpu_do_interrupt function. Also kindly add cpu_synchronize_state
before calling since we are changing the register values.

Regards
Maddy

> +    return handle;
> +}
> +
>  int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
>  {
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
> @@ -1315,6 +1378,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;
> @@ -2003,16 +2076,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;
> @@ -2027,10 +2090,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;
>      /*
>