Am 04.04.2011 um 18:07 schrieb Scott Wood <scottwood@xxxxxxxxxxxxx>:
> On Mon, 4 Apr 2011 17:01:31 +0200
> Alexander Graf <agraf@xxxxxxx> wrote:
>
>> On 04/01/2011 09:17 PM, Scott Wood wrote:
>>> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
>>> index 5120a63..4d39f2d 100644
>>> --- a/arch/powerpc/kernel/asm-offsets.c
>>> +++ b/arch/powerpc/kernel/asm-offsets.c
>>> @@ -494,6 +494,12 @@ int main(void)
>>> DEFINE(TLBCAM_MAS3, offsetof(struct tlbcam, MAS3));
>>> DEFINE(TLBCAM_MAS7, offsetof(struct tlbcam, MAS7));
>>> #endif
>>> +#ifdef CONFIG_SPE
>>
>> if defined(CONFIG_KVM) && defined(CONFIG_SPE)
>
> Right.
>
>>> +static void kvmppc_vcpu_sync_spe(struct kvm_vcpu *vcpu)
>>> +{
>>> + if (vcpu->arch.shared->msr& MSR_SPE) {
>>> + if (!(vcpu->arch.shadow_msr& MSR_SPE))
>>> + kvmppc_vcpu_enable_spe(vcpu);
>>> + } else if (vcpu->arch.shadow_msr& MSR_SPE) {
>>> + kvmppc_vcpu_disable_spe(vcpu);
>>
>> So what if shared->msr & MSR_SPE && shadow_msr & MSR_SPE? Do you disable
>> it then?
>
> No.
>
> The only times it should get disabled are if the guest clears its MSR_SPE,
> or from kvmppc_core_vcpu_put().
>
> If you're saying you think this code does so by accident, I don't see it --
> the disable call is in an else branch that ensures !(shared->msr & MSR_SPE).
Ah, I must have missen the branch levels :). You're right.
>
>>> +#ifdef CONFIG_SPE
>>> +_GLOBAL(kvmppc_save_guest_spe)
>>> + cmpi 0,r3,0
>>> + beqlr-
>>> + addi r5,r3,VCPU_EVR
>>> + SAVE_32EVRS(0, r4, r5, 0)
>
> Hmm, I missed VCPU_EVR here, it can be directly passed to SAVE_32EVRS now.
Yup :)
>
>>> + evxor evr6, evr6, evr6
>>> + evmwumiaa evr6, evr6, evr6
>>> + li r4,VCPU_ACC
>>> + evstddx evr6, r4, r3 /* save acc */
>>
>> I'm not sure I fully understand SPE instructions yet, but isn't evr6
>> just r6 plus its upper 32 bits?
>
> Yes.
>
>> Then wouldn't it make sense to work in evr3/evr4 and only copy the
>> upper 32 bits over to another register? Not that it should matter -
>> I'm only being curious here :)
>
> We need to save the whole thing -- evr6 holds the accumulator at that
> point (produced by the preceding evxor+evwmumiaa), not a regular EVR
> whose lower half has already been saved. evstddx is the easiest way to
> do that.
Okies :)
Alex
>
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
