From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [RFC] Xen PV IOMMU interface draft B Date: Tue, 16 Jun 2015 16:56:30 +0100 Message-ID: <5580634F0200007800085AAD@mail.emea.novell.com> References: <557B0C35.4080907@citrix.com> <55803E8E020000780008583F@mail.emea.novell.com> <558036E9.9000004@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z4tE3-0004nj-5E for xen-devel@lists.xenproject.org; Tue, 16 Jun 2015 15:56:35 +0000 In-Reply-To: <558036E9.9000004@citrix.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Malcolm Crossley Cc: Kevin Tian , Yu C Zhang , AndrewCooper , Paul Durrant , David Vrabel , xen-devel , Zhiyuan Lv List-Id: xen-devel@lists.xenproject.org >>> On 16.06.15 at 16:47, wrote: > On 16/06/15 14:19, Jan Beulich wrote: >>>>> On 12.06.15 at 18:43, wrote: >>> IOMMU_QUERY_map_all_gfns 1 IOMMUOP_map_page subop can map any MFN >>> not used by Xen >> >> "gfns" or "MFN"? > > gfns . This is meant to apply to the hardware domain only, it's to allow the > same access control as dom0-relaxed mode allows currently. But why "gfns" in the name and "any MFN" in the description? >>> IOMMUOP_unmap_page >>> ------------------ >>> This subop uses `struct unmap_page` part of the `struct pv_iommu_op`. >>> >>> The subop usage of the "struct pv_iommu_op" and ``struct unmap_page` fields >>> are detailed below: >>> >>> -------------------------------------------------------------------- >>> Field Purpose >>> ----- ----------------------------------------------------- >>> `bfn` [in] Bus address frame number to be unmapped in DOMID_SELF >> >> Has it been determined that unmapping based on GFN is never >> going to be needed, and that unmapping by BFN is the more >> practical solution? The map counterpart doesn't seem to exclude >> establishing multiple mappings for the same BFN, and hence the >> inverse here would become kind of fuzzy in that case. > > There will be only one BFN to MFN mapping per domain, the map hypercall will > fail any attempt to have map a BFN to more than one GFN. This is why the > unmap > is based on the BFN. It is allowed to have multiple BFN mappings of the same > GFN however. Okay, I got confused again by the term BFN - I keep mixing up the parts of the bus between device and IOMMU vs between IOMMU and RAM. Alternatives I could think of (DFN for Device Frame Number) wouldn't be any better, so I guess we need to live with the ambiguity. >>> Each successful subop will add to the M2B if there was not an existing >>> identical >>> M2B entry. >>> >>> Every new M2B entry will take a reference to the MFN backing the GFN. >> >> This is a lookup - why would it add something somewhere? Perhaps >> this is just a copy-and-paste mistake? Or is the use of "lookup" here >> misleading (as the last section of the document seems to suggest)? > > I can see how the term "lookup" is misleading. This subop really does a > lookup and take reference. Can you suggest an alternative name? get_foreign_page_map? In any event, in particular with possibly ambiguous name the description should be particularly clear and obvious to help the reader (which also applies to the code to be written). > I'm wary of allowing ??? >>> PV IOMMU interactions with self ballooning >>> ========================================== >>> >>> The guest should clear any IOMMU mappings it has of it's own pages before >>> releasing a page back to Xen. It will need to add IOMMU mappings after >>> repopulating a page with the populate_physmap hypercall. >>> >>> This requires that IOMMU mappings get a writeable page type reference count >>> and >>> that guests clear any IOMMU mappings before pinning page table pages. >> >> I suppose this is only for aware PV guests. If so, perhaps this should >> be made explicit. > > This is only for PV guests. I will make the correction. The emphasis was on "aware", not on "PV" (which is already stated). >>> The grant map operation would then behave similarly to the IOMMUOP_map_page >>> subop for the creation of the IOMMU mapping. >>> >>> The grant unmap operation would then behave similarly to the >>> IOMMUOP_unmap_page >>> subop for the removal of the IOMMU mapping. >> >> We're talking about mappings of foreign pages here - aren't these the >> wrong IOMMUOPs then? And if so, where would the ioserver id come >> from? >> > > I don't expected grant mapped pages to be ballooned out or to be directly > used by > ioservers so I believe the grant mapped pages match more closely to the > standard > map_page than the foreign_map_page. Right, that became clear with you saying that the ioserver id is meant to be used for balloon out notifications only. But that should be made explicit. > Generally I think I need to rework the document to introduce the some > concepts > before the actual interface itself. Yes, that would be very helpful. The interface spec should probably the (almost) last thing. Jan