| Message ID | 1370292868-2697-3-git-send-email-mihai.caraman@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/03/2013 03:54:24 PM, Mihai Caraman wrote: > SPE_FP interrupts are shared with ALTIVEC. Refactor SPE_FP exit > handling > to detect KVM support for the featured unit at run-time, in order to > accommodate ALTIVEC later. > > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com> > --- > arch/powerpc/kvm/booke.c | 80 > ++++++++++++++++++++++++++++++++++------------ > 1 files changed, 59 insertions(+), 21 deletions(-) > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index 1020119..d082bbc 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -822,6 +822,15 @@ static void kvmppc_restart_interrupt(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; > +} Whitespace > /** > * kvmppc_handle_exit > * > @@ -931,42 +940,71 @@ int kvmppc_handle_exit(struct kvm_run *run, > struct kvm_vcpu *vcpu, > r = RESUME_GUEST; > break; > > -#ifdef CONFIG_SPE > case BOOKE_INTERRUPT_SPE_UNAVAIL: { > - if (vcpu->arch.shared->msr & MSR_SPE) > - kvmppc_vcpu_enable_spe(vcpu); > - else > - kvmppc_booke_queue_irqprio(vcpu, > - > BOOKE_IRQPRIO_SPE_UNAVAIL); > + /* > + * The interrupt is shared, KVM support for the > featured unit > + * is detected at run-time. > + */ This is a decent comment for the changelog, but for the code itself it seems fairly obvious if you look at the definition of kvmppc_supports_spe(). > + bool handled = false; > + > + if (kvmppc_supports_spe()) { > +#ifdef CONFIG_SPE > + if (cpu_has_feature(CPU_FTR_SPE)) Didn't you already check this using kvmppc_supports_spe()? > case BOOKE_INTERRUPT_SPE_FP_ROUND: > +#ifdef CONFIG_SPE > kvmppc_booke_queue_irqprio(vcpu, > BOOKE_IRQPRIO_SPE_FP_ROUND); > r = RESUME_GUEST; > break; Why not use kvmppc_supports_spe() here, for consistency? -Scott -- 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
> > + /* > > + * The interrupt is shared, KVM support for the > > featured unit > > + * is detected at run-time. > > + */ > > This is a decent comment for the changelog, but for the code itself it > seems fairly obvious if you look at the definition of > kvmppc_supports_spe(). I will move it to change log. > > > + bool handled = false; > > + > > + if (kvmppc_supports_spe()) { > > +#ifdef CONFIG_SPE > > + if (cpu_has_feature(CPU_FTR_SPE)) > > Didn't you already check this using kvmppc_supports_spe()? It makes sense with the next patch. It handles the improbable case of having CONFIG_ALTIVEC and CONFIG_SPE defined: if (kvmppc_supports_altivec() || kvmppc_supports_spe()). > > > case BOOKE_INTERRUPT_SPE_FP_ROUND: > > +#ifdef CONFIG_SPE > > kvmppc_booke_queue_irqprio(vcpu, > > BOOKE_IRQPRIO_SPE_FP_ROUND); > > r = RESUME_GUEST; > > break; > > Why not use kvmppc_supports_spe() here, for consistency? I added cpu_has_feature(CPU_FTR_SPE) for the case specified above, but here SPE_FP_ROUND is not shared with ALTIVEC. CONFIG_SPE is used in other places in KVM without this check, shouldn't be all or nothing? -Mike -- 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/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..d082bbc 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -822,6 +822,15 @@ static void kvmppc_restart_interrupt(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; +} + /** * kvmppc_handle_exit * @@ -931,42 +940,71 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, r = RESUME_GUEST; break; -#ifdef CONFIG_SPE case BOOKE_INTERRUPT_SPE_UNAVAIL: { - if (vcpu->arch.shared->msr & MSR_SPE) - kvmppc_vcpu_enable_spe(vcpu); - else - kvmppc_booke_queue_irqprio(vcpu, - BOOKE_IRQPRIO_SPE_UNAVAIL); + /* + * The interrupt is shared, KVM support for the featured unit + * is detected at run-time. + */ + bool handled = false; + + if (kvmppc_supports_spe()) { +#ifdef CONFIG_SPE + if (cpu_has_feature(CPU_FTR_SPE)) + if (vcpu->arch.shared->msr & MSR_SPE) { + kvmppc_vcpu_enable_spe(vcpu); + handled = true; + } +#endif + if (!handled) + kvmppc_booke_queue_irqprio(vcpu, + BOOKE_IRQPRIO_SPE_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: - kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SPE_FP_DATA); - r = RESUME_GUEST; + /* + * The interrupt is shared, KVM support for the featured unit + * is detected at run-time. + */ + if (kvmppc_supports_spe()) { + kvmppc_booke_queue_irqprio(vcpu, + BOOKE_IRQPRIO_SPE_FP_DATA); + 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; case BOOKE_INTERRUPT_SPE_FP_ROUND: +#ifdef CONFIG_SPE kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SPE_FP_ROUND); r = RESUME_GUEST; break; #else - case BOOKE_INTERRUPT_SPE_UNAVAIL: /* - * Guest wants SPE, but host kernel doesn't support it. Send - * an "unimplemented operation" program check to the guest. + * These really should never happen without CONFIG_SPE, + * as we should never enable the real MSR[SPE] in the + * guest. */ - kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); - r = RESUME_GUEST; - 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: - 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;
SPE_FP interrupts are shared with ALTIVEC. Refactor SPE_FP exit handling to detect KVM support for the featured unit at run-time, in order to accommodate ALTIVEC later. Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com> --- arch/powerpc/kvm/booke.c | 80 ++++++++++++++++++++++++++++++++++------------ 1 files changed, 59 insertions(+), 21 deletions(-)