Re: [Xen-devel] [PATCH v2 12/13] x86: add iommu_ops to modify and flush IOMMU mappings

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: George Dunlap [mailto:george.dunlap@xxxxxxxxxx]
> Sent: 11 July 2018 12:46
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Jan Beulich <jbeulich@xxxxxxxx>; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Julien
> Grall <julien.grall@xxxxxxx>; Konrad Rzeszutek Wilk
> <konrad.wilk@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Tim
> (Xen.org) <tim@xxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>
> Subject: Re: [PATCH v2 12/13] x86: add iommu_ops to modify and flush
> IOMMU mappings
> 
> On 07/07/2018 12:05 PM, Paul Durrant wrote:
> > This patch adds iommu_ops to add (map) or remove (unmap) frames in the
> > domain's IOMMU mappings, and an iommu_op to synchronize (flush)
> those
> > manipulations with the hardware.
> >
> > Mappings added by the map operation are tracked and only those
> mappings
> > may be removed by a subsequent unmap operation. Frames are specified
> by the
> > owning domain and GFN. It is, of course, permissable for a domain to map
> > its own frames using DOMID_SELF.
> >
> > NOTE: The owning domain and GFN must also be specified in the unmap
> >       operation, as well as the BFN, so that they can be cross-checked
> >       with the existent mapping.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> 
> The code on the whole looks correct, but I don't see any reference
> counting.  The call to get_paged_gfn() in iommuop_unmap() kind of
> underlines the issue -- what's to stop the following sequence of events?
> 
> * iommuop_map() page A
> * share(A)
> * DMA write into A #
>

I don't follow. In iommuop_map() we do a get_paged_gfn() and that reference is 
retained. In iommuop_unmap() there is another call to get_paged_gfn() but that 
is there to check that the specified gfn matches the mfn that's looked up in 
the IOMMU. Only if they match is the original page ref dropped.

  Paul

 
>  -George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel