On Wed, 30 Mar 2011 10:17:55 +0200
Alexander Graf <agraf@xxxxxxx> wrote:
> On 30.03.2011, at 01:43, Scott Wood wrote:
>
> > + case BOOKE_INTERRUPT_SPE_UNAVAIL: {
> > + extern void kvmppc_vcpu_spe_load(struct kvm_vcpu *vcpu);
> > +
> > + /* reload the SPE env if guest first use SPE since last save */
> > + if (vcpu->arch.msr_block & MSR_SPE)
> > + kvmppc_vcpu_spe_load(vcpu);
> > +
> > + if (!(vcpu->arch.shared->msr & MSR_SPE))
> > + kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SPE_UNAVAIL);
>
> These are in the wrong order, no? If the guest doesn't do SPE, we can
> save ourselves the loading.
The most likely thing that's going to happen is that the guest turns on
SPE shortly after it receives the SPE unavailable exception.
We could skip it here if guest MSR[SPE] is not set, but then we have to
do it in kvmppc_set_msr() once guest MSR[SPE] gets set. Otherwise we take an
additional trap in the normal case.
> I was wondering why we need to take the guest's MSR value into account
> in the first place though. Are there any bits in there that we
> wouldn't catch through a lazy trap? At the end of the day, the MSR
> will be shared between the guest and the host without traps, so we will
> depend on trapping for events anyways. Why can't we just keep a "guest
> mode MSR" field in the vcpu that contains the bits activated inside the
> guest? This way no bit just accidently slips through and we save a
> good bunch of instructions in the lightweight exit path.
So update things like the real guest-mode MSR[SPE] in kvmppc_set_msr()?
In that case, doing the state transfer there would be cleaner.
> > +#ifdef CONFIG_SPE
> > +#define KVMPPC_SAVE_EVR(n,s,base) evmergehi s,s,n; stw s,(4*(n))(base)
>
> Can't we somehow combine those with the normal Linux SAVE_EVR macros?
>
> All we need to do is define a new set of macros that take a generic
> offset and make the normal macros use that with offset=THREAD_EVR0
> while we use offset=0. This means we could probably even specify the
> offset directly inside the kvm code. And maybe even the offset in the
> code that currently uses SAVE_EVR.
Yeah, we could. I wasn't sure whether it was worth hacking up the
existing macros to break the dependency on THREAD_EVR0, but it shouldn't
be too bad.
-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
