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
