diff mbox series

[RFC,09/21] ramblock: Add RAM_READONLY

Message ID 20230117220914.2062125-10-peterx@redhat.com (mailing list archive)
State New, archived
Headers show
Series migration: Support hugetlb doublemaps | expand

Commit Message

Peter Xu Jan. 17, 2023, 10:09 p.m. UTC
This allows us to have RAM_READONLY to be set in ram_flags to show that
this ramblock can only be read not write.

We used to pass in readonly boolean along the way for allocating the
ramblock, now let it be together with the rest ramblock flags.

The main purpose of this patch is not for clean up though, it's for caching
mapping information of each ramblock so when we want to mmap() it again for
whatever reason we can have all the information on hand.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 backends/hostmem-file.c |  3 ++-
 include/exec/memory.h   |  4 ++--
 include/exec/ram_addr.h |  5 ++---
 softmmu/memory.c        |  8 +++-----
 softmmu/physmem.c       | 16 +++++++---------
 5 files changed, 16 insertions(+), 20 deletions(-)

Comments

Dr. David Alan Gilbert Jan. 23, 2023, 7:42 p.m. UTC | #1
* Peter Xu (peterx@redhat.com) wrote:
> This allows us to have RAM_READONLY to be set in ram_flags to show that
> this ramblock can only be read not write.
> 
> We used to pass in readonly boolean along the way for allocating the
> ramblock, now let it be together with the rest ramblock flags.
> 
> The main purpose of this patch is not for clean up though, it's for caching
> mapping information of each ramblock so when we want to mmap() it again for
> whatever reason we can have all the information on hand.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  backends/hostmem-file.c |  3 ++-
>  include/exec/memory.h   |  4 ++--
>  include/exec/ram_addr.h |  5 ++---
>  softmmu/memory.c        |  8 +++-----
>  softmmu/physmem.c       | 16 +++++++---------
>  5 files changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 25141283c4..1daf00d2da 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -56,9 +56,10 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>      ram_flags = backend->share ? RAM_SHARED : 0;
>      ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
>      ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
> +    ram_flags |= fb->readonly ? RAM_READONLY : 0;
>      memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
>                                       backend->size, fb->align, ram_flags,
> -                                     fb->mem_path, fb->readonly, errp);
> +                                     fb->mem_path, errp);
>      g_free(name);
>  #endif
>  }
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index c37ffdbcd1..006ba77ede 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -188,6 +188,8 @@ typedef struct IOMMUTLBEvent {
>  /* RAM is a persistent kind memory */
>  #define RAM_PMEM (1 << 5)
>  
> +/* RAM is read-only */
> +#define RAM_READONLY (1 << 6)
>  
>  /*
>   * UFFDIO_WRITEPROTECT is used on this RAMBlock to
> @@ -1292,7 +1294,6 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
>   * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM,
>   *             RAM_NORESERVE,
>   * @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
> @@ -1305,7 +1306,6 @@ 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 f4fb6a2111..0bf9cfc659 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -110,7 +110,6 @@ long qemu_maxrampagesize(void);
>   *  @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM,
>   *              RAM_NORESERVE.
>   *  @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:
> @@ -119,10 +118,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,
> -                                   bool readonly, Error **errp);
> +                                   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 e05332d07f..2137028773 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -1601,18 +1601,16 @@ 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->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,
> -                                             readonly, &err);
> +    mr->ram_block = qemu_ram_alloc_from_file(size, mr, ram_flags, path, &err);
>      if (err) {
>          mr->size = int128_zero();
>          object_unparent(OBJECT(mr));
> @@ -1635,7 +1633,7 @@ void memory_region_init_ram_from_fd(MemoryRegion *mr,
>      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 b5be02f1cb..6096eac286 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -1534,7 +1534,6 @@ static int file_ram_open(const char *path,
>  
>  static void *file_ram_alloc(RAMBlock *block,
>                              int fd,
> -                            bool readonly,
>                              bool truncate,
>                              off_t offset,
>                              Error **errp)
> @@ -1589,7 +1588,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;
> @@ -2057,7 +2056,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;
> @@ -2065,7 +2064,7 @@ 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)) == 0);
> +                          RAM_PROTECTED | RAM_READONLY)) == 0);
>  
>      if (xen_enabled()) {
>          error_setg(errp, "-mem-path not supported with Xen");
> @@ -2100,8 +2099,7 @@ 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, fd, readonly,
> -                                     !file_size, offset, errp);
> +    new_block->host = file_ram_alloc(new_block, fd, !file_size, offset, errp);
>      if (!new_block->host) {
>          g_free(new_block);
>          return NULL;
> @@ -2120,11 +2118,11 @@ 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,
> -                                   bool readonly, Error **errp)
> +                                   Error **errp)
>  {
>      int fd;
> -    bool created;
>      RAMBlock *block;
> +    bool created, readonly = ram_flags & RAM_READONLY;
>  
>      fd = file_ram_open(mem_path, memory_region_name(mr), readonly, &created,
>                         errp);
> @@ -2132,7 +2130,7 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
>          return NULL;
>      }
>  
> -    block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, 0, readonly, errp);
> +    block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, 0, errp);
>      if (!block) {
>          if (created) {
>              unlink(mem_path);
> -- 
> 2.37.3
>
Juan Quintela Jan. 30, 2023, 5:06 a.m. UTC | #2
Peter Xu <peterx@redhat.com> wrote:
> This allows us to have RAM_READONLY to be set in ram_flags to show that
> this ramblock can only be read not write.
>
> We used to pass in readonly boolean along the way for allocating the
> ramblock, now let it be together with the rest ramblock flags.
>
> The main purpose of this patch is not for clean up though, it's for caching
> mapping information of each ramblock so when we want to mmap() it again for
> whatever reason we can have all the information on hand.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>
diff mbox series

Patch

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 25141283c4..1daf00d2da 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -56,9 +56,10 @@  file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
     ram_flags = backend->share ? RAM_SHARED : 0;
     ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
     ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
+    ram_flags |= fb->readonly ? RAM_READONLY : 0;
     memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
                                      backend->size, fb->align, ram_flags,
-                                     fb->mem_path, fb->readonly, errp);
+                                     fb->mem_path, errp);
     g_free(name);
 #endif
 }
diff --git a/include/exec/memory.h b/include/exec/memory.h
index c37ffdbcd1..006ba77ede 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -188,6 +188,8 @@  typedef struct IOMMUTLBEvent {
 /* RAM is a persistent kind memory */
 #define RAM_PMEM (1 << 5)
 
+/* RAM is read-only */
+#define RAM_READONLY (1 << 6)
 
 /*
  * UFFDIO_WRITEPROTECT is used on this RAMBlock to
@@ -1292,7 +1294,6 @@  void memory_region_init_resizeable_ram(MemoryRegion *mr,
  * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM,
  *             RAM_NORESERVE,
  * @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
@@ -1305,7 +1306,6 @@  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 f4fb6a2111..0bf9cfc659 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -110,7 +110,6 @@  long qemu_maxrampagesize(void);
  *  @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM,
  *              RAM_NORESERVE.
  *  @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:
@@ -119,10 +118,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,
-                                   bool readonly, Error **errp);
+                                   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 e05332d07f..2137028773 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1601,18 +1601,16 @@  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->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,
-                                             readonly, &err);
+    mr->ram_block = qemu_ram_alloc_from_file(size, mr, ram_flags, path, &err);
     if (err) {
         mr->size = int128_zero();
         object_unparent(OBJECT(mr));
@@ -1635,7 +1633,7 @@  void memory_region_init_ram_from_fd(MemoryRegion *mr,
     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 b5be02f1cb..6096eac286 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -1534,7 +1534,6 @@  static int file_ram_open(const char *path,
 
 static void *file_ram_alloc(RAMBlock *block,
                             int fd,
-                            bool readonly,
                             bool truncate,
                             off_t offset,
                             Error **errp)
@@ -1589,7 +1588,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;
@@ -2057,7 +2056,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;
@@ -2065,7 +2064,7 @@  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)) == 0);
+                          RAM_PROTECTED | RAM_READONLY)) == 0);
 
     if (xen_enabled()) {
         error_setg(errp, "-mem-path not supported with Xen");
@@ -2100,8 +2099,7 @@  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, fd, readonly,
-                                     !file_size, offset, errp);
+    new_block->host = file_ram_alloc(new_block, fd, !file_size, offset, errp);
     if (!new_block->host) {
         g_free(new_block);
         return NULL;
@@ -2120,11 +2118,11 @@  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,
-                                   bool readonly, Error **errp)
+                                   Error **errp)
 {
     int fd;
-    bool created;
     RAMBlock *block;
+    bool created, readonly = ram_flags & RAM_READONLY;
 
     fd = file_ram_open(mem_path, memory_region_name(mr), readonly, &created,
                        errp);
@@ -2132,7 +2130,7 @@  RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
         return NULL;
     }
 
-    block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, 0, readonly, errp);
+    block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, 0, errp);
     if (!block) {
         if (created) {
             unlink(mem_path);