Re: [PATCH 7/7] x86: add iommu_ops to map and unmap pages, and also to flush the IOTLB

From: Paul Durrant <Paul.Durrant@citrix.com>
To: 'Jan Beulich' <JBeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Andrew Cooper <Andrew.Cooper3@citrix.com>,
	"Tim (Xen.org)" <tim@xen.org>,
	George Dunlap <George.Dunlap@citrix.com>,
	Ian Jackson <Ian.Jackson@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 7/7] x86: add iommu_ops to map and unmap pages, and also to flush the IOTLB
Date: Tue, 20 Mar 2018 09:32:24 +0000	[thread overview]
Message-ID: <66ae597256be4b0f86d12d5ec9abe485@AMSPEX02CL03.citrite.net> (raw)
In-Reply-To: <5AB0D05F02000078001B3C49@prv-mh.provo.novell.com>

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 20 March 2018 08:12
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Wei Liu
> <wei.liu2@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen-
> devel@lists.xenproject.org; Tim (Xen.org) <tim@xen.org>
> Subject: RE: [Xen-devel] [PATCH 7/7] x86: add iommu_ops to map and
> unmap pages, and also to flush the IOTLB
> 
> >>> On 19.03.18 at 17:57, <Paul.Durrant@citrix.com> wrote:
> >>  -----Original Message-----
> > [snip]
> >> >> How are you making sure this is a mapping that was established via
> >> >> the map op? Without that this can be (ab)used to ...
> >> >>
> >> >> > +    put_page(page);
> >> >>
> >> >> ... underflow the refcount of a page.
> >> >>
> >> >
> >> > Yes, I guess I need to ensure that only non-RAM (i.e. RMRR and E820
> >> reserved
> >> > areas) are mapped through the IOMMU or this could indeed be abused.
> >>
> >> Now I'm confused - then you don't need to deal with struct page_info
> >> and page references at all. Nor would you need to call
> >> get_page_from_gfn() and check p2m_is_any_ram(). Also - what use
> >> would the interface be if you couldn't map any RAM?
> >>
> >
> > Sorry to confuse...
> >
> > What I meant was that safety (against underflow) is predicated on
> > iommu_lookup_page() failing if the mapping was not established through
> an
> > iommu op hypercall. So, the only things that should be valid in the iommu
> > (and hence that iommu_lookup_page() would succeed for) at the point
> where the
> > guest starts to boot must all fall within reserved regions, so thay they are
> > ruled out by the earlier check.
> 
> Ah, I see. What I don't see is how you want to arrange for that.
> The tool stack wouldn't know ahead of time whether the guest
> wants to use the PV IOMMU interfaces, would it? IOW rather than
> guaranteeing said state at start of guest, shouldn't you blow away
> all non-special mappings the first time a PV IOMMU request is made?
> 

I suspect we want both. Kevin suggested a 'big switch' when the domain boots, in which I could blow away all non-reserved mappings. But, for performance sake, I think it would also be worth a Xen command line option to avoid populating the IOMMU mappings for dom0 in the first place (so when it pulls the 'big switch' it's a no-op). Non-aware dom0s will, of course, probably fail to boot but whoever is setting the command line for Xen should know what their dom0 is capable of. As for other domains, it may be worth adding a similar domain create option to the toolstack but that could be done at a later date.

  Paul

> Jan

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