On 24.06.2013, at 11:08, 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.
>
> This is how we save/restore debug register context when switching
> between guest, userspace and kernel user-process:
>
> When QEMU is running
> -> thread->debug_reg == QEMU debug register context.
> -> Kernel will handle switching the debug register on context switch.
> -> no vcpu_load() called
>
> QEMU makes ioctls (except RUN)
> -> This will call vcpu_load()
> -> should not change context.
> -> Some ioctls can change vcpu debug register, context saved in vcpu->debug_regs
>
> QEMU Makes RUN ioctl
> -> Save thread->debug_reg on STACK
> -> Store thread->debug_reg == vcpu->debug_reg
> -> load thread->debug_reg
> -> RUN VCPU ( So thread points to vcpu context )
>
> Context switch happens When VCPU running
> -> makes vcpu_load() should not load any context
> -> kernel loads the vcpu context as thread->debug_regs points to vcpu context.
>
> On heavyweight_exit
> -> Load the context saved on stack in thread->debug_reg
>
> Currently we do not support debug resource emulation to guest,
> On debug exception, always exit to user space irrespective of
> user space is expecting the debug exception or not. If this is
> unexpected exception (breakpoint/watchpoint event not set by
> userspace) then let us leave the action on user space. This
> is similar to what it was before, only thing is that now we
> have proper exit state available to user space.
>
> Signed-off-by: Bharat Bhushan <bharat.bhushan@xxxxxxxxxxxxx>
> ---
> arch/powerpc/include/asm/kvm_host.h | 3 +
> arch/powerpc/include/uapi/asm/kvm.h | 1 +
> arch/powerpc/kvm/booke.c | 233 ++++++++++++++++++++++++++++++++---
> arch/powerpc/kvm/booke.h | 5 +
> 4 files changed, 224 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 838a577..aeb490d 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -524,7 +524,10 @@ struct kvm_vcpu_arch {
> u32 eptcfg;
> u32 epr;
> u32 crit_save;
> + /* guest debug registers*/
> struct debug_reg dbg_reg;
> + /* hardware visible debug registers when in guest state */
> + struct debug_reg shadow_dbg_reg;
> #endif
> gpa_t paddr_accessed;
> gva_t vaddr_accessed;
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index ded0607..f5077c2 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -27,6 +27,7 @@
> #define __KVM_HAVE_PPC_SMT
> #define __KVM_HAVE_IRQCHIP
> #define __KVM_HAVE_IRQ_LINE
> +#define __KVM_HAVE_GUEST_DEBUG
>
> struct kvm_regs {
> __u64 pc;
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 3e9fc1d..8be3502 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -133,6 +133,29 @@ 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.
> + */
> + vcpu->arch.shared->msr |= MSR_DE;
> +#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 +173,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,
> @@ -655,6 +679,7 @@ int kvmppc_core_check_requests(struct kvm_vcpu *vcpu)
> int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
> {
> int ret, s;
> + struct thread_struct thread;
> #ifdef CONFIG_PPC_FPU
> unsigned int fpscr;
> int fpexc_mode;
> @@ -698,12 +723,21 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
>
> kvmppc_load_guest_fp(vcpu);
> #endif
> + /* Switch to guest debug context */
> + thread.debug = vcpu->arch.shadow_dbg_reg;
> + switch_booke_debug_regs(&thread);
> + thread.debug = current->thread.debug;
> + current->thread.debug = vcpu->arch.shadow_dbg_reg;
>
> ret = __kvmppc_vcpu_run(kvm_run, vcpu);
>
> /* No need for kvm_guest_exit. It's done in handle_exit.
> We also get here with interrupts enabled. */
>
> + /* Switch back to user space debug context */
> + switch_booke_debug_regs(&thread);
> + current->thread.debug = thread.debug;
> +
> #ifdef CONFIG_PPC_FPU
> kvmppc_save_guest_fp(vcpu);
>
> @@ -759,6 +793,30 @@ 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)
> +{
> + struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg);
> + u32 dbsr = vcpu->arch.dbsr;
> +
> + run->debug.arch.status = 0;
> + run->debug.arch.address = vcpu->arch.pc;
> +
> + 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 = dbg_reg->dac1;
> + else if (dbsr & (DBSR_DAC2R | DBSR_DAC2W))
> + run->debug.arch.address = dbg_reg->dac2;
> + }
> +
> + return RESUME_HOST;
> +}
> +
> static void kvmppc_fill_pt_regs(struct pt_regs *regs)
> {
> ulong r1, ip, msr, lr;
> @@ -819,6 +877,11 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
> case BOOKE_INTERRUPT_CRITICAL:
> unknown_exception(®s);
> break;
> + case BOOKE_INTERRUPT_DEBUG:
> + /* Save DBSR before preemption is enabled */
> + vcpu->arch.dbsr = mfspr(SPRN_DBSR);
> + kvmppc_clear_dbsr();
> + break;
> }
> }
>
> @@ -1118,18 +1181,10 @@ 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;
> }
>
> @@ -1180,7 +1235,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
> @@ -1557,12 +1612,6 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
> return r;
> }
>
> -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> - struct kvm_guest_debug *dbg)
> -{
> - return -EINVAL;
> -}
> -
> int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
> {
> return -ENOTSUPP;
> @@ -1668,6 +1717,151 @@ void kvmppc_decrementer_func(unsigned long data)
> kvmppc_set_tsr_bits(vcpu, TSR_DIS);
> }
>
> +static int kvmppc_booke_add_breakpoint(struct debug_reg *dbg_reg,
> + uint64_t addr, int index)
> +{
> + switch (index) {
> + case 0:
> + dbg_reg->dbcr0 |= DBCR0_IAC1;
> + dbg_reg->iac1 = addr;
> + break;
> + case 1:
> + dbg_reg->dbcr0 |= DBCR0_IAC2;
> + dbg_reg->iac2 = addr;
> + break;
> +#if CONFIG_PPC_ADV_DEBUG_IACS > 2
> + case 2:
> + dbg_reg->dbcr0 |= DBCR0_IAC3;
> + dbg_reg->iac3 = addr;
> + break;
> + case 3:
> + dbg_reg->dbcr0 |= DBCR0_IAC4;
> + dbg_reg->iac4 = addr;
> + break;
> +#endif
> + default:
> + return -EINVAL;
> + }
> +
> + dbg_reg->dbcr0 |= DBCR0_IDM;
> + return 0;
> +}
> +
> +static int kvmppc_booke_add_watchpoint(struct debug_reg *dbg_reg, uint64_t addr,
> + int type, int index)
> +{
> + switch (index) {
> + case 0:
> + if (type & KVMPPC_DEBUG_WATCH_READ)
> + dbg_reg->dbcr0 |= DBCR0_DAC1R;
> + if (type & KVMPPC_DEBUG_WATCH_WRITE)
> + dbg_reg->dbcr0 |= DBCR0_DAC1W;
> + dbg_reg->dac1 = addr;
> + break;
> + case 1:
> + if (type & KVMPPC_DEBUG_WATCH_READ)
> + dbg_reg->dbcr0 |= DBCR0_DAC2R;
> + if (type & KVMPPC_DEBUG_WATCH_WRITE)
> + dbg_reg->dbcr0 |= DBCR0_DAC2W;
> + dbg_reg->dac2 = addr;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + dbg_reg->dbcr0 |= DBCR0_IDM;
> + return 0;
> +}
> +
> +#ifdef CONFIG_KVM_BOOKE_HV
> +void kvm_guest_may_change_msr_de(struct kvm_vcpu *vcpu, bool may_change)
> +{
> + if (may_change)
> + /* Allow guest to change MSR.DE */
> + vcpu->arch.shadow_msrp &= ~MSRP_DEP;
> + else
> + /* Don't allow guest to change MSR.DE */
> + vcpu->arch.shadow_msrp |= MSRP_DEP;
> +
> +}
> +#endif
Could you please provide this function regardless of CONFIG_KVM_BOOKE_HV? For PR, just save a bool away somewhere and emulate the same behavior (guest may not modify MSR.DE).
That way we ensure consistency between the targets and get rid of ugly #ifdef's :).
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
