All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] nvdimm: read-only file support
@ 2021-01-04 17:13 Stefan Hajnoczi
  2021-01-04 17:13 ` [PATCH v3 1/3] memory: add readonly support to memory_region_init_ram_from_file() Stefan Hajnoczi
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2021-01-04 17:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Xiao Guangrong, Michael S. Tsirkin, Julio Montes,
	Eduardo Habkost, eric.g.ernst, Stefan Hajnoczi, Igor Mammedov,
	Paolo Bonzini, Richard Henderson

v3:
 * Produce an error when -device nvdimm,unarmed=off is used with -object
   memory-backend-file,readonly=on instead of silently switching on
   unarmed. [Igor]
 * Use Object *obj instead of Object *o [Igor]
 * Do not dereference MEMORY_BACKEND_FILE(o)->readonly directly, use a
   local variable to hold the HostMemoryBackendFile pointer. [Igor]
v2:
 * s/the the/the/ in documentation [Philippe]
 * Assign nvdimm->unarmed earlier [Philippe]

There is currently no way to back an NVDIMM with a read-only file so it can be
safely shared between untrusted guests.

Introduce an -object memory-backend-file,readonly=on|off option.

Julio Montes sent an earlier patch here:
https://patchew.org/QEMU/20190708211936.8037-1-julio.montes@intel.com/

Eric Ernst requested this feature again for Kata Containers so I gave it a try.

Stefan Hajnoczi (3):
  memory: add readonly support to memory_region_init_ram_from_file()
  hostmem-file: add readonly=on|off option
  nvdimm: check -object memory-backend-file,readonly=on option

 docs/nvdimm.txt           | 24 +++++++++++++++++-------
 include/exec/memory.h     |  2 ++
 include/exec/ram_addr.h   |  5 +++--
 include/qemu/mmap-alloc.h |  2 ++
 backends/hostmem-file.c   | 28 +++++++++++++++++++++++++++-
 hw/mem/nvdimm.c           |  9 +++++++++
 softmmu/memory.c          |  7 +++++--
 softmmu/physmem.c         | 18 +++++++++++-------
 util/mmap-alloc.c         | 10 ++++++----
 util/oslib-posix.c        |  2 +-
 qemu-options.hx           |  5 ++++-
 11 files changed, 87 insertions(+), 25 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/3] memory: add readonly support to memory_region_init_ram_from_file()
  2021-01-04 17:13 [PATCH v3 0/3] nvdimm: read-only file support Stefan Hajnoczi
@ 2021-01-04 17:13 ` Stefan Hajnoczi
  2021-01-04 17:13 ` [PATCH v3 2/3] hostmem-file: add readonly=on|off option Stefan Hajnoczi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2021-01-04 17:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Xiao Guangrong, Michael S. Tsirkin, Julio Montes,
	Eduardo Habkost, eric.g.ernst, Stefan Hajnoczi, Igor Mammedov,
	Paolo Bonzini, Philippe Mathieu-Daudé,
	Richard Henderson

There is currently no way to open(O_RDONLY) and mmap(PROT_READ) when
creating a memory region from a file. This functionality is needed since
the underlying host file may not allow writing.

Add a bool readonly argument to memory_region_init_ram_from_file() and
the APIs it calls.

Extend memory_region_init_ram_from_file() rather than introducing a
memory_region_init_rom_from_file() API so that callers can easily make a
choice between read/write and read-only at runtime without calling
different APIs.

No new RAMBlock flag is introduced for read-only because it's unclear
whether RAMBlocks need to know that they are read-only. Pass a bool
readonly argument instead.

Both of these design decisions can be changed in the future. It just
seemed like the simplest approach to me.

Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/exec/memory.h     |  2 ++
 include/exec/ram_addr.h   |  5 +++--
 include/qemu/mmap-alloc.h |  2 ++
 backends/hostmem-file.c   |  2 +-
 softmmu/memory.c          |  7 +++++--
 softmmu/physmem.c         | 18 +++++++++++-------
 util/mmap-alloc.c         | 10 ++++++----
 util/oslib-posix.c        |  2 +-
 8 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 521d9901d7..c6ce74fb79 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -966,6 +966,7 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
  *             - RAM_PMEM: the memory is persistent memory
  *             Other bits are ignored now.
  * @path: the path in which to allocate the RAM.
+ * @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
@@ -978,6 +979,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
                                       uint64_t align,
                                       uint32_t ram_flags,
                                       const char *path,
+                                      bool readonly,
                                       Error **errp);
 
 /**
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index c6d2ef1d07..40b16609ab 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -110,6 +110,7 @@ long qemu_maxrampagesize(void);
  *              - RAM_PMEM: the backend @mem_path or @fd is persistent memory
  *              Other bits are ignored.
  *  @mem_path or @fd: specify the backing file or device
+ *  @readonly: true to open @path for reading, false for read/write.
  *  @errp: pointer to Error*, to store an error if it happens
  *
  * Return:
@@ -118,9 +119,9 @@ long qemu_maxrampagesize(void);
  */
 RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
                                    uint32_t ram_flags, const char *mem_path,
-                                   Error **errp);
+                                   bool readonly, Error **errp);
 RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
-                                 uint32_t ram_flags, int fd,
+                                 uint32_t ram_flags, int fd, bool readonly,
                                  Error **errp);
 
 RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h
index e786266b92..8b7a5c70f3 100644
--- a/include/qemu/mmap-alloc.h
+++ b/include/qemu/mmap-alloc.h
@@ -14,6 +14,7 @@ size_t qemu_mempath_getpagesize(const char *mem_path);
  *  @size: the number of bytes to be mmaped
  *  @align: if not zero, specify the alignment of the starting mapping address;
  *          otherwise, the alignment in use will be determined by QEMU.
+ *  @readonly: true for a read-only mapping, false for read/write.
  *  @shared: map has RAM_SHARED flag.
  *  @is_pmem: map has RAM_PMEM flag.
  *
@@ -24,6 +25,7 @@ size_t qemu_mempath_getpagesize(const char *mem_path);
 void *qemu_ram_mmap(int fd,
                     size_t size,
                     size_t align,
+                    bool readonly,
                     bool shared,
                     bool is_pmem);
 
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 40e1e5b3e3..e827692124 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -56,7 +56,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
                                      backend->size, fb->align,
                                      (backend->share ? RAM_SHARED : 0) |
                                      (fb->is_pmem ? RAM_PMEM : 0),
-                                     fb->mem_path, errp);
+                                     fb->mem_path, false, errp);
     g_free(name);
 #endif
 }
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 333e1ed7b0..676c298b60 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1587,15 +1587,18 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
                                       uint64_t align,
                                       uint32_t ram_flags,
                                       const char *path,
+                                      bool readonly,
                                       Error **errp)
 {
     Error *err = NULL;
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
+    mr->readonly = 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, &err);
+    mr->ram_block = qemu_ram_alloc_from_file(size, mr, ram_flags, path,
+                                             readonly, &err);
     if (err) {
         mr->size = int128_zero();
         object_unparent(OBJECT(mr));
@@ -1618,7 +1621,7 @@ void memory_region_init_ram_from_fd(MemoryRegion *mr,
     mr->destructor = memory_region_destructor_ram;
     mr->ram_block = qemu_ram_alloc_from_fd(size, mr,
                                            share ? RAM_SHARED : 0,
-                                           fd, &err);
+                                           fd, false, &err);
     if (err) {
         mr->size = int128_zero();
         object_unparent(OBJECT(mr));
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 2cd1de4a2c..5753cfc39f 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -1398,6 +1398,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)
 {
@@ -1408,7 +1409,7 @@ static int file_ram_open(const char *path,
 
     *created = false;
     for (;;) {
-        fd = open(path, O_RDWR);
+        fd = open(path, readonly ? O_RDONLY : O_RDWR);
         if (fd >= 0) {
             /* @path names an existing file, use it */
             break;
@@ -1460,6 +1461,7 @@ 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,
                             Error **errp)
 {
@@ -1510,7 +1512,7 @@ static void *file_ram_alloc(RAMBlock *block,
         perror("ftruncate");
     }
 
-    area = qemu_ram_mmap(fd, memory, block->mr->align,
+    area = qemu_ram_mmap(fd, memory, block->mr->align, readonly,
                          block->flags & RAM_SHARED, block->flags & RAM_PMEM);
     if (area == MAP_FAILED) {
         error_setg_errno(errp, errno,
@@ -1942,7 +1944,7 @@ static void ram_block_add(RAMBlock *new_block, Error **errp, bool shared)
 
 #ifdef CONFIG_POSIX
 RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
-                                 uint32_t ram_flags, int fd,
+                                 uint32_t ram_flags, int fd, bool readonly,
                                  Error **errp)
 {
     RAMBlock *new_block;
@@ -1996,7 +1998,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, !file_size, errp);
+    new_block->host = file_ram_alloc(new_block, size, fd, readonly,
+                                     !file_size, errp);
     if (!new_block->host) {
         g_free(new_block);
         return NULL;
@@ -2015,18 +2018,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,
-                                   Error **errp)
+                                   bool readonly, Error **errp)
 {
     int fd;
     bool created;
     RAMBlock *block;
 
-    fd = file_ram_open(mem_path, memory_region_name(mr), &created, errp);
+    fd = file_ram_open(mem_path, memory_region_name(mr), readonly, &created,
+                       errp);
     if (fd < 0) {
         return NULL;
     }
 
-    block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, errp);
+    block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, readonly, errp);
     if (!block) {
         if (created) {
             unlink(mem_path);
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 27dcccd8ec..890fda6a35 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -85,9 +85,11 @@ size_t qemu_mempath_getpagesize(const char *mem_path)
 void *qemu_ram_mmap(int fd,
                     size_t size,
                     size_t align,
+                    bool readonly,
                     bool shared,
                     bool is_pmem)
 {
+    int prot;
     int flags;
     int map_sync_flags = 0;
     int guardfd;
@@ -146,8 +148,9 @@ void *qemu_ram_mmap(int fd,
 
     offset = QEMU_ALIGN_UP((uintptr_t)guardptr, align) - (uintptr_t)guardptr;
 
-    ptr = mmap(guardptr + offset, size, PROT_READ | PROT_WRITE,
-               flags | map_sync_flags, fd, 0);
+    prot = PROT_READ | (readonly ? 0 : PROT_WRITE);
+
+    ptr = mmap(guardptr + offset, size, prot, flags | map_sync_flags, fd, 0);
 
     if (ptr == MAP_FAILED && map_sync_flags) {
         if (errno == ENOTSUP) {
@@ -171,8 +174,7 @@ void *qemu_ram_mmap(int fd,
          * if map failed with MAP_SHARED_VALIDATE | MAP_SYNC,
          * we will remove these flags to handle compatibility.
          */
-        ptr = mmap(guardptr + offset, size, PROT_READ | PROT_WRITE,
-                   flags, fd, 0);
+        ptr = mmap(guardptr + offset, size, prot, flags, fd, 0);
     }
 
     if (ptr == MAP_FAILED) {
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index f15234b5c0..cfe86553da 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -227,7 +227,7 @@ void *qemu_memalign(size_t alignment, size_t size)
 void *qemu_anon_ram_alloc(size_t size, uint64_t *alignment, bool shared)
 {
     size_t align = QEMU_VMALLOC_ALIGN;
-    void *ptr = qemu_ram_mmap(-1, size, align, shared, false);
+    void *ptr = qemu_ram_mmap(-1, size, align, false, shared, false);
 
     if (ptr == MAP_FAILED) {
         return NULL;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/3] hostmem-file: add readonly=on|off option
  2021-01-04 17:13 [PATCH v3 0/3] nvdimm: read-only file support Stefan Hajnoczi
  2021-01-04 17:13 ` [PATCH v3 1/3] memory: add readonly support to memory_region_init_ram_from_file() Stefan Hajnoczi
