lkml.org 
[lkml]   [2010]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/3] KVM: MMU: don not retry #PF for nonpaging guest
    On Tue, Nov 09, 2010 at 04:48:40PM +0800, Xiao Guangrong wrote:
    > On 11/09/2010 04:03 PM, Gleb Natapov wrote:
    > > On Fri, Nov 05, 2010 at 01:39:18PM +0800, Xiao Guangrong wrote:
    > >> On 11/04/2010 06:35 PM, Gleb Natapov wrote:
    > >>> On Thu, Nov 04, 2010 at 06:32:42PM +0800, Xiao Guangrong wrote:
    > >>>> nonpaing guest's 'direct_map' is also true, retry #PF for those
    > >>>> guests is useless, so use 'tdp_enabled' instead
    > >>>>
    > >>> nonpaging guest will not attempt async pf.
    > >>
    > >> Ah, my mistake, but why we can not attempt async pf for nonpaging guest?
    > >>
    > >>> And by checking tdp_enabled
    > >>> here instead of direct_map we will screw nested ntp.
    > >>>
    > >>
    > >> It looks like something broken: apfs can generated in L2 guest (nested ntp guest)
    > >> and be retried in L1 guest.
    > >>
    > > OK now when Xiao explained me the problem privately I can review this.
    > >
    > >> Below patch fix it and let nonpaging guest support async pf. I'll post it properly
    > >> if you like. :-)
    > >>
    > > Apf for nonpaging guest should be separate patch of course.
    >
    > Sure, i'll separate it when i post.
    >
    > >
    > >> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
    > >> index 7f20f2c..606978e 100644
    > >> --- a/arch/x86/include/asm/kvm_host.h
    > >> +++ b/arch/x86/include/asm/kvm_host.h
    > >> @@ -600,6 +600,7 @@ struct kvm_x86_ops {
    > >> struct kvm_arch_async_pf {
    > >> u32 token;
    > >> gfn_t gfn;
    > >> + bool softmmu;
    > >> };
    > >>
    > >> extern struct kvm_x86_ops *kvm_x86_ops;
    > >> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
    > >> index f3fad4f..48ca312 100644
    > >> --- a/arch/x86/kvm/mmu.c
    > >> +++ b/arch/x86/kvm/mmu.c
    > >> static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn)
    > >> @@ -2602,6 +2607,7 @@ static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn)
    > >> struct kvm_arch_async_pf arch;
    > >> arch.token = (vcpu->arch.apf.id++ << 12) | vcpu->vcpu_id;
    > >> arch.gfn = gfn;
    > >> + arch.softmmu = mmu_is_softmmu(vcpu);
    > >>
    > > We can do:
    > > if (mmu_is_nested(vcpu))
    > > gva = vcpu->mmu.gva_to_gpa(gva);
    > > And this should fix everything no?
    > >
    >
    > No, since it can't help us to avoid NPF when nested guest run again.
    >
    Of course it will not prevent NPF if L2 guest touches it again, but from
    correctness point of view it is OK. So if L1 will re-use the page for
    L1 process the page will be already mapped. Not a huge gain I agree, but
    fix is much more simple.

    > More detailed:
    >
    > +--------------+ nested guest VA
    > L2 | nested guest |
    > +--------------+ nested guest PA + +
    > | | |
    > +--------------+ guest VA | |
    > L1 | guest | | |
    > +--------------+ guest PA + | V
    > | | |
    > +--------------+ host VA | |
    > L0 | host | | |
    > +--------------+ host PA V V
    > PT10 PT20 PT21
    >
    > If do it like this way, we will prefault 'gva' in PT10, but L2 guest's
    > running not depends on it(PT10), the same NPF is avoid only we prefault
    > it in PT20.
    >
    > >> return kvm_setup_async_pf(vcpu, gva, gfn, &arch);
    > >> }
    > >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
    > >> index 2044302..d826d78 100644
    > >> --- a/arch/x86/kvm/x86.c
    > >> +++ b/arch/x86/kvm/x86.c
    > >> @@ -6172,9 +6172,10 @@ EXPORT_SYMBOL_GPL(kvm_set_rflags);
    > >>
    > >> void kvm_arch_async_page_ready(struct kvm_vcpu *vcpu, struct kvm_async_pf *work)
    > >> {
    > >> + bool softmmu = mmu_is_softmmu(vcpu);
    > >> int r;
    > >>
    > >> - if (!vcpu->arch.mmu.direct_map || is_error_page(work->page))
    > >> + if (softmmu || work->arch.softmmu || is_error_page(work->page))
    > >> return;
    > >>
    > >> r = kvm_mmu_reload(vcpu);
    > >> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
    > >> index 2cea414..48796c7 100644
    > >> --- a/arch/x86/kvm/x86.h
    > >> +++ b/arch/x86/kvm/x86.h
    > >> @@ -55,6 +55,11 @@ static inline bool mmu_is_nested(struct kvm_vcpu *vcpu)
    > >> return vcpu->arch.walk_mmu == &vcpu->arch.nested_mmu;
    > >> }
    > >>
    > >> +static inline bool mmu_is_softmmu(struct kvm_vcpu *vcpu)
    > >> +{
    > >> + return !tdp_enabled || mmu_is_nested(vcpu);
    > >> +}
    > >> +
    > > Isn't this the same as checking vcpu->arch.mmu.direct_map? The only
    > > difference that this will be true for nonpaging mode too but this is
    > > even better since we can prefault in nonpaging mode.
    > >
    >
    > Um, maybe prefault in nonpaging mode is unreliable,
    >
    > For shadow page: it doesn't know the CR0.PG is changed, for example, when apf is
    > generated CR0.PG = 1 and when apf is completed CR0.PG = 0, it can prefault in
    > different mmu context.
    We can (and may be should) call kvm_clear_async_pf_completion_queue() on
    CR0.PG 1->0 transaction.

    >
    > For nested paging: if nested guest vcpu is nonpaging. it will prefault PT10's NPF
    > in PT20, it means apf is generated in L1 guest and prefault in L2 guest.
    I think for that L1 should be in nonpaging mode and it is impossible to
    run nested guest while vcpu is nonpaging.

    --
    Gleb.


    \
     
     \ /
      Last update: 2010-11-09 10:29    [W:0.034 / U:269.688 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site