On 07/03/2013 10:13:57 AM, Alexander Graf wrote:
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 :).
kvmppc_vcpu_enable_spe isn't defined unless CONFIG_SPE is defined.
More seriously, MSR_SPE is the same as MSR_VEC, so we shouldn't
interpret it as SPE unless CONFIG_SPE is defined. And we can't rely on
the "if (kvmppc_supports_spe())" here because a later patch changes it
to "if (kvmppc_supports_altivec() || kvmppc_supports_spe())". So I
think we still need the ifdef CONFIG_SPE here.
As for the HV ifndef, we should try not to confuse HV/PR with
e500mc/e500v2, even if we happen to only run HV on e500mc and PR on
e500v2. We would not want to call kvmppc_vcpu_enable_spe() here on a
hypothetical HV target with SPE. And we *would* want to call
kvmppc_vcpu_enable_fp() here on a hypothetical PR target with normal
FP. It's one thing to leave out the latter, since it would involve
writing actual code that we have no way to test at this point, but
quite another to leave out the proper conditions for when we want to
run code that we do have.
-Scott
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