@ 2021-01-04 17:13 ` Stefan Hajnoczi
  2021-01-04 17:13 ` [PATCH v3 3/3] nvdimm: check -object memory-backend-file, readonly=on option Stefan Hajnoczi
  2021-01-04 21:02 ` [PATCH v3 0/3] nvdimm: read-only file support Eduardo Habkost
  3 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2021-01-04 17:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Xiao Guangrong, Michael S. Tsirkin, Julio Montes,
	Eduardo Habkost, eric.g.ernst, Stefan Hajnoczi, Igor Mammedov,
	Paolo Bonzini, Philippe Mathieu-Daudé,
	Richard Henderson

Let -object memory-backend-file work on read-only files when the
readonly=on option is given. This can be used to share the contents of a
file between multiple guests while preventing them from consuming
Copy-on-Write memory if guests dirty the pages, for example.

Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v3:
 * Use Object *obj instead of Object *o [Igor]
 * Do not dereference MEMORY_BACKEND_FILE(o)->readonly directly, use a
   local variable to hold the HostMemoryBackendFile pointer. [Igor]
---
 backends/hostmem-file.c | 28 +++++++++++++++++++++++++++-
 qemu-options.hx         |  5 ++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index e827692124..733408e076 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -29,6 +29,7 @@ struct HostMemoryBackendFile {
     uint64_t align;
     bool discard_data;
     bool is_pmem;
+    bool readonly;
 };
 
 static void
@@ -56,7 +57,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
                                      backend->size, fb->align,
                                      (backend->share ? RAM_SHARED : 0) |
                                      (fb->is_pmem ? RAM_PMEM : 0),
-                                     fb->mem_path, false, errp);
+                                     fb->mem_path, fb->readonly, errp);
     g_free(name);
 #endif
 }
@@ -151,6 +152,28 @@ static void file_memory_backend_set_pmem(Object *o, bool value, Error **errp)
     fb->is_pmem = value;
 }
 
+static bool file_memory_backend_get_readonly(Object *obj, Error **errp)
+{
+    HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj);
+
+    return fb->readonly;
+}
+
+static void file_memory_backend_set_readonly(Object *obj, bool value,
+                                             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 'readonly' of %s.",
+                   object_get_typename(obj));
+        return;
+    }
+
+    fb->readonly = value;
+}
+
 static void file_backend_unparent(Object *obj)
 {
     HostMemoryBackend *backend = MEMORY_BACKEND(obj);
@@ -182,6 +205,9 @@ file_backend_class_init(ObjectClass *oc, void *data)
         NULL, NULL);
     object_class_property_add_bool(oc, "pmem",
         file_memory_backend_get_pmem, file_memory_backend_set_pmem);
+    object_class_property_add_bool(oc, "readonly",
+        file_memory_backend_get_readonly,
+        file_memory_backend_set_readonly);
 }
 
 static void file_backend_instance_finalize(Object *o)
diff --git a/qemu-options.hx b/qemu-options.hx
index 459c916d3d..4732c6b45e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4407,7 +4407,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``
+    ``-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,readonly=on|off``
         Creates a memory file backend object, which can be used to back
         the guest RAM with huge pages.
 
@@ -4490,6 +4490,9 @@ SRST
         4.15) and the filesystem of ``mem-path`` mounted with DAX
         option.
 
+        The ``readonly`` option specifies whether the backing file is opened
+        read-only or read-write (default).
+
     ``-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
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 3/3] nvdimm: check -object memory-backend-file, readonly=on option
  2021-01-04 17:13 [PATCH v3 0/3] nvdimm: read-only file support Stefan Hajnoczi
  2021-01-04 17:13 ` [PATCH v3 1/3] memory: add readonly support to memory_region_init_ram_from_file() Stefan Hajnoczi
  2021-01-04 17:13 ` [PATCH v3 2/3] hostmem-file: add readonly=on|off option Stefan Hajnoczi
@ 2021-01-04 17:13 ` Stefan Hajnoczi
  2021-01-04 21:02 ` [PATCH v3 0/3] nvdimm: read-only file support Eduardo Habkost
  3 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2021-01-04 17:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Xiao Guangrong, Michael S. Tsirkin, Julio Montes,
	Eduardo Habkost, eric.g.ernst, Stefan Hajnoczi, Igor Mammedov,
	Paolo Bonzini, Philippe Mathieu-Daudé,
	Richard Henderson

Check that -device nvdimm,unarmed=on is used when -object
memory-backend-file,readonly=on and document that -device
nvdimm,unarmed=on|off controls whether the NVDIMM appears read-only to
the guest.

Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v3:
 * Produce an error when -device nvdimm,unarmed=off is used with -object
   memory-backend-file,readonly=on instead of silently switching on
   unarmed. [Igor]
---
 docs/nvdimm.txt | 24 +++++++++++++++++-------
 hw/mem/nvdimm.c |  9 +++++++++
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
index c2c6e441b3..0aae682be3 100644
--- a/docs/nvdimm.txt
+++ b/docs/nvdimm.txt
@@ -17,8 +17,8 @@ following command line options:
 
  -machine pc,nvdimm
  -m $RAM_SIZE,slots=$N,maxmem=$MAX_SIZE
- -object memory-backend-file,id=mem1,share=on,mem-path=$PATH,size=$NVDIMM_SIZE
- -device nvdimm,id=nvdimm1,memdev=mem1
+ -object memory-backend-file,id=mem1,share=on,mem-path=$PATH,size=$NVDIMM_SIZE,readonly=off
+ -device nvdimm,id=nvdimm1,memdev=mem1,unarmed=off
 
 Where,
 
@@ -31,9 +31,10 @@ Where,
    of normal RAM devices and vNVDIMM devices, e.g. $MAX_SIZE should be
    >= $RAM_SIZE + $NVDIMM_SIZE here.
 
- - "object memory-backend-file,id=mem1,share=on,mem-path=$PATH,size=$NVDIMM_SIZE"
-   creates a backend storage of size $NVDIMM_SIZE on a file $PATH. All
-   accesses to the virtual NVDIMM device go to the file $PATH.
+ - "object memory-backend-file,id=mem1,share=on,mem-path=$PATH,
+   size=$NVDIMM_SIZE,readonly=off" creates a backend storage of size
+   $NVDIMM_SIZE on a file $PATH. All accesses to the virtual NVDIMM device go
+   to the file $PATH.
 
    "share=on/off" controls the visibility of guest writes. If
    "share=on", then guest writes will be applied to the backend
@@ -42,8 +43,17 @@ Where,
    "share=off", then guest writes won't be applied to the backend
    file and thus will be invisible to other guests.
 
- - "device nvdimm,id=nvdimm1,memdev=mem1" creates a virtual NVDIMM
-   device whose storage is provided by above memory backend device.
+   "readonly=on/off" controls whether the file $PATH is opened read-only or
+   read/write (default).
+
+ - "device nvdimm,id=nvdimm1,memdev=mem1,unarmed=off" creates a read/write
+   virtual NVDIMM device whose storage is provided by above memory backend
+   device.
+
+   "unarmed" controls the ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM
+   State Flags" Bit 3 indicating that the device is "unarmed" and cannot accept
+   persistent writes. Linux guest drivers set the device to read-only when this
+   bit is present. Set unarmed to on when the memdev has readonly=on.
 
 Multiple vNVDIMM devices can be created if multiple pairs of "-object"
 and "-device" are provided.
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 03c2201b56..e0a9d606e1 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -146,6 +146,15 @@ static void nvdimm_prepare_memory_region(NVDIMMDevice *nvdimm, Error **errp)
         return;
     }
 
+    if (!nvdimm->unarmed && memory_region_is_rom(mr)) {
+        HostMemoryBackend *hostmem = dimm->hostmem;
+
+        error_setg(errp, "'unarmed' property must be off since memdev %s "
+                   "is read-only",
+                   object_get_canonical_path_component(OBJECT(hostmem)));
+        return;
+    }
+
     nvdimm->nvdimm_mr = g_new(MemoryRegion, 1);
     memory_region_init_alias(nvdimm->nvdimm_mr, OBJECT(dimm),
                              "nvdimm-memory", mr, 0, pmem_size);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/3] nvdimm: read-only file support
  2021-01-04 17:13 [PATCH v3 0/3] nvdimm: read-only file support Stefan Hajnoczi
                   ` (2 preceding siblings ...)
  2021-01-04 17:13 ` [PATCH v3 3/3] nvdimm: check -object memory-backend-file, readonly=on option Stefan Hajnoczi
@ 2021-01-04 21:02 ` Eduardo Habkost
  2021-01-14 14:05   ` Stefan Hajnoczi
  3 siblings, 1 reply; 7+ messages in thread
From: Eduardo Habkost @ 2021-01-04 21:02 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Xiao Guangrong, Michael S. Tsirkin, Julio Montes, qemu-devel,
	eric.g.ernst, Igor Mammedov, Paolo Bonzini, Richard Henderson

Is anybody already going to merge this?  If not, I can merge it.

On Mon, Jan 04, 2021 at 05:13:17PM +0000, Stefan Hajnoczi wrote:
> v3:
>  * Produce an error when -device nvdimm,unarmed=off is used with -object
>    memory-backend-file,readonly=on instead of silently switching on
>    unarmed. [Igor]
>  * Use Object *obj instead of Object *o [Igor]
>  * Do not dereference MEMORY_BACKEND_FILE(o)->readonly directly, use a
>    local variable to hold the HostMemoryBackendFile pointer. [Igor]
> v2:
>  * s/the the/the/ in documentation [Philippe]
>  * Assign nvdimm->unarmed earlier [Philippe]
> 
> There is currently no way to back an NVDIMM with a read-only file so it can be
> safely shared between untrusted guests.
> 
> Introduce an -object memory-backend-file,readonly=on|off option.
> 
> Julio Montes sent an earlier patch here:
> https://patchew.org/QEMU/20190708211936.8037-1-julio.montes@intel.com/
> 
> Eric Ernst requested this feature again for Kata Containers so I gave it a try.
> 
> Stefan Hajnoczi (3):
>   memory: add readonly support to memory_region_init_ram_from_file()
>   hostmem-file: add readonly=on|off option
>   nvdimm: check -object memory-backend-file,readonly=on option
> 
>  docs/nvdimm.txt           | 24 +++++++++++++++++-------
>  include/exec/memory.h     |  2 ++
>  include/exec/ram_addr.h   |  5 +++--
>  include/qemu/mmap-alloc.h |  2 ++
>  backends/hostmem-file.c   | 28 +++++++++++++++++++++++++++-
>  hw/mem/nvdimm.c           |  9 +++++++++
>  softmmu/memory.c          |  7 +++++--
>  softmmu/physmem.c         | 18 +++++++++++-------
>  util/mmap-alloc.c         | 10 ++++++----
>  util/oslib-posix.c        |  2 +-
>  qemu-options.hx           |  5 ++++-
>  11 files changed, 87 insertions(+), 25 deletions(-)
> 
> -- 
> 2.29.2
> 

-- 
Eduardo



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/3] nvdimm: read-only file support
  2021-01-04 21:02 ` [PATCH v3 0/3] nvdimm: read-only file support Eduardo Habkost
