On 28.02.2013, at 05:13, Bharat Bhushan wrote:

> This patch adds the debug stub support on booke/bookehv.
> Now QEMU debug stub can use hw breakpoint, watchpoint and
> software breakpoint to debug guest.
> 
> Signed-off-by: Bharat Bhushan <bharat.bhus...@freescale.com>
> ---
> arch/powerpc/include/uapi/asm/kvm.h |   22 +++++-
> arch/powerpc/kvm/booke.c            |  143 +++++++++++++++++++++++++++++++---
> arch/powerpc/kvm/e500_emulate.c     |    6 ++
> arch/powerpc/kvm/e500mc.c           |    3 +-
> 4 files changed, 155 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h 
> b/arch/powerpc/include/uapi/asm/kvm.h
> index 15f9a00..d7ce449 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -25,6 +25,7 @@
> /* Select powerpc specific features in <linux/kvm.h> */
> #define __KVM_HAVE_SPAPR_TCE
> #define __KVM_HAVE_PPC_SMT
> +#define __KVM_HAVE_GUEST_DEBUG
> 
> struct kvm_regs {
>       __u64 pc;
> @@ -267,7 +268,24 @@ struct kvm_fpu {
>       __u64 fpr[32];
> };
> 
> +/*
> + * Defines for h/w breakpoint, watchpoint (read, write or both) and
> + * software breakpoint.
> + * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and "status"
> + * for KVM_DEBUG_EXIT.
> + */
> +#define KVMPPC_DEBUG_NONE            0x0
> +#define KVMPPC_DEBUG_BREAKPOINT              (1UL << 1)
> +#define KVMPPC_DEBUG_WATCH_WRITE     (1UL << 2)
> +#define KVMPPC_DEBUG_WATCH_READ              (1UL << 3)
> struct kvm_debug_exit_arch {
> +     __u64 address;
> +     /*
> +      * exiting to userspace because of h/w breakpoint, watchpoint
> +      * (read, write or both) and software breakpoint.
> +      */
> +     __u32 status;
> +     __u32 reserved;
> };
> 
> /* for KVM_SET_GUEST_DEBUG */
> @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch {
>                * Type denotes h/w breakpoint, read watchpoint, write
>                * watchpoint or watchpoint (both read and write).
>                */
> -#define KVMPPC_DEBUG_NOTYPE          0x0
> -#define KVMPPC_DEBUG_BREAKPOINT              (1UL << 1)
> -#define KVMPPC_DEBUG_WATCH_WRITE     (1UL << 2)
> -#define KVMPPC_DEBUG_WATCH_READ              (1UL << 3)
>               __u32 type;
>               __u32 reserved;
>       } bp[16];
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 1de93a8..21b0313 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu)
> #endif
> }
> 
> +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu)
> +{
> +     /* Synchronize guest's desire to get debug interrupts into shadow MSR */
> +#ifndef CONFIG_KVM_BOOKE_HV
> +     vcpu->arch.shadow_msr &= ~MSR_DE;
> +     vcpu->arch.shadow_msr |= vcpu->arch.shared->msr & MSR_DE;
> +#endif
> +
> +     /* Force enable debug interrupts when user space wants to debug */
> +     if (vcpu->guest_debug) {
> +#ifdef CONFIG_KVM_BOOKE_HV
> +             /*
> +              * Since there is no shadow MSR, sync MSR_DE into the guest
> +              * visible MSR. Do not allow guest to change MSR[DE].
> +              */
> +             vcpu->arch.shared->msr |= MSR_DE;
> +             mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP);
> +#else
> +             vcpu->arch.shadow_msr |= MSR_DE;
> +             vcpu->arch.shared->msr &= ~MSR_DE;
> +#endif
> +     }
> +}
> +
> /*
>  * Helper function for "full" MSR writes.  No need to call this if only
>  * EE/CE/ME/DE/RI are changing.
> @@ -150,6 +174,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr)
>       kvmppc_mmu_msr_notify(vcpu, old_msr);
>       kvmppc_vcpu_sync_spe(vcpu);
>       kvmppc_vcpu_sync_fpu(vcpu);
> +     kvmppc_vcpu_sync_debug(vcpu);
> }
> 
> static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu,
> @@ -736,6 +761,13 @@ static int emulation_exit(struct kvm_run *run, struct 
> kvm_vcpu *vcpu)
>               run->exit_reason = KVM_EXIT_DCR;
>               return RESUME_HOST;
> 
> +     case EMULATE_EXIT_USER:
> +             run->exit_reason = KVM_EXIT_DEBUG;
> +             run->debug.arch.address = vcpu->arch.pc;
> +             run->debug.arch.status = 0;
> +             kvmppc_account_exit(vcpu, DEBUG_EXITS);

As mentioned previously, this is wrong and needs to go into the instruction 
emulation code for that opcode.

> +             return RESUME_HOST;
> +
>       case EMULATE_FAIL:
>               printk(KERN_CRIT "%s: emulation at %lx failed (%08x)\n",
>                      __func__, vcpu->arch.pc, vcpu->arch.last_inst);
> @@ -751,6 +783,28 @@ static int emulation_exit(struct kvm_run *run, struct 
> kvm_vcpu *vcpu)
>       }
> }
> 
> +static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu)
> +{
> +     u32 dbsr = vcpu->arch.dbsr;
> +     run->debug.arch.status = 0;
> +     run->debug.arch.address = vcpu->arch.pc;

This should go into the if(breakpoint) branch.

> +
> +     if (dbsr & (DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4)) {
> +             run->debug.arch.status |= KVMPPC_DEBUG_BREAKPOINT;
> +     } else {
> +             if (dbsr & (DBSR_DAC1W | DBSR_DAC2W))
> +                     run->debug.arch.status |= KVMPPC_DEBUG_WATCH_WRITE;
> +             else if (dbsr & (DBSR_DAC1R | DBSR_DAC2R))
> +                     run->debug.arch.status |= KVMPPC_DEBUG_WATCH_READ;
> +             if (dbsr & (DBSR_DAC1R | DBSR_DAC1W))
> +                     run->debug.arch.address = 
> vcpu->arch.shadow_dbg_reg.dac[0];
> +             else if (dbsr & (DBSR_DAC2R | DBSR_DAC2W))
> +                     run->debug.arch.address = 
> vcpu->arch.shadow_dbg_reg.dac[1];
> +     }
> +
> +     return RESUME_HOST;
> +}
> +
> static void kvmppc_fill_pt_regs(struct pt_regs *regs)
> {
>       ulong r1, ip, msr, lr;
> @@ -1110,18 +1164,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
> kvm_vcpu *vcpu,
>       }
> 
>       case BOOKE_INTERRUPT_DEBUG: {
> -             u32 dbsr;
> -
> -             vcpu->arch.pc = mfspr(SPRN_CSRR0);
> -
> -             /* clear IAC events in DBSR register */
> -             dbsr = mfspr(SPRN_DBSR);
> -             dbsr &= DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4;
> -             mtspr(SPRN_DBSR, dbsr);
> -
> -             run->exit_reason = KVM_EXIT_DEBUG;
> +             r = kvmppc_handle_debug(run, vcpu);
> +             if (r == RESUME_HOST) {
> +                     run->exit_reason = KVM_EXIT_DEBUG;
> +             }
>               kvmppc_account_exit(vcpu, DEBUG_EXITS);
> -             r = RESUME_HOST;
>               break;
>       }
> 
> @@ -1172,7 +1219,7 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>       kvmppc_set_msr(vcpu, 0);
> 
> #ifndef CONFIG_KVM_BOOKE_HV
> -     vcpu->arch.shadow_msr = MSR_USER | MSR_DE | MSR_IS | MSR_DS;
> +     vcpu->arch.shadow_msr = MSR_USER | MSR_IS | MSR_DS;
>       vcpu->arch.shadow_pid = 1;
>       vcpu->arch.shared->msr = 0;
> #endif
> @@ -1527,10 +1574,80 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, 
> struct kvm_one_reg *reg)
>       return r;
> }
> 
> +#define BP_NUM       KVMPPC_BOOKE_IAC_NUM
> +#define WP_NUM       KVMPPC_BOOKE_DAC_NUM
> +
> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>                                        struct kvm_guest_debug *dbg)
> {
> -     return -EINVAL;
> +
> +     if (!(dbg->control & KVM_GUESTDBG_ENABLE)) {
> +             /* Clear All debug events */
> +             vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
> +             vcpu->guest_debug = 0;
> +             return 0;
> +     }
> +
> +     vcpu->guest_debug = dbg->control;
> +     vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
> +     /* Set DBCR0_EDM in guest visible DBCR0 register. */
> +     vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM;
> +
> +     if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
> +             vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC;
> +
> +     if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {

if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)) {
    /* Code below handles only HW breakpoints */
    return 0;
}

