| Message ID | 1372855359-13452-3-git-send-email-mihai.caraman@freescale.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 03.07.2013, at 14:42, Mihai Caraman wrote: > SPE/FP/AltiVec interrupts share the same numbers. Refactor SPE/FP exit handling > to accommodate AltiVec later. Detect the targeted unit at run time since it can > be configured in the kernel but not featured on hardware. > > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com> > --- > arch/powerpc/kvm/booke.c | 102 +++++++++++++++++++++++++++++++--------------- > arch/powerpc/kvm/booke.h | 1 + > 2 files changed, 70 insertions(+), 33 deletions(-) > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index fb47e85..113961f 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -89,6 +89,15 @@ void kvmppc_dump_vcpu(struct kvm_vcpu *vcpu) > } > } > > +static inline bool kvmppc_supports_spe(void) > +{ > +#ifdef CONFIG_SPE > + if (cpu_has_feature(CPU_FTR_SPE)) > + return true; > +#endif > + return false; > +} > + > #ifdef CONFIG_SPE > void kvmppc_vcpu_disable_spe(struct kvm_vcpu *vcpu) > { > @@ -99,7 +108,7 @@ void kvmppc_vcpu_disable_spe(struct kvm_vcpu *vcpu) > preempt_enable(); > } > > -static void kvmppc_vcpu_enable_spe(struct kvm_vcpu *vcpu) > +void kvmppc_vcpu_enable_spe(struct kvm_vcpu *vcpu) > { > preempt_disable(); > enable_kernel_spe(); > @@ -118,6 +127,14 @@ static void kvmppc_vcpu_sync_spe(struct kvm_vcpu *vcpu) > } > } > #else > +void kvmppc_vcpu_disable_spe(struct kvm_vcpu *vcpu) > +{ > +} > + > +void kvmppc_vcpu_enable_spe(struct kvm_vcpu *vcpu) > +{ > +} > + > static void kvmppc_vcpu_sync_spe(struct kvm_vcpu *vcpu) > { > } > @@ -943,48 +960,67 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, > r = RESUME_GUEST; > break; > > -#ifdef CONFIG_SPE > 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_ALTIVEC_UNAVAIL); > + if (kvmppc_supports_spe()) { > + bool enabled = false; > + > +#ifndef CONFIG_KVM_BOOKE_HV > + if (vcpu->arch.shared->msr & MSR_SPE) { > + kvmppc_vcpu_enable_spe(vcpu); > + enabled = true; > + } > +#endif Why the #ifdef? On HV capable systems kvmppc_supports_spe() will just always return false. And I don't really understand why HV would be special in the first place here. Is it because we're accessing shared->msr? > + if (!enabled) > + kvmppc_booke_queue_irqprio(vcpu, > + BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL); > + } else { > + /* > + * Guest wants SPE, but host kernel doesn't support it. host kernel or hardware Alex
> > -#ifdef CONFIG_SPE > > 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_ALTIVEC_UNAVAIL); > > + if (kvmppc_supports_spe()) { > > + bool enabled = false; > > + > > +#ifndef CONFIG_KVM_BOOKE_HV > > + if (vcpu->arch.shared->msr & MSR_SPE) { > > + kvmppc_vcpu_enable_spe(vcpu); > > + enabled = true; > > + } > > +#endif > > Why the #ifdef? On HV capable systems kvmppc_supports_spe() will just > always return false. AltiVec and SPE unavailable exceptions follows the same path. While kvmppc_supports_spe() will always return false kvmppc_supports_altivec() may not. > And I don't really understand why HV would be special in the first place > here. Is it because we're accessing shared->msr? You are right on HV case MSP[SPV] should be always zero when an unavailabe exception take place. The distrinction was made because on non HV the guest doesn't have direct access to MSR[SPE]. The name of the bit (not the position) was changed on HV cores. > > > + if (!enabled) > > + kvmppc_booke_queue_irqprio(vcpu, > > + BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL); > > + } else { > > + /* > > + * Guest wants SPE, but host kernel doesn't support it. > > host kernel or hardware Ok. -Mike
On 03.07.2013, at 15:53, Caraman Mihai Claudiu-B02008 wrote: >>> -#ifdef CONFIG_SPE >>> 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_ALTIVEC_UNAVAIL); >>> + if (kvmppc_supports_spe()) { >>> + bool enabled = false; >>> + >>> +#ifndef CONFIG_KVM_BOOKE_HV >>> + if (vcpu->arch.shared->msr & MSR_SPE) { >>> + kvmppc_vcpu_enable_spe(vcpu); >>> + enabled = true; >>> + } >>> +#endif >> >> Why the #ifdef? On HV capable systems kvmppc_supports_spe() will just >> always return false. > > AltiVec and SPE unavailable exceptions follows the same path. While > kvmppc_supports_spe() will always return false kvmppc_supports_altivec() > may not. There is no chip that supports SPE and HV at the same time. So we'll never hit this anyway, since kvmppc_supports_spe() always returns false on HV capable systems. Just add a comment saying so and remove the ifdef :). Alex > >> And I don't really understand why HV would be special in the first place >> here. Is it because we're accessing shared->msr? > > You are right on HV case MSP[SPV] should be always zero when an unavailabe > exception take place. The distrinction was made because on non HV the guest > doesn't have direct access to MSR[SPE]. The name of the bit (not the position) > was changed on HV cores. > >> >>> + if (!enabled) >>> + kvmppc_booke_queue_irqprio(vcpu, >>> + BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL); >>> + } else { >>> + /* >>> + * Guest wants SPE, but host kernel doesn't support it. >> >> host kernel or hardware > > Ok. > > -Mike >
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index fb47e85..113961f 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -89,6 +89,15 @@ void kvmppc_dump_vcpu(struct kvm_vcpu *vcpu) } } +static inline bool kvmppc_supports_spe(void) +{ +#ifdef CONFIG_SPE + if (cpu_has_feature(CPU_FTR_SPE)) + return true; +#endif + return false; +} + #ifdef CONFIG_SPE void kvmppc_vcpu_disable_spe(struct kvm_vcpu *vcpu) { @@ -99,7 +108,7 @@ void kvmppc_vcpu_disable_spe(struct kvm_vcpu *vcpu) preempt_enable(); } -static void kvmppc_vcpu_enable_spe(struct kvm_vcpu *vcpu) +void kvmppc_vcpu_enable_spe(struct kvm_vcpu *vcpu) { preempt_disable(); enable_kernel_spe(); @@ -118,6 +127,14 @@ static void kvmppc_vcpu_sync_spe(struct kvm_vcpu *vcpu) } } #else +void kvmppc_vcpu_disable_spe(struct kvm_vcpu *vcpu) +{ +} + +void kvmppc_vcpu_enable_spe(struct kvm_vcpu *vcpu) +{ +} + static void kvmppc_vcpu_sync_spe(struct kvm_vcpu *vcpu) { } @@ -943,48 +960,67 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, r = RESUME_GUEST; break; -#ifdef CONFIG_SPE 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_ALTIVEC_UNAVAIL); + if (kvmppc_supports_spe()) { + bool enabled = false; + +#ifndef CONFIG_KVM_BOOKE_HV + if (vcpu->arch.shared->msr & MSR_SPE) { + kvmppc_vcpu_enable_spe(vcpu); + enabled = true; + } +#endif + if (!enabled) + kvmppc_booke_queue_irqprio(vcpu, + BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL); + } else { + /* + * Guest wants SPE, but host kernel doesn't support it. + * Send an "unimplemented operation" program check to + * the guest. + */ + kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); + } + r = RESUME_GUEST; break; } case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST: - kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST); - r = RESUME_GUEST; - break; - - case BOOKE_INTERRUPT_SPE_FP_ROUND: - kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SPE_FP_ROUND); - r = RESUME_GUEST; - break; -#else - 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. - */ - kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); - r = RESUME_GUEST; + if (kvmppc_supports_spe()) { + kvmppc_booke_queue_irqprio(vcpu, + BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST); + r = RESUME_GUEST; + } else { + /* + * These really should never happen without CONFIG_SPE, + * as we should never enable the real MSR[SPE] in the + * guest. + */ + printk(KERN_CRIT "%s: unexpected SPE interrupt %u at \ + %08lx\n", __func__, exit_nr, vcpu->arch.pc); + run->hw.hardware_exit_reason = exit_nr; + r = RESUME_HOST; + } break; - /* - * 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_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); - run->hw.hardware_exit_reason = exit_nr; - r = RESUME_HOST; + if (kvmppc_supports_spe()) { + kvmppc_booke_queue_irqprio(vcpu, + BOOKE_IRQPRIO_SPE_FP_ROUND); + r = RESUME_GUEST; + } else { + /* + * These really should never happen without CONFIG_SPE, + * as we should never enable the real MSR[SPE] in the + * guest. + */ + printk(KERN_CRIT "%s: unexpected SPE interrupt %u at %08lx\n", + __func__, exit_nr, vcpu->arch.pc); + run->hw.hardware_exit_reason = exit_nr; + r = RESUME_HOST; + } break; -#endif case BOOKE_INTERRUPT_DATA_STORAGE: kvmppc_core_queue_data_storage(vcpu, vcpu->arch.fault_dear, diff --git a/arch/powerpc/kvm/booke.h b/arch/powerpc/kvm/booke.h index 9e92006..e92ce5b 100644 --- a/arch/powerpc/kvm/booke.h +++ b/arch/powerpc/kvm/booke.h @@ -85,6 +85,7 @@ void kvmppc_load_guest_spe(struct kvm_vcpu *vcpu); void kvmppc_save_guest_spe(struct kvm_vcpu *vcpu); /* high-level function, manages flags, host state */ +void kvmppc_vcpu_enable_spe(struct kvm_vcpu *vcpu); void kvmppc_vcpu_disable_spe(struct kvm_vcpu *vcpu); void kvmppc_booke_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
SPE/FP/AltiVec interrupts share the same numbers. Refactor SPE/FP exit handling to accommodate AltiVec later. Detect the targeted unit at run time since it can be configured in the kernel but not featured on hardware. Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com> --- arch/powerpc/kvm/booke.c | 102 +++++++++++++++++++++++++++++++--------------- arch/powerpc/kvm/booke.h | 1 + 2 files changed, 70 insertions(+), 33 deletions(-)