Re: [PATCH 0/2] Introduce iommu_commit() function

["Roedel, Joerg" <Joerg.Roedel@amd.com>]

On Thu, Jun 23, 2011 at 01:20:53PM -0400, Chris Wright wrote:
> * David Woodhouse (dwmw2@infradead.org) wrote:
> > I'd much rather KVM just gave us a list of the pages to map, in a single
> > call.
> 
> This makes most sense to me.

But how is this supposed to work with arbitrary page-sizes? When we
define the interface as

	int iommu_map(struct iommu_domain *domain,
		      unsigned long iova,
		      struct page **pages,
		      int num_pages);

how does the IOMMU driver know which page-sizes are the best to use?
Sure, it can check if the pages in the list are physically contiguous,
but that is really bad design.
The information about the right page-sizes is already available in the
caller and the best is to pass this information down to the iommu
driver (which then maps the region with the best-fitting page-sizes it
supports). I would rather change the interface to something like

	int iommu_map(struct iommu_domain *domain,
		      unsigned long iova,
		      phys_addr_t phys_addr,
		      size_t len);

In this interface the caller specifies that the system physical region
starting at phys_addr, ending at phys_addr+len-1 should be mapped at
iova. The IOMMU driver already knows that this region is physically
contiguous and can apply the right page-sizes itself.

This interface is also much friendlier to the caller because there is no
need to build a list of pages. And yes, there will be more users of the
iommu-api than KVM in the future.

The page-table page cache-flushing problem on VT-d can be solved with
the iommu_commit() function like I described in an previous email. This
one can be used for a number of things necessary on other iommus too
(like flushing io/tlbs) so that this interface makes sense for all iommu
hardware supported by the iommu-api.

Regards,

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632