@ 2021-01-14 14:05   ` Stefan Hajnoczi
  2021-01-21 20:41     ` Eduardo Habkost
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2021-01-14 14:05 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Xiao Guangrong, Michael S. Tsirkin, Julio Montes, qemu-devel,
	eric.g.ernst, Igor Mammedov, Paolo Bonzini, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 183 bytes --]

On Mon, Jan 04, 2021 at 04:02:26PM -0500, Eduardo Habkost wrote:
> Is anybody already going to merge this?  If not, I can merge it.

Great, thank you for merging it, Eduardo!

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/3] nvdimm: read-only file support
  2021-01-14 14:05   ` Stefan Hajnoczi
@ 2021-01-21 20:41     ` Eduardo Habkost
  0 siblings, 0 replies; 7+ messages in thread
From: Eduardo Habkost @ 2021-01-21 20:41 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Xiao Guangrong, Michael S. Tsirkin, Julio Montes, qemu-devel,
	eric.g.ernst, Igor Mammedov, Paolo Bonzini, Richard Henderson

On Thu, Jan 14, 2021 at 02:05:06PM +0000, Stefan Hajnoczi wrote:
> On Mon, Jan 04, 2021 at 04:02:26PM -0500, Eduardo Habkost wrote:
> > Is anybody already going to merge this?  If not, I can merge it.
> 
> Great, thank you for merging it, Eduardo!

I had just queued the patches, but I will be able to send a pull
request only next Monday.  Sorry for the delay!

-- 
Eduardo



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-01-21 20:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-04 17:13 [PATCH v3 0/3] nvdimm: read-only file support Stefan Hajnoczi
2021-01-04 17:13 ` [PATCH v3 1/3] memory: add readonly support to memory_region_init_ram_from_file() Stefan Hajnoczi
2021-01-04 17:13 ` [PATCH v3 2/3] hostmem-file: add readonly=on|off option Stefan Hajnoczi
2021-01-04 17:13 ` [PATCH v3 3/3] nvdimm: check -object memory-backend-file, readonly=on option Stefan Hajnoczi
2021-01-04 21:02 ` [PATCH v3 0/3] nvdimm: read-only file support Eduardo Habkost
2021-01-14 14:05   ` Stefan Hajnoczi
2021-01-21 20:41     ` Eduardo Habkost

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.