[Top][All Lists]
[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: |
Ian Jackson |
Subject: |
Re: [Qemu-devel] Re: [PATCH 1/5] Add target memory mapping API |
Date: |
Wed, 21 Jan 2009 17:09:27 +0000 |
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.
> 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:
* The unmap call should take a transfer length, for the reasons
I have explained.
This is absolutely critical for correctness if bounce buffers are
used.
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.
Note that Xen is not such an implementation, although in the Xen
case the additional performance of mapping is not so important so
we might not choose to implement the mapping right away. Obviously
it would be much easier for us just to implement this mapping than
to argue at length on this list. It's not hard for Xen. So I'm
not trying to special-case Xen here. You shouldn't read my
criticisms as some kind of Xen fanatic trying to defend some crazy
Xen behaviour.
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.
Of my criticisms, I think this is by far the most important. It
relates to the correctness of the API and in my view it is essential
that this be properly addressed.
Avi will say again (as if repetition made things true) that `bounce
buffers are never used' but of course they will be sometimes. If they
were never used his patch wouldn't need to have that code in them.
It's true that they will be _rarely_ used but a lack of correctness in
a rare case is not acceptable either.
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.
* The documentation, in the form of comments describing the
semantics of and restrictions on the use of the DMA mapping,
is inadequate.
Ian.
- Re: [Qemu-devel] [PATCH 1/5] Add target memory mapping API, (continued)
- Re: [Qemu-devel] [PATCH 1/5] Add target memory mapping API, Avi Kivity, 2009/01/19
- Re: [Qemu-devel] [PATCH 1/5] Add target memory mapping API, Gerd Hoffmann, 2009/01/19
- Re: [Qemu-devel] [PATCH 1/5] Add target memory mapping API, Avi Kivity, 2009/01/19
- Re: [Qemu-devel] [PATCH 1/5] Add target memory mapping API, Ian Jackson, 2009/01/19
- Re: [Qemu-devel] [PATCH 1/5] Add target memory mapping API, Avi Kivity, 2009/01/19
[Qemu-devel] Re: [PATCH 1/5] Add target memory mapping API, Anthony Liguori, 2009/01/19
[Qemu-devel] Re: [PATCH 1/5] Add target memory mapping API, Anthony Liguori, 2009/01/20
Re: [Qemu-devel] Re: [PATCH 1/5] Add target memory mapping API, Anthony Liguori, 2009/01/21
Re: [Qemu-devel] Re: [PATCH 1/5] Add target memory mapping API, Ian Jackson, 2009/01/22
Re: [Qemu-devel] Re: [PATCH 1/5] Add target memory mapping API, Anthony Liguori, 2009/01/22
Re: [Qemu-devel] Re: [PATCH 1/5] Add target memory mapping API, Ian Jackson, 2009/01/26
Re: [Qemu-devel] Re: [PATCH 1/5] Add target memory mapping API, Anthony Liguori, 2009/01/26
[Qemu-devel] Re: Add target memory mapping API, Mike Day, 2009/01/21