From 3a1258399bdf4d4412cbfde36d0d94965eec87b6 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 6 Sep 2023 14:04:53 +0200 Subject: [PATCH 01/12] nvdimm: Reject writing label data to ROM instead of crashing QEMU Currently, when using a true R/O NVDIMM (ROM memory backend) with a label area, the VM can easily crash QEMU by trying to write to the label area, because the ROM memory is mmap'ed without PROT_WRITE. [root@vm-0 ~]# ndctl disable-region region0 disabled 1 region [root@vm-0 ~]# ndctl zero-labels nmem0 -> QEMU segfaults Let's remember whether we have a ROM memory backend and properly reject the write request: [root@vm-0 ~]# ndctl disable-region region0 disabled 1 region [root@vm-0 ~]# ndctl zero-labels nmem0 zeroed 0 nmem In comparison, on a system with a R/W NVDIMM: [root@vm-0 ~]# ndctl disable-region region0 disabled 1 region [root@vm-0 ~]# ndctl zero-labels nmem0 zeroed 1 nmem For ACPI, just return "unsupported", like if no label exists. For spapr, return "H_P2", similar to when no label area exists. Could we rely on the "unarmed" property? Maybe, but it looks cleaner to only disallow what certainly cannot work. After all "unarmed=on" primarily means: cannot accept persistent writes. In theory, there might be setups where devices with "unarmed=on" set could be used to host non-persistent data (temporary files, system RAM, ...); for example, in Linux, admins can overwrite the "readonly" setting and still write to the device -- which will work as long as we're not using ROM. Allowing writing label data in such configurations can make sense. Message-ID: <20230906120503.359863-2-david@redhat.com> Fixes: dbd730e85987 ("nvdimm: check -object memory-backend-file, readonly=on option") Reviewed-by: Stefan Hajnoczi Signed-off-by: David Hildenbrand --- hw/acpi/nvdimm.c | 11 ++++++++--- hw/mem/nvdimm.c | 10 +++++++--- hw/ppc/spapr_nvdimm.c | 3 ++- include/hw/mem/nvdimm.h | 6 ++++++ 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c index a3b25a92f3..3cbd41629d 100644 --- a/hw/acpi/nvdimm.c +++ b/hw/acpi/nvdimm.c @@ -670,7 +670,8 @@ static void nvdimm_dsm_label_size(NVDIMMDevice *nvdimm, hwaddr dsm_mem_addr) } static uint32_t nvdimm_rw_label_data_check(NVDIMMDevice *nvdimm, - uint32_t offset, uint32_t length) + uint32_t offset, uint32_t length, + bool is_write) { uint32_t ret = NVDIMM_DSM_RET_STATUS_INVALID; @@ -690,6 +691,10 @@ static uint32_t nvdimm_rw_label_data_check(NVDIMMDevice *nvdimm, return ret; } + if (is_write && nvdimm->readonly) { + return NVDIMM_DSM_RET_STATUS_UNSUPPORT; + } + return NVDIMM_DSM_RET_STATUS_SUCCESS; } @@ -713,7 +718,7 @@ static void nvdimm_dsm_get_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in, get_label_data->length); status = nvdimm_rw_label_data_check(nvdimm, get_label_data->offset, - get_label_data->length); + get_label_data->length, false); if (status != NVDIMM_DSM_RET_STATUS_SUCCESS) { nvdimm_dsm_no_payload(status, dsm_mem_addr); return; @@ -752,7 +757,7 @@ static void nvdimm_dsm_set_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in, set_label_data->length); status = nvdimm_rw_label_data_check(nvdimm, set_label_data->offset, - set_label_data->length); + set_label_data->length, true); if (status != NVDIMM_DSM_RET_STATUS_SUCCESS) { nvdimm_dsm_no_payload(status, dsm_mem_addr); return; diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c index 31080c22c9..1631a7d13f 100644 --- a/hw/mem/nvdimm.c +++ b/hw/mem/nvdimm.c @@ -154,6 +154,9 @@ static void nvdimm_prepare_memory_region(NVDIMMDevice *nvdimm, Error **errp) object_get_canonical_path_component(OBJECT(hostmem))); return; } + if (memory_region_is_rom(mr)) { + nvdimm->readonly = true; + } nvdimm->nvdimm_mr = g_new(MemoryRegion, 1); memory_region_init_alias(nvdimm->nvdimm_mr, OBJECT(dimm), @@ -207,15 +210,16 @@ static void nvdimm_unrealize(PCDIMMDevice *dimm) * label read/write functions. */ static void nvdimm_validate_rw_label_data(NVDIMMDevice *nvdimm, uint64_t size, - uint64_t offset) + uint64_t offset, bool is_write) { assert((nvdimm->label_size >= size + offset) && (offset + size > offset)); + assert(!is_write || !nvdimm->readonly); } static void nvdimm_read_label_data(NVDIMMDevice *nvdimm, void *buf, uint64_t size, uint64_t offset) { - nvdimm_validate_rw_label_data(nvdimm, size, offset); + nvdimm_validate_rw_label_data(nvdimm, size, offset, false); memcpy(buf, nvdimm->label_data + offset, size); } @@ -229,7 +233,7 @@ static void nvdimm_write_label_data(NVDIMMDevice *nvdimm, const void *buf, "pmem", NULL); uint64_t backend_offset; - nvdimm_validate_rw_label_data(nvdimm, size, offset); + nvdimm_validate_rw_label_data(nvdimm, size, offset, true); if (!is_pmem) { memcpy(nvdimm->label_data + offset, buf, size); diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c index a8688243a6..60d6d0acc0 100644 --- a/hw/ppc/spapr_nvdimm.c +++ b/hw/ppc/spapr_nvdimm.c @@ -320,7 +320,8 @@ static target_ulong h_scm_write_metadata(PowerPCCPU *cpu, nvdimm = NVDIMM(drc->dev); if ((offset + len < offset) || - (nvdimm->label_size < len + offset)) { + (nvdimm->label_size < len + offset) || + nvdimm->readonly) { return H_P2; } diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h index acf887c83d..d3b763453a 100644 --- a/include/hw/mem/nvdimm.h +++ b/include/hw/mem/nvdimm.h @@ -77,6 +77,12 @@ struct NVDIMMDevice { */ bool unarmed; + /* + * Whether our DIMM is backed by ROM, and even label data cannot be + * written. If set, implies that "unarmed" is also set. + */ + bool readonly; + /* * The PPC64 - spapr requires each nvdimm device have a uuid. */ From 5c52a219bbd38724650e27e14741190d3004e26b Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 6 Sep 2023 14:04:54 +0200 Subject: [PATCH 02/12] softmmu/physmem: Distinguish between file access mode and mmap protection There is a difference between how we open a file and how we mmap it, and we want to support writable private mappings of readonly files. Let's define RAM_READONLY and RAM_READONLY_FD flags, to replace the single "readonly" parameter for file-related functions. In memory_region_init_ram_from_fd() and memory_region_init_ram_from_file(), initialize mr->readonly based on the new RAM_READONLY flag. While at it, add some RAM_* flags we missed to add to the list of accepted flags in the documentation of some functions. No change in functionality intended. We'll make use of both flags next and start setting them independently for memory-backend-file. Message-ID: <20230906120503.359863-3-david@redhat.com> Acked-by: Peter Xu Signed-off-by: David Hildenbrand --- backends/hostmem-file.c | 4 ++-- include/exec/memory.h | 14 ++++++++++---- include/exec/ram_addr.h | 8 ++++---- softmmu/memory.c | 8 ++++---- softmmu/physmem.c | 21 ++++++++++----------- 5 files changed, 30 insertions(+), 25 deletions(-) diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c index b4335a80e6..ef2d5533af 100644 --- a/backends/hostmem-file.c +++ b/backends/hostmem-file.c @@ -55,13 +55,13 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) name = host_memory_backend_get_name(backend); ram_flags = backend->share ? RAM_SHARED : 0; + ram_flags |= fb->readonly ? RAM_READONLY | RAM_READONLY_FD : 0; ram_flags |= backend->reserve ? 0 : RAM_NORESERVE; ram_flags |= fb->is_pmem ? RAM_PMEM : 0; ram_flags |= RAM_NAMED_FILE; memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name, backend->size, fb->align, ram_flags, - fb->mem_path, fb->offset, fb->readonly, - errp); + fb->mem_path, fb->offset, errp); g_free(name); #endif } diff --git a/include/exec/memory.h b/include/exec/memory.h index 68284428f8..cc68249eda 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -235,6 +235,12 @@ typedef struct IOMMUTLBEvent { /* RAM is an mmap-ed named file */ #define RAM_NAMED_FILE (1 << 9) +/* RAM is mmap-ed read-only */ +#define RAM_READONLY (1 << 10) + +/* RAM FD is opened read-only */ +#define RAM_READONLY_FD (1 << 11) + static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn, IOMMUNotifierFlag flags, hwaddr start, hwaddr end, @@ -1331,10 +1337,10 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr, * @align: alignment of the region base address; if 0, the default alignment * (getpagesize()) will be used. * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM, - * RAM_NORESERVE, + * RAM_NORESERVE, RAM_PROTECTED, RAM_NAMED_FILE, RAM_READONLY, + * RAM_READONLY_FD * @path: the path in which to allocate the RAM. * @offset: offset within the file referenced by path - * @readonly: true to open @path for reading, false for read/write. * @errp: pointer to Error*, to store an error if it happens. * * Note that this function does not do anything to cause the data in the @@ -1348,7 +1354,6 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, uint32_t ram_flags, const char *path, ram_addr_t offset, - bool readonly, Error **errp); /** @@ -1360,7 +1365,8 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, * @name: the name of the region. * @size: size of the region. * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM, - * RAM_NORESERVE, RAM_PROTECTED. + * RAM_NORESERVE, RAM_PROTECTED, RAM_NAMED_FILE, RAM_READONLY, + * RAM_READONLY_FD * @fd: the fd to mmap. * @offset: offset within the file referenced by fd * @errp: pointer to Error*, to store an error if it happens. diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index 9f2e3893f5..90676093f5 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -108,10 +108,10 @@ long qemu_maxrampagesize(void); * @size: the size in bytes of the ram block * @mr: the memory region where the ram block is * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM, - * RAM_NORESERVE. + * RAM_NORESERVE, RAM_PROTECTED, RAM_NAMED_FILE, RAM_READONLY, + * RAM_READONLY_FD * @mem_path or @fd: specify the backing file or device * @offset: Offset into target file - * @readonly: true to open @path for reading, false for read/write. * @errp: pointer to Error*, to store an error if it happens * * Return: @@ -120,10 +120,10 @@ long qemu_maxrampagesize(void); */ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, uint32_t ram_flags, const char *mem_path, - off_t offset, bool readonly, Error **errp); + off_t offset, Error **errp); RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, uint32_t ram_flags, int fd, off_t offset, - bool readonly, Error **errp); + Error **errp); RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, MemoryRegion *mr, Error **errp); diff --git a/softmmu/memory.c b/softmmu/memory.c index 7d9494ce70..2cb60ec9b8 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -1620,18 +1620,17 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, uint32_t ram_flags, const char *path, ram_addr_t offset, - bool readonly, Error **errp) { Error *err = NULL; memory_region_init(mr, owner, name, size); mr->ram = true; - mr->readonly = readonly; + mr->readonly = !!(ram_flags & RAM_READONLY); mr->terminates = true; mr->destructor = memory_region_destructor_ram; mr->align = align; mr->ram_block = qemu_ram_alloc_from_file(size, mr, ram_flags, path, - offset, readonly, &err); + offset, &err); if (err) { mr->size = int128_zero(); object_unparent(OBJECT(mr)); @@ -1651,10 +1650,11 @@ void memory_region_init_ram_from_fd(MemoryRegion *mr, Error *err = NULL; memory_region_init(mr, owner, name, size); mr->ram = true; + mr->readonly = !!(ram_flags & RAM_READONLY); mr->terminates = true; mr->destructor = memory_region_destructor_ram; mr->ram_block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, offset, - false, &err); + &err); if (err) { mr->size = int128_zero(); object_unparent(OBJECT(mr)); diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 18277ddd67..7e03ed7e3e 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -1350,7 +1350,6 @@ static int file_ram_open(const char *path, static void *file_ram_alloc(RAMBlock *block, ram_addr_t memory, int fd, - bool readonly, bool truncate, off_t offset, Error **errp) @@ -1408,7 +1407,7 @@ static void *file_ram_alloc(RAMBlock *block, perror("ftruncate"); } - qemu_map_flags = readonly ? QEMU_MAP_READONLY : 0; + qemu_map_flags = (block->flags & RAM_READONLY) ? QEMU_MAP_READONLY : 0; qemu_map_flags |= (block->flags & RAM_SHARED) ? QEMU_MAP_SHARED : 0; qemu_map_flags |= (block->flags & RAM_PMEM) ? QEMU_MAP_SYNC : 0; qemu_map_flags |= (block->flags & RAM_NORESERVE) ? QEMU_MAP_NORESERVE : 0; @@ -1876,7 +1875,7 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) #ifdef CONFIG_POSIX RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, uint32_t ram_flags, int fd, off_t offset, - bool readonly, Error **errp) + Error **errp) { RAMBlock *new_block; Error *local_err = NULL; @@ -1884,7 +1883,8 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, /* Just support these ram flags by now. */ assert((ram_flags & ~(RAM_SHARED | RAM_PMEM | RAM_NORESERVE | - RAM_PROTECTED | RAM_NAMED_FILE)) == 0); + RAM_PROTECTED | RAM_NAMED_FILE | RAM_READONLY | + RAM_READONLY_FD)) == 0); if (xen_enabled()) { error_setg(errp, "-mem-path not supported with Xen"); @@ -1919,8 +1919,8 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, new_block->used_length = size; new_block->max_length = size; new_block->flags = ram_flags; - new_block->host = file_ram_alloc(new_block, size, fd, readonly, - !file_size, offset, errp); + new_block->host = file_ram_alloc(new_block, size, fd, !file_size, offset, + errp); if (!new_block->host) { g_free(new_block); return NULL; @@ -1939,20 +1939,19 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, uint32_t ram_flags, const char *mem_path, - off_t offset, bool readonly, Error **errp) + off_t offset, Error **errp) { int fd; bool created; RAMBlock *block; - fd = file_ram_open(mem_path, memory_region_name(mr), readonly, &created, - errp); + fd = file_ram_open(mem_path, memory_region_name(mr), + !!(ram_flags & RAM_READONLY_FD), &created, errp); if (fd < 0) { return NULL; } - block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, offset, readonly, - errp); + block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, offset, errp); if (!block) { if (created) { unlink(mem_path); From e92666b0ba4cbaa71a5dd98c31414926a9915487 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 6 Sep 2023 14:04:55 +0200 Subject: [PATCH 03/12] backends/hostmem-file: Add "rom" property to support VM templating with R/O files For now, "share=off,readonly=on" would always result in us opening the file R/O and mmap'ing the opened file MAP_PRIVATE R/O -- effectively turning it into ROM. Especially for VM templating, "share=off" is a common use case. However, that use case is impossible with files that lack write permissions, because "share=off,readonly=on" will not give us writable RAM. The sole user of ROM via memory-backend-file are R/O NVDIMMs, but as we have users (Kata Containers) that rely on the existing behavior -- malicious VMs should not be able to consume COW memory for R/O NVDIMMs -- we cannot change the semantics of "share=off,readonly=on" So let's add a new "rom" property with on/off/auto values. "auto" is the default and what most people will use: for historical reasons, to not change the old semantics, it defaults to the value of the "readonly" property. For VM templating, one can now use: -object memory-backend-file,share=off,readonly=on,rom=off,... But we'll disallow: -object memory-backend-file,share=on,readonly=on,rom=off,... because we would otherwise get an error when trying to mmap the R/O file shared and writable. An explicit error message is cleaner. We will also disallow for now: -object memory-backend-file,share=off,readonly=off,rom=on,... -object memory-backend-file,share=on,readonly=off,rom=on,... It's not harmful, but also not really required for now. Alternatives that were abandoned: * Make "unarmed=on" for the NVDIMM set the memory region container readonly. We would still see a change of ROM->RAM and possibly run into memslot limits with vhost-user. Further, there might be use cases for "unarmed=on" that should still allow writing to that memory (temporary files, system RAM, ...). * Add a new "readonly=on/off/auto" parameter for NVDIMMs. Similar issues as with "unarmed=on". * Make "readonly" consume "on/off/file" instead of being a 'bool' type. This would slightly changes the behavior of the "readonly" parameter: values like true/false (as accepted by a 'bool'type) would no longer be accepted. Message-ID: <20230906120503.359863-4-david@redhat.com> Acked-by: Markus Armbruster Signed-off-by: David Hildenbrand --- backends/hostmem-file.c | 59 ++++++++++++++++++++++++++++++++++++++++- qapi/qom.json | 17 +++++++++++- qemu-options.hx | 16 ++++++++++- 3 files changed, 89 insertions(+), 3 deletions(-) diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c index ef2d5533af..361d4a8103 100644 --- a/backends/hostmem-file.c +++ b/backends/hostmem-file.c @@ -18,6 +18,8 @@ #include "sysemu/hostmem.h" #include "qom/object_interfaces.h" #include "qom/object.h" +#include "qapi/visitor.h" +#include "qapi/qapi-visit-common.h" OBJECT_DECLARE_SIMPLE_TYPE(HostMemoryBackendFile, MEMORY_BACKEND_FILE) @@ -31,6 +33,7 @@ struct HostMemoryBackendFile { bool discard_data; bool is_pmem; bool readonly; + OnOffAuto rom; }; static void @@ -53,9 +56,33 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) return; } + switch (fb->rom) { + case ON_OFF_AUTO_AUTO: + /* Traditionally, opening the file readonly always resulted in ROM. */ + fb->rom = fb->readonly ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; + break; + case ON_OFF_AUTO_ON: + if (!fb->readonly) { + error_setg(errp, "property 'rom' = 'on' is not supported with" + " 'readonly' = 'off'"); + return; + } + break; + case ON_OFF_AUTO_OFF: + if (fb->readonly && backend->share) { + error_setg(errp, "property 'rom' = 'off' is incompatible with" + " 'readonly' = 'on' and 'share' = 'on'"); + return; + } + break; + default: + assert(false); + } + name = host_memory_backend_get_name(backend); ram_flags = backend->share ? RAM_SHARED : 0; - ram_flags |= fb->readonly ? RAM_READONLY | RAM_READONLY_FD : 0; + ram_flags |= fb->readonly ? RAM_READONLY_FD : 0; + ram_flags |= fb->rom == ON_OFF_AUTO_ON ? RAM_READONLY : 0; ram_flags |= backend->reserve ? 0 : RAM_NORESERVE; ram_flags |= fb->is_pmem ? RAM_PMEM : 0; ram_flags |= RAM_NAMED_FILE; @@ -201,6 +228,32 @@ static void file_memory_backend_set_readonly(Object *obj, bool value, fb->readonly = value; } +static void file_memory_backend_get_rom(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj); + OnOffAuto rom = fb->rom; + + visit_type_OnOffAuto(v, name, &rom, errp); +} + +static void file_memory_backend_set_rom(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ + HostMemoryBackend *backend = MEMORY_BACKEND(obj); + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj); + + if (host_memory_backend_mr_inited(backend)) { + error_setg(errp, "cannot change property '%s' of %s.", name, + object_get_typename(obj)); + return; + } + + visit_type_OnOffAuto(v, name, &fb->rom, errp); +} + static void file_backend_unparent(Object *obj) { HostMemoryBackend *backend = MEMORY_BACKEND(obj); @@ -243,6 +296,10 @@ file_backend_class_init(ObjectClass *oc, void *data) object_class_property_add_bool(oc, "readonly", file_memory_backend_get_readonly, file_memory_backend_set_readonly); + object_class_property_add(oc, "rom", "OnOffAuto", + file_memory_backend_get_rom, file_memory_backend_set_rom, NULL, NULL); + object_class_property_set_description(oc, "rom", + "Whether to create Read Only Memory (ROM)"); } static void file_backend_instance_finalize(Object *o) diff --git a/qapi/qom.json b/qapi/qom.json index fa3e88c8e6..c53ef978ff 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -668,6 +668,20 @@ # @readonly: if true, the backing file is opened read-only; if false, # it is opened read-write. (default: false) # +# @rom: whether to create Read Only Memory (ROM) that cannot be modified +# by the VM. Any write attempts to such ROM will be denied. Most +# use cases want writable RAM instead of ROM. However, selected use +# cases, like R/O NVDIMMs, can benefit from ROM. If set to 'on', +# create ROM; if set to 'off', create writable RAM; if set to +# 'auto', the value of the @readonly property is used. This +# property is primarily helpful when we want to have proper RAM in +# configurations that would traditionally create ROM before this +# property was introduced: VM templating, where we want to open a +# file readonly (@readonly set to true) and mark the memory to be +# private for QEMU (@share set to false). For this use case, we need +# writable RAM instead of ROM, and want to set this property to 'off'. +# (default: auto, since 8.2) +# # Since: 2.1 ## { 'struct': 'MemoryBackendFileProperties', @@ -677,7 +691,8 @@ '*discard-data': 'bool', 'mem-path': 'str', '*pmem': { 'type': 'bool', 'if': 'CONFIG_LIBPMEM' }, - '*readonly': 'bool' } } + '*readonly': 'bool', + '*rom': 'OnOffAuto' } } ## # @MemoryBackendMemfdProperties: diff --git a/qemu-options.hx b/qemu-options.hx index 6be621c232..3aabd6e0cc 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4995,7 +4995,7 @@ SRST they are specified. Note that the 'id' property must be set. These objects are placed in the '/objects' path. - ``-object memory-backend-file,id=id,size=size,mem-path=dir,share=on|off,discard-data=on|off,merge=on|off,dump=on|off,prealloc=on|off,host-nodes=host-nodes,policy=default|preferred|bind|interleave,align=align,offset=offset,readonly=on|off`` + ``-object memory-backend-file,id=id,size=size,mem-path=dir,share=on|off,discard-data=on|off,merge=on|off,dump=on|off,prealloc=on|off,host-nodes=host-nodes,policy=default|preferred|bind|interleave,align=align,offset=offset,readonly=on|off,rom=on|off|auto`` Creates a memory file backend object, which can be used to back the guest RAM with huge pages. @@ -5085,6 +5085,20 @@ SRST The ``readonly`` option specifies whether the backing file is opened read-only or read-write (default). + The ``rom`` option specifies whether to create Read Only Memory + (ROM) that cannot be modified by the VM. Any write attempts to such + ROM will be denied. Most use cases want proper RAM instead of ROM. + However, selected use cases, like R/O NVDIMMs, can benefit from + ROM. If set to ``on``, create ROM; if set to ``off``, create + writable RAM; if set to ``auto`` (default), the value of the + ``readonly`` option is used. This option is primarily helpful when + we want to have writable RAM in configurations that would + traditionally create ROM before the ``rom`` option was introduced: + VM templating, where we want to open a file readonly + (``readonly=on``) and mark the memory to be private for QEMU + (``share=off``). For this use case, we need writable RAM instead + of ROM, and want to also set ``rom=off``. + ``-object memory-backend-ram,id=id,merge=on|off,dump=on|off,share=on|off,prealloc=on|off,size=size,host-nodes=host-nodes,policy=default|preferred|bind|interleave`` Creates a memory backend object, which can be used to back the guest RAM. Memory backend objects offer more control than the From 9e6b9f379130c77a08c8ea67a09fc90cb30f8d09 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 6 Sep 2023 14:04:56 +0200 Subject: [PATCH 04/12] softmmu/physmem: Remap with proper protection in qemu_ram_remap() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's remap with the proper protection that we can derive from RAM_READONLY. Message-ID: <20230906120503.359863-5-david@redhat.com> Reviewed-by: Peter Xu Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Hildenbrand --- softmmu/physmem.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 7e03ed7e3e..88482bd32a 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -2069,6 +2069,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length) ram_addr_t offset; int flags; void *area, *vaddr; + int prot; RAMBLOCK_FOREACH(block) { offset = addr - block->offset; @@ -2083,13 +2084,14 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length) flags |= block->flags & RAM_SHARED ? MAP_SHARED : MAP_PRIVATE; flags |= block->flags & RAM_NORESERVE ? MAP_NORESERVE : 0; + prot = PROT_READ; + prot |= block->flags & RAM_READONLY ? 0 : PROT_WRITE; if (block->fd >= 0) { - area = mmap(vaddr, length, PROT_READ | PROT_WRITE, - flags, block->fd, offset + block->fd_offset); + area = mmap(vaddr, length, prot, flags, block->fd, + offset + block->fd_offset); } else { flags |= MAP_ANONYMOUS; - area = mmap(vaddr, length, PROT_READ | PROT_WRITE, - flags, -1, 0); + area = mmap(vaddr, length, prot, flags, -1, 0); } if (area != vaddr) { error_report("Could not remap addr: " From b2cccb52bd9bef2948a150d204b20119b6c3ad58 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 6 Sep 2023 14:04:57 +0200 Subject: [PATCH 05/12] softmmu/physmem: Bail out early in ram_block_discard_range() with readonly files fallocate() will fail, let's print a nicer error message. Message-ID: <20230906120503.359863-6-david@redhat.com> Suggested-by: Peter Xu Reviewed-by: Peter Xu Signed-off-by: David Hildenbrand --- softmmu/physmem.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 88482bd32a..c520c2ac55 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -3481,6 +3481,16 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) * so a userfault will trigger. */ #ifdef CONFIG_FALLOCATE_PUNCH_HOLE + /* + * fallocate() will fail with readonly files. Let's print a + * proper error message. + */ + if (rb->flags & RAM_READONLY_FD) { + error_report("ram_block_discard_range: Discarding RAM" + " with readonly files is not supported"); + goto err; + + } /* * We'll discard data from the actual file, even though we only * have a MAP_PRIVATE mapping, possibly messing with other From 4d6b23f7e2b7a34a6ab3b7c40693d8b1a0dee0b5 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 6 Sep 2023 14:04:58 +0200 Subject: [PATCH 06/12] softmmu/physmem: Fail creation of new files in file_ram_open() with readonly=true Currently, if a file does not exist yet, file_ram_open() will create new empty file and open it writable. However, it even does that when readonly=true was specified. Specifying O_RDONLY instead to create a new readonly file would theoretically work, however, ftruncate() will refuse to resize the new empty file and we'll get a warning: ftruncate: Invalid argument And later eventually more problems when actually mmap'ing that file and accessing it. If someone intends to let QEMU open+mmap a file read-only, better create+resize+fill that file ahead of time outside of QEMU context. We'll now fail with: ./qemu-system-x86_64 \ -object memory-backend-file,id=ram0,mem-path=tmp,readonly=true,size=1g qemu-system-x86_64: can't open backing store tmp for guest RAM: No such file or directory All use cases of readonly files (R/O NVDIMMs, VM templating) work on existing files, so silently creating new files might just hide user errors when accidentally specifying a non-existent file. Note that the only memory-backend-file will end up calling memory_region_init_ram_from_file() -> qemu_ram_alloc_from_file() -> file_ram_open(). Move error reporting to the single caller. Message-ID: <20230906120503.359863-7-david@redhat.com> Acked-by: Peter Xu Signed-off-by: David Hildenbrand --- softmmu/physmem.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/softmmu/physmem.c b/softmmu/physmem.c index c520c2ac55..138402b6cf 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -1288,8 +1288,7 @@ static int64_t get_file_align(int fd) static int file_ram_open(const char *path, const char *region_name, bool readonly, - bool *created, - Error **errp) + bool *created) { char *filename; char *sanitized_name; @@ -1304,6 +1303,10 @@ static int file_ram_open(const char *path, break; } if (errno == ENOENT) { + if (readonly) { + /* Refuse to create new, readonly files. */ + return -ENOENT; + } /* @path names a file that doesn't exist, create it */ fd = open(path, O_RDWR | O_CREAT | O_EXCL, 0644); if (fd >= 0) { @@ -1333,10 +1336,7 @@ static int file_ram_open(const char *path, g_free(filename); } if (errno != EEXIST && errno != EINTR) { - error_setg_errno(errp, errno, - "can't open backing store %s for guest RAM", - path); - return -1; + return -errno; } /* * Try again on EINTR and EEXIST. The latter happens when @@ -1946,8 +1946,10 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, RAMBlock *block; fd = file_ram_open(mem_path, memory_region_name(mr), - !!(ram_flags & RAM_READONLY_FD), &created, errp); + !!(ram_flags & RAM_READONLY_FD), &created); if (fd < 0) { + error_setg_errno(errp, -fd, "can't open backing store %s for guest RAM", + mem_path); return NULL; } From ca01f1b89b0884e39a58d8bc1d3fbd7ca91272a1 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 6 Sep 2023 14:04:59 +0200 Subject: [PATCH 07/12] softmmu/physmem: Never return directories from file_ram_open() open() does not fail on directories when opening them readonly (O_RDONLY). Currently, we succeed opening such directories and fail later during mmap(), resulting in a misleading error message. $ ./qemu-system-x86_64 \ -object memory-backend-file,id=ram0,mem-path=tmp,readonly=true,size=1g qemu-system-x86_64: unable to map backing store for guest RAM: No such device To identify directories and handle them accordingly in file_ram_open() also when readonly=true was specified, detect if we just opened a directory using fstat() instead. Then, fail file_ram_open() right away, similarly to how we now fail if the file does not exist and we want to open the file readonly. With this change, we get a nicer error message: qemu-system-x86_64: can't open backing store tmp for guest RAM: Is a directory Note that the only memory-backend-file will end up calling memory_region_init_ram_from_file() -> qemu_ram_alloc_from_file() -> file_ram_open(). Message-ID: <20230906120503.359863-8-david@redhat.com> Reported-by: Thiner Logoer Reviewed-by: Peter Xu Tested-by: Mario Casquero Signed-off-by: David Hildenbrand --- softmmu/physmem.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 138402b6cf..f1cd3ec28a 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -1299,6 +1299,25 @@ static int file_ram_open(const char *path, for (;;) { fd = open(path, readonly ? O_RDONLY : O_RDWR); if (fd >= 0) { + /* + * open(O_RDONLY) won't fail with EISDIR. Check manually if we + * opened a directory and fail similarly to how we fail ENOENT + * in readonly mode. Note that mkstemp() would imply O_RDWR. + */ + if (readonly) { + struct stat file_stat; + + if (fstat(fd, &file_stat)) { + close(fd); + if (errno == EINTR) { + continue; + } + return -errno; + } else if (S_ISDIR(file_stat.st_mode)) { + close(fd); + return -EISDIR; + } + } /* @path names an existing file, use it */ break; } From 9e6180d22c482e09e9fac97cf45a09b58109287e Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 6 Sep 2023 14:05:00 +0200 Subject: [PATCH 08/12] docs: Don't mention "-mem-path" in multi-process.rst "-mem-path" corresponds to "memory-backend-file,share=off" and, therefore, creates a private COW mapping of the file. For multi-proces QEMU, we need proper shared file-backed memory. Let's make that clearer. Message-ID: <20230906120503.359863-9-david@redhat.com> Signed-off-by: David Hildenbrand --- docs/devel/multi-process.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/devel/multi-process.rst b/docs/devel/multi-process.rst index e4801751f2..4ef539c0b0 100644 --- a/docs/devel/multi-process.rst +++ b/docs/devel/multi-process.rst @@ -409,8 +409,9 @@ the initial messages sent to the emulation process is a guest memory table. Each entry in this table consists of a file descriptor and size that the emulation process can ``mmap()`` to directly access guest memory, similar to ``vhost_user_set_mem_table()``. Note guest memory -must be backed by file descriptors, such as when QEMU is given the -*-mem-path* command line option. +must be backed by shared file-backed memory, for example, using +*-object memory-backend-file,share=on* and setting that memory backend +as RAM for the machine. IOMMU operations ^^^^^^^^^^^^^^^^ From 9cd9313fc3cb94243512bf36d0d67d4dbd60d6f1 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 6 Sep 2023 14:05:01 +0200 Subject: [PATCH 09/12] docs: Start documenting VM templating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's add some details about VM templating, focusing on the VM memory configuration only. There is much more to VM templating (VM state? block devices?), but I leave that as future work. Message-ID: <20230906120503.359863-10-david@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Hildenbrand --- MAINTAINERS | 1 + docs/system/index.rst | 1 + docs/system/vm-templating.rst | 125 ++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+) create mode 100644 docs/system/vm-templating.rst diff --git a/MAINTAINERS b/MAINTAINERS index 00562f924f..3cf53553fc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2963,6 +2963,7 @@ M: Igor Mammedov S: Maintained F: backends/hostmem*.c F: include/sysemu/hostmem.h +F: docs/system/vm-templating.rst T: git https://gitlab.com/ehabkost/qemu.git machine-next Cryptodev Backends diff --git a/docs/system/index.rst b/docs/system/index.rst index 45bf1f19e7..c21065e519 100644 --- a/docs/system/index.rst +++ b/docs/system/index.rst @@ -38,3 +38,4 @@ or Hypervisor.Framework. security multi-process confidential-guest-support + vm-templating diff --git a/docs/system/vm-templating.rst b/docs/system/vm-templating.rst new file mode 100644 index 0000000000..28905a1eeb --- /dev/null +++ b/docs/system/vm-templating.rst @@ -0,0 +1,125 @@ +QEMU VM templating +================== + +This document explains how to use VM templating in QEMU. + +For now, the focus is on VM memory aspects, and not about how to save and +restore other VM state (i.e., migrate-to-file with ``x-ignore-shared``). + +Overview +-------- + +With VM templating, a single template VM serves as the starting point for +new VMs. This allows for fast and efficient replication of VMs, resulting +in fast startup times and reduced memory consumption. + +Conceptually, the VM state is frozen, to then be used as a basis for new +VMs. The Copy-On-Write mechanism in the operating systems makes sure that +new VMs are able to read template VM memory; however, any modifications +stay private and don't modify the original template VM or any other +created VM. + +!!! Security Alert !!! +---------------------- + +When effectively cloning VMs by VM templating, hardware identifiers +(such as UUIDs and NIC MAC addresses), and similar data in the guest OS +(such as machine IDs, SSH keys, certificates) that are supposed to be +*unique* are no longer unique, which can be a security concern. + +Please be aware of these implications and how to mitigate them for your +use case, which might involve vmgenid, hot(un)plug of NIC, etc.. + +Memory configuration +-------------------- + +In order to create the template VM, we have to make sure that VM memory +ends up in a file, from where it can be reused for the new VMs: + +Supply VM RAM via memory-backend-file, with ``share=on`` (modifications go +to the file) and ``readonly=off`` (open the file writable). Note that +``readonly=off`` is implicit. + +In the following command-line example, a 2GB VM is created, whereby VM RAM +is to be stored in the ``template`` file. + +.. parsed-literal:: + + |qemu_system| [...] -m 2g \\ + -object memory-backend-file,id=pc.ram,mem-path=template,size=2g,share=on,... \\ + -machine q35,memory-backend=pc.ram + +If multiple memory backends are used (vNUMA, DIMMs), configure all +memory backends accordingly. + +Once the VM is in the desired state, stop the VM and save other VM state, +leaving the current state of VM RAM reside in the file. + +In order to have a new VM be based on a template VM, we have to +configure VM RAM to be based on a template VM RAM file; however, the VM +should not be able to modify file content. + +Supply VM RAM via memory-backend-file, with ``share=off`` (modifications +stay private), ``readonly=on`` (open the file readonly) and ``rom=off`` +(don't make the memory readonly for the VM). Note that ``share=off`` is +implicit and that other VM state has to be restored separately. + +In the following command-line example, a 2GB VM is created based on the +existing 2GB file ``template``. + +.. parsed-literal:: + + |qemu_system| [...] -m 2g \\ + -object memory-backend-file,id=pc.ram,mem-path=template,size=2g,readonly=on,rom=off,... \\ + -machine q35,memory-backend=pc.ram + +If multiple memory backends are used (vNUMA, DIMMs), configure all +memory backends accordingly. + +Note that ``-mem-path`` cannot be used for VM templating when creating the +template VM or when starting new VMs based on a template VM. + +Incompatible features +--------------------- + +Some features are incompatible with VM templating, as the underlying file +cannot be modified to discard VM RAM, or to actually share memory with +another process. + +vhost-user and multi-process QEMU +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +vhost-user and multi-process QEMU are incompatible with VM templating. +These technologies rely on shared memory, however, the template VMs +don't actually share memory (``share=off``), even though they are +file-based. + +virtio-balloon +~~~~~~~~~~~~~~ + +virtio-balloon inflation and "free page reporting" cannot discard VM RAM +and will repeatedly report errors. While virtio-balloon can be used +for template VMs (e.g., report VM RAM stats), "free page reporting" +should be disabled and the balloon should not be inflated. + +virtio-mem +~~~~~~~~~~ + +virtio-mem cannot discard VM RAM that is managed by the virtio-mem +device. virtio-mem will fail early when realizing the device. To use +VM templating with virtio-mem, either hotplug virtio-mem devices to the +new VM, or don't supply any memory to the template VM using virtio-mem +(requested-size=0), not using a template VM file as memory backend for the +virtio-mem device. + +VM migration +~~~~~~~~~~~~ + +For VM migration, "x-release-ram" similarly relies on discarding of VM +RAM on the migration source to free up migrated RAM, and will +repeatedly report errors. + +Postcopy live migration fails discarding VM RAM on the migration +destination early and refuses to activate postcopy live migration. Note +that postcopy live migration usually only works on selected filesystems +(shmem/tmpfs, hugetlbfs) either way. From 6da4b1c25d89cc8e8d1be40b96602d08a3e88e0c Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 6 Sep 2023 14:05:02 +0200 Subject: [PATCH 10/12] softmmu/physmem: Hint that "readonly=on,rom=off" exists when opening file R/W for private mapping fails It's easy to miss that memory-backend-file with "share=off" (default) will always try opening the file R/W as default, and fail if we don't have write permissions to the file. In that case, the user has to explicit specify "readonly=on,rom=off" to get usable RAM, for example, for VM templating. Let's hint that '-object memory-backend-file,readonly=on,rom=off,...' exists to consume R/O files in a private mapping to create writable RAM, but only if we have permissions to open the file read-only. Message-ID: <20230906120503.359863-11-david@redhat.com> Suggested-by: ThinerLogoer Signed-off-by: David Hildenbrand --- softmmu/physmem.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/softmmu/physmem.c b/softmmu/physmem.c index f1cd3ec28a..4f6ca653b3 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -1969,6 +1969,25 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, if (fd < 0) { error_setg_errno(errp, -fd, "can't open backing store %s for guest RAM", mem_path); + if (!(ram_flags & RAM_READONLY_FD) && !(ram_flags & RAM_SHARED) && + fd == -EACCES) { + /* + * If we can open the file R/O (note: will never create a new file) + * and we are dealing with a private mapping, there are still ways + * to consume such files and get RAM instead of ROM. + */ + fd = file_ram_open(mem_path, memory_region_name(mr), true, + &created); + if (fd < 0) { + return NULL; + } + assert(!created); + close(fd); + error_append_hint(errp, "Consider opening the backing store" + " read-only but still creating writable RAM using" + " '-object memory-backend-file,readonly=on,rom=off...'" + " (see \"VM templating\" documentation)\n"); + } return NULL; } From 41ddcd2308f76cc95459c2961f4a50b66a70d3c4 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 6 Sep 2023 14:05:03 +0200 Subject: [PATCH 11/12] machine: Improve error message when using default RAM backend id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For migration purposes, users might want to reuse the default RAM backend id, but specify a different memory backend. For example, to reuse "pc.ram" on q35, one has to set -machine q35,memory-backend=pc.ram Only then, can a memory backend with the id "pc.ram" be created manually. Let's improve the error message by improving the hint. Use error_append_hint() -- which in turn requires ERRP_GUARD(). Message-ID: <20230906120503.359863-12-david@redhat.com> Suggested-by: ThinerLogoer Reviewed-by: Philippe Mathieu-Daudé Tested-by: Mario Casquero Reviewed-by: Markus Armbruster Signed-off-by: David Hildenbrand --- hw/core/machine.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index da699cf4e1..db0c263ff6 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -1355,6 +1355,7 @@ out: void machine_run_board_init(MachineState *machine, const char *mem_path, Error **errp) { + ERRP_GUARD(); MachineClass *machine_class = MACHINE_GET_CLASS(machine); ObjectClass *oc = object_class_by_name(machine->cpu_type); CPUClass *cc; @@ -1383,9 +1384,13 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error * numa_uses_legacy_mem()) { if (object_property_find(object_get_objects_root(), machine_class->default_ram_id)) { - error_setg(errp, "object name '%s' is reserved for the default" - " RAM backend, it can't be used for any other purposes." - " Change the object's 'id' to something else", + error_setg(errp, "object's id '%s' is reserved for the default" + " RAM backend, it can't be used for any other purposes", + machine_class->default_ram_id); + error_append_hint(errp, + "Change the object's 'id' to something else or disable" + " automatic creation of the default RAM backend by setting" + " 'memory-backend=%s' with '-machine'.\n", machine_class->default_ram_id); return; } From 544cff46c018036cd66e98ffb224dd9f098065c8 Mon Sep 17 00:00:00 2001 From: hongmianquan Date: Wed, 30 Aug 2023 11:29:06 +0800 Subject: [PATCH 12/12] memory: avoid updating ioeventfds for some address_space When updating ioeventfds, we need to iterate all address spaces, but some address spaces do not register eventfd_add|del call when memory_listener_register() and they do nothing when updating ioeventfds. So we can skip these AS in address_space_update_ioeventfds(). The overhead of memory_region_transaction_commit() can be significantly reduced. For example, a VM with 8 vhost net devices and each one has 64 vectors, can reduce the time spent on memory_region_transaction_commit by 20%. Message-ID: <20230830032906.12488-1-hongmianquan@bytedance.com> Reviewed-by: Peter Xu Signed-off-by: hongmianquan Signed-off-by: David Hildenbrand --- include/exec/memory.h | 1 + softmmu/memory.c | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index cc68249eda..ef23d65afc 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -1095,6 +1095,7 @@ struct AddressSpace { struct FlatView *current_map; int ioeventfd_nb; + int ioeventfd_notifiers; struct MemoryRegionIoeventfd *ioeventfds; QTAILQ_HEAD(, MemoryListener) listeners; QTAILQ_ENTRY(AddressSpace) address_spaces_link; diff --git a/softmmu/memory.c b/softmmu/memory.c index 2cb60ec9b8..c0383a163d 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -842,6 +842,10 @@ static void address_space_update_ioeventfds(AddressSpace *as) AddrRange tmp; unsigned i; + if (!as->ioeventfd_notifiers) { + return; + } + /* * It is likely that the number of ioeventfds hasn't changed much, so use * the previous size as the starting value, with some headroom to avoid @@ -3075,6 +3079,10 @@ void memory_listener_register(MemoryListener *listener, AddressSpace *as) } listener_add_address_space(listener, as); + + if (listener->eventfd_add || listener->eventfd_del) { + as->ioeventfd_notifiers++; + } } void memory_listener_unregister(MemoryListener *listener) @@ -3083,6 +3091,10 @@ void memory_listener_unregister(MemoryListener *listener) return; } + if (listener->eventfd_add || listener->eventfd_del) { + listener->address_space->ioeventfd_notifiers--; + } + listener_del_address_space(listener, listener->address_space); QTAILQ_REMOVE(&memory_listeners, listener, link); QTAILQ_REMOVE(&listener->address_space->listeners, listener, link_as);