| Message ID | 8609dbecb51f77d45edc1add4b6ccc27b1886a05.1606853298.git.jag.raman@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Initial support for multi-process Qemu | expand |
On Wed, Dec 2, 2020 at 12:23 AM Jagannathan Raman <jag.raman@oracle.com> wrote: > SyncSysMemMsg message format is defined. It is used to send > file descriptors of the RAM regions to remote device. > RAM on the remote device is configured with a set of file descriptors. > Old RAM regions are deleted and new regions, each with an fd, is > added to the RAM. > > Signed-off-by: Jagannathan Raman <jag.raman@oracle.com> > Signed-off-by: John G Johnson <john.g.johnson@oracle.com> > Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > include/hw/remote/memory.h | 19 ++++++++++++++ > include/hw/remote/mpqemu-link.h | 13 +++++++++ > hw/remote/memory.c | 58 > +++++++++++++++++++++++++++++++++++++++++ > hw/remote/mpqemu-link.c | 11 ++++++++ > MAINTAINERS | 2 ++ > hw/remote/meson.build | 2 ++ > 6 files changed, 105 insertions(+) > create mode 100644 include/hw/remote/memory.h > create mode 100644 hw/remote/memory.c > > diff --git a/include/hw/remote/memory.h b/include/hw/remote/memory.h > new file mode 100644 > index 0000000..4fd548e > --- /dev/null > +++ b/include/hw/remote/memory.h > @@ -0,0 +1,19 @@ > +/* > + * Memory manager for remote device > + * > + * Copyright © 2018, 2020 Oracle and/or its affiliates. > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or > later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#ifndef REMOTE_MEMORY_H > +#define REMOTE_MEMORY_H > + > +#include "exec/hwaddr.h" > +#include "hw/remote/mpqemu-link.h" > + > +void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp); > + > +#endif > diff --git a/include/hw/remote/mpqemu-link.h > b/include/hw/remote/mpqemu-link.h > index 2d79ff8..070ac77 100644 > --- a/include/hw/remote/mpqemu-link.h > +++ b/include/hw/remote/mpqemu-link.h > @@ -14,6 +14,7 @@ > #include "qom/object.h" > #include "qemu/thread.h" > #include "io/channel.h" > +#include "exec/hwaddr.h" > > #define REMOTE_MAX_FDS 8 > > @@ -24,12 +25,22 @@ > * > * MPQemuCmd enum type to specify the command to be executed on the remote > * device. > + * > + * SYNC_SYSMEM Shares QEMU's RAM with remote device's RAM > My understanding is that it's deliberately a private protocol between qemu and remote host processes, so that it can break anytime. And in the future it will be vfio-user based. Correct? It would be worth to leave a note about it somewhere. */ > typedef enum { > MPQEMU_CMD_INIT, > + SYNC_SYSMEM, > + RET_MSG, > MPQEMU_CMD_MAX, > } MPQemuCmd; > > +typedef struct { > + hwaddr gpas[REMOTE_MAX_FDS]; > + uint64_t sizes[REMOTE_MAX_FDS]; > + off_t offsets[REMOTE_MAX_FDS]; > +} SyncSysmemMsg; > + > /** > * MPQemuMsg: > * @cmd: The remote command > @@ -40,12 +51,14 @@ typedef enum { > * MPQemuMsg Format of the message sent to the remote device from QEMU. > * > */ > + > typedef struct { > int cmd; > size_t size; > > union { > uint64_t u64; > + SyncSysmemMsg sync_sysmem; > } data; > > int fds[REMOTE_MAX_FDS]; > diff --git a/hw/remote/memory.c b/hw/remote/memory.c > new file mode 100644 > index 0000000..6d1e830 > --- /dev/null > +++ b/hw/remote/memory.c > @@ -0,0 +1,58 @@ > +/* > + * Memory manager for remote device > + * > + * Copyright © 2018, 2020 Oracle and/or its affiliates. > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or > later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include "qemu/osdep.h" > +#include "qemu-common.h" > + > +#include "hw/remote/memory.h" > +#include "exec/address-spaces.h" > +#include "exec/ram_addr.h" > +#include "qapi/error.h" > + > +void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp) > +{ > + SyncSysmemMsg *sysmem_info = &msg->data.sync_sysmem; > + MemoryRegion *sysmem, *subregion, *next; > + static unsigned int suffix; > + Error *local_err = NULL; > + char *name; > + int region; > + > + sysmem = get_system_memory(); > + > + memory_region_transaction_begin(); > It looks like this transaction business isn't really helping here. Both del_subregion and add_subregion already handle it. + > + QTAILQ_FOREACH_SAFE(subregion, &sysmem->subregions, subregions_link, > next) { > + if (subregion->ram) { > + memory_region_del_subregion(sysmem, subregion); > + object_unparent(OBJECT(subregion)); > + } > + } > + > + for (region = 0; region < msg->num_fds; region++) { > + subregion = g_new(MemoryRegion, 1); > + name = g_strdup_printf("remote-mem-%u", suffix++); > + memory_region_init_ram_from_fd(subregion, NULL, > + name, sysmem_info->sizes[region], > + true, msg->fds[region], > + sysmem_info->offsets[region], > + &local_err); > + g_free(name); > We are quite happily using g_auto these days > + if (local_err) { > + error_propagate(errp, local_err); > here that would help prevent leaking subregion. And ERRP_GUARD could also help remove local_err and error_propagate + break; > + } > + > + memory_region_add_subregion(sysmem, sysmem_info->gpas[region], > + subregion); > + } > + > + memory_region_transaction_commit(); > +} > diff --git a/hw/remote/mpqemu-link.c b/hw/remote/mpqemu-link.c > index e535ed2..bbd9df3 100644 > --- a/hw/remote/mpqemu-link.c > +++ b/hw/remote/mpqemu-link.c > @@ -238,5 +238,16 @@ bool mpqemu_msg_valid(MPQemuMsg *msg) > } > } > > + /* Verify message specific fields. */ > + switch (msg->cmd) { > + case SYNC_SYSMEM: > + if (msg->num_fds == 0 || msg->size != sizeof(SyncSysmemMsg)) { > + return false; > + } > + break; > + default: > + break; > + } > + > return true; > } > diff --git a/MAINTAINERS b/MAINTAINERS > index aedfc27..24cb36e 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3146,6 +3146,8 @@ F: include/hw/remote/mpqemu-link.h > F: hw/remote/message.c > F: include/hw/remote/remote-obj.h > F: hw/remote/remote-obj.c > +F: include/hw/remote/memory.h > +F: hw/remote/memory.c > > Build and test automation > ------------------------- > diff --git a/hw/remote/meson.build b/hw/remote/meson.build > index 71d0a56..64da16c 100644 > --- a/hw/remote/meson.build > +++ b/hw/remote/meson.build > @@ -5,4 +5,6 @@ remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: > files('mpqemu-link.c')) > remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('message.c')) > remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('remote-obj.c')) > > +specific_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('memory.c')) > + > softmmu_ss.add_all(when: 'CONFIG_MULTIPROCESS', if_true: remote_ss) > -- > 1.8.3.1 > > I can't comment much on the overall approach. That looks very straightforward to me, I would be surprised if there are not underlying subtleties. Again, is this only experimental until a vfio-user implementation emerge? If not, having unit tests would be important (if not required).
On Wed, Dec 2, 2020 at 12:23 AM Jagannathan Raman <jag.raman@oracle.com> wrote: > SyncSysMemMsg message format is defined. It is used to send > file descriptors of the RAM regions to remote device. > RAM on the remote device is configured with a set of file descriptors. > Old RAM regions are deleted and new regions, each with an fd, is > added to the RAM. > > Signed-off-by: Jagannathan Raman <jag.raman@oracle.com> > Signed-off-by: John G Johnson <john.g.johnson@oracle.com> > Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > include/hw/remote/memory.h | 19 ++++++++++++++ > include/hw/remote/mpqemu-link.h | 13 +++++++++ > hw/remote/memory.c | 58 > +++++++++++++++++++++++++++++++++++++++++ > hw/remote/mpqemu-link.c | 11 ++++++++ > MAINTAINERS | 2 ++ > hw/remote/meson.build | 2 ++ > 6 files changed, 105 insertions(+) > create mode 100644 include/hw/remote/memory.h > create mode 100644 hw/remote/memory.c > > diff --git a/include/hw/remote/memory.h b/include/hw/remote/memory.h > new file mode 100644 > index 0000000..4fd548e > --- /dev/null > +++ b/include/hw/remote/memory.h > @@ -0,0 +1,19 @@ > +/* > + * Memory manager for remote device > + * > + * Copyright © 2018, 2020 Oracle and/or its affiliates. > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or > later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#ifndef REMOTE_MEMORY_H > +#define REMOTE_MEMORY_H > + > +#include "exec/hwaddr.h" > +#include "hw/remote/mpqemu-link.h" > + > +void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp); > + > +#endif > diff --git a/include/hw/remote/mpqemu-link.h > b/include/hw/remote/mpqemu-link.h > index 2d79ff8..070ac77 100644 > --- a/include/hw/remote/mpqemu-link.h > +++ b/include/hw/remote/mpqemu-link.h > @@ -14,6 +14,7 @@ > #include "qom/object.h" > #include "qemu/thread.h" > #include "io/channel.h" > +#include "exec/hwaddr.h" > > #define REMOTE_MAX_FDS 8 > > @@ -24,12 +25,22 @@ > * > * MPQemuCmd enum type to specify the command to be executed on the remote > * device. > + * > + * SYNC_SYSMEM Shares QEMU's RAM with remote device's RAM > */ > typedef enum { > MPQEMU_CMD_INIT, > + SYNC_SYSMEM, > + RET_MSG, > RET_MSG is not used here, but later. ok. MPQEMU_CMD_MAX, > } MPQemuCmd; > > +typedef struct { > + hwaddr gpas[REMOTE_MAX_FDS]; > + uint64_t sizes[REMOTE_MAX_FDS]; > + off_t offsets[REMOTE_MAX_FDS]; > +} SyncSysmemMsg; > + > /** > * MPQemuMsg: > * @cmd: The remote command > @@ -40,12 +51,14 @@ typedef enum { > * MPQemuMsg Format of the message sent to the remote device from QEMU. > * > */ > + > typedef struct { > int cmd; > size_t size; > > union { > uint64_t u64; > + SyncSysmemMsg sync_sysmem; > } data; > > int fds[REMOTE_MAX_FDS]; > diff --git a/hw/remote/memory.c b/hw/remote/memory.c > new file mode 100644 > index 0000000..6d1e830 > --- /dev/null > +++ b/hw/remote/memory.c > @@ -0,0 +1,58 @@ > +/* > + * Memory manager for remote device > + * > + * Copyright © 2018, 2020 Oracle and/or its affiliates. > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or > later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include "qemu/osdep.h" > +#include "qemu-common.h" > + > +#include "hw/remote/memory.h" > +#include "exec/address-spaces.h" > +#include "exec/ram_addr.h" > +#include "qapi/error.h" > + > +void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp) > +{ > + SyncSysmemMsg *sysmem_info = &msg->data.sync_sysmem; > + MemoryRegion *sysmem, *subregion, *next; > + static unsigned int suffix; > + Error *local_err = NULL; > + char *name; > + int region; > + > + sysmem = get_system_memory(); > + > + memory_region_transaction_begin(); > + > + QTAILQ_FOREACH_SAFE(subregion, &sysmem->subregions, subregions_link, > next) { > + if (subregion->ram) { > + memory_region_del_subregion(sysmem, subregion); > + object_unparent(OBJECT(subregion)); > + } > + } > + > + for (region = 0; region < msg->num_fds; region++) { > + subregion = g_new(MemoryRegion, 1); > + name = g_strdup_printf("remote-mem-%u", suffix++); > + memory_region_init_ram_from_fd(subregion, NULL, > + name, sysmem_info->sizes[region], > + true, msg->fds[region], > + sysmem_info->offsets[region], > + &local_err); > + g_free(name); > + if (local_err) { > + error_propagate(errp, local_err); > + break; > + } > + > + memory_region_add_subregion(sysmem, sysmem_info->gpas[region], > + subregion); > + } > + > + memory_region_transaction_commit(); > +} > diff --git a/hw/remote/mpqemu-link.c b/hw/remote/mpqemu-link.c > index e535ed2..bbd9df3 100644 > --- a/hw/remote/mpqemu-link.c > +++ b/hw/remote/mpqemu-link.c > @@ -238,5 +238,16 @@ bool mpqemu_msg_valid(MPQemuMsg *msg) > } > } > > + /* Verify message specific fields. */ > + switch (msg->cmd) { > + case SYNC_SYSMEM: > + if (msg->num_fds == 0 || msg->size != sizeof(SyncSysmemMsg)) { > + return false; > + } > + break; > + default: > + break; > + } > + > return true; > } > diff --git a/MAINTAINERS b/MAINTAINERS > index aedfc27..24cb36e 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3146,6 +3146,8 @@ F: include/hw/remote/mpqemu-link.h > F: hw/remote/message.c > F: include/hw/remote/remote-obj.h > F: hw/remote/remote-obj.c > +F: include/hw/remote/memory.h > +F: hw/remote/memory.c > > Build and test automation > ------------------------- > diff --git a/hw/remote/meson.build b/hw/remote/meson.build > index 71d0a56..64da16c 100644 > --- a/hw/remote/meson.build > +++ b/hw/remote/meson.build > @@ -5,4 +5,6 @@ remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: > files('mpqemu-link.c')) > remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('message.c')) > remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('remote-obj.c')) > > +specific_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('memory.c')) > + > softmmu_ss.add_all(when: 'CONFIG_MULTIPROCESS', if_true: remote_ss) > -- > 1.8.3.1 > >
diff --git a/include/hw/remote/memory.h b/include/hw/remote/memory.h new file mode 100644 index 0000000..4fd548e --- /dev/null +++ b/include/hw/remote/memory.h @@ -0,0 +1,19 @@ +/* + * Memory manager for remote device + * + * Copyright © 2018, 2020 Oracle and/or its affiliates. + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#ifndef REMOTE_MEMORY_H +#define REMOTE_MEMORY_H + +#include "exec/hwaddr.h" +#include "hw/remote/mpqemu-link.h" + +void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp); + +#endif diff --git a/include/hw/remote/mpqemu-link.h b/include/hw/remote/mpqemu-link.h index 2d79ff8..070ac77 100644 --- a/include/hw/remote/mpqemu-link.h +++ b/include/hw/remote/mpqemu-link.h @@ -14,6 +14,7 @@ #include "qom/object.h" #include "qemu/thread.h" #include "io/channel.h" +#include "exec/hwaddr.h" #define REMOTE_MAX_FDS 8 @@ -24,12 +25,22 @@ * * MPQemuCmd enum type to specify the command to be executed on the remote * device. + * + * SYNC_SYSMEM Shares QEMU's RAM with remote device's RAM */ typedef enum { MPQEMU_CMD_INIT, + SYNC_SYSMEM, + RET_MSG, MPQEMU_CMD_MAX, } MPQemuCmd; +typedef struct { + hwaddr gpas[REMOTE_MAX_FDS]; + uint64_t sizes[REMOTE_MAX_FDS]; + off_t offsets[REMOTE_MAX_FDS]; +} SyncSysmemMsg; + /** * MPQemuMsg: * @cmd: The remote command @@ -40,12 +51,14 @@ typedef enum { * MPQemuMsg Format of the message sent to the remote device from QEMU. * */ + typedef struct { int cmd; size_t size; union { uint64_t u64; + SyncSysmemMsg sync_sysmem; } data; int fds[REMOTE_MAX_FDS]; diff --git a/hw/remote/memory.c b/hw/remote/memory.c new file mode 100644 index 0000000..6d1e830 --- /dev/null +++ b/hw/remote/memory.c @@ -0,0 +1,58 @@ +/* + * Memory manager for remote device + * + * Copyright © 2018, 2020 Oracle and/or its affiliates. + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#include "qemu/osdep.h" +#include "qemu-common.h" + +#include "hw/remote/memory.h" +#include "exec/address-spaces.h" +#include "exec/ram_addr.h" +#include "qapi/error.h" + +void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp) +{ + SyncSysmemMsg *sysmem_info = &msg->data.sync_sysmem; + MemoryRegion *sysmem, *subregion, *next; + static unsigned int suffix; + Error *local_err = NULL; + char *name; + int region; + + sysmem = get_system_memory(); + + memory_region_transaction_begin(); + + QTAILQ_FOREACH_SAFE(subregion, &sysmem->subregions, subregions_link, next) { + if (subregion->ram) { + memory_region_del_subregion(sysmem, subregion); + object_unparent(OBJECT(subregion)); + } + } + + for (region = 0; region < msg->num_fds; region++) { + subregion = g_new(MemoryRegion, 1); + name = g_strdup_printf("remote-mem-%u", suffix++); + memory_region_init_ram_from_fd(subregion, NULL, + name, sysmem_info->sizes[region], + true, msg->fds[region], + sysmem_info->offsets[region], + &local_err); + g_free(name); + if (local_err) { + error_propagate(errp, local_err); + break; + } + + memory_region_add_subregion(sysmem, sysmem_info->gpas[region], + subregion); + } + + memory_region_transaction_commit(); +} diff --git a/hw/remote/mpqemu-link.c b/hw/remote/mpqemu-link.c index e535ed2..bbd9df3 100644 --- a/hw/remote/mpqemu-link.c +++ b/hw/remote/mpqemu-link.c @@ -238,5 +238,16 @@ bool mpqemu_msg_valid(MPQemuMsg *msg) } } + /* Verify message specific fields. */ + switch (msg->cmd) { + case SYNC_SYSMEM: + if (msg->num_fds == 0 || msg->size != sizeof(SyncSysmemMsg)) { + return false; + } + break; + default: + break; + } + return true; } diff --git a/MAINTAINERS b/MAINTAINERS index aedfc27..24cb36e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3146,6 +3146,8 @@ F: include/hw/remote/mpqemu-link.h F: hw/remote/message.c F: include/hw/remote/remote-obj.h F: hw/remote/remote-obj.c +F: include/hw/remote/memory.h +F: hw/remote/memory.c Build and test automation ------------------------- diff --git a/hw/remote/meson.build b/hw/remote/meson.build index 71d0a56..64da16c 100644 --- a/hw/remote/meson.build +++ b/hw/remote/meson.build @@ -5,4 +5,6 @@ remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('mpqemu-link.c')) remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('message.c')) remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('remote-obj.c')) +specific_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('memory.c')) + softmmu_ss.add_all(when: 'CONFIG_MULTIPROCESS', if_true: remote_ss)