Re: [Xen-devel] [RFC] Xen PV IOMMU interface draft B

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.06.15 at 16:47, <malcolm.crossley@xxxxxxxxxx> wrote:
> On 16/06/15 14:19, Jan Beulich wrote:
>>>>> On 12.06.15 at 18:43, <malcolm.crossley@xxxxxxxxxx> 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

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