On Thu, Nov 29, 2012 at 02:33:11PM +0200, Michael S. Tsirkin wrote: > On Thu, Nov 22, 2012 at 04:16:44PM +0100, Stefan Hajnoczi wrote: > > The data plane thread needs to map guest physical addresses to host > > pointers. Normally this is done with cpu_physical_memory_map() but the > > function assumes the global mutex is held. The data plane thread does > > not touch the global mutex and therefore needs a thread-safe memory > > mapping mechanism. > > > > Hostmem registers a MemoryListener similar to how vhost collects and > > pushes memory region information into the kernel. There is a > > fine-grained lock on the regions list which is held during lookup and > > when installing a new regions list. > > > > When the physical memory map changes the MemoryListener callbacks are > > invoked. They build up a new list of memory regions which is finally > > installed when the list has been completed. > > > > Note that this approach is not safe across memory hotplug because mapped > > pointers may still be in used across memory unplug. However, this is > > currently a problem for QEMU in general and needs to be addressed in the > > future. > > Sounds like a serious problem. > I'm not sure I understand - do you say this currently a problem for QEMU > virtio? Coul you give an example please?
This is a limitation of the memory API but cannot be triggered by users today since we don't support memory hot unplug. I'm just explaining that virtio-blk-data-plane has the same issue as hw/virtio-blk.c or any other device emulation code here. Some more detail: The issue is that hw/virtio-blk.c submits an asynchronous I/O request on the host with the guest buffer. Then virtio-blk emulation returns back to the caller and continues QEMU execution. It is unsafe to unplug memory while the I/O request is pending since there's no mechanism (e.g. refcount) to wait until the guest memory is no longer in use. This is a known issue. There's no way to trigger a problem today but we need to eventually enhance QEMU's memory API to handle this case. Stefan
