| Message ID | 1362024796-4237-5-git-send-email-bharat.bhushan@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
On 28.02.2013, at 05:13, Bharat Bhushan wrote: > On Guest entry: if guest is wants to use the debug register then > save h/w debug register in host_dbg_reg and load the debug registers > with shadow_dbg_reg. Otherwise leave h/w debug registers as is. Why can't we switch the majority of registers on vcpu_put/get and only enable or disable debugging on guest entry/exit? Alex > > On guest exit: If guest/user-space is using the debug resource then > restore the h/w debug register with host_dbg_reg. No need to save guest > debug register as shadow_dbg_reg is having required values. If guest is not > using the debug resources then no need to restore h/w registers. > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > --- > arch/powerpc/include/asm/kvm_host.h | 5 ++ > arch/powerpc/kernel/asm-offsets.c | 26 ++++++++ > arch/powerpc/kvm/booke_interrupts.S | 114 +++++++++++++++++++++++++++++++++++ > 3 files changed, 145 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index f4ba881..a9feeb0 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -504,7 +504,12 @@ struct kvm_vcpu_arch { > u32 mmucfg; > u32 epr; > u32 crit_save; > + /* guest debug registers*/ > struct kvmppc_booke_debug_reg dbg_reg; > + /* shadow debug registers */ > + struct kvmppc_booke_debug_reg shadow_dbg_reg; > + /* host debug registers*/ > + struct kvmppc_booke_debug_reg host_dbg_reg; > #endif > gpa_t paddr_accessed; > gva_t vaddr_accessed; > diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c > index 02048f3..22deda7 100644 > --- a/arch/powerpc/kernel/asm-offsets.c > +++ b/arch/powerpc/kernel/asm-offsets.c > @@ -563,6 +563,32 @@ int main(void) > DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear)); > DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr)); > DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save)); > + DEFINE(VCPU_DBSR, offsetof(struct kvm_vcpu, arch.dbsr)); > + DEFINE(VCPU_SHADOW_DBG, offsetof(struct kvm_vcpu, arch.shadow_dbg_reg)); > + DEFINE(VCPU_HOST_DBG, offsetof(struct kvm_vcpu, arch.host_dbg_reg)); > + DEFINE(KVMPPC_DBG_DBCR0, offsetof(struct kvmppc_booke_debug_reg, > + dbcr0)); > + DEFINE(KVMPPC_DBG_DBCR1, offsetof(struct kvmppc_booke_debug_reg, > + dbcr1)); > + DEFINE(KVMPPC_DBG_DBCR2, offsetof(struct kvmppc_booke_debug_reg, > + dbcr2)); > +#ifdef CONFIG_KVM_E500MC > + DEFINE(KVMPPC_DBG_DBCR4, offsetof(struct kvmppc_booke_debug_reg, > + dbcr4)); > +#endif > + DEFINE(KVMPPC_DBG_IAC1, offsetof(struct kvmppc_booke_debug_reg, > + iac[0])); > + DEFINE(KVMPPC_DBG_IAC2, offsetof(struct kvmppc_booke_debug_reg, > + iac[1])); > + DEFINE(KVMPPC_DBG_IAC3, offsetof(struct kvmppc_booke_debug_reg, > + iac[2])); > + DEFINE(KVMPPC_DBG_IAC4, offsetof(struct kvmppc_booke_debug_reg, > + iac[3])); > + DEFINE(KVMPPC_DBG_DAC1, offsetof(struct kvmppc_booke_debug_reg, > + dac[0])); > + DEFINE(KVMPPC_DBG_DAC2, offsetof(struct kvmppc_booke_debug_reg, > + dac[1])); > + DEFINE(VCPU_GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug)); > #endif /* CONFIG_PPC_BOOK3S */ > #endif /* CONFIG_KVM */ > > diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S > index 2c6deb5..6d78e01 100644 > --- a/arch/powerpc/kvm/booke_interrupts.S > +++ b/arch/powerpc/kvm/booke_interrupts.S > @@ -39,6 +39,8 @@ > #define HOST_MIN_STACK_SIZE (HOST_NV_GPR(R31) + 4) > #define HOST_STACK_SIZE (((HOST_MIN_STACK_SIZE + 15) / 16) * 16) /* Align. */ > #define HOST_STACK_LR (HOST_STACK_SIZE + 4) /* In caller stack frame. */ > +#define DBCR0_AC_BITS (DBCR0_IAC1 | DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4 | \ > + DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W) > > #define NEED_INST_MASK ((1<<BOOKE_INTERRUPT_PROGRAM) | \ > (1<<BOOKE_INTERRUPT_DTLB_MISS) | \ > @@ -54,6 +56,8 @@ > (1<<BOOKE_INTERRUPT_DTLB_MISS) | \ > (1<<BOOKE_INTERRUPT_ALIGNMENT)) > > +#define NEED_DEBUG_SAVE (1<<BOOKE_INTERRUPT_DEBUG) > + > .macro __KVM_HANDLER ivor_nr scratch srr0 > /* Get pointer to vcpu and record exit number. */ > mtspr \scratch , r4 > @@ -215,6 +219,59 @@ _GLOBAL(kvmppc_resume_host) > stw r9, VCPU_FAULT_ESR(r4) > ..skip_esr: > > + lwz r9, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR0(r4) > + rlwinm. r8, r9, 0, ~DBCR0_IDM > + beq skip_load_host_debug > + lwz r8, VCPU_HOST_DBG+KVMPPC_DBG_DBCR0(r4) > + andis. r9, r9, DBCR0_AC_BITS@h > + li r9, 0 > + mtspr SPRN_DBCR0, r9 /* disable all debug event */ > + beq skip_load_hw_bkpts > + lwz r7, VCPU_HOST_DBG+KVMPPC_DBG_DBCR1(r4) > + lwz r9, VCPU_HOST_DBG+KVMPPC_DBG_DBCR2(r4) > + mtspr SPRN_DBCR1, r7 > + mtspr SPRN_DBCR2, r9 > + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC1, r4) > + PPC_LD(r9, VCPU_HOST_DBG+KVMPPC_DBG_IAC2, r4) > + mtspr SPRN_IAC1, r7 > + mtspr SPRN_IAC2, r9 > +#if CONFIG_PPC_ADV_DEBUG_IACS > 2 > + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) > + PPC_LD(r9, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) > + mtspr SPRN_IAC3, r3 > + mtspr SPRN_IAC4, r4 > +#endif > + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_DAC1, r4) > + PPC_LD(r9, VCPU_HOST_DBG+KVMPPC_DBG_DAC2, r4) > + mtspr SPRN_DAC1, r7 > + mtspr SPRN_DAC2, r9 > +skip_load_hw_bkpts: > + /* Clear h/w DBSR and save current(guest) DBSR */ > + mfspr r9, SPRN_DBSR > + mtspr SPRN_DBSR, r9 > + isync > + andi. r7, r6, NEED_DEBUG_SAVE > + beq skip_dbsr_save > + /* > + * If vcpu->guest_debug flag is set then do not check for > + * shared->msr.DE as this debugging (say by QEMU) does not > + * depends on shared->msr.de. In these scanerios MSR.DE is > + * always set using shared_msr and should be handled always. > + */ > + lwz r7, VCPU_GUEST_DEBUG(r4) > + cmpwi r7, 0 > + bne skip_save_trap_event > + PPC_LL r3, VCPU_SHARED(r4) > + PPC_LD(r3, VCPU_SHARED_MSR, r3) > + andi. r3, r3, MSR_DE > + bne skip_save_trap_event > + andis. r9, r9, DBSR_TIE@h > +skip_save_trap_event: > + stw r9, VCPU_DBSR(r4) > +skip_dbsr_save: > + mtspr SPRN_DBCR0, r8 > +skip_load_host_debug: > + > /* Save remaining volatile guest register state to vcpu. */ > stw r0, VCPU_GPR(R0)(r4) > stw r1, VCPU_GPR(R1)(r4) > @@ -468,6 +525,63 @@ lightweight_exit: > PPC_LD(r3, VCPU_SHARED_SPRG7, r5) > mtspr SPRN_SPRG7W, r3 > > + mfmsr r7 > + rlwinm r7, r7, 0, ~MSR_DE > + mtmsr r7 > + lwz r6, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR0(r4) > + rlwinm. r7, r6, 0, ~DBCR0_IDM > + beq skip_load_guest_debug > + mfspr r8, SPRN_DBCR0 > + stw r8, VCPU_HOST_DBG+KVMPPC_DBG_DBCR0(r4) > + andis. r3, r6, DBCR0_AC_BITS@h > + beq skip_hw_bkpts > + mfspr r7, SPRN_DBCR1 > + stw r7, VCPU_HOST_DBG+KVMPPC_DBG_DBCR1(r4) > + mfspr r8, SPRN_DBCR2 > + stw r8, VCPU_HOST_DBG+KVMPPC_DBG_DBCR2(r4) > + mfspr r7, SPRN_IAC1 > + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC1, r4) > + mfspr r8, SPRN_IAC2 > + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC2, r4) > +#if CONFIG_PPC_ADV_DEBUG_IACS > 2 > + mfspr r7, SPRN_IAC3 > + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) > + mfspr r8, SPRN_IAC4 > + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) > +#endif > + mfspr r7, SPRN_DAC1 > + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_DAC1, r4) > + mfspr r8, SPRN_DAC2 > + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_DAC2, r4) > + li r8, 0 > + mtspr SPRN_DBCR0, r8 /* disable all debug event */ > + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR1, r4) > + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR2, r4) > + mtspr SPRN_DBCR1, r7 > + mtspr SPRN_DBCR2, r8 > + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC1, r4) > + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC2, r4) > + mtspr SPRN_IAC1, r7 > + mtspr SPRN_IAC2, r8 > +#if CONFIG_PPC_ADV_DEBUG_IACS > 2 > + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC3, r4) > + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC4, r4) > + mtspr SPRN_IAC3, r7 > + mtspr SPRN_IAC4, r8 > +#endif > + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_DAC1, r4) > + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_DAC2, r4) > + mtspr SPRN_DAC1, r7 > + mtspr SPRN_DAC2, r8 > +skip_hw_bkpts: > + /* Clear if any deferred debug event */ > + mfspr r8, SPRN_DBSR > + mtspr SPRN_DBSR, r8 > + isync > + /* Restore guest DBCR */ > + mtspr SPRN_DBCR0, r6 > +skip_load_guest_debug: > + > #ifdef CONFIG_KVM_EXIT_TIMING > /* save enter time */ > 1: > -- > 1.7.0.4 > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Thursday, March 07, 2013 6:56 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Bhushan > Bharat-R65777 > Subject: Re: [PATCH 4/7] booke: Save and restore debug registers on guest entry > and exit > > > On 28.02.2013, at 05:13, Bharat Bhushan wrote: > > > On Guest entry: if guest is wants to use the debug register then save > > h/w debug register in host_dbg_reg and load the debug registers with > > shadow_dbg_reg. Otherwise leave h/w debug registers as is. > > Why can't we switch the majority of registers on vcpu_put/get and only enable or > disable debugging on guest entry/exit? One of the reason for not doing this is that the KVM is a host kernel module and let this be debugged by host (I do not this how much useful this is :)) So I am not able to recall the specific reason, maybe we have just coded this like this and tried to keep overhead as low as possible by switching registers only when they are used. As we discussed before, we can keep this option open for future. -Bharat > > > Alex > > > > > On guest exit: If guest/user-space is using the debug resource then > > restore the h/w debug register with host_dbg_reg. No need to save > > guest debug register as shadow_dbg_reg is having required values. If > > guest is not using the debug resources then no need to restore h/w registers. > > > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > > --- > > arch/powerpc/include/asm/kvm_host.h | 5 ++ > > arch/powerpc/kernel/asm-offsets.c | 26 ++++++++ > > arch/powerpc/kvm/booke_interrupts.S | 114 > > +++++++++++++++++++++++++++++++++++ > > 3 files changed, 145 insertions(+), 0 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/kvm_host.h > > b/arch/powerpc/include/asm/kvm_host.h > > index f4ba881..a9feeb0 100644 > > --- a/arch/powerpc/include/asm/kvm_host.h > > +++ b/arch/powerpc/include/asm/kvm_host.h > > @@ -504,7 +504,12 @@ struct kvm_vcpu_arch { > > u32 mmucfg; > > u32 epr; > > u32 crit_save; > > + /* guest debug registers*/ > > struct kvmppc_booke_debug_reg dbg_reg; > > + /* shadow debug registers */ > > + struct kvmppc_booke_debug_reg shadow_dbg_reg; > > + /* host debug registers*/ > > + struct kvmppc_booke_debug_reg host_dbg_reg; > > #endif > > gpa_t paddr_accessed; > > gva_t vaddr_accessed; > > diff --git a/arch/powerpc/kernel/asm-offsets.c > > b/arch/powerpc/kernel/asm-offsets.c > > index 02048f3..22deda7 100644 > > --- a/arch/powerpc/kernel/asm-offsets.c > > +++ b/arch/powerpc/kernel/asm-offsets.c > > @@ -563,6 +563,32 @@ int main(void) > > DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear)); > > DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr)); > > DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save)); > > + DEFINE(VCPU_DBSR, offsetof(struct kvm_vcpu, arch.dbsr)); > > + DEFINE(VCPU_SHADOW_DBG, offsetof(struct kvm_vcpu, arch.shadow_dbg_reg)); > > + DEFINE(VCPU_HOST_DBG, offsetof(struct kvm_vcpu, arch.host_dbg_reg)); > > + DEFINE(KVMPPC_DBG_DBCR0, offsetof(struct kvmppc_booke_debug_reg, > > + dbcr0)); > > + DEFINE(KVMPPC_DBG_DBCR1, offsetof(struct kvmppc_booke_debug_reg, > > + dbcr1)); > > + DEFINE(KVMPPC_DBG_DBCR2, offsetof(struct kvmppc_booke_debug_reg, > > + dbcr2)); > > +#ifdef CONFIG_KVM_E500MC > > + DEFINE(KVMPPC_DBG_DBCR4, offsetof(struct kvmppc_booke_debug_reg, > > + dbcr4)); > > +#endif > > + DEFINE(KVMPPC_DBG_IAC1, offsetof(struct kvmppc_booke_debug_reg, > > + iac[0])); > > + DEFINE(KVMPPC_DBG_IAC2, offsetof(struct kvmppc_booke_debug_reg, > > + iac[1])); > > + DEFINE(KVMPPC_DBG_IAC3, offsetof(struct kvmppc_booke_debug_reg, > > + iac[2])); > > + DEFINE(KVMPPC_DBG_IAC4, offsetof(struct kvmppc_booke_debug_reg, > > + iac[3])); > > + DEFINE(KVMPPC_DBG_DAC1, offsetof(struct kvmppc_booke_debug_reg, > > + dac[0])); > > + DEFINE(KVMPPC_DBG_DAC2, offsetof(struct kvmppc_booke_debug_reg, > > + dac[1])); > > + DEFINE(VCPU_GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug)); > > #endif /* CONFIG_PPC_BOOK3S */ > > #endif /* CONFIG_KVM */ > > > > diff --git a/arch/powerpc/kvm/booke_interrupts.S > > b/arch/powerpc/kvm/booke_interrupts.S > > index 2c6deb5..6d78e01 100644 > > --- a/arch/powerpc/kvm/booke_interrupts.S > > +++ b/arch/powerpc/kvm/booke_interrupts.S > > @@ -39,6 +39,8 @@ > > #define HOST_MIN_STACK_SIZE (HOST_NV_GPR(R31) + 4) #define > > HOST_STACK_SIZE (((HOST_MIN_STACK_SIZE + 15) / 16) * 16) /* Align. */ > > #define HOST_STACK_LR (HOST_STACK_SIZE + 4) /* In caller stack frame. */ > > +#define DBCR0_AC_BITS (DBCR0_IAC1 | DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4 | \ > > + DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W) > > > > #define NEED_INST_MASK ((1<<BOOKE_INTERRUPT_PROGRAM) | \ > > (1<<BOOKE_INTERRUPT_DTLB_MISS) | \ @@ -54,6 > > +56,8 @@ > > (1<<BOOKE_INTERRUPT_DTLB_MISS) | \ > > (1<<BOOKE_INTERRUPT_ALIGNMENT)) > > > > +#define NEED_DEBUG_SAVE (1<<BOOKE_INTERRUPT_DEBUG) > > + > > .macro __KVM_HANDLER ivor_nr scratch srr0 > > /* Get pointer to vcpu and record exit number. */ > > mtspr \scratch , r4 > > @@ -215,6 +219,59 @@ _GLOBAL(kvmppc_resume_host) > > stw r9, VCPU_FAULT_ESR(r4) > > ..skip_esr: > > > > + lwz r9, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR0(r4) > > + rlwinm. r8, r9, 0, ~DBCR0_IDM > > + beq skip_load_host_debug > > + lwz r8, VCPU_HOST_DBG+KVMPPC_DBG_DBCR0(r4) > > + andis. r9, r9, DBCR0_AC_BITS@h > > + li r9, 0 > > + mtspr SPRN_DBCR0, r9 /* disable all debug event */ > > + beq skip_load_hw_bkpts > > + lwz r7, VCPU_HOST_DBG+KVMPPC_DBG_DBCR1(r4) > > + lwz r9, VCPU_HOST_DBG+KVMPPC_DBG_DBCR2(r4) > > + mtspr SPRN_DBCR1, r7 > > + mtspr SPRN_DBCR2, r9 > > + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC1, r4) > > + PPC_LD(r9, VCPU_HOST_DBG+KVMPPC_DBG_IAC2, r4) > > + mtspr SPRN_IAC1, r7 > > + mtspr SPRN_IAC2, r9 > > +#if CONFIG_PPC_ADV_DEBUG_IACS > 2 > > + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) > > + PPC_LD(r9, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) > > + mtspr SPRN_IAC3, r3 > > + mtspr SPRN_IAC4, r4 > > +#endif > > + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_DAC1, r4) > > + PPC_LD(r9, VCPU_HOST_DBG+KVMPPC_DBG_DAC2, r4) > > + mtspr SPRN_DAC1, r7 > > + mtspr SPRN_DAC2, r9 > > +skip_load_hw_bkpts: > > + /* Clear h/w DBSR and save current(guest) DBSR */ > > + mfspr r9, SPRN_DBSR > > + mtspr SPRN_DBSR, r9 > > + isync > > + andi. r7, r6, NEED_DEBUG_SAVE > > + beq skip_dbsr_save > > + /* > > + * If vcpu->guest_debug flag is set then do not check for > > + * shared->msr.DE as this debugging (say by QEMU) does not > > + * depends on shared->msr.de. In these scanerios MSR.DE is > > + * always set using shared_msr and should be handled always. > > + */ > > + lwz r7, VCPU_GUEST_DEBUG(r4) > > + cmpwi r7, 0 > > + bne skip_save_trap_event > > + PPC_LL r3, VCPU_SHARED(r4) > > + PPC_LD(r3, VCPU_SHARED_MSR, r3) > > + andi. r3, r3, MSR_DE > > + bne skip_save_trap_event > > + andis. r9, r9, DBSR_TIE@h > > +skip_save_trap_event: > > + stw r9, VCPU_DBSR(r4) > > +skip_dbsr_save: > > + mtspr SPRN_DBCR0, r8 > > +skip_load_host_debug: > > + > > /* Save remaining volatile guest register state to vcpu. */ > > stw r0, VCPU_GPR(R0)(r4) > > stw r1, VCPU_GPR(R1)(r4) > > @@ -468,6 +525,63 @@ lightweight_exit: > > PPC_LD(r3, VCPU_SHARED_SPRG7, r5) > > mtspr SPRN_SPRG7W, r3 > > > > + mfmsr r7 > > + rlwinm r7, r7, 0, ~MSR_DE > > + mtmsr r7 > > + lwz r6, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR0(r4) > > + rlwinm. r7, r6, 0, ~DBCR0_IDM > > + beq skip_load_guest_debug > > + mfspr r8, SPRN_DBCR0 > > + stw r8, VCPU_HOST_DBG+KVMPPC_DBG_DBCR0(r4) > > + andis. r3, r6, DBCR0_AC_BITS@h > > + beq skip_hw_bkpts > > + mfspr r7, SPRN_DBCR1 > > + stw r7, VCPU_HOST_DBG+KVMPPC_DBG_DBCR1(r4) > > + mfspr r8, SPRN_DBCR2 > > + stw r8, VCPU_HOST_DBG+KVMPPC_DBG_DBCR2(r4) > > + mfspr r7, SPRN_IAC1 > > + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC1, r4) > > + mfspr r8, SPRN_IAC2 > > + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC2, r4) #if > > +CONFIG_PPC_ADV_DEBUG_IACS > 2 > > + mfspr r7, SPRN_IAC3 > > + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) > > + mfspr r8, SPRN_IAC4 > > + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) #endif > > + mfspr r7, SPRN_DAC1 > > + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_DAC1, r4) > > + mfspr r8, SPRN_DAC2 > > + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_DAC2, r4) > > + li r8, 0 > > + mtspr SPRN_DBCR0, r8 /* disable all debug event */ > > + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR1, r4) > > + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR2, r4) > > + mtspr SPRN_DBCR1, r7 > > + mtspr SPRN_DBCR2, r8 > > + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC1, r4) > > + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC2, r4) > > + mtspr SPRN_IAC1, r7 > > + mtspr SPRN_IAC2, r8 > > +#if CONFIG_PPC_ADV_DEBUG_IACS > 2 > > + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC3, r4) > > + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC4, r4) > > + mtspr SPRN_IAC3, r7 > > + mtspr SPRN_IAC4, r8 > > +#endif > > + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_DAC1, r4) > > + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_DAC2, r4) > > + mtspr SPRN_DAC1, r7 > > + mtspr SPRN_DAC2, r8 > > +skip_hw_bkpts: > > + /* Clear if any deferred debug event */ > > + mfspr r8, SPRN_DBSR > > + mtspr SPRN_DBSR, r8 > > + isync > > + /* Restore guest DBCR */ > > + mtspr SPRN_DBCR0, r6 > > +skip_load_guest_debug: > > + > > #ifdef CONFIG_KVM_EXIT_TIMING > > /* save enter time */ > > 1: > > -- > > 1.7.0.4 > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > > the body of a message to majordomo@vger.kernel.org More majordomo info > > at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14.03.2013, at 05:50, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Thursday, March 07, 2013 6:56 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Bhushan >> Bharat-R65777 >> Subject: Re: [PATCH 4/7] booke: Save and restore debug registers on guest entry >> and exit >> >> >> On 28.02.2013, at 05:13, Bharat Bhushan wrote: >> >>> On Guest entry: if guest is wants to use the debug register then save >>> h/w debug register in host_dbg_reg and load the debug registers with >>> shadow_dbg_reg. Otherwise leave h/w debug registers as is. >> >> Why can't we switch the majority of registers on vcpu_put/get and only enable or >> disable debugging on guest entry/exit? > > > One of the reason for not doing this is that the KVM is a host kernel module and let this be debugged by host (I do not this how much useful this is :)) > So I am not able to recall the specific reason, maybe we have just coded this like this and tried to keep overhead as low as possible by switching registers only when they are used. My point is that the overhead is _higher_ this way, because we need to do checks and switches on every guest entry/exit, which happens a _lot_ more often than a host context switch. > As we discussed before, we can keep this option open for future. What future? Just ignore debug events in the entry/exit code path and suddenly a lot of the code becomes a lot easier. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> -----Original Message----- > >> From: Alexander Graf [mailto:agraf@suse.de] > >> Sent: Thursday, March 07, 2013 6:56 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; > >> Bhushan > >> Bharat-R65777 > >> Subject: Re: [PATCH 4/7] booke: Save and restore debug registers on > >> guest entry and exit > >> > >> > >> On 28.02.2013, at 05:13, Bharat Bhushan wrote: > >> > >>> On Guest entry: if guest is wants to use the debug register then > >>> save h/w debug register in host_dbg_reg and load the debug registers > >>> with shadow_dbg_reg. Otherwise leave h/w debug registers as is. > >> > >> Why can't we switch the majority of registers on vcpu_put/get and > >> only enable or disable debugging on guest entry/exit? > > > > > > One of the reason for not doing this is that the KVM is a host kernel > > module and let this be debugged by host (I do not this how much useful this is > :)) So I am not able to recall the specific reason, maybe we have just coded > this like this and tried to keep overhead as low as possible by switching > registers only when they are used. > > My point is that the overhead is _higher_ this way, because we need to do checks > and switches on every guest entry/exit, which happens a _lot_ more often than a > host context switch. > > > As we discussed before, we can keep this option open for future. > > What future? Just ignore debug events in the entry/exit code path and suddenly a > lot of the code becomes a lot easier. Just to summarize what we agreed upon: - Save/restore will happen on vcpu_get()/vcpu_put(). This will happen only if guest is using debug registers. Probably using a flag to indicate guest is using debug APU. - On debug register access from QEMU, always set value in h/w debug register. - On guest access of debug register, also save xxx h/w register in vcpu->host_debug_reg.xxx and load guest provided value in h/w debug register, ensure this happen on first access only, probably all debug registers once debug events enabled in dbcr0. Direct access from guest was not part of this patchset and support for this will be done separately. Thanks -Bharat > > > Alex > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index f4ba881..a9feeb0 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -504,7 +504,12 @@ struct kvm_vcpu_arch { u32 mmucfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct kvmppc_booke_debug_reg dbg_reg; + /* shadow debug registers */ + struct kvmppc_booke_debug_reg shadow_dbg_reg; + /* host debug registers*/ + struct kvmppc_booke_debug_reg host_dbg_reg; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c index 02048f3..22deda7 100644 --- a/arch/powerpc/kernel/asm-offsets.c +++ b/arch/powerpc/kernel/asm-offsets.c @@ -563,6 +563,32 @@ int main(void) DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear)); DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr)); DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save)); + DEFINE(VCPU_DBSR, offsetof(struct kvm_vcpu, arch.dbsr)); + DEFINE(VCPU_SHADOW_DBG, offsetof(struct kvm_vcpu, arch.shadow_dbg_reg)); + DEFINE(VCPU_HOST_DBG, offsetof(struct kvm_vcpu, arch.host_dbg_reg)); + DEFINE(KVMPPC_DBG_DBCR0, offsetof(struct kvmppc_booke_debug_reg, + dbcr0)); + DEFINE(KVMPPC_DBG_DBCR1, offsetof(struct kvmppc_booke_debug_reg, + dbcr1)); + DEFINE(KVMPPC_DBG_DBCR2, offsetof(struct kvmppc_booke_debug_reg, + dbcr2)); +#ifdef CONFIG_KVM_E500MC + DEFINE(KVMPPC_DBG_DBCR4, offsetof(struct kvmppc_booke_debug_reg, + dbcr4)); +#endif + DEFINE(KVMPPC_DBG_IAC1, offsetof(struct kvmppc_booke_debug_reg, + iac[0])); + DEFINE(KVMPPC_DBG_IAC2, offsetof(struct kvmppc_booke_debug_reg, + iac[1])); + DEFINE(KVMPPC_DBG_IAC3, offsetof(struct kvmppc_booke_debug_reg, + iac[2])); + DEFINE(KVMPPC_DBG_IAC4, offsetof(struct kvmppc_booke_debug_reg, + iac[3])); + DEFINE(KVMPPC_DBG_DAC1, offsetof(struct kvmppc_booke_debug_reg, + dac[0])); + DEFINE(KVMPPC_DBG_DAC2, offsetof(struct kvmppc_booke_debug_reg, + dac[1])); + DEFINE(VCPU_GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug)); #endif /* CONFIG_PPC_BOOK3S */ #endif /* CONFIG_KVM */ diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index 2c6deb5..6d78e01 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -39,6 +39,8 @@ #define HOST_MIN_STACK_SIZE (HOST_NV_GPR(R31) + 4) #define HOST_STACK_SIZE (((HOST_MIN_STACK_SIZE + 15) / 16) * 16) /* Align. */ #define HOST_STACK_LR (HOST_STACK_SIZE + 4) /* In caller stack frame. */ +#define DBCR0_AC_BITS (DBCR0_IAC1 | DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4 | \ + DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W) #define NEED_INST_MASK ((1<<BOOKE_INTERRUPT_PROGRAM) | \ (1<<BOOKE_INTERRUPT_DTLB_MISS) | \ @@ -54,6 +56,8 @@ (1<<BOOKE_INTERRUPT_DTLB_MISS) | \ (1<<BOOKE_INTERRUPT_ALIGNMENT)) +#define NEED_DEBUG_SAVE (1<<BOOKE_INTERRUPT_DEBUG) + .macro __KVM_HANDLER ivor_nr scratch srr0 /* Get pointer to vcpu and record exit number. */ mtspr \scratch , r4 @@ -215,6 +219,59 @@ _GLOBAL(kvmppc_resume_host) stw r9, VCPU_FAULT_ESR(r4) ..skip_esr: + lwz r9, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR0(r4) + rlwinm. r8, r9, 0, ~DBCR0_IDM + beq skip_load_host_debug + lwz r8, VCPU_HOST_DBG+KVMPPC_DBG_DBCR0(r4) + andis. r9, r9, DBCR0_AC_BITS@h + li r9, 0 + mtspr SPRN_DBCR0, r9 /* disable all debug event */ + beq skip_load_hw_bkpts + lwz r7, VCPU_HOST_DBG+KVMPPC_DBG_DBCR1(r4) + lwz r9, VCPU_HOST_DBG+KVMPPC_DBG_DBCR2(r4) + mtspr SPRN_DBCR1, r7 + mtspr SPRN_DBCR2, r9 + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC1, r4) + PPC_LD(r9, VCPU_HOST_DBG+KVMPPC_DBG_IAC2, r4) + mtspr SPRN_IAC1, r7 + mtspr SPRN_IAC2, r9 +#if CONFIG_PPC_ADV_DEBUG_IACS > 2 + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) + PPC_LD(r9, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) + mtspr SPRN_IAC3, r3 + mtspr SPRN_IAC4, r4 +#endif + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_DAC1, r4) + PPC_LD(r9, VCPU_HOST_DBG+KVMPPC_DBG_DAC2, r4) + mtspr SPRN_DAC1, r7 + mtspr SPRN_DAC2, r9 +skip_load_hw_bkpts: + /* Clear h/w DBSR and save current(guest) DBSR */ + mfspr r9, SPRN_DBSR + mtspr SPRN_DBSR, r9 + isync + andi. r7, r6, NEED_DEBUG_SAVE + beq skip_dbsr_save + /* + * If vcpu->guest_debug flag is set then do not check for + * shared->msr.DE as this debugging (say by QEMU) does not + * depends on shared->msr.de. In these scanerios MSR.DE is + * always set using shared_msr and should be handled always. + */ + lwz r7, VCPU_GUEST_DEBUG(r4) + cmpwi r7, 0 + bne skip_save_trap_event + PPC_LL r3, VCPU_SHARED(r4) + PPC_LD(r3, VCPU_SHARED_MSR, r3) + andi. r3, r3, MSR_DE + bne skip_save_trap_event + andis. r9, r9, DBSR_TIE@h +skip_save_trap_event: + stw r9, VCPU_DBSR(r4) +skip_dbsr_save: + mtspr SPRN_DBCR0, r8 +skip_load_host_debug: + /* Save remaining volatile guest register state to vcpu. */ stw r0, VCPU_GPR(R0)(r4) stw r1, VCPU_GPR(R1)(r4) @@ -468,6 +525,63 @@ lightweight_exit: PPC_LD(r3, VCPU_SHARED_SPRG7, r5) mtspr SPRN_SPRG7W, r3 + mfmsr r7 + rlwinm r7, r7, 0, ~MSR_DE + mtmsr r7 + lwz r6, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR0(r4) + rlwinm. r7, r6, 0, ~DBCR0_IDM + beq skip_load_guest_debug + mfspr r8, SPRN_DBCR0 + stw r8, VCPU_HOST_DBG+KVMPPC_DBG_DBCR0(r4) + andis. r3, r6, DBCR0_AC_BITS@h + beq skip_hw_bkpts + mfspr r7, SPRN_DBCR1 + stw r7, VCPU_HOST_DBG+KVMPPC_DBG_DBCR1(r4) + mfspr r8, SPRN_DBCR2 + stw r8, VCPU_HOST_DBG+KVMPPC_DBG_DBCR2(r4) + mfspr r7, SPRN_IAC1 + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC1, r4) + mfspr r8, SPRN_IAC2 + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC2, r4) +#if CONFIG_PPC_ADV_DEBUG_IACS > 2 + mfspr r7, SPRN_IAC3 + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) + mfspr r8, SPRN_IAC4 + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) +#endif + mfspr r7, SPRN_DAC1 + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_DAC1, r4) + mfspr r8, SPRN_DAC2 + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_DAC2, r4) + li r8, 0 + mtspr SPRN_DBCR0, r8 /* disable all debug event */ + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR1, r4) + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR2, r4) + mtspr SPRN_DBCR1, r7 + mtspr SPRN_DBCR2, r8 + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC1, r4) + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC2, r4) + mtspr SPRN_IAC1, r7 + mtspr SPRN_IAC2, r8 +#if CONFIG_PPC_ADV_DEBUG_IACS > 2 + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC3, r4) + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC4, r4) + mtspr SPRN_IAC3, r7 + mtspr SPRN_IAC4, r8 +#endif + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_DAC1, r4) + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_DAC2, r4) + mtspr SPRN_DAC1, r7 + mtspr SPRN_DAC2, r8 +skip_hw_bkpts: + /* Clear if any deferred debug event */ + mfspr r8, SPRN_DBSR + mtspr SPRN_DBSR, r8 + isync + /* Restore guest DBCR */ + mtspr SPRN_DBCR0, r6 +skip_load_guest_debug: + #ifdef CONFIG_KVM_EXIT_TIMING /* save enter time */ 1:
On Guest entry: if guest is wants to use the debug register then save h/w debug register in host_dbg_reg and load the debug registers with shadow_dbg_reg. Otherwise leave h/w debug registers as is. On guest exit: If guest/user-space is using the debug resource then restore the h/w debug register with host_dbg_reg. No need to save guest debug register as shadow_dbg_reg is having required values. If guest is not using the debug resources then no need to restore h/w registers. Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> --- arch/powerpc/include/asm/kvm_host.h | 5 ++ arch/powerpc/kernel/asm-offsets.c | 26 ++++++++ arch/powerpc/kvm/booke_interrupts.S | 114 +++++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 0 deletions(-)