[PATCH 4/6] KVM: PPC: Book3E: Add AltiVec support

[Alexander Graf]

On 03.07.2013, at 18:49, Caraman Mihai Claudiu-B02008 wrote:

>>>>> +
>>>>> 	if (!vcpu->arch.sane) {
>>>>> 		kvm_run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
>>>>> 		return -EINVAL;
>>>>> @@ -716,6 +750,22 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run,
>>>> struct kvm_vcpu *vcpu)
>>>>> 	kvmppc_load_guest_fp(vcpu);
>>>>> #endif
>>>>> 
>>>>> +#ifdef CONFIG_ALTIVEC
>>>> 
>>>> /* Switch from user space Altivec to guest Altivec state */
>>>> 
>>>>> +	if (cpu_has_feature(CPU_FTR_ALTIVEC)) {
>>>> 
>>>> Why not use your kvmppc_supports_altivec() helper here?
>>> 
>>> Give it a try ... because Linus guarded this members with
>> CONFIG_ALTIVEC :)
>> 
>> Huh? You already are in an #ifdef CONFIG_ALTIVEC here. I think it's a
>> good idea to be consistent in helper usage. And the name you gave to the
>> helper (kvmppc_supports_altivec) is actually quite nice and tells us
>> exactly what we're asking for.
> 
> I thought you asking to replace #ifdef CONFIG_ALTIVEC.
> 
>>>> Do we need to do this even when the guest doesn't use Altivec? Can't
>> we
>>>> just load it on demand then once we fault? This code path really
>> should
>>>> only be a prefetch enable when MSR_VEC is already set in guest
>> context.
>>> 
>>> No we can't, read 6/6.
>> 
>> So we have to make sure we're completely unlazy when it comes to a KVM
>> guest. Are we?
> 
> Yes, because MSR[SPV] is under its control.

Oh, sure, KVM wants it unlazy. That part is obvious. But does the kernel always give us unlazyness? The way I read the code, process.c goes lazy when !CONFIG_SMP.

So the big question is why we're manually enforcing FPU giveup, but not Altivec giveup? One of the 2 probably is wrong :).


Alex