| Message ID | 1358324685-30225-2-git-send-email-bharat.bhushan@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
On 16.01.2013, at 09:24, Bharat Bhushan wrote: > From: Bharat Bhushan <Bharat.Bhushan@freescale.com> > > Installed debug handler will be used for guest debug support > and debug facility emulation features (patches for these > features will follow this patch). > > Signed-off-by: Liu Yu <yu.liu@freescale.com> > [bharat.bhushan@freescale.com: Substantial changes] > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > --- > arch/powerpc/include/asm/kvm_host.h | 1 + > arch/powerpc/kernel/asm-offsets.c | 1 + > arch/powerpc/kvm/booke_interrupts.S | 49 ++++++++++++++++++++++++++++++----- > 3 files changed, 44 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 8a72d59..f4ba881 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { > u32 tlbcfg[4]; > u32 mmucfg; > u32 epr; > + u32 crit_save; > struct kvmppc_booke_debug_reg dbg_reg; > #endif > gpa_t paddr_accessed; > diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c > index 46f6afd..02048f3 100644 > --- a/arch/powerpc/kernel/asm-offsets.c > +++ b/arch/powerpc/kernel/asm-offsets.c > @@ -562,6 +562,7 @@ int main(void) > DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); > 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)); > #endif /* CONFIG_PPC_BOOK3S */ > #endif /* CONFIG_KVM */ > > diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S > index eae8483..dd9c5d4 100644 > --- a/arch/powerpc/kvm/booke_interrupts.S > +++ b/arch/powerpc/kvm/booke_interrupts.S > @@ -52,12 +52,7 @@ > (1<<BOOKE_INTERRUPT_PROGRAM) | \ > (1<<BOOKE_INTERRUPT_DTLB_MISS)) > > -.macro KVM_HANDLER ivor_nr scratch srr0 > -_GLOBAL(kvmppc_handler_\ivor_nr) > - /* Get pointer to vcpu and record exit number. */ > - mtspr \scratch , r4 > - mfspr r4, SPRN_SPRG_THREAD > - lwz r4, THREAD_KVM_VCPU(r4) > +.macro __KVM_HANDLER ivor_nr scratch srr0 > stw r3, VCPU_GPR(R3)(r4) > stw r5, VCPU_GPR(R5)(r4) > stw r6, VCPU_GPR(R6)(r4) > @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) > bctr > .endm > > +.macro KVM_HANDLER ivor_nr scratch srr0 > +_GLOBAL(kvmppc_handler_\ivor_nr) > + /* Get pointer to vcpu and record exit number. */ > + mtspr \scratch , r4 > + mfspr r4, SPRN_SPRG_THREAD > + lwz r4, THREAD_KVM_VCPU(r4) > + __KVM_HANDLER \ivor_nr \scratch \srr0 > +.endm > + > +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 > +_GLOBAL(kvmppc_handler_\ivor_nr) > + mtspr \scratch, r4 > + mfspr r4, SPRN_SPRG_THREAD > + lwz r4, THREAD_KVM_VCPU(r4) > + stw r3, VCPU_CRIT_SAVE(r4) > + mfcr r3 > + mfspr r4, SPRN_CSRR1 > + andi. r4, r4, MSR_PR > + bne 1f > + /* debug interrupt happened in enter/exit path */ > + mfspr r4, SPRN_CSRR1 > + rlwinm r4, r4, 0, ~MSR_DE > + mtspr SPRN_CSRR1, r4 > + lis r4, 0xffff > + ori r4, r4, 0xffff > + mtspr SPRN_DBSR, r4 > + mfspr r4, SPRN_SPRG_THREAD > + lwz r4, THREAD_KVM_VCPU(r4) > + mtcr r3 > + lwz r3, VCPU_CRIT_SAVE(r4) > + mfspr r4, \scratch > + rfci What is this part doing? Try to ignore the debug exit? Why would we have MSR_DE enabled in the first place when we can't handle it? > +1: /* debug interrupt happened in guest */ > + mtcr r3 > + mfspr r4, SPRN_SPRG_THREAD > + lwz r4, THREAD_KVM_VCPU(r4) > + lwz r3, VCPU_CRIT_SAVE(r4) > + __KVM_HANDLER \ivor_nr \scratch \srr0 I don't think you need the __KVM_HANDLER split. This should be quite easily refactorable into a simple DBG prolog. Alex > +.endm > + > .macro KVM_HANDLER_ADDR ivor_nr > .long kvmppc_handler_\ivor_nr > .endm > @@ -98,7 +133,7 @@ KVM_HANDLER BOOKE_INTERRUPT_FIT SPRN_SPRG_RSCRATCH0 SPRN_SRR0 > KVM_HANDLER BOOKE_INTERRUPT_WATCHDOG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0 > KVM_HANDLER BOOKE_INTERRUPT_DTLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0 > KVM_HANDLER BOOKE_INTERRUPT_ITLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0 > -KVM_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0 > +KVM_DBG_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0 > KVM_HANDLER BOOKE_INTERRUPT_SPE_UNAVAIL SPRN_SPRG_RSCRATCH0 SPRN_SRR0 > KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_DATA SPRN_SPRG_RSCRATCH0 SPRN_SRR0 > KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_ROUND SPRN_SPRG_RSCRATCH0 SPRN_SRR0 > -- > 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
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Friday, January 25, 2013 5:13 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777 > Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler > > > On 16.01.2013, at 09:24, Bharat Bhushan wrote: > > > From: Bharat Bhushan <Bharat.Bhushan@freescale.com> > > > > Installed debug handler will be used for guest debug support and debug > > facility emulation features (patches for these features will follow > > this patch). > > > > Signed-off-by: Liu Yu <yu.liu@freescale.com> > > [bharat.bhushan@freescale.com: Substantial changes] > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > > --- > > arch/powerpc/include/asm/kvm_host.h | 1 + > > arch/powerpc/kernel/asm-offsets.c | 1 + > > arch/powerpc/kvm/booke_interrupts.S | 49 ++++++++++++++++++++++++++++++----- > > 3 files changed, 44 insertions(+), 7 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/kvm_host.h > > b/arch/powerpc/include/asm/kvm_host.h > > index 8a72d59..f4ba881 100644 > > --- a/arch/powerpc/include/asm/kvm_host.h > > +++ b/arch/powerpc/include/asm/kvm_host.h > > @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { > > u32 tlbcfg[4]; > > u32 mmucfg; > > u32 epr; > > + u32 crit_save; > > struct kvmppc_booke_debug_reg dbg_reg; #endif > > gpa_t paddr_accessed; > > diff --git a/arch/powerpc/kernel/asm-offsets.c > > b/arch/powerpc/kernel/asm-offsets.c > > index 46f6afd..02048f3 100644 > > --- a/arch/powerpc/kernel/asm-offsets.c > > +++ b/arch/powerpc/kernel/asm-offsets.c > > @@ -562,6 +562,7 @@ int main(void) > > DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); > > 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)); > > #endif /* CONFIG_PPC_BOOK3S */ > > #endif /* CONFIG_KVM */ > > > > diff --git a/arch/powerpc/kvm/booke_interrupts.S > > b/arch/powerpc/kvm/booke_interrupts.S > > index eae8483..dd9c5d4 100644 > > --- a/arch/powerpc/kvm/booke_interrupts.S > > +++ b/arch/powerpc/kvm/booke_interrupts.S > > @@ -52,12 +52,7 @@ > > (1<<BOOKE_INTERRUPT_PROGRAM) | \ > > (1<<BOOKE_INTERRUPT_DTLB_MISS)) > > > > -.macro KVM_HANDLER ivor_nr scratch srr0 > > -_GLOBAL(kvmppc_handler_\ivor_nr) > > - /* Get pointer to vcpu and record exit number. */ > > - mtspr \scratch , r4 > > - mfspr r4, SPRN_SPRG_THREAD > > - lwz r4, THREAD_KVM_VCPU(r4) > > +.macro __KVM_HANDLER ivor_nr scratch srr0 > > stw r3, VCPU_GPR(R3)(r4) > > stw r5, VCPU_GPR(R5)(r4) > > stw r6, VCPU_GPR(R6)(r4) > > @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) > > bctr > > .endm > > > > +.macro KVM_HANDLER ivor_nr scratch srr0 > > +_GLOBAL(kvmppc_handler_\ivor_nr) > > + /* Get pointer to vcpu and record exit number. */ > > + mtspr \scratch , r4 > > + mfspr r4, SPRN_SPRG_THREAD > > + lwz r4, THREAD_KVM_VCPU(r4) > > + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > > + > > +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 > > +_GLOBAL(kvmppc_handler_\ivor_nr) > > + mtspr \scratch, r4 > > + mfspr r4, SPRN_SPRG_THREAD > > + lwz r4, THREAD_KVM_VCPU(r4) > > + stw r3, VCPU_CRIT_SAVE(r4) > > + mfcr r3 > > + mfspr r4, SPRN_CSRR1 > > + andi. r4, r4, MSR_PR > > + bne 1f > > > > + /* debug interrupt happened in enter/exit path */ > > + mfspr r4, SPRN_CSRR1 > > + rlwinm r4, r4, 0, ~MSR_DE > > + mtspr SPRN_CSRR1, r4 > > + lis r4, 0xffff > > + ori r4, r4, 0xffff > > + mtspr SPRN_DBSR, r4 > > + mfspr r4, SPRN_SPRG_THREAD > > + lwz r4, THREAD_KVM_VCPU(r4) > > + mtcr r3 > > + lwz r3, VCPU_CRIT_SAVE(r4) > > + mfspr r4, \scratch > > + rfci > > What is this part doing? Try to ignore the debug exit? As BOOKE doesn't have hardware support for virtualization, hardware never know current pc is in guest or in host. So when enable hardware single step for guest, it cannot be disabled at the time guest exit. Thus, we'll see that an single step interrupt happens at the beginning of guest exit path. With the above code we recognize this kind of single step interrupt disable single step and rfci. > Why would we have MSR_DE > enabled in the first place when we can't handle it? When QEMU is using hardware debug resource then we always set MSR_DE during guest is running. > > > +1: /* debug interrupt happened in guest */ > > + mtcr r3 > > + mfspr r4, SPRN_SPRG_THREAD > > + lwz r4, THREAD_KVM_VCPU(r4) > > + lwz r3, VCPU_CRIT_SAVE(r4) > > + __KVM_HANDLER \ivor_nr \scratch \srr0 > > I don't think you need the __KVM_HANDLER split. This should be quite easily > refactorable into a simple DBG prolog. Can you please elaborate how you are envisioning this? Thanks -Bharat > > > Alex > > > +.endm > > + > > .macro KVM_HANDLER_ADDR ivor_nr > > .long kvmppc_handler_\ivor_nr > > .endm > > @@ -98,7 +133,7 @@ KVM_HANDLER BOOKE_INTERRUPT_FIT SPRN_SPRG_RSCRATCH0 > > SPRN_SRR0 KVM_HANDLER BOOKE_INTERRUPT_WATCHDOG SPRN_SPRG_RSCRATCH_CRIT > > SPRN_CSRR0 KVM_HANDLER BOOKE_INTERRUPT_DTLB_MISS SPRN_SPRG_RSCRATCH0 > > SPRN_SRR0 KVM_HANDLER BOOKE_INTERRUPT_ITLB_MISS SPRN_SPRG_RSCRATCH0 > > SPRN_SRR0 -KVM_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT > > SPRN_CSRR0 > > +KVM_DBG_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT > > +SPRN_CSRR0 > > KVM_HANDLER BOOKE_INTERRUPT_SPE_UNAVAIL SPRN_SPRG_RSCRATCH0 SPRN_SRR0 > > KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_DATA SPRN_SPRG_RSCRATCH0 SPRN_SRR0 > > KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_ROUND SPRN_SPRG_RSCRATCH0 SPRN_SRR0 > > -- > > 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
On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Friday, January 25, 2013 5:13 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777 >> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >> >> >> On 16.01.2013, at 09:24, Bharat Bhushan wrote: >> >>> From: Bharat Bhushan <Bharat.Bhushan@freescale.com> >>> >>> Installed debug handler will be used for guest debug support and debug >>> facility emulation features (patches for these features will follow >>> this patch). >>> >>> Signed-off-by: Liu Yu <yu.liu@freescale.com> >>> [bharat.bhushan@freescale.com: Substantial changes] >>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> >>> --- >>> arch/powerpc/include/asm/kvm_host.h | 1 + >>> arch/powerpc/kernel/asm-offsets.c | 1 + >>> arch/powerpc/kvm/booke_interrupts.S | 49 ++++++++++++++++++++++++++++++----- >>> 3 files changed, 44 insertions(+), 7 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/kvm_host.h >>> b/arch/powerpc/include/asm/kvm_host.h >>> index 8a72d59..f4ba881 100644 >>> --- a/arch/powerpc/include/asm/kvm_host.h >>> +++ b/arch/powerpc/include/asm/kvm_host.h >>> @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { >>> u32 tlbcfg[4]; >>> u32 mmucfg; >>> u32 epr; >>> + u32 crit_save; >>> struct kvmppc_booke_debug_reg dbg_reg; #endif >>> gpa_t paddr_accessed; >>> diff --git a/arch/powerpc/kernel/asm-offsets.c >>> b/arch/powerpc/kernel/asm-offsets.c >>> index 46f6afd..02048f3 100644 >>> --- a/arch/powerpc/kernel/asm-offsets.c >>> +++ b/arch/powerpc/kernel/asm-offsets.c >>> @@ -562,6 +562,7 @@ int main(void) >>> DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); >>> 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)); >>> #endif /* CONFIG_PPC_BOOK3S */ >>> #endif /* CONFIG_KVM */ >>> >>> diff --git a/arch/powerpc/kvm/booke_interrupts.S >>> b/arch/powerpc/kvm/booke_interrupts.S >>> index eae8483..dd9c5d4 100644 >>> --- a/arch/powerpc/kvm/booke_interrupts.S >>> +++ b/arch/powerpc/kvm/booke_interrupts.S >>> @@ -52,12 +52,7 @@ >>> (1<<BOOKE_INTERRUPT_PROGRAM) | \ >>> (1<<BOOKE_INTERRUPT_DTLB_MISS)) >>> >>> -.macro KVM_HANDLER ivor_nr scratch srr0 >>> -_GLOBAL(kvmppc_handler_\ivor_nr) >>> - /* Get pointer to vcpu and record exit number. */ >>> - mtspr \scratch , r4 >>> - mfspr r4, SPRN_SPRG_THREAD >>> - lwz r4, THREAD_KVM_VCPU(r4) >>> +.macro __KVM_HANDLER ivor_nr scratch srr0 >>> stw r3, VCPU_GPR(R3)(r4) >>> stw r5, VCPU_GPR(R5)(r4) >>> stw r6, VCPU_GPR(R6)(r4) >>> @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) >>> bctr >>> .endm >>> >>> +.macro KVM_HANDLER ivor_nr scratch srr0 >>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>> + /* Get pointer to vcpu and record exit number. */ >>> + mtspr \scratch , r4 >>> + mfspr r4, SPRN_SPRG_THREAD >>> + lwz r4, THREAD_KVM_VCPU(r4) >>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>> + >>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>> + mtspr \scratch, r4 >>> + mfspr r4, SPRN_SPRG_THREAD >>> + lwz r4, THREAD_KVM_VCPU(r4) >>> + stw r3, VCPU_CRIT_SAVE(r4) >>> + mfcr r3 >>> + mfspr r4, SPRN_CSRR1 >>> + andi. r4, r4, MSR_PR >>> + bne 1f >> >> >>> + /* debug interrupt happened in enter/exit path */ >>> + mfspr r4, SPRN_CSRR1 >>> + rlwinm r4, r4, 0, ~MSR_DE >>> + mtspr SPRN_CSRR1, r4 >>> + lis r4, 0xffff >>> + ori r4, r4, 0xffff >>> + mtspr SPRN_DBSR, r4 >>> + mfspr r4, SPRN_SPRG_THREAD >>> + lwz r4, THREAD_KVM_VCPU(r4) >>> + mtcr r3 >>> + lwz r3, VCPU_CRIT_SAVE(r4) >>> + mfspr r4, \scratch >>> + rfci >> >> What is this part doing? Try to ignore the debug exit? > > As BOOKE doesn't have hardware support for virtualization, hardware never know current pc is in guest or in host. > So when enable hardware single step for guest, it cannot be disabled at the time guest exit. Thus, we'll see that an single step interrupt happens at the beginning of guest exit path. > > With the above code we recognize this kind of single step interrupt disable single step and rfci. > >> Why would we have MSR_DE >> enabled in the first place when we can't handle it? > > When QEMU is using hardware debug resource then we always set MSR_DE during guest is running. Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't set, you wouldn't get a single step exit. During the exit code path, you could then swap DBSR back to what the host expects (which means no single step). Only after that enable MSR_DE again. > >> >>> +1: /* debug interrupt happened in guest */ >>> + mtcr r3 >>> + mfspr r4, SPRN_SPRG_THREAD >>> + lwz r4, THREAD_KVM_VCPU(r4) >>> + lwz r3, VCPU_CRIT_SAVE(r4) >>> + __KVM_HANDLER \ivor_nr \scratch \srr0 >> >> I don't think you need the __KVM_HANDLER split. This should be quite easily >> refactorable into a simple DBG prolog. > > Can you please elaborate how you are envisioning this? With this patch, you have KVM_HANLDER: <code> __KVM_HANDLER KVM_DBG_HANDLER: <code> __KVM_HANDLER Right? In KVM_HANDLER, you get: > .macro KVM_HANDLER ivor_nr scratch srr0 > _GLOBAL(kvmppc_handler_\ivor_nr) > /* Get pointer to vcpu and record exit number. */ > mtspr \scratch , r4 > mfspr r4, SPRN_SPRG_THREAD > lwz r4, THREAD_KVM_VCPU(r4) > __KVM_HANDLER \ivor_nr \scratch \srr0 > .endm while KVM_DBG_HANDLER is: > +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 > +_GLOBAL(kvmppc_handler_\ivor_nr) > <debug specific handling> > +1: /* debug interrupt happened in guest */ > + mtcr r3 > + mfspr r4, SPRN_SPRG_THREAD > + lwz r4, THREAD_KVM_VCPU(r4) > + lwz r3, VCPU_CRIT_SAVE(r4) > + __KVM_HANDLER \ivor_nr \scratch \srr0 > +.endm So if you write this as KVM_DBG_HANDLER: <debug specific handling> 1: mtcr r3 mfspr r4, SPRN_SPRG_THREAD lwz r4, THREAD_KVM_VCPU(r4) lwz r3, VCPU_CRIT_SAVE(r4) lwz r4, \scratch <KVM_HANDLER> then you get code that is slower :) but it should be easier to read, since the interface between the individual pieces is always the same. Debug shouldn't be a fast path anyway, right? 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, January 31, 2013 5:47 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org > Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler > > > On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote: > > > > > > >> -----Original Message----- > >> From: Alexander Graf [mailto:agraf@suse.de] > >> Sent: Friday, January 25, 2013 5:13 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan > >> Bharat-R65777 > >> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler > >> > >> > >> On 16.01.2013, at 09:24, Bharat Bhushan wrote: > >> > >>> From: Bharat Bhushan <Bharat.Bhushan@freescale.com> > >>> > >>> Installed debug handler will be used for guest debug support and > >>> debug facility emulation features (patches for these features will > >>> follow this patch). > >>> > >>> Signed-off-by: Liu Yu <yu.liu@freescale.com> > >>> [bharat.bhushan@freescale.com: Substantial changes] > >>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > >>> --- > >>> arch/powerpc/include/asm/kvm_host.h | 1 + > >>> arch/powerpc/kernel/asm-offsets.c | 1 + > >>> arch/powerpc/kvm/booke_interrupts.S | 49 ++++++++++++++++++++++++++++++--- > -- > >>> 3 files changed, 44 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/arch/powerpc/include/asm/kvm_host.h > >>> b/arch/powerpc/include/asm/kvm_host.h > >>> index 8a72d59..f4ba881 100644 > >>> --- a/arch/powerpc/include/asm/kvm_host.h > >>> +++ b/arch/powerpc/include/asm/kvm_host.h > >>> @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { > >>> u32 tlbcfg[4]; > >>> u32 mmucfg; > >>> u32 epr; > >>> + u32 crit_save; > >>> struct kvmppc_booke_debug_reg dbg_reg; #endif > >>> gpa_t paddr_accessed; > >>> diff --git a/arch/powerpc/kernel/asm-offsets.c > >>> b/arch/powerpc/kernel/asm-offsets.c > >>> index 46f6afd..02048f3 100644 > >>> --- a/arch/powerpc/kernel/asm-offsets.c > >>> +++ b/arch/powerpc/kernel/asm-offsets.c > >>> @@ -562,6 +562,7 @@ int main(void) > >>> DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); > >>> 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)); > >>> #endif /* CONFIG_PPC_BOOK3S */ > >>> #endif /* CONFIG_KVM */ > >>> > >>> diff --git a/arch/powerpc/kvm/booke_interrupts.S > >>> b/arch/powerpc/kvm/booke_interrupts.S > >>> index eae8483..dd9c5d4 100644 > >>> --- a/arch/powerpc/kvm/booke_interrupts.S > >>> +++ b/arch/powerpc/kvm/booke_interrupts.S > >>> @@ -52,12 +52,7 @@ > >>> (1<<BOOKE_INTERRUPT_PROGRAM) | \ > >>> (1<<BOOKE_INTERRUPT_DTLB_MISS)) > >>> > >>> -.macro KVM_HANDLER ivor_nr scratch srr0 > >>> -_GLOBAL(kvmppc_handler_\ivor_nr) > >>> - /* Get pointer to vcpu and record exit number. */ > >>> - mtspr \scratch , r4 > >>> - mfspr r4, SPRN_SPRG_THREAD > >>> - lwz r4, THREAD_KVM_VCPU(r4) > >>> +.macro __KVM_HANDLER ivor_nr scratch srr0 > >>> stw r3, VCPU_GPR(R3)(r4) > >>> stw r5, VCPU_GPR(R5)(r4) > >>> stw r6, VCPU_GPR(R6)(r4) > >>> @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) > >>> bctr > >>> .endm > >>> > >>> +.macro KVM_HANDLER ivor_nr scratch srr0 > >>> +_GLOBAL(kvmppc_handler_\ivor_nr) > >>> + /* Get pointer to vcpu and record exit number. */ > >>> + mtspr \scratch , r4 > >>> + mfspr r4, SPRN_SPRG_THREAD > >>> + lwz r4, THREAD_KVM_VCPU(r4) > >>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > >>> + > >>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 > >>> +_GLOBAL(kvmppc_handler_\ivor_nr) > >>> + mtspr \scratch, r4 > >>> + mfspr r4, SPRN_SPRG_THREAD > >>> + lwz r4, THREAD_KVM_VCPU(r4) > >>> + stw r3, VCPU_CRIT_SAVE(r4) > >>> + mfcr r3 > >>> + mfspr r4, SPRN_CSRR1 > >>> + andi. r4, r4, MSR_PR > >>> + bne 1f > >> > >> > >>> + /* debug interrupt happened in enter/exit path */ > >>> + mfspr r4, SPRN_CSRR1 > >>> + rlwinm r4, r4, 0, ~MSR_DE > >>> + mtspr SPRN_CSRR1, r4 > >>> + lis r4, 0xffff > >>> + ori r4, r4, 0xffff > >>> + mtspr SPRN_DBSR, r4 > >>> + mfspr r4, SPRN_SPRG_THREAD > >>> + lwz r4, THREAD_KVM_VCPU(r4) > >>> + mtcr r3 > >>> + lwz r3, VCPU_CRIT_SAVE(r4) > >>> + mfspr r4, \scratch > >>> + rfci > >> > >> What is this part doing? Try to ignore the debug exit? > > > > As BOOKE doesn't have hardware support for virtualization, hardware never know > current pc is in guest or in host. > > So when enable hardware single step for guest, it cannot be disabled at the > time guest exit. Thus, we'll see that an single step interrupt happens at the > beginning of guest exit path. > > > > With the above code we recognize this kind of single step interrupt disable > single step and rfci. > > > >> Why would we have MSR_DE > >> enabled in the first place when we can't handle it? > > > > When QEMU is using hardware debug resource then we always set MSR_DE during > guest is running. > > Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't set, you > wouldn't get a single step exit. We always set MSR_DE in hw MSR when qemu using the debug resource. > During the exit code path, you could then swap > DBSR back to what the host expects (which means no single step). Only after that > enable MSR_DE again. We do not support deferred debug interrupt, so we do save restore dbsr. > > > > >> > >>> +1: /* debug interrupt happened in guest */ > >>> + mtcr r3 > >>> + mfspr r4, SPRN_SPRG_THREAD > >>> + lwz r4, THREAD_KVM_VCPU(r4) > >>> + lwz r3, VCPU_CRIT_SAVE(r4) > >>> + __KVM_HANDLER \ivor_nr \scratch \srr0 > >> > >> I don't think you need the __KVM_HANDLER split. This should be quite > >> easily refactorable into a simple DBG prolog. > > > > Can you please elaborate how you are envisioning this? > > With this patch, you have > > KVM_HANLDER: > > <code> > __KVM_HANDLER > > KVM_DBG_HANDLER: > > <code> > __KVM_HANDLER > > Right? > > In KVM_HANDLER, you get: > > > .macro KVM_HANDLER ivor_nr scratch srr0 > > _GLOBAL(kvmppc_handler_\ivor_nr) > > /* Get pointer to vcpu and record exit number. */ > > mtspr \scratch , r4 > > mfspr r4, SPRN_SPRG_THREAD > > lwz r4, THREAD_KVM_VCPU(r4) > > __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > > > while KVM_DBG_HANDLER is: > > > +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 > > +_GLOBAL(kvmppc_handler_\ivor_nr) > > <debug specific handling> > > +1: /* debug interrupt happened in guest */ > > + mtcr r3 > > + mfspr r4, SPRN_SPRG_THREAD > > + lwz r4, THREAD_KVM_VCPU(r4) > > + lwz r3, VCPU_CRIT_SAVE(r4) > > + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > > > So if you write this as > > KVM_DBG_HANDLER: > <debug specific handling> > 1: > mtcr r3 > mfspr r4, SPRN_SPRG_THREAD > lwz r4, THREAD_KVM_VCPU(r4) > lwz r3, VCPU_CRIT_SAVE(r4) > lwz r4, \scratch > <KVM_HANDLER> > > then you get code that is slower :) but it should be easier to read, since the > interface between the individual pieces is always the same. Debug shouldn't be a > fast path anyway, right? Frankly speaking I do not see much difference :). If we have to do as you mentioned then I think we can just do KVM_DBG_HANDLER: <debug specific handling> 1: mtcr r3 lwz r3, VCPU_CRIT_SAVE(r4) lwz r4, \scratch <KVM_HANDLER> 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
On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Thursday, January 31, 2013 5:47 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >> >> >> On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote: >> >>> >>> >>>> -----Original Message----- >>>> From: Alexander Graf [mailto:agraf@suse.de] >>>> Sent: Friday, January 25, 2013 5:13 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan >>>> Bharat-R65777 >>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >>>> >>>> >>>> On 16.01.2013, at 09:24, Bharat Bhushan wrote: >>>> >>>>> From: Bharat Bhushan <Bharat.Bhushan@freescale.com> >>>>> >>>>> Installed debug handler will be used for guest debug support and >>>>> debug facility emulation features (patches for these features will >>>>> follow this patch). >>>>> >>>>> Signed-off-by: Liu Yu <yu.liu@freescale.com> >>>>> [bharat.bhushan@freescale.com: Substantial changes] >>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> >>>>> --- >>>>> arch/powerpc/include/asm/kvm_host.h | 1 + >>>>> arch/powerpc/kernel/asm-offsets.c | 1 + >>>>> arch/powerpc/kvm/booke_interrupts.S | 49 ++++++++++++++++++++++++++++++--- >> -- >>>>> 3 files changed, 44 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h >>>>> b/arch/powerpc/include/asm/kvm_host.h >>>>> index 8a72d59..f4ba881 100644 >>>>> --- a/arch/powerpc/include/asm/kvm_host.h >>>>> +++ b/arch/powerpc/include/asm/kvm_host.h >>>>> @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { >>>>> u32 tlbcfg[4]; >>>>> u32 mmucfg; >>>>> u32 epr; >>>>> + u32 crit_save; >>>>> struct kvmppc_booke_debug_reg dbg_reg; #endif >>>>> gpa_t paddr_accessed; >>>>> diff --git a/arch/powerpc/kernel/asm-offsets.c >>>>> b/arch/powerpc/kernel/asm-offsets.c >>>>> index 46f6afd..02048f3 100644 >>>>> --- a/arch/powerpc/kernel/asm-offsets.c >>>>> +++ b/arch/powerpc/kernel/asm-offsets.c >>>>> @@ -562,6 +562,7 @@ int main(void) >>>>> DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); >>>>> 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)); >>>>> #endif /* CONFIG_PPC_BOOK3S */ >>>>> #endif /* CONFIG_KVM */ >>>>> >>>>> diff --git a/arch/powerpc/kvm/booke_interrupts.S >>>>> b/arch/powerpc/kvm/booke_interrupts.S >>>>> index eae8483..dd9c5d4 100644 >>>>> --- a/arch/powerpc/kvm/booke_interrupts.S >>>>> +++ b/arch/powerpc/kvm/booke_interrupts.S >>>>> @@ -52,12 +52,7 @@ >>>>> (1<<BOOKE_INTERRUPT_PROGRAM) | \ >>>>> (1<<BOOKE_INTERRUPT_DTLB_MISS)) >>>>> >>>>> -.macro KVM_HANDLER ivor_nr scratch srr0 >>>>> -_GLOBAL(kvmppc_handler_\ivor_nr) >>>>> - /* Get pointer to vcpu and record exit number. */ >>>>> - mtspr \scratch , r4 >>>>> - mfspr r4, SPRN_SPRG_THREAD >>>>> - lwz r4, THREAD_KVM_VCPU(r4) >>>>> +.macro __KVM_HANDLER ivor_nr scratch srr0 >>>>> stw r3, VCPU_GPR(R3)(r4) >>>>> stw r5, VCPU_GPR(R5)(r4) >>>>> stw r6, VCPU_GPR(R6)(r4) >>>>> @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) >>>>> bctr >>>>> .endm >>>>> >>>>> +.macro KVM_HANDLER ivor_nr scratch srr0 >>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>> + /* Get pointer to vcpu and record exit number. */ >>>>> + mtspr \scratch , r4 >>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>>> + >>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>> + mtspr \scratch, r4 >>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>> + stw r3, VCPU_CRIT_SAVE(r4) >>>>> + mfcr r3 >>>>> + mfspr r4, SPRN_CSRR1 >>>>> + andi. r4, r4, MSR_PR >>>>> + bne 1f >>>> >>>> >>>>> + /* debug interrupt happened in enter/exit path */ >>>>> + mfspr r4, SPRN_CSRR1 >>>>> + rlwinm r4, r4, 0, ~MSR_DE >>>>> + mtspr SPRN_CSRR1, r4 >>>>> + lis r4, 0xffff >>>>> + ori r4, r4, 0xffff >>>>> + mtspr SPRN_DBSR, r4 >>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>> + mtcr r3 >>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>> + mfspr r4, \scratch >>>>> + rfci >>>> >>>> What is this part doing? Try to ignore the debug exit? >>> >>> As BOOKE doesn't have hardware support for virtualization, hardware never know >> current pc is in guest or in host. >>> So when enable hardware single step for guest, it cannot be disabled at the >> time guest exit. Thus, we'll see that an single step interrupt happens at the >> beginning of guest exit path. >>> >>> With the above code we recognize this kind of single step interrupt disable >> single step and rfci. >>> >>>> Why would we have MSR_DE >>>> enabled in the first place when we can't handle it? >>> >>> When QEMU is using hardware debug resource then we always set MSR_DE during >> guest is running. >> >> Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't set, you >> wouldn't get a single step exit. > > We always set MSR_DE in hw MSR when qemu using the debug resource. In the _guest_ MSR, yes. But once we exit the guest, it shouldn't be set anymore, because we're in an interrupt handler, no? Or is MSR_DE kept alive on interrupts? > >> During the exit code path, you could then swap >> DBSR back to what the host expects (which means no single step). Only after that >> enable MSR_DE again. > > We do not support deferred debug interrupt, so we do save restore dbsr. > >> >>> >>>> >>>>> +1: /* debug interrupt happened in guest */ >>>>> + mtcr r3 >>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 >>>> >>>> I don't think you need the __KVM_HANDLER split. This should be quite >>>> easily refactorable into a simple DBG prolog. >>> >>> Can you please elaborate how you are envisioning this? >> >> With this patch, you have >> >> KVM_HANLDER: >> >> <code> >> __KVM_HANDLER >> >> KVM_DBG_HANDLER: >> >> <code> >> __KVM_HANDLER >> >> Right? >> >> In KVM_HANDLER, you get: >> >>> .macro KVM_HANDLER ivor_nr scratch srr0 >>> _GLOBAL(kvmppc_handler_\ivor_nr) >>> /* Get pointer to vcpu and record exit number. */ >>> mtspr \scratch , r4 >>> mfspr r4, SPRN_SPRG_THREAD >>> lwz r4, THREAD_KVM_VCPU(r4) >>> __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >> >> >> while KVM_DBG_HANDLER is: >> >>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>> <debug specific handling> >>> +1: /* debug interrupt happened in guest */ >>> + mtcr r3 >>> + mfspr r4, SPRN_SPRG_THREAD >>> + lwz r4, THREAD_KVM_VCPU(r4) >>> + lwz r3, VCPU_CRIT_SAVE(r4) >>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >> >> >> So if you write this as >> >> KVM_DBG_HANDLER: >> <debug specific handling> >> 1: >> mtcr r3 >> mfspr r4, SPRN_SPRG_THREAD >> lwz r4, THREAD_KVM_VCPU(r4) >> lwz r3, VCPU_CRIT_SAVE(r4) >> lwz r4, \scratch >> <KVM_HANDLER> >> >> then you get code that is slower :) but it should be easier to read, since the >> interface between the individual pieces is always the same. Debug shouldn't be a >> fast path anyway, right? > > Frankly speaking I do not see much difference :). > > If we have to do as you mentioned then I think we can just do > > KVM_DBG_HANDLER: > <debug specific handling> > 1: > mtcr r3 > lwz r3, VCPU_CRIT_SAVE(r4) > lwz r4, \scratch > <KVM_HANDLER> Whatever it takes to keep the oddball (debug) an oddball and keep the normal case easy :). 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
On 31.01.2013, at 18:08, Alexander Graf wrote: > > On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote: > >> >> >>> -----Original Message----- >>> From: Alexander Graf [mailto:agraf@suse.de] >>> Sent: Thursday, January 31, 2013 5:47 PM >>> To: Bhushan Bharat-R65777 >>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >>> >>> >>> On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote: >>> >>>> >>>> >>>>> -----Original Message----- >>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>> Sent: Friday, January 25, 2013 5:13 PM >>>>> To: Bhushan Bharat-R65777 >>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan >>>>> Bharat-R65777 >>>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >>>>> >>>>> >>>>> On 16.01.2013, at 09:24, Bharat Bhushan wrote: >>>>> >>>>>> From: Bharat Bhushan <Bharat.Bhushan@freescale.com> >>>>>> >>>>>> Installed debug handler will be used for guest debug support and >>>>>> debug facility emulation features (patches for these features will >>>>>> follow this patch). >>>>>> >>>>>> Signed-off-by: Liu Yu <yu.liu@freescale.com> >>>>>> [bharat.bhushan@freescale.com: Substantial changes] >>>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> >>>>>> --- >>>>>> arch/powerpc/include/asm/kvm_host.h | 1 + >>>>>> arch/powerpc/kernel/asm-offsets.c | 1 + >>>>>> arch/powerpc/kvm/booke_interrupts.S | 49 ++++++++++++++++++++++++++++++--- >>> -- >>>>>> 3 files changed, 44 insertions(+), 7 deletions(-) >>>>>> >>>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h >>>>>> b/arch/powerpc/include/asm/kvm_host.h >>>>>> index 8a72d59..f4ba881 100644 >>>>>> --- a/arch/powerpc/include/asm/kvm_host.h >>>>>> +++ b/arch/powerpc/include/asm/kvm_host.h >>>>>> @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { >>>>>> u32 tlbcfg[4]; >>>>>> u32 mmucfg; >>>>>> u32 epr; >>>>>> + u32 crit_save; >>>>>> struct kvmppc_booke_debug_reg dbg_reg; #endif >>>>>> gpa_t paddr_accessed; >>>>>> diff --git a/arch/powerpc/kernel/asm-offsets.c >>>>>> b/arch/powerpc/kernel/asm-offsets.c >>>>>> index 46f6afd..02048f3 100644 >>>>>> --- a/arch/powerpc/kernel/asm-offsets.c >>>>>> +++ b/arch/powerpc/kernel/asm-offsets.c >>>>>> @@ -562,6 +562,7 @@ int main(void) >>>>>> DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); >>>>>> 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)); >>>>>> #endif /* CONFIG_PPC_BOOK3S */ >>>>>> #endif /* CONFIG_KVM */ >>>>>> >>>>>> diff --git a/arch/powerpc/kvm/booke_interrupts.S >>>>>> b/arch/powerpc/kvm/booke_interrupts.S >>>>>> index eae8483..dd9c5d4 100644 >>>>>> --- a/arch/powerpc/kvm/booke_interrupts.S >>>>>> +++ b/arch/powerpc/kvm/booke_interrupts.S >>>>>> @@ -52,12 +52,7 @@ >>>>>> (1<<BOOKE_INTERRUPT_PROGRAM) | \ >>>>>> (1<<BOOKE_INTERRUPT_DTLB_MISS)) >>>>>> >>>>>> -.macro KVM_HANDLER ivor_nr scratch srr0 >>>>>> -_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>> - /* Get pointer to vcpu and record exit number. */ >>>>>> - mtspr \scratch , r4 >>>>>> - mfspr r4, SPRN_SPRG_THREAD >>>>>> - lwz r4, THREAD_KVM_VCPU(r4) >>>>>> +.macro __KVM_HANDLER ivor_nr scratch srr0 >>>>>> stw r3, VCPU_GPR(R3)(r4) >>>>>> stw r5, VCPU_GPR(R5)(r4) >>>>>> stw r6, VCPU_GPR(R6)(r4) >>>>>> @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) >>>>>> bctr >>>>>> .endm >>>>>> >>>>>> +.macro KVM_HANDLER ivor_nr scratch srr0 >>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>> + /* Get pointer to vcpu and record exit number. */ >>>>>> + mtspr \scratch , r4 >>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>>>> + >>>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>> + mtspr \scratch, r4 >>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>> + stw r3, VCPU_CRIT_SAVE(r4) >>>>>> + mfcr r3 >>>>>> + mfspr r4, SPRN_CSRR1 >>>>>> + andi. r4, r4, MSR_PR >>>>>> + bne 1f >>>>> >>>>> >>>>>> + /* debug interrupt happened in enter/exit path */ >>>>>> + mfspr r4, SPRN_CSRR1 >>>>>> + rlwinm r4, r4, 0, ~MSR_DE >>>>>> + mtspr SPRN_CSRR1, r4 >>>>>> + lis r4, 0xffff >>>>>> + ori r4, r4, 0xffff >>>>>> + mtspr SPRN_DBSR, r4 >>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>> + mtcr r3 >>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>>> + mfspr r4, \scratch >>>>>> + rfci >>>>> >>>>> What is this part doing? Try to ignore the debug exit? >>>> >>>> As BOOKE doesn't have hardware support for virtualization, hardware never know >>> current pc is in guest or in host. >>>> So when enable hardware single step for guest, it cannot be disabled at the >>> time guest exit. Thus, we'll see that an single step interrupt happens at the >>> beginning of guest exit path. >>>> >>>> With the above code we recognize this kind of single step interrupt disable >>> single step and rfci. >>>> >>>>> Why would we have MSR_DE >>>>> enabled in the first place when we can't handle it? >>>> >>>> When QEMU is using hardware debug resource then we always set MSR_DE during >>> guest is running. >>> >>> Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't set, you >>> wouldn't get a single step exit. >> >> We always set MSR_DE in hw MSR when qemu using the debug resource. > > In the _guest_ MSR, yes. But once we exit the guest, it shouldn't be set anymore, because we're in an interrupt handler, no? Or is MSR_DE kept alive on interrupts? Ah, it's kept for non-debug interrupts. That explains things. 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: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On > Behalf Of Alexander Graf > Sent: Thursday, January 31, 2013 10:38 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org > Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler > > > On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote: > > > > > > >> -----Original Message----- > >> From: Alexander Graf [mailto:agraf@suse.de] > >> Sent: Thursday, January 31, 2013 5:47 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org > >> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler > >> > >> > >> On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote: > >> > >>> > >>> > >>>> -----Original Message----- > >>>> From: Alexander Graf [mailto:agraf@suse.de] > >>>> Sent: Friday, January 25, 2013 5:13 PM > >>>> To: Bhushan Bharat-R65777 > >>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan > >>>> Bharat-R65777 > >>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler > >>>> > >>>> > >>>> On 16.01.2013, at 09:24, Bharat Bhushan wrote: > >>>> > >>>>> From: Bharat Bhushan <Bharat.Bhushan@freescale.com> > >>>>> > >>>>> Installed debug handler will be used for guest debug support and > >>>>> debug facility emulation features (patches for these features will > >>>>> follow this patch). > >>>>> > >>>>> Signed-off-by: Liu Yu <yu.liu@freescale.com> > >>>>> [bharat.bhushan@freescale.com: Substantial changes] > >>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > >>>>> --- > >>>>> arch/powerpc/include/asm/kvm_host.h | 1 + > >>>>> arch/powerpc/kernel/asm-offsets.c | 1 + > >>>>> arch/powerpc/kvm/booke_interrupts.S | 49 ++++++++++++++++++++++++++++++- > -- > >> -- > >>>>> 3 files changed, 44 insertions(+), 7 deletions(-) > >>>>> > >>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h > >>>>> b/arch/powerpc/include/asm/kvm_host.h > >>>>> index 8a72d59..f4ba881 100644 > >>>>> --- a/arch/powerpc/include/asm/kvm_host.h > >>>>> +++ b/arch/powerpc/include/asm/kvm_host.h > >>>>> @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { > >>>>> u32 tlbcfg[4]; > >>>>> u32 mmucfg; > >>>>> u32 epr; > >>>>> + u32 crit_save; > >>>>> struct kvmppc_booke_debug_reg dbg_reg; #endif > >>>>> gpa_t paddr_accessed; > >>>>> diff --git a/arch/powerpc/kernel/asm-offsets.c > >>>>> b/arch/powerpc/kernel/asm-offsets.c > >>>>> index 46f6afd..02048f3 100644 > >>>>> --- a/arch/powerpc/kernel/asm-offsets.c > >>>>> +++ b/arch/powerpc/kernel/asm-offsets.c > >>>>> @@ -562,6 +562,7 @@ int main(void) > >>>>> DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); > >>>>> 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)); > >>>>> #endif /* CONFIG_PPC_BOOK3S */ > >>>>> #endif /* CONFIG_KVM */ > >>>>> > >>>>> diff --git a/arch/powerpc/kvm/booke_interrupts.S > >>>>> b/arch/powerpc/kvm/booke_interrupts.S > >>>>> index eae8483..dd9c5d4 100644 > >>>>> --- a/arch/powerpc/kvm/booke_interrupts.S > >>>>> +++ b/arch/powerpc/kvm/booke_interrupts.S > >>>>> @@ -52,12 +52,7 @@ > >>>>> (1<<BOOKE_INTERRUPT_PROGRAM) | \ > >>>>> (1<<BOOKE_INTERRUPT_DTLB_MISS)) > >>>>> > >>>>> -.macro KVM_HANDLER ivor_nr scratch srr0 > >>>>> -_GLOBAL(kvmppc_handler_\ivor_nr) > >>>>> - /* Get pointer to vcpu and record exit number. */ > >>>>> - mtspr \scratch , r4 > >>>>> - mfspr r4, SPRN_SPRG_THREAD > >>>>> - lwz r4, THREAD_KVM_VCPU(r4) > >>>>> +.macro __KVM_HANDLER ivor_nr scratch srr0 > >>>>> stw r3, VCPU_GPR(R3)(r4) > >>>>> stw r5, VCPU_GPR(R5)(r4) > >>>>> stw r6, VCPU_GPR(R6)(r4) > >>>>> @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) > >>>>> bctr > >>>>> .endm > >>>>> > >>>>> +.macro KVM_HANDLER ivor_nr scratch srr0 > >>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) > >>>>> + /* Get pointer to vcpu and record exit number. */ > >>>>> + mtspr \scratch , r4 > >>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > >>>>> + > >>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 > >>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) > >>>>> + mtspr \scratch, r4 > >>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>> + stw r3, VCPU_CRIT_SAVE(r4) > >>>>> + mfcr r3 > >>>>> + mfspr r4, SPRN_CSRR1 > >>>>> + andi. r4, r4, MSR_PR > >>>>> + bne 1f > >>>> > >>>> > >>>>> + /* debug interrupt happened in enter/exit path */ > >>>>> + mfspr r4, SPRN_CSRR1 > >>>>> + rlwinm r4, r4, 0, ~MSR_DE > >>>>> + mtspr SPRN_CSRR1, r4 > >>>>> + lis r4, 0xffff > >>>>> + ori r4, r4, 0xffff > >>>>> + mtspr SPRN_DBSR, r4 > >>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>> + mtcr r3 > >>>>> + lwz r3, VCPU_CRIT_SAVE(r4) > >>>>> + mfspr r4, \scratch > >>>>> + rfci > >>>> > >>>> What is this part doing? Try to ignore the debug exit? > >>> > >>> As BOOKE doesn't have hardware support for virtualization, hardware > >>> never know > >> current pc is in guest or in host. > >>> So when enable hardware single step for guest, it cannot be disabled > >>> at the > >> time guest exit. Thus, we'll see that an single step interrupt > >> happens at the beginning of guest exit path. > >>> > >>> With the above code we recognize this kind of single step interrupt > >>> disable > >> single step and rfci. > >>> > >>>> Why would we have MSR_DE > >>>> enabled in the first place when we can't handle it? > >>> > >>> When QEMU is using hardware debug resource then we always set MSR_DE > >>> during > >> guest is running. > >> > >> Right, but why is MSR_DE enabled during the exit path? If MSR_DE > >> wasn't set, you wouldn't get a single step exit. > > > > We always set MSR_DE in hw MSR when qemu using the debug resource. > > In the _guest_ MSR, yes. But once we exit the guest, it shouldn't be set > anymore, because we're in an interrupt handler, no? Or is MSR_DE kept alive on > interrupts? > > > > >> During the exit code path, you could then swap DBSR back to what the > >> host expects (which means no single step). Only after that enable > >> MSR_DE again. > > > > We do not support deferred debug interrupt, so we do save restore dbsr. > > > >> > >>> > >>>> > >>>>> +1: /* debug interrupt happened in guest */ > >>>>> + mtcr r3 > >>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>> + lwz r3, VCPU_CRIT_SAVE(r4) > >>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 > >>>> > >>>> I don't think you need the __KVM_HANDLER split. This should be > >>>> quite easily refactorable into a simple DBG prolog. > >>> > >>> Can you please elaborate how you are envisioning this? > >> > >> With this patch, you have > >> > >> KVM_HANLDER: > >> > >> <code> > >> __KVM_HANDLER > >> > >> KVM_DBG_HANDLER: > >> > >> <code> > >> __KVM_HANDLER > >> > >> Right? > >> > >> In KVM_HANDLER, you get: > >> > >>> .macro KVM_HANDLER ivor_nr scratch srr0 > >>> _GLOBAL(kvmppc_handler_\ivor_nr) > >>> /* Get pointer to vcpu and record exit number. */ > >>> mtspr \scratch , r4 > >>> mfspr r4, SPRN_SPRG_THREAD > >>> lwz r4, THREAD_KVM_VCPU(r4) > >>> __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > >> > >> > >> while KVM_DBG_HANDLER is: > >> > >>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 > >>> +_GLOBAL(kvmppc_handler_\ivor_nr) > >>> <debug specific handling> > >>> +1: /* debug interrupt happened in guest */ > >>> + mtcr r3 > >>> + mfspr r4, SPRN_SPRG_THREAD > >>> + lwz r4, THREAD_KVM_VCPU(r4) > >>> + lwz r3, VCPU_CRIT_SAVE(r4) > >>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > >> > >> > >> So if you write this as > >> > >> KVM_DBG_HANDLER: > >> <debug specific handling> > >> 1: > >> mtcr r3 > >> mfspr r4, SPRN_SPRG_THREAD > >> lwz r4, THREAD_KVM_VCPU(r4) > >> lwz r3, VCPU_CRIT_SAVE(r4) > >> lwz r4, \scratch > >> <KVM_HANDLER> > >> > >> then you get code that is slower :) but it should be easier to read, > >> since the interface between the individual pieces is always the same. > >> Debug shouldn't be a fast path anyway, right? > > > > Frankly speaking I do not see much difference :). > > > > If we have to do as you mentioned then I think we can just do > > > > KVM_DBG_HANDLER: > > <debug specific handling> > > 1: > > mtcr r3 > > lwz r3, VCPU_CRIT_SAVE(r4) > > lwz r4, \scratch > > <KVM_HANDLER> > > Whatever it takes to keep the oddball (debug) an oddball and keep the normal > case easy :). I think there will be another problem as the kvmppc_handler_\ivor_nr will not be the starting address which is required as per our ivor/ivpr usages for booke architecture. I am thinking of keeping as is :). 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 -- 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 01.02.2013, at 06:04, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On >> Behalf Of Alexander Graf >> Sent: Thursday, January 31, 2013 10:38 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >> >> >> On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote: >> >>> >>> >>>> -----Original Message----- >>>> From: Alexander Graf [mailto:agraf@suse.de] >>>> Sent: Thursday, January 31, 2013 5:47 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >>>> >>>> >>>> On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote: >>>> >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>>> Sent: Friday, January 25, 2013 5:13 PM >>>>>> To: Bhushan Bharat-R65777 >>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan >>>>>> Bharat-R65777 >>>>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >>>>>> >>>>>> >>>>>> On 16.01.2013, at 09:24, Bharat Bhushan wrote: >>>>>> >>>>>>> From: Bharat Bhushan <Bharat.Bhushan@freescale.com> >>>>>>> >>>>>>> Installed debug handler will be used for guest debug support and >>>>>>> debug facility emulation features (patches for these features will >>>>>>> follow this patch). >>>>>>> >>>>>>> Signed-off-by: Liu Yu <yu.liu@freescale.com> >>>>>>> [bharat.bhushan@freescale.com: Substantial changes] >>>>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> >>>>>>> --- >>>>>>> arch/powerpc/include/asm/kvm_host.h | 1 + >>>>>>> arch/powerpc/kernel/asm-offsets.c | 1 + >>>>>>> arch/powerpc/kvm/booke_interrupts.S | 49 ++++++++++++++++++++++++++++++- >> -- >>>> -- >>>>>>> 3 files changed, 44 insertions(+), 7 deletions(-) >>>>>>> >>>>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h >>>>>>> b/arch/powerpc/include/asm/kvm_host.h >>>>>>> index 8a72d59..f4ba881 100644 >>>>>>> --- a/arch/powerpc/include/asm/kvm_host.h >>>>>>> +++ b/arch/powerpc/include/asm/kvm_host.h >>>>>>> @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { >>>>>>> u32 tlbcfg[4]; >>>>>>> u32 mmucfg; >>>>>>> u32 epr; >>>>>>> + u32 crit_save; >>>>>>> struct kvmppc_booke_debug_reg dbg_reg; #endif >>>>>>> gpa_t paddr_accessed; >>>>>>> diff --git a/arch/powerpc/kernel/asm-offsets.c >>>>>>> b/arch/powerpc/kernel/asm-offsets.c >>>>>>> index 46f6afd..02048f3 100644 >>>>>>> --- a/arch/powerpc/kernel/asm-offsets.c >>>>>>> +++ b/arch/powerpc/kernel/asm-offsets.c >>>>>>> @@ -562,6 +562,7 @@ int main(void) >>>>>>> DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); >>>>>>> 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)); >>>>>>> #endif /* CONFIG_PPC_BOOK3S */ >>>>>>> #endif /* CONFIG_KVM */ >>>>>>> >>>>>>> diff --git a/arch/powerpc/kvm/booke_interrupts.S >>>>>>> b/arch/powerpc/kvm/booke_interrupts.S >>>>>>> index eae8483..dd9c5d4 100644 >>>>>>> --- a/arch/powerpc/kvm/booke_interrupts.S >>>>>>> +++ b/arch/powerpc/kvm/booke_interrupts.S >>>>>>> @@ -52,12 +52,7 @@ >>>>>>> (1<<BOOKE_INTERRUPT_PROGRAM) | \ >>>>>>> (1<<BOOKE_INTERRUPT_DTLB_MISS)) >>>>>>> >>>>>>> -.macro KVM_HANDLER ivor_nr scratch srr0 >>>>>>> -_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>> - /* Get pointer to vcpu and record exit number. */ >>>>>>> - mtspr \scratch , r4 >>>>>>> - mfspr r4, SPRN_SPRG_THREAD >>>>>>> - lwz r4, THREAD_KVM_VCPU(r4) >>>>>>> +.macro __KVM_HANDLER ivor_nr scratch srr0 >>>>>>> stw r3, VCPU_GPR(R3)(r4) >>>>>>> stw r5, VCPU_GPR(R5)(r4) >>>>>>> stw r6, VCPU_GPR(R6)(r4) >>>>>>> @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>> bctr >>>>>>> .endm >>>>>>> >>>>>>> +.macro KVM_HANDLER ivor_nr scratch srr0 >>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>> + /* Get pointer to vcpu and record exit number. */ >>>>>>> + mtspr \scratch , r4 >>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>>>>> + >>>>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>> + mtspr \scratch, r4 >>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>> + stw r3, VCPU_CRIT_SAVE(r4) >>>>>>> + mfcr r3 >>>>>>> + mfspr r4, SPRN_CSRR1 >>>>>>> + andi. r4, r4, MSR_PR >>>>>>> + bne 1f >>>>>> >>>>>> >>>>>>> + /* debug interrupt happened in enter/exit path */ >>>>>>> + mfspr r4, SPRN_CSRR1 >>>>>>> + rlwinm r4, r4, 0, ~MSR_DE >>>>>>> + mtspr SPRN_CSRR1, r4 >>>>>>> + lis r4, 0xffff >>>>>>> + ori r4, r4, 0xffff >>>>>>> + mtspr SPRN_DBSR, r4 >>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>> + mtcr r3 >>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>>>> + mfspr r4, \scratch >>>>>>> + rfci >>>>>> >>>>>> What is this part doing? Try to ignore the debug exit? >>>>> >>>>> As BOOKE doesn't have hardware support for virtualization, hardware >>>>> never know >>>> current pc is in guest or in host. >>>>> So when enable hardware single step for guest, it cannot be disabled >>>>> at the >>>> time guest exit. Thus, we'll see that an single step interrupt >>>> happens at the beginning of guest exit path. >>>>> >>>>> With the above code we recognize this kind of single step interrupt >>>>> disable >>>> single step and rfci. >>>>> >>>>>> Why would we have MSR_DE >>>>>> enabled in the first place when we can't handle it? >>>>> >>>>> When QEMU is using hardware debug resource then we always set MSR_DE >>>>> during >>>> guest is running. >>>> >>>> Right, but why is MSR_DE enabled during the exit path? If MSR_DE >>>> wasn't set, you wouldn't get a single step exit. >>> >>> We always set MSR_DE in hw MSR when qemu using the debug resource. >> >> In the _guest_ MSR, yes. But once we exit the guest, it shouldn't be set >> anymore, because we're in an interrupt handler, no? Or is MSR_DE kept alive on >> interrupts? >> >>> >>>> During the exit code path, you could then swap DBSR back to what the >>>> host expects (which means no single step). Only after that enable >>>> MSR_DE again. >>> >>> We do not support deferred debug interrupt, so we do save restore dbsr. >>> >>>> >>>>> >>>>>> >>>>>>> +1: /* debug interrupt happened in guest */ >>>>>>> + mtcr r3 >>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 >>>>>> >>>>>> I don't think you need the __KVM_HANDLER split. This should be >>>>>> quite easily refactorable into a simple DBG prolog. >>>>> >>>>> Can you please elaborate how you are envisioning this? >>>> >>>> With this patch, you have >>>> >>>> KVM_HANLDER: >>>> >>>> <code> >>>> __KVM_HANDLER >>>> >>>> KVM_DBG_HANDLER: >>>> >>>> <code> >>>> __KVM_HANDLER >>>> >>>> Right? >>>> >>>> In KVM_HANDLER, you get: >>>> >>>>> .macro KVM_HANDLER ivor_nr scratch srr0 >>>>> _GLOBAL(kvmppc_handler_\ivor_nr) >>>>> /* Get pointer to vcpu and record exit number. */ >>>>> mtspr \scratch , r4 >>>>> mfspr r4, SPRN_SPRG_THREAD >>>>> lwz r4, THREAD_KVM_VCPU(r4) >>>>> __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>> >>>> >>>> while KVM_DBG_HANDLER is: >>>> >>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>> <debug specific handling> >>>>> +1: /* debug interrupt happened in guest */ >>>>> + mtcr r3 >>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>> >>>> >>>> So if you write this as >>>> >>>> KVM_DBG_HANDLER: >>>> <debug specific handling> >>>> 1: >>>> mtcr r3 >>>> mfspr r4, SPRN_SPRG_THREAD >>>> lwz r4, THREAD_KVM_VCPU(r4) >>>> lwz r3, VCPU_CRIT_SAVE(r4) >>>> lwz r4, \scratch >>>> <KVM_HANDLER> >>>> >>>> then you get code that is slower :) but it should be easier to read, >>>> since the interface between the individual pieces is always the same. >>>> Debug shouldn't be a fast path anyway, right? >>> >>> Frankly speaking I do not see much difference :). >>> >>> If we have to do as you mentioned then I think we can just do >>> >>> KVM_DBG_HANDLER: >>> <debug specific handling> >>> 1: >>> mtcr r3 >>> lwz r3, VCPU_CRIT_SAVE(r4) >>> lwz r4, \scratch >>> <KVM_HANDLER> >> >> Whatever it takes to keep the oddball (debug) an oddball and keep the normal >> case easy :). > > I think there will be another problem as the kvmppc_handler_\ivor_nr will not be the starting address which is required as per our ivor/ivpr usages for booke architecture. > > I am thinking of keeping as is :). How about we take a hybrid approach? You write the code as I described above, but call __KVM_HANDLER at the end. The normal KVM_HANDLER would look like: KVM_HANDLER: kvmppc_handler_\ivor_nr: __KVM_HANDLER ... That way the code should still be more understandable :) 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: Friday, February 01, 2013 1:36 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org > Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler > > > On 01.02.2013, at 06:04, Bhushan Bharat-R65777 wrote: > > > > > > >> -----Original Message----- > >> From: kvm-ppc-owner@vger.kernel.org > >> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf > >> Sent: Thursday, January 31, 2013 10:38 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org > >> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler > >> > >> > >> On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote: > >> > >>> > >>> > >>>> -----Original Message----- > >>>> From: Alexander Graf [mailto:agraf@suse.de] > >>>> Sent: Thursday, January 31, 2013 5:47 PM > >>>> To: Bhushan Bharat-R65777 > >>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org > >>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler > >>>> > >>>> > >>>> On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote: > >>>> > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Alexander Graf [mailto:agraf@suse.de] > >>>>>> Sent: Friday, January 25, 2013 5:13 PM > >>>>>> To: Bhushan Bharat-R65777 > >>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan > >>>>>> Bharat-R65777 > >>>>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler > >>>>>> > >>>>>> > >>>>>> On 16.01.2013, at 09:24, Bharat Bhushan wrote: > >>>>>> > >>>>>>> From: Bharat Bhushan <Bharat.Bhushan@freescale.com> > >>>>>>> > >>>>>>> Installed debug handler will be used for guest debug support and > >>>>>>> debug facility emulation features (patches for these features > >>>>>>> will follow this patch). > >>>>>>> > >>>>>>> Signed-off-by: Liu Yu <yu.liu@freescale.com> > >>>>>>> [bharat.bhushan@freescale.com: Substantial changes] > >>>>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > >>>>>>> --- > >>>>>>> arch/powerpc/include/asm/kvm_host.h | 1 + > >>>>>>> arch/powerpc/kernel/asm-offsets.c | 1 + > >>>>>>> arch/powerpc/kvm/booke_interrupts.S | 49 > ++++++++++++++++++++++++++++++- > >> -- > >>>> -- > >>>>>>> 3 files changed, 44 insertions(+), 7 deletions(-) > >>>>>>> > >>>>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h > >>>>>>> b/arch/powerpc/include/asm/kvm_host.h > >>>>>>> index 8a72d59..f4ba881 100644 > >>>>>>> --- a/arch/powerpc/include/asm/kvm_host.h > >>>>>>> +++ b/arch/powerpc/include/asm/kvm_host.h > >>>>>>> @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { > >>>>>>> u32 tlbcfg[4]; > >>>>>>> u32 mmucfg; > >>>>>>> u32 epr; > >>>>>>> + u32 crit_save; > >>>>>>> struct kvmppc_booke_debug_reg dbg_reg; #endif > >>>>>>> gpa_t paddr_accessed; > >>>>>>> diff --git a/arch/powerpc/kernel/asm-offsets.c > >>>>>>> b/arch/powerpc/kernel/asm-offsets.c > >>>>>>> index 46f6afd..02048f3 100644 > >>>>>>> --- a/arch/powerpc/kernel/asm-offsets.c > >>>>>>> +++ b/arch/powerpc/kernel/asm-offsets.c > >>>>>>> @@ -562,6 +562,7 @@ int main(void) > >>>>>>> DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); > >>>>>>> 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)); > >>>>>>> #endif /* CONFIG_PPC_BOOK3S */ > >>>>>>> #endif /* CONFIG_KVM */ > >>>>>>> > >>>>>>> diff --git a/arch/powerpc/kvm/booke_interrupts.S > >>>>>>> b/arch/powerpc/kvm/booke_interrupts.S > >>>>>>> index eae8483..dd9c5d4 100644 > >>>>>>> --- a/arch/powerpc/kvm/booke_interrupts.S > >>>>>>> +++ b/arch/powerpc/kvm/booke_interrupts.S > >>>>>>> @@ -52,12 +52,7 @@ > >>>>>>> (1<<BOOKE_INTERRUPT_PROGRAM) | \ > >>>>>>> (1<<BOOKE_INTERRUPT_DTLB_MISS)) > >>>>>>> > >>>>>>> -.macro KVM_HANDLER ivor_nr scratch srr0 > >>>>>>> -_GLOBAL(kvmppc_handler_\ivor_nr) > >>>>>>> - /* Get pointer to vcpu and record exit number. */ > >>>>>>> - mtspr \scratch , r4 > >>>>>>> - mfspr r4, SPRN_SPRG_THREAD > >>>>>>> - lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>> +.macro __KVM_HANDLER ivor_nr scratch srr0 > >>>>>>> stw r3, VCPU_GPR(R3)(r4) > >>>>>>> stw r5, VCPU_GPR(R5)(r4) > >>>>>>> stw r6, VCPU_GPR(R6)(r4) > >>>>>>> @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) > >>>>>>> bctr > >>>>>>> .endm > >>>>>>> > >>>>>>> +.macro KVM_HANDLER ivor_nr scratch srr0 > >>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) > >>>>>>> + /* Get pointer to vcpu and record exit number. */ > >>>>>>> + mtspr \scratch , r4 > >>>>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > >>>>>>> + > >>>>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 > >>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) > >>>>>>> + mtspr \scratch, r4 > >>>>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>> + stw r3, VCPU_CRIT_SAVE(r4) > >>>>>>> + mfcr r3 > >>>>>>> + mfspr r4, SPRN_CSRR1 > >>>>>>> + andi. r4, r4, MSR_PR > >>>>>>> + bne 1f > >>>>>> > >>>>>> > >>>>>>> + /* debug interrupt happened in enter/exit path */ > >>>>>>> + mfspr r4, SPRN_CSRR1 > >>>>>>> + rlwinm r4, r4, 0, ~MSR_DE > >>>>>>> + mtspr SPRN_CSRR1, r4 > >>>>>>> + lis r4, 0xffff > >>>>>>> + ori r4, r4, 0xffff > >>>>>>> + mtspr SPRN_DBSR, r4 > >>>>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>> + mtcr r3 > >>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) > >>>>>>> + mfspr r4, \scratch > >>>>>>> + rfci > >>>>>> > >>>>>> What is this part doing? Try to ignore the debug exit? > >>>>> > >>>>> As BOOKE doesn't have hardware support for virtualization, > >>>>> hardware never know > >>>> current pc is in guest or in host. > >>>>> So when enable hardware single step for guest, it cannot be > >>>>> disabled at the > >>>> time guest exit. Thus, we'll see that an single step interrupt > >>>> happens at the beginning of guest exit path. > >>>>> > >>>>> With the above code we recognize this kind of single step > >>>>> interrupt disable > >>>> single step and rfci. > >>>>> > >>>>>> Why would we have MSR_DE > >>>>>> enabled in the first place when we can't handle it? > >>>>> > >>>>> When QEMU is using hardware debug resource then we always set > >>>>> MSR_DE during > >>>> guest is running. > >>>> > >>>> Right, but why is MSR_DE enabled during the exit path? If MSR_DE > >>>> wasn't set, you wouldn't get a single step exit. > >>> > >>> We always set MSR_DE in hw MSR when qemu using the debug resource. > >> > >> In the _guest_ MSR, yes. But once we exit the guest, it shouldn't be > >> set anymore, because we're in an interrupt handler, no? Or is MSR_DE > >> kept alive on interrupts? > >> > >>> > >>>> During the exit code path, you could then swap DBSR back to what > >>>> the host expects (which means no single step). Only after that > >>>> enable MSR_DE again. > >>> > >>> We do not support deferred debug interrupt, so we do save restore dbsr. > >>> > >>>> > >>>>> > >>>>>> > >>>>>>> +1: /* debug interrupt happened in guest */ > >>>>>>> + mtcr r3 > >>>>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) > >>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 > >>>>>> > >>>>>> I don't think you need the __KVM_HANDLER split. This should be > >>>>>> quite easily refactorable into a simple DBG prolog. > >>>>> > >>>>> Can you please elaborate how you are envisioning this? > >>>> > >>>> With this patch, you have > >>>> > >>>> KVM_HANLDER: > >>>> > >>>> <code> > >>>> __KVM_HANDLER > >>>> > >>>> KVM_DBG_HANDLER: > >>>> > >>>> <code> > >>>> __KVM_HANDLER > >>>> > >>>> Right? > >>>> > >>>> In KVM_HANDLER, you get: > >>>> > >>>>> .macro KVM_HANDLER ivor_nr scratch srr0 > >>>>> _GLOBAL(kvmppc_handler_\ivor_nr) > >>>>> /* Get pointer to vcpu and record exit number. */ > >>>>> mtspr \scratch , r4 > >>>>> mfspr r4, SPRN_SPRG_THREAD > >>>>> lwz r4, THREAD_KVM_VCPU(r4) > >>>>> __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > >>>> > >>>> > >>>> while KVM_DBG_HANDLER is: > >>>> > >>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 > >>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) > >>>>> <debug specific handling> > >>>>> +1: /* debug interrupt happened in guest */ > >>>>> + mtcr r3 > >>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>> + lwz r3, VCPU_CRIT_SAVE(r4) > >>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > >>>> > >>>> > >>>> So if you write this as > >>>> > >>>> KVM_DBG_HANDLER: > >>>> <debug specific handling> > >>>> 1: > >>>> mtcr r3 > >>>> mfspr r4, SPRN_SPRG_THREAD > >>>> lwz r4, THREAD_KVM_VCPU(r4) > >>>> lwz r3, VCPU_CRIT_SAVE(r4) > >>>> lwz r4, \scratch > >>>> <KVM_HANDLER> > >>>> > >>>> then you get code that is slower :) but it should be easier to > >>>> read, since the interface between the individual pieces is always the same. > >>>> Debug shouldn't be a fast path anyway, right? > >>> > >>> Frankly speaking I do not see much difference :). > >>> > >>> If we have to do as you mentioned then I think we can just do > >>> > >>> KVM_DBG_HANDLER: > >>> <debug specific handling> > >>> 1: > >>> mtcr r3 > >>> lwz r3, VCPU_CRIT_SAVE(r4) > >>> lwz r4, \scratch > >>> <KVM_HANDLER> > >> > >> Whatever it takes to keep the oddball (debug) an oddball and keep the > >> normal case easy :). > > > > I think there will be another problem as the kvmppc_handler_\ivor_nr will not > be the starting address which is required as per our ivor/ivpr usages for booke > architecture. > > > > I am thinking of keeping as is :). > > How about we take a hybrid approach? You write the code as I described above, > but call __KVM_HANDLER at the end. The normal KVM_HANDLER would look like: > > KVM_HANDLER: > kvmppc_handler_\ivor_nr: > __KVM_HANDLER ... > > That way the code should still be more understandable :) > With my current Patch it is defined as: .macro KVM_HANDLER ivor_nr scratch srr0 _GLOBAL(kvmppc_handler_\ivor_nr) /* Get pointer to vcpu and record exit number. */ mtspr \scratch , r4 mfspr r4, SPRN_SPRG_THREAD lwz r4, THREAD_KVM_VCPU(r4) __KVM_HANDLER \ivor_nr \scratch \srr0 .endm .macro KVM_DBG_HANDLER ivor_nr scratch srr0 _GLOBAL(kvmppc_handler_\ivor_nr) <<<<<<Debug related handling>>>>> 1: /* debug interrupt happened in guest */ mtcr r3 mfspr r4, SPRN_SPRG_THREAD lwz r4, THREAD_KVM_VCPU(r4) lwz r3, VCPU_CRIT_SAVE(r4) __KVM_HANDLER \ivor_nr \scratch \srr0 .endm So the kvmppc_handler_\ivor_nr is defined and should always be at the start of exception handling? So if KVM_DBG_HANDLER need to call KVM_HANDLER then there will be issue of 2 definition for DBG interrupt. I am sorry but I did not understood how you want this to define. Can you please describe ? Thanks -Bharat -- 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 01.02.2013, at 10:07, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Friday, February 01, 2013 1:36 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >> >> >> On 01.02.2013, at 06:04, Bhushan Bharat-R65777 wrote: >> >>> >>> >>>> -----Original Message----- >>>> From: kvm-ppc-owner@vger.kernel.org >>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf >>>> Sent: Thursday, January 31, 2013 10:38 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >>>> >>>> >>>> On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote: >>>> >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>>> Sent: Thursday, January 31, 2013 5:47 PM >>>>>> To: Bhushan Bharat-R65777 >>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >>>>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >>>>>> >>>>>> >>>>>> On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>>>>> Sent: Friday, January 25, 2013 5:13 PM >>>>>>>> To: Bhushan Bharat-R65777 >>>>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan >>>>>>>> Bharat-R65777 >>>>>>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >>>>>>>> >>>>>>>> >>>>>>>> On 16.01.2013, at 09:24, Bharat Bhushan wrote: >>>>>>>> >>>>>>>>> From: Bharat Bhushan <Bharat.Bhushan@freescale.com> >>>>>>>>> >>>>>>>>> Installed debug handler will be used for guest debug support and >>>>>>>>> debug facility emulation features (patches for these features >>>>>>>>> will follow this patch). >>>>>>>>> >>>>>>>>> Signed-off-by: Liu Yu <yu.liu@freescale.com> >>>>>>>>> [bharat.bhushan@freescale.com: Substantial changes] >>>>>>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> >>>>>>>>> --- >>>>>>>>> arch/powerpc/include/asm/kvm_host.h | 1 + >>>>>>>>> arch/powerpc/kernel/asm-offsets.c | 1 + >>>>>>>>> arch/powerpc/kvm/booke_interrupts.S | 49 >> ++++++++++++++++++++++++++++++- >>>> -- >>>>>> -- >>>>>>>>> 3 files changed, 44 insertions(+), 7 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h >>>>>>>>> b/arch/powerpc/include/asm/kvm_host.h >>>>>>>>> index 8a72d59..f4ba881 100644 >>>>>>>>> --- a/arch/powerpc/include/asm/kvm_host.h >>>>>>>>> +++ b/arch/powerpc/include/asm/kvm_host.h >>>>>>>>> @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { >>>>>>>>> u32 tlbcfg[4]; >>>>>>>>> u32 mmucfg; >>>>>>>>> u32 epr; >>>>>>>>> + u32 crit_save; >>>>>>>>> struct kvmppc_booke_debug_reg dbg_reg; #endif >>>>>>>>> gpa_t paddr_accessed; >>>>>>>>> diff --git a/arch/powerpc/kernel/asm-offsets.c >>>>>>>>> b/arch/powerpc/kernel/asm-offsets.c >>>>>>>>> index 46f6afd..02048f3 100644 >>>>>>>>> --- a/arch/powerpc/kernel/asm-offsets.c >>>>>>>>> +++ b/arch/powerpc/kernel/asm-offsets.c >>>>>>>>> @@ -562,6 +562,7 @@ int main(void) >>>>>>>>> DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); >>>>>>>>> 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)); >>>>>>>>> #endif /* CONFIG_PPC_BOOK3S */ >>>>>>>>> #endif /* CONFIG_KVM */ >>>>>>>>> >>>>>>>>> diff --git a/arch/powerpc/kvm/booke_interrupts.S >>>>>>>>> b/arch/powerpc/kvm/booke_interrupts.S >>>>>>>>> index eae8483..dd9c5d4 100644 >>>>>>>>> --- a/arch/powerpc/kvm/booke_interrupts.S >>>>>>>>> +++ b/arch/powerpc/kvm/booke_interrupts.S >>>>>>>>> @@ -52,12 +52,7 @@ >>>>>>>>> (1<<BOOKE_INTERRUPT_PROGRAM) | \ >>>>>>>>> (1<<BOOKE_INTERRUPT_DTLB_MISS)) >>>>>>>>> >>>>>>>>> -.macro KVM_HANDLER ivor_nr scratch srr0 >>>>>>>>> -_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>>>> - /* Get pointer to vcpu and record exit number. */ >>>>>>>>> - mtspr \scratch , r4 >>>>>>>>> - mfspr r4, SPRN_SPRG_THREAD >>>>>>>>> - lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>> +.macro __KVM_HANDLER ivor_nr scratch srr0 >>>>>>>>> stw r3, VCPU_GPR(R3)(r4) >>>>>>>>> stw r5, VCPU_GPR(R5)(r4) >>>>>>>>> stw r6, VCPU_GPR(R6)(r4) >>>>>>>>> @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>>>> bctr >>>>>>>>> .endm >>>>>>>>> >>>>>>>>> +.macro KVM_HANDLER ivor_nr scratch srr0 >>>>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>>>> + /* Get pointer to vcpu and record exit number. */ >>>>>>>>> + mtspr \scratch , r4 >>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>>>>>>> + >>>>>>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>>>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>>>> + mtspr \scratch, r4 >>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>> + stw r3, VCPU_CRIT_SAVE(r4) >>>>>>>>> + mfcr r3 >>>>>>>>> + mfspr r4, SPRN_CSRR1 >>>>>>>>> + andi. r4, r4, MSR_PR >>>>>>>>> + bne 1f >>>>>>>> >>>>>>>> >>>>>>>>> + /* debug interrupt happened in enter/exit path */ >>>>>>>>> + mfspr r4, SPRN_CSRR1 >>>>>>>>> + rlwinm r4, r4, 0, ~MSR_DE >>>>>>>>> + mtspr SPRN_CSRR1, r4 >>>>>>>>> + lis r4, 0xffff >>>>>>>>> + ori r4, r4, 0xffff >>>>>>>>> + mtspr SPRN_DBSR, r4 >>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>> + mtcr r3 >>>>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>>>>>> + mfspr r4, \scratch >>>>>>>>> + rfci >>>>>>>> >>>>>>>> What is this part doing? Try to ignore the debug exit? >>>>>>> >>>>>>> As BOOKE doesn't have hardware support for virtualization, >>>>>>> hardware never know >>>>>> current pc is in guest or in host. >>>>>>> So when enable hardware single step for guest, it cannot be >>>>>>> disabled at the >>>>>> time guest exit. Thus, we'll see that an single step interrupt >>>>>> happens at the beginning of guest exit path. >>>>>>> >>>>>>> With the above code we recognize this kind of single step >>>>>>> interrupt disable >>>>>> single step and rfci. >>>>>>> >>>>>>>> Why would we have MSR_DE >>>>>>>> enabled in the first place when we can't handle it? >>>>>>> >>>>>>> When QEMU is using hardware debug resource then we always set >>>>>>> MSR_DE during >>>>>> guest is running. >>>>>> >>>>>> Right, but why is MSR_DE enabled during the exit path? If MSR_DE >>>>>> wasn't set, you wouldn't get a single step exit. >>>>> >>>>> We always set MSR_DE in hw MSR when qemu using the debug resource. >>>> >>>> In the _guest_ MSR, yes. But once we exit the guest, it shouldn't be >>>> set anymore, because we're in an interrupt handler, no? Or is MSR_DE >>>> kept alive on interrupts? >>>> >>>>> >>>>>> During the exit code path, you could then swap DBSR back to what >>>>>> the host expects (which means no single step). Only after that >>>>>> enable MSR_DE again. >>>>> >>>>> We do not support deferred debug interrupt, so we do save restore dbsr. >>>>> >>>>>> >>>>>>> >>>>>>>> >>>>>>>>> +1: /* debug interrupt happened in guest */ >>>>>>>>> + mtcr r3 >>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 >>>>>>>> >>>>>>>> I don't think you need the __KVM_HANDLER split. This should be >>>>>>>> quite easily refactorable into a simple DBG prolog. >>>>>>> >>>>>>> Can you please elaborate how you are envisioning this? >>>>>> >>>>>> With this patch, you have >>>>>> >>>>>> KVM_HANLDER: >>>>>> >>>>>> <code> >>>>>> __KVM_HANDLER >>>>>> >>>>>> KVM_DBG_HANDLER: >>>>>> >>>>>> <code> >>>>>> __KVM_HANDLER >>>>>> >>>>>> Right? >>>>>> >>>>>> In KVM_HANDLER, you get: >>>>>> >>>>>>> .macro KVM_HANDLER ivor_nr scratch srr0 >>>>>>> _GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>> /* Get pointer to vcpu and record exit number. */ >>>>>>> mtspr \scratch , r4 >>>>>>> mfspr r4, SPRN_SPRG_THREAD >>>>>>> lwz r4, THREAD_KVM_VCPU(r4) >>>>>>> __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>>>> >>>>>> >>>>>> while KVM_DBG_HANDLER is: >>>>>> >>>>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>> <debug specific handling> >>>>>>> +1: /* debug interrupt happened in guest */ >>>>>>> + mtcr r3 >>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>>>> >>>>>> >>>>>> So if you write this as >>>>>> >>>>>> KVM_DBG_HANDLER: >>>>>> <debug specific handling> >>>>>> 1: >>>>>> mtcr r3 >>>>>> mfspr r4, SPRN_SPRG_THREAD >>>>>> lwz r4, THREAD_KVM_VCPU(r4) >>>>>> lwz r3, VCPU_CRIT_SAVE(r4) >>>>>> lwz r4, \scratch >>>>>> <KVM_HANDLER> >>>>>> >>>>>> then you get code that is slower :) but it should be easier to >>>>>> read, since the interface between the individual pieces is always the same. >>>>>> Debug shouldn't be a fast path anyway, right? >>>>> >>>>> Frankly speaking I do not see much difference :). >>>>> >>>>> If we have to do as you mentioned then I think we can just do >>>>> >>>>> KVM_DBG_HANDLER: >>>>> <debug specific handling> >>>>> 1: >>>>> mtcr r3 >>>>> lwz r3, VCPU_CRIT_SAVE(r4) >>>>> lwz r4, \scratch >>>>> <KVM_HANDLER> >>>> >>>> Whatever it takes to keep the oddball (debug) an oddball and keep the >>>> normal case easy :). >>> >>> I think there will be another problem as the kvmppc_handler_\ivor_nr will not >> be the starting address which is required as per our ivor/ivpr usages for booke >> architecture. >>> >>> I am thinking of keeping as is :). >> >> How about we take a hybrid approach? You write the code as I described above, >> but call __KVM_HANDLER at the end. The normal KVM_HANDLER would look like: >> >> KVM_HANDLER: >> kvmppc_handler_\ivor_nr: >> __KVM_HANDLER ... >> >> That way the code should still be more understandable :) >> > > With my current Patch it is defined as: > > .macro KVM_HANDLER ivor_nr scratch srr0 > _GLOBAL(kvmppc_handler_\ivor_nr) > /* Get pointer to vcpu and record exit number. */ > mtspr \scratch , r4 > mfspr r4, SPRN_SPRG_THREAD > lwz r4, THREAD_KVM_VCPU(r4) Move these into __KVM_HANDLER (aka: keep the code in there the same as KVM_HANDLER today) > __KVM_HANDLER \ivor_nr \scratch \srr0 > .endm > > .macro KVM_DBG_HANDLER ivor_nr scratch srr0 > _GLOBAL(kvmppc_handler_\ivor_nr) > > <<<<<<Debug related handling>>>>> > > 1: /* debug interrupt happened in guest */ > mtcr r3 > mfspr r4, SPRN_SPRG_THREAD > lwz r4, THREAD_KVM_VCPU(r4) > lwz r3, VCPU_CRIT_SAVE(r4) Restore the state here as if a non-debug interrupt occurred. __KVM_HANDLER will fetch r4 itself from SPRG_THREAD. I'm basically advocating to not optimize the debug case at all. Instead, I would prefer to have the exception ABI be identical to the fallback case ABI. That way we don't have to worry about 4 code paths, but only about 3, keeping the complexity of the code low. Alex > __KVM_HANDLER \ivor_nr \scratch \srr0 > .endm > > So the kvmppc_handler_\ivor_nr is defined and should always be at the start of exception handling? > > So if KVM_DBG_HANDLER need to call KVM_HANDLER then there will be issue of 2 definition for DBG interrupt. > > I am sorry but I did not understood how you want this to define. Can you please describe ? > > Thanks > -Bharat > > -- 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: Friday, January 25, 2013 5:13 PM > >>>>>>>> To: Bhushan Bharat-R65777 > >>>>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan > >>>>>>>> Bharat-R65777 > >>>>>>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler > >>>>>>>> > >>>>>>>> > >>>>>>>> On 16.01.2013, at 09:24, Bharat Bhushan wrote: > >>>>>>>> > >>>>>>>>> From: Bharat Bhushan <Bharat.Bhushan@freescale.com> > >>>>>>>>> > >>>>>>>>> Installed debug handler will be used for guest debug support > >>>>>>>>> and debug facility emulation features (patches for these > >>>>>>>>> features will follow this patch). > >>>>>>>>> > >>>>>>>>> Signed-off-by: Liu Yu <yu.liu@freescale.com> > >>>>>>>>> [bharat.bhushan@freescale.com: Substantial changes] > >>>>>>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > >>>>>>>>> --- > >>>>>>>>> arch/powerpc/include/asm/kvm_host.h | 1 + > >>>>>>>>> arch/powerpc/kernel/asm-offsets.c | 1 + > >>>>>>>>> arch/powerpc/kvm/booke_interrupts.S | 49 > >> ++++++++++++++++++++++++++++++- > >>>> -- > >>>>>> -- > >>>>>>>>> 3 files changed, 44 insertions(+), 7 deletions(-) > >>>>>>>>> > >>>>>>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h > >>>>>>>>> b/arch/powerpc/include/asm/kvm_host.h > >>>>>>>>> index 8a72d59..f4ba881 100644 > >>>>>>>>> --- a/arch/powerpc/include/asm/kvm_host.h > >>>>>>>>> +++ b/arch/powerpc/include/asm/kvm_host.h > >>>>>>>>> @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { > >>>>>>>>> u32 tlbcfg[4]; > >>>>>>>>> u32 mmucfg; > >>>>>>>>> u32 epr; > >>>>>>>>> + u32 crit_save; > >>>>>>>>> struct kvmppc_booke_debug_reg dbg_reg; #endif > >>>>>>>>> gpa_t paddr_accessed; > >>>>>>>>> diff --git a/arch/powerpc/kernel/asm-offsets.c > >>>>>>>>> b/arch/powerpc/kernel/asm-offsets.c > >>>>>>>>> index 46f6afd..02048f3 100644 > >>>>>>>>> --- a/arch/powerpc/kernel/asm-offsets.c > >>>>>>>>> +++ b/arch/powerpc/kernel/asm-offsets.c > >>>>>>>>> @@ -562,6 +562,7 @@ int main(void) > >>>>>>>>> DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); > >>>>>>>>> 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)); > >>>>>>>>> #endif /* CONFIG_PPC_BOOK3S */ #endif /* CONFIG_KVM */ > >>>>>>>>> > >>>>>>>>> diff --git a/arch/powerpc/kvm/booke_interrupts.S > >>>>>>>>> b/arch/powerpc/kvm/booke_interrupts.S > >>>>>>>>> index eae8483..dd9c5d4 100644 > >>>>>>>>> --- a/arch/powerpc/kvm/booke_interrupts.S > >>>>>>>>> +++ b/arch/powerpc/kvm/booke_interrupts.S > >>>>>>>>> @@ -52,12 +52,7 @@ > >>>>>>>>> (1<<BOOKE_INTERRUPT_PROGRAM) | \ > >>>>>>>>> (1<<BOOKE_INTERRUPT_DTLB_MISS)) > >>>>>>>>> > >>>>>>>>> -.macro KVM_HANDLER ivor_nr scratch srr0 > >>>>>>>>> -_GLOBAL(kvmppc_handler_\ivor_nr) > >>>>>>>>> - /* Get pointer to vcpu and record exit number. */ > >>>>>>>>> - mtspr \scratch , r4 > >>>>>>>>> - mfspr r4, SPRN_SPRG_THREAD > >>>>>>>>> - lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>>>> +.macro __KVM_HANDLER ivor_nr scratch srr0 > >>>>>>>>> stw r3, VCPU_GPR(R3)(r4) > >>>>>>>>> stw r5, VCPU_GPR(R5)(r4) > >>>>>>>>> stw r6, VCPU_GPR(R6)(r4) > >>>>>>>>> @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) > >>>>>>>>> bctr > >>>>>>>>> .endm > >>>>>>>>> > >>>>>>>>> +.macro KVM_HANDLER ivor_nr scratch srr0 > >>>>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) > >>>>>>>>> + /* Get pointer to vcpu and record exit number. */ > >>>>>>>>> + mtspr \scratch , r4 > >>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > >>>>>>>>> + > >>>>>>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 > >>>>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) > >>>>>>>>> + mtspr \scratch, r4 > >>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>>>> + stw r3, VCPU_CRIT_SAVE(r4) > >>>>>>>>> + mfcr r3 > >>>>>>>>> + mfspr r4, SPRN_CSRR1 > >>>>>>>>> + andi. r4, r4, MSR_PR > >>>>>>>>> + bne 1f > >>>>>>>> > >>>>>>>> > >>>>>>>>> + /* debug interrupt happened in enter/exit path */ > >>>>>>>>> + mfspr r4, SPRN_CSRR1 > >>>>>>>>> + rlwinm r4, r4, 0, ~MSR_DE > >>>>>>>>> + mtspr SPRN_CSRR1, r4 > >>>>>>>>> + lis r4, 0xffff > >>>>>>>>> + ori r4, r4, 0xffff > >>>>>>>>> + mtspr SPRN_DBSR, r4 > >>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>>>> + mtcr r3 > >>>>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) > >>>>>>>>> + mfspr r4, \scratch > >>>>>>>>> + rfci > >>>>>>>> > >>>>>>>> What is this part doing? Try to ignore the debug exit? > >>>>>>> > >>>>>>> As BOOKE doesn't have hardware support for virtualization, > >>>>>>> hardware never know > >>>>>> current pc is in guest or in host. > >>>>>>> So when enable hardware single step for guest, it cannot be > >>>>>>> disabled at the > >>>>>> time guest exit. Thus, we'll see that an single step interrupt > >>>>>> happens at the beginning of guest exit path. > >>>>>>> > >>>>>>> With the above code we recognize this kind of single step > >>>>>>> interrupt disable > >>>>>> single step and rfci. > >>>>>>> > >>>>>>>> Why would we have MSR_DE > >>>>>>>> enabled in the first place when we can't handle it? > >>>>>>> > >>>>>>> When QEMU is using hardware debug resource then we always set > >>>>>>> MSR_DE during > >>>>>> guest is running. > >>>>>> > >>>>>> Right, but why is MSR_DE enabled during the exit path? If MSR_DE > >>>>>> wasn't set, you wouldn't get a single step exit. > >>>>> > >>>>> We always set MSR_DE in hw MSR when qemu using the debug resource. > >>>> > >>>> In the _guest_ MSR, yes. But once we exit the guest, it shouldn't > >>>> be set anymore, because we're in an interrupt handler, no? Or is > >>>> MSR_DE kept alive on interrupts? > >>>> > >>>>> > >>>>>> During the exit code path, you could then swap DBSR back to what > >>>>>> the host expects (which means no single step). Only after that > >>>>>> enable MSR_DE again. > >>>>> > >>>>> We do not support deferred debug interrupt, so we do save restore dbsr. > >>>>> > >>>>>> > >>>>>>> > >>>>>>>> > >>>>>>>>> +1: /* debug interrupt happened in guest */ > >>>>>>>>> + mtcr r3 > >>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) > >>>>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 > >>>>>>>> > >>>>>>>> I don't think you need the __KVM_HANDLER split. This should be > >>>>>>>> quite easily refactorable into a simple DBG prolog. > >>>>>>> > >>>>>>> Can you please elaborate how you are envisioning this? > >>>>>> > >>>>>> With this patch, you have > >>>>>> > >>>>>> KVM_HANLDER: > >>>>>> > >>>>>> <code> > >>>>>> __KVM_HANDLER > >>>>>> > >>>>>> KVM_DBG_HANDLER: > >>>>>> > >>>>>> <code> > >>>>>> __KVM_HANDLER > >>>>>> > >>>>>> Right? > >>>>>> > >>>>>> In KVM_HANDLER, you get: > >>>>>> > >>>>>>> .macro KVM_HANDLER ivor_nr scratch srr0 > >>>>>>> _GLOBAL(kvmppc_handler_\ivor_nr) > >>>>>>> /* Get pointer to vcpu and record exit number. */ > >>>>>>> mtspr \scratch , r4 > >>>>>>> mfspr r4, SPRN_SPRG_THREAD > >>>>>>> lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>> __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > >>>>>> > >>>>>> > >>>>>> while KVM_DBG_HANDLER is: > >>>>>> > >>>>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 > >>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) > >>>>>>> <debug specific handling> > >>>>>>> +1: /* debug interrupt happened in guest */ > >>>>>>> + mtcr r3 > >>>>>>> + mfspr r4, SPRN_SPRG_THREAD > >>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) > >>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) > >>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > >>>>>> > >>>>>> > >>>>>> So if you write this as > >>>>>> > >>>>>> KVM_DBG_HANDLER: > >>>>>> <debug specific handling> > >>>>>> 1: > >>>>>> mtcr r3 > >>>>>> mfspr r4, SPRN_SPRG_THREAD > >>>>>> lwz r4, THREAD_KVM_VCPU(r4) > >>>>>> lwz r3, VCPU_CRIT_SAVE(r4) > >>>>>> lwz r4, \scratch > >>>>>> <KVM_HANDLER> > >>>>>> > >>>>>> then you get code that is slower :) but it should be easier to > >>>>>> read, since the interface between the individual pieces is always the > same. > >>>>>> Debug shouldn't be a fast path anyway, right? > >>>>> > >>>>> Frankly speaking I do not see much difference :). > >>>>> > >>>>> If we have to do as you mentioned then I think we can just do > >>>>> > >>>>> KVM_DBG_HANDLER: > >>>>> <debug specific handling> > >>>>> 1: > >>>>> mtcr r3 > >>>>> lwz r3, VCPU_CRIT_SAVE(r4) > >>>>> lwz r4, \scratch > >>>>> <KVM_HANDLER> > >>>> > >>>> Whatever it takes to keep the oddball (debug) an oddball and keep > >>>> the normal case easy :). > >>> > >>> I think there will be another problem as the > >>> kvmppc_handler_\ivor_nr will not > >> be the starting address which is required as per our ivor/ivpr usages > >> for booke architecture. > >>> > >>> I am thinking of keeping as is :). > >> > >> How about we take a hybrid approach? You write the code as I > >> described above, but call __KVM_HANDLER at the end. The normal KVM_HANDLER > would look like: > >> > >> KVM_HANDLER: > >> kvmppc_handler_\ivor_nr: > >> __KVM_HANDLER ... > >> > >> That way the code should still be more understandable :) > >> > > > > With my current Patch it is defined as: > > > > .macro KVM_HANDLER ivor_nr scratch srr0 > > _GLOBAL(kvmppc_handler_\ivor_nr) > > /* Get pointer to vcpu and record exit number. */ > > mtspr \scratch , r4 > > mfspr r4, SPRN_SPRG_THREAD > > lwz r4, THREAD_KVM_VCPU(r4) > > Move these into __KVM_HANDLER (aka: keep the code in there the same as > KVM_HANDLER today) > > > __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > > > > .macro KVM_DBG_HANDLER ivor_nr scratch srr0 > > _GLOBAL(kvmppc_handler_\ivor_nr) > > > > <<<<<<Debug related handling>>>>> > > > > 1: /* debug interrupt happened in guest */ > > mtcr r3 > > mfspr r4, SPRN_SPRG_THREAD > > lwz r4, THREAD_KVM_VCPU(r4) > > lwz r3, VCPU_CRIT_SAVE(r4) > > Restore the state here as if a non-debug interrupt occurred. __KVM_HANDLER will > fetch r4 itself from SPRG_THREAD. > > I'm basically advocating to not optimize the debug case at all. Instead, I would > prefer to have the exception ABI be identical to the fallback case ABI. That way > we don't have to worry about 4 code paths, but only about 3, keeping the > complexity of the code low. > > > Alex > > > __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > > Do you mean something like this ? .macro __KVM_HANDLER + /* Get pointer to vcpu and record exit number. */ + mtspr \scratch , r4 + mfspr r4, SPRN_SPRG_THREAD + lwz r4, THREAD_KVM_VCPU(r4) << Existing code >> .macro KVM_HANDLER ivor_nr scratch srr0 _GLOBAL(kvmppc_handler_\ivor_nr) __KVM_HANDLER \ivor_nr \scratch \srr0 .endm .macro KVM_DBG_HANDLER ivor_nr scratch srr0 _GLOBAL(kvmppc_handler_\ivor_nr) <<<<<<Debug related handling>>>>> 1: /* debug interrupt happened in guest */ mtcr r3 mfspr r4, SPRN_SPRG_THREAD lwz r4, THREAD_KVM_VCPU(r4) lwz r3, VCPU_CRIT_SAVE(r4) lwz r4, \scratch __KVM_HANDLER \ivor_nr \scratch \srr0 .endm Thanks -Bharat > > So the kvmppc_handler_\ivor_nr is defined and should always be at the start of > exception handling? > > > > So if KVM_DBG_HANDLER need to call KVM_HANDLER then there will be issue of 2 > definition for DBG interrupt. > > > > I am sorry but I did not understood how you want this to define. Can you > please describe ? > > > > Thanks > > -Bharat > > > > > > -- > 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 07.02.2013, at 15:48, Bhushan Bharat-R65777 wrote: >>>>>>>>>> -----Original Message----- >>>>>>>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>>>>>>> Sent: Friday, January 25, 2013 5:13 PM >>>>>>>>>> To: Bhushan Bharat-R65777 >>>>>>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan >>>>>>>>>> Bharat-R65777 >>>>>>>>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 16.01.2013, at 09:24, Bharat Bhushan wrote: >>>>>>>>>> >>>>>>>>>>> From: Bharat Bhushan <Bharat.Bhushan@freescale.com> >>>>>>>>>>> >>>>>>>>>>> Installed debug handler will be used for guest debug support >>>>>>>>>>> and debug facility emulation features (patches for these >>>>>>>>>>> features will follow this patch). >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Liu Yu <yu.liu@freescale.com> >>>>>>>>>>> [bharat.bhushan@freescale.com: Substantial changes] >>>>>>>>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> >>>>>>>>>>> --- >>>>>>>>>>> arch/powerpc/include/asm/kvm_host.h | 1 + >>>>>>>>>>> arch/powerpc/kernel/asm-offsets.c | 1 + >>>>>>>>>>> arch/powerpc/kvm/booke_interrupts.S | 49 >>>> ++++++++++++++++++++++++++++++- >>>>>> -- >>>>>>>> -- >>>>>>>>>>> 3 files changed, 44 insertions(+), 7 deletions(-) >>>>>>>>>>> >>>>>>>>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h >>>>>>>>>>> b/arch/powerpc/include/asm/kvm_host.h >>>>>>>>>>> index 8a72d59..f4ba881 100644 >>>>>>>>>>> --- a/arch/powerpc/include/asm/kvm_host.h >>>>>>>>>>> +++ b/arch/powerpc/include/asm/kvm_host.h >>>>>>>>>>> @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { >>>>>>>>>>> u32 tlbcfg[4]; >>>>>>>>>>> u32 mmucfg; >>>>>>>>>>> u32 epr; >>>>>>>>>>> + u32 crit_save; >>>>>>>>>>> struct kvmppc_booke_debug_reg dbg_reg; #endif >>>>>>>>>>> gpa_t paddr_accessed; >>>>>>>>>>> diff --git a/arch/powerpc/kernel/asm-offsets.c >>>>>>>>>>> b/arch/powerpc/kernel/asm-offsets.c >>>>>>>>>>> index 46f6afd..02048f3 100644 >>>>>>>>>>> --- a/arch/powerpc/kernel/asm-offsets.c >>>>>>>>>>> +++ b/arch/powerpc/kernel/asm-offsets.c >>>>>>>>>>> @@ -562,6 +562,7 @@ int main(void) >>>>>>>>>>> DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); >>>>>>>>>>> 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)); >>>>>>>>>>> #endif /* CONFIG_PPC_BOOK3S */ #endif /* CONFIG_KVM */ >>>>>>>>>>> >>>>>>>>>>> diff --git a/arch/powerpc/kvm/booke_interrupts.S >>>>>>>>>>> b/arch/powerpc/kvm/booke_interrupts.S >>>>>>>>>>> index eae8483..dd9c5d4 100644 >>>>>>>>>>> --- a/arch/powerpc/kvm/booke_interrupts.S >>>>>>>>>>> +++ b/arch/powerpc/kvm/booke_interrupts.S >>>>>>>>>>> @@ -52,12 +52,7 @@ >>>>>>>>>>> (1<<BOOKE_INTERRUPT_PROGRAM) | \ >>>>>>>>>>> (1<<BOOKE_INTERRUPT_DTLB_MISS)) >>>>>>>>>>> >>>>>>>>>>> -.macro KVM_HANDLER ivor_nr scratch srr0 >>>>>>>>>>> -_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>>>>>> - /* Get pointer to vcpu and record exit number. */ >>>>>>>>>>> - mtspr \scratch , r4 >>>>>>>>>>> - mfspr r4, SPRN_SPRG_THREAD >>>>>>>>>>> - lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>>>> +.macro __KVM_HANDLER ivor_nr scratch srr0 >>>>>>>>>>> stw r3, VCPU_GPR(R3)(r4) >>>>>>>>>>> stw r5, VCPU_GPR(R5)(r4) >>>>>>>>>>> stw r6, VCPU_GPR(R6)(r4) >>>>>>>>>>> @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>>>>>> bctr >>>>>>>>>>> .endm >>>>>>>>>>> >>>>>>>>>>> +.macro KVM_HANDLER ivor_nr scratch srr0 >>>>>>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>>>>>> + /* Get pointer to vcpu and record exit number. */ >>>>>>>>>>> + mtspr \scratch , r4 >>>>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>>>>>>>>> + >>>>>>>>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>>>>>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>>>>>> + mtspr \scratch, r4 >>>>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>>>> + stw r3, VCPU_CRIT_SAVE(r4) >>>>>>>>>>> + mfcr r3 >>>>>>>>>>> + mfspr r4, SPRN_CSRR1 >>>>>>>>>>> + andi. r4, r4, MSR_PR >>>>>>>>>>> + bne 1f >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> + /* debug interrupt happened in enter/exit path */ >>>>>>>>>>> + mfspr r4, SPRN_CSRR1 >>>>>>>>>>> + rlwinm r4, r4, 0, ~MSR_DE >>>>>>>>>>> + mtspr SPRN_CSRR1, r4 >>>>>>>>>>> + lis r4, 0xffff >>>>>>>>>>> + ori r4, r4, 0xffff >>>>>>>>>>> + mtspr SPRN_DBSR, r4 >>>>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>>>> + mtcr r3 >>>>>>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>>>>>>>> + mfspr r4, \scratch >>>>>>>>>>> + rfci >>>>>>>>>> >>>>>>>>>> What is this part doing? Try to ignore the debug exit? >>>>>>>>> >>>>>>>>> As BOOKE doesn't have hardware support for virtualization, >>>>>>>>> hardware never know >>>>>>>> current pc is in guest or in host. >>>>>>>>> So when enable hardware single step for guest, it cannot be >>>>>>>>> disabled at the >>>>>>>> time guest exit. Thus, we'll see that an single step interrupt >>>>>>>> happens at the beginning of guest exit path. >>>>>>>>> >>>>>>>>> With the above code we recognize this kind of single step >>>>>>>>> interrupt disable >>>>>>>> single step and rfci. >>>>>>>>> >>>>>>>>>> Why would we have MSR_DE >>>>>>>>>> enabled in the first place when we can't handle it? >>>>>>>>> >>>>>>>>> When QEMU is using hardware debug resource then we always set >>>>>>>>> MSR_DE during >>>>>>>> guest is running. >>>>>>>> >>>>>>>> Right, but why is MSR_DE enabled during the exit path? If MSR_DE >>>>>>>> wasn't set, you wouldn't get a single step exit. >>>>>>> >>>>>>> We always set MSR_DE in hw MSR when qemu using the debug resource. >>>>>> >>>>>> In the _guest_ MSR, yes. But once we exit the guest, it shouldn't >>>>>> be set anymore, because we're in an interrupt handler, no? Or is >>>>>> MSR_DE kept alive on interrupts? >>>>>> >>>>>>> >>>>>>>> During the exit code path, you could then swap DBSR back to what >>>>>>>> the host expects (which means no single step). Only after that >>>>>>>> enable MSR_DE again. >>>>>>> >>>>>>> We do not support deferred debug interrupt, so we do save restore dbsr. >>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>>> +1: /* debug interrupt happened in guest */ >>>>>>>>>>> + mtcr r3 >>>>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>>>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 >>>>>>>>>> >>>>>>>>>> I don't think you need the __KVM_HANDLER split. This should be >>>>>>>>>> quite easily refactorable into a simple DBG prolog. >>>>>>>>> >>>>>>>>> Can you please elaborate how you are envisioning this? >>>>>>>> >>>>>>>> With this patch, you have >>>>>>>> >>>>>>>> KVM_HANLDER: >>>>>>>> >>>>>>>> <code> >>>>>>>> __KVM_HANDLER >>>>>>>> >>>>>>>> KVM_DBG_HANDLER: >>>>>>>> >>>>>>>> <code> >>>>>>>> __KVM_HANDLER >>>>>>>> >>>>>>>> Right? >>>>>>>> >>>>>>>> In KVM_HANDLER, you get: >>>>>>>> >>>>>>>>> .macro KVM_HANDLER ivor_nr scratch srr0 >>>>>>>>> _GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>>>> /* Get pointer to vcpu and record exit number. */ >>>>>>>>> mtspr \scratch , r4 >>>>>>>>> mfspr r4, SPRN_SPRG_THREAD >>>>>>>>> lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>> __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>>>>>> >>>>>>>> >>>>>>>> while KVM_DBG_HANDLER is: >>>>>>>> >>>>>>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>>>>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>>>>> <debug specific handling> >>>>>>>>> +1: /* debug interrupt happened in guest */ >>>>>>>>> + mtcr r3 >>>>>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>>>>>> >>>>>>>> >>>>>>>> So if you write this as >>>>>>>> >>>>>>>> KVM_DBG_HANDLER: >>>>>>>> <debug specific handling> >>>>>>>> 1: >>>>>>>> mtcr r3 >>>>>>>> mfspr r4, SPRN_SPRG_THREAD >>>>>>>> lwz r4, THREAD_KVM_VCPU(r4) >>>>>>>> lwz r3, VCPU_CRIT_SAVE(r4) >>>>>>>> lwz r4, \scratch >>>>>>>> <KVM_HANDLER> >>>>>>>> >>>>>>>> then you get code that is slower :) but it should be easier to >>>>>>>> read, since the interface between the individual pieces is always the >> same. >>>>>>>> Debug shouldn't be a fast path anyway, right? >>>>>>> >>>>>>> Frankly speaking I do not see much difference :). >>>>>>> >>>>>>> If we have to do as you mentioned then I think we can just do >>>>>>> >>>>>>> KVM_DBG_HANDLER: >>>>>>> <debug specific handling> >>>>>>> 1: >>>>>>> mtcr r3 >>>>>>> lwz r3, VCPU_CRIT_SAVE(r4) >>>>>>> lwz r4, \scratch >>>>>>> <KVM_HANDLER> >>>>>> >>>>>> Whatever it takes to keep the oddball (debug) an oddball and keep >>>>>> the normal case easy :). >>>>> >>>>> I think there will be another problem as the >>>>> kvmppc_handler_\ivor_nr will not >>>> be the starting address which is required as per our ivor/ivpr usages >>>> for booke architecture. >>>>> >>>>> I am thinking of keeping as is :). >>>> >>>> How about we take a hybrid approach? You write the code as I >>>> described above, but call __KVM_HANDLER at the end. The normal KVM_HANDLER >> would look like: >>>> >>>> KVM_HANDLER: >>>> kvmppc_handler_\ivor_nr: >>>> __KVM_HANDLER ... >>>> >>>> That way the code should still be more understandable :) >>>> >>> >>> With my current Patch it is defined as: >>> >>> .macro KVM_HANDLER ivor_nr scratch srr0 >>> _GLOBAL(kvmppc_handler_\ivor_nr) >>> /* Get pointer to vcpu and record exit number. */ >>> mtspr \scratch , r4 >>> mfspr r4, SPRN_SPRG_THREAD >>> lwz r4, THREAD_KVM_VCPU(r4) >> >> Move these into __KVM_HANDLER (aka: keep the code in there the same as >> KVM_HANDLER today) >> >>> __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>> >>> .macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>> _GLOBAL(kvmppc_handler_\ivor_nr) >>> >>> <<<<<<Debug related handling>>>>> >>> >>> 1: /* debug interrupt happened in guest */ >>> mtcr r3 >>> mfspr r4, SPRN_SPRG_THREAD >>> lwz r4, THREAD_KVM_VCPU(r4) >>> lwz r3, VCPU_CRIT_SAVE(r4) >> >> Restore the state here as if a non-debug interrupt occurred. __KVM_HANDLER will >> fetch r4 itself from SPRG_THREAD. >> >> I'm basically advocating to not optimize the debug case at all. Instead, I would >> prefer to have the exception ABI be identical to the fallback case ABI. That way >> we don't have to worry about 4 code paths, but only about 3, keeping the >> complexity of the code low. >> >> >> Alex >> >>> __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>> > > Do you mean something like this ? > > .macro __KVM_HANDLER > + /* Get pointer to vcpu and record exit number. */ > + mtspr \scratch , r4 > + mfspr r4, SPRN_SPRG_THREAD > + lwz r4, THREAD_KVM_VCPU(r4) This wouldn't be a +, but rather just stay the exact same code as it is, right? :) > << Existing code >> > > > .macro KVM_HANDLER ivor_nr scratch srr0 > _GLOBAL(kvmppc_handler_\ivor_nr) > __KVM_HANDLER \ivor_nr \scratch \srr0 .endm > > > > > .macro KVM_DBG_HANDLER ivor_nr scratch srr0 > _GLOBAL(kvmppc_handler_\ivor_nr) > <<<<<<Debug related handling>>>>> > 1: /* debug interrupt happened in guest */ > mtcr r3 > mfspr r4, SPRN_SPRG_THREAD > lwz r4, THREAD_KVM_VCPU(r4) > lwz r3, VCPU_CRIT_SAVE(r4) You need to swap the above 2 operations. > lwz r4, \scratch s/lwz/mfspr/ > __KVM_HANDLER \ivor_nr \scratch \srr0 .endm Otherwise, pretty much, yeah :) 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 8a72d59..f4ba881 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { u32 tlbcfg[4]; u32 mmucfg; u32 epr; + u32 crit_save; struct kvmppc_booke_debug_reg dbg_reg; #endif gpa_t paddr_accessed; diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c index 46f6afd..02048f3 100644 --- a/arch/powerpc/kernel/asm-offsets.c +++ b/arch/powerpc/kernel/asm-offsets.c @@ -562,6 +562,7 @@ int main(void) DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); 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)); #endif /* CONFIG_PPC_BOOK3S */ #endif /* CONFIG_KVM */ diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index eae8483..dd9c5d4 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -52,12 +52,7 @@ (1<<BOOKE_INTERRUPT_PROGRAM) | \ (1<<BOOKE_INTERRUPT_DTLB_MISS)) -.macro KVM_HANDLER ivor_nr scratch srr0 -_GLOBAL(kvmppc_handler_\ivor_nr) - /* Get pointer to vcpu and record exit number. */ - mtspr \scratch , r4 - mfspr r4, SPRN_SPRG_THREAD - lwz r4, THREAD_KVM_VCPU(r4) +.macro __KVM_HANDLER ivor_nr scratch srr0 stw r3, VCPU_GPR(R3)(r4) stw r5, VCPU_GPR(R5)(r4) stw r6, VCPU_GPR(R6)(r4) @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) bctr .endm +.macro KVM_HANDLER ivor_nr scratch srr0 +_GLOBAL(kvmppc_handler_\ivor_nr) + /* Get pointer to vcpu and record exit number. */ + mtspr \scratch , r4 + mfspr r4, SPRN_SPRG_THREAD + lwz r4, THREAD_KVM_VCPU(r4) + __KVM_HANDLER \ivor_nr \scratch \srr0 +.endm + +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 +_GLOBAL(kvmppc_handler_\ivor_nr) + mtspr \scratch, r4 + mfspr r4, SPRN_SPRG_THREAD + lwz r4, THREAD_KVM_VCPU(r4) + stw r3, VCPU_CRIT_SAVE(r4) + mfcr r3 + mfspr r4, SPRN_CSRR1 + andi. r4, r4, MSR_PR + bne 1f + /* debug interrupt happened in enter/exit path */ + mfspr r4, SPRN_CSRR1 + rlwinm r4, r4, 0, ~MSR_DE + mtspr SPRN_CSRR1, r4 + lis r4, 0xffff + ori r4, r4, 0xffff + mtspr SPRN_DBSR, r4 + mfspr r4, SPRN_SPRG_THREAD + lwz r4, THREAD_KVM_VCPU(r4) + mtcr r3 + lwz r3, VCPU_CRIT_SAVE(r4) + mfspr r4, \scratch + rfci +1: /* debug interrupt happened in guest */ + mtcr r3 + mfspr r4, SPRN_SPRG_THREAD + lwz r4, THREAD_KVM_VCPU(r4) + lwz r3, VCPU_CRIT_SAVE(r4) + __KVM_HANDLER \ivor_nr \scratch \srr0 +.endm + .macro KVM_HANDLER_ADDR ivor_nr .long kvmppc_handler_\ivor_nr .endm @@ -98,7 +133,7 @@ KVM_HANDLER BOOKE_INTERRUPT_FIT SPRN_SPRG_RSCRATCH0 SPRN_SRR0 KVM_HANDLER BOOKE_INTERRUPT_WATCHDOG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0 KVM_HANDLER BOOKE_INTERRUPT_DTLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0 KVM_HANDLER BOOKE_INTERRUPT_ITLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0 -KVM_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0 +KVM_DBG_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0 KVM_HANDLER BOOKE_INTERRUPT_SPE_UNAVAIL SPRN_SPRG_RSCRATCH0 SPRN_SRR0 KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_DATA SPRN_SPRG_RSCRATCH0 SPRN_SRR0 KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_ROUND SPRN_SPRG_RSCRATCH0 SPRN_SRR0