qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] Re: [PATCH 1/5] Add target memory mapping API


From: Anthony Liguori
Subject: Re: [Qemu-devel] Re: [PATCH 1/5] Add target memory mapping API
Date: Wed, 21 Jan 2009 13:36:41 -0600
User-agent: Thunderbird 2.0.0.19 (X11/20090105)

Ian Jackson wrote:
Anthony Liguori writes ("[Qemu-devel] Re: [PATCH 1/5] Add target memory mapping 
API"):
I've gone though the thread again and I think this patch series is a pretty good start. I think the API can be refined a bit more down the road to better support Xen but I think this is a good start.

I'm afraid I disagree.

That's why I sent a note first instead of just committing :-)

Does anyone have major blockers with this API? If not, I'd like to commit these patches. The consumers of it should be small provided that we make sure to write helpers for the various types of IO pipelines.

I have three criticisms, the first of which is in my view a major
blocker:

Okay, then let's ignore the last two for now. A DMA API is really important for performance and I'd like to get something in the tree we can start working with.

 * The unmap call should take a transfer length, for the reasons
   I have explained.

I have a hard time disagree that it should take a transfer length, if for nothing else, to use to update the right amount of the dirty bitmap. Avi, do you object to having unmap take a transfer length?

FWIW, I don't think we need to support RMW operations right now, but I think the transfer length is still required since you may get a partial IO result. It may not be an important issue of correctness but it's still an issue of correctness.


   I think it is important that the API permits implementations where
   the memory cannot be just mapped.  At the moment the APIs used by
   qemu device emulations do not assume that they can access RAM
   willy-nilly; they are all expected to go through
   cpu_physical_memory_rw.  I think it is important to preserve this
   for the future flexibility of the qemu codebase.

map() can, and will, fail under certain conditions and the code has to accommodate that.

   I'm trying to preserve an important and useful property of the
   internal qemu API.  My suggestion will mean the device emulation
   parts of qemu continue to be reasonably easily useable separately
   from the rest of qemu, and possibly very separately from any
   emulation of CPU or memory.  The benefit is difficult to evaluate
   exactly but the cost is very small.

Are you arguing for "callback" based mapping? The vast majority of devices will end up using a callback based approach so I'm not sure what you're unhappy about.

The second criticism is a matter of taste, perhaps, and the third can
be addressed later:

 * The mixed call/callback API: DMAs which can be mappped immediately
   are treated as synchronous calls; ones which can't involve a
   separate callback.

   This is no different semantically from a pure-callback API.
   However, it makes life more clumsy for the caller - the caller now
   needs two code paths: one for immediate success, and one for
   deferred allocation.

   Callbacks are increasingly dominating the innards of qemu for good
   reasons.  I think I have explained how a callback style can provide
   the necessary functionality.

I understand the point you make here and I think we'll end up having a callback API. But for reasons I've already mentioned, I don't think it makes sense for it to be the core API since it introduces very difficult to eliminate deadlocks.

 * The documentation, in the form of comments describing the
   semantics of and restrictions on the use of the DMA mapping,
   is inadequate.

It is.  Avi has promised to fix that :-)

Regards,

Anthony Liguori

Ian.







reply via email to

[Prev in Thread] Current Thread [Next in Thread]