Re: [PATCH 06/11] kvm: Change kvm_iommu_map_pages to map large pages

[Joerg Roedel <joerg.roedel@amd.com>]

Hi Marcelo, Avi,

On Mon, Feb 01, 2010 at 05:30:17PM -0200, Marcelo Tosatti wrote:
> On Mon, Feb 01, 2010 at 03:18:04PM +0100, Joerg Roedel wrote:
> > On Fri, Jan 29, 2010 at 10:32:33AM +0100, Joerg Roedel wrote:
> > > On Thu, Jan 28, 2010 at 08:24:55PM -0200, Marcelo Tosatti wrote:
> > > > On Thu, Jan 28, 2010 at 12:37:57PM +0100, Joerg Roedel wrote:
> > > > > +static pfn_t kvm_pin_pages(struct kvm *kvm, struct kvm_memory_slot *slot,
> > > > > +			   gfn_t gfn, unsigned long size)
> > > > > +{
> > > > > +	gfn_t end_gfn;
> > > > > +	pfn_t pfn;
> > > > > +
> > > > > +	pfn     = gfn_to_pfn_memslot(kvm, slot, gfn);
> > > > 
> > > > If gfn_to_pfn_memslot returns pfn of bad_page, you might create a
> > > > large iommu translation for it?
> > > 
> > > Right. But that was broken even before this patch. Anyway, I will fix
> > > it.
> > > 
> > > > > +		/* Map into IO address space */
> > > > > +		r = iommu_map(domain, gfn_to_gpa(gfn), pfn_to_hpa(pfn),
> > > > > +			      get_order(page_size), flags);
> > > > > +
> > > > > +		gfn += page_size >> PAGE_SHIFT;
> > > > 
> > > > Should increase gfn after checking for failure, otherwise wrong
> > > > npages is passed to kvm_iommu_put_pages.
> > > 
> > > True. Will fix that too.
> > 
> > Here is the updated patch (also updated in the iommu/largepage branch of
> > my tree). Does it look ok?
> 
> Yes, addresses the concern.

Are there any further objections against this patchset? If not it would
be cool if you could give me some acks for the kvm specific parts of
this patchset.

	Joerg