| Message ID | 1363801557-27436-1-git-send-email-Bharat.Bhushan@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 20.03.2013, at 18:45, Bharat Bhushan wrote: > EPCR.DUVD controls whether the debug events can come in > hypervisor mode or not. When KVM guest is using the debug > resource then we do not want debug events to be captured > in guest entry/exit path. So we set EPCR.DUVD when entering > and clears EPCR.DUVD when exiting from guest. > > Debug instruction complete is a post-completion debug > exception but debug event gets posted on the basis of MSR > before the instruction is executed. Now if the instruction > switches the context from guest mode (MSR.GS = 1) to hypervisor > mode (MSR.GS = 0) then the xSRR0 points to first instruction of > KVM handler and xSRR1 points that MSR.GS is clear > (hypervisor context). Now as xSRR1.GS is used to decide whether > KVM handler will be invoked to handle the exception or host > host kernel debug handler will be invoked to handle the exception. > This leads to host kernel debug handler handling the exception > which should either be handled by KVM. > > This is tested on e500mc in 32 bit mode > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > --- > v0: > - Do not apply this change for debug_crit as we do not know those chips have issue or not. > - corrected 64bit case branching > > arch/powerpc/kernel/exceptions-64e.S | 29 ++++++++++++++++++++++++++++- > arch/powerpc/kernel/head_booke.h | 26 ++++++++++++++++++++++++++ > 2 files changed, 54 insertions(+), 1 deletions(-) > > diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S > index 4684e33..8b26294 100644 > --- a/arch/powerpc/kernel/exceptions-64e.S > +++ b/arch/powerpc/kernel/exceptions-64e.S > @@ -516,6 +516,33 @@ kernel_dbg_exc: > andis. r15,r14,DBSR_IC@h > beq+ 1f > > +#ifdef CONFIG_KVM_BOOKE_HV > + /* > + * EPCR.DUVD controls whether the debug events can come in > + * hypervisor mode or not. When KVM guest is using the debug > + * resource then we do not want debug events to be captured > + * in guest entry/exit path. So we set EPCR.DUVD when entering > + * and clears EPCR.DUVD when exiting from guest. > + * Debug instruction complete is a post-completion debug > + * exception but debug event gets posted on the basis of MSR > + * before the instruction is executed. Now if the instruction > + * switches the context from guest mode (MSR.GS = 1) to hypervisor > + * mode (MSR.GS = 0) then the xSRR0 points to first instruction of Can't we just execute that code path with MSR.DE=0? Alex > + * KVM handler and xSRR1 points that MSR.GS is clear > + * (hypervisor context). Now as xSRR1.GS is used to decide whether > + * KVM handler will be invoked to handle the exception or host > + * host kernel debug handler will be invoked to handle the exception. > + * This leads to host kernel debug handler handling the exception > + * which should either be handled by KVM. > + */ > + mfspr r10, SPRN_EPCR > + andis. r10,r10,SPRN_EPCR_DUVD@h > + beq+ 2f > + > + andis. r10,r9,MSR_GS@h > + beq+ 3f > +2: > +#endif > LOAD_REG_IMMEDIATE(r14,interrupt_base_book3e) > LOAD_REG_IMMEDIATE(r15,interrupt_end_book3e) > cmpld cr0,r10,r14 > @@ -523,7 +550,7 @@ kernel_dbg_exc: > blt+ cr0,1f > bge+ cr1,1f > > - /* here it looks like we got an inappropriate debug exception. */ > +3: /* here it looks like we got an inappropriate debug exception. */ > lis r14,DBSR_IC@h /* clear the IC event */ > rlwinm r11,r11,0,~MSR_DE /* clear DE in the DSRR1 value */ > mtspr SPRN_DBSR,r14 > diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h > index 5f051ee..edc6a3b 100644 > --- a/arch/powerpc/kernel/head_booke.h > +++ b/arch/powerpc/kernel/head_booke.h > @@ -285,7 +285,33 @@ label: > mfspr r10,SPRN_DBSR; /* check single-step/branch taken */ \ > andis. r10,r10,(DBSR_IC|DBSR_BT)@h; \ > beq+ 2f; \ > +#ifdef CONFIG_KVM_BOOKE_HV \ > + /* \ > + * EPCR.DUVD controls whether the debug events can come in \ > + * hypervisor mode or not. When KVM guest is using the debug \ > + * resource then we do not want debug events to be captured \ > + * in guest entry/exit path. So we set EPCR.DUVD when entering \ > + * and clears EPCR.DUVD when exiting from guest. \ > + * Debug instruction complete is a post-completion debug \ > + * exception but debug event gets posted on the basis of MSR \ > + * before the instruction is executed. Now if the instruction \ > + * switches the context from guest mode (MSR.GS = 1) to hypervisor \ > + * mode (MSR.GS = 0) then the xSRR0 points to first instruction of \ > + * KVM handler and xSRR1 points that MSR.GS is clear \ > + * (hypervisor context). Now as xSRR1.GS is used to decide whether \ > + * KVM handler will be invoked to handle the exception or host \ > + * host kernel debug handler will be invoked to handle the exception. \ > + * This leads to host kernel debug handler handling the exception \ > + * which should either be handled by KVM. \ > + */ \ > + mfspr r10, SPRN_EPCR; \ > + andis. r10,r10,SPRN_EPCR_DUVD@h; \ > + beq+ 3f; \ > \ > + andis. r10,r9,MSR_GS@h; \ > + beq+ 1f; \ > +3: \ > +#endif \ > lis r10,KERNELBASE@h; /* check if exception in vectors */ \ > ori r10,r10,KERNELBASE@l; \ > cmplw r12,r10; \ > -- > 1.7.0.4 > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" 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, April 04, 2013 6:55 PM > To: Bhushan Bharat-R65777 > Cc: linuxppc-dev@lists.ozlabs.org; kvm@vger.kernel.org; kvm-ppc@vger.kernel.org; > Wood Scott-B07421; Bhushan Bharat-R65777 > Subject: Re: [PATCH] bookehv: Handle debug exception on guest exit > > > On 20.03.2013, at 18:45, Bharat Bhushan wrote: > > > EPCR.DUVD controls whether the debug events can come in hypervisor > > mode or not. When KVM guest is using the debug resource then we do not > > want debug events to be captured in guest entry/exit path. So we set > > EPCR.DUVD when entering and clears EPCR.DUVD when exiting from guest. > > > > Debug instruction complete is a post-completion debug exception but > > debug event gets posted on the basis of MSR before the instruction is > > executed. Now if the instruction switches the context from guest mode > > (MSR.GS = 1) to hypervisor mode (MSR.GS = 0) then the xSRR0 points to > > first instruction of KVM handler and xSRR1 points that MSR.GS is clear > > (hypervisor context). Now as xSRR1.GS is used to decide whether KVM > > handler will be invoked to handle the exception or host host kernel > > debug handler will be invoked to handle the exception. > > This leads to host kernel debug handler handling the exception which > > should either be handled by KVM. > > > > This is tested on e500mc in 32 bit mode > > > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > > --- > > v0: > > - Do not apply this change for debug_crit as we do not know those chips have > issue or not. > > - corrected 64bit case branching > > > > arch/powerpc/kernel/exceptions-64e.S | 29 ++++++++++++++++++++++++++++- > > arch/powerpc/kernel/head_booke.h | 26 ++++++++++++++++++++++++++ > > 2 files changed, 54 insertions(+), 1 deletions(-) > > > > diff --git a/arch/powerpc/kernel/exceptions-64e.S > > b/arch/powerpc/kernel/exceptions-64e.S > > index 4684e33..8b26294 100644 > > --- a/arch/powerpc/kernel/exceptions-64e.S > > +++ b/arch/powerpc/kernel/exceptions-64e.S > > @@ -516,6 +516,33 @@ kernel_dbg_exc: > > andis. r15,r14,DBSR_IC@h > > beq+ 1f > > > > +#ifdef CONFIG_KVM_BOOKE_HV > > + /* > > + * EPCR.DUVD controls whether the debug events can come in > > + * hypervisor mode or not. When KVM guest is using the debug > > + * resource then we do not want debug events to be captured > > + * in guest entry/exit path. So we set EPCR.DUVD when entering > > + * and clears EPCR.DUVD when exiting from guest. > > + * Debug instruction complete is a post-completion debug > > + * exception but debug event gets posted on the basis of MSR > > + * before the instruction is executed. Now if the instruction > > + * switches the context from guest mode (MSR.GS = 1) to hypervisor > > + * mode (MSR.GS = 0) then the xSRR0 points to first instruction of > > Can't we just execute that code path with MSR.DE=0? Single stepping uses DBCR0.IC (instruction complete). Can you describe how MSR.DE = 0 will work? > > > Alex > > > + * KVM handler and xSRR1 points that MSR.GS is clear > > + * (hypervisor context). Now as xSRR1.GS is used to decide whether > > + * KVM handler will be invoked to handle the exception or host > > + * host kernel debug handler will be invoked to handle the exception. > > + * This leads to host kernel debug handler handling the exception > > + * which should either be handled by KVM. > > + */ > > + mfspr r10, SPRN_EPCR > > + andis. r10,r10,SPRN_EPCR_DUVD@h > > + beq+ 2f > > + > > + andis. r10,r9,MSR_GS@h > > + beq+ 3f > > +2: > > +#endif > > LOAD_REG_IMMEDIATE(r14,interrupt_base_book3e) > > LOAD_REG_IMMEDIATE(r15,interrupt_end_book3e) > > cmpld cr0,r10,r14 > > @@ -523,7 +550,7 @@ kernel_dbg_exc: > > blt+ cr0,1f > > bge+ cr1,1f > > > > - /* here it looks like we got an inappropriate debug exception. */ > > +3: /* here it looks like we got an inappropriate debug exception. */ > > lis r14,DBSR_IC@h /* clear the IC event */ > > rlwinm r11,r11,0,~MSR_DE /* clear DE in the DSRR1 value */ > > mtspr SPRN_DBSR,r14 > > diff --git a/arch/powerpc/kernel/head_booke.h > > b/arch/powerpc/kernel/head_booke.h > > index 5f051ee..edc6a3b 100644 > > --- a/arch/powerpc/kernel/head_booke.h > > +++ b/arch/powerpc/kernel/head_booke.h > > @@ -285,7 +285,33 @@ label: > > mfspr r10,SPRN_DBSR; /* check single-step/branch taken */ \ > > andis. r10,r10,(DBSR_IC|DBSR_BT)@h; \ > > beq+ 2f; \ > > +#ifdef CONFIG_KVM_BOOKE_HV \ > > + /* \ > > + * EPCR.DUVD controls whether the debug events can come in \ > > + * hypervisor mode or not. When KVM guest is using the debug \ > > + * resource then we do not want debug events to be captured \ > > + * in guest entry/exit path. So we set EPCR.DUVD when entering \ > > + * and clears EPCR.DUVD when exiting from guest. \ > > + * Debug instruction complete is a post-completion debug \ > > + * exception but debug event gets posted on the basis of MSR \ > > + * before the instruction is executed. Now if the instruction \ > > + * switches the context from guest mode (MSR.GS = 1) to hypervisor \ > > + * mode (MSR.GS = 0) then the xSRR0 points to first instruction of \ > > + * KVM handler and xSRR1 points that MSR.GS is clear \ > > + * (hypervisor context). Now as xSRR1.GS is used to decide whether \ > > + * KVM handler will be invoked to handle the exception or host \ > > + * host kernel debug handler will be invoked to handle the exception. \ > > + * This leads to host kernel debug handler handling the exception \ > > + * which should either be handled by KVM. \ > > + */ \ > > + mfspr r10, SPRN_EPCR; \ > > + andis. r10,r10,SPRN_EPCR_DUVD@h; \ > > + beq+ 3f; \ > > \ > > + andis. r10,r9,MSR_GS@h; \ > > + beq+ 1f; \ > > +3: \ > > +#endif \ > > lis r10,KERNELBASE@h; /* check if exception in vectors */ \ > > ori r10,r10,KERNELBASE@l; \ > > cmplw r12,r10; \ > > -- > > 1.7.0.4 > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > > the body of a message to majordomo@vger.kernel.org More majordomo info > > at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Kumar/Benh,
After further looking into the code I think that if we correct the vector range below in DebugDebug handler then we do not need the change I provided in this patch.
Here is the snapshot for 32 bit (head_booke.h, same will be true for 64 bit):
#define DEBUG_DEBUG_EXCEPTION \
START_EXCEPTION(DebugDebug); \
DEBUG_EXCEPTION_PROLOG; \
\
/* \
* If there is a single step or branch-taken exception in an \
* exception entry sequence, it was probably meant to apply to \
* the code where the exception occurred (since exception entry \
* doesn't turn off DE automatically). We simulate the effect \
* of turning off DE on entry to an exception handler by turning \
* off DE in the DSRR1 value and clearing the debug status. \
*/ \
mfspr r10,SPRN_DBSR; /* check single-step/branch taken */ \
andis. r10,r10,(DBSR_IC|DBSR_BT)@h; \
beq+ 2f; \
\
lis r10,KERNELBASE@h; /* check if exception in vectors */ \
ori r10,r10,KERNELBASE@l; \
cmplw r12,r10; \
blt+ 2f; /* addr below exception vectors */ \
\
lis r10,DebugDebug@h; \
ori r10,r10,DebugDebug@l; \
^^^^
Here we assume all exception vector ends at DebugDebug, which is not correct.
We probably should get proper end by using some start_vector and end_vector lebels
or at least use end at Ehvpriv (which is last defined in head_fsl_booke.S for PowerPC. Is that correct?
cmplw r12,r10; \
bgt+ 2f; /* addr above exception vectors */ \
Thanks
-Bharat
> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On
> Behalf Of Bhushan Bharat-R65777
> Sent: Thursday, April 04, 2013 8:29 PM
> To: Alexander Graf
> Cc: linuxppc-dev@lists.ozlabs.org; kvm@vger.kernel.org; kvm-ppc@vger.kernel.org;
> Wood Scott-B07421
> Subject: RE: [PATCH] bookehv: Handle debug exception on guest exit
>
>
>
> > -----Original Message-----
> > From: Alexander Graf [mailto:agraf@suse.de]
> > Sent: Thursday, April 04, 2013 6:55 PM
> > To: Bhushan Bharat-R65777
> > Cc: linuxppc-dev@lists.ozlabs.org; kvm@vger.kernel.org;
> > kvm-ppc@vger.kernel.org; Wood Scott-B07421; Bhushan Bharat-R65777
> > Subject: Re: [PATCH] bookehv: Handle debug exception on guest exit
> >
> >
> > On 20.03.2013, at 18:45, Bharat Bhushan wrote:
> >
> > > EPCR.DUVD controls whether the debug events can come in hypervisor
> > > mode or not. When KVM guest is using the debug resource then we do
> > > not want debug events to be captured in guest entry/exit path. So we
> > > set EPCR.DUVD when entering and clears EPCR.DUVD when exiting from guest.
> > >
> > > Debug instruction complete is a post-completion debug exception but
> > > debug event gets posted on the basis of MSR before the instruction
> > > is executed. Now if the instruction switches the context from guest
> > > mode (MSR.GS = 1) to hypervisor mode (MSR.GS = 0) then the xSRR0
> > > points to first instruction of KVM handler and xSRR1 points that
> > > MSR.GS is clear (hypervisor context). Now as xSRR1.GS is used to
> > > decide whether KVM handler will be invoked to handle the exception
> > > or host host kernel debug handler will be invoked to handle the exception.
> > > This leads to host kernel debug handler handling the exception which
> > > should either be handled by KVM.
> > >
> > > This is tested on e500mc in 32 bit mode
> > >
> > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> > > ---
> > > v0:
> > > - Do not apply this change for debug_crit as we do not know those
> > > chips have
> > issue or not.
> > > - corrected 64bit case branching
> > >
> > > arch/powerpc/kernel/exceptions-64e.S | 29 ++++++++++++++++++++++++++++-
> > > arch/powerpc/kernel/head_booke.h | 26 ++++++++++++++++++++++++++
> > > 2 files changed, 54 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/arch/powerpc/kernel/exceptions-64e.S
> > > b/arch/powerpc/kernel/exceptions-64e.S
> > > index 4684e33..8b26294 100644
> > > --- a/arch/powerpc/kernel/exceptions-64e.S
> > > +++ b/arch/powerpc/kernel/exceptions-64e.S
> > > @@ -516,6 +516,33 @@ kernel_dbg_exc:
> > > andis. r15,r14,DBSR_IC@h
> > > beq+ 1f
> > >
> > > +#ifdef CONFIG_KVM_BOOKE_HV
> > > + /*
> > > + * EPCR.DUVD controls whether the debug events can come in
> > > + * hypervisor mode or not. When KVM guest is using the debug
> > > + * resource then we do not want debug events to be captured
> > > + * in guest entry/exit path. So we set EPCR.DUVD when entering
> > > + * and clears EPCR.DUVD when exiting from guest.
> > > + * Debug instruction complete is a post-completion debug
> > > + * exception but debug event gets posted on the basis of MSR
> > > + * before the instruction is executed. Now if the instruction
> > > + * switches the context from guest mode (MSR.GS = 1) to hypervisor
> > > + * mode (MSR.GS = 0) then the xSRR0 points to first instruction of
> >
> > Can't we just execute that code path with MSR.DE=0?
>
> Single stepping uses DBCR0.IC (instruction complete).
> Can you describe how MSR.DE = 0 will work?
>
> >
> >
> > Alex
> >
> > > + * KVM handler and xSRR1 points that MSR.GS is clear
> > > + * (hypervisor context). Now as xSRR1.GS is used to decide whether
> > > + * KVM handler will be invoked to handle the exception or host
> > > + * host kernel debug handler will be invoked to handle the exception.
> > > + * This leads to host kernel debug handler handling the exception
> > > + * which should either be handled by KVM.
> > > + */
> > > + mfspr r10, SPRN_EPCR
> > > + andis. r10,r10,SPRN_EPCR_DUVD@h
> > > + beq+ 2f
> > > +
> > > + andis. r10,r9,MSR_GS@h
> > > + beq+ 3f
> > > +2:
> > > +#endif
> > > LOAD_REG_IMMEDIATE(r14,interrupt_base_book3e)
> > > LOAD_REG_IMMEDIATE(r15,interrupt_end_book3e)
> > > cmpld cr0,r10,r14
> > > @@ -523,7 +550,7 @@ kernel_dbg_exc:
> > > blt+ cr0,1f
> > > bge+ cr1,1f
> > >
> > > - /* here it looks like we got an inappropriate debug exception. */
> > > +3: /* here it looks like we got an inappropriate debug exception. */
> > > lis r14,DBSR_IC@h /* clear the IC event */
> > > rlwinm r11,r11,0,~MSR_DE /* clear DE in the DSRR1 value */
> > > mtspr SPRN_DBSR,r14
> > > diff --git a/arch/powerpc/kernel/head_booke.h
> > > b/arch/powerpc/kernel/head_booke.h
> > > index 5f051ee..edc6a3b 100644
> > > --- a/arch/powerpc/kernel/head_booke.h
> > > +++ b/arch/powerpc/kernel/head_booke.h
> > > @@ -285,7 +285,33 @@ label:
> > > mfspr r10,SPRN_DBSR; /* check single-step/branch taken */ \
> > > andis. r10,r10,(DBSR_IC|DBSR_BT)@h; \
> > > beq+ 2f; \
> > > +#ifdef CONFIG_KVM_BOOKE_HV \
> > > + /* \
> > > + * EPCR.DUVD controls whether the debug events can come in \
> > > + * hypervisor mode or not. When KVM guest is using the debug \
> > > + * resource then we do not want debug events to be captured \
> > > + * in guest entry/exit path. So we set EPCR.DUVD when entering \
> > > + * and clears EPCR.DUVD when exiting from guest. \
> > > + * Debug instruction complete is a post-completion debug \
> > > + * exception but debug event gets posted on the basis of MSR \
> > > + * before the instruction is executed. Now if the instruction \
> > > + * switches the context from guest mode (MSR.GS = 1) to hypervisor \
> > > + * mode (MSR.GS = 0) then the xSRR0 points to first instruction of \
> > > + * KVM handler and xSRR1 points that MSR.GS is clear \
> > > + * (hypervisor context). Now as xSRR1.GS is used to decide whether \
> > > + * KVM handler will be invoked to handle the exception or host \
> > > + * host kernel debug handler will be invoked to handle the exception. \
> > > + * This leads to host kernel debug handler handling the exception \
> > > + * which should either be handled by KVM. \
> > > + */ \
> > > + mfspr r10, SPRN_EPCR; \
> > > + andis. r10,r10,SPRN_EPCR_DUVD@h; \
> > > + beq+ 3f; \
> > > \
> > > + andis. r10,r9,MSR_GS@h; \
> > > + beq+ 1f; \
> > > +3: \
> > > +#endif \
> > > lis r10,KERNELBASE@h; /* check if exception in vectors */ \
> > > ori r10,r10,KERNELBASE@l; \
> > > cmplw r12,r10; \
> > > --
> > > 1.7.0.4
> > >
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe kvm-ppc"
> > > in the body of a message to majordomo@vger.kernel.org More majordomo
> > > info at http://vger.kernel.org/majordomo-info.html
> >
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Apr 5, 2013, at 2:53 AM, Bhushan Bharat-R65777 wrote: > Hi Kumar/Benh, > > After further looking into the code I think that if we correct the vector range below in DebugDebug handler then we do not need the change I provided in this patch. > > Here is the snapshot for 32 bit (head_booke.h, same will be true for 64 bit): > > #define DEBUG_DEBUG_EXCEPTION \ > START_EXCEPTION(DebugDebug); \ > DEBUG_EXCEPTION_PROLOG; \ > \ > /* \ > * If there is a single step or branch-taken exception in an \ > * exception entry sequence, it was probably meant to apply to \ > * the code where the exception occurred (since exception entry \ > * doesn't turn off DE automatically). We simulate the effect \ > * of turning off DE on entry to an exception handler by turning \ > * off DE in the DSRR1 value and clearing the debug status. \ > */ \ > mfspr r10,SPRN_DBSR; /* check single-step/branch taken */ \ > andis. r10,r10,(DBSR_IC|DBSR_BT)@h; \ > beq+ 2f; \ > \ > lis r10,KERNELBASE@h; /* check if exception in vectors */ \ > ori r10,r10,KERNELBASE@l; \ > cmplw r12,r10; \ > blt+ 2f; /* addr below exception vectors */ \ > \ > lis r10,DebugDebug@h; \ > ori r10,r10,DebugDebug@l; \ > > ^^^^ > Here we assume all exception vector ends at DebugDebug, which is not correct. > We probably should get proper end by using some start_vector and end_vector lebels > or at least use end at Ehvpriv (which is last defined in head_fsl_booke.S for PowerPC. Is that correct? > > > cmplw r12,r10; \ > bgt+ 2f; /* addr above exception vectors */ \ > > Thanks > -Bharat I talked to Stuart and this general approach is good. Just make sure to update both head_44x.S and head_fsl_booke.S. Plus do this for both DEBUG_CRIT_EXCEPTION & DEBUG_DEBUG_EXCEPTION - k-- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 11, 2013 at 1:33 PM, Kumar Gala <galak@kernel.crashing.org> wrote: > > On Apr 5, 2013, at 2:53 AM, Bhushan Bharat-R65777 wrote: > >> Hi Kumar/Benh, >> >> After further looking into the code I think that if we correct the vector range below in DebugDebug handler then we do not need the change I provided in this patch. >> >> Here is the snapshot for 32 bit (head_booke.h, same will be true for 64 bit): >> >> #define DEBUG_DEBUG_EXCEPTION \ >> START_EXCEPTION(DebugDebug); \ >> DEBUG_EXCEPTION_PROLOG; \ >> \ >> /* \ >> * If there is a single step or branch-taken exception in an \ >> * exception entry sequence, it was probably meant to apply to \ >> * the code where the exception occurred (since exception entry \ >> * doesn't turn off DE automatically). We simulate the effect \ >> * of turning off DE on entry to an exception handler by turning \ >> * off DE in the DSRR1 value and clearing the debug status. \ >> */ \ >> mfspr r10,SPRN_DBSR; /* check single-step/branch taken */ \ >> andis. r10,r10,(DBSR_IC|DBSR_BT)@h; \ >> beq+ 2f; \ >> \ >> lis r10,KERNELBASE@h; /* check if exception in vectors */ \ >> ori r10,r10,KERNELBASE@l; \ >> cmplw r12,r10; \ >> blt+ 2f; /* addr below exception vectors */ \ >> \ >> lis r10,DebugDebug@h; \ >> ori r10,r10,DebugDebug@l; \ >> >> ^^^^ >> Here we assume all exception vector ends at DebugDebug, which is not correct. >> We probably should get proper end by using some start_vector and end_vector lebels >> or at least use end at Ehvpriv (which is last defined in head_fsl_booke.S for PowerPC. Is that correct? >> >> >> cmplw r12,r10; \ >> bgt+ 2f; /* addr above exception vectors */ \ >> >> Thanks >> -Bharat > > I talked to Stuart and this general approach is good. Just make sure to update both head_44x.S and head_fsl_booke.S. Plus do this for both DEBUG_CRIT_EXCEPTION & DEBUG_DEBUG_EXCEPTION Also, it looks like 64-bit already handles this properly with symbols identifying the start/end of the vectors (exceptions-64e.S). Stuart -- To unsubscribe from this list: send the line "unsubscribe kvm" 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/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S index 4684e33..8b26294 100644 --- a/arch/powerpc/kernel/exceptions-64e.S +++ b/arch/powerpc/kernel/exceptions-64e.S @@ -516,6 +516,33 @@ kernel_dbg_exc: andis. r15,r14,DBSR_IC@h beq+ 1f +#ifdef CONFIG_KVM_BOOKE_HV + /* + * EPCR.DUVD controls whether the debug events can come in + * hypervisor mode or not. When KVM guest is using the debug + * resource then we do not want debug events to be captured + * in guest entry/exit path. So we set EPCR.DUVD when entering + * and clears EPCR.DUVD when exiting from guest. + * Debug instruction complete is a post-completion debug + * exception but debug event gets posted on the basis of MSR + * before the instruction is executed. Now if the instruction + * switches the context from guest mode (MSR.GS = 1) to hypervisor + * mode (MSR.GS = 0) then the xSRR0 points to first instruction of + * KVM handler and xSRR1 points that MSR.GS is clear + * (hypervisor context). Now as xSRR1.GS is used to decide whether + * KVM handler will be invoked to handle the exception or host + * host kernel debug handler will be invoked to handle the exception. + * This leads to host kernel debug handler handling the exception + * which should either be handled by KVM. + */ + mfspr r10, SPRN_EPCR + andis. r10,r10,SPRN_EPCR_DUVD@h + beq+ 2f + + andis. r10,r9,MSR_GS@h + beq+ 3f +2: +#endif LOAD_REG_IMMEDIATE(r14,interrupt_base_book3e) LOAD_REG_IMMEDIATE(r15,interrupt_end_book3e) cmpld cr0,r10,r14 @@ -523,7 +550,7 @@ kernel_dbg_exc: blt+ cr0,1f bge+ cr1,1f - /* here it looks like we got an inappropriate debug exception. */ +3: /* here it looks like we got an inappropriate debug exception. */ lis r14,DBSR_IC@h /* clear the IC event */ rlwinm r11,r11,0,~MSR_DE /* clear DE in the DSRR1 value */ mtspr SPRN_DBSR,r14 diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h index 5f051ee..edc6a3b 100644 --- a/arch/powerpc/kernel/head_booke.h +++ b/arch/powerpc/kernel/head_booke.h @@ -285,7 +285,33 @@ label: mfspr r10,SPRN_DBSR; /* check single-step/branch taken */ \ andis. r10,r10,(DBSR_IC|DBSR_BT)@h; \ beq+ 2f; \ +#ifdef CONFIG_KVM_BOOKE_HV \ + /* \ + * EPCR.DUVD controls whether the debug events can come in \ + * hypervisor mode or not. When KVM guest is using the debug \ + * resource then we do not want debug events to be captured \ + * in guest entry/exit path. So we set EPCR.DUVD when entering \ + * and clears EPCR.DUVD when exiting from guest. \ + * Debug instruction complete is a post-completion debug \ + * exception but debug event gets posted on the basis of MSR \ + * before the instruction is executed. Now if the instruction \ + * switches the context from guest mode (MSR.GS = 1) to hypervisor \ + * mode (MSR.GS = 0) then the xSRR0 points to first instruction of \ + * KVM handler and xSRR1 points that MSR.GS is clear \ + * (hypervisor context). Now as xSRR1.GS is used to decide whether \ + * KVM handler will be invoked to handle the exception or host \ + * host kernel debug handler will be invoked to handle the exception. \ + * This leads to host kernel debug handler handling the exception \ + * which should either be handled by KVM. \ + */ \ + mfspr r10, SPRN_EPCR; \ + andis. r10,r10,SPRN_EPCR_DUVD@h; \ + beq+ 3f; \ \ + andis. r10,r9,MSR_GS@h; \ + beq+ 1f; \ +3: \ +#endif \ lis r10,KERNELBASE@h; /* check if exception in vectors */ \ ori r10,r10,KERNELBASE@l; \ cmplw r12,r10; \
EPCR.DUVD controls whether the debug events can come in hypervisor mode or not. When KVM guest is using the debug resource then we do not want debug events to be captured in guest entry/exit path. So we set EPCR.DUVD when entering and clears EPCR.DUVD when exiting from guest. Debug instruction complete is a post-completion debug exception but debug event gets posted on the basis of MSR before the instruction is executed. Now if the instruction switches the context from guest mode (MSR.GS = 1) to hypervisor mode (MSR.GS = 0) then the xSRR0 points to first instruction of KVM handler and xSRR1 points that MSR.GS is clear (hypervisor context). Now as xSRR1.GS is used to decide whether KVM handler will be invoked to handle the exception or host host kernel debug handler will be invoked to handle the exception. This leads to host kernel debug handler handling the exception which should either be handled by KVM. This is tested on e500mc in 32 bit mode Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> --- v0: - Do not apply this change for debug_crit as we do not know those chips have issue or not. - corrected 64bit case branching arch/powerpc/kernel/exceptions-64e.S | 29 ++++++++++++++++++++++++++++- arch/powerpc/kernel/head_booke.h | 26 ++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletions(-)