Hello, At 2023-08-22 19:44:50, "David Hildenbrand" <da...@redhat.com> wrote: >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. > >Signed-off-by: David Hildenbrand <da...@redhat.com> >--- > 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..d8974a1e65 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;
I only did a quick code auditing, but I suspect that ``` mr->readonly = !!(ram_flags & RAM_READONLY); ``` is safer. So is the other parts of the code probably. > 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; for example here. -- Regards, logoerthiner