Hi Cam,
Cam Macdonell wrote:
This patch supports sharing memory between VMs and between the host/VM. It's a first
cut and comments are encouraged. The goal is to support simple Inter-VM communication
with zero-copy access to shared memory.
Nice work!
I would suggest two design changes to make here. The first is that I
think you should use virtio. The second is that I think instead of
relying on mapping in device memory to the guest, you should have the
guest allocate it's own memory to dedicate to sharing.
A lot of what you're doing is duplicating functionality in virtio-pci.
You can also obtain greater portability by building the drivers with
virtio. It may not seem obvious how to make the memory sharing via BAR
fit into virtio, but if you follow my second suggestion, it will be a
lot easier.
Right now, you've got a bit of a hole in your implementation because you
only support files that are powers-of-two in size even though that's not
documented/enforced. This is a limitation of PCI resource regions.
Also, the PCI memory hole is limited in size today which is going to put
an upper bound on the amount of memory you could ever map into a guest.
Since you're using qemu_ram_alloc() also, it makes hotplug unworkable
too since qemu_ram_alloc() is a static allocation from a contiguous heap.
If you used virtio, what you could do is provide a ring queue that was
used to communicate a series of requests/response. The exchange might
look like this:
guest: REQ discover memory region
host: RSP memory region id: 4 size: 8k
guest: REQ map region id: 4 size: 8k: sgl: {(addr=43000, size=4k),
(addr=944000,size=4k)}
host: RSP mapped region id: 4
guest: REQ notify region id: 4
host: RSP notify region id: 4
guest: REQ poll region id: 4
host: RSP poll region id: 4
And the REQ/RSP order does not have to be in series like this. In
general, you need one entry on the queue to poll for new memory regions,
one entry for each mapped region to poll for incoming notification, and
then the remaining entries can be used to send short-lived
requests/responses.
It's important that the REQ map takes a scatter/gather list of physical
addresses because after running for a while, it's unlikely that you'll
be able to allocate any significant size of contiguous memory.
From a QEMU perspective, you would do memory sharing by waiting for a
map REQ from the guest and then you would complete the request by doing
an mmap(MAP_FIXED) with the appropriate parameters into phys_ram_base.
Notifications are a topic for discussion I think. A CharDriverState
could be used by I think it would be more interesting to do something
like a fd passed by SCM_RIGHTS so that eventfd can be used.
To simplify things, I'd suggest starting out only supporting one memory
region mapping.
Regards,
Anthony Liguori
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
