Re: [RFT][PATCH v1 6/6] vfio: Replace phys_pfn with phys_page for vfio_pin_pages()

[Nicolin Chen]

On Fri, Jun 17, 2022 at 01:54:05AM -0700, Christoph Hellwig wrote:
> There is a bunch of code an comments in the iommu type1 code that
> suggest we can pin memory that is not page backed.  

Would you mind explaining the use case for pinning memory that
isn't page backed? And do we have such use case so far?

> >  int vfio_pin_pages(struct vfio_device *device, dma_addr_t iova,
> > +		   int npage, int prot, struct page **phys_page)
> 
> I don't think phys_page makes much sense as an argument name.
> I'd just call this pages.

OK.

> > +			phys_page[i] = pfn_to_page(vpfn->pfn);
> 
> Please store the actual page pointer in the vfio_pfn structure.

OK.

> >  		remote_vaddr = dma->vaddr + (iova - dma->iova);
> > -		ret = vfio_pin_page_external(dma, remote_vaddr, &phys_pfn[i],
> > +		ret = vfio_pin_page_external(dma, remote_vaddr, &phys_pfn,
> >  					     do_accounting);
> 
> Please just return the actual page from vaddr_get_pfns through
> vfio_pin_pages_remote and vfio_pin_page_external, maybe even as a prep
> patch as that is a fair amount of churn.

I can do that. I tried once, but there were just too much changes
inside type1 code that felt like a chain reaction. If we plan to
eventually replace with IOMMUFD implementations, these changes in
type1 might not be necessary, I thought.