On 07/03/2013 01:42:12 PM, Alexander Graf wrote:
On 03.07.2013, at 20:28, Scott Wood wrote:
> On 07/03/2013 10:13:57 AM, Alexander Graf wrote:
>> 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.
So we should make this an #ifdef CONFIG_SPE rather than #ifndef
CONFIG_KVM_BOOKE_HV?
I think it should be "#if !defined(CONFIG_KVM_BOOKE_HV) &&
defined(CONFIG_SPE)" for the reasons I described in my second paragraph.
-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