> +             struct kvmppc_booke_debug_reg *gdbgr =
> +                             &(vcpu->arch.shadow_dbg_reg);
> +             int n, b = 0, w = 0;
> +             const u32 bp_code[] = {
> +                     DBCR0_IAC1 | DBCR0_IDM,
> +                     DBCR0_IAC2 | DBCR0_IDM,
> +                     DBCR0_IAC3 | DBCR0_IDM,
> +                     DBCR0_IAC4 | DBCR0_IDM
> +             };
> +             const u32 wp_code[] = {
> +                     DBCR0_DAC1W | DBCR0_IDM,
> +                     DBCR0_DAC2W | DBCR0_IDM,
> +                     DBCR0_DAC1R | DBCR0_IDM,
> +                     DBCR0_DAC2R | DBCR0_IDM
> +             };
> +
> +#ifndef CONFIG_KVM_BOOKE_HV

Please no double negation. Also, what is this about?

> +             gdbgr->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US |
> +                             DBCR1_IAC3US | DBCR1_IAC4US;
> +             gdbgr->dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US;
> +#else
> +             gdbgr->dbcr1 = 0;
> +             gdbgr->dbcr2 = 0;
> +#endif
> +
> +             for (n = 0; n < (BP_NUM + WP_NUM); n++) {
> +                     u32 type = dbg->arch.bp[n].type;
> +
> +                     if (!type)
> +                             break;
> +
> +                     if (type & (KVMPPC_DEBUG_WATCH_READ |
> +                                 KVMPPC_DEBUG_WATCH_WRITE)) {
> +                             if (w < WP_NUM) {
> +                                     if (type & KVMPPC_DEBUG_WATCH_READ)
> +                                             gdbgr->dbcr0 |= wp_code[w + 2];
> +                                     if (type & KVMPPC_DEBUG_WATCH_WRITE)
> +                                             gdbgr->dbcr0 |= wp_code[w];
> +                                     gdbgr->dac[w] = dbg->arch.bp[n].addr;
> +                                     w++;
> +                             }
> +                     } else if (type & KVMPPC_DEBUG_BREAKPOINT) {
> +                             if (b < BP_NUM) {
> +                                     gdbgr->dbcr0 |= bp_code[b];
> +                                     gdbgr->iac[b] = dbg->arch.bp[n].addr;
> +                                     b++;
> +                             }
> +                     }
> +             }
> +     }
> +     return 0;
> }
> 
> int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
> diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c
> index e78f353..83ac877 100644
> --- a/arch/powerpc/kvm/e500_emulate.c
> +++ b/arch/powerpc/kvm/e500_emulate.c
> @@ -26,6 +26,7 @@
> #define XOP_TLBRE   946
> #define XOP_TLBWE   978
> #define XOP_TLBILX  18
> +#define XOP_EHPRIV  270
> 
> #ifdef CONFIG_KVM_E500MC
> static int dbell2prio(ulong param)
> @@ -130,6 +131,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct 
> kvm_vcpu *vcpu,
>                       emulated = kvmppc_e500_emul_tlbivax(vcpu, ea);
>                       break;
> 
> +             case XOP_EHPRIV:
> +                     emulated = EMULATE_EXIT_USER;
> +                     *advance = 0;
> +                     break;
> +
>               default:
>                       emulated = EMULATE_FAIL;
>               }
> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
> index 1f89d26..f5fc6f5 100644
> --- a/arch/powerpc/kvm/e500mc.c
> +++ b/arch/powerpc/kvm/e500mc.c
> @@ -182,8 +182,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu)
> {
>       struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
> 
> -     vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI | \
> -                              SPRN_EPCR_DUVD;
> +     vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI;

Doesn't this route all debug events through the host?


Alex

> #ifdef CONFIG_64BIT
>       vcpu->arch.shadow_epcr |= SPRN_EPCR_ICM;
> #endif
> -- 
> 1.7.0.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to