Re: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state
On Mon, 4 Apr 2011 17:01:31 +0200
Alexander Graf ag...@suse.de 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)
+ cmpi0,r3,0
+ beqlr-
+ addir5,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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state
On 31.03.2011, at 05:21, Liu Yu-B13201 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Scott Wood Sent: Thursday, March 31, 2011 7:35 AM To: ag...@suse.de Cc: kvm-ppc@vger.kernel.org Subject: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state This is done lazily. The SPE save will be done only if the guest has used SPE since the last preemption or heavyweight exit. Restore will be done only on demand, when enabling MSR_SPE in the shadow MSR, in response to an SPE fault or mtmsr emulation. For SPEFSCR, Linux already switches it on context switch (non-lazily), so the only remaining bit is to save it between qemu and the guest. Signed-off-by: Liu Yu yu@freescale.com Signed-off-by: Scott Wood scottw...@freescale.com --- v5: disable preemption when restoring SPE state Saving SPE state is only done from a preempt notifier or vcpu_put(), where preemption is already disabled. The other patches in this series are the same as v4. arch/powerpc/include/asm/kvm_host.h |6 +++ arch/powerpc/include/asm/reg_booke.h |1 + arch/powerpc/kernel/asm-offsets.c|6 +++ arch/powerpc/kvm/booke.c | 72 +- arch/powerpc/kvm/booke.h | 18 ++--- arch/powerpc/kvm/booke_interrupts.S | 40 +++ arch/powerpc/kvm/e500.c | 19 - 7 files changed, 145 insertions(+), 17 deletions(-) I think the patch miss the bit to handle the case that if guest clear the MSR_SPE. Right. On set_msr the shadow_msr should get MSR_SPE removed. Thanks for the catch! Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state
On 31.03.2011, at 10:06, Liu Yu-B13201 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Thursday, March 31, 2011 3:51 PM To: Liu Yu-B13201 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org Subject: Re: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state On 31.03.2011, at 05:21, Liu Yu-B13201 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Scott Wood Sent: Thursday, March 31, 2011 7:35 AM To: ag...@suse.de Cc: kvm-ppc@vger.kernel.org Subject: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state This is done lazily. The SPE save will be done only if the guest has used SPE since the last preemption or heavyweight exit. Restore will be done only on demand, when enabling MSR_SPE in the shadow MSR, in response to an SPE fault or mtmsr emulation. For SPEFSCR, Linux already switches it on context switch (non-lazily), so the only remaining bit is to save it between qemu and the guest. Signed-off-by: Liu Yu yu@freescale.com Signed-off-by: Scott Wood scottw...@freescale.com --- v5: disable preemption when restoring SPE state Saving SPE state is only done from a preempt notifier or vcpu_put(), where preemption is already disabled. The other patches in this series are the same as v4. arch/powerpc/include/asm/kvm_host.h |6 +++ arch/powerpc/include/asm/reg_booke.h |1 + arch/powerpc/kernel/asm-offsets.c|6 +++ arch/powerpc/kvm/booke.c | 72 +- arch/powerpc/kvm/booke.h | 18 ++--- arch/powerpc/kvm/booke_interrupts.S | 40 +++ arch/powerpc/kvm/e500.c | 19 - 7 files changed, 145 insertions(+), 17 deletions(-) I think the patch miss the bit to handle the case that if guest clear the MSR_SPE. Right. On set_msr the shadow_msr should get MSR_SPE removed. Thanks for the catch! BTW, looks like guest trap on MSR[SPE] in para virt mode for now, is it? If we don't want to trap on that, then shadow_msr doesn't work here. That's the intend to use msr_block.. I see. I still like it better when we explicitly set the MSR value that's used while running the guest. Makes it harder for things to slip through. Also, so far paravirt is disabled for BookE. So what happens is that since MSR_SPE is disabled, we get a BOOKE_INTERRUPT_SPE_UNAVAIL. At that point, we need to check if it's enabled inside the guest already or if we merely need to activate the flag. That's what the patch does. We'd have to do the exact same when using msr_block, no? The only difference is that we'd set msr_block instead of shadow_msr when we are actually able to use the SPE inside the guest. Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state
On 31.03.2011, at 11:01, Liu Yu-B13201 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 31, 2011 4:43 PM To: Liu Yu-B13201 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org Subject: Re: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state On 31.03.2011, at 10:40, Liu Yu-B13201 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 31, 2011 4:15 PM To: Liu Yu-B13201 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org Subject: Re: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state On 31.03.2011, at 10:06, Liu Yu-B13201 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Thursday, March 31, 2011 3:51 PM To: Liu Yu-B13201 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org Subject: Re: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state On 31.03.2011, at 05:21, Liu Yu-B13201 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Scott Wood Sent: Thursday, March 31, 2011 7:35 AM To: ag...@suse.de Cc: kvm-ppc@vger.kernel.org Subject: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state This is done lazily. The SPE save will be done only if the guest has used SPE since the last preemption or heavyweight exit. Restore will be done only on demand, when enabling MSR_SPE in the shadow MSR, in response to an SPE fault or mtmsr emulation. For SPEFSCR, Linux already switches it on context switch (non-lazily), so the only remaining bit is to save it between qemu and the guest. Signed-off-by: Liu Yu yu@freescale.com Signed-off-by: Scott Wood scottw...@freescale.com --- v5: disable preemption when restoring SPE state Saving SPE state is only done from a preempt notifier or vcpu_put(), where preemption is already disabled. The other patches in this series are the same as v4. arch/powerpc/include/asm/kvm_host.h |6 +++ arch/powerpc/include/asm/reg_booke.h |1 + arch/powerpc/kernel/asm-offsets.c|6 +++ arch/powerpc/kvm/booke.c | 72 +- arch/powerpc/kvm/booke.h | 18 ++--- arch/powerpc/kvm/booke_interrupts.S | 40 +++ arch/powerpc/kvm/e500.c | 19 - 7 files changed, 145 insertions(+), 17 deletions(-) I think the patch miss the bit to handle the case that if guest clear the MSR_SPE. Right. On set_msr the shadow_msr should get MSR_SPE removed. Thanks for the catch! BTW, looks like guest trap on MSR[SPE] in para virt mode for now, is it? If we don't want to trap on that, then shadow_msr doesn't work here. That's the intend to use msr_block.. I see. I still like it better when we explicitly set the MSR value that's used while running the guest. Makes it harder for things to slip through. Also, so far paravirt is disabled for BookE. We already use paravirt in internal tree. So what happens is that since MSR_SPE is disabled, we get a BOOKE_INTERRUPT_SPE_UNAVAIL. At that point, we need to check if it's enabled inside the guest already or if we merely need to activate the flag. That's what the patch does. We'd have to do the exact same when using msr_block, no? The only difference is that we'd set msr_block instead of shadow_msr when we are actually able to use the SPE inside the guest. Hmm. I think you're right. I thought that in paravirt mode, if we don't trap on MSR[SPE], the shadow_msr cannot get notified timely (we don't have chance to call kvmppc_set_msr()). But looks like msr_block has the same issue. So that MSR[SPE] should always be traped. It doesn't have to be trapped - we can enable it lazily on SPE_UNAVAIL :). Of course we should also explicitly enable it on set_msr. It works on set MSR[SPE], but doesn't work on clear MSR[SPE]. Guest may also do lazy SPE, if guest clear MSR[SPE], but kvm don't know and still keep it. Then guest app may get SPE state clobbered. Ah, yes. On SPE clear we definitely have to trap :). Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state
On Thu, 31 Mar 2011 12:11:48 +0200 Alexander Graf ag...@suse.de wrote: On 31.03.2011, at 05:21, Liu Yu-B13201 wrote: I think the patch miss the bit to handle the case that if guest clear the MSR_SPE. Doh. So that MSR[SPE] should always be traped. It doesn't have to be trapped - we can enable it lazily on SPE_UNAVAIL :). Of course we should also explicitly enable it on set_msr. I don't see any benefit to doing this, if we're going to take a trap one way or another. Since we need to trap on clearing it anyway, might as well just keep things simple and have SPE always be a trapworthy MSR bit. Scott, to verify that I don't completely screw up the lazy FPU work back when I did it, I wrote some small test program that would just initialize an fpu register with a value and then constantly loop to verify if it's still the same. I ran that program with different values on the host and guest sides. That exposed quite a lot of glitches. It would be nice if you could provide something similar for the SPE patches, so we have some more faith in it :). OK. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Scott Wood Sent: Thursday, March 31, 2011 7:35 AM To: ag...@suse.de Cc: kvm-ppc@vger.kernel.org Subject: [PATCH v5 4/4] KVM: PPC: e500: Save/restore SPE state This is done lazily. The SPE save will be done only if the guest has used SPE since the last preemption or heavyweight exit. Restore will be done only on demand, when enabling MSR_SPE in the shadow MSR, in response to an SPE fault or mtmsr emulation. For SPEFSCR, Linux already switches it on context switch (non-lazily), so the only remaining bit is to save it between qemu and the guest. Signed-off-by: Liu Yu yu@freescale.com Signed-off-by: Scott Wood scottw...@freescale.com --- v5: disable preemption when restoring SPE state Saving SPE state is only done from a preempt notifier or vcpu_put(), where preemption is already disabled. The other patches in this series are the same as v4. arch/powerpc/include/asm/kvm_host.h |6 +++ arch/powerpc/include/asm/reg_booke.h |1 + arch/powerpc/kernel/asm-offsets.c|6 +++ arch/powerpc/kvm/booke.c | 72 +- arch/powerpc/kvm/booke.h | 18 ++--- arch/powerpc/kvm/booke_interrupts.S | 40 +++ arch/powerpc/kvm/e500.c | 19 - 7 files changed, 145 insertions(+), 17 deletions(-) I think the patch miss the bit to handle the case that if guest clear the MSR_SPE. Thanks, Yu -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
