Re: [kvm-devel] [PATCH 7/8] MMU: add TDP support to the KVM MMU

[Izik Eidus]

Joerg Roedel wrote:
> On Thu, Feb 07, 2008 at 03:27:19PM +0200, Izik Eidus wrote:
>   
> > Joerg Roedel wrote:
> >     
> > > This patch contains the changes to the KVM MMU necessary for support of the
> > > Nested Paging feature in AMD Barcelona and Phenom Processors.
> > >  
> > >       
> > good patch, it look like things will be very fixable with it
> >
> >     
> > > Signed-off-by: Joerg Roedel <joerg.roedel@xxxxxxx>
> > > ---
> > > arch/x86/kvm/mmu.c |   79 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> > > arch/x86/kvm/mmu.h |    6 ++++
> > > 2 files changed, 82 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > index 5e76963..5304d55 100644
> > > --- a/arch/x86/kvm/mmu.c
> > > +++ b/arch/x86/kvm/mmu.c
> > > @@ -1081,6 +1081,7 @@ static void mmu_alloc_roots(struct kvm_vcpu *vcpu)
> > > 	int i;
> > > 	gfn_t root_gfn;
> > > 	struct kvm_mmu_page *sp;
> > > +	int metaphysical = 0;
> > >  	root_gfn = vcpu->arch.cr3 >> PAGE_SHIFT;
> > > @@ -1089,14 +1090,20 @@ static void mmu_alloc_roots(struct kvm_vcpu *vcpu)
> > > 		hpa_t root = vcpu->arch.mmu.root_hpa;
> > >  		ASSERT(!VALID_PAGE(root));
> > > +		if (tdp_enabled)
> > > +			metaphysical = 1;
> > > 		sp = kvm_mmu_get_page(vcpu, root_gfn, 0,
> > > -				      PT64_ROOT_LEVEL, 0, ACC_ALL, NULL, NULL);
> > > +				      PT64_ROOT_LEVEL, metaphysical,
> > > +				      ACC_ALL, NULL, NULL);
> > > 		root = __pa(sp->spt);
> > > 		++sp->root_count;
> > > 		vcpu->arch.mmu.root_hpa = root;
> > > 		return;
> > > 	}
> > > #endif
> > > +	metaphysical = !is_paging(vcpu);
> > > +	if (tdp_enabled)
> > > +		metaphysical = 1;
> > > 	for (i = 0; i < 4; ++i) {
> > > 		hpa_t root = vcpu->arch.mmu.pae_root[i];
> > > @@ -1110,7 +1117,7 @@ static void mmu_alloc_roots(struct kvm_vcpu *vcpu)
> > > 		} else if (vcpu->arch.mmu.root_level == 0)
> > > 			root_gfn = 0;
> > > 		sp = kvm_mmu_get_page(vcpu, root_gfn, i << 30,
> > > -				      PT32_ROOT_LEVEL, !is_paging(vcpu),
> > > +				      PT32_ROOT_LEVEL, metaphysical,
> > > 				      ACC_ALL, NULL, NULL);
> > > 		root = __pa(sp->spt);
> > > 		++sp->root_count;
> > > @@ -1144,6 +1151,36 @@ static int nonpaging_page_fault(struct kvm_vcpu *vcpu, gva_t gva,
> > > 			     error_code & PFERR_WRITE_MASK, gfn);
> > > }
> > > +static int tdp_page_fault(struct kvm_vcpu *vcpu, gva_t gpa,
> > > +				u32 error_code)
> > >  
> > >       
> > you probably mean gpa_t ?
> >     
>
> Yes. But the function is assigned to a function pointer. And the type of
> that pointer expects gva_t there. So I named the parameter gpa to
> describe that a guest physical address is meant there.
>
>   
> > > +{
> > > +	struct page *page;
> > > +	int r;
> > > +
> > > +	ASSERT(vcpu);
> > > +	ASSERT(VALID_PAGE(vcpu->arch.mmu.root_hpa));
> > > +
> > > +	r = mmu_topup_memory_caches(vcpu);
> > > +	if (r)
> > > +		return r;
> > > +
> > > +	down_read(&current->mm->mmap_sem);
> > > +	page = gfn_to_page(vcpu->kvm, gpa >> PAGE_SHIFT);
> > > +	if (is_error_page(page)) {
> > > +		kvm_release_page_clean(page);
> > > +		up_read(&current->mm->mmap_sem);
> > > +		return 1;
> > > +	}
> > >  
> > >       
> > i dont know if it worth checking it here,
> > in the worth case we will map the error page and the host will be safe
> >     
>
> Looking at the nonpaging_map function it is the right place to check for
> the error page.
>   

thinking about it again you are right, (for some reason i was thinking 
about old kvm code that was replace already)
the is_error_page should be here.

> Joerg Roedel
>
>   


--
woof.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/