| Message ID | 1456771254-17511-30-git-send-email-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 29/02/2016 19:40, Markus Armbruster wrote: > ivshmem has its very own code to create and map shared memory. > Replace that with an implicitly created memory backend. Reduces the > number of ways we create BAR 2 from three to two. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Very appreciated, but do not use user_creatable_add_opts. Instead, create the object with object_initialize, object_property_set_* and user_creatable_complete. After the object_initialize, add it with object_property_add_child *under the ivshmem device itself*, giving it a name like "internal-shm-backend". This matches what virtio-blk dataplane used to do for x-dataplane (now removed). Thanks, Paolo > +static HostMemoryBackend *desugar_shm(const char *shm, size_t size) > +{ > + /* TODO avoid the detour through QemuOpts */ > + static int counter; > + QemuOpts *opts = qemu_opts_create(qemu_find_opts("object"), > + NULL, 0, &error_abort); > + char *path; > + Object *obj; > + > + qemu_opt_set(opts, "qom-type", "memory-backend-file", > + &error_abort); > + /* FIXME need a better way to make up an ID */ > + qemu_opts_set_id(opts, g_strdup_printf("ivshmem-backend-%d", counter++)); > + path = g_strdup_printf("/dev/shm/%s", shm); > + qemu_opt_set(opts, "mem-path", path, &error_abort); > + qemu_opt_set_number(opts, "size", size, &error_abort); > + qemu_opt_set_bool(opts, "share", true, &error_abort); > + g_free(path); > + > + obj = user_creatable_add_opts(opts, &error_abort); > + qemu_opts_del(opts); > + > + user_creatable_complete(obj, &error_abort); > + > + return MEMORY_BACKEND(obj); > +} > + > static void pci_ivshmem_realize(PCIDevice *dev, Error **errp) > { > IVShmemState *s = IVSHMEM(dev); > @@ -911,6 +914,10 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error **errp) > attr |= PCI_BASE_ADDRESS_MEM_TYPE_64; > } > > + if (s->shmobj) { > + s->hostmem = desugar_shm(s->shmobj, s->ivshmem_size); > + } > + > if (s->hostmem != NULL) { > MemoryRegion *mr; > > @@ -921,7 +928,7 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error **errp) > vmstate_register_ram(mr, DEVICE(s)); > memory_region_add_subregion(&s->bar, 0, mr); > pci_register_bar(PCI_DEVICE(s), 2, attr, &s->bar); > - } else if (s->server_chr != NULL) { > + } else { > IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", > s->server_chr->filename); > > @@ -948,36 +955,6 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error **errp) > error_setg(errp, "failed to initialize interrupts"); > return; > } > - } else { > - /* just map the file immediately, we're not using a server */ > - int fd; > - > - IVSHMEM_DPRINTF("using shm_open (shm object = %s)\n", s->shmobj); > - > - /* try opening with O_EXCL and if it succeeds zero the memory > - * by truncating to 0 */ > - if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR|O_EXCL, > - S_IRWXU|S_IRWXG|S_IRWXO)) > 0) { > - /* truncate file to length PCI device's memory */ > - if (ftruncate(fd, s->ivshmem_size) != 0) { > - error_report("could not truncate shared file"); > - } > - > - } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR, > - S_IRWXU|S_IRWXG|S_IRWXO)) < 0) { > - error_setg(errp, "could not open shared file"); > - return; > - } > - > - if (check_shm_size(s, fd, errp) == -1) { > - return; > - } > - > - create_shared_memory_BAR(s, fd, attr, &err); > - if (err) { > - error_propagate(errp, err); > - return; > - } > } > > if (s->role_val == IVSHMEM_PEER) { >
Paolo Bonzini <pbonzini@redhat.com> writes: > On 29/02/2016 19:40, Markus Armbruster wrote: >> ivshmem has its very own code to create and map shared memory. >> Replace that with an implicitly created memory backend. Reduces the >> number of ways we create BAR 2 from three to two. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > > Very appreciated, but do not use user_creatable_add_opts. Instead, > create the object with object_initialize, object_property_set_* and > user_creatable_complete. After the object_initialize, add it with > object_property_add_child *under the ivshmem device itself*, giving it a > name like "internal-shm-backend". Will do. > This matches what virtio-blk dataplane used to do for x-dataplane (now > removed). Note to self: commit a616fb7, try to steal that code. Thanks!
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c index 8d54fa9..9931d5e 100644 --- a/hw/misc/ivshmem.c +++ b/hw/misc/ivshmem.c @@ -26,6 +26,7 @@ #include "migration/migration.h" #include "qemu/error-report.h" #include "qemu/event_notifier.h" +#include "qom/object_interfaces.h" #include "sysemu/char.h" #include "sysemu/hostmem.h" #include "qapi/visitor.h" @@ -369,31 +370,6 @@ static int check_shm_size(IVShmemState *s, int fd, Error **errp) } } -/* create the shared memory BAR when we are not using the server, so we can - * create the BAR and map the memory immediately */ -static int create_shared_memory_BAR(IVShmemState *s, int fd, uint8_t attr, - Error **errp) -{ - void * ptr; - - ptr = mmap(0, s->ivshmem_size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); - if (ptr == MAP_FAILED) { - error_setg_errno(errp, errno, "Failed to mmap shared memory"); - return -1; - } - - memory_region_init_ram_ptr(&s->ivshmem, OBJECT(s), "ivshmem.bar2", - s->ivshmem_size, ptr); - qemu_set_ram_fd(s->ivshmem.ram_addr, fd); - vmstate_register_ram(&s->ivshmem, DEVICE(s)); - memory_region_add_subregion(&s->bar, 0, &s->ivshmem); - - /* region for shared memory */ - pci_register_bar(PCI_DEVICE(s), 2, attr, &s->bar); - - return 0; -} - static void ivshmem_add_eventfd(IVShmemState *s, int posn, int i) { memory_region_add_eventfd(&s->ivshmem_mmio, @@ -833,6 +809,33 @@ static void ivshmem_write_config(PCIDevice *pdev, uint32_t address, } } +static HostMemoryBackend *desugar_shm(const char *shm, size_t size) +{ + /* TODO avoid the detour through QemuOpts */ + static int counter; + QemuOpts *opts = qemu_opts_create(qemu_find_opts("object"), + NULL, 0, &error_abort); + char *path; + Object *obj; + + qemu_opt_set(opts, "qom-type", "memory-backend-file", + &error_abort); + /* FIXME need a better way to make up an ID */ + qemu_opts_set_id(opts, g_strdup_printf("ivshmem-backend-%d", counter++)); + path = g_strdup_printf("/dev/shm/%s", shm); + qemu_opt_set(opts, "mem-path", path, &error_abort); + qemu_opt_set_number(opts, "size", size, &error_abort); + qemu_opt_set_bool(opts, "share", true, &error_abort); + g_free(path); + + obj = user_creatable_add_opts(opts, &error_abort); + qemu_opts_del(opts); + + user_creatable_complete(obj, &error_abort); + + return MEMORY_BACKEND(obj); +} + static void pci_ivshmem_realize(PCIDevice *dev, Error **errp) { IVShmemState *s = IVSHMEM(dev); @@ -911,6 +914,10 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error **errp) attr |= PCI_BASE_ADDRESS_MEM_TYPE_64; } + if (s->shmobj) { + s->hostmem = desugar_shm(s->shmobj, s->ivshmem_size); + } + if (s->hostmem != NULL) { MemoryRegion *mr; @@ -921,7 +928,7 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error **errp) vmstate_register_ram(mr, DEVICE(s)); memory_region_add_subregion(&s->bar, 0, mr); pci_register_bar(PCI_DEVICE(s), 2, attr, &s->bar); - } else if (s->server_chr != NULL) { + } else { IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", s->server_chr->filename); @@ -948,36 +955,6 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error **errp) error_setg(errp, "failed to initialize interrupts"); return; } - } else { - /* just map the file immediately, we're not using a server */ - int fd; - - IVSHMEM_DPRINTF("using shm_open (shm object = %s)\n", s->shmobj); - - /* try opening with O_EXCL and if it succeeds zero the memory - * by truncating to 0 */ - if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR|O_EXCL, - S_IRWXU|S_IRWXG|S_IRWXO)) > 0) { - /* truncate file to length PCI device's memory */ - if (ftruncate(fd, s->ivshmem_size) != 0) { - error_report("could not truncate shared file"); - } - - } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR, - S_IRWXU|S_IRWXG|S_IRWXO)) < 0) { - error_setg(errp, "could not open shared file"); - return; - } - - if (check_shm_size(s, fd, errp) == -1) { - return; - } - - create_shared_memory_BAR(s, fd, attr, &err); - if (err) { - error_propagate(errp, err); - return; - } } if (s->role_val == IVSHMEM_PEER) {
ivshmem has its very own code to create and map shared memory. Replace that with an implicitly created memory backend. Reduces the number of ways we create BAR 2 from three to two. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/misc/ivshmem.c | 89 +++++++++++++++++++++---------------------------------- 1 file changed, 33 insertions(+), 56 deletions(-)