On Tue, 14 Jun 2011 11:55:11 +0200
Alexander Graf <agraf@xxxxxxx> wrote:
>
> On 03.06.2011, at 01:17, Scott Wood wrote:
>
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > index e1d368c..45ad454 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -472,6 +472,17 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> > gpa_t gpaddr;
> > gfn_t gfn;
> >
> > +#ifdef CONFIG_KVM_E500
> > + if (!(vcpu->arch.shared->msr & MSR_PR) &&
> > + (eaddr & PAGE_MASK) == vcpu->arch.magic_page_ea) {
> > + kvmppc_map_magic(vcpu);
> > + kvmppc_account_exit(vcpu, DTLB_VIRT_MISS_EXITS);
>
> This is fine for now, but please note that I find it unpretty. The magic page hook should be in the guest tlb lookup and then reuse the same mapping mechanisms as real TLB entries, as it's basically the same.
>
> However, for now just leave it like this.
I'm not thrilled with it either, but it's different in that there is no
entry for this in the guest TLB. If there were to be one, that would be a
guest-visible change in the magic page interface.
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index 616dd51..d5beb72 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -66,16 +66,17 @@ int kvmppc_kvm_pv(struct kvm_vcpu *vcpu)
> > vcpu->arch.magic_page_pa = param1;
> > vcpu->arch.magic_page_ea = param2;
> >
> > - r2 = KVM_MAGIC_FEAT_SR;
> > +#ifdef CONFIG_KVM_E500
> > + kvmppc_map_magic(vcpu);
> > +#endif
>
> Is this necessary? Sure, we save a few ms on the trap, but is it worth it? If we had a generic function that could map the magic page on all systems, sure, but I'd like to keep generic code clean of too many #ifdefs.
I think that's a relic from when I was trying to avoid adding code to the
TLB miss path to load this on demand. It should be able to come out.
-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
