postcopy+vhost-user/shared ram
cover postcopy+vhost-user/shared ram
From: "Dr. David Alan Gilbert" <[email protected]>
Hi,
This is a RFC/WIP series that enables postcopy migration
with shared memory to a vhost-user process.
It's based off current-head + Juan's load_cleanup series, and Alexey's bitmap series (v4). It's very lightly tested and seems to work, but it's quite rough.
I've modified the vhost-user-bridge (aka vhub) in qemu's tests/ to use the new feature, since this is about the simplest client around.
Structure:
The basic idea is that near the start of postcopy, the client opens its own userfaultfd fd and sends that back to QEMU over the socket it's already using for VHUST_USER_* commands. Then when VHOST_USER_SET_MEM_TABLE arrives it registers the areas with userfaultfd and sends the mapped addresses back to QEMU.
QEMU then reads the clients UFD in it's fault thread and issues requests back to the source as needed.
QEMU also issues 'WAKE' ioctls on the UFD to let the client know that the page has arrived and can carry on.
A new feature (VHOST_USER_PROTOCOL_F_POSTCOPY) is added so that the QEMU knows the client can talk postcopy.
Three new messages (VHOST_USER_POSTCOPY_{ADVISE/LISTEN/END}) are added to guide the process along.
Current known issues:
I've not tested it with hugepages yet; and I suspect the madvises will need tweaking for it.
The qemu gets to see the base addresses that the client has its regions mapped at; that's not great for security
Take care of deadlocking; any thread in the client that accesses a userfault protected page can stall.
There's a nasty hack of a lock around the set_mem_table message.
I've not looked at the recent IOMMU code.
Some cleanup and a lot of corner cases need thinking about.
There are probably plenty of unknown issues as well.
Test setup:
I'm running on one host at the moment, with the guest scping a large file from the host as it migrates. The setup is based on one I found in the vhost-user setups. You'll need a recent kernel for the shared memory support in userfaultfd, and userfault isn't that happy if a process using shared memory core's - so make sure you have the latest fixes.
SESS=vhost
ulimit -c unlimited
tmux -L $SESS new-session -d
tmux -L $SESS set-option -g history-limit 30000
# Start a router using the system qemu
tmux -L $SESS new-window -n router ./x86_64-softmmu/qemu-system-x86_64 -M none -nographic -net socket,vlan=0,udp=loca
lhost:4444,localaddr=localhost:5555 -net socket,vlan=0,udp=localhost:4445,localaddr=localhost:5556 -net user,vlan=0
tmux -L $SESS set-option -g set-remain-on-exit on
# Start source vhost bridge
tmux -L $SESS new-window -n srcvhostbr "./tests/vhost-user-bridge -u /tmp/vubrsrc.sock 2>src-vub-log"
sleep 0.5
tmux -L $SESS new-window -n source "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backe
nd-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/
tmp/vubrsrc.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :0 -monitor stdio -trace events=/root/trace-file 2>src-qemu-log " # Start dest vhost bridge
tmux -L $SESS new-window -n destvhostbr "./tests/vhost-user-bridge -u /tmp/vubrdst.sock -l 127.0.0.1:4445 -r 127.0.0.
1:5556 2>dst-vub-log"
sleep 0.5
tmux -L $SESS new-window -n dest "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backend
-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/tm
p/vubrdst.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :1 -monitor stdio -incoming tcp::8888 -trace events=/root/trace-file 2>dst-qemu-log" tmux -L $SESS send-keys -t source "migrate_set_capability postcopy-ram on
tmux -L $SESS send-keys -t source "migrate_set_speed 20M
tmux -L $SESS send-keys -t dest "migrate_set_capability postcopy-ram on
then once booted:
tmux -L vhost send-keys -t source 'migrate -d tcp:0:8888^M' tmux -L vhost send-keys -t source 'migrate_start_postcopy^M' (Note those ^M's are actual ctrl-M's i.e. ctrl-v ctrl-M)
Dave
Dr. David Alan Gilbert (29):
RAMBlock/migration: Add migration flags
migrate: Update ram_block_discard_range for shared qemu_ram_block_host_offset
migration/ram: ramblock_recv_bitmap_test_byte_offset
postcopy: use UFFDIO_ZEROPAGE only when available
postcopy: Add notifier chain
postcopy: Add vhost-user flag for postcopy and check it
vhost-user: Add 'VHOST_USER_POSTCOPY_ADVISE' message
vhub: Support sending fds back to qemu
vhub: Open userfaultfd
postcopy: Allow registering of fd handler vhost+postcopy: Register shared ufd with postcopy vhost+postcopy: Transmit 'listen' to client vhost+postcopy: Register new regions with the ufd vhost+postcopy: Send address back to qemu vhost+postcopy: Stash RAMBlock and offset vhost+postcopy: Send requests to source for shared pages vhost+postcopy: Resolve client address
postcopy: wake shared
postcopy: postcopy_notify_shared_wake vhost+postcopy: Add vhost waker
vhost+postcopy: Call wakeups
vub+postcopy: madvises
vhost+postcopy: Lock around set_mem_table
vhu: enable = false on get_vring_base
vhost: Add VHOST_USER_POSTCOPY_END message vhost+postcopy: Wire up POSTCOPY_END notify
postcopy: Allow shared memory
vhost-user: Claim support for postcopy
contrib/libvhost-user/libvhost-user.c | 178 ++++++++++++++++- contrib/libvhost-user/libvhost-user.h | 8 + exec.c | 44 +++-- hw/virtio/trace-events | 13 ++ hw/virtio/vhost-user.c | 293 +++++++++++++++++++++++++++- include/exec/cpu-common.h | 3 + include/exec/ram_addr.h | 2 + migration/migration.c | 3 + migration/migration.h | 8 + migration/postcopy-ram.c | 357 +++++++++++++++++++++++++++------- migration/postcopy-ram.h | 69 +++++++ migration/ram.c | 5 + migration/ram.h | 1 + migration/savevm.c | 13 ++ migration/trace-events | 6 + trace-events | 3 + vl.c | 4 +- 17 files changed, 926 insertions(+), 84 deletions(-)
Signature · 1 line
2.13.0
RFC 1/29 RAMBlock/migration: Add migration flags
From: "Dr. David Alan Gilbert" <[email protected]>
Add a migration flags field to each RAMBlock so that the migration code can hold a set of private flags on the RAMBlock. Add accessors.
--- exec.c | 10 ++++++++++ include/exec/cpu-common.h | 2 ++ include/exec/ram_addr.h | 2 ++ 3 files changed, 14 insertions(+)
exec.c +10 -0
@@ -1741,6 +1741,16 @@ size_t qemu_ram_pagesize_largest(void)return largest; } +uint32_t qemu_ram_get_migration_flags (const RAMBlock *rb) +{ +return rb->migration_flags; +} + +void qemu_ram_set_migration_flags (RAMBlock *rb,uint32_t flags) +{ + rb->migration_flags = flags; +} +static int memory_try_enable_merging (void *addr,size_t len) {if (!machine_mem_merge (current_machine)) {
include/exec/cpu-common.h +2 -0
@@ -72,6 +72,8 @@ const char *qemu_ram_get_idstr(RAMBlock *rb);bool qemu_ram_is_shared (RAMBlock *rb);size_t qemu_ram_pagesize (RAMBlock *block);size_t qemu_ram_pagesize_largest (void ); +void qemu_ram_set_migration_flags (RAMBlock *rb,uint32_t flags); +uint32_t qemu_ram_get_migration_flags (const RAMBlock *rb);void cpu_physical_memory_rw (hwaddr addr,uint8_t *buf,int len,int is_write);
include/exec/ram_addr.h +2 -0
@@ -32,6 +32,8 @@ struct RAMBlock {ram_addr_t max_length;void (*resized)(const char *,uint64_t length,void *host);uint32_t flags; +/* These flags are owned by migration, initialised to 0 */ +uint32_t migration_flags;/* Protected by iothread lock. */ char idstr[256 ];/* RCU-enabled, writes protected by the ramlist lock */
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 01/29] RAMBlock/migration: Add migration flags
On Wed, Jun 28, 2017 at 08:00:19PM +0100, Dr. David Alan Gilbert (git) wrote:
[...]
include/exec/ram_addr.h struct RAMBlock { · line 36
Quoted 10 lines
include/exec/ram_addr.h +2
-0@@ -32,6 +32,8 @@ struct RAMBlock {ram_addr_t max_length;void (*resized)(const char *,uint64_t length,void *host);uint32_t flags; +/* These flags are owned by migration, initialised to 0 */ +uint32_t migration_flags;
Since we have RAMBlock.flags, would it be possible to use that directly? Currently it only used 3 bits. Thanks,
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [RFC 01/29] RAMBlock/migration: Add migration flags
Quoted 4 lines · * Peter Xu ([email protected]) wrote:
On Wed, Jun 28, 2017 at 08:00:19PM +0100, Dr. David Alan Gilbert (git) wrote:
[...]
Quoted 13 lines
include/exec/ram_addr.h struct RAMBlock { · line 36
Quoted 10 lines
include/exec/ram_addr.h +2
-0@@ -32,6 +32,8 @@ struct RAMBlock {ram_addr_t max_length;void (*resized)(const char *,uint64_t length,void *host);uint32_t flags; +/* These flags are owned by migration, initialised to 0 */ +uint32_t migration_flags;Since we have RAMBlock.flags, would it be possible to use that directly? Currently it only used 3 bits. Thanks,
OK, gone - we now have:
bool qemu_ram_is_uf_zeroable(RAMBlock *rb);
void qemu_ram_set_uf_zeroable(RAMBlock *rb);
which work on the new RAM_UF_ZEROPAGE flag.
Dave
Quoted 2 lines
Signature · 1 line
Peter Xu
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 2/29 migrate: Update ram_block_discard_range for shared
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
The choice of call to discard a block is getting more complicated
for other cases. We use fallocate PUNCH_HOLE in any file cases;
it works for both hugepage and for tmpfs.
We use the DONTNEED for non-hugepage cases either where they're anonymous or where they're private.
Care should be taken when trying other backing files.
--- exec.c | 28 ++++++++++++++++------------ trace-events | 3 +++ 2 files changed, 19 insertions(+), 12 deletions(-)
exec.c +16 -12
@@ -3557,6 +3557,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) }if ((start + length) <= rb->used_length) { +bool need_madvise, need_fallocate;uint8_t *host_endaddr = host_startaddr + length;if ((uintptr_t )host_endaddr & (rb->page_size -1 )) { error_report("ram_block_discard_range: Unaligned end address: %p",
@@ -3566,23 +3567,26 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) errno =ENOTSUP ;/* If we are missing MADVISE etc */ -if (rb->page_size == qemu_host_page_size) { -#if defined(CONFIG_MADVISE ) -/* Note: We need the madvise MADV_DONTNEED behaviour of definitely -* freeing the page. -*/ - ret =madvise (host_startaddr, length,MADV_DONTNEED ); -#endif - }else { -/* Huge page case - unfortunately it can't do DONTNEED, but -* it can do the equivalent by FALLOC_FL_PUNCH_HOLE in the -* huge page file. -*/ +/* The logic here is messy; +* madvise DONTNEED fails for hugepages +* fallocate works on hugepages and shmem +*/ + need_madvise = (rb->page_size == qemu_host_page_size) && + (rb->fd ==-1 || !(rb->flags &RAM_SHARED )); + need_fallocate = rb->fd !=-1 ; +if (ret ==-1 && need_fallocate) {#ifdef CONFIG_FALLOCATE_PUNCH_HOLE ret =fallocate (rb->fd,FALLOC_FL_PUNCH_HOLE |FALLOC_FL_KEEP_SIZE , start, length);#endif } +if (need_madvise && (!need_fallocate || (ret ==0 ))) { +#if defined(CONFIG_MADVISE ) + ret =madvise (host_startaddr, length,MADV_DONTNEED ); +#endif + } +trace_ram_block_discard_range (rb->idstr, host_startaddr, + need_madvise, need_fallocate, ret);if (ret) { ret = -errno; error_report("ram_block_discard_range: Failed to discard range "
trace-events +3 -0
@@ -55,6 +55,9 @@ dma_complete(void *dbs, int ret, void *cb) "dbs=%p ret=%d cb=%p" dma_blk_cb(void *dbs, int ret) "dbs=%p ret=%d" dma_map_wait(void *dbs) "dbs=%p" +# exec.c +ram_block_discard_range(const char *rbname, void *hva, bool need_madvise, bool need_fallocate, int ret) "%s@%p: madvise: %d fallocate: %d ret: %d" + # memory.c memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr %#"PRIx64" value %#"PRIx64" size %u" memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr %#"PRIx64" value %#"PRIx64" size %u"
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 02/29] migrate: Update ram_block_discard_range for shared
Quoted 42 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
The choice of call to discard a block is getting more complicated
for other cases. We use fallocatePUNCH_HOLEin any file cases;
it works for both hugepage and for tmpfs.
We use the DONTNEED for non-hugepage cases either where they're anonymous or where they're private.Care should be taken when trying other backing files.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- exec.c | 28 ++++++++++++++++------------ trace-events | 3 +++ 2 files changed, 19 insertions(+), 12 deletions(-)exec.c +1
-10@@ -3557,6 +3557,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) }if ((start + length) <= rb->used_length) { +bool need_madvise, need_fallocate;uint8_t *host_endaddr = host_startaddr + length;if ((uintptr_t )host_endaddr & (rb->page_size -1 )) { error_report("ram_block_discard_range: Unaligned end address: %p",@@ -3566,23 +3567,26 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) errno =ENOTSUP ;/* If we are missing MADVISE etc */ -if (rb->page_size == qemu_host_page_size) { -#if defined(CONFIG_MADVISE ) -/* Note: We need the madvise MADV_DONTNEED behaviour of definitely -* freeing the page. -*/ - ret =madvise (host_startaddr, length,MADV_DONTNEED ); -#endif - }else { -/* Huge page case - unfortunately it can't do DONTNEED, but -* it can do the equivalent by FALLOC_FL_PUNCH_HOLE in the
exec.c int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) · line 3577
Quoted 10 lines
exec.c +8
-2-* huge page file. -*/ +/* The logic here is messy; +* madvise DONTNEED fails for hugepages +* fallocate works on hugepages and shmem +*/ + need_madvise = (rb->page_size == qemu_host_page_size) && + (rb->fd ==-1 || !(rb->flags &RAM_SHARED )); + need_fallocate = rb->fd !=-1 ; +if (ret ==-1 && need_fallocate) {
(ret will always be -1 when reach here?)
exec.c int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) · line 3589
Quoted 12 lines
exec.c +7
-0#ifdef CONFIG_FALLOCATE_PUNCH_HOLE ret =fallocate (rb->fd,FALLOC_FL_PUNCH_HOLE |FALLOC_FL_KEEP_SIZE , start, length);#endif } +if (need_madvise && (!need_fallocate || (ret ==0 ))) { +#if defined(CONFIG_MADVISE ) + ret =madvise (host_startaddr, length,MADV_DONTNEED ); +#endif + } +trace_ram_block_discard_range (rb->idstr, host_startaddr, + need_madvise, need_fallocate, ret);
How about make the check easier by:
if (rb->page_size != qemu_host_page_size ||
rb->flags & RAM_SHARED) {
/* Either huge pages or shared memory will contain rb->fd */
assert(rb->fd);
fallocate(rb->fd, ...);
} else {
madvise();
}Thanks,
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [RFC 02/29] migrate: Update ram_block_discard_range for shared
Quoted 47 lines · * Peter Xu ([email protected]) wrote:
Quoted 46 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
The choice of call to discard a block is getting more complicated
for other cases. We use fallocatePUNCH_HOLEin any file cases;
it works for both hugepage and for tmpfs.
We use the DONTNEED for non-hugepage cases either where they're anonymous or where they're private.Care should be taken when trying other backing files.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- exec.c | 28 ++++++++++++++++------------ trace-events | 3 +++ 2 files changed, 19 insertions(+), 12 deletions(-)exec.c +3
-12@@ -3557,6 +3557,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) }if ((start + length) <= rb->used_length) { +bool need_madvise, need_fallocate;uint8_t *host_endaddr = host_startaddr + length;if ((uintptr_t )host_endaddr & (rb->page_size -1 )) { error_report("ram_block_discard_range: Unaligned end address: %p",@@ -3566,23 +3567,26 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) errno =ENOTSUP ;/* If we are missing MADVISE etc */ -if (rb->page_size == qemu_host_page_size) { -#if defined(CONFIG_MADVISE ) -/* Note: We need the madvise MADV_DONTNEED behaviour of definitely -* freeing the page. -*/ - ret =madvise (host_startaddr, length,MADV_DONTNEED ); -#endif - }else { -/* Huge page case - unfortunately it can't do DONTNEED, but -* it can do the equivalent by FALLOC_FL_PUNCH_HOLE in the -* huge page file. -*/ +/* The logic here is messy; +* madvise DONTNEED fails for hugepages
Quoted 8 lines
exec.c int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) · line 3577
Quoted 6 lines
exec.c +6
-0+* fallocate works on hugepages and shmem +*/ + need_madvise = (rb->page_size == qemu_host_page_size) && + (rb->fd ==-1 || !(rb->flags &RAM_SHARED )); + need_fallocate = rb->fd !=-1 ; +if (ret ==-1 && need_fallocate) {(ret will always be -1 when reach here?)
Yes, I was just making the code independent of order.
Quoted 15 lines
exec.c int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) · line 3589
Quoted 12 lines
exec.c +7
-0#ifdef CONFIG_FALLOCATE_PUNCH_HOLE ret =fallocate (rb->fd,FALLOC_FL_PUNCH_HOLE |FALLOC_FL_KEEP_SIZE , start, length);#endif } +if (need_madvise && (!need_fallocate || (ret ==0 ))) { +#if defined(CONFIG_MADVISE ) + ret =madvise (host_startaddr, length,MADV_DONTNEED ); +#endif + } +trace_ram_block_discard_range (rb->idstr, host_startaddr, + need_madvise, need_fallocate, ret);How about make the check easier by:
Quoted 8 lines
if (rb->page_size != qemu_host_page_size ||
rb->flags & RAM_SHARED) {
/* Either huge pages or shared memory will contain rb->fd */
assert(rb->fd);
fallocate(rb->fd, ...);
} else {
madvise();
}I've reworked this.
There are situations where you want both (I think!) - for shared memory, that's not a hugepage, you do an fallocate to clear the underlying storage, and then do an madvise to force the local mappings to be cleared.
Dave
Quoted 4 lines
Thanks,
Signature · 1 line
Peter Xu
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 3/29 qemu_ram_block_host_offset
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Utility to give the offset of a host pointer within a RAMBlock (assuming we already know it's in that RAMBlock)
--- exec.c | 6 ++++++ include/exec/cpu-common.h | 1 + 2 files changed, 7 insertions(+)
exec.c +6 -0
@@ -2218,6 +2218,12 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr,return ramblock_ptr (block, addr); } +/* Return the offset of a hostpointer within a ramblock */ +ram_addr_t qemu_ram_block_host_offset (RAMBlock *rb,void *host) +{ +return (uint8_t *)host - (uint8_t *)rb->host; +} +/* * Translates a host ptr back to a RAMBlock, a ram_addr and an offset * in that RAMBlock.
include/exec/cpu-common.h +1 -0
@@ -66,6 +66,7 @@ ram_addr_t qemu_ram_addr_from_host(void *ptr);RAMBlock *qemu_ram_block_by_name (const char *name);RAMBlock *qemu_ram_block_from_host (void *ptr,bool round_offset,ram_addr_t *offset); +ram_addr_t qemu_ram_block_host_offset (RAMBlock *rb,void *host);void qemu_ram_set_idstr (RAMBlock *block,const char *name,DeviceState *dev);void qemu_ram_unset_idstr (RAMBlock *block);const char *qemu_ram_get_idstr (RAMBlock *rb);
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 03/29] qemu_ram_block_host_offset
Quoted 16 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Utility to give the offset of a host pointer within a RAMBlock (assuming we already know it's in that RAMBlock)
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- exec.c | 6 ++++++ include/exec/cpu-common.h | 1 + 2 files changed, 7 insertions(+)exec.c +0
-0
exec.c static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr, · line 2226
Quoted 9 lines
exec.c +6
-0@@ -2218,6 +2218,12 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr,return ramblock_ptr (block, addr); } +/* Return the offset of a hostpointer within a ramblock */ +ram_addr_t qemu_ram_block_host_offset (RAMBlock *rb,void *host) +{ +return (uint8_t *)host - (uint8_t *)rb->host; +} +
I'd also assert that it's within that block.
Quoted 17 lines
exec.c +0
-0/* * Translates a host ptr back to a RAMBlock, a ram_addr and an offset * in that RAMBlock. include/exec/cpu-common.h +1
-0@@ -66,6 +66,7 @@ ram_addr_t qemu_ram_addr_from_host(void *ptr);RAMBlock *qemu_ram_block_by_name (const char *name);RAMBlock *qemu_ram_block_from_host (void *ptr,bool round_offset,ram_addr_t *offset); +ram_addr_t qemu_ram_block_host_offset (RAMBlock *rb,void *host);void qemu_ram_set_idstr (RAMBlock *block,const char *name,DeviceState *dev);void qemu_ram_unset_idstr (RAMBlock *block);const char *qemu_ram_get_idstr (RAMBlock *rb);Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 03/29] qemu_ram_block_host_offset
Quoted 20 lines · * Michael S. Tsirkin ([email protected]) wrote:
Quoted 19 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Utility to give the offset of a host pointer within a RAMBlock (assuming we already know it's in that RAMBlock)
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- exec.c | 6 ++++++ include/exec/cpu-common.h | 1 + 2 files changed, 7 insertions(+)exec.c +0
-0@@ -2218,6 +2218,12 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr,return ramblock_ptr (block, addr); }
Quoted 8 lines
exec.c static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr, · line 2226
Quoted 6 lines
exec.c +6
-0+/* Return the offset of a hostpointer within a ramblock */ +ram_addr_t qemu_ram_block_host_offset (RAMBlock *rb,void *host) +{ +return (uint8_t *)host - (uint8_t *)rb->host; +} +I'd also assert that it's within that block.
Done
Quoted 9 lines
Quoted 9 lines
exec.c +0
-0/* * Translates a host ptr back to a RAMBlock, a ram_addr and an offset * in that RAMBlock. include/exec/cpu-common.h +0
-0@@ -66,6 +66,7 @@ ram_addr_t qemu_ram_addr_from_host(void *ptr);RAMBlock *qemu_ram_block_by_name (const char *name);
Quoted 8 lines
Quoted 8 lines
include/exec/cpu-common.h +1
-0RAMBlock *qemu_ram_block_from_host (void *ptr,bool round_offset,ram_addr_t *offset); +ram_addr_t qemu_ram_block_host_offset (RAMBlock *rb,void *host);void qemu_ram_set_idstr (RAMBlock *block,const char *name,DeviceState *dev);void qemu_ram_unset_idstr (RAMBlock *block);const char *qemu_ram_get_idstr (RAMBlock *rb);Signature · 1 line
2.13.0
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 4/29 migration/ram: ramblock_recv_bitmap_test_byte_offset
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Utility for testing the map when you already know the offset in the RAMBlock.
--- migration/ram.c | 5 +++++ migration/ram.h | 1 + 2 files changed, 6 insertions(+)
migration/ram.c +5 -0
@@ -165,6 +165,11 @@ int ramblock_recv_bitmap_test(void *host_addr, RAMBlock *rb) rb->receivedmap); } +bool ramblock_recv_bitmap_test_byte_offset (uint64_t byte_offset,RAMBlock *rb) +{ +return test_bit (byte_offset >>TARGET_PAGE_BITS , rb->receivedmap); +} +void ramblock_recv_bitmap_set (void *host_addr,RAMBlock *rb) {set_bit_atomic (ramblock_recv_bitmap_offset (host_addr, rb), rb->receivedmap);
migration/ram.h +1 -0
@@ -55,6 +55,7 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);void ramblock_recv_map_init (void );int ramblock_recv_bitmap_test (void *host_addr,RAMBlock *rb); +bool ramblock_recv_bitmap_test_byte_offset (uint64_t byte_offset,RAMBlock *rb);void ramblock_recv_bitmap_set (void *host_addr,RAMBlock *rb);void ramblock_recv_bitmap_clear (void *host_addr,RAMBlock *rb);
Signature · 1 line
2.13.0
RFC 5/29 postcopy: use UFFDIO_ZEROPAGE only when available
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Use the recently added migration flag to hold whether each RAMBlock has the UFFDIO_ZEROPAGE capability, use it when it's available.
This allows the use of postcopy on tmpfs as well as hugepage backed files.
--- migration/migration.h | 4 ++++ migration/postcopy-ram.c | 12 +++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-)
migration/migration.h +4 -0
@@ -20,6 +20,10 @@#include "exec/cpu-common.h" #include "qemu/coroutine_int.h" +/* Migration flags to be set using qemu_ram_set_migration_flags */ +/* Postcopy can atomically zero pages in this RAMBlock */ +#define QEMU_MIGFLAG_POSTCOPY_ZERO 0x00000001 +/* State for the incoming migration */ struct MigrationIncomingState {QEMUFile *from_src_file;
migration/postcopy-ram.c +9 -3
@@ -408,6 +408,12 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,error_report ("%s userfault: Region doesn't support COPY" , __func__);return -1 ; } +if (reg_struct.ioctls & ((__u64 )1 << _UFFDIO_ZEROPAGE)) { +RAMBlock *rb =qemu_ram_block_by_name (block_name); +qemu_ram_set_migration_flags (rb,qemu_ram_get_migration_flags (rb) | +QEMU_MIGFLAG_POSTCOPY_ZERO ); + } +return 0 ; }
@@ -620,11 +626,11 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,int postcopy_place_page_zero (MigrationIncomingState *mis,void *host,RAMBlock *rb) { +size_t pagesize =qemu_ram_pagesize (rb);trace_postcopy_place_page_zero (host); -if (qemu_ram_pagesize (rb) ==getpagesize ()) { -if (qemu_ufd_copy_ioctl (mis->userfault_fd, host,NULL ,getpagesize (), - rb)) { +if (qemu_ram_get_migration_flags (rb) &QEMU_MIGFLAG_POSTCOPY_ZERO ) { +if (qemu_ufd_copy_ioctl (mis->userfault_fd, host,NULL , pagesize, rb)) {int e = errno;error_report ("%s: %s zero host: %p" , __func__,strerror (e), host);
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 05/29] postcopy: use UFFDIO_ZEROPAGE only when available
Quoted 35 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Use the recently added migration flag to hold whether each RAMBlock has the
UFFDIO_ZEROPAGEcapability, use it when it's available.This allows the use of postcopy on tmpfs as well as hugepage backed files.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- migration/migration.h | 4 ++++ migration/postcopy-ram.c | 12 +++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-)migration/migration.h +4
-0@@ -20,6 +20,10 @@#include "exec/cpu-common.h" #include "qemu/coroutine_int.h" +/* Migration flags to be set using qemu_ram_set_migration_flags */ +/* Postcopy can atomically zero pages in this RAMBlock */ +#define QEMU_MIGFLAG_POSTCOPY_ZERO 0x00000001 +/* State for the incoming migration */ struct MigrationIncomingState {QEMUFile *from_src_file;migration/postcopy-ram.c +0
-0
migration/postcopy-ram.c static int ram_block_enable_notify(const char *block_name, void *host_addr, · line 414
Quoted 7 lines
migration/postcopy-ram.c +4
-0@@ -408,6 +408,12 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,error_report ("%s userfault: Region doesn't support COPY" , __func__);return -1 ; } +if (reg_struct.ioctls & ((__u64 )1 << _UFFDIO_ZEROPAGE)) { +RAMBlock *rb =qemu_ram_block_by_name (block_name); +qemu_ram_set_migration_flags (rb,qemu_ram_get_migration_flags (rb) | +QEMU_MIGFLAG_POSTCOPY_ZERO );
Shall we use atomic_or() inside qemu_ram_set_migration_flags()? Then no need to fetch, and we'll be thread safe as well?
Quoted 6 lines
migration/postcopy-ram.c +2
-0+ } +return 0 ; }
migration/postcopy-ram.c int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, · line 632
Quoted 10 lines
migration/postcopy-ram.c +2
-3@@ -620,11 +626,11 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,int postcopy_place_page_zero (MigrationIncomingState *mis,void *host,RAMBlock *rb) { +size_t pagesize =qemu_ram_pagesize (rb);trace_postcopy_place_page_zero (host); -if (qemu_ram_pagesize (rb) ==getpagesize ()) { -if (qemu_ufd_copy_ioctl (mis->userfault_fd, host,NULL ,getpagesize (), - rb)) { +if (qemu_ram_get_migration_flags (rb) &QEMU_MIGFLAG_POSTCOPY_ZERO ) {
IIUC, _UFFDIO_ZEROPAGE is not supported on huge pages. If so, would here worth a comment?
Quoted 7 lines
migration/postcopy-ram.c +1
-0+if (qemu_ufd_copy_ioctl (mis->userfault_fd, host,NULL , pagesize, rb)) {int e = errno;error_report ("%s: %s zero host: %p" , __func__,strerror (e), host);Signature · 1 line
2.13.0
Thanks,
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [RFC 05/29] postcopy: use UFFDIO_ZEROPAGE only when available
Quoted 38 lines · * Peter Xu ([email protected]) wrote:
Quoted 37 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Use the recently added migration flag to hold whether each RAMBlock has the
UFFDIO_ZEROPAGEcapability, use it when it's available.This allows the use of postcopy on tmpfs as well as hugepage backed files.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- migration/migration.h | 4 ++++ migration/postcopy-ram.c | 12 +++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-)migration/migration.h +4
-0@@ -20,6 +20,10 @@#include "exec/cpu-common.h" #include "qemu/coroutine_int.h" +/* Migration flags to be set using qemu_ram_set_migration_flags */ +/* Postcopy can atomically zero pages in this RAMBlock */ +#define QEMU_MIGFLAG_POSTCOPY_ZERO 0x00000001 +/* State for the incoming migration */ struct MigrationIncomingState {QEMUFile *from_src_file;migration/postcopy-ram.c +0
-0@@ -408,6 +408,12 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,error_report ("%s userfault: Region doesn't support COPY" , __func__);return -1 ;
Quoted 8 lines
migration/postcopy-ram.c static int ram_block_enable_notify(const char *block_name, void *host_addr, · line 414
Quoted 5 lines
migration/postcopy-ram.c +4
-0} +if (reg_struct.ioctls & ((__u64 )1 << _UFFDIO_ZEROPAGE)) { +RAMBlock *rb =qemu_ram_block_by_name (block_name); +qemu_ram_set_migration_flags (rb,qemu_ram_get_migration_flags (rb) | +QEMU_MIGFLAG_POSTCOPY_ZERO );Shall we use
atomic_or()insideqemu_ram_set_migration_flags()? Then no need to fetch, and we'll be thread safe as well?
I've changed it to a simple |= in the new qemu_ram_set_uf_zeroable that works on the rb->flags field.
Quoted 12 lines
Quoted 12 lines
migration/postcopy-ram.c +3
-0+ } +return 0 ; }@@ -620,11 +626,11 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,int postcopy_place_page_zero (MigrationIncomingState *mis,void *host,RAMBlock *rb) { +size_t pagesize =qemu_ram_pagesize (rb);trace_postcopy_place_page_zero (host);
Quoted 7 lines
migration/postcopy-ram.c int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, · line 632
Quoted 4 lines
migration/postcopy-ram.c +1
-3-if (qemu_ram_pagesize (rb) ==getpagesize ()) { -if (qemu_ufd_copy_ioctl (mis->userfault_fd, host,NULL ,getpagesize (), - rb)) { +if (qemu_ram_get_migration_flags (rb) &QEMU_MIGFLAG_POSTCOPY_ZERO ) {IIUC,
_UFFDIO_ZEROPAGEis not supported on huge pages. If so, would here worth a comment?
Added.
Dave
Quoted 8 lines
Quoted 7 lines
migration/postcopy-ram.c +1
-0+if (qemu_ufd_copy_ioctl (mis->userfault_fd, host,NULL , pagesize, rb)) {int e = errno;error_report ("%s: %s zero host: %p" , __func__,strerror (e), host);Signature · 1 line
2.13.0
Quoted 4 lines
Thanks,
Signature · 1 line
Peter Xu
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 6/29 postcopy: Add notifier chain
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Add a notifier chain for postcopy with a 'reason' flag and an opportunity for a notifier member to return an error.
Call it when enabling postcopy.
This will initially used to enable devices to declare they're unable to postcopy and later to notify of devices of stages within postcopy.
--- migration/postcopy-ram.c | 41 +++++++++++++++++++++++++++++++++++++++++ migration/postcopy-ram.h | 26 ++++++++++++++++++++++++++ vl.c | 4 +++- 3 files changed, 70 insertions(+), 1 deletion(-)
migration/postcopy-ram.c +41 -0
@@ -23,6 +23,8 @@#include "savevm.h" #include "postcopy-ram.h" #include "ram.h" +#include "qapi/error.h" +#include "qemu/notify.h" #include "sysemu/sysemu.h" #include "sysemu/balloon.h" #include "qemu/error-report.h"
@@ -45,6 +47,38 @@ struct PostcopyDiscardState {unsigned int nsentcmds; }; +/* A notifier chain for postcopy +* The notifier should return 0 if it's OK, or a +* -errno on error. +* The notifier should expect an Error ** as it's data +*/ +static NotifierWithReturnList postcopy_notifier_list; + +void postcopy_infrastructure_init (void ) +{ +notifier_with_return_list_init (&postcopy_notifier_list); +} + +void postcopy_add_notifier (NotifierWithReturn *nn) +{ +notifier_with_return_list_add (&postcopy_notifier_list, nn); +} + +void postcopy_remove_notifier (NotifierWithReturn *n) +{ +notifier_with_return_remove (n); +} + +int postcopy_notify (enum PostcopyNotifyReason reason,Error **errp) +{ +struct PostcopyNotifyData pnd; + pnd.reason = reason; + pnd.errp = errp; + +return notifier_with_return_list_notify (&postcopy_notifier_list, + &pnd); +} +/* Postcopy needs to detect accesses to pages that haven't yet been copied * across, and efficiently map new pages in, the techniques for doing this * are target OS specific.
@@ -133,6 +167,7 @@ bool postcopy_ram_supported_by_host(void)struct uffdio_register reg_struct;struct uffdio_range range_struct;uint64_t feature_mask; +Error *local_err =NULL ;if (qemu_target_page_size () > pagesize) {error_report ("Target page size bigger than host page size" );
@@ -146,6 +181,12 @@ bool postcopy_ram_supported_by_host(void) goto out; } +/* Give devices a chance to object */ +if (postcopy_notify (POSTCOPY_NOTIFY_PROBE , &local_err)) { +error_report_err (local_err); + goto out; + } +/* Version and features check */ if (!ufd_version_check (ufd)) { goto out;
migration/postcopy-ram.h +26 -0
@@ -114,4 +114,30 @@ PostcopyState postcopy_state_get(void);/* Set the state and return the old state */ PostcopyState postcopy_state_set (PostcopyState new_state); +/* +* To be called once at the start before any device initialisation +*/ +void postcopy_infrastructure_init (void ); + +/* Add a notifier to a list to be called when checking whether the devices +* can support postcopy. +* It's data is a *PostcopyNotifyData +* It should return 0 if OK, or a negative value on failure. +* On failure it must set the data->errp to an error. +* +*/ +enum PostcopyNotifyReason { +POSTCOPY_NOTIFY_PROBE =0 , +}; + +struct PostcopyNotifyData { +enum PostcopyNotifyReason reason; +Error **errp; +}; + +void postcopy_add_notifier (NotifierWithReturn *nn); +void postcopy_remove_notifier (NotifierWithReturn *n); +/* Call the notifier list set by postcopy_add_start_notifier */ +int postcopy_notify (enum PostcopyNotifyReason reason,Error **errp); +#endif
vl.c +3 -1
@@ -93,8 +93,9 @@ int main(int argc, char **argv)#include "sysemu/dma.h" #include "hw/audio/soundhw.h" #include "audio/audio.h" -#include "sysemu/cpus.h" #include "migration/colo.h" +#include "migration/postcopy-ram.h" +#include "sysemu/cpus.h" #include "sysemu/kvm.h" #include "sysemu/hax.h" #include "qapi/qobject-input-visitor.h"
@@ -3060,6 +3061,7 @@ int main(int argc, char **argv, char **envp)module_call_init (MODULE_INIT_OPTS );runstate_init (); +postcopy_infrastructure_init ();if (qcrypto_init (&err) <0 ) {error_reportf_err (err,"cannot initialize crypto: " );
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 06/29] postcopy: Add notifier chain
On Wed, Jun 28, 2017 at 08:00:24PM +0100, Dr. David Alan Gilbert (git) wrote:
[...]
migration/postcopy-ram.h PostcopyState postcopy_state_get(void); · line 118
Quoted 10 lines
migration/postcopy-ram.h +2
-0@@ -114,4 +114,30 @@ PostcopyState postcopy_state_get(void);/* Set the state and return the old state */ PostcopyState postcopy_state_set (PostcopyState new_state); +/* +* To be called once at the start before any device initialisation
initialization?
Quoted 30 lines
migration/postcopy-ram.h +24
-0+*/ +void postcopy_infrastructure_init (void ); + +/* Add a notifier to a list to be called when checking whether the devices +* can support postcopy. +* It's data is a *PostcopyNotifyData +* It should return 0 if OK, or a negative value on failure. +* On failure it must set the data->errp to an error. +* +*/ +enum PostcopyNotifyReason { +POSTCOPY_NOTIFY_PROBE =0 , +}; + +struct PostcopyNotifyData { +enum PostcopyNotifyReason reason; +Error **errp; +}; + +void postcopy_add_notifier (NotifierWithReturn *nn); +void postcopy_remove_notifier (NotifierWithReturn *n); +/* Call the notifier list set by postcopy_add_start_notifier */ +int postcopy_notify (enum PostcopyNotifyReason reason,Error **errp); +#endif vl.c +0
-0
vl.c int main(int argc, char **argv) · line 98
Quoted 7 lines
vl.c +2
-1@@ -93,8 +93,9 @@ int main(int argc, char **argv)#include "sysemu/dma.h" #include "hw/audio/soundhw.h" #include "audio/audio.h" -#include "sysemu/cpus.h" #include "migration/colo.h" +#include "migration/postcopy-ram.h" +#include "sysemu/cpus.h"
(just curious: is moving sysemu/cpus.h intended?)
Quoted 14 lines
vl.c +1
-0#include "sysemu/kvm.h" #include "sysemu/hax.h" #include "qapi/qobject-input-visitor.h" @@ -3060,6 +3061,7 @@ int main(int argc, char **argv, char **envp)module_call_init (MODULE_INIT_OPTS );runstate_init (); +postcopy_infrastructure_init ();if (qcrypto_init (&err) <0 ) {error_reportf_err (err,"cannot initialize crypto: " );Signature · 1 line
2.13.0
Thanks,
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [RFC 06/29] postcopy: Add notifier chain
Quoted 12 lines · * Peter Xu ([email protected]) wrote:
On Wed, Jun 28, 2017 at 08:00:24PM +0100, Dr. David Alan Gilbert (git) wrote:
[...]
Quoted 8 lines
migration/postcopy-ram.h +0
-0@@ -114,4 +114,30 @@ PostcopyState postcopy_state_get(void);/* Set the state and return the old state */ PostcopyState postcopy_state_set (PostcopyState new_state);
Quoted 4 lines
migration/postcopy-ram.h PostcopyState postcopy_state_get(void); · line 118
Quoted 2 lines
migration/postcopy-ram.h +2
-0+/* +* To be called once at the start before any device initialisation initialization?
en_US vs en_GB :-)
Quoted 31 lines
Quoted 31 lines
migration/postcopy-ram.h +24
-0+*/ +void postcopy_infrastructure_init (void ); + +/* Add a notifier to a list to be called when checking whether the devices +* can support postcopy. +* It's data is a *PostcopyNotifyData +* It should return 0 if OK, or a negative value on failure. +* On failure it must set the data->errp to an error. +* +*/ +enum PostcopyNotifyReason { +POSTCOPY_NOTIFY_PROBE =0 , +}; + +struct PostcopyNotifyData { +enum PostcopyNotifyReason reason; +Error **errp; +}; + +void postcopy_add_notifier (NotifierWithReturn *nn); +void postcopy_remove_notifier (NotifierWithReturn *n); +/* Call the notifier list set by postcopy_add_start_notifier */ +int postcopy_notify (enum PostcopyNotifyReason reason,Error **errp); +#endif vl.c +0
-0@@ -93,8 +93,9 @@ int main(int argc, char **argv)#include "sysemu/dma.h"
Quoted 8 lines
vl.c int main(int argc, char **argv) · line 98
Quoted 6 lines
vl.c +2
-1#include "hw/audio/soundhw.h" #include "audio/audio.h" -#include "sysemu/cpus.h" #include "migration/colo.h" +#include "migration/postcopy-ram.h" +#include "sysemu/cpus.h" (just curious: is moving
sysemu/cpus.hintended?)
No! Removed.
Dave
Quoted 10 lines
Quoted 10 lines
vl.c +1
-0#include "sysemu/kvm.h" #include "sysemu/hax.h" #include "qapi/qobject-input-visitor.h" @@ -3060,6 +3061,7 @@ int main(int argc, char **argv, char **envp)module_call_init (MODULE_INIT_OPTS );runstate_init (); +postcopy_infrastructure_init ();
Quoted 10 lines
Quoted 5 lines
vl.c +0
-0if (qcrypto_init (&err) <0 ) {error_reportf_err (err,"cannot initialize crypto: " );Signature · 1 line
2.13.0Thanks,
Signature · 1 line
Peter Xu
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 7/29 postcopy: Add vhost-user flag for postcopy and check it
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Add a vhost feature flag for postcopy support, and use the postcopy notifier to check it before allowing postcopy.
--- contrib/libvhost-user/libvhost-user.h | 1 + hw/virtio/vhost-user.c | 39 ++++++++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-)
contrib/libvhost-user/libvhost-user.h +1 -0
@@ -34,6 +34,7 @@ enum VhostUserProtocolFeature {VHOST_USER_PROTOCOL_F_MQ =0 ,VHOST_USER_PROTOCOL_F_LOG_SHMFD =1 ,VHOST_USER_PROTOCOL_F_RARP =2 , +VHOST_USER_PROTOCOL_F_POSTCOPY =6 , VHOST_USER_PROTOCOL_F_MAX };
hw/virtio/vhost-user.c +38 -1
@@ -17,6 +17,8 @@#include "sysemu/kvm.h" #include "qemu/error-report.h" #include "qemu/sockets.h" +#include "migration/migration.h" +#include "migration/postcopy-ram.h" #include <sys/ioctl.h> #include <sys/socket.h>
@@ -33,7 +35,7 @@ enum VhostUserProtocolFeature {VHOST_USER_PROTOCOL_F_REPLY_ACK =3 ,VHOST_USER_PROTOCOL_F_NET_MTU =4 ,VHOST_USER_PROTOCOL_F_SLAVE_REQ =5 , - +VHOST_USER_PROTOCOL_F_POSTCOPY =6 , VHOST_USER_PROTOCOL_F_MAX };
@@ -121,8 +123,10 @@ static VhostUserMsg m __attribute__ ((unused));#define VHOST_USER_VERSION (0x1)struct vhost_user { +struct vhost_dev *dev;CharBackend *chr;int slave_fd; +NotifierWithReturn postcopy_notifier; };static bool ioeventfd_enabled (void )
@@ -701,6 +705,33 @@ out:return ret; } +static int vhost_user_postcopy_notifier (NotifierWithReturn *notifier, +void *opaque) +{ +struct PostcopyNotifyData *pnd = opaque; + struct vhost_user *u = container_of(notifier, struct vhost_user, + postcopy_notifier); +struct vhost_dev *dev = u->dev; + +switch (pnd->reason) { +case POSTCOPY_NOTIFY_PROBE : +if (!virtio_has_feature (dev->protocol_features, +VHOST_USER_PROTOCOL_F_POSTCOPY )) { +/* TODO: Get the device name into this error somehow */ +error_setg (pnd->errp, +"vhost-user backend not capable of postcopy" ); +return -ENOENT ; + } +break ; + +default : +/* We ignore notifications we don't know */ +break ; + } + +return 0 ; +} +static int vhost_user_init (struct vhost_dev *dev,void *opaque) {uint64_t features, protocol_features;
@@ -712,6 +743,7 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) u = g_new0(struct vhost_user, 1); u->chr = opaque; u->slave_fd =-1 ; + u->dev = dev; dev->opaque = u; err =vhost_user_get_features (dev, &features);
@@ -763,11 +795,15 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) "VHOST_USER_PROTOCOL_F_LOG_SHMFD feature."); } + u->postcopy_notifier.notify = vhost_user_postcopy_notifier; + err =vhost_setup_slave_channel (dev);if (err <0 ) {return err; } +postcopy_add_notifier (&u->postcopy_notifier); +return 0 ; }
@@ -778,6 +814,7 @@ static int vhost_user_cleanup(struct vhost_dev *dev)assert (dev->vhost_ops->backend_type ==VHOST_BACKEND_TYPE_USER ); u = dev->opaque; +postcopy_remove_notifier (&u->postcopy_notifier);if (u->slave_fd >=0 ) {close (u->slave_fd); u->slave_fd =-1 ;
Signature · 1 line
2.13.0
RFC 8/29 vhost-user: Add 'VHOST_USER_POSTCOPY_ADVISE' message
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Wire up a notifier to send a VHOST_USER_POSTCOPY_ADVISE message on an incoming advise.
Later patches will fill in the behaviour/contents of the message.
--- contrib/libvhost-user/libvhost-user.c | 14 ++++++++++ contrib/libvhost-user/libvhost-user.h | 1 + hw/virtio/vhost-user.c | 48 +++++++++++++++++++++++++++++++++++ migration/postcopy-ram.h | 1 + migration/savevm.c | 6 +++++ 5 files changed, 70 insertions(+)
contrib/libvhost-user/libvhost-user.c +14 -0
@@ -63,6 +63,7 @@ vu_request_to_string(int req)REQ (VHOST_USER_SET_VRING_ENABLE ),REQ (VHOST_USER_SEND_RARP ),REQ (VHOST_USER_INPUT_GET_CONFIG ), +REQ (VHOST_USER_POSTCOPY_ADVISE ), REQ(VHOST_USER_MAX), };#undef REQ
@@ -744,6 +745,17 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg) }static bool +vu_set_postcopy_advise (VuDev *dev,VhostUserMsg *vmsg) +{ +/* TODO: Open ufd, pass it back in the request +/* TODO: Add addresses +*/ + vmsg->payload.u64 =0xcafe ; + vmsg->size =sizeof (vmsg->payload.u64); +return true;/* = send a reply */ +} + +static bool vu_process_message (VuDev *dev,VhostUserMsg *vmsg) {int do_reply =0 ;
@@ -806,6 +818,8 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)return vu_get_queue_num_exec (dev, vmsg);case VHOST_USER_SET_VRING_ENABLE :return vu_set_vring_enable_exec (dev, vmsg); +case VHOST_USER_POSTCOPY_ADVISE : +return vu_set_postcopy_advise (dev, vmsg);default :vmsg_close_fds (vmsg);vu_panic (dev,"Unhandled request: %d" , vmsg->request);
contrib/libvhost-user/libvhost-user.h +1 -0
@@ -63,6 +63,7 @@ typedef enum VhostUserRequest {VHOST_USER_SET_VRING_ENABLE =18 ,VHOST_USER_SEND_RARP =19 ,VHOST_USER_INPUT_GET_CONFIG =20 , +VHOST_USER_POSTCOPY_ADVISE =23 , VHOST_USER_MAX } VhostUserRequest;
hw/virtio/vhost-user.c +48 -0
66 lines not on this page · raw message
migration/postcopy-ram.h +1 -0
@@ -128,6 +128,7 @@ void postcopy_infrastructure_init(void);*/ enum PostcopyNotifyReason {POSTCOPY_NOTIFY_PROBE =0 , +POSTCOPY_NOTIFY_INBOUND_ADVISE , };struct PostcopyNotifyData {
migration/savevm.c +6 -0
@@ -1365,6 +1365,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis) {PostcopyState ps =postcopy_state_set (POSTCOPY_INCOMING_ADVISE );uint64_t remote_pagesize_summary, local_pagesize_summary, remote_tps; +Error *local_err =NULL ;trace_loadvm_postcopy_handle_advise ();if (ps !=POSTCOPY_INCOMING_NONE ) {
@@ -1412,6 +1413,11 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis)return -1 ; } +if (postcopy_notify (POSTCOPY_NOTIFY_INBOUND_ADVISE , &local_err)) { +error_report_err (local_err); +return -1 ; + } +if (ram_postcopy_incoming_init (mis)) {return -1 ; }
Signature · 1 line
2.13.0
RFC 9/29 vhub: Support sending fds back to qemu
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Allow replies with fds (for postcopy)
--- contrib/libvhost-user/libvhost-user.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)
contrib/libvhost-user/libvhost-user.c +28 -1
@@ -214,6 +214,30 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg) {int rc;uint8_t *p = (uint8_t *)vmsg; +char control[CMSG_SPACE (VHOST_MEMORY_MAX_NREGIONS *sizeof (int ))] = { }; +struct iovec iov = { + .iov_base = (char *)vmsg, + .iov_len =VHOST_USER_HDR_SIZE , + }; +struct msghdr msg = { + .msg_iov = &iov, + .msg_iovlen =1 , + .msg_control = control, + }; +struct cmsghdr *cmsg; + +memset (control,0 ,sizeof (control)); +if (vmsg->fds) { +size_t fdsize = vmsg->fd_num *sizeof (int ); + msg.msg_controllen =CMSG_SPACE (fdsize); + cmsg =CMSG_FIRSTHDR (&msg); + cmsg->cmsg_len =CMSG_LEN (fdsize); + cmsg->cmsg_level =SOL_SOCKET ; + cmsg->cmsg_type =SCM_RIGHTS ; +memcpy (CMSG_DATA (cmsg), vmsg->fds, fdsize); + }else { + msg.msg_controllen =0 ; + }/* Set the version in the flags when sending the reply */ vmsg->flags &= ~VHOST_USER_VERSION_MASK ;
@@ -221,7 +245,7 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg) vmsg->flags |=VHOST_USER_REPLY_MASK ;do { - rc =write (conn_fd, p,VHOST_USER_HDR_SIZE ); + rc =sendmsg (conn_fd, &msg,0 ); }while (rc <0 && (errno ==EINTR || errno ==EAGAIN )); do {
@@ -314,6 +338,7 @@ vu_get_features_exec(VuDev *dev, VhostUserMsg *vmsg) } vmsg->size =sizeof (vmsg->payload.u64); + vmsg->fd_num =0 ;DPRINT ("Sending back to guest u64: 0x%016" PRIx64"\n" , vmsg->payload.u64);
@@ -455,6 +480,7 @@ vu_set_log_base_exec(VuDev *dev, VhostUserMsg *vmsg) dev->log_size = log_mmap_size; vmsg->size =sizeof (vmsg->payload.u64); + vmsg->fd_num =0 ;return true; }
@@ -699,6 +725,7 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg) vmsg->payload.u64 = features; vmsg->size =sizeof (vmsg->payload.u64); + vmsg->fd_num =0 ;return true; }
Signature · 1 line
2.13.0
RFC 10/29 vhub: Open userfaultfd
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Open a userfaultfd (on a postcopy_advise) and send it back in the reply to the qemu for it to monitor.
--- contrib/libvhost-user/libvhost-user.c | 24 +++++++++++++++++++++--- contrib/libvhost-user/libvhost-user.h | 3 +++ 2 files changed, 24 insertions(+), 3 deletions(-)
contrib/libvhost-user/libvhost-user.c +21 -3
@@ -15,6 +15,7 @@#include <qemu/osdep.h> #include <sys/eventfd.h> +#include <sys/syscall.h> #include <linux/vhost.h> #include "qemu/atomic.h"
@@ -774,11 +775,28 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)static bool vu_set_postcopy_advise (VuDev *dev,VhostUserMsg *vmsg) { -/* TODO: Open ufd, pass it back in the request -/* TODO: Add addresses -*/ +struct uffdio_api api_struct; + + dev->postcopy_ufd =syscall (__NR_userfaultfd,O_CLOEXEC |O_NONBLOCK ); +/* TODO: Add addresses */ vmsg->payload.u64 =0xcafe ; vmsg->size =sizeof (vmsg->payload.u64); + +if (dev->postcopy_ufd ==-1 ) { +vu_panic (dev,"Userfaultfd not available: %s" ,strerror (errno)); +return false; + } + api_struct.api =UFFD_API ; + api_struct.features =0 ; +if (ioctl (dev->postcopy_ufd,UFFDIO_API , &api_struct)) { +vu_panic (dev,"Failed UFFDIO_API: %s" ,strerror (errno)); +close (dev->postcopy_ufd); +return false; + } +/* TODO: Stash feature flags somewhere */ +/* Return a ufd to the QEMU */ + vmsg->fd_num =1 ; + vmsg->fds[0 ] = dev->postcopy_ufd;return true;/* = send a reply */ }
contrib/libvhost-user/libvhost-user.h +3 -0
@@ -231,6 +231,9 @@ struct VuDev {* re-initialize */ vu_panic_cb panic;const VuDevIface *iface; + +/* Postcopy data */ +int postcopy_ufd; }; typedef struct VuVirtqElement {
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 10/29] vhub: Open userfaultfd
Quoted 30 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Open a userfaultfd (on a
postcopy_advise) and send it back in the reply to the qemu for it to monitor.Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 24 +++++++++++++++++++++--- contrib/libvhost-user/libvhost-user.h | 3 +++ 2 files changed, 24 insertions(+), 3 deletions(-)contrib/libvhost-user/libvhost-user.c +1
-3@@ -15,6 +15,7 @@#include <qemu/osdep.h> #include <sys/eventfd.h> +#include <sys/syscall.h> #include <linux/vhost.h> #include "qemu/atomic.h" @@ -774,11 +775,28 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)static bool vu_set_postcopy_advise (VuDev *dev,VhostUserMsg *vmsg) { -/* TODO: Open ufd, pass it back in the request -/* TODO: Add addresses -*/
contrib/libvhost-user/libvhost-user.c vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg) · line 787
Quoted 10 lines
contrib/libvhost-user/libvhost-user.c +8
-0+struct uffdio_api api_struct; + + dev->postcopy_ufd =syscall (__NR_userfaultfd,O_CLOEXEC |O_NONBLOCK ); +/* TODO: Add addresses */ vmsg->payload.u64 =0xcafe ; vmsg->size =sizeof (vmsg->payload.u64); + +if (dev->postcopy_ufd ==-1 ) { +vu_panic (dev,"Userfaultfd not available: %s" ,strerror (errno)); +return false;
I think we may want to reply something even in case of error.
Indeed, if something goes wrong on backend side, Qemu will remain blocked waiting for the reply.
contrib/libvhost-user/libvhost-user.c vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg) · line 794
Quoted 7 lines
contrib/libvhost-user/libvhost-user.c +7
-0+ } + api_struct.api =UFFD_API ; + api_struct.features =0 ; +if (ioctl (dev->postcopy_ufd,UFFDIO_API , &api_struct)) { +vu_panic (dev,"Failed UFFDIO_API: %s" ,strerror (errno)); +close (dev->postcopy_ufd); +return false;
Ditto
Quoted 23 lines
contrib/libvhost-user/libvhost-user.c +5
-0+ } +/* TODO: Stash feature flags somewhere */ +/* Return a ufd to the QEMU */ + vmsg->fd_num =1 ; + vmsg->fds[0 ] = dev->postcopy_ufd;return true;/* = send a reply */ }contrib/libvhost-user/libvhost-user.h +3
-0@@ -231,6 +231,9 @@ struct VuDev {* re-initialize */ vu_panic_cb panic;const VuDevIface *iface; + +/* Postcopy data */ +int postcopy_ufd; }; typedef struct VuVirtqElement {
Re: [Qemu-devel] [RFC 10/29] vhub: Open userfaultfd
Quoted 44 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 28 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Open a userfaultfd (on a
postcopy_advise) and send it back in the reply to the qemu for it to monitor.Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 24 +++++++++++++++++++++--- contrib/libvhost-user/libvhost-user.h | 3 +++ 2 files changed, 24 insertions(+), 3 deletions(-)contrib/libvhost-user/libvhost-user.c +1
-3@@ -15,6 +15,7 @@#include <qemu/osdep.h> #include <sys/eventfd.h> +#include <sys/syscall.h> #include <linux/vhost.h> #include "qemu/atomic.h" @@ -774,11 +775,28 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)static bool vu_set_postcopy_advise (VuDev *dev,VhostUserMsg *vmsg) { -/* TODO: Open ufd, pass it back in the request -/* TODO: Add addresses -*/ contrib/libvhost-user/libvhost-user.c vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg) · line 787
Quoted 10 lines
contrib/libvhost-user/libvhost-user.c +8
-0+struct uffdio_api api_struct; + + dev->postcopy_ufd =syscall (__NR_userfaultfd,O_CLOEXEC |O_NONBLOCK ); +/* TODO: Add addresses */ vmsg->payload.u64 =0xcafe ; vmsg->size =sizeof (vmsg->payload.u64); + +if (dev->postcopy_ufd ==-1 ) { +vu_panic (dev,"Userfaultfd not available: %s" ,strerror (errno)); +return false;I think we may want to reply something even in case of error.
Quoted 2 lines
Indeed, if something goes wrong on backend side, Qemu will remain blocked waiting for the reply.
Fixed.
Quoted 8 lines
contrib/libvhost-user/libvhost-user.c vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg) · line 794
Quoted 7 lines
contrib/libvhost-user/libvhost-user.c +7
-0+ } + api_struct.api =UFFD_API ; + api_struct.features =0 ; +if (ioctl (dev->postcopy_ufd,UFFDIO_API , &api_struct)) { +vu_panic (dev,"Failed UFFDIO_API: %s" ,strerror (errno)); +close (dev->postcopy_ufd); +return false;Ditto
Fixed.
Thanks,
Dave
Quoted 13 lines
Quoted 13 lines
contrib/libvhost-user/libvhost-user.c +5
-0+ } +/* TODO: Stash feature flags somewhere */ +/* Return a ufd to the QEMU */ + vmsg->fd_num =1 ; + vmsg->fds[0 ] = dev->postcopy_ufd;return true;/* = send a reply */ }contrib/libvhost-user/libvhost-user.h +0
-0@@ -231,6 +231,9 @@ struct VuDev {* re-initialize */
Quoted 8 lines
contrib/libvhost-user/libvhost-user.h struct VuDev {
Quoted 8 lines
contrib/libvhost-user/libvhost-user.h +3
-0vu_panic_cb panic;const VuDevIface *iface; + +/* Postcopy data */ +int postcopy_ufd; }; typedef struct VuVirtqElement {
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 11/29 postcopy: Allow registering of fd handler
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Allow other userfaultfd's to be registered into the fault thread so that handlers for shared memory can get responses.
--- migration/migration.c | 3 + migration/migration.h | 2 + migration/postcopy-ram.c | 212 +++++++++++++++++++++++++++++++++++------------ migration/postcopy-ram.h | 21 +++++ migration/trace-events | 2 + 5 files changed, 186 insertions(+), 54 deletions(-)
migration/migration.c +3 -0
@@ -134,6 +134,8 @@ MigrationIncomingState *migration_incoming_get_current(void)if (!once) { mis_current.state =MIGRATION_STATUS_NONE ;memset (&mis_current,0 ,sizeof (MigrationIncomingState)); + mis_current.postcopy_remote_fds =g_array_new (FALSE, TRUE, +sizeof (struct PostCopyFD ));qemu_mutex_init (&mis_current.rp_mutex);qemu_event_init (&mis_current.main_thread_load_event, false); once = true;
@@ -157,6 +159,7 @@ void migration_incoming_state_destroy(void)qemu_fclose (mis->from_src_file); mis->from_src_file =NULL ; } +g_array_free (mis->postcopy_remote_fds, TRUE);qemu_event_destroy (&mis->main_thread_load_event); }
migration/migration.h +2 -0
@@ -51,6 +51,8 @@ struct MigrationIncomingState {QemuMutex rp_mutex;/* We send replies from multiple threads */ void *postcopy_tmp_page;void *postcopy_tmp_zero_page; +/* PostCopyFD's for external userfaultfds & handlers of shared memory */ +GArray *postcopy_remote_fds;QEMUBH *bh;
migration/postcopy-ram.c +158 -54
247 lines not on this page · raw message
migration/postcopy-ram.h +21 -0
@@ -141,4 +141,25 @@ void postcopy_remove_notifier(NotifierWithReturn *n);/* Call the notifier list set by postcopy_add_start_notifier */ int postcopy_notify (enum PostcopyNotifyReason reason,Error **errp); +struct PostCopyFD ; + +/* ufd is a pointer to the struct uffd_msg *TODO: more Portable! */ +typedef int (*pcfdhandler )(struct PostCopyFD *pcfd,void *ufd); + +struct PostCopyFD { +int fd; +/* Data to pass to handler */ +void *data; +/* Handler to be called whenever we get a poll event */ +pcfdhandler handler; +/* A string to use in error messages */ +char *idstr; +}; + +/* Register a userfaultfd owned by an external process for +* shared memory. +*/ +void postcopy_register_shared_ufd (struct PostCopyFD *pcfd); +void postcopy_unregister_shared_ufd (struct PostCopyFD *pcfd); +#endif
migration/trace-events +2 -0
@@ -189,6 +189,8 @@ postcopy_place_page_zero(void *host_addr) "host=%p" postcopy_ram_enable_notify(void) "" postcopy_ram_fault_thread_entry(void) "" postcopy_ram_fault_thread_exit(void) "" +postcopy_ram_fault_thread_fds_core(int baseufd, int quitfd) "ufd: %d quitfd: %d" +postcopy_ram_fault_thread_fds_extra(size_t index, const char *name, int fd) "%zd/%s: %d" postcopy_ram_fault_thread_quit(void) "" postcopy_ram_fault_thread_request(uint64_t hostaddr, const char *ramblock, size_t offset) "Request for HVA=%" PRIx64 " rb=%s offset=%zx" postcopy_ram_incoming_cleanup_closeuf(void) ""
Signature · 1 line
2.13.0
RFC 12/29 vhost+postcopy: Register shared ufd with postcopy
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Register the UFD that comes in as the response to the 'advise' method with the postcopy code.
--- hw/virtio/vhost-user.c | 23 ++++++++++++++++++++++- migration/postcopy-ram.h | 2 +- 2 files changed, 23 insertions(+), 2 deletions(-)
hw/virtio/vhost-user.c +22 -1
@@ -24,6 +24,7 @@#include <sys/socket.h> #include <sys/un.h> #include <linux/vhost.h> +#include <linux/userfaultfd.h> #define VHOST_MEMORY_MAX_NREGIONS 8#define VHOST_USER_F_PROTOCOL_FEATURES 30
@@ -128,6 +129,7 @@ struct vhost_user {CharBackend *chr;int slave_fd;NotifierWithReturn postcopy_notifier; +struct PostCopyFD postcopy_fd; };static bool ioeventfd_enabled (void )
@@ -707,6 +709,19 @@ out: }/* +* Called back from the postcopy fault thread when a fault is received on our +* ufd. +* TODO: This is Linux specific +*/ +static int vhost_user_postcopy_fault_handler (struct PostCopyFD *pcfd, +void *ufd) +{ +struct uffd_msg *msg = ufd; + +return 0 ; +} + +/* * Called at the start of an inbound postcopy on reception of the * 'advise' command. */
@@ -745,8 +760,14 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)error_setg (errp,"%s: Failed to get ufd" , __func__);return -1 ; } +fcntl (ufd,F_SETFL ,O_NONBLOCK ); -/* TODO: register ufd with userfault thread */ +/* register ufd with userfault thread */ + u->postcopy_fd.fd = ufd; + u->postcopy_fd.data = dev; + u->postcopy_fd.handler = vhost_user_postcopy_fault_handler; + u->postcopy_fd.idstr ="vhost-user" ;/* Need to find unique name */ +postcopy_register_shared_ufd (&u->postcopy_fd);return 0 ; }
migration/postcopy-ram.h +1 -1
@@ -153,7 +153,7 @@ struct PostCopyFD {/* Handler to be called whenever we get a poll event */ pcfdhandler handler;/* A string to use in error messages */ -char *idstr; +const char *idstr; };/* Register a userfaultfd owned by an external process for
Signature · 1 line
2.13.0
RFC 13/29 vhost+postcopy: Transmit 'listen' to client
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Notify the vhost-user client on reception of the 'postcopy-listen' event from the source.
--- contrib/libvhost-user/libvhost-user.c | 16 ++++++++++++++++ contrib/libvhost-user/libvhost-user.h | 2 ++ hw/virtio/trace-events | 3 +++ hw/virtio/vhost-user.c | 23 +++++++++++++++++++++++ migration/postcopy-ram.h | 1 + migration/savevm.c | 7 +++++++ 6 files changed, 52 insertions(+)
contrib/libvhost-user/libvhost-user.c +16 -0
@@ -15,7 +15,9 @@#include <qemu/osdep.h> #include <sys/eventfd.h> +#include <sys/ioctl.h> #include <sys/syscall.h> +#include <linux/userfaultfd.h> #include <linux/vhost.h> #include "qemu/atomic.h"
@@ -65,6 +67,7 @@ vu_request_to_string(int req)REQ (VHOST_USER_SEND_RARP ),REQ (VHOST_USER_INPUT_GET_CONFIG ),REQ (VHOST_USER_POSTCOPY_ADVISE ), +REQ (VHOST_USER_POSTCOPY_LISTEN ), REQ(VHOST_USER_MAX), };#undef REQ
@@ -801,6 +804,17 @@ vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg) }static bool +vu_set_postcopy_listen (VuDev *dev,VhostUserMsg *vmsg) +{ +if (dev->nregions) { +vu_panic (dev,"Regions already registered at postcopy-listen" ); +return false; + } + dev->postcopy_listening = true; + +return false; +} +static bool vu_process_message (VuDev *dev,VhostUserMsg *vmsg) {int do_reply =0 ;
@@ -865,6 +879,8 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)return vu_set_vring_enable_exec (dev, vmsg);case VHOST_USER_POSTCOPY_ADVISE :return vu_set_postcopy_advise (dev, vmsg); +case VHOST_USER_POSTCOPY_LISTEN : +return vu_set_postcopy_listen (dev, vmsg);default :vmsg_close_fds (vmsg);vu_panic (dev,"Unhandled request: %d" , vmsg->request);
contrib/libvhost-user/libvhost-user.h +2 -0
@@ -64,6 +64,7 @@ typedef enum VhostUserRequest {VHOST_USER_SEND_RARP =19 ,VHOST_USER_INPUT_GET_CONFIG =20 ,VHOST_USER_POSTCOPY_ADVISE =23 , +VHOST_USER_POSTCOPY_LISTEN =24 , VHOST_USER_MAX } VhostUserRequest;
@@ -234,6 +235,7 @@ struct VuDev {/* Postcopy data */ int postcopy_ufd; +bool postcopy_listening; }; typedef struct VuVirtqElement {
hw/virtio/trace-events +3 -0
@@ -1,5 +1,8 @@ # See docs/tracing.txt for syntax documentation. +# hw/virtio/vhost-user.c +vhost_user_postcopy_listen(void) "" + # hw/virtio/virtio.c virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u" virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u"
hw/virtio/vhost-user.c +23 -0
@@ -67,6 +67,7 @@ typedef enum VhostUserRequest {VHOST_USER_SET_SLAVE_REQ_FD =21 ,VHOST_USER_IOTLB_MSG =22 ,VHOST_USER_POSTCOPY_ADVISE =23 , +VHOST_USER_POSTCOPY_LISTEN =24 , VHOST_USER_MAX } VhostUserRequest;
@@ -771,6 +772,25 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)return 0 ; } +/* +* Called at the switch to postcopy on reception of the 'listen' command. +*/ +static int vhost_user_postcopy_listen (struct vhost_dev *dev,Error **errp) +{ +VhostUserMsg msg = { + .request =VHOST_USER_POSTCOPY_LISTEN , + .flags =VHOST_USER_VERSION , + }; + +trace_vhost_user_postcopy_listen (); +if (vhost_user_write (dev, &msg,NULL ,0 ) <0 ) { +error_setg (errp,"Failed to send postcopy_listen to vhost" ); +return -1 ; + } + +return 0 ; +} +static int vhost_user_postcopy_notifier (NotifierWithReturn *notifier,void *opaque) {
@@ -793,6 +813,9 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,case POSTCOPY_NOTIFY_INBOUND_ADVISE :return vhost_user_postcopy_advise (dev, pnd->errp); +case POSTCOPY_NOTIFY_INBOUND_LISTEN : +return vhost_user_postcopy_listen (dev, pnd->errp); +default :/* We ignore notifications we don't know */ break ;
migration/postcopy-ram.h +1 -0
@@ -129,6 +129,7 @@ void postcopy_infrastructure_init(void);enum PostcopyNotifyReason {POSTCOPY_NOTIFY_PROBE =0 ,POSTCOPY_NOTIFY_INBOUND_ADVISE , +POSTCOPY_NOTIFY_INBOUND_LISTEN , };struct PostcopyNotifyData {
migration/savevm.c +7 -0
@@ -1579,6 +1579,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis) {PostcopyState ps =postcopy_state_set (POSTCOPY_INCOMING_LISTENING );trace_loadvm_postcopy_handle_listen (); +Error *local_err =NULL ; +if (ps !=POSTCOPY_INCOMING_ADVISE && ps !=POSTCOPY_INCOMING_DISCARD ) {error_report ("CMD_POSTCOPY_LISTEN in wrong postcopy state (%d)" , ps);return -1 ;
@@ -1600,6 +1602,11 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)return -1 ; } +if (postcopy_notify (POSTCOPY_NOTIFY_INBOUND_LISTEN , &local_err)) { +error_report_err (local_err); +return -1 ; + } +if (mis->have_listen_thread) {error_report ("CMD_POSTCOPY_RAM_LISTEN already has a listen thread" );return -1 ;
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 13/29] vhost+postcopy: Transmit 'listen' to client
Quoted 15 lines · Dr. David Alan Gilbert (git)
hw/virtio/vhost-user.c +1
-0@@ -67,6 +67,7 @@ typedef enum VhostUserRequest {VHOST_USER_SET_SLAVE_REQ_FD =21 ,VHOST_USER_IOTLB_MSG =22 ,VHOST_USER_POSTCOPY_ADVISE =23 , +VHOST_USER_POSTCOPY_LISTEN =24 , VHOST_USER_MAX } VhostUserRequest;@@ -771,6 +772,25 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)return 0 ; }
hw/virtio/vhost-user.c static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp) · line 783
Quoted 10 lines
hw/virtio/vhost-user.c +9
-0+/* +* Called at the switch to postcopy on reception of the 'listen' command. +*/ +static int vhost_user_postcopy_listen (struct vhost_dev *dev,Error **errp) +{ +VhostUserMsg msg = { + .request =VHOST_USER_POSTCOPY_LISTEN , + .flags =VHOST_USER_VERSION , + };
I think it should use REPLY_ACK feature when available for two reasons:
1. The backend could reply nack if nregions is already set.
2. When leaving vhost_user_postcopy_listen(), the message might likely not have been handled yet by the backend.
Quoted 22 lines
hw/virtio/vhost-user.c +12
-0+trace_vhost_user_postcopy_listen (); +if (vhost_user_write (dev, &msg,NULL ,0 ) <0 ) { +error_setg (errp,"Failed to send postcopy_listen to vhost" ); +return -1 ; + } + +return 0 ; +} +static int vhost_user_postcopy_notifier (NotifierWithReturn *notifier,void *opaque) {@@ -793,6 +813,9 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,case POSTCOPY_NOTIFY_INBOUND_ADVISE :return vhost_user_postcopy_advise (dev, pnd->errp); +case POSTCOPY_NOTIFY_INBOUND_LISTEN : +return vhost_user_postcopy_listen (dev, pnd->errp); +default :/* We ignore notifications we don't know */ break ;
Re: [Qemu-devel] [RFC 13/29] vhost+postcopy: Transmit 'listen' to client
Quoted 23 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 20 lines · Dr. David Alan Gilbert (git)
hw/virtio/vhost-user.c +7
-0@@ -67,6 +67,7 @@ typedef enum VhostUserRequest {VHOST_USER_SET_SLAVE_REQ_FD =21 ,VHOST_USER_IOTLB_MSG =22 ,VHOST_USER_POSTCOPY_ADVISE =23 , +VHOST_USER_POSTCOPY_LISTEN =24 , VHOST_USER_MAX } VhostUserRequest;@@ -771,6 +772,25 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)return 0 ; } +/* +* Called at the switch to postcopy on reception of the 'listen' command. +*/ +static int vhost_user_postcopy_listen (struct vhost_dev *dev,Error **errp) +{ +VhostUserMsg msg = {
Quoted 8 lines
hw/virtio/vhost-user.c static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp) · line 783
Quoted 3 lines
hw/virtio/vhost-user.c +3
-0+ .request = VHOST_USER_POSTCOPY_LISTEN, + .flags = VHOST_USER_VERSION, + };I think it should use
REPLY_ACKfeature when available for two reasons:
1. The backend could reply nack if nregions is already set.
2. When leavingvhost_user_postcopy_listen(), the message might likely not have been handled yet by the backend.
OK, I can do that. What are the rules on features like that? Can I
just assume that if you've got POSTCOPY then you'll ack for new messages we add?
The current contrib/libvhost-user code doesn't seem to do acks yet, but I can add it here quite easily.
Dave
Quoted 13 lines
Quoted 13 lines
hw/virtio/vhost-user.c +9
-0+trace_vhost_user_postcopy_listen (); +if (vhost_user_write (dev, &msg,NULL ,0 ) <0 ) { +error_setg (errp,"Failed to send postcopy_listen to vhost" ); +return -1 ; + } + +return 0 ; +} +static int vhost_user_postcopy_notifier (NotifierWithReturn *notifier,void *opaque) {@@ -793,6 +813,9 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
Quoted 8 lines
hw/virtio/vhost-user.c static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier, · line 821
Quoted 8 lines
hw/virtio/vhost-user.c +3
-0case POSTCOPY_NOTIFY_INBOUND_ADVISE :return vhost_user_postcopy_advise (dev, pnd->errp); +case POSTCOPY_NOTIFY_INBOUND_LISTEN : +return vhost_user_postcopy_listen (dev, pnd->errp); +default :/* We ignore notifications we don't know */ break ;
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [RFC 13/29] vhost+postcopy: Transmit 'listen' to client
Quoted 33 lines · Dr. David Alan Gilbert
Quoted 23 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 20 lines · Dr. David Alan Gilbert (git)
hw/virtio/vhost-user.c +7
-0@@ -67,6 +67,7 @@ typedef enum VhostUserRequest {VHOST_USER_SET_SLAVE_REQ_FD =21 ,VHOST_USER_IOTLB_MSG =22 ,VHOST_USER_POSTCOPY_ADVISE =23 , +VHOST_USER_POSTCOPY_LISTEN =24 , VHOST_USER_MAX } VhostUserRequest;@@ -771,6 +772,25 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)return 0 ; } +/* +* Called at the switch to postcopy on reception of the 'listen' command. +*/ +static int vhost_user_postcopy_listen (struct vhost_dev *dev,Error **errp) +{ +VhostUserMsg msg = {Quoted 8 lines
hw/virtio/vhost-user.c static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp) · line 783
Quoted 3 lines
hw/virtio/vhost-user.c +3
-0+ .request = VHOST_USER_POSTCOPY_LISTEN, + .flags = VHOST_USER_VERSION, + };I think it should use
REPLY_ACKfeature when available for two reasons:
1. The backend could reply nack if nregions is already set.
2. When leavingvhost_user_postcopy_listen(), the message might likely not have been handled yet by the backend.
Quoted 3 lines
OK, I can do that. What are the rules on features like that? Can I
just assume that if you've got POSTCOPY then you'll ack for new messages we add?
You can have a look at vhost_user_net_set_mtu() for example. Before sending the message, just set VHOST_USER_NEED_REPLY_MASK bit in message's flags if VHOST_USER_PROTOCOL_F_REPLY_ACK is advertised by the backend.
After having sent the message, instead of returning directly, wait for the ack/nack by calling process_message_reply() if VHOST_USER_PROTOCOL_F_REPLY_ACK is advertised.
Quoted 2 lines
The current contrib/libvhost-user code doesn't seem to do acks yet, but I can add it here quite easily.
Yes, agree it is not implemented in libvhost-user, but we will be able to test it with DPDK prototype.
Thanks,
Maxime
Quoted 26 lines
Dave
Quoted 13 lines
Quoted 13 lines
hw/virtio/vhost-user.c +9
-0+trace_vhost_user_postcopy_listen (); +if (vhost_user_write (dev, &msg,NULL ,0 ) <0 ) { +error_setg (errp,"Failed to send postcopy_listen to vhost" ); +return -1 ; + } + +return 0 ; +} +static int vhost_user_postcopy_notifier (NotifierWithReturn *notifier,void *opaque) {@@ -793,6 +813,9 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,Quoted 8 lines
hw/virtio/vhost-user.c static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier, · line 821
Quoted 8 lines
hw/virtio/vhost-user.c +3
-0case POSTCOPY_NOTIFY_INBOUND_ADVISE :return vhost_user_postcopy_advise (dev, pnd->errp); +case POSTCOPY_NOTIFY_INBOUND_LISTEN : +return vhost_user_postcopy_listen (dev, pnd->errp); +default :/* We ignore notifications we don't know */ break ;--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 14/29 vhost+postcopy: Register new regions with the ufd
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
When new regions are sent to the client using SET_MEM_TABLE, register them with the userfaultfd.
--- contrib/libvhost-user/libvhost-user.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
contrib/libvhost-user/libvhost-user.c +33 -0
@@ -450,6 +450,39 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) dev_region->mmap_addr); } +if (dev->postcopy_listening) { +/* We should already have an open ufd need to mark each memory +* range as ufd. +* Note: Do we need any madvises? Well it's not been accessed +* yet, still probably need no THP to be safe, discard to be safe? +*/ +struct uffdio_register reg_struct; +/* Note: We might need to go back to using mmap_addr and +* len + mmap_offset for * huge pages, but then we do hope not to +* see accesses in that area below the offset +*/ + reg_struct.range.start = (uintptr_t )(dev_region->mmap_addr + + dev_region->mmap_offset); + reg_struct.range.len = dev_region->size; + reg_struct.mode =UFFDIO_REGISTER_MODE_MISSING ; + +if (ioctl (dev->postcopy_ufd,UFFDIO_REGISTER , ®_struct)) { +vu_panic (dev,"%s: Failed to userfault region %d: (ufd=%d)%s\n" , + __func__, i,strerror (errno), dev->postcopy_ufd); +continue ; + } +if (!(reg_struct.ioctls & ((__u64 )1 << _UFFDIO_COPY))) { +vu_panic (dev,"%s Region (%d) doesn't support COPY" , + __func__, i); +continue ; + } +DPRINT ("%s: region %d: Registered userfault for %llx + %llx\n" , + __func__, i, reg_struct.range.start, reg_struct.range.len); +/* TODO: Stash 'zero' support flags somewhere */ +/* TODO: Get address back to QEMU */ + + } +close (vmsg->fds[i]); }
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 14/29] vhost+postcopy: Register new regions with the ufd
Quoted 27 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
When new regions are sent to the client using
SET_MEM_TABLE, register them with the userfaultfd.Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)contrib/libvhost-user/libvhost-user.c +9
-0@@ -450,6 +450,39 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) dev_region->mmap_addr); } +if (dev->postcopy_listening) { +/* We should already have an open ufd need to mark each memory +* range as ufd. +* Note: Do we need any madvises? Well it's not been accessed +* yet, still probably need no THP to be safe, discard to be safe? +*/ +struct uffdio_register reg_struct; +/* Note: We might need to go back to using mmap_addr and +* len + mmap_offset for * huge pages, but then we do hope not to
contrib/libvhost-user/libvhost-user.c vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) · line 471
Quoted 10 lines
contrib/libvhost-user/libvhost-user.c +10
-0+* see accesses in that area below the offset +*/ + reg_struct.range.start = (uintptr_t )(dev_region->mmap_addr + + dev_region->mmap_offset); + reg_struct.range.len = dev_region->size; + reg_struct.mode =UFFDIO_REGISTER_MODE_MISSING ; + +if (ioctl (dev->postcopy_ufd,UFFDIO_REGISTER , ®_struct)) { +vu_panic (dev,"%s: Failed to userfault region %d: (ufd=%d)%s\n" , + __func__, i,strerror (errno), dev->postcopy_ufd);
Aren't the two last args swapped?
Quoted 18 lines
contrib/libvhost-user/libvhost-user.c +14
-0+continue ; + } +if (!(reg_struct.ioctls & ((__u64 )1 << _UFFDIO_COPY))) { +vu_panic (dev,"%s Region (%d) doesn't support COPY" , + __func__, i); +continue ; + } +DPRINT ("%s: region %d: Registered userfault for %llx + %llx\n" , + __func__, i, reg_struct.range.start, reg_struct.range.len); +/* TODO: Stash 'zero' support flags somewhere */ +/* TODO: Get address back to QEMU */ + + } +close (vmsg->fds[i]); }
Re: [Qemu-devel] [RFC 14/29] vhost+postcopy: Register new regions with the ufd
Quoted 32 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 29 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
When new regions are sent to the client using
SET_MEM_TABLE, register them with the userfaultfd.Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)contrib/libvhost-user/libvhost-user.c +12
-0@@ -450,6 +450,39 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) dev_region->mmap_addr); } +if (dev->postcopy_listening) { +/* We should already have an open ufd need to mark each memory +* range as ufd. +* Note: Do we need any madvises? Well it's not been accessed +* yet, still probably need no THP to be safe, discard to be safe? +*/ +struct uffdio_register reg_struct; +/* Note: We might need to go back to using mmap_addr and +* len + mmap_offset for * huge pages, but then we do hope not to +* see accesses in that area below the offset +*/ + reg_struct.range.start = (uintptr_t)(dev_region->mmap_addr +
Quoted 8 lines
contrib/libvhost-user/libvhost-user.c vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) · line 471
Quoted 7 lines
contrib/libvhost-user/libvhost-user.c +7
-0+ dev_region->mmap_offset); + reg_struct.range.len = dev_region->size; + reg_struct.mode =UFFDIO_REGISTER_MODE_MISSING ; + +if (ioctl (dev->postcopy_ufd,UFFDIO_REGISTER , ®_struct)) { +vu_panic (dev,"%s: Failed to userfault region %d: (ufd=%d)%s\n" , + __func__, i,strerror (errno), dev->postcopy_ufd);Aren't the two last args swapped?
Fixed.
Thanks for spotting that; adding a GCC_FMT_ATTR(2, 3) before vu_panic doesn't show any more from me, but there are a couple of existing cases that look wrong; I think they just need a * before them, but am not 100% sure.
Dave
Quoted 9 lines
Quoted 9 lines
contrib/libvhost-user/libvhost-user.c +9
-0+continue ; + } +if (!(reg_struct.ioctls & ((__u64 )1 << _UFFDIO_COPY))) { +vu_panic (dev,"%s Region (%d) doesn't support COPY" , + __func__, i); +continue ; + } +DPRINT ("%s: region %d: Registered userfault for %llx + %llx\n" , + __func__, i, reg_struct.range.start, reg_struct.range.len);
Quoted 8 lines
contrib/libvhost-user/libvhost-user.c vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) · line 488
Quoted 8 lines
contrib/libvhost-user/libvhost-user.c +5
-0+/* TODO: Stash 'zero' support flags somewhere */ +/* TODO: Get address back to QEMU */ + + } +close (vmsg->fds[i]); }
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 15/29 vhost+postcopy: Send address back to qemu
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
We need a better way, but at the moment we need the address of the mappings sent back to qemu so it can interpret the messages on the userfaultfd it reads.
--- contrib/libvhost-user/libvhost-user.c | 15 +++++++++- hw/virtio/trace-events | 1 + hw/virtio/vhost-user.c | 54 +++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-)
contrib/libvhost-user/libvhost-user.c +14 -1
@@ -479,13 +479,26 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)DPRINT ("%s: region %d: Registered userfault for %llx + %llx\n" , __func__, i, reg_struct.range.start, reg_struct.range.len);/* TODO: Stash 'zero' support flags somewhere */ -/* TODO: Get address back to QEMU */ +/* TODO: We need to find a way for the qemu not to see the virtual +* addresses of the clients, so as to keep better separation. +*/ +/* Return the address to QEMU so that it can translate the ufd +* fault addresses back. +*/ + msg_region->userspace_addr = (uintptr_t )(mmap_addr + + dev_region->mmap_offset); }close (vmsg->fds[i]); } +if (dev->postcopy_listening) { +/* Need to return the addresses - send the updated message back */ + vmsg->fd_num =0 ; +return true; + } +return false; }
hw/virtio/trace-events +1 -0
@@ -2,6 +2,7 @@ # hw/virtio/vhost-user.c vhost_user_postcopy_listen(void) "" +vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int reply_i, int region_i) "client:%"PRIx64" for hva: %"PRIx64" reply %d region %d" # hw/virtio/virtio.c virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
hw/virtio/vhost-user.c +54 -0
78 lines not on this page · raw message
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 15/29] vhost+postcopy: Send address back to qemu
<snip\>
Quoted 77 lines
hw/virtio/vhost-user.c +47
-0@@ -19,6 +19,7 @@#include "qemu/sockets.h" #include "migration/migration.h" #include "migration/postcopy-ram.h" +#include "trace.h" #include <sys/ioctl.h> #include <sys/socket.h> @@ -131,6 +132,7 @@ struct vhost_user {int slave_fd;NotifierWithReturn postcopy_notifier;struct PostCopyFD postcopy_fd; +uint64_t postcopy_client_bases[VHOST_MEMORY_MAX_NREGIONS ]; };static bool ioeventfd_enabled (void )@@ -298,6 +300,7 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,static int vhost_user_set_mem_table (struct vhost_dev *dev,struct vhost_memory *mem) { +struct vhost_user *u = dev->opaque;int fds[VHOST_MEMORY_MAX_NREGIONS ];int i, fd;size_t fd_num =0 ;@@ -348,6 +351,57 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,return -1 ; } +if (u->postcopy_fd.handler) { +VhostUserMsg msg_reply; +int region_i, reply_i; +if (vhost_user_read (dev, &msg_reply) <0 ) { +return -1 ; + } + +if (msg_reply.request !=VHOST_USER_SET_MEM_TABLE ) { +error_report ("%s: Received unexpected msg type." +"Expected %d received %d" , __func__, +VHOST_USER_SET_MEM_TABLE , msg_reply.request); +return -1 ; + } +/* We're using the same structure, just reusing one of the +* fields, so it should be the same size. +*/ +if (msg_reply.size != msg.size) { +error_report ("%s: Unexpected size for postcopy reply " +"%d vs %d" , __func__, msg_reply.size, msg.size); +return -1 ; + } + +memset (u->postcopy_client_bases,0 , +sizeof (uint64_t ) *VHOST_MEMORY_MAX_NREGIONS ); + +/* They're in the same order as the regions that were sent +* but some of the regions were skipped (above) if they +* didn't have fd's +*/ +for (reply_i =0 , region_i =0 ; + region_i < dev->mem->nregions; + region_i++) { +if (reply_i < fd_num && + msg_reply.payload.memory.regions[region_i].guest_phys_addr == + dev->mem->regions[region_i].guest_phys_addr) { + u->postcopy_client_bases[region_i] = + msg_reply.payload.memory.regions[reply_i].userspace_addr; +trace_vhost_user_set_mem_table_postcopy ( + msg_reply.payload.memory.regions[reply_i].userspace_addr, + msg.payload.memory.regions[reply_i].userspace_addr, + reply_i, region_i); + reply_i++; + } + }
hw/virtio/vhost-user.c static int vhost_user_set_mem_table(struct vhost_dev *dev, · line 407
Quoted 10 lines
hw/virtio/vhost-user.c +7
-0+if (reply_i != fd_num) { +error_report ("%s: postcopy reply not fully consumed " +"%d vs %zd" , + __func__, reply_i, fd_num); +return -1 ; + } + }if (reply_supported) {return process_message_reply (dev, &msg); }
If the backend supports reply-ack feature, the VHOST_USER_F_NEED_REPLY flag is set to the message earlier in this function.
In this case, when postcopy is also enabled, it means that for this request, to replies will have to be send by the backend.
Maybe it would be better not to set the VHOST_USER_F_NEED_REPLY in this specific case? Of course, it should be documented in the spec.
RFC 16/29 vhost+postcopy: Stash RAMBlock and offset
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Stash the RAMBlock and offset for later use looking up addresses.
--- hw/virtio/trace-events | 1 + hw/virtio/vhost-user.c | 11 +++++++++++ 2 files changed, 12 insertions(+)
hw/virtio/trace-events +1 -0
@@ -3,6 +3,7 @@ # hw/virtio/vhost-user.c vhost_user_postcopy_listen(void) "" vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int reply_i, int region_i) "client:%"PRIx64" for hva: %"PRIx64" reply %d region %d" +vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t memory_size, uint64_t guest_phys_addr, uint64_t userspace_addr, uint64_t offset) "%d:%s: size:%"PRIx64" GPA:%"PRIx64" QVA/userspace:%"PRIx64" RB offset:%"PRIx64 # hw/virtio/virtio.c virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
hw/virtio/vhost-user.c +11 -0
@@ -133,6 +133,11 @@ struct vhost_user {NotifierWithReturn postcopy_notifier;struct PostCopyFD postcopy_fd;uint64_t postcopy_client_bases[VHOST_MEMORY_MAX_NREGIONS ]; +RAMBlock *region_rb[VHOST_MEMORY_MAX_NREGIONS ]; +/* The offset from the start of the RAMBlock to the start of the +* vhost region. +*/ +ram_addr_t region_rb_offset[VHOST_MEMORY_MAX_NREGIONS ]; };static bool ioeventfd_enabled (void )
@@ -324,8 +329,14 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,assert ((uintptr_t )reg->userspace_addr == reg->userspace_addr); mr =memory_region_from_host ((void *)(uintptr_t )reg->userspace_addr, &offset); + u->region_rb_offset[i] = offset; + u->region_rb[i] = mr->ram_block; fd =memory_region_get_fd (mr);if (fd >0 ) { +trace_vhost_user_set_mem_table_withfd (fd_num, mr->name, + reg->memory_size, + reg->guest_phys_addr, + reg->userspace_addr, offset); msg.payload.memory.regions[fd_num].userspace_addr = reg->userspace_addr; msg.payload.memory.regions[fd_num].memory_size = reg->memory_size; msg.payload.memory.regions[fd_num].guest_phys_addr = reg->guest_phys_addr;
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 16/29] vhost+postcopy: Stash RAMBlock and offset
Quoted 28 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Stash the RAMBlock and offset for later use looking up addresses.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- hw/virtio/trace-events | 1 + hw/virtio/vhost-user.c | 11 +++++++++++ 2 files changed, 12 insertions(+)hw/virtio/trace-events +1
-0@@ -3,6 +3,7 @@ # hw/virtio/vhost-user.c vhost_user_postcopy_listen(void) "" vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int reply_i, int region_i) "client:%"PRIx64" for hva: %"PRIx64" reply %d region %d" +vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t memory_size, uint64_t guest_phys_addr, uint64_t userspace_addr, uint64_t offset) "%d:%s: size:%"PRIx64" GPA:%"PRIx64" QVA/userspace:%"PRIx64" RB offset:%"PRIx64 # hw/virtio/virtio.c virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"hw/virtio/vhost-user.c +0
-0
hw/virtio/vhost-user.c struct vhost_user { · line 140
Quoted 8 lines
hw/virtio/vhost-user.c +5
-0@@ -133,6 +133,11 @@ struct vhost_user {NotifierWithReturn postcopy_notifier;struct PostCopyFD postcopy_fd;uint64_t postcopy_client_bases[VHOST_MEMORY_MAX_NREGIONS ]; +RAMBlock *region_rb[VHOST_MEMORY_MAX_NREGIONS ]; +/* The offset from the start of the RAMBlock to the start of the +* vhost region. +*/ +ram_addr_t region_rb_offset[VHOST_MEMORY_MAX_NREGIONS ];
Here the array size is VHOST_MEMORY_MAX_NREGIONS, while...
hw/virtio/vhost-user.c static int vhost_user_set_mem_table(struct vhost_dev *dev, · line 333
Quoted 9 lines
hw/virtio/vhost-user.c +2
-0};static bool ioeventfd_enabled (void )@@ -324,8 +329,14 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,assert ((uintptr_t )reg->userspace_addr == reg->userspace_addr); mr =memory_region_from_host ((void *)(uintptr_t )reg->userspace_addr, &offset); + u->region_rb_offset[i] = offset; + u->region_rb[i] = mr->ram_block;
... can i>=VHOST_MEMORY_MAX_NREGIONS here? Or do we only need to note this down if fd > 0 below? Thanks,
Quoted 12 lines
hw/virtio/vhost-user.c +4
-0fd =memory_region_get_fd (mr);if (fd >0 ) { +trace_vhost_user_set_mem_table_withfd (fd_num, mr->name, + reg->memory_size, + reg->guest_phys_addr, + reg->userspace_addr, offset); msg.payload.memory.regions[fd_num].userspace_addr = reg->userspace_addr; msg.payload.memory.regions[fd_num].memory_size = reg->memory_size; msg.payload.memory.regions[fd_num].guest_phys_addr = reg->guest_phys_addr;Signature · 1 line
2.13.0
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [RFC 16/29] vhost+postcopy: Stash RAMBlock and offset
Quoted 42 lines · * Peter Xu ([email protected]) wrote:
Quoted 28 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Stash the RAMBlock and offset for later use looking up addresses.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- hw/virtio/trace-events | 1 + hw/virtio/vhost-user.c | 11 +++++++++++ 2 files changed, 12 insertions(+)hw/virtio/trace-events +1
-0@@ -3,6 +3,7 @@ # hw/virtio/vhost-user.c vhost_user_postcopy_listen(void) "" vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int reply_i, int region_i) "client:%"PRIx64" for hva: %"PRIx64" reply %d region %d" +vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t memory_size, uint64_t guest_phys_addr, uint64_t userspace_addr, uint64_t offset) "%d:%s: size:%"PRIx64" GPA:%"PRIx64" QVA/userspace:%"PRIx64" RB offset:%"PRIx64 # hw/virtio/virtio.c virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"hw/virtio/vhost-user.c +0
-0hw/virtio/vhost-user.c struct vhost_user { · line 140
Quoted 8 lines
hw/virtio/vhost-user.c +5
-0@@ -133,6 +133,11 @@ struct vhost_user {NotifierWithReturn postcopy_notifier;struct PostCopyFD postcopy_fd;uint64_t postcopy_client_bases[VHOST_MEMORY_MAX_NREGIONS ]; +RAMBlock *region_rb[VHOST_MEMORY_MAX_NREGIONS ]; +/* The offset from the start of the RAMBlock to the start of the +* vhost region. +*/ +ram_addr_t region_rb_offset[VHOST_MEMORY_MAX_NREGIONS ];Here the array size is
VHOST_MEMORY_MAX_NREGIONS, while...Quoted 2 lines
hw/virtio/vhost-user.c +0
-0};
Quoted 10 lines
hw/virtio/vhost-user.c static int vhost_user_set_mem_table(struct vhost_dev *dev, · line 333
Quoted 7 lines
hw/virtio/vhost-user.c +2
-0static bool ioeventfd_enabled (void )@@ -324,8 +329,14 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,assert ((uintptr_t )reg->userspace_addr == reg->userspace_addr); mr =memory_region_from_host ((void *)(uintptr_t )reg->userspace_addr, &offset); + u->region_rb_offset[i] = offset; + u->region_rb[i] = mr->ram_block;... can i>=VHOST_MEMORY_MAX_NREGIONS here? Or do we only need to note this down if fd > 0 below? Thanks,
I don't *think* so - I mean:
for (i = 0; i < dev->mem->nregions; ++i) {so if that's the maximum number of regions and that's the number of regions we should be safe???
Dave
Quoted 4 lines
Quoted 4 lines
hw/virtio/vhost-user.c +2
-0fd =memory_region_get_fd (mr);if (fd >0 ) { + trace_vhost_user_set_mem_table_withfd(fd_num, mr->name, + reg->memory_size,
Quoted 11 lines
Quoted 8 lines
hw/virtio/vhost-user.c +2
-0+ reg->guest_phys_addr, + reg->userspace_addr, offset); msg.payload.memory.regions[fd_num].userspace_addr = reg->userspace_addr; msg.payload.memory.regions[fd_num].memory_size = reg->memory_size; msg.payload.memory.regions[fd_num].guest_phys_addr = reg->guest_phys_addr;Signature · 1 line
2.13.0Signature · 1 line
Peter Xu
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [RFC 16/29] vhost+postcopy: Stash RAMBlock and offset
Quoted 54 lines · Dr. David Alan Gilbert
Quoted 42 lines · * Peter Xu ([email protected]) wrote:
Quoted 28 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Stash the RAMBlock and offset for later use looking up addresses.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- hw/virtio/trace-events | 1 + hw/virtio/vhost-user.c | 11 +++++++++++ 2 files changed, 12 insertions(+)hw/virtio/trace-events +1
-0@@ -3,6 +3,7 @@ # hw/virtio/vhost-user.c vhost_user_postcopy_listen(void) "" vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int reply_i, int region_i) "client:%"PRIx64" for hva: %"PRIx64" reply %d region %d" +vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t memory_size, uint64_t guest_phys_addr, uint64_t userspace_addr, uint64_t offset) "%d:%s: size:%"PRIx64" GPA:%"PRIx64" QVA/userspace:%"PRIx64" RB offset:%"PRIx64 # hw/virtio/virtio.c virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"hw/virtio/vhost-user.c +0
-0hw/virtio/vhost-user.c struct vhost_user { · line 140
Quoted 8 lines
hw/virtio/vhost-user.c +5
-0@@ -133,6 +133,11 @@ struct vhost_user {NotifierWithReturn postcopy_notifier;struct PostCopyFD postcopy_fd;uint64_t postcopy_client_bases[VHOST_MEMORY_MAX_NREGIONS ]; +RAMBlock *region_rb[VHOST_MEMORY_MAX_NREGIONS ]; +/* The offset from the start of the RAMBlock to the start of the +* vhost region. +*/ +ram_addr_t region_rb_offset[VHOST_MEMORY_MAX_NREGIONS ];Here the array size is
VHOST_MEMORY_MAX_NREGIONS, while...Quoted 2 lines
hw/virtio/vhost-user.c +0
-0};Quoted 10 lines
hw/virtio/vhost-user.c static int vhost_user_set_mem_table(struct vhost_dev *dev, · line 333
Quoted 7 lines
hw/virtio/vhost-user.c +2
-0static bool ioeventfd_enabled (void )@@ -324,8 +329,14 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,assert ((uintptr_t )reg->userspace_addr == reg->userspace_addr); mr =memory_region_from_host ((void *)(uintptr_t )reg->userspace_addr, &offset); + u->region_rb_offset[i] = offset; + u->region_rb[i] = mr->ram_block;... can i>=VHOST_MEMORY_MAX_NREGIONS here? Or do we only need to note this down if fd > 0 below? Thanks,
Quoted 5 lines
I don't *think* so - I mean:
for (i = 0; i < dev->mem->nregions; ++i) {so if that's the maximum number of regions and that's the number of regions we should be safe???
That's my concern - looks like dev->mem->nregions can be bigger than that? At least I didn't really see a restriction on its size. The size is changed in following stack:
vhost_region_add
vhost_set_memory
vhost_dev_assign_memory
And it's dynamic extended, without checks.
Indeed in the function vhost_user_set_mem_table() we have:
int fds[VHOST_MEMORY_MAX_NREGIONS];
But we are safe iiuc because we also have assertions to protect:
assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
fds[fd_num++] = fd;Do we at least need that assert?
Thanks,
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [RFC 16/29] vhost+postcopy: Stash RAMBlock and offset
Quoted 73 lines · * Peter Xu ([email protected]) wrote:
Quoted 54 lines · Dr. David Alan Gilbert
Quoted 42 lines · * Peter Xu ([email protected]) wrote:
Quoted 28 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Stash the RAMBlock and offset for later use looking up addresses.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- hw/virtio/trace-events | 1 + hw/virtio/vhost-user.c | 11 +++++++++++ 2 files changed, 12 insertions(+)hw/virtio/trace-events +1
-0@@ -3,6 +3,7 @@ # hw/virtio/vhost-user.c vhost_user_postcopy_listen(void) "" vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int reply_i, int region_i) "client:%"PRIx64" for hva: %"PRIx64" reply %d region %d" +vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t memory_size, uint64_t guest_phys_addr, uint64_t userspace_addr, uint64_t offset) "%d:%s: size:%"PRIx64" GPA:%"PRIx64" QVA/userspace:%"PRIx64" RB offset:%"PRIx64 # hw/virtio/virtio.c virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"hw/virtio/vhost-user.c +0
-0hw/virtio/vhost-user.c struct vhost_user { · line 140
Quoted 8 lines
hw/virtio/vhost-user.c +5
-0@@ -133,6 +133,11 @@ struct vhost_user {NotifierWithReturn postcopy_notifier;struct PostCopyFD postcopy_fd;uint64_t postcopy_client_bases[VHOST_MEMORY_MAX_NREGIONS ]; +RAMBlock *region_rb[VHOST_MEMORY_MAX_NREGIONS ]; +/* The offset from the start of the RAMBlock to the start of the +* vhost region. +*/ +ram_addr_t region_rb_offset[VHOST_MEMORY_MAX_NREGIONS ];Here the array size is
VHOST_MEMORY_MAX_NREGIONS, while...Quoted 2 lines
hw/virtio/vhost-user.c +0
-0};Quoted 10 lines
hw/virtio/vhost-user.c static int vhost_user_set_mem_table(struct vhost_dev *dev, · line 333
Quoted 7 lines
hw/virtio/vhost-user.c +2
-0static bool ioeventfd_enabled (void )@@ -324,8 +329,14 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,assert ((uintptr_t )reg->userspace_addr == reg->userspace_addr); mr =memory_region_from_host ((void *)(uintptr_t )reg->userspace_addr, &offset); + u->region_rb_offset[i] = offset; + u->region_rb[i] = mr->ram_block;... can i>=VHOST_MEMORY_MAX_NREGIONS here? Or do we only need to note this down if fd > 0 below? Thanks,
Quoted 5 lines
I don't *think* so - I mean:
for (i = 0; i < dev->mem->nregions; ++i) {so if that's the maximum number of regions and that's the number of regions we should be safe???
That's my concern - looks like
dev->mem->nregionscan be bigger than that? At least I didn't really see a restriction on its size. The size is changed in following stack:
vhost_region_add
vhost_set_memory
vhost_dev_assign_memoryAnd it's dynamic extended, without checks.
Indeed in the function
vhost_user_set_mem_table()we have:
Quoted 8 lines
int fds[VHOST_MEMORY_MAX_NREGIONS];But we are safe iiuc because we also have assertions to protect:
assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); fds[fd_num++] = fd;Do we at least need that assert?
I think you're right that it can be validly larger than MAX_NREGIONS; I think the idea is that the dev->mem->regions can be larger, with the limit being only on the number that actually have fd's.
I'll rework the structure.
Dave
Quoted 4 lines
Thanks,
Signature · 1 line
Peter Xu
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 17/29 vhost+postcopy: Send requests to source for shared pages
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Send requests back to the source for shared page requests.
--- migration/migration.h | 2 ++ migration/postcopy-ram.c | 31 ++++++++++++++++++++++++++++--- migration/postcopy-ram.h | 3 +++ migration/trace-events | 2 ++ 4 files changed, 35 insertions(+), 3 deletions(-)
migration/migration.h +2 -0
@@ -49,6 +49,8 @@ struct MigrationIncomingState {int userfault_quit_fd;QEMUFile *to_src_file;QemuMutex rp_mutex;/* We send replies from multiple threads */ +/* RAMBlock of last request sent to source */ +RAMBlock *last_rb;void *postcopy_tmp_page;void *postcopy_tmp_zero_page;/* PostCopyFD's for external userfaultfds & handlers of shared memory */
migration/postcopy-ram.c +28 -3
@@ -460,6 +460,31 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr, }/* +* Callback from shared fault handlers to ask for a page, +* the page must be specified by a RAMBlock and an offset in that rb +*/ +int postcopy_request_shared_page (struct PostCopyFD *pcfd,RAMBlock *rb, +uint64_t client_addr,uint64_t rb_offset) +{ +size_t pagesize =qemu_ram_pagesize (rb); +uint64_t aligned_rbo = rb_offset & ~(pagesize -1 ); +MigrationIncomingState *mis =migration_incoming_get_current (); + +trace_postcopy_request_shared_page (pcfd->idstr,qemu_ram_get_idstr (rb), + rb_offset); +/* TODO: Check bitmap to see if we already have the page */ +if (rb != mis->last_rb) { + mis->last_rb = rb; +migrate_send_rp_req_pages (mis,qemu_ram_get_idstr (rb), + aligned_rbo, pagesize); + }else { +/* Save some space */ +migrate_send_rp_req_pages (mis,NULL , aligned_rbo, pagesize); + } +return 0 ; +} + +/* * Handle faults detected by the USERFAULT markings */ static void *postcopy_ram_fault_thread (void *opaque)
@@ -469,9 +494,9 @@ static void *postcopy_ram_fault_thread(void *opaque)int ret;size_t index;RAMBlock *rb =NULL ; -RAMBlock *last_rb =NULL ;/* last RAMBlock we sent part of */ trace_postcopy_ram_fault_thread_entry (); + mis->last_rb =NULL ;/* last RAMBlock we sent part of */ qemu_sem_post (&mis->fault_thread_sem);struct pollfd *pfd;
@@ -560,8 +585,8 @@ static void *postcopy_ram_fault_thread(void *opaque)* Send the request to the source - we want to request one * of our host page sizes (which is >= TPS) */ -if (rb != last_rb) { - last_rb = rb; +if (rb != mis->last_rb) { + mis->last_rb = rb;migrate_send_rp_req_pages (mis,qemu_ram_get_idstr (rb), rb_offset,qemu_ram_pagesize (rb)); }else {
migration/postcopy-ram.h +3 -0
@@ -162,5 +162,8 @@ struct PostCopyFD {*/ void postcopy_register_shared_ufd (struct PostCopyFD *pcfd);void postcopy_unregister_shared_ufd (struct PostCopyFD *pcfd); +/* Callback from shared fault handlers to ask for a page */ +int postcopy_request_shared_page (struct PostCopyFD *pcfd,RAMBlock *rb, +uint64_t client_addr,uint64_t offset);#endif
migration/trace-events +2 -0
@@ -197,6 +197,8 @@ postcopy_ram_incoming_cleanup_closeuf(void) "" postcopy_ram_incoming_cleanup_entry(void) "" postcopy_ram_incoming_cleanup_exit(void) "" postcopy_ram_incoming_cleanup_join(void) "" +postcopy_request_shared_page(const char *sharer, const char *rb, uint64_t rb_offset) "for %s in %s offset %"PRIx64 + save_xbzrle_page_skipping(void) "" save_xbzrle_page_overflow(void) "" ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations"
Signature · 1 line
2.13.0
RFC 18/29 vhost+postcopy: Resolve client address
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Resolve fault addresses read off the clients UFD into RAMBlock and offset, and call back to the postcopy code to ask for the page.
--- hw/virtio/trace-events | 3 +++ hw/virtio/vhost-user.c | 28 +++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-)
hw/virtio/trace-events +3 -0
@@ -1,6 +1,9 @@ # See docs/tracing.txt for syntax documentation. # hw/virtio/vhost-user.c +vhost_user_postcopy_fault_handler(const char *name, uint64_t fault_address, int nregions) "%s: @%"PRIx64" nregions:%d" +vhost_user_postcopy_fault_handler_loop(int i, uint64_t client_base, uint64_t size) "%d: client%"PRIx64" +%"PRIx64 +vhost_user_postcopy_fault_handler_found(int i, uint64_t region_offset, uint64_t rb_offset) "%d: region_offset: %"PRIx64" rb_offset:%"PRIx64 vhost_user_postcopy_listen(void) "" vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int reply_i, int region_i) "client:%"PRIx64" for hva: %"PRIx64" reply %d region %d" vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t memory_size, uint64_t guest_phys_addr, uint64_t userspace_addr, uint64_t offset) "%d:%s: size:%"PRIx64" GPA:%"PRIx64" QVA/userspace:%"PRIx64" RB offset:%"PRIx64
hw/virtio/vhost-user.c +27 -1
@@ -782,9 +782,35 @@ out:static int vhost_user_postcopy_fault_handler (struct PostCopyFD *pcfd,void *ufd) { +struct vhost_dev *dev = pcfd->data; +struct vhost_user *u = dev->opaque;struct uffd_msg *msg = ufd; +uint64_t faultaddr = msg->arg.pagefault.address; +RAMBlock *rb =NULL ; +uint64_t rb_offset; +int i; -return 0 ; +trace_vhost_user_postcopy_fault_handler (pcfd->idstr, faultaddr, + dev->mem->nregions); +for (i =0 ; i < dev->mem->nregions; i++) { +trace_vhost_user_postcopy_fault_handler_loop (i, + u->postcopy_client_bases[i], dev->mem->regions[i].memory_size); +if (faultaddr >= u->postcopy_client_bases[i]) { +/* Ofset of the fault address in the vhost region */ +uint64_t region_offset = faultaddr - u->postcopy_client_bases[i]; +if (region_offset <= dev->mem->regions[i].memory_size) { + rb_offset = region_offset + u->region_rb_offset[i]; +trace_vhost_user_postcopy_fault_handler_found (i, + region_offset, rb_offset); + rb = u->region_rb[i]; +return postcopy_request_shared_page (pcfd, rb, faultaddr, + rb_offset); + } + } + } +error_report ("%s: Failed to find region for fault %" PRIx64, + __func__, faultaddr); +return -1 ; }/*
Signature · 1 line
2.13.0
RFC 19/29 postcopy: wake shared
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Send a 'wake' request on a userfaultfd for a shared process. The address in the clients address space is specified together with the RAMBlock it was resolved to.
--- migration/postcopy-ram.c | 26 ++++++++++++++++++++++++++ migration/postcopy-ram.h | 6 ++++++ migration/trace-events | 1 + 3 files changed, 33 insertions(+)
migration/postcopy-ram.c +26 -0
@@ -459,6 +459,25 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,return 0 ; } +int postcopy_wake_shared (struct PostCopyFD *pcfd, +uint64_t client_addr, +RAMBlock *rb) +{ +size_t pagesize =qemu_ram_pagesize (rb); +struct uffdio_range range; +int ret; +trace_postcopy_wake_shared (client_addr,qemu_ram_get_idstr (rb)); + range.start = client_addr & ~(pagesize -1 ); + range.len = pagesize; + ret =ioctl (pcfd->fd,UFFDIO_WAKE , &range); +if (ret) { +error_report ("%s: Failed to wake: %zx in %s (%s)" , + __func__, client_addr,qemu_ram_get_idstr (rb), +strerror (errno)); + } +return ret; +} +/* * Callback from shared fault handlers to ask for a page, * the page must be specified by a RAMBlock and an offset in that rb
@@ -877,6 +896,13 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis) return NULL; } +int postcopy_wake_shared (struct PostCopyFD *pcfd, +uint64_t client_addr, +RAMBlock *rb) +{ +assert (0 ); +return -1 ; +}#endif /* ------------------------------------------------------------------------- */
migration/postcopy-ram.h +6 -0
@@ -162,6 +162,12 @@ struct PostCopyFD {*/ void postcopy_register_shared_ufd (struct PostCopyFD *pcfd);void postcopy_unregister_shared_ufd (struct PostCopyFD *pcfd); +/* Notify a client ufd that a page is available +* Note: The 'client_address' is in the address space of the client +* program not QEMU +*/ +int postcopy_wake_shared (struct PostCopyFD *pcfd,uint64_t client_addr, +RAMBlock *rb);/* Callback from shared fault handlers to ask for a page */ int postcopy_request_shared_page (struct PostCopyFD *pcfd,RAMBlock *rb,uint64_t client_addr,uint64_t offset);
migration/trace-events +1 -0
@@ -198,6 +198,7 @@ postcopy_ram_incoming_cleanup_entry(void) "" postcopy_ram_incoming_cleanup_exit(void) "" postcopy_ram_incoming_cleanup_join(void) "" postcopy_request_shared_page(const char *sharer, const char *rb, uint64_t rb_offset) "for %s in %s offset %"PRIx64 +postcopy_wake_shared(uint64_t client_addr, const char *rb) "at %"PRIx64" in %s" save_xbzrle_page_skipping(void) "" save_xbzrle_page_overflow(void) ""
Signature · 1 line
2.13.0
RFC 20/29 postcopy: postcopy_notify_shared_wake
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Add a hook to allow a client userfaultfd to be 'woken' when a page arrives, and a walker that calls that hook for relevant clients given a RAMBlock and offset.
--- migration/postcopy-ram.c | 16 ++++++++++++++++ migration/postcopy-ram.h | 10 ++++++++++ 2 files changed, 26 insertions(+)
migration/postcopy-ram.c +16 -0
@@ -753,6 +753,22 @@ static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,return ret; } +int postcopy_notify_shared_wake (RAMBlock *rb,uint64_t offset) +{ +int i; +MigrationIncomingState *mis =migration_incoming_get_current (); +GArray *pcrfds = mis->postcopy_remote_fds; + +for (i =0 ; i < pcrfds->len; i++) { + struct PostCopyFD *cur = &g_array_index(pcrfds, struct PostCopyFD, i); +int ret = cur->waker (cur, rb, offset); +if (ret) { +return ret; + } + } +return 0 ; +} +/* * Place a host page (from) at (host) atomically * returns 0 on success
migration/postcopy-ram.h +10 -0
@@ -146,6 +146,10 @@ struct PostCopyFD;/* ufd is a pointer to the struct uffd_msg *TODO: more Portable! */ typedef int (*pcfdhandler )(struct PostCopyFD *pcfd,void *ufd); +/* Notification to wake, either on place or on reception of +* a fault on something that's already arrived (race) +*/ +typedef int (*pcfdwake )(struct PostCopyFD *pcfd,RAMBlock *rb,uint64_t offset);struct PostCopyFD {int fd;
@@ -153,6 +157,8 @@ struct PostCopyFD {void *data;/* Handler to be called whenever we get a poll event */ pcfdhandler handler; +/* Notification to wake shared client */ +pcfdwake waker;/* A string to use in error messages */ const char *idstr; };
@@ -162,6 +168,10 @@ struct PostCopyFD {*/ void postcopy_register_shared_ufd (struct PostCopyFD *pcfd);void postcopy_unregister_shared_ufd (struct PostCopyFD *pcfd); +/* Call each of the shared 'waker's registerd telling them of +* availability of a block. +*/ +int postcopy_notify_shared_wake (RAMBlock *rb,uint64_t offset);/* Notify a client ufd that a page is available * Note: The 'client_address' is in the address space of the client * program not QEMU
Signature · 1 line
2.13.0
RFC 21/29 vhost+postcopy: Add vhost waker
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Register a waker function in vhost-user code to be notified when pages arrive or requests to previously mapped pages get requested.
--- hw/virtio/trace-events | 3 +++ hw/virtio/vhost-user.c | 26 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+)
hw/virtio/trace-events +3 -0
@@ -5,6 +5,9 @@ vhost_user_postcopy_fault_handler(const char *name, uint64_t fault_address, int vhost_user_postcopy_fault_handler_loop(int i, uint64_t client_base, uint64_t size) "%d: client%"PRIx64" +%"PRIx64 vhost_user_postcopy_fault_handler_found(int i, uint64_t region_offset, uint64_t rb_offset) "%d: region_offset: %"PRIx64" rb_offset:%"PRIx64 vhost_user_postcopy_listen(void) "" +vhost_user_postcopy_waker(const char *rb, uint64_t rb_offset) "%s + %"PRIx64 +vhost_user_postcopy_waker_found(uint64_t client_addr) "%"PRIx64 +vhost_user_postcopy_waker_nomatch(const char *rb, uint64_t rb_offset) "%s + %"PRIx64 vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int reply_i, int region_i) "client:%"PRIx64" for hva: %"PRIx64" reply %d region %d" vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t memory_size, uint64_t guest_phys_addr, uint64_t userspace_addr, uint64_t offset) "%d:%s: size:%"PRIx64" GPA:%"PRIx64" QVA/userspace:%"PRIx64" RB offset:%"PRIx64
hw/virtio/vhost-user.c +26 -0
@@ -813,6 +813,31 @@ static int vhost_user_postcopy_fault_handler(struct PostCopyFD *pcfd,return -1 ; } +static int vhost_user_postcopy_waker (struct PostCopyFD *pcfd,RAMBlock *rb, +uint64_t offset) +{ +struct vhost_dev *dev = pcfd->data; +struct vhost_user *u = dev->opaque; +int i; + +trace_vhost_user_postcopy_waker (qemu_ram_get_idstr (rb), offset); +/* Translate the offset into an address in the clients address space */ +for (i =0 ; i < dev->mem->nregions; i++) { +if (u->region_rb[i] == rb && + offset >= u->region_rb_offset[i] && + offset < (u->region_rb_offset[i] + + dev->mem->regions[i].memory_size)) { +uint64_t client_addr = (offset - u->region_rb_offset[i]) + + u->postcopy_client_bases[i]; +trace_vhost_user_postcopy_waker_found (client_addr); +return postcopy_wake_shared (pcfd, client_addr, rb); + } + } + +trace_vhost_user_postcopy_waker_nomatch (qemu_ram_get_idstr (rb), offset); +return 0 ; +} +/* * Called at the start of an inbound postcopy on reception of the * 'advise' command.
@@ -858,6 +883,7 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp) u->postcopy_fd.fd = ufd; u->postcopy_fd.data = dev; u->postcopy_fd.handler = vhost_user_postcopy_fault_handler; + u->postcopy_fd.waker = vhost_user_postcopy_waker; u->postcopy_fd.idstr ="vhost-user" ;/* Need to find unique name */ postcopy_register_shared_ufd (&u->postcopy_fd);return 0 ;
Signature · 1 line
2.13.0
RFC 22/29 vhost+postcopy: Call wakeups
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Cause the vhost-user client to be woken up whenever:
a) We place a page in postcopy mode
b) We get a fault and the page has already been received
--- migration/postcopy-ram.c | 14 ++++++++++---- migration/trace-events | 1 + 2 files changed, 11 insertions(+), 4 deletions(-)
migration/postcopy-ram.c +10 -4
@@ -491,7 +491,11 @@ int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb,trace_postcopy_request_shared_page (pcfd->idstr,qemu_ram_get_idstr (rb), rb_offset); -/* TODO: Check bitmap to see if we already have the page */ +if (ramblock_recv_bitmap_test_byte_offset (aligned_rbo, rb)) { +trace_postcopy_request_shared_page_present (pcfd->idstr, +qemu_ram_get_idstr (rb), rb_offset); +return postcopy_wake_shared (pcfd, client_addr, rb); + }if (rb != mis->last_rb) { mis->last_rb = rb; migrate_send_rp_req_pages(mis, qemu_ram_get_idstr(rb),
@@ -792,7 +796,8 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from, }trace_postcopy_place_page (host); -return 0 ; +return postcopy_notify_shared_wake (rb, +qemu_ram_block_host_offset (rb, host)); }/*
@@ -813,6 +818,9 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,return -e; } +return postcopy_notify_shared_wake (rb, +qemu_ram_block_host_offset (rb, + host)); }else {/* The kernel can't use UFFDIO_ZEROPAGE for hugepages */ if (!mis->postcopy_tmp_zero_page) {
@@ -832,8 +840,6 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,return postcopy_place_page (mis, host, mis->postcopy_tmp_zero_page, rb); } - -return 0 ; }/*
migration/trace-events +1 -0
@@ -198,6 +198,7 @@ postcopy_ram_incoming_cleanup_entry(void) "" postcopy_ram_incoming_cleanup_exit(void) "" postcopy_ram_incoming_cleanup_join(void) "" postcopy_request_shared_page(const char *sharer, const char *rb, uint64_t rb_offset) "for %s in %s offset %"PRIx64 +postcopy_request_shared_page_present(const char *sharer, const char *rb, uint64_t rb_offset) "%s already %s offset %"PRIx64 postcopy_wake_shared(uint64_t client_addr, const char *rb) "at %"PRIx64" in %s" save_xbzrle_page_skipping(void) ""
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 22/29] vhost+postcopy: Call wakeups
Quoted 4 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Cause the vhost-user client to be woken up whenever:
a) We place a page in postcopy mode
Just to make sure I understand it correctly - UFFDIO_COPY will only wake up the waiters on the same userfaultfd context, so we don't need to wake up QEMU userfaultfd (vcpu threads), but we need to explicitly wake up other ufds/threads, like vhost-user backends. Am I right?
Thanks,
Quoted 1 line
b) We get a fault and the page has already been received
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [RFC 22/29] vhost+postcopy: Call wakeups
Quoted 10 lines · Peter Xu
Quoted 4 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Cause the vhost-user client to be woken up whenever:
a) We place a page in postcopy modeJust to make sure I understand it correctly -
UFFDIO_COPYwill only wake up the waiters on the same userfaultfd context, so we don't need to wake up QEMU userfaultfd (vcpu threads), but we need to explicitly wake up other ufds/threads, like vhost-user backends. Am I right?
Yes.
Every "uffd" represents one and only one "mm" (i.e. a process). So there is no way a single UFFDIO_COPY can wake the faults happening on a process different from the "mm" the uffd is associated with.
vhost-bridge being a different process requires a UFFDIO_WAKE on its own uffd it passed to qemu in addition of the UFFDIO_COPY that like you said implicitly wakes the userfaults happening on the qemu process (vcpus iothread, dataplane etc..).
On a side note there's a way not to wake userfaults implicitly in UFFDIO_COPY in case you want to wake userfaults in batches but nobody uses that for now (uffdio_copy.mode |= UFFDIO_COPY_MODE_DONTWAKE).
It'd be theoretically nice to optimize away the additional enter/exit kernel introduced by the UFFDIO_WAKE and the translation table as well.
What we could do is to add a UFFDIO_BIND that takes an "fd" as parameter to the ioctl to bind the two uffd together. Then we could push logical offsets in addition to the virtual address ranges when calling UFFDIO_REGISTER_LOGICAL (the logical offsets would then match the guest physical addresses) so that the UFFDIO_COPY_LOGICAL would then be able to get a logical range to wakeup that the kernel would translate into virtual addresses for all uffds bind together. Pushing offsets into UFFDIO_REGISTER was David's idea.
That would eliminate the enter/exit kernel for the explicit UFFDIO_WAKE and calling a single UFFDIO_COPY would be enough.
Alternatively we should make the uffd work based on file offsets instead of virtual addresses but that would involve changes to filesystems and it only would move the needle on top of tmpfs (shared=on/off no difference) and hugetlbfs. It would be enough for vhost-bridge.
Usually the uffd fault lives at the higher level of the virtual memory subsystem and never deals with file offsets so if we can get away with logical ranges per-uffd for UFFDIO_REGISTER and UFFDIO_COPY, it may be simpler and easier to extend automatically to all memory types supported by uffd (including anon which has no file offset).
No major improvement is to be expected by such an enhancement though so it's not very high priority to implement. It's not even clear if the complexity is worth it. Doing one more syscall per page I think might be measurable only on very fast network. The current way of operation where uffd are independent of each other and the translation table is transferred by userland means is quite optimal already and much simpler. Furthermore for hugetlbfs the performance difference most certainly wouldn't be measurable, as the enter/exit kernel would be diluted by a factor of 512 compared to 4k userfaults.
Thanks,
Andrea
Re: [Qemu-devel] [RFC 22/29] vhost+postcopy: Call wakeups
Quoted 40 lines · Andrea Arcangeli
Quoted 10 lines · Peter Xu
Quoted 4 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Cause the vhost-user client to be woken up whenever:
a) We place a page in postcopy modeJust to make sure I understand it correctly -
UFFDIO_COPYwill only wake up the waiters on the same userfaultfd context, so we don't need to wake up QEMU userfaultfd (vcpu threads), but we need to explicitly wake up other ufds/threads, like vhost-user backends. Am I right?Yes.
Every "uffd" represents one and only one "mm" (i.e. a process). So there is no way a single
UFFDIO_COPYcan wake the faults happening on a process different from the "mm" the uffd is associated with.vhost-bridge being a different process requires a
UFFDIO_WAKEon its own uffd it passed to qemu in addition of theUFFDIO_COPYthat like you said implicitly wakes the userfaults happening on the qemu process (vcpus iothread, dataplane etc..).On a side note there's a way not to wake userfaults implicitly in
UFFDIO_COPYin case you want to wake userfaults in batches but nobody uses that for now (uffdio_copy.mode |=UFFDIO_COPY_MODE_DONTWAKE).It'd be theoretically nice to optimize away the additional enter/exit kernel introduced by the
UFFDIO_WAKEand the translation table as well.What we could do is to add a
UFFDIO_BINDthat takes an "fd" as parameter to the ioctl to bind the two uffd together. Then we could push logical offsets in addition to the virtual address ranges when callingUFFDIO_REGISTER_LOGICAL(the logical offsets would then match the guest physical addresses) so that theUFFDIO_COPY_LOGICALwould then be able to get a logical range to wakeup that the kernel would translate into virtual addresses for all uffds bind together. Pushing offsets intoUFFDIO_REGISTERwas David's idea.
Quoted 8 lines
That would eliminate the enter/exit kernel for the explicit
UFFDIO_WAKEand calling a singleUFFDIO_COPYwould be enough.Alternatively we should make the uffd work based on file offsets instead of virtual addresses but that would involve changes to filesystems and it only would move the needle on top of tmpfs (shared=on/off no difference) and hugetlbfs. It would be enough for vhost-bridge.
Really glad to know these ideas.
Quoted 7 lines
Usually the uffd fault lives at the higher level of the virtual memory subsystem and never deals with file offsets so if we can get away with logical ranges per-uffd for
UFFDIO_REGISTERandUFFDIO_COPY, it may be simpler and easier to extend automatically to all memory types supported by uffd (including anon which has no file offset).
Quoted 9 lines
No major improvement is to be expected by such an enhancement though so it's not very high priority to implement. It's not even clear if the complexity is worth it. Doing one more syscall per page I think might be measurable only on very fast network. The current way of operation where uffd are independent of each other and the translation table is transferred by userland means is quite optimal already and much simpler. Furthermore for hugetlbfs the performance difference most certainly wouldn't be measurable, as the enter/exit kernel would be diluted by a factor of 512 compared to 4k userfaults.
Indeed, performance critical scenarios should be using huge pages, and that means that extra WAKE will have even smaller impact.
Thanks Andrea!
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [RFC 22/29] vhost+postcopy: Call wakeups
In reply to Andrea ArcangeliQuoted 8 lines · Andrea Arcangeli
What we could do is to add a
UFFDIO_BINDthat takes an "fd" as parameter to the ioctl to bind the two uffd together. Then we could push logical offsets in addition to the virtual address ranges when callingUFFDIO_REGISTER_LOGICAL(the logical offsets would then match the guest physical addresses) so that theUFFDIO_COPY_LOGICALwould then be able to get a logical range to wakeup that the kernel would translate into virtual addresses for all uffds bind together. Pushing offsets intoUFFDIO_REGISTERwas David's idea.
I think it was mine originally just in an off-list discussion. To me it seems cleaner to do UFFDIO_DUP which gives you a new fd bound to the current one, though.
Quoted 27 lines
That would eliminate the enter/exit kernel for the explicit
UFFDIO_WAKEand calling a singleUFFDIO_COPYwould be enough.Alternatively we should make the uffd work based on file offsets instead of virtual addresses but that would involve changes to filesystems and it only would move the needle on top of tmpfs (shared=on/off no difference) and hugetlbfs. It would be enough for vhost-bridge.
Usually the uffd fault lives at the higher level of the virtual memory subsystem and never deals with file offsets so if we can get away with logical ranges per-uffd for
UFFDIO_REGISTERandUFFDIO_COPY, it may be simpler and easier to extend automatically to all memory types supported by uffd (including anon which has no file offset).No major improvement is to be expected by such an enhancement though so it's not very high priority to implement. It's not even clear if the complexity is worth it. Doing one more syscall per page I think might be measurable only on very fast network. The current way of operation where uffd are independent of each other and the translation table is transferred by userland means is quite optimal already and much simpler. Furthermore for hugetlbfs the performance difference most certainly wouldn't be measurable, as the enter/exit kernel would be diluted by a factor of 512 compared to 4k userfaults.
Thanks,
Andrea
RFC 23/29 vub+postcopy: madvises
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Clear the area and turn off THP.
--- contrib/libvhost-user/libvhost-user.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)
contrib/libvhost-user/libvhost-user.c +30 -2
@@ -451,11 +451,39 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) }if (dev->postcopy_listening) { +int ret;/* We should already have an open ufd need to mark each memory * range as ufd. -* Note: Do we need any madvises? Well it's not been accessed -* yet, still probably need no THP to be safe, discard to be safe? */ + +/* Discard any mapping we have here; note I can't use MADV_REMOVE +* or fallocate to make the hole since I don't want to lose +* data that's already arrived in the shared process. +* TODO: How to do hugepage +*/ + ret =madvise ((void *)dev_region->mmap_addr, + dev_region->size + dev_region->mmap_offset, +MADV_DONTNEED ); +if (ret) { +fprintf (stderr, +"%s: Failed to madvise(DONTNEED) region %d: %s\n" , + __func__, i,strerror (errno)); + } +/* Turn off transparent hugepages so we dont get lose wakeups +* in neighbouring pages. +* TODO: Turn this backon later. +*/ + ret =madvise ((void *)dev_region->mmap_addr, + dev_region->size + dev_region->mmap_offset, +MADV_NOHUGEPAGE ); +if (ret) { +/* Note: This can happen legally on kernels that are configured +* without madvise'able hugepages +*/ +fprintf (stderr, +"%s: Failed to madvise(NOHUGEPAGE) region %d: %s\n" , + __func__, i,strerror (errno)); + }struct uffdio_register reg_struct;/* Note: We might need to go back to using mmap_addr and * len + mmap_offset for * huge pages, but then we do hope not to
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 23/29] vub+postcopy: madvises
Quoted 19 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Clear the area and turn off THP.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)contrib/libvhost-user/libvhost-user.c +1
-0@@ -451,11 +451,39 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) }if (dev->postcopy_listening) { +int ret;/* We should already have an open ufd need to mark each memory
contrib/libvhost-user/libvhost-user.c vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) · line 463
Quoted 10 lines
contrib/libvhost-user/libvhost-user.c +6
-2* range as ufd. -* Note: Do we need any madvises? Well it's not been accessed -* yet, still probably need no THP to be safe, discard to be safe? */ + +/* Discard any mapping we have here; note I can't use MADV_REMOVE +* or fallocate to make the hole since I don't want to lose +* data that's already arrived in the shared process. +* TODO: How to do hugepage +*/
Hi, David, frankly saying, I stuck with my solution, and I have also another issues, but here I could suggest solution for hugepages. I think we could transmit a received pages bitmap in VHOST_USER_SET_MEM_TABLE (VhostUserMemoryRegion), but it will raise a compatibility issue, or introduce special message type for that and send it before VHOST_USER_SET_MEM_TABLE. So it will be possible to do fallocate on received bitmap basis, just skip already copied pages. If you wish, I could send patches, rebased on yours, for doing it.
Quoted 26 lines
contrib/libvhost-user/libvhost-user.c +23
-0+ ret =madvise ((void *)dev_region->mmap_addr, + dev_region->size + dev_region->mmap_offset, +MADV_DONTNEED ); +if (ret) { +fprintf (stderr, +"%s: Failed to madvise(DONTNEED) region %d: %s\n" , + __func__, i,strerror (errno)); + } +/* Turn off transparent hugepages so we dont get lose wakeups +* in neighbouring pages. +* TODO: Turn this backon later. +*/ + ret =madvise ((void *)dev_region->mmap_addr, + dev_region->size + dev_region->mmap_offset, +MADV_NOHUGEPAGE ); +if (ret) { +/* Note: This can happen legally on kernels that are configured +* without madvise'able hugepages +*/ +fprintf (stderr, +"%s: Failed to madvise(NOHUGEPAGE) region %d: %s\n" , + __func__, i,strerror (errno)); + }struct uffdio_register reg_struct;/* Note: We might need to go back to using mmap_addr and * len + mmap_offset for * huge pages, but then we do hope not to
Signature · 2 lines
Best regards, Alexey Perevalov
Re: [Qemu-devel] [RFC 23/29] vub+postcopy: madvises
Quoted 32 lines · * Alexey Perevalov ([email protected]) wrote:
Quoted 18 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Clear the area and turn off THP.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)contrib/libvhost-user/libvhost-user.c +1
-0@@ -451,11 +451,39 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) }if (dev->postcopy_listening) { +int ret;/* We should already have an open ufd need to mark each memory contrib/libvhost-user/libvhost-user.c vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) · line 462
Quoted 10 lines
contrib/libvhost-user/libvhost-user.c +6
-2* range as ufd. -* Note: Do we need any madvises? Well it's not been accessed -* yet, still probably need no THP to be safe, discard to be safe? */ + +/* Discard any mapping we have here; note I can't use MADV_REMOVE +* or fallocate to make the hole since I don't want to lose +* data that's already arrived in the shared process. +* TODO: How to do hugepage +*/ Hi, David, frankly saying, I stuck with my solution, and I have also another issues,
but here I could suggest solution for hugepages. I think we could transmit a
Quoted 8 lines
received pages
bitmap inVHOST_USER_SET_MEM_TABLE(VhostUserMemoryRegion), but it will raise a compatibility issue,
or introduce special message type for that and send it beforeVHOST_USER_SET_MEM_TABLE.
So it will be possible to do fallocate on received bitmap basis, just skip already copied pages.
If you wish, I could send patches, rebased on yours, for doing it.
What we found works is that actually we don't need to do a discard - since we've only just done the mmap of the arena, nothing will be occupying it on the shared client, so we don't need to discard.
We've had a postcopy migrate work now, with a few hacks we're still cleaning up, both on vhost-user-bridge and dpdk; so I'll get this updated and reposted.
Dave
Quoted 18 lines
Quoted 18 lines
contrib/libvhost-user/libvhost-user.c +18
-0+ ret =madvise ((void *)dev_region->mmap_addr, + dev_region->size + dev_region->mmap_offset, +MADV_DONTNEED ); +if (ret) { +fprintf (stderr, +"%s: Failed to madvise(DONTNEED) region %d: %s\n" , + __func__, i,strerror (errno)); + } +/* Turn off transparent hugepages so we dont get lose wakeups +* in neighbouring pages. +* TODO: Turn this backon later. +*/ + ret =madvise ((void *)dev_region->mmap_addr, + dev_region->size + dev_region->mmap_offset, +MADV_NOHUGEPAGE ); +if (ret) { +/* Note: This can happen legally on kernels that are configured +* without madvise'able hugepages
Quoted 13 lines
Quoted 8 lines
contrib/libvhost-user/libvhost-user.c +5
-0+*/ +fprintf (stderr, +"%s: Failed to madvise(NOHUGEPAGE) region %d: %s\n" , + __func__, i,strerror (errno)); + }struct uffdio_register reg_struct;/* Note: We might need to go back to using mmap_addr and * len + mmap_offset for * huge pages, but then we do hope not to Signature · 2 lines
Best regards, Alexey Perevalov
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [RFC 23/29] vub+postcopy: madvises
Quoted 41 lines · Dr. David Alan Gilbert
Quoted 32 lines · * Alexey Perevalov ([email protected]) wrote:
Quoted 18 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Clear the area and turn off THP.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)contrib/libvhost-user/libvhost-user.c +1
-0@@ -451,11 +451,39 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) }if (dev->postcopy_listening) { +int ret;/* We should already have an open ufd need to mark each memory contrib/libvhost-user/libvhost-user.c vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) · line 462
Quoted 10 lines
contrib/libvhost-user/libvhost-user.c +6
-2* range as ufd. -* Note: Do we need any madvises? Well it's not been accessed -* yet, still probably need no THP to be safe, discard to be safe? */ + +/* Discard any mapping we have here; note I can't use MADV_REMOVE +* or fallocate to make the hole since I don't want to lose +* data that's already arrived in the shared process. +* TODO: How to do hugepage +*/ Hi, David, frankly saying, I stuck with my solution, and I have also another issues,
but here I could suggest solution for hugepages. I think we could transmit aQuoted 8 lines
received pages
bitmap inVHOST_USER_SET_MEM_TABLE(VhostUserMemoryRegion), but it will raise a compatibility issue,
or introduce special message type for that and send it beforeVHOST_USER_SET_MEM_TABLE.
So it will be possible to do fallocate on received bitmap basis, just skip already copied pages.
If you wish, I could send patches, rebased on yours, for doing it.
Quoted 3 lines
What we found works is that actually we don't need to do a discard - since we've only just done the mmap of the arena, nothing will be occupying it on the shared client, so we don't need to discard.
Looks like yes, I checked on kernel from Andrea's git, there is any more EEXIST error in case when client doesn't fallocate.
Quoted 4 lines
We've had a postcopy migrate work now, with a few hacks we're still cleaning up, both on vhost-user-bridge and dpdk; so I'll get this updated and reposted.
In you patch series vring is disabling in case of VHOST_USER_GET_VRING_BASE. It's being called when vhost-user server want's to stop vring.
QEMU is enabling vring as soon as virtual machine is started, so I didn't see explicit vring disabling for migrating VRING.
So migrating VRING is protected just by uffd_register, isn't it? And PMD thread (any vhost-user thread which accessing migrating VRING) will wait page copying in this case, right?
Quoted 38 lines
Dave
Quoted 18 lines
Quoted 18 lines
contrib/libvhost-user/libvhost-user.c +18
-0+ ret =madvise ((void *)dev_region->mmap_addr, + dev_region->size + dev_region->mmap_offset, +MADV_DONTNEED ); +if (ret) { +fprintf (stderr, +"%s: Failed to madvise(DONTNEED) region %d: %s\n" , + __func__, i,strerror (errno)); + } +/* Turn off transparent hugepages so we dont get lose wakeups +* in neighbouring pages. +* TODO: Turn this backon later. +*/ + ret =madvise ((void *)dev_region->mmap_addr, + dev_region->size + dev_region->mmap_offset, +MADV_NOHUGEPAGE ); +if (ret) { +/* Note: This can happen legally on kernels that are configured +* without madvise'able hugepages Quoted 12 lines
Quoted 8 lines
contrib/libvhost-user/libvhost-user.c +5
-0+*/ +fprintf (stderr, +"%s: Failed to madvise(NOHUGEPAGE) region %d: %s\n" , + __func__, i,strerror (errno)); + }struct uffdio_register reg_struct;/* Note: We might need to go back to using mmap_addr and * len + mmap_offset for * huge pages, but then we do hope not to Signature · 2 lines
Best regards, Alexey Perevalov--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Signature · 2 lines
Best regards, Alexey Perevalov
Re: [Qemu-devel] [RFC 23/29] vub+postcopy: madvises
Quoted 53 lines · * Alexey Perevalov ([email protected]) wrote:
Quoted 41 lines · Dr. David Alan Gilbert
Quoted 32 lines · * Alexey Perevalov ([email protected]) wrote:
Quoted 18 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Clear the area and turn off THP.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)contrib/libvhost-user/libvhost-user.c +1
-0@@ -451,11 +451,39 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) }if (dev->postcopy_listening) { +int ret;/* We should already have an open ufd need to mark each memory contrib/libvhost-user/libvhost-user.c vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) · line 462
Quoted 10 lines
contrib/libvhost-user/libvhost-user.c +6
-2* range as ufd. -* Note: Do we need any madvises? Well it's not been accessed -* yet, still probably need no THP to be safe, discard to be safe? */ + +/* Discard any mapping we have here; note I can't use MADV_REMOVE +* or fallocate to make the hole since I don't want to lose +* data that's already arrived in the shared process. +* TODO: How to do hugepage +*/ Hi, David, frankly saying, I stuck with my solution, and I have also another issues,
but here I could suggest solution for hugepages. I think we could transmit aQuoted 8 lines
received pages
bitmap inVHOST_USER_SET_MEM_TABLE(VhostUserMemoryRegion), but it will raise a compatibility issue,
or introduce special message type for that and send it beforeVHOST_USER_SET_MEM_TABLE.
So it will be possible to do fallocate on received bitmap basis, just skip already copied pages.
If you wish, I could send patches, rebased on yours, for doing it.Quoted 3 lines
What we found works is that actually we don't need to do a discard - since we've only just done the mmap of the arena, nothing will be occupying it on the shared client, so we don't need to discard.
Looks like yes, I checked on kernel from Andrea's git, there is any more EEXIST error in case when client doesn't fallocate.
Quoted 4 lines
We've had a postcopy migrate work now, with a few hacks we're still cleaning up, both on vhost-user-bridge and dpdk; so I'll get this updated and reposted.
Quoted 10 lines
In you patch series vring is disabling in case of
VHOST_USER_GET_VRING_BASE. It's being called when vhost-user server want's to stop vring. QEMU is enabling vring as soon as virtual machine is started, so I didn't see
explicit vring disabling for migrating VRING.
So migrating VRING is protected just byuffd_register, isn't it? And PMD thread (any
vhost-user thread which accessing migrating VRING) will wait page copying in this case,
right?
Yes I believe that's the case; although I don't know the structure of dpdk to know the effect of that.
Dave
Quoted 31 lines
Quoted 31 lines
Dave
Quoted 27 lines
Quoted 26 lines
contrib/libvhost-user/libvhost-user.c +23
-0+ ret =madvise ((void *)dev_region->mmap_addr, + dev_region->size + dev_region->mmap_offset, +MADV_DONTNEED ); +if (ret) { +fprintf (stderr, +"%s: Failed to madvise(DONTNEED) region %d: %s\n" , + __func__, i,strerror (errno)); + } +/* Turn off transparent hugepages so we dont get lose wakeups +* in neighbouring pages. +* TODO: Turn this backon later. +*/ + ret =madvise ((void *)dev_region->mmap_addr, + dev_region->size + dev_region->mmap_offset, +MADV_NOHUGEPAGE ); +if (ret) { +/* Note: This can happen legally on kernels that are configured +* without madvise'able hugepages +*/ +fprintf (stderr, +"%s: Failed to madvise(NOHUGEPAGE) region %d: %s\n" , + __func__, i,strerror (errno)); + }struct uffdio_register reg_struct;/* Note: We might need to go back to using mmap_addr and * len + mmap_offset for * huge pages, but then we do hope not to
Quoted 12 lines
Quoted 8 lines
Quoted 3 lines
Signature · 2 lines
Best regards, Alexey Perevalov--
Dr. David Alan Gilbert / [email protected] / Manchester, UKSignature · 2 lines
Best regards, Alexey Perevalov
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 24/29 vhost+postcopy: Lock around set_mem_table
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
**HACK - better solution needed **
We have the situation where:
qemu bridge
send set_mem_table
map memory
a) mark area with UFD
send reply with map addresses
b) start using
c) receive reply
As soon as (a) happens qemu might start seeing faults
from memory accesses (but doesn't until b); but it can't process those faults until (c) when it's received the mmap addresses.
Make the fault handler spin until it gets the reply in (c).
At the very least this needs some proper locks, but preferably we need to split the message.
--- hw/virtio/vhost-user.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
hw/virtio/vhost-user.c +13 -1
@@ -138,6 +138,7 @@ struct vhost_user {* vhost region. */ ram_addr_t region_rb_offset[VHOST_MEMORY_MAX_NREGIONS ]; +uint64_t in_set_mem_table;/*Hack! 1 while waiting for set_mem_table reply */ };static bool ioeventfd_enabled (void )
@@ -321,6 +322,7 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, msg.flags |=VHOST_USER_NEED_REPLY_MASK ; } +atomic_set (&u->in_set_mem_table, true);for (i =0 ; i < dev->mem->nregions; ++i) {struct vhost_memory_region *reg = dev->mem->regions + i;ram_addr_t offset;
@@ -351,14 +353,15 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,if (!fd_num) {error_report ("Failed initializing vhost-user memory map, " "consider using -object memory-backend-file share=on" ); +atomic_set (&u->in_set_mem_table, false);return -1 ; } msg.size =sizeof (msg.payload.memory.nregions); msg.size +=sizeof (msg.payload.memory.padding); msg.size += fd_num *sizeof (VhostUserMemoryRegion); -if (vhost_user_write (dev, &msg, fds, fd_num) <0 ) { +atomic_set (&u->in_set_mem_table, false);return -1 ; }
@@ -373,6 +376,7 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,error_report ("%s: Received unexpected msg type." "Expected %d received %d" , __func__,VHOST_USER_SET_MEM_TABLE , msg_reply.request); +atomic_set (&u->in_set_mem_table, false);return -1 ; }/* We're using the same structure, just reusing one of the
@@ -381,6 +385,7 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,if (msg_reply.size != msg.size) {error_report ("%s: Unexpected size for postcopy reply " "%d vs %d" , __func__, msg_reply.size, msg.size); +atomic_set (&u->in_set_mem_table, false);return -1 ; }
@@ -410,9 +415,11 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,error_report ("%s: postcopy reply not fully consumed " "%d vs %zd" , __func__, reply_i, fd_num); +atomic_set (&u->in_set_mem_table, false);return -1 ; } } +atomic_set (&u->in_set_mem_table, false);if (reply_supported) {return process_message_reply (dev, &msg); }
@@ -821,6 +828,11 @@ static int vhost_user_postcopy_waker(struct PostCopyFD *pcfd, RAMBlock *rb,int i;trace_vhost_user_postcopy_waker (qemu_ram_get_idstr (rb), offset); +while (atomic_mb_read (&u->in_set_mem_table)) { +fprintf (stderr,"%s: Spin waiting for memtable\n" , __func__); +usleep (1000 *100 ); + } +/* Translate the offset into an address in the clients address space */ for (i =0 ; i < dev->mem->nregions; i++) { if (u->region_rb[i] == rb &&
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 24/29] vhost+postcopy: Lock around set_mem_table
Quoted 19 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert"<[email protected]>
**HACK - better solution needed **
We have the situation where:qemu bridge
send
set_mem_table
map memory
a) mark area with UFD
send reply with map addresses
b) start using
c) receive replyAs soon as (a) happens qemu might start seeing faults
from memory accesses (but doesn't until b); but it can't process those faults until (c) when it's received the mmap addresses.
Quoted 4 lines
Make the fault handler spin until it gets the reply in (c).
At the very least this needs some proper locks, but preferably we need to split the message.
Yes, maybe the slave channel could be used to send the ufds with a dedicated request? The backend would set the reply-ack flag, so that it starts accessing the guest memory only when Qemu is ready to handle faults.
Note that the slave channel support has not been implemented in Qemu's libvhost-user yet, but this is something I can do if we feel the need.
Maxime
Re: [Qemu-devel] [RFC 24/29] vhost+postcopy: Lock around set_mem_table
Quoted 24 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 21 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert"<[email protected]>
**HACK - better solution needed **
We have the situation where:qemu bridge
send
set_mem_table
map memory
a) mark area with UFD
send reply with map addresses
b) start using
c) receive replyAs soon as (a) happens qemu might start seeing faults
from memory accesses (but doesn't until b); but it can't process those faults until (c) when it's received the mmap addresses.Make the fault handler spin until it gets the reply in (c).
Quoted 7 lines
Quoted 2 lines
At the very least this needs some proper locks, but preferably we need to split the message.
Yes, maybe the slave channel could be used to send the ufds with a dedicated request? The backend would set the reply-ack flag, so that it starts accessing the guest memory only when Qemu is ready to handle faults.
Yes, that would make life a lot easier.
Quoted 2 lines
Note that the slave channel support has not been implemented in Qemu's libvhost-user yet, but this is something I can do if we feel the need.
Can you tell me a bit about how the slave channel works?
Dave
Quoted 1 line
Maxime
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [RFC 24/29] vhost+postcopy: Lock around set_mem_table
Quoted 33 lines · Dr. David Alan Gilbert
Quoted 24 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 21 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert"<[email protected]>
**HACK - better solution needed **
We have the situation where:qemu bridge
send
set_mem_table
map memory
a) mark area with UFD
send reply with map addresses
b) start using
c) receive replyAs soon as (a) happens qemu might start seeing faults
from memory accesses (but doesn't until b); but it can't process those faults until (c) when it's received the mmap addresses.Make the fault handler spin until it gets the reply in (c).
Quoted 7 lines
Quoted 2 lines
At the very least this needs some proper locks, but preferably we need to split the message.
Yes, maybe the slave channel could be used to send the ufds with a dedicated request? The backend would set the reply-ack flag, so that it starts accessing the guest memory only when Qemu is ready to handle faults.
Quoted 6 lines
Yes, that would make life a lot easier.
Quoted 2 lines
Note that the slave channel support has not been implemented in Qemu's libvhost-user yet, but this is something I can do if we feel the need.
Can you tell me a bit about how the slave channel works?
When the backend advertises VHOST_USER_PROTOCOL_F_SLAVE_REQ protocol feature, Qemu creates a new channel using socketpair() and pass one of the file descriptor to the backend using a dedicated request.
Then, the backend can send requests to Qemu, using the same protocol Qemu uses to send requests to the backend. So, as "master" channel, the backend can set the VHOST_USER_F_NEED_REPLY flag to the request, so that it can wait for Qemu to ack (or nack) that the request has been handled.
It is currently only used by the backend to send IOTLB miss requests.
Note that you might be careful regarding deadlocks, as the libvhost-user is single-threaded.
More info may be found in docs/interop/vhost-user.txt (docs/specs/vhost-user.txt in older versions) Maxime
Quoted 6 lines
Dave
Quoted 1 line
Maxime
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [RFC 24/29] vhost+postcopy: Lock around set_mem_table
In reply to Dr. David Alan GilbertQuoted 33 lines · * Dr. David Alan Gilbert ([email protected]) wrote:
Quoted 24 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 21 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert"<[email protected]>
**HACK - better solution needed **
We have the situation where:qemu bridge
send
set_mem_table
map memory
a) mark area with UFD
send reply with map addresses
b) start using
c) receive replyAs soon as (a) happens qemu might start seeing faults
from memory accesses (but doesn't until b); but it can't process those faults until (c) when it's received the mmap addresses.Make the fault handler spin until it gets the reply in (c).
Quoted 7 lines
Quoted 2 lines
At the very least this needs some proper locks, but preferably we need to split the message.
Yes, maybe the slave channel could be used to send the ufds with a dedicated request? The backend would set the reply-ack flag, so that it starts accessing the guest memory only when Qemu is ready to handle faults.
Quoted 6 lines
Yes, that would make life a lot easier.
Quoted 2 lines
Note that the slave channel support has not been implemented in Qemu's libvhost-user yet, but this is something I can do if we feel the need.
Can you tell me a bit about how the slave channel works?
I've looked at the slave-channel; and I'm worried that it's not suitable for this case.
The problem is that 'slave_read' is wired to a fd_handler that I think is serviced by the main thread, and while postcopy is running I don't want to rely on the operation of the main thread (since it could be blocked by a page fault).
I could still use an explicit ack at that point though over the main channel I think (or use the slave synchronously?).
Dave
Quoted 5 lines
Dave
Quoted 1 line
Maxime
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [RFC 24/29] vhost+postcopy: Lock around set_mem_table
Quoted 39 lines · Dr. David Alan Gilbert
Quoted 38 lines · * Dr. David Alan Gilbert ([email protected]) wrote:
Quoted 24 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 21 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert"<[email protected]>
**HACK - better solution needed **
We have the situation where:qemu bridge
send
set_mem_table
map memory
a) mark area with UFD
send reply with map addresses
b) start using
c) receive replyAs soon as (a) happens qemu might start seeing faults
from memory accesses (but doesn't until b); but it can't process those faults until (c) when it's received the mmap addresses.Make the fault handler spin until it gets the reply in (c).
Quoted 7 lines
Quoted 2 lines
At the very least this needs some proper locks, but preferably we need to split the message.
Yes, maybe the slave channel could be used to send the ufds with a dedicated request? The backend would set the reply-ack flag, so that it starts accessing the guest memory only when Qemu is ready to handle faults.
Yes, that would make life a lot easier.
Quoted 2 lines
Note that the slave channel support has not been implemented in Qemu's libvhost-user yet, but this is something I can do if we feel the need.
Quoted 6 lines
Quoted 1 line
Can you tell me a bit about how the slave channel works?
I've looked at the slave-channel; and I'm worried that it's not suitable for this case.
The problem is that 'slave_read' is wired to afd_handlerthat I think is serviced by the main thread,
I confirm, this is serviced by the main thread.
Quoted 3 lines
and while postcopy is running I don't
want to rely on the operation of the main thread (since it could be blocked by a page fault).
IIUC, you mean QEMU being blocked by a page fault.
In this case, I don't think this is an issue, because QEMU doesn't rely on the backend to handle the page fault, so the slave request can be handled only once QEMU has handled the fault.
Maybe I am missing something?
Quoted 2 lines
I could still use an explicit ack at that point though over the main channel I think (or use the slave synchronously?).
Can you please elaborate, I'm not sure to understand what you mean.
Thanks,
Maxime
Quoted 10 lines
Dave
Quoted 5 lines
Dave
Quoted 1 line
Maxime
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [RFC 24/29] vhost+postcopy: Lock around set_mem_table
Quoted 48 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 39 lines · Dr. David Alan Gilbert
Quoted 38 lines · * Dr. David Alan Gilbert ([email protected]) wrote:
Quoted 24 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 21 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert"<[email protected]>
**HACK - better solution needed **
We have the situation where:qemu bridge
send
set_mem_table
map memory
a) mark area with UFD
send reply with map addresses
b) start using
c) receive replyAs soon as (a) happens qemu might start seeing faults
from memory accesses (but doesn't until b); but it can't process those faults until (c) when it's received the mmap addresses.Make the fault handler spin until it gets the reply in (c).
Quoted 7 lines
Quoted 2 lines
At the very least this needs some proper locks, but preferably we need to split the message.
Yes, maybe the slave channel could be used to send the ufds with a dedicated request? The backend would set the reply-ack flag, so that it starts accessing the guest memory only when Qemu is ready to handle faults.
Yes, that would make life a lot easier.
Quoted 2 lines
Note that the slave channel support has not been implemented in Qemu's libvhost-user yet, but this is something I can do if we feel the need.
Quoted 6 lines
Quoted 1 line
Can you tell me a bit about how the slave channel works?
I've looked at the slave-channel; and I'm worried that it's not suitable for this case.
The problem is that 'slave_read' is wired to afd_handlerthat I think is serviced by the main thread,
Quoted 7 lines
I confirm, this is serviced by the main thread.
Quoted 3 lines
and while postcopy is running I don't
want to rely on the operation of the main thread (since it could be blocked by a page fault).IIUC, you mean QEMU being blocked by a page fault.
Yes.
Quoted 5 lines
In this case, I don't think this is an issue, because QEMU doesn't rely on the backend to handle the page fault, so the slave request can be handled only once QEMU has handled the fault.
Maybe I am missing something?
It feels delicate; with the vhost client blocked waiting for the ack from the qemu to the registration reply on the slave, and some other part blocked by a page fault, it makes it sound likely to hit deadlocks even if I can't put my finger on one.
Quoted 4 lines
Quoted 2 lines
I could still use an explicit ack at that point though over the main channel I think (or use the slave synchronously?).
Can you please elaborate, I'm not sure to understand what you mean.
In the world I'm currently working on I've got it just using the main channel but:
settable -> client
settable-results -> qemu
ack -> client
all over the main channel with each side waiting for the other.
Dave
Quoted 14 lines
Thanks,
MaximeQuoted 10 lines
Dave
Quoted 5 lines
Dave
Quoted 1 line
Maxime
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [RFC 24/29] vhost+postcopy: Lock around set_mem_table
Quoted 63 lines · Dr. David Alan Gilbert
Quoted 48 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 39 lines · Dr. David Alan Gilbert
Quoted 38 lines · * Dr. David Alan Gilbert ([email protected]) wrote:
Quoted 24 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 21 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert"<[email protected]>
**HACK - better solution needed **
We have the situation where:qemu bridge
send
set_mem_table
map memory
a) mark area with UFD
send reply with map addresses
b) start using
c) receive replyAs soon as (a) happens qemu might start seeing faults
from memory accesses (but doesn't until b); but it can't process those faults until (c) when it's received the mmap addresses.Make the fault handler spin until it gets the reply in (c).
Quoted 7 lines
Quoted 2 lines
At the very least this needs some proper locks, but preferably we need to split the message.
Yes, maybe the slave channel could be used to send the ufds with a dedicated request? The backend would set the reply-ack flag, so that it starts accessing the guest memory only when Qemu is ready to handle faults.
Yes, that would make life a lot easier.
Quoted 2 lines
Note that the slave channel support has not been implemented in Qemu's libvhost-user yet, but this is something I can do if we feel the need.
Quoted 6 lines
Quoted 1 line
Can you tell me a bit about how the slave channel works?
I've looked at the slave-channel; and I'm worried that it's not suitable for this case.
The problem is that 'slave_read' is wired to afd_handlerthat I think is serviced by the main thread,Quoted 7 lines
I confirm, this is serviced by the main thread.
Quoted 3 lines
and while postcopy is running I don't
want to rely on the operation of the main thread (since it could be blocked by a page fault).IIUC, you mean QEMU being blocked by a page fault.
Yes.
Quoted 4 lines
In this case, I don't think this is an issue, because QEMU doesn't rely on the backend to handle the page fault, so the slave request can be handled only once QEMU has handled the fault.
Quoted 6 lines
Quoted 1 line
Maybe I am missing something?
It feels delicate; with the vhost client blocked waiting for the ack from the qemu to the registration reply on the slave, and some other part blocked by a page fault, it makes it sound likely to hit deadlocks even if I can't put my finger on one.
Right, it is hard to be sure there is no risk of deadlock.
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
I could still use an explicit ack at that point though over the main channel I think (or use the slave synchronously?).
Can you please elaborate, I'm not sure to understand what you mean.
Quoted 7 lines
In the world I'm currently working on I've got it just using the main channel but:
settable -> client settable-results -> qemu ack -> clientall over the main channel with each side waiting for the other.
Ok, thanks for the clarification.
Maxime
Quoted 19 lines
Dave
Quoted 14 lines
Thanks,
MaximeQuoted 10 lines
Dave
Quoted 5 lines
Dave
Quoted 1 line
Maxime
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK--
Dr. David Alan Gilbert / [email protected] / Manchester, UK--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 25/29 vhu: enable = false on get_vring_base
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
When we receive a GET_VRING_BASE message set enable = false to stop any new received packets modifying the ring.
--- contrib/libvhost-user/libvhost-user.c | 1 + 1 file changed, 1 insertion(+)
contrib/libvhost-user/libvhost-user.c +1 -0
@@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg) vmsg->size =sizeof (vmsg->payload.state); dev->vq[index].started = false; + dev->vq[index].enable = false;if (dev->iface->queue_set_started) { dev->iface->queue_set_started (dev, index, false); }
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base
Quoted 6 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
When we receive a
GET_VRING_BASEmessage set enable = false to stop any new received packets modifying the ring.Signed-off-by Dr. David Alan Gilbert <[email protected]>
Maxime
Quoted 17 lines
--- contrib/libvhost-user/libvhost-user.c | 1 + 1 file changed, 1 insertion(+)contrib/libvhost-user/libvhost-user.c +1
-0@@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg) vmsg->size =sizeof (vmsg->payload.state); dev->vq[index].started = false; + dev->vq[index].enable = false;if (dev->iface->queue_set_started) { dev->iface->queue_set_started (dev, index, false); }
Re: [Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base
In reply to Dr. David Alan Gilbert (git)Quoted 6 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
When we receive a
GET_VRING_BASEmessage set enable = false to stop any new received packets modifying the ring.Signed-off-by Dr. David Alan Gilbert <[email protected]>
I think I already reviewed a similar patch. Spec says:
Client must only process each ring when it is started.
IMHO the real fix is to fix client to check the started flag before processing the ring.
Quoted 18 lines
--- contrib/libvhost-user/libvhost-user.c | 1 + 1 file changed, 1 insertion(+)contrib/libvhost-user/libvhost-user.c +1
-0@@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg) vmsg->size =sizeof (vmsg->payload.state); dev->vq[index].started = false; + dev->vq[index].enable = false;if (dev->iface->queue_set_started) { dev->iface->queue_set_started (dev, index, false); }Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base
Quoted 9 lines · * Michael S. Tsirkin ([email protected]) wrote:
Quoted 6 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
When we receive a
GET_VRING_BASEmessage set enable = false to stop any new received packets modifying the ring.Signed-off-by Dr. David Alan Gilbert <[email protected]>I think I already reviewed a similar patch.
Yes you replied to my off-list mail; I hadn't got around to fixing it yet.
Quoted 2 lines
Spec says:
Client must only process each ring when it is started.
but in that reply you said the spec said
Client must only pass data between the ring and the backend, when the ring is enabled.
So does the spec say 'started' or 'enabled'
(Pointer to the spec?)
Quoted 2 lines
IMHO the real fix is to fix client to check the started flag before processing the ring.
Yep I can do that. I was curious however whether it was specified as 'started' or 'enabled' or both.
Dave
Quoted 11 lines
Quoted 11 lines
--- contrib/libvhost-user/libvhost-user.c | 1 + 1 file changed, 1 insertion(+)contrib/libvhost-user/libvhost-user.c +0
-0@@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg) vmsg->size =sizeof (vmsg->payload.state);
Quoted 7 lines
Quoted 7 lines
contrib/libvhost-user/libvhost-user.c +1
-0dev->vq[index].started = false; + dev->vq[index].enable = false;if (dev->iface->queue_set_started) { dev->iface->queue_set_started (dev, index, false); }Signature · 1 line
2.13.0
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base
Quoted 17 lines · Dr. David Alan Gilbert
Quoted 9 lines · * Michael S. Tsirkin ([email protected]) wrote:
Quoted 6 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
When we receive a
GET_VRING_BASEmessage set enable = false to stop any new received packets modifying the ring.Signed-off-by Dr. David Alan Gilbert <[email protected]>I think I already reviewed a similar patch.
Yes you replied to my off-list mail; I hadn't got
around to fixing it yet.Quoted 2 lines
Spec says:
Client must only process each ring when it is started.
Quoted 6 lines
but in that reply you said the spec said
Client must only pass data between the ring and the backend, when the ring is enabled.
So does the spec say 'started' or 'enabled'
Both. Ring processing is limited by ring being started. Passing actual data - by ring also being enabled. With ring started but disabled you drop all packets.
Quoted 1 line
(Pointer to the spec?)
It's part of QEMU source:docs/interop/vhost-user.txt
Quoted 28 lines
Quoted 2 lines
IMHO the real fix is to fix client to check the started flag before processing the ring.
Yep I can do that. I was curious however whether it was specified as 'started' or 'enabled' or both.
Dave
Quoted 11 lines
Quoted 11 lines
--- contrib/libvhost-user/libvhost-user.c | 1 + 1 file changed, 1 insertion(+)contrib/libvhost-user/libvhost-user.c +0
-0@@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg) vmsg->size =sizeof (vmsg->payload.state);Quoted 7 lines
Quoted 7 lines
contrib/libvhost-user/libvhost-user.c +1
-0dev->vq[index].started = false; + dev->vq[index].enable = false;if (dev->iface->queue_set_started) { dev->iface->queue_set_started (dev, index, false); }Signature · 1 line
2.13.0--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base
In reply to Michael S. TsirkinQuoted 6 lines · * Michael S. Tsirkin ([email protected]) wrote:
Quoted 5 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
When we receive a
GET_VRING_BASEmessage set enable = false to stop any new received packets modifying the ring.
Quoted 8 lines
Quoted 1 line
Signed-off-by Dr. David Alan Gilbert <[email protected]>I think I already reviewed a similar patch. Spec says:
Client must only process each ring when it is started.IMHO the real fix is to fix client to check the started flag before processing the ring.
Done, I added a vu_queue_started to match vu_queue_enabled and then used it.
Dave
Quoted 11 lines
Quoted 11 lines
--- contrib/libvhost-user/libvhost-user.c | 1 + 1 file changed, 1 insertion(+)contrib/libvhost-user/libvhost-user.c +0
-0@@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg) vmsg->size =sizeof (vmsg->payload.state);
Quoted 7 lines
Quoted 7 lines
contrib/libvhost-user/libvhost-user.c +1
-0dev->vq[index].started = false; + dev->vq[index].enable = false;if (dev->iface->queue_set_started) { dev->iface->queue_set_started (dev, index, false); }Signature · 1 line
2.13.0
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 26/29 vhost: Add VHOST_USER_POSTCOPY_END message
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
This message is sent just before the end of postcopy to get the client to stop using userfault since we wont respond to any more requests. It should close userfaultfd so that any other pages get mapped to the backing file automatically by the kernel, since at this point we know we've received everything.
--- contrib/libvhost-user/libvhost-user.c | 23 +++++++++++++++++++++++ contrib/libvhost-user/libvhost-user.h | 1 + hw/virtio/vhost-user.c | 1 + 3 files changed, 25 insertions(+)
contrib/libvhost-user/libvhost-user.c +23 -0
@@ -68,6 +68,7 @@ vu_request_to_string(int req)REQ (VHOST_USER_INPUT_GET_CONFIG ),REQ (VHOST_USER_POSTCOPY_ADVISE ),REQ (VHOST_USER_POSTCOPY_LISTEN ), +REQ (VHOST_USER_POSTCOPY_END ), REQ(VHOST_USER_MAX), };#undef REQ
@@ -889,6 +890,26 @@ vu_set_postcopy_listen(VuDev *dev, VhostUserMsg *vmsg)return false; } + +static bool +vu_set_postcopy_end (VuDev *dev,VhostUserMsg *vmsg) +{ +fprintf (stderr,"%s: Entry\n" , __func__); + dev->postcopy_listening = false; +if (dev->postcopy_ufd >0 ) { +close (dev->postcopy_ufd); + dev->postcopy_ufd =-1 ; +fprintf (stderr,"%s: Done close\n" , __func__); + } + + vmsg->fd_num =0 ; + vmsg->payload.u64 =0 ; + vmsg->size =sizeof (vmsg->payload.u64); + vmsg->flags =VHOST_USER_VERSION |VHOST_USER_REPLY_MASK ; +fprintf (stderr,"%s: exit\n" , __func__); +return true; +} +static bool vu_process_message (VuDev *dev,VhostUserMsg *vmsg) {
@@ -956,6 +977,8 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)return vu_set_postcopy_advise (dev, vmsg);case VHOST_USER_POSTCOPY_LISTEN :return vu_set_postcopy_listen (dev, vmsg); +case VHOST_USER_POSTCOPY_END : +return vu_set_postcopy_end (dev, vmsg);default :vmsg_close_fds (vmsg);vu_panic (dev,"Unhandled request: %d" , vmsg->request);
contrib/libvhost-user/libvhost-user.h +1 -0
@@ -65,6 +65,7 @@ typedef enum VhostUserRequest {VHOST_USER_INPUT_GET_CONFIG =20 ,VHOST_USER_POSTCOPY_ADVISE =23 ,VHOST_USER_POSTCOPY_LISTEN =24 , +VHOST_USER_POSTCOPY_END =25 , VHOST_USER_MAX } VhostUserRequest;
hw/virtio/vhost-user.c +1 -0
@@ -69,6 +69,7 @@ typedef enum VhostUserRequest {VHOST_USER_IOTLB_MSG =22 ,VHOST_USER_POSTCOPY_ADVISE =23 ,VHOST_USER_POSTCOPY_LISTEN =24 , +VHOST_USER_POSTCOPY_END =25 , VHOST_USER_MAX } VhostUserRequest;
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 26/29] vhost: Add VHOST_USER_POSTCOPY_END message
Quoted 41 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
This message is sent just before the end of postcopy to get the client to stop using userfault since we wont respond to any more requests. It should close userfaultfd so that any other pages get mapped to the backing file automatically by the kernel, since at this point we know we've received everything.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 23 +++++++++++++++++++++++ contrib/libvhost-user/libvhost-user.h | 1 + hw/virtio/vhost-user.c | 1 + 3 files changed, 25 insertions(+)contrib/libvhost-user/libvhost-user.c +11
-0@@ -68,6 +68,7 @@ vu_request_to_string(int req)REQ (VHOST_USER_INPUT_GET_CONFIG ),REQ (VHOST_USER_POSTCOPY_ADVISE ),REQ (VHOST_USER_POSTCOPY_LISTEN ), +REQ (VHOST_USER_POSTCOPY_END ), REQ(VHOST_USER_MAX), };#undef REQ@@ -889,6 +890,26 @@ vu_set_postcopy_listen(VuDev *dev, VhostUserMsg *vmsg)return false; } + +static bool +vu_set_postcopy_end (VuDev *dev,VhostUserMsg *vmsg) +{ +fprintf (stderr,"%s: Entry\n" , __func__); + dev->postcopy_listening = false; +if (dev->postcopy_ufd >0 ) { +close (dev->postcopy_ufd); + dev->postcopy_ufd =-1 ; +fprintf (stderr,"%s: Done close\n" , __func__);
contrib/libvhost-user/libvhost-user.c vu_set_postcopy_listen(VuDev *dev, VhostUserMsg *vmsg) · line 912
Quoted 10 lines
contrib/libvhost-user/libvhost-user.c +10
-0+ } + + vmsg->fd_num =0 ; + vmsg->payload.u64 =0 ; + vmsg->size =sizeof (vmsg->payload.u64); + vmsg->flags =VHOST_USER_VERSION |VHOST_USER_REPLY_MASK ; +fprintf (stderr,"%s: exit\n" , __func__); +return true; +} +
It is what reply-ack is done for, so to avoid code duplication, maybe Qemu could set VHOST_USER_NEED_REPLY_MASK bit when reply-ack feature is supported.
I'm wondering if we shouldn't consider adding reply-ack feature support to libvhost-user, and make postcopy support to depend on this feature.
Cheers,
Maxime
Re: [Qemu-devel] [RFC 26/29] vhost: Add VHOST_USER_POSTCOPY_END message
Quoted 58 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 40 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
This message is sent just before the end of postcopy to get the client to stop using userfault since we wont respond to any more requests. It should close userfaultfd so that any other pages get mapped to the backing file automatically by the kernel, since at this point we know we've received everything.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 23 +++++++++++++++++++++++ contrib/libvhost-user/libvhost-user.h | 1 + hw/virtio/vhost-user.c | 1 + 3 files changed, 25 insertions(+)contrib/libvhost-user/libvhost-user.c +11
-0@@ -68,6 +68,7 @@ vu_request_to_string(int req)REQ (VHOST_USER_INPUT_GET_CONFIG ),REQ (VHOST_USER_POSTCOPY_ADVISE ),REQ (VHOST_USER_POSTCOPY_LISTEN ), +REQ (VHOST_USER_POSTCOPY_END ), REQ(VHOST_USER_MAX), };#undef REQ@@ -889,6 +890,26 @@ vu_set_postcopy_listen(VuDev *dev, VhostUserMsg *vmsg)return false; } + +static bool +vu_set_postcopy_end (VuDev *dev,VhostUserMsg *vmsg) +{ +fprintf (stderr,"%s: Entry\n" , __func__); + dev->postcopy_listening = false; +if (dev->postcopy_ufd >0 ) { +close (dev->postcopy_ufd); + dev->postcopy_ufd =-1 ; +fprintf (stderr,"%s: Done close\n" , __func__);contrib/libvhost-user/libvhost-user.c vu_set_postcopy_listen(VuDev *dev, VhostUserMsg *vmsg) · line 911
Quoted 10 lines
contrib/libvhost-user/libvhost-user.c +10
-0+ } + + vmsg->fd_num =0 ; + vmsg->payload.u64 =0 ; + vmsg->size =sizeof (vmsg->payload.u64); + vmsg->flags =VHOST_USER_VERSION |VHOST_USER_REPLY_MASK ; +fprintf (stderr,"%s: exit\n" , __func__); +return true; +} +It is what reply-ack is done for, so to avoid code duplication, maybe Qemu could set
VHOST_USER_NEED_REPLY_MASKbit when reply-ack feature is supported.
Quoted 2 lines
I'm wondering if we shouldn't consider adding reply-ack feature support to libvhost-user, and make postcopy support to depend on this feature.
Yes, that would make sense; for the moment I'm adding what I think is the same replies in the places I need ack's, and setting the VHOST_USER_NEED_REPLY_MASK so for the messages where I need it the semantics should be the same as that.
Dave
Quoted 2 lines
Cheers,
Maxime
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
RFC 27/29 vhost+postcopy: Wire up POSTCOPY_END notify
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Wire up a call to VHOST_USER_POSTCOPY_END message to the vhost clients right before we ask the listener thread to shutdown.
--- hw/virtio/trace-events | 2 ++ hw/virtio/vhost-user.c | 30 ++++++++++++++++++++++++++++++ migration/postcopy-ram.c | 5 +++++ migration/postcopy-ram.h | 1 + 4 files changed, 38 insertions(+)
hw/virtio/trace-events +2 -0
@@ -1,6 +1,8 @@ # See docs/tracing.txt for syntax documentation. # hw/virtio/vhost-user.c +vhost_user_postcopy_end_entry(void) "" +vhost_user_postcopy_end_exit(void) "" vhost_user_postcopy_fault_handler(const char *name, uint64_t fault_address, int nregions) "%s: @%"PRIx64" nregions:%d" vhost_user_postcopy_fault_handler_loop(int i, uint64_t client_base, uint64_t size) "%d: client%"PRIx64" +%"PRIx64 vhost_user_postcopy_fault_handler_found(int i, uint64_t region_offset, uint64_t rb_offset) "%d: region_offset: %"PRIx64" rb_offset:%"PRIx64
hw/virtio/vhost-user.c +30 -0
@@ -921,6 +921,33 @@ static int vhost_user_postcopy_listen(struct vhost_dev *dev, Error **errp)return 0 ; } +/* +* Called at the end of postcopy +*/ +static int vhost_user_postcopy_end (struct vhost_dev *dev,Error **errp) +{ +VhostUserMsg msg = { + .request =VHOST_USER_POSTCOPY_END , + .flags =VHOST_USER_VERSION |VHOST_USER_NEED_REPLY_MASK , + }; +int ret; + +trace_vhost_user_postcopy_end_entry (); +if (vhost_user_write (dev, &msg,NULL ,0 ) <0 ) { +error_setg (errp,"Failed to send postcopy_end to vhost" ); +return -1 ; + } + + ret =process_message_reply (dev, &msg); +if (ret) { +error_setg (errp,"Failed to receive reply to postcopy_end" ); +return ret; + } +trace_vhost_user_postcopy_end_exit (); + +return 0 ; +} +static int vhost_user_postcopy_notifier (NotifierWithReturn *notifier,void *opaque) {
@@ -946,6 +973,9 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,case POSTCOPY_NOTIFY_INBOUND_LISTEN :return vhost_user_postcopy_listen (dev, pnd->errp); +case POSTCOPY_NOTIFY_INBOUND_END : +return vhost_user_postcopy_end (dev, pnd->errp); +default :/* We ignore notifications we don't know */ break ;
migration/postcopy-ram.c +5 -0
@@ -337,7 +337,12 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)if (mis->have_fault_thread) {uint64_t tmp64; +Error *local_err =NULL ; +if (postcopy_notify (POSTCOPY_NOTIFY_INBOUND_END , &local_err)) { +error_report_err (local_err); +return -1 ; + }if (qemu_ram_foreach_block (cleanup_range, mis)) {return -1 ; }
migration/postcopy-ram.h +1 -0
@@ -130,6 +130,7 @@ enum PostcopyNotifyReason {POSTCOPY_NOTIFY_PROBE =0 ,POSTCOPY_NOTIFY_INBOUND_ADVISE ,POSTCOPY_NOTIFY_INBOUND_LISTEN , + POSTCOPY_NOTIFY_INBOUND_END, }; struct PostcopyNotifyData {
Signature · 1 line
2.13.0
RFC 28/29 postcopy: Allow shared memory
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Now that we have the mechanisms in here, allow shared memory in a postcopy.
--- migration/postcopy-ram.c | 6 ------ 1 file changed, 6 deletions(-)
migration/postcopy-ram.c +0 -6
@@ -138,12 +138,6 @@ static int test_ramblock_postcopiable(const char *block_name, void *host_addr,RAMBlock *rb =qemu_ram_block_by_name (block_name);size_t pagesize =qemu_ram_pagesize (rb); -if (qemu_ram_is_shared (rb)) { -error_report ("Postcopy on shared RAM (%s) is not yet supported" , - block_name); -return 1 ; - } -if (length % pagesize) {error_report ("Postcopy requires RAM blocks to be a page size multiple," " block %s is 0x" RAM_ADDR_FMT " bytes with a "
Signature · 1 line
2.13.0
RFC 29/29 vhost-user: Claim support for postcopy
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Tell QEMU we understand the protocol features needed for postcopy.
--- contrib/libvhost-user/libvhost-user.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
contrib/libvhost-user/libvhost-user.c +2 -1
@@ -797,7 +797,8 @@ vu_set_vring_err_exec(VuDev *dev, VhostUserMsg *vmsg)static bool vu_get_protocol_features_exec (VuDev *dev,VhostUserMsg *vmsg) { -uint64_t features =1ULL <<VHOST_USER_PROTOCOL_F_LOG_SHMFD ; +uint64_t features =1ULL <<VHOST_USER_PROTOCOL_F_LOG_SHMFD | +1ULL <<VHOST_USER_PROTOCOL_F_POSTCOPY ;if (dev->iface->get_protocol_features) { features |= dev->iface->get_protocol_features (dev);
Signature · 1 line
2.13.0
Re: [Qemu-devel] [RFC 29/29] vhost-user: Claim support for postcopy
Quoted 14 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Tell QEMU we understand the protocol features needed for postcopy.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)contrib/libvhost-user/libvhost-user.c +0
-0
contrib/libvhost-user/libvhost-user.c vu_set_vring_err_exec(VuDev *dev, VhostUserMsg *vmsg) · line 801
Quoted 6 lines
contrib/libvhost-user/libvhost-user.c +2
-1@@ -797,7 +797,8 @@ vu_set_vring_err_exec(VuDev *dev, VhostUserMsg *vmsg)static bool vu_get_protocol_features_exec (VuDev *dev,VhostUserMsg *vmsg) { -uint64_t features =1ULL <<VHOST_USER_PROTOCOL_F_LOG_SHMFD ; +uint64_t features =1ULL <<VHOST_USER_PROTOCOL_F_LOG_SHMFD | +1ULL <<VHOST_USER_PROTOCOL_F_POSTCOPY ;
Maybe advertising VHOST_USER_PROTOCOL_F_POSTCOPY could be done only if userfaultfd syscall is supported?
Quoted 4 lines
contrib/libvhost-user/libvhost-user.c +0
-0if (dev->iface->get_protocol_features) { features |= dev->iface->get_protocol_features (dev);
Re: [Qemu-devel] [RFC 29/29] vhost-user: Claim support for postcopy
Quoted 12 lines · * Maxime Coquelin ([email protected]) wrote:
Quoted 9 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Tell QEMU we understand the protocol features needed for postcopy.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- contrib/libvhost-user/libvhost-user.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Quoted 14 lines
contrib/libvhost-user/libvhost-user.c vu_set_vring_err_exec(VuDev *dev, VhostUserMsg *vmsg) · line 801
Quoted 11 lines
contrib/libvhost-user/libvhost-user.c +2
-1@@ -797,7 +797,8 @@ vu_set_vring_err_exec(VuDev *dev, VhostUserMsg *vmsg)static bool vu_get_protocol_features_exec (VuDev *dev,VhostUserMsg *vmsg) { -uint64_t features =1ULL <<VHOST_USER_PROTOCOL_F_LOG_SHMFD ; +uint64_t features =1ULL <<VHOST_USER_PROTOCOL_F_LOG_SHMFD | +1ULL <<VHOST_USER_PROTOCOL_F_POSTCOPY ;Maybe advertising
VHOST_USER_PROTOCOL_F_POSTCOPYcould be done only if userfaultfd syscall is supported?
Done.
Dave
Quoted 3 lines
contrib/libvhost-user/libvhost-user.c vu_set_vring_err_exec(VuDev *dev, VhostUserMsg *vmsg)
Quoted 3 lines
contrib/libvhost-user/libvhost-user.c +0
-0if (dev->iface->get_protocol_features) { features |= dev->iface->get_protocol_features (dev);
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Quoted 8 lines · * Dr. David Alan Gilbert (git) ([email protected]) wrote:
From: "Dr. David Alan Gilbert" <[email protected]>
Hi,
This is a RFC/WIP series that enables postcopy migration
with shared memory to a vhost-user process.
It's based off current-head + Juan'sload_cleanupseries, and Alexey's bitmap series (v4). It's very lightly tested and seems to work, but it's quite rough.
Marc-André asked if I had a git with it all applied; so here we are:
https://github.com/dagrh/qemu/commits/vhost
[email protected]:dagrh/qemu.git on the vhost branch
Dave
Quoted 123 lines
I've modified the vhost-user-bridge (aka vhub) in qemu's tests/ to use the new feature, since this is about the simplest client around.
Structure:
The basic idea is that near the start of postcopy, the client opens its own userfaultfd fd and sends that back to QEMU over the socket it's already using for VHUST_USER_* commands. Then when
VHOST_USER_SET_MEM_TABLEarrives it registers the areas with userfaultfd and sends the mapped addresses back to QEMU.QEMU then reads the clients UFD in it's fault thread and issues requests back to the source as needed.
QEMU also issues 'WAKE' ioctls on the UFD to let the client know that the page has arrived and can carry on.A new feature (
VHOST_USER_PROTOCOL_F_POSTCOPY) is added so that the QEMU knows the client can talk postcopy.
Three new messages (VHOST_USER_POSTCOPY_{ADVISE/LISTEN/END}) are added to guide the process along.Current known issues:
I've not tested it with hugepages yet; and I suspect the madvises will need tweaking for it.The qemu gets to see the base addresses that the client has its regions mapped at; that's not great for security
Take care of deadlocking; any thread in the client that accesses a userfault protected page can stall.
There's a nasty hack of a lock around the
set_mem_tablemessage.I've not looked at the recent IOMMU code.
Some cleanup and a lot of corner cases need thinking about.
There are probably plenty of unknown issues as well.
Test setup:
I'm running on one host at the moment, with the guest scping a large file from the host as it migrates. The setup is based on one I found in the vhost-user setups. You'll need a recent kernel for the shared memory support in userfaultfd, and userfault isn't that happy if a process using shared memory core's - so make sure you have the latest fixes.SESS=vhost
ulimit -c unlimited
tmux -L $SESS new-session -d
tmux -L $SESS set-option -g history-limit 30000
# Start a router using the system qemu
tmux -L $SESS new-window -n router ./x86_64-softmmu/qemu-system-x86_64 -M none -nographic -net socket,vlan=0,udp=loca
lhost:4444,localaddr=localhost:5555 -net socket,vlan=0,udp=localhost:4445,localaddr=localhost:5556 -net user,vlan=0
tmux -L $SESS set-option -g set-remain-on-exit on
# Start source vhost bridge
tmux -L $SESS new-window -n srcvhostbr "./tests/vhost-user-bridge -u/tmp/vubrsrc.sock2>src-vub-log"
sleep 0.5
tmux -L $SESS new-window -n source "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backe
nd-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/
tmp/vubrsrc.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :0 -monitor stdio -trace events=/root/trace-file 2>src-qemu-log " # Start dest vhost bridge
tmux -L $SESS new-window -n destvhostbr "./tests/vhost-user-bridge -u/tmp/vubrdst.sock-l 127.0.0.1:4445 -r 127.0.0.
1:5556 2>dst-vub-log"
sleep 0.5
tmux -L $SESS new-window -n dest "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backend
-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/tm
p/vubrdst.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :1 -monitor stdio -incoming tcp::8888 -trace events=/root/trace-file 2>dst-qemu-log" tmux -L $SESS send-keys -t source "migrate_set_capabilitypostcopy-ram on
tmux -L $SESS send-keys -t source "migrate_set_speed20M
tmux -L $SESS send-keys -t dest "migrate_set_capabilitypostcopy-ram onthen once booted:
tmux -L vhost send-keys -t source 'migrate -d tcp:0:8888^M' tmux -L vhost send-keys -t source 'migrate_start_postcopy^M' (Note those ^M's are actual ctrl-M's i.e. ctrl-v ctrl-M)Dave
Dr. David Alan Gilbert (29):
RAMBlock/migration: Add migration flags
migrate: Updateram_block_discard_rangefor sharedqemu_ram_block_host_offset
migration/ram:ramblock_recv_bitmap_test_byte_offset
postcopy: useUFFDIO_ZEROPAGEonly when available
postcopy: Add notifier chain
postcopy: Add vhost-user flag for postcopy and check it
vhost-user: Add 'VHOST_USER_POSTCOPY_ADVISE' message
vhub: Support sending fds back to qemu
vhub: Open userfaultfd
postcopy: Allow registering of fd handler vhost+postcopy: Register shared ufd with postcopy vhost+postcopy: Transmit 'listen' to client vhost+postcopy: Register new regions with the ufd vhost+postcopy: Send address back to qemu vhost+postcopy: Stash RAMBlock and offset vhost+postcopy: Send requests to source for shared pages vhost+postcopy: Resolve client address
postcopy: wake shared
postcopy:postcopy_notify_shared_wakevhost+postcopy: Add vhost waker
vhost+postcopy: Call wakeups
vub+postcopy: madvises
vhost+postcopy: Lock aroundset_mem_table
vhu: enable = false onget_vring_base
vhost: AddVHOST_USER_POSTCOPY_ENDmessage vhost+postcopy: Wire upPOSTCOPY_ENDnotify
postcopy: Allow shared memory
vhost-user: Claim support for postcopycontrib/libvhost-user/libvhost-user.c | 178 ++++++++++++++++- contrib/libvhost-user/libvhost-user.h | 8 + exec.c | 44 +++-- hw/virtio/trace-events | 13 ++ hw/virtio/vhost-user.c | 293 +++++++++++++++++++++++++++- include/exec/cpu-common.h | 3 + include/exec/ram_addr.h | 2 + migration/migration.c | 3 + migration/migration.h | 8 + migration/postcopy-ram.c | 357 +++++++++++++++++++++++++++-------
Quoted 13 lines
migration/postcopy-ram.h | 69 +++++++ migration/ram.c | 5 + migration/ram.h | 1 + migration/savevm.c | 13 ++ migration/trace-events | 6 + trace-events | 3 + vl.c | 4 +- 17 files changed, 926 insertions(+), 84 deletions(-)Signature · 1 line
2.13.0
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Hi
Quoted 10 lines · Dr. David Alan Gilbert
Quoted 8 lines · * Dr. David Alan Gilbert (git) ([email protected]) wrote:
From: "Dr. David Alan Gilbert" <[email protected]>
Hi,
This is a RFC/WIP series that enables postcopy migration
with shared memory to a vhost-user process.
It's based off current-head + Juan'sload_cleanupseries, and Alexey's bitmap series (v4). It's very lightly tested and seems to work, but it's quite rough.
Quoted 5 lines
Marc-André asked if I had a git with it all applied; so here we are:
https://github.com/dagrh/qemu/commits/vhost
[email protected]:dagrh/qemu.git on the vhost branch
I started looking at the series, but I am not familiar with ufd/postcopy. Could you update vhost-user.txt to describe the new messages? Otherwise, make check hangs in /x86_64/vhost-user/connect-fail (might be an unrelated regression?) Thanks
Signature · 1 line
Marc-André Lureau
Quoted 15 lines · * Marc-André Lureau ([email protected]) wrote:
Hi
Quoted 10 lines · Dr. David Alan Gilbert
Quoted 8 lines · * Dr. David Alan Gilbert (git) ([email protected]) wrote:
From: "Dr. David Alan Gilbert" <[email protected]>
Hi,
This is a RFC/WIP series that enables postcopy migration
with shared memory to a vhost-user process.
It's based off current-head + Juan'sload_cleanupseries, and Alexey's bitmap series (v4). It's very lightly tested and seems to work, but it's quite rough.
Quoted 6 lines
Quoted 5 lines
Marc-André asked if I had a git with it all applied; so here we are:
https://github.com/dagrh/qemu/commits/vhost
[email protected]:dagrh/qemu.git on the vhost branchI started looking at the series, but I am not familiar with ufd/postcopy.
I'm similarly unfamiliar with the vhost code when I started this (which probably shows!).
The main thing about ufd is that a process registers with the ufd system and registers an area of memory with it; accesses to the memory block until the page is available, a message is sent down the ufd, and whoever receives that message may then respond by atomically copying a page into memory, or wakeing the process when it knows the page is there. This is the first time we've tried to use userfaultfd with shared memory and it does need a very recent kernel for it (4.11.0 or rhel 7.4 beta)
Quoted 1 line
Could you update vhost-user.txt to describe the new messages?
See below; I'll add that in.
Quoted 3 lines
Otherwise,
make check hangs in /x86_64/vhost-user/connect-fail (might be an unrelated regression?) Thanks
Entirely possible I broke it; I'll have a look - at the moment I'm more interested in comments on the structure of this set.
Dave
docs/interop/vhost-user.txt +37 -0
@@ -273,6 +273,14 @@ Once the source has finished migration, rings will be stopped by the source. No further update must be done before rings are restarted. +In postcopy migration the slave is started before all the memory has been +received from the source host, and care must be taken to avoid accessing pages +that have yet to be received. The slave opens a 'userfault'-fd and registers +the memory with it; this fd is then passed back over to the master. +The master services requests on the userfaultfd for pages that are accessed +and when the page is available it performs WAKE ioctl's on the userfaultfd +to wake the stalled slave. + IOMMU support -------------
@@ -326,6 +334,7 @@ Protocol features #define VHOST_USER_PROTOCOL_F_REPLY_ACK 3 #define VHOST_USER_PROTOCOL_F_MTU 4 #define VHOST_USER_PROTOCOL_F_SLAVE_REQ 5 +#define VHOST_USER_PROTOCOL_F_POSTCOPY 6 Master message types --------------------
@@ -402,12 +411,17 @@ Master message types Id: 5 Equivalent ioctl: VHOST_SET_MEM_TABLE Master payload: memory regions description + Slave payload: (postcopy only) memory regions description Sets the memory map regions on the slave so it can translate the vring addresses. In the ancillary data there is an array of file descriptors for each memory mapped region. The size and ordering of the fds matches the number and ordering of memory regions. + When postcopy-listening has been received, SET_MEM_TABLE replies with + the bases of the memory mapped regions to the master. It must have mmap'd + the regions and enabled userfaultfd on them. + * VHOST_USER_SET_LOG_BASE Id: 6
@@ -580,6 +594,29 @@ Master message types This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature has been successfully negotiated. + * VHOST_USER_POSTCOPY_ADVISE + Id: 23 + Master payload: N/A + Slave payload: userfault fd + u64 + + Master advises slave that a migration with postcopy enabled is underway, + the slave must open a userfaultfd for later use. + Note that at this stage the migration is still in precopy mode. + + * VHOST_USER_POSTCOPY_LISTEN + Id: 24 + Master payload: N/A + + Master advises slave that a transition to postcopy mode has happened. + + * VHOST_USER_POSTCOPY_END + Id: 25 + Slave payload: u64 + + Master advises that postcopy migration has now completed. The + slave must disable the userfaultfd. The response is an acknowledgement + only. + Slave message types -------------------
Quoted 2 lines
Signature · 1 line
Marc-André Lureau
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Quoted 20 lines · * Marc-André Lureau ([email protected]) wrote:
Hi
Quoted 10 lines · Dr. David Alan Gilbert
Quoted 8 lines · * Dr. David Alan Gilbert (git) ([email protected]) wrote:
From: "Dr. David Alan Gilbert" <[email protected]>
Hi,
This is a RFC/WIP series that enables postcopy migration
with shared memory to a vhost-user process.
It's based off current-head + Juan'sload_cleanupseries, and Alexey's bitmap series (v4). It's very lightly tested and seems to work, but it's quite rough.Quoted 5 lines
Marc-André asked if I had a git with it all applied; so here we are:
https://github.com/dagrh/qemu/commits/vhost
[email protected]:dagrh/qemu.git on the vhost branch
Quoted 4 lines
I started looking at the series, but I am not familiar with ufd/postcopy. Could you update vhost-user.txt to describe the new messages? Otherwise, make check hangs in /x86_64/vhost-user/connect-fail (might be an unrelated regression?) Thanks
OK, figured that one out; it was a cleanup path for the postcopy notifier trying to remove the notifier when because we were testing a failure path the notifier hadn't been added in the first place.
(That was really nasty to find, for some reason those tests refuse to generate core dumps; I ended up with a while loop doing gdb --pid $(pgrep....) to nail the qemu that was about to seg)
Dave
Quoted 2 lines
Signature · 1 line
Marc-André Lureau
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Hello, David!
Thank for you patch set.
Quoted 13 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Hi,
This is a RFC/WIP series that enables postcopy migration
with shared memory to a vhost-user process.
It's based off current-head + Juan'sload_cleanupseries, and Alexey's bitmap series (v4). It's very lightly tested and seems to work, but it's quite rough.I've modified the vhost-user-bridge (aka vhub) in qemu's tests/ to use the new feature, since this is about the simplest client around.
Quoted 7 lines
Structure:
The basic idea is that near the start of postcopy, the client opens its own userfaultfd fd and sends that back to QEMU over the socket it's already using for VHUST_USER_* commands. Then when
VHOST_USER_SET_MEM_TABLEarrives it registers the areas with userfaultfd and sends the mapped addresses back to QEMU.
userfault fd should be only one per all affected processes. But why are you opening userfaultfd on client side, why not to pass userfault fd which was opened at QEMU side? I guess, it could be several virtual switches with different ports (it's exotic configuration, but configuration when we have one QEMU, one vswitchd, and serveral vhost-user ports is typical), and as example, QEMU could be connected to these vswitches through these ports. In this case you will obtain 2 different userfault fd in QEMU. In case of one QEMU, one vswitchd and several vhost-user ports, you are keeping userfaultfd in VuDev structure on client side, looks like it's virtion_net sibling from DPDK, and that structure is per vhost-user connection (per one port).
So from my point of view it's better to open fd on QEMU side, and pass it the same way as shared mem fd in SET_MEM_TABLE, but in POSTCOPY_ADVISE.
Quoted 5 lines
QEMU then reads the clients UFD in it's fault thread and issues requests back to the source as needed.
QEMU also issues 'WAKE' ioctls on the UFD to let the client know that the page has arrived and can carry on.
Not so clear for me why QEMU have to inform vhost client, due to single userfault fd, and kernel should wake up another faulted thread/processes.
In my approach I just to send information about copied/received page to vhot client, to be able to enable previously disabled VRING.
Quoted 9 lines
A new feature (
VHOST_USER_PROTOCOL_F_POSTCOPY) is added so that the QEMU knows the client can talk postcopy.
Three new messages (VHOST_USER_POSTCOPY_{ADVISE/LISTEN/END}) are added to guide the process along.Current known issues:
I've not tested it with hugepages yet; and I suspect the madvises will need tweaking for it.
I saw you didn't change order of SET_MEM_TABLE call in QEMU side, some part or pages already arrived and copied, so I'm doing hole here according to received map.
Quoted 6 lines
The qemu gets to see the base addresses that the client has its regions mapped at; that's not great for security
Take care of deadlocking; any thread in the client that accesses a userfault protected page can stall.
That's why I decided to disable VRINGs, but not the way as you did in GET_VRING_BASE, I send received bitmap, right after SET_MEM_TABLE, here could be synchronization problem, maybe similar problem as you described in "vhost+postcopy: Lock around set_mem_table"
Unfortunately, my patches isn't yet ready.
Quoted 105 lines
There's a nasty hack of a lock around the
set_mem_tablemessage.I've not looked at the recent IOMMU code.
Some cleanup and a lot of corner cases need thinking about.
There are probably plenty of unknown issues as well.
Test setup:
I'm running on one host at the moment, with the guest scping a large file from the host as it migrates. The setup is based on one I found in the vhost-user setups. You'll need a recent kernel for the shared memory support in userfaultfd, and userfault isn't that happy if a process using shared memory core's - so make sure you have the latest fixes.SESS=vhost
ulimit -c unlimited
tmux -L $SESS new-session -d
tmux -L $SESS set-option -g history-limit 30000
# Start a router using the system qemu
tmux -L $SESS new-window -n router ./x86_64-softmmu/qemu-system-x86_64 -M none -nographic -net socket,vlan=0,udp=loca
lhost:4444,localaddr=localhost:5555 -net socket,vlan=0,udp=localhost:4445,localaddr=localhost:5556 -net user,vlan=0
tmux -L $SESS set-option -g set-remain-on-exit on
# Start source vhost bridge
tmux -L $SESS new-window -n srcvhostbr "./tests/vhost-user-bridge -u/tmp/vubrsrc.sock2>src-vub-log"
sleep 0.5
tmux -L $SESS new-window -n source "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backe
nd-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/
tmp/vubrsrc.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :0 -monitor stdio -trace events=/root/trace-file 2>src-qemu-log " # Start dest vhost bridge
tmux -L $SESS new-window -n destvhostbr "./tests/vhost-user-bridge -u/tmp/vubrdst.sock-l 127.0.0.1:4445 -r 127.0.0.
1:5556 2>dst-vub-log"
sleep 0.5
tmux -L $SESS new-window -n dest "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backend
-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/tm
p/vubrdst.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :1 -monitor stdio -incoming tcp::8888 -trace events=/root/trace-file 2>dst-qemu-log" tmux -L $SESS send-keys -t source "migrate_set_capabilitypostcopy-ram on
tmux -L $SESS send-keys -t source "migrate_set_speed20M
tmux -L $SESS send-keys -t dest "migrate_set_capabilitypostcopy-ram onthen once booted:
tmux -L vhost send-keys -t source 'migrate -d tcp:0:8888^M' tmux -L vhost send-keys -t source 'migrate_start_postcopy^M' (Note those ^M's are actual ctrl-M's i.e. ctrl-v ctrl-M)Dave
Dr. David Alan Gilbert (29):
RAMBlock/migration: Add migration flags
migrate: Updateram_block_discard_rangefor sharedqemu_ram_block_host_offset
migration/ram:ramblock_recv_bitmap_test_byte_offset
postcopy: useUFFDIO_ZEROPAGEonly when available
postcopy: Add notifier chain
postcopy: Add vhost-user flag for postcopy and check it
vhost-user: Add 'VHOST_USER_POSTCOPY_ADVISE' message
vhub: Support sending fds back to qemu
vhub: Open userfaultfd
postcopy: Allow registering of fd handler vhost+postcopy: Register shared ufd with postcopy vhost+postcopy: Transmit 'listen' to client vhost+postcopy: Register new regions with the ufd vhost+postcopy: Send address back to qemu vhost+postcopy: Stash RAMBlock and offset vhost+postcopy: Send requests to source for shared pages vhost+postcopy: Resolve client address
postcopy: wake shared
postcopy:postcopy_notify_shared_wakevhost+postcopy: Add vhost waker
vhost+postcopy: Call wakeups
vub+postcopy: madvises
vhost+postcopy: Lock aroundset_mem_table
vhu: enable = false onget_vring_base
vhost: AddVHOST_USER_POSTCOPY_ENDmessage vhost+postcopy: Wire upPOSTCOPY_ENDnotify
postcopy: Allow shared memory
vhost-user: Claim support for postcopycontrib/libvhost-user/libvhost-user.c | 178 ++++++++++++++++- contrib/libvhost-user/libvhost-user.h | 8 + exec.c | 44 +++-- hw/virtio/trace-events | 13 ++ hw/virtio/vhost-user.c | 293 +++++++++++++++++++++++++++- include/exec/cpu-common.h | 3 + include/exec/ram_addr.h | 2 + migration/migration.c | 3 + migration/migration.h | 8 + migration/postcopy-ram.c | 357 +++++++++++++++++++++++++++------- migration/postcopy-ram.h | 69 +++++++ migration/ram.c | 5 + migration/ram.h | 1 + migration/savevm.c | 13 ++ migration/trace-events | 6 + trace-events | 3 + vl.c | 4 +- 17 files changed, 926 insertions(+), 84 deletions(-)Signature · 1 line
2.13.0
Signature · 2 lines
BR Alexey
Quoted 27 lines · * Alexey ([email protected]) wrote:
Hello, David!
Thank for you patch set.
Quoted 13 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Hi,
This is a RFC/WIP series that enables postcopy migration
with shared memory to a vhost-user process.
It's based off current-head + Juan'sload_cleanupseries, and Alexey's bitmap series (v4). It's very lightly tested and seems to work, but it's quite rough.I've modified the vhost-user-bridge (aka vhub) in qemu's tests/ to use the new feature, since this is about the simplest client around.
Quoted 7 lines
Structure:
The basic idea is that near the start of postcopy, the client opens its own userfaultfd fd and sends that back to QEMU over the socket it's already using for VHUST_USER_* commands. Then when
VHOST_USER_SET_MEM_TABLEarrives it registers the areas with userfaultfd and sends the mapped addresses back to QEMU.
Quoted 3 lines
userfault fd should be only one per all affected processes. But why are you opening userfaultfd on client side, why not to pass userfault fd which was opened at QEMU side?
I just checked with Andrea on the semantics, and ufd don't work like that. Any given userfaultfd only works on the address space of the process that opened it; so if you want a process to block on it's memory space it's the one that has to open the ufd.
(I don't think I knew that when I wrote the code!)
The nice thing about that is that you never get too confused about address spaces - any one ufd always has one address space in it's ioctls associated with one process.
Quoted 10 lines
I guess, it could
be several virtual switches with different ports (it's exotic configuration, but configuration when we have one QEMU, one vswitchd, and serveral vhost-user ports is typical), and as example, QEMU could be connected to these vswitches through these ports. In this case you will obtain 2 different userfault fd in QEMU. In case of one QEMU, one vswitchd and several vhost-user ports, you are keeping userfaultfd in VuDev structure on client side, looks like it'svirtion_netsibling from DPDK, and that structure is per vhost-user connection (per one port).
Multiple switches make sense to me actually; running two switches and having redundant routes in each VM let you live update the switch process one at a time.
Quoted 2 lines
So from my point of view it's better to open fd on QEMU side, and pass it the same way as shared mem fd in
SET_MEM_TABLE, but inPOSTCOPY_ADVISE.
Yes I see where you're coming from; but it's one address space per-ufd; If you had one ufd then you'd have to change the messages to be
'pid ... is waiting on address ....'
and all the ioctls for doing wakes etc would have to gain a PID.
Quoted 10 lines
Quoted 5 lines
QEMU then reads the clients UFD in it's fault thread and issues requests back to the source as needed.
QEMU also issues 'WAKE' ioctls on the UFD to let the client know that the page has arrived and can carry on.Not so clear for me why QEMU have to inform vhost client, due to single userfault fd, and kernel should wake up another faulted thread/processes.
In my approach I just to send information about copied/received page to vhot client, to be able to enable previously disabled VRING.
The client itself doesn't get notified; it's a UFFDIO_WAKE ioctl on the ufd that tells the kernel it can unblock a process that's trying to access the page.
(Their is potential to remove some of that - if we can get the kernel to wake all the waiters for a physical page when a UFFDIO_COPY is done it would remove a lot of those).
Quoted 11 lines
Quoted 8 lines
A new feature (
VHOST_USER_PROTOCOL_F_POSTCOPY) is added so that the QEMU knows the client can talk postcopy.
Three new messages (VHOST_USER_POSTCOPY_{ADVISE/LISTEN/END}) are added to guide the process along.Current known issues:
I've not tested it with hugepages yet; and I suspect the madvises will need tweaking for it.I saw you didn't change order of
SET_MEM_TABLEcall in QEMU side, some part or pages already arrived and copied, so I'm doing hole here according to received map.
right, so I'm assuming they'll hit ufd faults and be immediately WAKEd when I find the bit is set in the received-bitmap.
Quoted 11 lines
Quoted 5 lines
The qemu gets to see the base addresses that the client has its regions mapped at; that's not great for security
Take care of deadlocking; any thread in the client that accesses a userfault protected page can stall.
That's why I decided to disable VRINGs, but not the way as you did in
GET_VRING_BASE, I send received bitmap, right afterSET_MEM_TABLE, here could be synchronization problem, maybe similar problem as you described in "vhost+postcopy: Lock aroundset_mem_table"Unfortunately, my patches isn't yet ready.
That's OK; these patches just-about work; only enough for me to post them and ask for opinions.
Dave
Quoted 101 lines
Quoted 101 lines
There's a nasty hack of a lock around the
set_mem_tablemessage.I've not looked at the recent IOMMU code.
Some cleanup and a lot of corner cases need thinking about.
There are probably plenty of unknown issues as well.
Test setup:
I'm running on one host at the moment, with the guest scping a large file from the host as it migrates. The setup is based on one I found in the vhost-user setups. You'll need a recent kernel for the shared memory support in userfaultfd, and userfault isn't that happy if a process using shared memory core's - so make sure you have the latest fixes.SESS=vhost
ulimit -c unlimited
tmux -L $SESS new-session -d
tmux -L $SESS set-option -g history-limit 30000
# Start a router using the system qemu
tmux -L $SESS new-window -n router ./x86_64-softmmu/qemu-system-x86_64 -M none -nographic -net socket,vlan=0,udp=loca
lhost:4444,localaddr=localhost:5555 -net socket,vlan=0,udp=localhost:4445,localaddr=localhost:5556 -net user,vlan=0
tmux -L $SESS set-option -g set-remain-on-exit on
# Start source vhost bridge
tmux -L $SESS new-window -n srcvhostbr "./tests/vhost-user-bridge -u/tmp/vubrsrc.sock2>src-vub-log"
sleep 0.5
tmux -L $SESS new-window -n source "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backe
nd-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/
tmp/vubrsrc.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :0 -monitor stdio -trace events=/root/trace-file 2>src-qemu-log " # Start dest vhost bridge
tmux -L $SESS new-window -n destvhostbr "./tests/vhost-user-bridge -u/tmp/vubrdst.sock-l 127.0.0.1:4445 -r 127.0.0.
1:5556 2>dst-vub-log"
sleep 0.5
tmux -L $SESS new-window -n dest "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backend
-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/tm
p/vubrdst.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :1 -monitor stdio -incoming tcp::8888 -trace events=/root/trace-file 2>dst-qemu-log" tmux -L $SESS send-keys -t source "migrate_set_capabilitypostcopy-ram on
tmux -L $SESS send-keys -t source "migrate_set_speed20M
tmux -L $SESS send-keys -t dest "migrate_set_capabilitypostcopy-ram onthen once booted:
tmux -L vhost send-keys -t source 'migrate -d tcp:0:8888^M' tmux -L vhost send-keys -t source 'migrate_start_postcopy^M' (Note those ^M's are actual ctrl-M's i.e. ctrl-v ctrl-M)Dave
Dr. David Alan Gilbert (29):
RAMBlock/migration: Add migration flags
migrate: Updateram_block_discard_rangefor sharedqemu_ram_block_host_offset
migration/ram:ramblock_recv_bitmap_test_byte_offset
postcopy: useUFFDIO_ZEROPAGEonly when available
postcopy: Add notifier chain
postcopy: Add vhost-user flag for postcopy and check it
vhost-user: Add 'VHOST_USER_POSTCOPY_ADVISE' message
vhub: Support sending fds back to qemu
vhub: Open userfaultfd
postcopy: Allow registering of fd handler vhost+postcopy: Register shared ufd with postcopy vhost+postcopy: Transmit 'listen' to client vhost+postcopy: Register new regions with the ufd vhost+postcopy: Send address back to qemu vhost+postcopy: Stash RAMBlock and offset vhost+postcopy: Send requests to source for shared pages vhost+postcopy: Resolve client address
postcopy: wake shared
postcopy:postcopy_notify_shared_wakevhost+postcopy: Add vhost waker
vhost+postcopy: Call wakeups
vub+postcopy: madvises
vhost+postcopy: Lock aroundset_mem_table
vhu: enable = false onget_vring_base
vhost: AddVHOST_USER_POSTCOPY_ENDmessage vhost+postcopy: Wire upPOSTCOPY_ENDnotify
postcopy: Allow shared memory
vhost-user: Claim support for postcopycontrib/libvhost-user/libvhost-user.c | 178 ++++++++++++++++- contrib/libvhost-user/libvhost-user.h | 8 + exec.c | 44 +++-- hw/virtio/trace-events | 13 ++ hw/virtio/vhost-user.c | 293 +++++++++++++++++++++++++++- include/exec/cpu-common.h | 3 + include/exec/ram_addr.h | 2 + migration/migration.c | 3 + migration/migration.h | 8 + migration/postcopy-ram.c | 357 +++++++++++++++++++++++++++------- migration/postcopy-ram.h | 69 +++++++ migration/ram.c | 5 + migration/ram.h | 1 + migration/savevm.c | 13 ++ migration/trace-events | 6 + trace-events | 3 + vl.c | 4 +- 17 files changed, 926 insertions(+), 84 deletions(-)
Quoted 9 lines
Quoted 4 lines
Signature · 1 line
2.13.0Signature · 2 lines
BR Alexey
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Quoted 28 lines · Dr. David Alan Gilbert
Quoted 27 lines · * Alexey ([email protected]) wrote:
Hello, David!
Thank for you patch set.
Quoted 20 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Hi,
This is a RFC/WIP series that enables postcopy migration
with shared memory to a vhost-user process.
It's based off current-head + Juan'sload_cleanupseries, and Alexey's bitmap series (v4). It's very lightly tested and seems to work, but it's quite rough.I've modified the vhost-user-bridge (aka vhub) in qemu's tests/ to use the new feature, since this is about the simplest client around.
Structure:
The basic idea is that near the start of postcopy, the client opens its own userfaultfd fd and sends that back to QEMU over the socket it's already using for VHUST_USER_* commands. Then when
VHOST_USER_SET_MEM_TABLEarrives it registers the areas with userfaultfd and sends the mapped addresses back to QEMU.
Quoted 8 lines
Quoted 3 lines
userfault fd should be only one per all affected processes. But why are you opening userfaultfd on client side, why not to pass userfault fd which was opened at QEMU side?
I just checked with Andrea on the semantics, and ufd don't work like that. Any given userfaultfd only works on the address space of the process that opened it; so if you want a process to block on it's memory space it's the one that has to open the ufd.
yes it obtains from vma in handle_userfault
ctx = vmf->vma->vm_userfaultfd_ctx.ctx;
so that's per vma
and it set into vmavma->vm_userfaultfd_ctx.ctx = ctx;
in userfaultfd_register(struct userfaultfd_ctx *ctx,
but into userfaultfd_register it puts from
struct userfaultfd_ctx *ctx = file->private_data;
becase file descriptor was transfered over unix domain socket (SOL_SOCKET) logically to assume userfaultfd context will be the same.
Quoted 191 lines
(I don't think I knew that when I wrote the code!)
The nice thing about that is that you never get too confused about address spaces - any one ufd always has one address space in it's ioctls associated with one process.Quoted 10 lines
I guess, it could
be several virtual switches with different ports (it's exotic configuration, but configuration when we have one QEMU, one vswitchd, and serveral vhost-user ports is typical), and as example, QEMU could be connected to these vswitches through these ports. In this case you will obtain 2 different userfault fd in QEMU. In case of one QEMU, one vswitchd and several vhost-user ports, you are keeping userfaultfd in VuDev structure on client side, looks like it'svirtion_netsibling from DPDK, and that structure is per vhost-user connection (per one port).Multiple switches make sense to me actually; running two switches and having redundant routes in each VM let you live update the switch process one at a time.
Quoted 2 lines
So from my point of view it's better to open fd on QEMU side, and pass it the same way as shared mem fd in
SET_MEM_TABLE, but inPOSTCOPY_ADVISE.Yes I see where you're coming from; but it's one address space per-ufd; If you had one ufd then you'd have to change the messages to be
'pid ... is waiting on address ....'
and all the ioctls for doing wakes etc would have to gain a PID.Quoted 10 lines
Quoted 5 lines
QEMU then reads the clients UFD in it's fault thread and issues requests back to the source as needed.
QEMU also issues 'WAKE' ioctls on the UFD to let the client know that the page has arrived and can carry on.Not so clear for me why QEMU have to inform vhost client, due to single userfault fd, and kernel should wake up another faulted thread/processes.
In my approach I just to send information about copied/received page to vhot client, to be able to enable previously disabled VRING.The client itself doesn't get notified; it's a
UFFDIO_WAKEioctl on the ufd that tells the kernel it can unblock a process that's trying to access the page.
(Their is potential to remove some of that - if we can get the kernel to wake all the waiters for a physical page when aUFFDIO_COPYis done it would remove a lot of those).Quoted 11 lines
Quoted 8 lines
A new feature (
VHOST_USER_PROTOCOL_F_POSTCOPY) is added so that the QEMU knows the client can talk postcopy.
Three new messages (VHOST_USER_POSTCOPY_{ADVISE/LISTEN/END}) are added to guide the process along.Current known issues:
I've not tested it with hugepages yet; and I suspect the madvises will need tweaking for it.I saw you didn't change order of
SET_MEM_TABLEcall in QEMU side, some part or pages already arrived and copied, so I'm doing hole here according to received map.right, so I'm assuming they'll hit ufd faults and be immediately WAKEd when I find the bit is set in the received-bitmap.
Quoted 11 lines
Quoted 5 lines
The qemu gets to see the base addresses that the client has its regions mapped at; that's not great for security
Take care of deadlocking; any thread in the client that accesses a userfault protected page can stall.
That's why I decided to disable VRINGs, but not the way as you did in
GET_VRING_BASE, I send received bitmap, right afterSET_MEM_TABLE, here could be synchronization problem, maybe similar problem as you described in "vhost+postcopy: Lock aroundset_mem_table"Unfortunately, my patches isn't yet ready.
That's OK; these patches just-about work; only enough for me to post them and ask for opinions.
Dave
Quoted 101 lines
Quoted 101 lines
There's a nasty hack of a lock around the
set_mem_tablemessage.I've not looked at the recent IOMMU code.
Some cleanup and a lot of corner cases need thinking about.
There are probably plenty of unknown issues as well.
Test setup:
I'm running on one host at the moment, with the guest scping a large file from the host as it migrates. The setup is based on one I found in the vhost-user setups. You'll need a recent kernel for the shared memory support in userfaultfd, and userfault isn't that happy if a process using shared memory core's - so make sure you have the latest fixes.SESS=vhost
ulimit -c unlimited
tmux -L $SESS new-session -d
tmux -L $SESS set-option -g history-limit 30000
# Start a router using the system qemu
tmux -L $SESS new-window -n router ./x86_64-softmmu/qemu-system-x86_64 -M none -nographic -net socket,vlan=0,udp=loca
lhost:4444,localaddr=localhost:5555 -net socket,vlan=0,udp=localhost:4445,localaddr=localhost:5556 -net user,vlan=0
tmux -L $SESS set-option -g set-remain-on-exit on
# Start source vhost bridge
tmux -L $SESS new-window -n srcvhostbr "./tests/vhost-user-bridge -u/tmp/vubrsrc.sock2>src-vub-log"
sleep 0.5
tmux -L $SESS new-window -n source "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backe
nd-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/
tmp/vubrsrc.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :0 -monitor stdio -trace events=/root/trace-file 2>src-qemu-log " # Start dest vhost bridge
tmux -L $SESS new-window -n destvhostbr "./tests/vhost-user-bridge -u/tmp/vubrdst.sock-l 127.0.0.1:4445 -r 127.0.0.
1:5556 2>dst-vub-log"
sleep 0.5
tmux -L $SESS new-window -n dest "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backend
-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/tm
p/vubrdst.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :1 -monitor stdio -incoming tcp::8888 -trace events=/root/trace-file 2>dst-qemu-log" tmux -L $SESS send-keys -t source "migrate_set_capabilitypostcopy-ram on
tmux -L $SESS send-keys -t source "migrate_set_speed20M
tmux -L $SESS send-keys -t dest "migrate_set_capabilitypostcopy-ram onthen once booted:
tmux -L vhost send-keys -t source 'migrate -d tcp:0:8888^M' tmux -L vhost send-keys -t source 'migrate_start_postcopy^M' (Note those ^M's are actual ctrl-M's i.e. ctrl-v ctrl-M)Dave
Dr. David Alan Gilbert (29):
RAMBlock/migration: Add migration flags
migrate: Updateram_block_discard_rangefor sharedqemu_ram_block_host_offset
migration/ram:ramblock_recv_bitmap_test_byte_offset
postcopy: useUFFDIO_ZEROPAGEonly when available
postcopy: Add notifier chain
postcopy: Add vhost-user flag for postcopy and check it
vhost-user: Add 'VHOST_USER_POSTCOPY_ADVISE' message
vhub: Support sending fds back to qemu
vhub: Open userfaultfd
postcopy: Allow registering of fd handler vhost+postcopy: Register shared ufd with postcopy vhost+postcopy: Transmit 'listen' to client vhost+postcopy: Register new regions with the ufd vhost+postcopy: Send address back to qemu vhost+postcopy: Stash RAMBlock and offset vhost+postcopy: Send requests to source for shared pages vhost+postcopy: Resolve client address
postcopy: wake shared
postcopy:postcopy_notify_shared_wakevhost+postcopy: Add vhost waker
vhost+postcopy: Call wakeups
vub+postcopy: madvises
vhost+postcopy: Lock aroundset_mem_table
vhu: enable = false onget_vring_base
vhost: AddVHOST_USER_POSTCOPY_ENDmessage vhost+postcopy: Wire upPOSTCOPY_ENDnotify
postcopy: Allow shared memory
vhost-user: Claim support for postcopycontrib/libvhost-user/libvhost-user.c | 178 ++++++++++++++++- contrib/libvhost-user/libvhost-user.h | 8 + exec.c | 44 +++-- hw/virtio/trace-events | 13 ++ hw/virtio/vhost-user.c | 293 +++++++++++++++++++++++++++- include/exec/cpu-common.h | 3 + include/exec/ram_addr.h | 2 + migration/migration.c | 3 + migration/migration.h | 8 + migration/postcopy-ram.c | 357 +++++++++++++++++++++++++++------- migration/postcopy-ram.h | 69 +++++++ migration/ram.c | 5 + migration/ram.h | 1 + migration/savevm.c | 13 ++ migration/trace-events | 6 + trace-events | 3 + vl.c | 4 +- 17 files changed, 926 insertions(+), 84 deletions(-)Quoted 9 lines
Quoted 4 lines
Signature · 1 line
2.13.0Signature · 2 lines
BR Alexey--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Signature · 2 lines
BR Alexey
Quoted 31 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Hi,
This is a RFC/WIP series that enables postcopy migration
with shared memory to a vhost-user process.
It's based off current-head + Juan'sload_cleanupseries, and Alexey's bitmap series (v4). It's very lightly tested and seems to work, but it's quite rough.I've modified the vhost-user-bridge (aka vhub) in qemu's tests/ to use the new feature, since this is about the simplest client around.
Structure:
The basic idea is that near the start of postcopy, the client opens its own userfaultfd fd and sends that back to QEMU over the socket it's already using for VHUST_USER_* commands. Then when
VHOST_USER_SET_MEM_TABLEarrives it registers the areas with userfaultfd and sends the mapped addresses back to QEMU.QEMU then reads the clients UFD in it's fault thread and issues requests back to the source as needed.
QEMU also issues 'WAKE' ioctls on the UFD to let the client know that the page has arrived and can carry on.A new feature (
VHOST_USER_PROTOCOL_F_POSTCOPY) is added so that the QEMU knows the client can talk postcopy.
Three new messages (VHOST_USER_POSTCOPY_{ADVISE/LISTEN/END}) are added to guide the process along.
Quoted 6 lines
Current known issues:
I've not tested it with hugepages yet; and I suspect the madvises will need tweaking for it.The qemu gets to see the base addresses that the client has its regions mapped at; that's not great for security
Not urgent to fix.
Quoted 2 lines
Take care of deadlocking; any thread in the client that accesses a userfault protected page can stall.
And it can happen under a lock quite easily.
What exactly is proposed here?
Maybe we want to reuse the new channel that the IOMMU uses.
Quoted 1 line
There's a nasty hack of a lock around the
set_mem_tablemessage.
Yes.
Quoted 5 lines
I've not looked at the recent IOMMU code.
Some cleanup and a lot of corner cases need thinking about.
There are probably plenty of unknown issues as well.
At the protocol level, I'd like to rename the feature to USER_PAGEFAULT. Client does not really know anything about copies, it's all internal to qemu.
Spec can document that it's used by qemu for postcopy.
Quoted 94 lines
Test setup:
I'm running on one host at the moment, with the guest scping a large file from the host as it migrates. The setup is based on one I found in the vhost-user setups. You'll need a recent kernel for the shared memory support in userfaultfd, and userfault isn't that happy if a process using shared memory core's - so make sure you have the latest fixes.SESS=vhost
ulimit -c unlimited
tmux -L $SESS new-session -d
tmux -L $SESS set-option -g history-limit 30000
# Start a router using the system qemu
tmux -L $SESS new-window -n router ./x86_64-softmmu/qemu-system-x86_64 -M none -nographic -net socket,vlan=0,udp=loca
lhost:4444,localaddr=localhost:5555 -net socket,vlan=0,udp=localhost:4445,localaddr=localhost:5556 -net user,vlan=0
tmux -L $SESS set-option -g set-remain-on-exit on
# Start source vhost bridge
tmux -L $SESS new-window -n srcvhostbr "./tests/vhost-user-bridge -u/tmp/vubrsrc.sock2>src-vub-log"
sleep 0.5
tmux -L $SESS new-window -n source "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backe
nd-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/
tmp/vubrsrc.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :0 -monitor stdio -trace events=/root/trace-file 2>src-qemu-log " # Start dest vhost bridge
tmux -L $SESS new-window -n destvhostbr "./tests/vhost-user-bridge -u/tmp/vubrdst.sock-l 127.0.0.1:4445 -r 127.0.0.
1:5556 2>dst-vub-log"
sleep 0.5
tmux -L $SESS new-window -n dest "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backend
-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/tm
p/vubrdst.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :1 -monitor stdio -incoming tcp::8888 -trace events=/root/trace-file 2>dst-qemu-log" tmux -L $SESS send-keys -t source "migrate_set_capabilitypostcopy-ram on
tmux -L $SESS send-keys -t source "migrate_set_speed20M
tmux -L $SESS send-keys -t dest "migrate_set_capabilitypostcopy-ram onthen once booted:
tmux -L vhost send-keys -t source 'migrate -d tcp:0:8888^M' tmux -L vhost send-keys -t source 'migrate_start_postcopy^M' (Note those ^M's are actual ctrl-M's i.e. ctrl-v ctrl-M)Dave
Dr. David Alan Gilbert (29):
RAMBlock/migration: Add migration flags
migrate: Updateram_block_discard_rangefor sharedqemu_ram_block_host_offset
migration/ram:ramblock_recv_bitmap_test_byte_offset
postcopy: useUFFDIO_ZEROPAGEonly when available
postcopy: Add notifier chain
postcopy: Add vhost-user flag for postcopy and check it
vhost-user: Add 'VHOST_USER_POSTCOPY_ADVISE' message
vhub: Support sending fds back to qemu
vhub: Open userfaultfd
postcopy: Allow registering of fd handler vhost+postcopy: Register shared ufd with postcopy vhost+postcopy: Transmit 'listen' to client vhost+postcopy: Register new regions with the ufd vhost+postcopy: Send address back to qemu vhost+postcopy: Stash RAMBlock and offset vhost+postcopy: Send requests to source for shared pages vhost+postcopy: Resolve client address
postcopy: wake shared
postcopy:postcopy_notify_shared_wakevhost+postcopy: Add vhost waker
vhost+postcopy: Call wakeups
vub+postcopy: madvises
vhost+postcopy: Lock aroundset_mem_table
vhu: enable = false onget_vring_base
vhost: AddVHOST_USER_POSTCOPY_ENDmessage vhost+postcopy: Wire upPOSTCOPY_ENDnotify
postcopy: Allow shared memory
vhost-user: Claim support for postcopycontrib/libvhost-user/libvhost-user.c | 178 ++++++++++++++++- contrib/libvhost-user/libvhost-user.h | 8 + exec.c | 44 +++-- hw/virtio/trace-events | 13 ++ hw/virtio/vhost-user.c | 293 +++++++++++++++++++++++++++- include/exec/cpu-common.h | 3 + include/exec/ram_addr.h | 2 + migration/migration.c | 3 + migration/migration.h | 8 + migration/postcopy-ram.c | 357 +++++++++++++++++++++++++++------- migration/postcopy-ram.h | 69 +++++++ migration/ram.c | 5 + migration/ram.h | 1 + migration/savevm.c | 13 ++ migration/trace-events | 6 + trace-events | 3 + vl.c | 4 +- 17 files changed, 926 insertions(+), 84 deletions(-)Signature · 1 line
2.13.0
Quoted 39 lines · * Michael S. Tsirkin ([email protected]) wrote:
Quoted 31 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Hi,
This is a RFC/WIP series that enables postcopy migration
with shared memory to a vhost-user process.
It's based off current-head + Juan'sload_cleanupseries, and Alexey's bitmap series (v4). It's very lightly tested and seems to work, but it's quite rough.I've modified the vhost-user-bridge (aka vhub) in qemu's tests/ to use the new feature, since this is about the simplest client around.
Structure:
The basic idea is that near the start of postcopy, the client opens its own userfaultfd fd and sends that back to QEMU over the socket it's already using for VHUST_USER_* commands. Then when
VHOST_USER_SET_MEM_TABLEarrives it registers the areas with userfaultfd and sends the mapped addresses back to QEMU.QEMU then reads the clients UFD in it's fault thread and issues requests back to the source as needed.
QEMU also issues 'WAKE' ioctls on the UFD to let the client know that the page has arrived and can carry on.A new feature (
VHOST_USER_PROTOCOL_F_POSTCOPY) is added so that the QEMU knows the client can talk postcopy.
Three new messages (VHOST_USER_POSTCOPY_{ADVISE/LISTEN/END}) are added to guide the process along.Quoted 6 lines
Current known issues:
I've not tested it with hugepages yet; and I suspect the madvises will need tweaking for it.The qemu gets to see the base addresses that the client has its regions mapped at; that's not great for security
Quoted 8 lines
Not urgent to fix.
Quoted 2 lines
Take care of deadlocking; any thread in the client that accesses a userfault protected page can stall.
And it can happen under a lock quite easily.
What exactly is proposed here?
Maybe we want to reuse the new channel that the IOMMU uses.
There's no fundamental reason to get deadlocks as long as you get it right; the qemu thread that processes the user-fault's is a separate independent thread, so once it's going the client can do whatever it likes and it will get woken up without intervention.
Some care is needed around the postcopy-end; reception of the message that tells you to drop the userfault enables (which frees anything that hasn't been woken) must be allowed to happen for the postcopy complete; we take care that QEMUs fault thread lives on until that message is acknowledged.
I'm more worried about how this will work in a full packet switch when one vhost-user client for an incoming migration stalls the whole switch unless care is taken about the design. How do we figure out whether this is going to fly on a full stack? That's my main reason for getting this WIP set out here to get comments.
Quoted 6 lines
Quoted 1 line
There's a nasty hack of a lock around the
set_mem_tablemessage.Yes.
Quoted 2 lines
I've not looked at the recent IOMMU code.
Quoted 8 lines
Quoted 3 lines
Some cleanup and a lot of corner cases need thinking about.
There are probably plenty of unknown issues as well.
At the protocol level, I'd like to rename the feature to
USER_PAGEFAULT. Client does not really know anything about copies, it's all internal to qemu.
Spec can document that it's used by qemu for postcopy.
OK, tbh I suspect that using it for anything else would be tricky without adding more protocol features for that other use case.
Dave
Quoted 92 lines
Quoted 92 lines
Test setup:
I'm running on one host at the moment, with the guest scping a large file from the host as it migrates. The setup is based on one I found in the vhost-user setups. You'll need a recent kernel for the shared memory support in userfaultfd, and userfault isn't that happy if a process using shared memory core's - so make sure you have the latest fixes.SESS=vhost
ulimit -c unlimited
tmux -L $SESS new-session -d
tmux -L $SESS set-option -g history-limit 30000
# Start a router using the system qemu
tmux -L $SESS new-window -n router ./x86_64-softmmu/qemu-system-x86_64 -M none -nographic -net socket,vlan=0,udp=loca
lhost:4444,localaddr=localhost:5555 -net socket,vlan=0,udp=localhost:4445,localaddr=localhost:5556 -net user,vlan=0
tmux -L $SESS set-option -g set-remain-on-exit on
# Start source vhost bridge
tmux -L $SESS new-window -n srcvhostbr "./tests/vhost-user-bridge -u/tmp/vubrsrc.sock2>src-vub-log"
sleep 0.5
tmux -L $SESS new-window -n source "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backe
nd-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/
tmp/vubrsrc.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :0 -monitor stdio -trace events=/root/trace-file 2>src-qemu-log " # Start dest vhost bridge
tmux -L $SESS new-window -n destvhostbr "./tests/vhost-user-bridge -u/tmp/vubrdst.sock-l 127.0.0.1:4445 -r 127.0.0.
1:5556 2>dst-vub-log"
sleep 0.5
tmux -L $SESS new-window -n dest "./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 8G -smp 2 -object memory-backend
-file,id=mem,size=8G,mem-path=/dev/shm,share=on -numa node,memdev=mem -mem-prealloc -chardev socket,id=char0,path=/tm
p/vubrdst.sock -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce -device virtio-net-pci,netdev=mynet1 my.qcow2 -net none -vnc :1 -monitor stdio -incoming tcp::8888 -trace events=/root/trace-file 2>dst-qemu-log" tmux -L $SESS send-keys -t source "migrate_set_capabilitypostcopy-ram on
tmux -L $SESS send-keys -t source "migrate_set_speed20M
tmux -L $SESS send-keys -t dest "migrate_set_capabilitypostcopy-ram onthen once booted:
tmux -L vhost send-keys -t source 'migrate -d tcp:0:8888^M' tmux -L vhost send-keys -t source 'migrate_start_postcopy^M' (Note those ^M's are actual ctrl-M's i.e. ctrl-v ctrl-M)Dave
Dr. David Alan Gilbert (29):
RAMBlock/migration: Add migration flags
migrate: Updateram_block_discard_rangefor sharedqemu_ram_block_host_offset
migration/ram:ramblock_recv_bitmap_test_byte_offset
postcopy: useUFFDIO_ZEROPAGEonly when available
postcopy: Add notifier chain
postcopy: Add vhost-user flag for postcopy and check it
vhost-user: Add 'VHOST_USER_POSTCOPY_ADVISE' message
vhub: Support sending fds back to qemu
vhub: Open userfaultfd
postcopy: Allow registering of fd handler vhost+postcopy: Register shared ufd with postcopy vhost+postcopy: Transmit 'listen' to client vhost+postcopy: Register new regions with the ufd vhost+postcopy: Send address back to qemu vhost+postcopy: Stash RAMBlock and offset vhost+postcopy: Send requests to source for shared pages vhost+postcopy: Resolve client address
postcopy: wake shared
postcopy:postcopy_notify_shared_wakevhost+postcopy: Add vhost waker
vhost+postcopy: Call wakeups
vub+postcopy: madvises
vhost+postcopy: Lock aroundset_mem_table
vhu: enable = false onget_vring_base
vhost: AddVHOST_USER_POSTCOPY_ENDmessage vhost+postcopy: Wire upPOSTCOPY_ENDnotify
postcopy: Allow shared memory
vhost-user: Claim support for postcopycontrib/libvhost-user/libvhost-user.c | 178 ++++++++++++++++- contrib/libvhost-user/libvhost-user.h | 8 + exec.c | 44 +++-- hw/virtio/trace-events | 13 ++ hw/virtio/vhost-user.c | 293 +++++++++++++++++++++++++++- include/exec/cpu-common.h | 3 + include/exec/ram_addr.h | 2 + migration/migration.c | 3 + migration/migration.h | 8 + migration/postcopy-ram.c | 357 +++++++++++++++++++++++++++------- migration/postcopy-ram.h | 69 +++++++ migration/ram.c | 5 + migration/ram.h | 1 + migration/savevm.c | 13 ++ migration/trace-events | 6 + trace-events | 3 + vl.c | 4 +- 17 files changed, 926 insertions(+), 84 deletions(-)
Quoted 2 lines
Quoted 2 lines
Signature · 1 line
2.13.0
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Quoted 7 lines · Dr. David Alan Gilbert
Quoted 6 lines
Quoted 2 lines
Take care of deadlocking; any thread in the client that accesses a userfault protected page can stall.
And it can happen under a lock quite easily.
What exactly is proposed here?
Maybe we want to reuse the new channel that the IOMMU uses.
Quoted 5 lines
There's no fundamental reason to get deadlocks as long as you get it right; the qemu thread that processes the user-fault's is a separate independent thread, so once it's going the client can do whatever it likes and it will get woken up without intervention.
You take a lock for the channel, then access guest memory. Then the thread that gets messages from qemu can't get on the channel to mark range as populated.
Quoted 10 lines
Some care is needed around the postcopy-end; reception of the message that tells you to drop the userfault enables (which frees anything that hasn't been woken) must be allowed to happen for the postcopy complete; we take care that QEMUs fault thread lives on until that message is acknowledged.
I'm more worried about how this will work in a full packet switch when one vhost-user client for an incoming migration stalls the whole switch unless care is taken about the design. How do we figure out whether this is going to fly on a full stack?
It's performance though. Client could run in a separate thread for a while until migration finishes. We need to make sure there's explicit documentation that tells clients at what point they might block.
Quoted 2 lines
That's my main reason for getting this WIP set out here to get comments.
What will happen if QEMU dies? Is there a way to unblock the client?
Quoted 15 lines
Quoted 6 lines
Quoted 1 line
There's a nasty hack of a lock around the
set_mem_tablemessage.Yes.
Quoted 2 lines
I've not looked at the recent IOMMU code.
Quoted 8 lines
Quoted 3 lines
Some cleanup and a lot of corner cases need thinking about.
There are probably plenty of unknown issues as well.
At the protocol level, I'd like to rename the feature to
USER_PAGEFAULT. Client does not really know anything about copies, it's all internal to qemu.
Spec can document that it's used by qemu for postcopy.
Quoted 4 lines
OK, tbh I suspect that using it for anything else would be tricky without adding more protocol features for that other use case.
Dave
Why exactly? How does client have to know it's migration?
Signature · 1 line
MST
Quoted 14 lines · * Michael S. Tsirkin ([email protected]) wrote:
Quoted 7 lines · Dr. David Alan Gilbert
Quoted 6 lines
Quoted 2 lines
Take care of deadlocking; any thread in the client that accesses a userfault protected page can stall.
And it can happen under a lock quite easily.
What exactly is proposed here?
Maybe we want to reuse the new channel that the IOMMU uses.Quoted 5 lines
There's no fundamental reason to get deadlocks as long as you get it right; the qemu thread that processes the user-fault's is a separate independent thread, so once it's going the client can do whatever it likes and it will get woken up without intervention.
Quoted 3 lines
You take a lock for the channel, then access guest memory. Then the thread that gets messages from qemu can't get on the channel to mark range as populated.
It doesn't need to get the message from qemu to know it's populated though; qemu performs a WAKE ioctl on the userfaultfd to cause it to wake, so there's no action needed by the client. (If it does need to take a lock then ye we have a problem).
Quoted 11 lines
Quoted 10 lines
Some care is needed around the postcopy-end; reception of the message that tells you to drop the userfault enables (which frees anything that hasn't been woken) must be allowed to happen for the postcopy complete; we take care that QEMUs fault thread lives on until that message is acknowledged.
I'm more worried about how this will work in a full packet switch when one vhost-user client for an incoming migration stalls the whole switch unless care is taken about the design. How do we figure out whether this is going to fly on a full stack?
Quoted 4 lines
It's performance though. Client could run in a separate thread for a while until migration finishes. We need to make sure there's explicit documentation that tells clients at what point they might block.
Right.
Quoted 4 lines
Quoted 2 lines
That's my main reason for getting this WIP set out here to get comments.
What will happen if QEMU dies? Is there a way to unblock the client?
If the client can detect this and close it's userfaultfd then yes; of course that detection has to be done in a thread that can't be being blocked by anything related to the userfaultfd that it might be blocked on.
Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
Quoted 1 line
There's a nasty hack of a lock around the
set_mem_tablemessage.Yes.
Quoted 5 lines
I've not looked at the recent IOMMU code.
Some cleanup and a lot of corner cases need thinking about.
There are probably plenty of unknown issues as well.
At the protocol level, I'd like to rename the feature to
USER_PAGEFAULT. Client does not really know anything about copies, it's all internal to qemu.
Spec can document that it's used by qemu for postcopy.
Quoted 6 lines
Quoted 4 lines
OK, tbh I suspect that using it for anything else would be tricky without adding more protocol features for that other use case.
Dave
Why exactly? How does client have to know it's migration?
It's more the sequence I worry about; we're reliant on making sure that the userfaultfd is registered with the RAM before it's ever accessed, and we unregister at the end.
This all keys in with migration requesting registration at the right point before loading the devices.
Dave
Quoted 2 lines
Signature · 1 line
MST
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK