Am 04.04.2011 um 18:07 schrieb Scott Wood <[email protected]>:
> On Mon, 4 Apr 2011 17:01:31 +0200 > Alexander Graf <[email protected]> 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
