| Message ID | 1404142497-6430-2-git-send-email-mihai.caraman@freescale.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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@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). Alex > KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_ROUND SPRN_SPRG_RSCRATCH0 SPRN_SRR0 > _GLOBAL(kvmppc_handlers_end) > > @@ -525,8 +526,8 @@ KVM_HANDLER_ADDR BOOKE_INTERRUPT_WATCHDOG > KVM_HANDLER_ADDR BOOKE_INTERRUPT_DTLB_MISS > KVM_HANDLER_ADDR BOOKE_INTERRUPT_ITLB_MISS > KVM_HANDLER_ADDR BOOKE_INTERRUPT_DEBUG > -KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_UNAVAIL > -KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_FP_DATA > +KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL > +KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST > KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_FP_ROUND > KVM_HANDLER_END /*Always keep this in end*/ > > diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S > index a1712b8..ff73143 100644 > --- a/arch/powerpc/kvm/bookehv_interrupts.S > +++ b/arch/powerpc/kvm/bookehv_interrupts.S > @@ -300,9 +300,9 @@ kvm_handler BOOKE_INTERRUPT_DTLB_MISS, EX_PARAMS_TLB, \ > SPRN_SRR0, SPRN_SRR1, (NEED_EMU | NEED_DEAR | NEED_ESR) > kvm_handler BOOKE_INTERRUPT_ITLB_MISS, EX_PARAMS_TLB, \ > SPRN_SRR0, SPRN_SRR1, 0 > -kvm_handler BOOKE_INTERRUPT_SPE_UNAVAIL, EX_PARAMS(GEN), \ > +kvm_handler BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ > SPRN_SRR0, SPRN_SRR1, 0 > -kvm_handler BOOKE_INTERRUPT_SPE_FP_DATA, EX_PARAMS(GEN), \ > +kvm_handler BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ > SPRN_SRR0, SPRN_SRR1, 0 > kvm_handler BOOKE_INTERRUPT_SPE_FP_ROUND, EX_PARAMS(GEN), \ > SPRN_SRR0, SPRN_SRR1, 0 > diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c > index 2e02ed8..3c1a30d 100644 > --- a/arch/powerpc/kvm/e500.c > +++ b/arch/powerpc/kvm/e500.c > @@ -383,8 +383,10 @@ static int kvmppc_core_get_sregs_e500(struct kvm_vcpu *vcpu, > sregs->u.e.impl.fsl.hid0 = vcpu_e500->hid0; > sregs->u.e.impl.fsl.mcar = vcpu_e500->mcar; > > - sregs->u.e.ivor_high[0] = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL]; > - sregs->u.e.ivor_high[1] = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA]; > + sregs->u.e.ivor_high[0] = > + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL]; > + sregs->u.e.ivor_high[1] = > + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST]; > sregs->u.e.ivor_high[2] = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND]; > sregs->u.e.ivor_high[3] = > vcpu->arch.ivor[BOOKE_IRQPRIO_PERFORMANCE_MONITOR]; > @@ -414,9 +416,9 @@ static int kvmppc_core_set_sregs_e500(struct kvm_vcpu *vcpu, > return 0; > > if (sregs->u.e.features & KVM_SREGS_E_SPE) { > - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL] = > + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL] = > sregs->u.e.ivor_high[0]; > - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA] = > + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST] = > sregs->u.e.ivor_high[1]; > vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND] = > sregs->u.e.ivor_high[2]; > diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c > index 98a22e5..6a6833f 100644 > --- a/arch/powerpc/kvm/e500_emulate.c > +++ b/arch/powerpc/kvm/e500_emulate.c > @@ -256,10 +256,11 @@ int kvmppc_core_emulate_mtspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong spr_va > > /* extra exceptions */ > case SPRN_IVOR32: > - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL] = spr_val; > + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL] = spr_val; > break; > case SPRN_IVOR33: > - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA] = spr_val; > + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST] = > + spr_val; > break; > case SPRN_IVOR34: > vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND] = spr_val; > @@ -378,10 +379,11 @@ int kvmppc_core_emulate_mfspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong *spr_v > > /* extra exceptions */ > case SPRN_IVOR32: > - *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL]; > + *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL]; > break; > case SPRN_IVOR33: > - *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA]; > + *spr_val = > + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST]; > break; > case SPRN_IVOR34: > *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND];
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Thursday, July 03, 2014 3:21 PM > To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org > Cc: kvm@vger.kernel.org; linuxppc-dev@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@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. What is the reason from changing your mind from ver 1? Do you want to have different defines with same values (we specifically mapped them to the hardware interrupt numbers). We already upstreamed the necessary changes in the kernel. Scott, please share your opinion here. > > > #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 -Mike
On 03.07.14 17:25, mihai.caraman@freescale.com wrote: >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Thursday, July 03, 2014 3:21 PM >> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org >> Cc: kvm@vger.kernel.org; linuxppc-dev@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@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. > What is the reason from changing your mind from ver 1? Do you want to have Uh, mind to point me to an email where I said I like the approach? :) > different defines with same values (we specifically mapped them to the > hardware interrupt numbers). We already upstreamed the necessary changes Yes, I think that'd end up the most readable flow of things. > in the kernel. Scott, please share your opinion here. I'm not going to be religious about it, but names like "BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST" are 1) too long 2) too ambiguous It just means the code gets harder to read. Any way we can take to simplify the code flow is a win IMHO. And if I don't even remotely have to consider SPE when reading an Altivec path, I think that's a good thing :). Alex
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Thursday, July 03, 2014 6:31 PM > To: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; kvm- > ppc@vger.kernel.org > Cc: kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for > SPE/FP/AltiVec int numbers > > > On 03.07.14 17:25, mihai.caraman@freescale.com wrote: > >> -----Original Message----- > >> From: Alexander Graf [mailto:agraf@suse.de] > >> Sent: Thursday, July 03, 2014 3:21 PM > >> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org > >> Cc: kvm@vger.kernel.org; linuxppc-dev@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@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. > > What is the reason from changing your mind from ver 1? Do you want to > have > > Uh, mind to point me to an email where I said I like the approach? :) You tacitly approved it in this thread ... I did not say you like it :) https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-July/108501.html -Mike
On Thu, 2014-07-03 at 10:25 -0500, Caraman Mihai Claudiu-B02008 wrote: > > -----Original Message----- > > From: Alexander Graf [mailto:agraf@suse.de] > > Sent: Thursday, July 03, 2014 3:21 PM > > To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org > > Cc: kvm@vger.kernel.org; linuxppc-dev@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@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. > > What is the reason from changing your mind from ver 1? Do you want to have > different defines with same values (we specifically mapped them to the > hardware interrupt numbers). We already upstreamed the necessary changes > in the kernel. Scott, please share your opinion here. I don't like hiding the fact that they're the same number, which could lead to wrong code in the absence of ifdefs that strictly mutually exclude SPE and Altivec code -- there was an instance of this with MSR_VEC versus MSR_SPE in a previous patchset. > > > #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 I don't think that's an improvement. -Scott
On Thu, 2014-07-03 at 17:15 -0500, Scott Wood wrote: > On Thu, 2014-07-03 at 10:25 -0500, Caraman Mihai Claudiu-B02008 wrote: > > > -----Original Message----- > > > From: Alexander Graf [mailto:agraf@suse.de] > > > Sent: Thursday, July 03, 2014 3:21 PM > > > To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org > > > Cc: kvm@vger.kernel.org; linuxppc-dev@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@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. > > > > What is the reason from changing your mind from ver 1? Do you want to have > > different defines with same values (we specifically mapped them to the > > hardware interrupt numbers). We already upstreamed the necessary changes > > in the kernel. Scott, please share your opinion here. > > I don't like hiding the fact that they're the same number, which could > lead to wrong code in the absence of ifdefs that strictly mutually > exclude SPE and Altivec code -- there was an instance of this with > MSR_VEC versus MSR_SPE in a previous patchset. That said, if you want to enforce that mutual exclusion in a way that is clear, I won't object too loudly -- but the code does look pretty similar between the two (as well as between the two IVORs). -Scott
On 04.07.14 00:15, Scott Wood wrote: > On Thu, 2014-07-03 at 10:25 -0500, Caraman Mihai Claudiu-B02008 wrote: >>> -----Original Message----- >>> From: Alexander Graf [mailto:agraf@suse.de] >>> Sent: Thursday, July 03, 2014 3:21 PM >>> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org >>> Cc: kvm@vger.kernel.org; linuxppc-dev@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@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. >> What is the reason from changing your mind from ver 1? Do you want to have >> different defines with same values (we specifically mapped them to the >> hardware interrupt numbers). We already upstreamed the necessary changes >> in the kernel. Scott, please share your opinion here. > I don't like hiding the fact that they're the same number, which could > lead to wrong code in the absence of ifdefs that strictly mutually > exclude SPE and Altivec code -- there was an instance of this with > MSR_VEC versus MSR_SPE in a previous patchset. The nice thing here is that we use almost all of these numbers in switch() statements which give us automated duplicate checking - so we don't accidentally go into the wrong code path without knowing it. Alex
On 04.07.14 00:31, Scott Wood wrote: > On Thu, 2014-07-03 at 17:15 -0500, Scott Wood wrote: >> On Thu, 2014-07-03 at 10:25 -0500, Caraman Mihai Claudiu-B02008 wrote: >>>> -----Original Message----- >>>> From: Alexander Graf [mailto:agraf@suse.de] >>>> Sent: Thursday, July 03, 2014 3:21 PM >>>> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org >>>> Cc: kvm@vger.kernel.org; linuxppc-dev@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@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. >>> What is the reason from changing your mind from ver 1? Do you want to have >>> different defines with same values (we specifically mapped them to the >>> hardware interrupt numbers). We already upstreamed the necessary changes >>> in the kernel. Scott, please share your opinion here. >> I don't like hiding the fact that they're the same number, which could >> lead to wrong code in the absence of ifdefs that strictly mutually >> exclude SPE and Altivec code -- there was an instance of this with >> MSR_VEC versus MSR_SPE in a previous patchset. > That said, if you want to enforce that mutual exclusion in a way that is > clear, I won't object too loudly -- but the code does look pretty > similar between the two (as well as between the two IVORs). Yes, I want to make sure we have 2 separate code paths for SPE and Altivec. No code sharing at all unless it's very generically possible. Also, which code does look pretty similar? The fact that we deflect interrupts back into the guest? That's mostly boilerplate. Alex
On Fri, 2014-07-04 at 00:35 +0200, Alexander Graf wrote: > On 04.07.14 00:31, Scott Wood wrote: > > On Thu, 2014-07-03 at 17:15 -0500, Scott Wood wrote: > >> On Thu, 2014-07-03 at 10:25 -0500, Caraman Mihai Claudiu-B02008 wrote: > >>>> -----Original Message----- > >>>> From: Alexander Graf [mailto:agraf@suse.de] > >>>> Sent: Thursday, July 03, 2014 3:21 PM > >>>> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org > >>>> Cc: kvm@vger.kernel.org; linuxppc-dev@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@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. > >>> What is the reason from changing your mind from ver 1? Do you want to have > >>> different defines with same values (we specifically mapped them to the > >>> hardware interrupt numbers). We already upstreamed the necessary changes > >>> in the kernel. Scott, please share your opinion here. > >> I don't like hiding the fact that they're the same number, which could > >> lead to wrong code in the absence of ifdefs that strictly mutually > >> exclude SPE and Altivec code -- there was an instance of this with > >> MSR_VEC versus MSR_SPE in a previous patchset. > > That said, if you want to enforce that mutual exclusion in a way that is > > clear, I won't object too loudly -- but the code does look pretty > > similar between the two (as well as between the two IVORs). > > Yes, I want to make sure we have 2 separate code paths for SPE and > Altivec. No code sharing at all unless it's very generically possible. > > Also, which code does look pretty similar? The fact that we deflect > interrupts back into the guest? That's mostly boilerplate. There's also the injection of a program check (or exiting to userspace) when CONFIG_SPE/ALTIVEC is missing. Not a big deal, but maybe it could be factored into a helper function. I like minimizing boilerplate. -Scott
On 04.07.14 01:00, Scott Wood wrote: > On Fri, 2014-07-04 at 00:35 +0200, Alexander Graf wrote: >> On 04.07.14 00:31, Scott Wood wrote: >>> On Thu, 2014-07-03 at 17:15 -0500, Scott Wood wrote: >>>> On Thu, 2014-07-03 at 10:25 -0500, Caraman Mihai Claudiu-B02008 wrote: >>>>>> -----Original Message----- >>>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>>> Sent: Thursday, July 03, 2014 3:21 PM >>>>>> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org >>>>>> Cc: kvm@vger.kernel.org; linuxppc-dev@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@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. >>>>> What is the reason from changing your mind from ver 1? Do you want to have >>>>> different defines with same values (we specifically mapped them to the >>>>> hardware interrupt numbers). We already upstreamed the necessary changes >>>>> in the kernel. Scott, please share your opinion here. >>>> I don't like hiding the fact that they're the same number, which could >>>> lead to wrong code in the absence of ifdefs that strictly mutually >>>> exclude SPE and Altivec code -- there was an instance of this with >>>> MSR_VEC versus MSR_SPE in a previous patchset. >>> That said, if you want to enforce that mutual exclusion in a way that is >>> clear, I won't object too loudly -- but the code does look pretty >>> similar between the two (as well as between the two IVORs). >> Yes, I want to make sure we have 2 separate code paths for SPE and >> Altivec. No code sharing at all unless it's very generically possible. >> >> Also, which code does look pretty similar? The fact that we deflect >> interrupts back into the guest? That's mostly boilerplate. > There's also the injection of a program check (or exiting to userspace) > when CONFIG_SPE/ALTIVEC is missing. Not a big deal, but maybe it could > be factored into a helper function. I like minimizing boilerplate. Yes, me too - but I also like to be explicit. If there's enough code to share, factoring those into helpers certainly works well for me. Alex
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Thursday, July 03, 2014 3:21 PM > To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org > Cc: kvm@vger.kernel.org; linuxppc-dev@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@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
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 #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: 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 #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 KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_ROUND SPRN_SPRG_RSCRATCH0 SPRN_SRR0 _GLOBAL(kvmppc_handlers_end) @@ -525,8 +526,8 @@ KVM_HANDLER_ADDR BOOKE_INTERRUPT_WATCHDOG KVM_HANDLER_ADDR BOOKE_INTERRUPT_DTLB_MISS KVM_HANDLER_ADDR BOOKE_INTERRUPT_ITLB_MISS KVM_HANDLER_ADDR BOOKE_INTERRUPT_DEBUG -KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_UNAVAIL -KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_FP_DATA +KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL +KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_FP_ROUND KVM_HANDLER_END /*Always keep this in end*/ diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index a1712b8..ff73143 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -300,9 +300,9 @@ kvm_handler BOOKE_INTERRUPT_DTLB_MISS, EX_PARAMS_TLB, \ SPRN_SRR0, SPRN_SRR1, (NEED_EMU | NEED_DEAR | NEED_ESR) kvm_handler BOOKE_INTERRUPT_ITLB_MISS, EX_PARAMS_TLB, \ SPRN_SRR0, SPRN_SRR1, 0 -kvm_handler BOOKE_INTERRUPT_SPE_UNAVAIL, EX_PARAMS(GEN), \ +kvm_handler BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 -kvm_handler BOOKE_INTERRUPT_SPE_FP_DATA, EX_PARAMS(GEN), \ +kvm_handler BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 kvm_handler BOOKE_INTERRUPT_SPE_FP_ROUND, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c index 2e02ed8..3c1a30d 100644 --- a/arch/powerpc/kvm/e500.c +++ b/arch/powerpc/kvm/e500.c @@ -383,8 +383,10 @@ static int kvmppc_core_get_sregs_e500(struct kvm_vcpu *vcpu, sregs->u.e.impl.fsl.hid0 = vcpu_e500->hid0; sregs->u.e.impl.fsl.mcar = vcpu_e500->mcar; - sregs->u.e.ivor_high[0] = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL]; - sregs->u.e.ivor_high[1] = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA]; + sregs->u.e.ivor_high[0] = + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL]; + sregs->u.e.ivor_high[1] = + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST]; sregs->u.e.ivor_high[2] = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND]; sregs->u.e.ivor_high[3] = vcpu->arch.ivor[BOOKE_IRQPRIO_PERFORMANCE_MONITOR]; @@ -414,9 +416,9 @@ static int kvmppc_core_set_sregs_e500(struct kvm_vcpu *vcpu, return 0; if (sregs->u.e.features & KVM_SREGS_E_SPE) { - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL] = + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL] = sregs->u.e.ivor_high[0]; - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA] = + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST] = sregs->u.e.ivor_high[1]; vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND] = sregs->u.e.ivor_high[2]; diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c index 98a22e5..6a6833f 100644 --- a/arch/powerpc/kvm/e500_emulate.c +++ b/arch/powerpc/kvm/e500_emulate.c @@ -256,10 +256,11 @@ int kvmppc_core_emulate_mtspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong spr_va /* extra exceptions */ case SPRN_IVOR32: - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL] = spr_val; + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL] = spr_val; break; case SPRN_IVOR33: - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA] = spr_val; + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST] = + spr_val; break; case SPRN_IVOR34: vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND] = spr_val; @@ -378,10 +379,11 @@ int kvmppc_core_emulate_mfspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong *spr_v /* extra exceptions */ case SPRN_IVOR32: - *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL]; + *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL]; break; case SPRN_IVOR33: - *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA]; + *spr_val = + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST]; break; case SPRN_IVOR34: *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND];
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@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(-)