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