Re: [Xen-devel] [RFC PATCH 0/7] Implement Xen PV-IOMMU interface

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Wed, Feb 10, 2016 at 10:33 AM, Malcolm Crossley
<malcolm.crossley@xxxxxxxxxx> wrote:
> This RFC series implements the PV-IOMMU interface according to the PV-IOMMU
> design draft D.
>
> The PV-IOMMU interface is currently restricted to being used by the hardware
> domain only as non hardware domain callers have not been fully tested yet.
>
> Significant effort was put into implementing a m2b tracking structure without
> increasing the size of struct page but no union was found that could be safely
> used in all cases when a page is allocated to an HVM domain. Comments and
> feedback on the m2b design are most welcome.
>
> The hardware domain specific IOMMU pre map mechanism was implemented in order
> to keep performance parity with current out of tree mechanisms to obtain BFNs
> for foreign guest owned memory. The pre map mechanism is not a weakening of
> the current security model of Xen and is only allowed when the hardware domain
> is allowed relaxed IOMMU mappings.
>
>
> Malcolm Crossley (7):
>   common/pv-iommu: Add stub hypercall for PV-IOMMU
>   iommu: add iommu_lookup_page to lookup guest gfn for a particular
>     IOMMU mapping
>   VT-d: Add iommu_lookup_page support
>   common/pv-iommu: Add query, map and unmap ops
>   x86/m2b: Add a tracking structure for mfn to bfn mappings per page
>   common/pv-iommu: Add foreign ops to PV-IOMMU interface
>   common/pv-iommu: Allow hardware_domain to pre IOMMU map foreign
>     memory
>
>  xen/arch/x86/domain.c               |  12 +-
>  xen/arch/x86/mm/Makefile            |   1 +
>  xen/arch/x86/mm/m2b.c               | 211 ++++++++++++

My first question from the RFC perspective is whether mm/ is the right
place for this file to go.  It doesn't seem to have any interaction
with any of the other p2m functionality, nor does it seem to resemble
the functionality much (using linked lists from the page struct rather
than the pagetable structure).  I don't mind it going under mm/ per se
but it seems like it might fit logically better under
xen/drivers/passthrough.

Definitely agree that the design doc needs to be checked into the tree
as a part of this series.

The other minor question is why you have the patches broken down quite
the way you do -- what's the point of adding a stub hypercall that
doesn't do anything, and adding a callback framework that doesn't have
any users?  Why not just squash patches 1 and 4 together, and patches
2 and 3 together?

The design doc is well-written and thorough, thanks. :-)

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel