Re: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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).

> > +#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.

> > +	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.

-Scott

--
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


[Index of Archives]     [KVM Development]     [KVM ARM]     [KVM ia64]     [Linux Virtualization]     [Linux USB Devel]     [Linux Video]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux