Re: [Xen-devel] [PATCH v3 3/3] xen: introduce XENMEM_get_dma_buf and XENMEM_put_dma_buf

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-08-08 at 15:16 +0100, Stefano Stabellini wrote:
> On Thu, 8 Aug 2013, Keir Fraser wrote:
> > On 08/08/2013 11:40, "Ian Campbell" <Ian.Campbell@xxxxxxxxxx> wrote:
> > 
> > >>  it exchanges the pages
> > >> + * passed in with a new set of pages, contiguous and under 4G if so
> > >> + * requested.
> > > 
> > > I guess it avoids exchanging the pages if they already meet this
> > > constraint?
> > > 
> > > The "if so requested" binds to "under 4G" and not "contiguous"? The
> > > result is always contiguous I think. Also 4G is just an example and the
> > > caller can ask for any order it likes.
> > > 
> > > The contents of the pages will be lost I think. Perhaps in debug builds
> > > we should consider actually nuking the page's content in the case where
> > > we don't actually exchange?
> > 
> > This confused me too, this talk of contiguity and being below 4GB. Doesn't
> > this new hypercall behave *exactly* like XENMEM_exchange, except for the
> > additional pinning of the MFNs? If so, just say that in the patch header and
> > code comments. The fact *you* currently use it to get memory below 4GB is an
> > application detail.
> 
> Yeah, you have a good point there. Trying to make the comment clearer I
> actually confused it. I'll fix it.
> 
> 
> > I wonder whether XENMEM_exchange is much use at all in the presence of
> > paging, without the additional semantics of pinning? Perhaps XENMEM_exchange
> > could be suitably adjusted to have the pinning semantics in this case?
> 
> It's not just about pinning, we also need to get the mfn back even if
> the guest is autotranslate, that XENMEM_exchange doesn't do.

Plus current users won't be expecting to have to unpin when they are
done, so just adding pinning to the existing function won't work.

the struct doesn't have a flags field so we can't do it that way either.

I suppose we could deprecate the existing exchange in favour of one with
a flags field, but that doesn't seem any better than renaming this new
addition XENMEM_exchange_and_pin.

Ian.


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