[PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
mihai.caraman at freescale.com
mihai.caraman at freescale.com
Mon Jul 21 23:23:26 EST 2014
> -----Original Message-----
> From: Alexander Graf [mailto:agraf at suse.de]
> Sent: Thursday, July 03, 2014 3:21 PM
> To: Caraman Mihai Claudiu-B02008; kvm-ppc at vger.kernel.org
> Cc: kvm at vger.kernel.org; linuxppc-dev at lists.ozlabs.org
> Subject: Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for
> SPE/FP/AltiVec int numbers
>
>
> On 30.06.14 17:34, Mihai Caraman wrote:
> > Use common BOOKE_IRQPRIO and BOOKE_INTERRUPT defines for SPE/FP/AltiVec
> > which share the same interrupt numbers.
> >
> > Signed-off-by: Mihai Caraman <mihai.caraman at freescale.com>
> > ---
> > v2:
> > - remove outdated definitions
> >
> > arch/powerpc/include/asm/kvm_asm.h | 8 --------
> > arch/powerpc/kvm/booke.c | 17 +++++++++--------
> > arch/powerpc/kvm/booke.h | 4 ++--
> > arch/powerpc/kvm/booke_interrupts.S | 9 +++++----
> > arch/powerpc/kvm/bookehv_interrupts.S | 4 ++--
> > arch/powerpc/kvm/e500.c | 10 ++++++----
> > arch/powerpc/kvm/e500_emulate.c | 10 ++++++----
> > 7 files changed, 30 insertions(+), 32 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm_asm.h
> b/arch/powerpc/include/asm/kvm_asm.h
> > index 9601741..c94fd33 100644
> > --- a/arch/powerpc/include/asm/kvm_asm.h
> > +++ b/arch/powerpc/include/asm/kvm_asm.h
> > @@ -56,14 +56,6 @@
> > /* E500 */
> > #define BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL 32
> > #define BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST 33
> > -/*
> > - * TODO: Unify 32-bit and 64-bit kernel exception handlers to use same
> defines
> > - */
> > -#define BOOKE_INTERRUPT_SPE_UNAVAIL
> BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL
> > -#define BOOKE_INTERRUPT_SPE_FP_DATA
> BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST
> > -#define BOOKE_INTERRUPT_ALTIVEC_UNAVAIL
> BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL
> > -#define BOOKE_INTERRUPT_ALTIVEC_ASSIST \
> > - BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST
>
> I think I'd prefer to keep them separate.
>
> > #define BOOKE_INTERRUPT_SPE_FP_ROUND 34
> > #define BOOKE_INTERRUPT_PERFORMANCE_MONITOR 35
> > #define BOOKE_INTERRUPT_DOORBELL 36
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > index ab62109..3c86d9b 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -388,8 +388,8 @@ static int kvmppc_booke_irqprio_deliver(struct
> kvm_vcpu *vcpu,
> > case BOOKE_IRQPRIO_ITLB_MISS:
> > case BOOKE_IRQPRIO_SYSCALL:
> > case BOOKE_IRQPRIO_FP_UNAVAIL:
> > - case BOOKE_IRQPRIO_SPE_UNAVAIL:
> > - case BOOKE_IRQPRIO_SPE_FP_DATA:
> > + case BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL:
> > + case BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST:
>
> #ifdef CONFIG_KVM_E500V2
> case ...SPE:
> #else
> case ..ALTIVEC:
> #endif
>
> > case BOOKE_IRQPRIO_SPE_FP_ROUND:
> > case BOOKE_IRQPRIO_AP_UNAVAIL:
> > allowed = 1;
> > @@ -977,18 +977,19 @@ int kvmppc_handle_exit(struct kvm_run *run,
> struct kvm_vcpu *vcpu,
> > break;
> >
> > #ifdef CONFIG_SPE
> > - case BOOKE_INTERRUPT_SPE_UNAVAIL: {
> > + case BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL: {
> > if (vcpu->arch.shared->msr & MSR_SPE)
> > kvmppc_vcpu_enable_spe(vcpu);
> > else
> > kvmppc_booke_queue_irqprio(vcpu,
> > - BOOKE_IRQPRIO_SPE_UNAVAIL);
> > + BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL);
> > r = RESUME_GUEST;
> > break;
> > }
> >
> > - case BOOKE_INTERRUPT_SPE_FP_DATA:
> > - kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SPE_FP_DATA);
> > + case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST:
> > + kvmppc_booke_queue_irqprio(vcpu,
> > + BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST);
> > r = RESUME_GUEST;
> > break;
> >
> > @@ -997,7 +998,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
> > r = RESUME_GUEST;
> > break;
> > #else
> > - case BOOKE_INTERRUPT_SPE_UNAVAIL:
> > + case BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL:
> > /*
> > * Guest wants SPE, but host kernel doesn't support it. Send
> > * an "unimplemented operation" program check to the guest.
> > @@ -1010,7 +1011,7 @@ int kvmppc_handle_exit(struct kvm_run *run,
> struct kvm_vcpu *vcpu,
> > * These really should never happen without CONFIG_SPE,
> > * as we should never enable the real MSR[SPE] in the guest.
> > */
> > - case BOOKE_INTERRUPT_SPE_FP_DATA:
> > + case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST:
> > case BOOKE_INTERRUPT_SPE_FP_ROUND:
> > printk(KERN_CRIT "%s: unexpected SPE interrupt %u at
> %08lx\n",
> > __func__, exit_nr, vcpu->arch.pc);
> > diff --git a/arch/powerpc/kvm/booke.h b/arch/powerpc/kvm/booke.h
> > index b632cd3..f182b32 100644
> > --- a/arch/powerpc/kvm/booke.h
> > +++ b/arch/powerpc/kvm/booke.h
> > @@ -32,8 +32,8 @@
> > #define BOOKE_IRQPRIO_ALIGNMENT 2
> > #define BOOKE_IRQPRIO_PROGRAM 3
> > #define BOOKE_IRQPRIO_FP_UNAVAIL 4
> > -#define BOOKE_IRQPRIO_SPE_UNAVAIL 5
> > -#define BOOKE_IRQPRIO_SPE_FP_DATA 6
> > +#define BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL 5
> > +#define BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST 6
>
> #ifdef CONFIG_KVM_E500V2
> #define ...SPE...
> #else
> #define ...ALTIVEC...
> #endif
>
> That way we can be 100% sure that no SPE cruft leaks into anything.
>
>
> > #define BOOKE_IRQPRIO_SPE_FP_ROUND 7
> > #define BOOKE_IRQPRIO_SYSCALL 8
> > #define BOOKE_IRQPRIO_AP_UNAVAIL 9
> > diff --git a/arch/powerpc/kvm/booke_interrupts.S
> b/arch/powerpc/kvm/booke_interrupts.S
> > index 2c6deb5ef..a275dc5 100644
> > --- a/arch/powerpc/kvm/booke_interrupts.S
> > +++ b/arch/powerpc/kvm/booke_interrupts.S
> > @@ -137,8 +137,9 @@ 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_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_ALTIVEC_UNAVAIL SPRN_SPRG_RSCRATCH0
> SPRN_SRR0
> > +KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST
> SPRN_SPRG_RSCRATCH0 \
> > + SPRN_SRR0
>
> Same thing here - just only trap SPE when CONFIG_KVM_E500V2 is available
> and trap altivec otherwise (to make sure we always have a handler).
This will not even build with current kernel. 32-bit FSL kernel defines SPE
handlers for e500v2/e500mc/e5500 (see head_fsl_booke.S which is guarded by
CONFIG_FSL_BOOKE) even when CONFIG_SPE is not defined. This is simple to
verify by removing KVM's HV 32-bit BOOKE_INTERRUPT_SPE_xxx handlers from
bookehv_interrupts.S.
The kernel equivalent of your CONFIG_KVM_E500V2 suggestion looks like this:
#ifndef CONFIG_PPC_E500MC
/* e500v2 */
#define BOOKE_INTERRUPT_SPE_UNAVAIL 32
#define BOOKE_INTERRUPT_SPE_FP_DATA 33
#define BOOKE_INTERRUPT_SPE_FP_ROUND 34
#elif
/* e500mc, e5500, e6500 */
#define BOOKE_INTERRUPT_ALTIVEC_UNAVAIL 32
#define BOOKE_INTERRUPT_ALTIVEC_ASSIST BOOKE_33
#endif
but instead, the current kernel expects something like this:
#ifdef CONFIG_FSL_BOOKE
/* 32-bit targets: e500v2, e500mc, e5500 */
#define BOOKE_INTERRUPT_SPE_UNAVAIL 32
#define BOOKE_INTERRUPT_SPE_FP_DATA 33
#define BOOKE_INTERRUPT_SPE_FP_ROUND 34
#elif CONFIG_PPC_BOOK3E_64
/* 64-bit targets: e5500, e6500 */
#define BOOKE_INTERRUPT_ALTIVEC_UNAVAIL 32
#define BOOKE_INTERRUPT_ALTIVEC_ASSIST BOOKE_33
#endif
We can guard kernel SPE handlers with !CONFIG_PPC_E500MC to have
something like:
#ifdef CONFIG_FSL_BOOKE
#ifndef CONFIG_PPC_E500MC
/* e500v2 */
#define BOOKE_INTERRUPT_SPE_UNAVAIL 32
#define BOOKE_INTERRUPT_SPE_FP_DATA 33
#define BOOKE_INTERRUPT_SPE_FP_ROUND 34
#endif
#elif CONFIG_PPC_BOOK3E_64
/* e5500, e6500 */
#define BOOKE_INTERRUPT_ALTIVEC_UNAVAIL 32
#define BOOKE_INTERRUPT_ALTIVEC_ASSIST BOOKE_33
#endif
My suggestion is to go ahead with KVM AltiVec patches without waiting for
this kernel cleanup. I will do the KVM synchronization later when you will
have these kernel changes in your tree.
Scott, do you agree to guard SPE kernel handlers in head_fsl_booke.S with
!CONFIG_PPC_E500MC?
-Mike
>
> Alex
More information about the Linuxppc-dev
mailing list