[Qemu-devel] [PATCH v2 0/7] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate

[Qemu-devel] [PATCH v2 0/7] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate

[Fam Zheng]

v2: In the optimization patch, factor out section_covers_addr() and use it.
    [Paolo, Peter]
    Check "ram_block == NULL" in patch 3. [Gonglei]
    Add Gonglei's rev-by in patches 1, 2, 4 and 5.

The first four patches drop ram_addr from MemoryRegion on top of Gonglei's
optimization.

The next patch simplifies qemu_ram_free a bit by passing the RAMBlock pointer.

The last patch speeds up address_space_translate with a cache pointer inside
the AddressSpaceDispatch.

Fam Zheng (7):
  exec: Return RAMBlock pointer from allocating functions
  memory: Move assignment to ram_block to memory_region_init_*
  memory: Implement memory_region_get_ram_addr with mr->ram_block
  memory: Drop MemoryRegion.ram_addr
  exec: Pass RAMBlock pointer to qemu_ram_free
  exec: Factor out section_covers_addr
  exec: Introduce AddressSpaceDispatch.mru_section

 cputlb.c                |   4 +-
 exec.c                  | 106 +++++++++++++++++++++++++-----------------------
 hw/misc/ivshmem.c       |   9 ++--
 include/exec/memory.h   |   9 +---
 include/exec/ram_addr.h |  24 +++++------
 kvm-all.c               |   3 +-
 memory.c                |  56 ++++++++++++++-----------
 7 files changed, 111 insertions(+), 100 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH v2 1/7] exec: Return RAMBlock pointer from allocating functions

[Fam Zheng]

Previously we return RAMBlock.offset; now return the pointer to the
whole structure.

ram_block_add returns void now, error is completely passed with errp.

Reviewed-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 exec.c                  | 51 +++++++++++++++++++++----------------------------
 include/exec/ram_addr.h | 22 ++++++++++-----------
 memory.c                | 25 +++++++++++++++++++-----
 3 files changed, 53 insertions(+), 45 deletions(-)

diff --git a/exec.c b/exec.c
index c62c439..2b14b79 100644
--- a/exec.c
+++ b/exec.c
@@ -1554,7 +1554,7 @@ static void dirty_memory_extend(ram_addr_t old_ram_size,
     }
 }
 
-static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
+static void ram_block_add(RAMBlock *new_block, Error **errp)
 {
     RAMBlock *block;
     RAMBlock *last_block = NULL;
@@ -1573,7 +1573,6 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
             if (err) {
                 error_propagate(errp, err);
                 qemu_mutex_unlock_ramlist();
-                return -1;
             }
         } else {
             new_block->host = phys_mem_alloc(new_block->max_length,
@@ -1583,7 +1582,6 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
                                  "cannot set up guest memory '%s'",
                                  memory_region_name(new_block->mr));
                 qemu_mutex_unlock_ramlist();
-                return -1;
             }
             memory_try_enable_merging(new_block->host, new_block->max_length);
         }
@@ -1631,22 +1629,19 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
             kvm_setup_guest_memory(new_block->host, new_block->max_length);
         }
     }
-
-    return new_block->offset;
 }
 
 #ifdef __linux__
-ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
-                                    bool share, const char *mem_path,
-                                    Error **errp)
+RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
+                                   bool share, const char *mem_path,
+                                   Error **errp)
 {
     RAMBlock *new_block;
-    ram_addr_t addr;
     Error *local_err = NULL;
 
     if (xen_enabled()) {
         error_setg(errp, "-mem-path not supported with Xen");
-        return -1;
+        return NULL;
     }
 
     if (phys_mem_alloc != qemu_anon_ram_alloc) {
@@ -1657,7 +1652,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
          */
         error_setg(errp,
                    "-mem-path not supported with this accelerator");
-        return -1;
+        return NULL;
     }
 
     size = HOST_PAGE_ALIGN(size);
@@ -1670,29 +1665,28 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
                                      mem_path, errp);
     if (!new_block->host) {
         g_free(new_block);
-        return -1;
+        return NULL;
     }
 
-    addr = ram_block_add(new_block, &local_err);
+    ram_block_add(new_block, &local_err);
     if (local_err) {
         g_free(new_block);
         error_propagate(errp, local_err);
-        return -1;
+        return NULL;
     }
-    return addr;
+    return new_block;
 }
 #endif
 
 static
-ram_addr_t qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
-                                   void (*resized)(const char*,
-                                                   uint64_t length,
-                                                   void *host),
-                                   void *host, bool resizeable,
-                                   MemoryRegion *mr, Error **errp)
+RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
+                                  void (*resized)(const char*,
+                                                  uint64_t length,
+                                                  void *host),
+                                  void *host, bool resizeable,
+                                  MemoryRegion *mr, Error **errp)
 {
     RAMBlock *new_block;
-    ram_addr_t addr;
     Error *local_err = NULL;
 
     size = HOST_PAGE_ALIGN(size);
@@ -1711,29 +1705,28 @@ ram_addr_t qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
     if (resizeable) {
         new_block->flags |= RAM_RESIZEABLE;
     }
-    addr = ram_block_add(new_block, &local_err);
+    ram_block_add(new_block, &local_err);
     if (local_err) {
         g_free(new_block);
         error_propagate(errp, local_err);
-        return -1;
+        return NULL;
     }
-
     mr->ram_block = new_block;
-    return addr;
+    return new_block;
 }
 
-ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
+RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
                                    MemoryRegion *mr, Error **errp)
 {
     return qemu_ram_alloc_internal(size, size, NULL, host, false, mr, errp);
 }
 
-ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp)
+RAMBlock *qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp)
 {
     return qemu_ram_alloc_internal(size, size, NULL, NULL, false, mr, errp);
 }
 
-ram_addr_t qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t maxsz,
+RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t maxsz,
                                      void (*resized)(const char*,
                                                      uint64_t length,
                                                      void *host),
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 5d33def..865e19b 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -94,17 +94,17 @@ ram_addr_t last_ram_offset(void);
 void qemu_mutex_lock_ramlist(void);
 void qemu_mutex_unlock_ramlist(void);
 
-ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
-                                    bool share, const char *mem_path,
-                                    Error **errp);
-ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
-                                   MemoryRegion *mr, Error **errp);
-ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp);
-ram_addr_t qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t max_size,
-                                     void (*resized)(const char*,
-                                                     uint64_t length,
-                                                     void *host),
-                                     MemoryRegion *mr, Error **errp);
+RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
+                                   bool share, const char *mem_path,
+                                   Error **errp);
+RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
+                                  MemoryRegion *mr, Error **errp);
+RAMBlock *qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp);
+RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t max_size,
+                                    void (*resized)(const char*,
+                                                    uint64_t length,
+                                                    void *host),
+                                    MemoryRegion *mr, Error **errp);
 int qemu_get_ram_fd(ram_addr_t addr);
 void qemu_set_ram_fd(ram_addr_t addr, int fd);
 void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
diff --git a/memory.c b/memory.c
index 0dd9695..ae13ba9 100644
--- a/memory.c
+++ b/memory.c
@@ -1226,11 +1226,14 @@ void memory_region_init_ram(MemoryRegion *mr,
                             uint64_t size,
                             Error **errp)
 {
+    RAMBlock *ram_block;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
+    ram_block = qemu_ram_alloc(size, mr, errp);
+    mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
@@ -1244,11 +1247,14 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
                                                        void *host),
                                        Error **errp)
 {
+    RAMBlock *ram_block;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    mr->ram_addr = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
+    ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
+    mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
@@ -1261,11 +1267,14 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
                                       const char *path,
                                       Error **errp)
 {
+    RAMBlock *ram_block;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    mr->ram_addr = qemu_ram_alloc_from_file(size, mr, share, path, errp);
+    ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
+    mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 #endif
@@ -1276,6 +1285,8 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
                                 uint64_t size,
                                 void *ptr)
 {
+    RAMBlock *ram_block;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
@@ -1284,7 +1295,8 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
 
     /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
     assert(ptr != NULL);
-    mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
+    ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
+    mr->ram_addr = ram_block->offset;
 }
 
 void memory_region_set_skip_dump(MemoryRegion *mr)
@@ -1312,13 +1324,16 @@ void memory_region_init_rom_device(MemoryRegion *mr,
                                    uint64_t size,
                                    Error **errp)
 {
+    RAMBlock *ram_block;
+
     memory_region_init(mr, owner, name, size);
     mr->ops = ops;
     mr->opaque = opaque;
     mr->terminates = true;
     mr->rom_device = true;
     mr->destructor = memory_region_destructor_rom_device;
-    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
+    ram_block = qemu_ram_alloc(size, mr, errp);
+    mr->ram_addr = ram_block->offset;
 }
 
 void memory_region_init_iommu(MemoryRegion *mr,
-- 
2.4.3

[Qemu-devel] [PATCH v2 2/7] memory: Move assignment to ram_block to memory_region_init_*

[Fam Zheng]

We don't force "const" qualifiers with pointers in QEMU, but it's still
good to keep a clean function interface. Assigning to mr->ram_block is
in this sense ugly - one initializer mutating its owning object's state.

Move it to memory_region_init_*, where mr->ram_addr is assigned.

Reviewed-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 exec.c   | 1 -
 memory.c | 5 +++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index 2b14b79..83e3f7d 100644
--- a/exec.c
+++ b/exec.c
@@ -1711,7 +1711,6 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
         error_propagate(errp, local_err);
         return NULL;
     }
-    mr->ram_block = new_block;
     return new_block;
 }
 
diff --git a/memory.c b/memory.c
index ae13ba9..fe70075 100644
--- a/memory.c
+++ b/memory.c
@@ -1233,6 +1233,7 @@ void memory_region_init_ram(MemoryRegion *mr,
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     ram_block = qemu_ram_alloc(size, mr, errp);
+    mr->ram_block = ram_block;
     mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
@@ -1254,6 +1255,7 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
+    mr->ram_block = ram_block;
     mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
@@ -1274,6 +1276,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
+    mr->ram_block = ram_block;
     mr->ram_addr = ram_block->offset;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
@@ -1296,6 +1299,7 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
     /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
     assert(ptr != NULL);
     ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
+    mr->ram_block = ram_block;
     mr->ram_addr = ram_block->offset;
 }
 
@@ -1333,6 +1337,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
     mr->rom_device = true;
     mr->destructor = memory_region_destructor_rom_device;
     ram_block = qemu_ram_alloc(size, mr, errp);
+    mr->ram_block = ram_block;
     mr->ram_addr = ram_block->offset;
 }
 
-- 
2.4.3

[Qemu-devel] [PATCH v2 3/7] memory: Implement memory_region_get_ram_addr with mr->ram_block

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/exec/memory.h | 8 +-------
 memory.c              | 5 +++++
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index d5284c2..810d2c0 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -978,14 +978,8 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr,
 /**
  * memory_region_get_ram_addr: Get the ram address associated with a memory
  *                             region
- *
- * DO NOT USE THIS FUNCTION.  This is a temporary workaround while the Xen
- * code is being reworked.
  */
-static inline ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
-{
-    return mr->ram_addr;
-}
+ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr);
 
 uint64_t memory_region_get_alignment(const MemoryRegion *mr);
 /**
diff --git a/memory.c b/memory.c
index fe70075..b2b2216 100644
--- a/memory.c
+++ b/memory.c
@@ -1596,6 +1596,11 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr)
     return ptr + offset;
 }
 
+ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
+{
+    return mr->ram_block ? mr->ram_block->offset : RAM_ADDR_INVALID;
+}
+
 void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp)
 {
     assert(mr->ram_addr != RAM_ADDR_INVALID);
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2 3/7] memory: Implement memory_region_get_ram_addr with mr->ram_block

[Gonglei (Arei)]

> Subject: [PATCH v2 3/7] memory: Implement memory_region_get_ram_addr
> with mr->ram_block
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  include/exec/memory.h | 8 +-------
>  memory.c              | 5 +++++
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 

Reviewed-by: Gonglei <arei.gonglei@huawei.com>


> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index d5284c2..810d2c0 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -978,14 +978,8 @@ void
> memory_region_add_subregion_overlap(MemoryRegion *mr,
>  /**
>   * memory_region_get_ram_addr: Get the ram address associated with a
> memory
>   *                             region
> - *
> - * DO NOT USE THIS FUNCTION.  This is a temporary workaround while the
> Xen
> - * code is being reworked.
>   */
> -static inline ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
> -{
> -    return mr->ram_addr;
> -}
> +ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr);
> 
>  uint64_t memory_region_get_alignment(const MemoryRegion *mr);
>  /**
> diff --git a/memory.c b/memory.c
> index fe70075..b2b2216 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1596,6 +1596,11 @@ void
> *memory_region_get_ram_ptr(MemoryRegion *mr)
>      return ptr + offset;
>  }
> 
> +ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
> +{
> +    return mr->ram_block ? mr->ram_block->offset : RAM_ADDR_INVALID;
> +}
> +
>  void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize,
> Error **errp)
>  {
>      assert(mr->ram_addr != RAM_ADDR_INVALID);
> --
> 2.4.3

[Qemu-devel] [PATCH v2 4/7] memory: Drop MemoryRegion.ram_addr

[Fam Zheng]

All references to mr->ram_addr are replaced by
memory_region_get_ram_addr(mr) (except for a few assertions that are
replaced with mr->ram_block).

Reviewed-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 cputlb.c              |  4 +--
 exec.c                |  3 ++-
 hw/misc/ivshmem.c     |  9 ++++---
 include/exec/memory.h |  1 -
 kvm-all.c             |  3 ++-
 memory.c              | 71 ++++++++++++++++++++-------------------------------
 6 files changed, 39 insertions(+), 52 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 3973030..2f7a166 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -416,8 +416,8 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
             /* Write access calls the I/O callback.  */
             te->addr_write = address | TLB_MMIO;
         } else if (memory_region_is_ram(section->mr)
-                   && cpu_physical_memory_is_clean(section->mr->ram_addr
-                                                   + xlat)) {
+                   && cpu_physical_memory_is_clean(
+                        memory_region_get_ram_addr(section->mr) + xlat)) {
             te->addr_write = address | TLB_NOTDIRTY;
         } else {
             te->addr_write = address;
diff --git a/exec.c b/exec.c
index 83e3f7d..6ed4203 100644
--- a/exec.c
+++ b/exec.c
@@ -2699,7 +2699,8 @@ MemTxResult address_space_read_continue(AddressSpace *as, hwaddr addr,
             }
         } else {
             /* RAM case */
-            ptr = qemu_get_ram_ptr(mr->ram_block, mr->ram_addr + addr1);
+            ptr = qemu_get_ram_ptr(mr->ram_block,
+                                   memory_region_get_ram_addr(mr) + addr1);
             memcpy(buf, ptr, l);
         }
 
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 48b7a34..1838bc8 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -400,7 +400,7 @@ static int create_shared_memory_BAR(IVShmemState *s, int fd, uint8_t attr,
 
     memory_region_init_ram_ptr(&s->ivshmem, OBJECT(s), "ivshmem.bar2",
                                s->ivshmem_size, ptr);
-    qemu_set_ram_fd(s->ivshmem.ram_addr, fd);
+    qemu_set_ram_fd(memory_region_get_ram_addr(&s->ivshmem), fd);
     vmstate_register_ram(&s->ivshmem, DEVICE(s));
     memory_region_add_subregion(&s->bar, 0, &s->ivshmem);
 
@@ -661,7 +661,8 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size)
         }
         memory_region_init_ram_ptr(&s->ivshmem, OBJECT(s),
                                    "ivshmem.bar2", s->ivshmem_size, map_ptr);
-        qemu_set_ram_fd(s->ivshmem.ram_addr, incoming_fd);
+        qemu_set_ram_fd(memory_region_get_ram_addr(&s->ivshmem),
+                        incoming_fd);
         vmstate_register_ram(&s->ivshmem, DEVICE(s));
 
         IVSHMEM_DPRINTF("guest h/w addr = %p, size = %" PRIu64 "\n",
@@ -996,8 +997,10 @@ static void pci_ivshmem_exit(PCIDevice *dev)
                              strerror(errno));
             }
 
-            if ((fd = qemu_get_ram_fd(s->ivshmem.ram_addr)) != -1)
+            fd = qemu_get_ram_fd(memory_region_get_ram_addr(&s->ivshmem));
+            if (fd != -1) {
                 close(fd);
+            }
         }
 
         vmstate_unregister_ram(&s->ivshmem, DEVICE(dev));
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 810d2c0..2de7898 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -169,7 +169,6 @@ struct MemoryRegion {
     bool flush_coalesced_mmio;
     bool global_locking;
     uint8_t dirty_log_mask;
-    ram_addr_t ram_addr;
     RAMBlock *ram_block;
     Object *owner;
     const MemoryRegionIOMMUOps *iommu_ops;
diff --git a/kvm-all.c b/kvm-all.c
index a65e73f..161200e 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -366,7 +366,8 @@ static void kvm_log_stop(MemoryListener *listener,
 static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section,
                                          unsigned long *bitmap)
 {
-    ram_addr_t start = section->offset_within_region + section->mr->ram_addr;
+    ram_addr_t start = section->offset_within_region +
+                       memory_region_get_ram_addr(section->mr);
     ram_addr_t pages = int128_get64(section->size) / getpagesize();
 
     cpu_physical_memory_set_dirty_lebitmap(bitmap, start, pages);
diff --git a/memory.c b/memory.c
index b2b2216..befdf6b 100644
--- a/memory.c
+++ b/memory.c
@@ -858,12 +858,12 @@ static void memory_region_destructor_none(MemoryRegion *mr)
 
 static void memory_region_destructor_ram(MemoryRegion *mr)
 {
-    qemu_ram_free(mr->ram_addr);
+    qemu_ram_free(memory_region_get_ram_addr(mr));
 }
 
 static void memory_region_destructor_rom_device(MemoryRegion *mr)
 {
-    qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
+    qemu_ram_free(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
 }
 
 static bool memory_region_need_escape(char c)
@@ -994,7 +994,6 @@ static void memory_region_initfn(Object *obj)
     ObjectProperty *op;
 
     mr->ops = &unassigned_mem_ops;
-    mr->ram_addr = RAM_ADDR_INVALID;
     mr->enabled = true;
     mr->romd_mode = true;
     mr->global_locking = true;
@@ -1226,15 +1225,11 @@ void memory_region_init_ram(MemoryRegion *mr,
                             uint64_t size,
                             Error **errp)
 {
-    RAMBlock *ram_block;
-
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    ram_block = qemu_ram_alloc(size, mr, errp);
-    mr->ram_block = ram_block;
-    mr->ram_addr = ram_block->offset;
+    mr->ram_block = qemu_ram_alloc(size, mr, errp);
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
@@ -1248,15 +1243,12 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
                                                        void *host),
                                        Error **errp)
 {
-    RAMBlock *ram_block;
-
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
-    mr->ram_block = ram_block;
-    mr->ram_addr = ram_block->offset;
+    mr->ram_block = qemu_ram_alloc_resizeable(size, max_size, resized,
+                                              mr, errp);
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
@@ -1269,15 +1261,11 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
                                       const char *path,
                                       Error **errp)
 {
-    RAMBlock *ram_block;
-
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
-    mr->ram_block = ram_block;
-    mr->ram_addr = ram_block->offset;
+    mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 #endif
@@ -1288,8 +1276,6 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
                                 uint64_t size,
                                 void *ptr)
 {
-    RAMBlock *ram_block;
-
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
@@ -1298,9 +1284,7 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
 
     /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
     assert(ptr != NULL);
-    ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
-    mr->ram_block = ram_block;
-    mr->ram_addr = ram_block->offset;
+    mr->ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_fatal);
 }
 
 void memory_region_set_skip_dump(MemoryRegion *mr)
@@ -1328,17 +1312,13 @@ void memory_region_init_rom_device(MemoryRegion *mr,
                                    uint64_t size,
                                    Error **errp)
 {
-    RAMBlock *ram_block;
-
     memory_region_init(mr, owner, name, size);
     mr->ops = ops;
     mr->opaque = opaque;
     mr->terminates = true;
     mr->rom_device = true;
     mr->destructor = memory_region_destructor_rom_device;
-    ram_block = qemu_ram_alloc(size, mr, errp);
-    mr->ram_block = ram_block;
-    mr->ram_addr = ram_block->offset;
+    mr->ram_block = qemu_ram_alloc(size, mr, errp);
 }
 
 void memory_region_init_iommu(MemoryRegion *mr,
@@ -1503,24 +1483,26 @@ void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
 bool memory_region_get_dirty(MemoryRegion *mr, hwaddr addr,
                              hwaddr size, unsigned client)
 {
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
-    return cpu_physical_memory_get_dirty(mr->ram_addr + addr, size, client);
+    assert(mr->ram_block);
+    return cpu_physical_memory_get_dirty(memory_region_get_ram_addr(mr) + addr,
+                                         size, client);
 }
 
 void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
                              hwaddr size)
 {
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
-    cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size,
+    assert(mr->ram_block);
+    cpu_physical_memory_set_dirty_range(memory_region_get_ram_addr(mr) + addr,
+                                        size,
                                         memory_region_get_dirty_log_mask(mr));
 }
 
 bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
                                         hwaddr size, unsigned client)
 {
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
-    return cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr,
-                                                    size, client);
+    assert(mr->ram_block);
+    return cpu_physical_memory_test_and_clear_dirty(
+                memory_region_get_ram_addr(mr) + addr, size, client);
 }
 
 
@@ -1563,9 +1545,9 @@ void memory_region_rom_device_set_romd(MemoryRegion *mr, bool romd_mode)
 void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
                                hwaddr size, unsigned client)
 {
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
-    cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr, size,
-                                             client);
+    assert(mr->ram_block);
+    cpu_physical_memory_test_and_clear_dirty(
+        memory_region_get_ram_addr(mr) + addr, size, client);
 }
 
 int memory_region_get_fd(MemoryRegion *mr)
@@ -1574,9 +1556,9 @@ int memory_region_get_fd(MemoryRegion *mr)
         return memory_region_get_fd(mr->alias);
     }
 
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
+    assert(mr->ram_block);
 
-    return qemu_get_ram_fd(mr->ram_addr & TARGET_PAGE_MASK);
+    return qemu_get_ram_fd(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
 }
 
 void *memory_region_get_ram_ptr(MemoryRegion *mr)
@@ -1589,8 +1571,9 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr)
         offset += mr->alias_offset;
         mr = mr->alias;
     }
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
-    ptr = qemu_get_ram_ptr(mr->ram_block, mr->ram_addr & TARGET_PAGE_MASK);
+    assert(mr->ram_block);
+    ptr = qemu_get_ram_ptr(mr->ram_block,
+                           memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
     rcu_read_unlock();
 
     return ptr + offset;
@@ -1603,9 +1586,9 @@ ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
 
 void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp)
 {
-    assert(mr->ram_addr != RAM_ADDR_INVALID);
+    assert(mr->ram_block);
 
-    qemu_ram_resize(mr->ram_addr, newsize, errp);
+    qemu_ram_resize(memory_region_get_ram_addr(mr), newsize, errp);
 }
 
 static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpace *as)
-- 
2.4.3

[Qemu-devel] [PATCH] scripts: Fix dump-guest-memory.py for MemoryRegion.ram_block removal

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>

---

This goes after "[PATCH v2 4/7] memory: Drop MemoryRegion.ram_addr" (or
squashed into it if we want strict synchronization).
---
 scripts/dump-guest-memory.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index f274bf8..c0a2e99 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -352,7 +352,7 @@ def memory_region_get_ram_ptr(memory_region):
         return (memory_region_get_ram_ptr(memory_region["alias"].dereference())
                 + memory_region["alias_offset"])
 
-    return qemu_get_ram_ptr(memory_region["ram_addr"] & TARGET_PAGE_MASK)
+    return qemu_get_ram_ptr(memory_region["ram_block"]["offset"])
 
 
 def get_guest_phys_blocks():
-- 
2.4.3

Re: [Qemu-devel] [PATCH] scripts: Fix dump-guest-memory.py for MemoryRegion.ram_block removal

[Laszlo Ersek]

On 03/07/16 10:17, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> ---
> 
> This goes after "[PATCH v2 4/7] memory: Drop MemoryRegion.ram_addr" (or
> squashed into it if we want strict synchronization).
> ---
>  scripts/dump-guest-memory.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index f274bf8..c0a2e99 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -352,7 +352,7 @@ def memory_region_get_ram_ptr(memory_region):
>          return (memory_region_get_ram_ptr(memory_region["alias"].dereference())
>                  + memory_region["alias_offset"])
>  
> -    return qemu_get_ram_ptr(memory_region["ram_addr"] & TARGET_PAGE_MASK)
> +    return qemu_get_ram_ptr(memory_region["ram_block"]["offset"])
>  
>  
>  def get_guest_phys_blocks():
> 

I didn't follow your series, and I've by now forgotten most of the dump
stuff, but if you tested the above with GDB, I can ack it.

Acked-by: Laszlo Ersek <lersek@redhat.com>

I hope that Janosch can chime in as well.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH] scripts: Fix dump-guest-memory.py for MemoryRegion.ram_block removal

[Fam Zheng]

On Mon, 03/07 10:37, Laszlo Ersek wrote:
> On 03/07/16 10:17, Fam Zheng wrote:
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > 
> > ---
> > 
> > This goes after "[PATCH v2 4/7] memory: Drop MemoryRegion.ram_addr" (or
> > squashed into it if we want strict synchronization).
> > ---
> >  scripts/dump-guest-memory.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> > index f274bf8..c0a2e99 100644
> > --- a/scripts/dump-guest-memory.py
> > +++ b/scripts/dump-guest-memory.py
> > @@ -352,7 +352,7 @@ def memory_region_get_ram_ptr(memory_region):
> >          return (memory_region_get_ram_ptr(memory_region["alias"].dereference())
> >                  + memory_region["alias_offset"])
> >  
> > -    return qemu_get_ram_ptr(memory_region["ram_addr"] & TARGET_PAGE_MASK)
> > +    return qemu_get_ram_ptr(memory_region["ram_block"]["offset"])
> >  
> >  
> >  def get_guest_phys_blocks():
> > 
> 
> I didn't follow your series, and I've by now forgotten most of the dump
> stuff, but if you tested the above with GDB, I can ack it.

For the record, yes, I've tested it on top of this series and the command works
for me.

Fam

> 
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> 
> I hope that Janosch can chime in as well.
> 
> Thanks
> Laszlo

Re: [Qemu-devel] [PATCH] scripts: Fix dump-guest-memory.py for MemoryRegion.ram_block removal

[Laszlo Ersek]

On 03/07/16 10:45, Fam Zheng wrote:
> On Mon, 03/07 10:37, Laszlo Ersek wrote:
>> On 03/07/16 10:17, Fam Zheng wrote:
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>
>>> ---
>>>
>>> This goes after "[PATCH v2 4/7] memory: Drop MemoryRegion.ram_addr" (or
>>> squashed into it if we want strict synchronization).
>>> ---
>>>  scripts/dump-guest-memory.py | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
>>> index f274bf8..c0a2e99 100644
>>> --- a/scripts/dump-guest-memory.py
>>> +++ b/scripts/dump-guest-memory.py
>>> @@ -352,7 +352,7 @@ def memory_region_get_ram_ptr(memory_region):
>>>          return (memory_region_get_ram_ptr(memory_region["alias"].dereference())
>>>                  + memory_region["alias_offset"])
>>>  
>>> -    return qemu_get_ram_ptr(memory_region["ram_addr"] & TARGET_PAGE_MASK)
>>> +    return qemu_get_ram_ptr(memory_region["ram_block"]["offset"])
>>>  
>>>  
>>>  def get_guest_phys_blocks():
>>>
>>
>> I didn't follow your series, and I've by now forgotten most of the dump
>> stuff, but if you tested the above with GDB, I can ack it.
> 
> For the record, yes, I've tested it on top of this series and the command works
> for me.

Fantastic, thanks a lot!
Laszlo

>>
>> Acked-by: Laszlo Ersek <lersek@redhat.com>
>>
>> I hope that Janosch can chime in as well.
>>
>> Thanks
>> Laszlo

Re: [Qemu-devel] [PATCH] scripts: Fix dump-guest-memory.py for MemoryRegion.ram_block removal

[Paolo Bonzini]

On 07/03/2016 10:17, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> ---
> 
> This goes after "[PATCH v2 4/7] memory: Drop MemoryRegion.ram_addr" (or
> squashed into it if we want strict synchronization).
> ---
>  scripts/dump-guest-memory.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index f274bf8..c0a2e99 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -352,7 +352,7 @@ def memory_region_get_ram_ptr(memory_region):
>          return (memory_region_get_ram_ptr(memory_region["alias"].dereference())
>                  + memory_region["alias_offset"])
>  
> -    return qemu_get_ram_ptr(memory_region["ram_addr"] & TARGET_PAGE_MASK)
> +    return qemu_get_ram_ptr(memory_region["ram_block"]["offset"])
>  
>  
>  def get_guest_phys_blocks():
> 

Great, squashed it with Laszlo's ack.

Paolo

Re: [Qemu-devel] [PATCH] scripts: Fix dump-guest-memory.py for MemoryRegion.ram_block removal

[Janosch Frank]

On 03/07/2016 10:17 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> ---
> 
> This goes after "[PATCH v2 4/7] memory: Drop MemoryRegion.ram_addr" (or
> squashed into it if we want strict synchronization).
> ---
>  scripts/dump-guest-memory.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index f274bf8..c0a2e99 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -352,7 +352,7 @@ def memory_region_get_ram_ptr(memory_region):
>          return (memory_region_get_ram_ptr(memory_region["alias"].dereference())
>                  + memory_region["alias_offset"])
> 
> -    return qemu_get_ram_ptr(memory_region["ram_addr"] & TARGET_PAGE_MASK)
> +    return qemu_get_ram_ptr(memory_region["ram_block"]["offset"])

If you get rid of TARGET_PAGE_MASK you might also want to get rid of its
definition, we only use it once.

I only had a short look, I'll look through your patches tomorrow morning.

Cheers
> 
> 
>  def get_guest_phys_blocks():
> 

Re: [Qemu-devel] [PATCH] scripts: Fix dump-guest-memory.py for MemoryRegion.ram_block removal

[Janosch Frank]

On 03/07/2016 05:35 PM, Janosch Frank wrote:
> On 03/07/2016 10:17 AM, Fam Zheng wrote:
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>
>> ---
>>
>> This goes after "[PATCH v2 4/7] memory: Drop MemoryRegion.ram_addr" (or
>> squashed into it if we want strict synchronization).
>> ---
>>  scripts/dump-guest-memory.py | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
>> index f274bf8..c0a2e99 100644
>> --- a/scripts/dump-guest-memory.py
>> +++ b/scripts/dump-guest-memory.py
>> @@ -352,7 +352,7 @@ def memory_region_get_ram_ptr(memory_region):
>>          return (memory_region_get_ram_ptr(memory_region["alias"].dereference())
>>                  + memory_region["alias_offset"])
>>
>> -    return qemu_get_ram_ptr(memory_region["ram_addr"] & TARGET_PAGE_MASK)
>> +    return qemu_get_ram_ptr(memory_region["ram_block"]["offset"])
> 
> If you get rid of TARGET_PAGE_MASK you might also want to get rid of its
> definition, we only use it once.
> 
> I only had a short look, I'll look through your patches tomorrow morning.
> 
> Cheers

Didn't see any obvious problems.
Thanks for CCing.

Cheers

>>
>>
>>  def get_guest_phys_blocks():
>>
> 
> 

Re: [Qemu-devel] [PATCH] scripts: Fix dump-guest-memory.py for MemoryRegion.ram_block removal

[Fam Zheng]

On Tue, 03/08 08:54, Janosch Frank wrote:
> On 03/07/2016 05:35 PM, Janosch Frank wrote:
> > On 03/07/2016 10:17 AM, Fam Zheng wrote:
> >> Signed-off-by: Fam Zheng <famz@redhat.com>
> >>
> >> ---
> >>
> >> This goes after "[PATCH v2 4/7] memory: Drop MemoryRegion.ram_addr" (or
> >> squashed into it if we want strict synchronization).
> >> ---
> >>  scripts/dump-guest-memory.py | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> >> index f274bf8..c0a2e99 100644
> >> --- a/scripts/dump-guest-memory.py
> >> +++ b/scripts/dump-guest-memory.py
> >> @@ -352,7 +352,7 @@ def memory_region_get_ram_ptr(memory_region):
> >>          return (memory_region_get_ram_ptr(memory_region["alias"].dereference())
> >>                  + memory_region["alias_offset"])
> >>
> >> -    return qemu_get_ram_ptr(memory_region["ram_addr"] & TARGET_PAGE_MASK)
> >> +    return qemu_get_ram_ptr(memory_region["ram_block"]["offset"])
> > 
> > If you get rid of TARGET_PAGE_MASK you might also want to get rid of its
> > definition, we only use it once.
> > 
> > I only had a short look, I'll look through your patches tomorrow morning.
> > 
> > Cheers
> 
> Didn't see any obvious problems.

Great. The patches are merged now, thank you for taking a look at the series!

Fam

[Qemu-devel] [PATCH v2 5/7] exec: Pass RAMBlock pointer to qemu_ram_free

[Fam Zheng]

The only caller now knows exactly which RAMBlock to free, so it's not
necessary to do the lookup.

Reviewed-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 exec.c                  | 21 +++++++--------------
 include/exec/ram_addr.h |  2 +-
 memory.c                |  4 ++--
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/exec.c b/exec.c
index 6ed4203..ad8b826 100644
--- a/exec.c
+++ b/exec.c
@@ -1751,22 +1751,15 @@ static void reclaim_ramblock(RAMBlock *block)
     g_free(block);
 }
 
-void qemu_ram_free(ram_addr_t addr)
+void qemu_ram_free(RAMBlock *block)
 {
-    RAMBlock *block;
-
     qemu_mutex_lock_ramlist();
-    QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
-        if (addr == block->offset) {
-            QLIST_REMOVE_RCU(block, next);
-            ram_list.mru_block = NULL;
-            /* Write list before version */
-            smp_wmb();
-            ram_list.version++;
-            call_rcu(block, reclaim_ramblock, rcu);
-            break;
-        }
-    }
+    QLIST_REMOVE_RCU(block, next);
+    ram_list.mru_block = NULL;
+    /* Write list before version */
+    smp_wmb();
+    ram_list.version++;
+    call_rcu(block, reclaim_ramblock, rcu);
     qemu_mutex_unlock_ramlist();
 }
 
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 865e19b..5adf7a4 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -108,7 +108,7 @@ RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t max_size,
 int qemu_get_ram_fd(ram_addr_t addr);
 void qemu_set_ram_fd(ram_addr_t addr, int fd);
 void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
-void qemu_ram_free(ram_addr_t addr);
+void qemu_ram_free(RAMBlock *block);
 
 int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp);
 
diff --git a/memory.c b/memory.c
index befdf6b..966ff2c 100644
--- a/memory.c
+++ b/memory.c
@@ -858,12 +858,12 @@ static void memory_region_destructor_none(MemoryRegion *mr)
 
 static void memory_region_destructor_ram(MemoryRegion *mr)
 {
-    qemu_ram_free(memory_region_get_ram_addr(mr));
+    qemu_ram_free(mr->ram_block);
 }
 
 static void memory_region_destructor_rom_device(MemoryRegion *mr)
 {
-    qemu_ram_free(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
+    qemu_ram_free(mr->ram_block);
 }
 
 static bool memory_region_need_escape(char c)
-- 
2.4.3

[Qemu-devel] [PATCH v2 6/7] exec: Factor out section_covers_addr

[Fam Zheng]

This will be shared by the next patch.

Also add a comment explaining the unobvious condition on "size.hi".

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 exec.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index ad8b826..b4e2eb5 100644
--- a/exec.c
+++ b/exec.c
@@ -307,6 +307,16 @@ static void phys_page_compact_all(AddressSpaceDispatch *d, int nodes_nb)
     }
 }
 
+static inline bool section_covers_addr(const MemoryRegionSection *section,
+                                       hwaddr addr)
+{
+    /* Memory topology clips a memory region to 2^64, size.hi >= 0 means the
+     * section must cover any addr. */
+    return section->size.hi ||
+           range_covers_byte(section->offset_within_address_space,
+                             section->size.lo, addr);
+}
+
 static MemoryRegionSection *phys_page_find(PhysPageEntry lp, hwaddr addr,
                                            Node *nodes, MemoryRegionSection *sections)
 {
@@ -322,9 +332,7 @@ static MemoryRegionSection *phys_page_find(PhysPageEntry lp, hwaddr addr,
         lp = p[(index >> (i * P_L2_BITS)) & (P_L2_SIZE - 1)];
     }
 
-    if (sections[lp.ptr].size.hi ||
-        range_covers_byte(sections[lp.ptr].offset_within_address_space,
-                          sections[lp.ptr].size.lo, addr)) {
+    if (section_covers_addr(&sections[lp.ptr], addr)) {
         return &sections[lp.ptr];
     } else {
         return &sections[PHYS_SECTION_UNASSIGNED];
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2 6/7] exec: Factor out section_covers_addr

[Peter Xu]

On Tue, Mar 01, 2016 at 02:18:23PM +0800, Fam Zheng wrote:
> This will be shared by the next patch.
> 
> Also add a comment explaining the unobvious condition on "size.hi".
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

> ---
>  exec.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index ad8b826..b4e2eb5 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -307,6 +307,16 @@ static void phys_page_compact_all(AddressSpaceDispatch *d, int nodes_nb)
>      }
>  }
>  
> +static inline bool section_covers_addr(const MemoryRegionSection *section,
> +                                       hwaddr addr)
> +{
> +    /* Memory topology clips a memory region to 2^64, size.hi >= 0 means the
> +     * section must cover any addr. */
> +    return section->size.hi ||
> +           range_covers_byte(section->offset_within_address_space,
> +                             section->size.lo, addr);
> +}
> +
>  static MemoryRegionSection *phys_page_find(PhysPageEntry lp, hwaddr addr,
>                                             Node *nodes, MemoryRegionSection *sections)
>  {
> @@ -322,9 +332,7 @@ static MemoryRegionSection *phys_page_find(PhysPageEntry lp, hwaddr addr,
>          lp = p[(index >> (i * P_L2_BITS)) & (P_L2_SIZE - 1)];
>      }
>  
> -    if (sections[lp.ptr].size.hi ||
> -        range_covers_byte(sections[lp.ptr].offset_within_address_space,
> -                          sections[lp.ptr].size.lo, addr)) {
> +    if (section_covers_addr(&sections[lp.ptr], addr)) {
>          return &sections[lp.ptr];
>      } else {
>          return &sections[PHYS_SECTION_UNASSIGNED];
> -- 
> 2.4.3
> 

Re: [Qemu-devel] [PATCH v2 6/7] exec: Factor out section_covers_addr

[Paolo Bonzini]

On 01/03/2016 07:18, Fam Zheng wrote:
> +    /* Memory topology clips a memory region to 2^64, size.hi >= 0 means the
> +     * section must cover any addr. */

Small improvement:

    /* Memory topology clips a memory region to [0, 2^64); size.hi > 0 means
     * the section must cover the entire address space.
     */

Paolo

Re: [Qemu-devel] [PATCH v2 6/7] exec: Factor out section_covers_addr

[Fam Zheng]

On Tue, 03/01 10:42, Paolo Bonzini wrote:
> 
> 
> On 01/03/2016 07:18, Fam Zheng wrote:
> > +    /* Memory topology clips a memory region to 2^64, size.hi >= 0 means the
> > +     * section must cover any addr. */
> 
> Small improvement:
> 
>     /* Memory topology clips a memory region to [0, 2^64); size.hi > 0 means
>      * the section must cover the entire address space.
>      */

Better! Thanks.

Fam

[Qemu-devel] [PATCH v2 7/7] exec: Introduce AddressSpaceDispatch.mru_section

[Fam Zheng]

Under heavy workloads the lookup will likely end up with the same
MemoryRegionSection from last time. Using a pointer to cache the result,
like ram_list.mru_block, significantly reduces cost of
address_space_translate.

During address space topology update, as->dispatch will be reallocated
so the pointer is invalidated automatically.

Perf reports a visible drop on the cpu usage, because phys_page_find is
not called.  Before:

   2.35%  qemu-system-x86_64       [.] phys_page_find
   0.97%  qemu-system-x86_64       [.] address_space_translate_internal
   0.95%  qemu-system-x86_64       [.] address_space_translate
   0.55%  qemu-system-x86_64       [.] address_space_lookup_region

After:

   0.97%  qemu-system-x86_64       [.] address_space_translate_internal
   0.97%  qemu-system-x86_64       [.] address_space_lookup_region
   0.84%  qemu-system-x86_64       [.] address_space_translate

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 exec.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index b4e2eb5..98d7733 100644
--- a/exec.c
+++ b/exec.c
@@ -135,6 +135,7 @@ typedef struct PhysPageMap {
 struct AddressSpaceDispatch {
     struct rcu_head rcu;
 
+    MemoryRegionSection *mru_section;
     /* This is a multi-level map on the physical address space.
      * The bottom level has pointers to MemoryRegionSections.
      */
@@ -350,14 +351,25 @@ static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch *d,
                                                         hwaddr addr,
                                                         bool resolve_subpage)
 {
-    MemoryRegionSection *section;
+    MemoryRegionSection *section = atomic_read(&d->mru_section);
     subpage_t *subpage;
+    bool update;
 
-    section = phys_page_find(d->phys_map, addr, d->map.nodes, d->map.sections);
+    if (section && section != &d->map.sections[PHYS_SECTION_UNASSIGNED] &&
+        section_covers_addr(section, addr)) {
+        update = false;
+    } else {
+        section = phys_page_find(d->phys_map, addr, d->map.nodes,
+                                 d->map.sections);
+        update = true;
+    }
     if (resolve_subpage && section->mr->subpage) {
         subpage = container_of(section->mr, subpage_t, iomem);
         section = &d->map.sections[subpage->sub_section[SUBPAGE_IDX(addr)]];
     }
+    if (update) {
+        atomic_set(&d->mru_section, section);
+    }
     return section;
 }
 
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2 0/7] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate

[Paolo Bonzini]

On 01/03/2016 07:18, Fam Zheng wrote:
> v2: In the optimization patch, factor out section_covers_addr() and use it.
>     [Paolo, Peter]
>     Check "ram_block == NULL" in patch 3. [Gonglei]
>     Add Gonglei's rev-by in patches 1, 2, 4 and 5.
> 
> The first four patches drop ram_addr from MemoryRegion on top of Gonglei's
> optimization.
> 
> The next patch simplifies qemu_ram_free a bit by passing the RAMBlock pointer.
> 
> The last patch speeds up address_space_translate with a cache pointer inside
> the AddressSpaceDispatch.
> 
> Fam Zheng (7):
>   exec: Return RAMBlock pointer from allocating functions
>   memory: Move assignment to ram_block to memory_region_init_*
>   memory: Implement memory_region_get_ram_addr with mr->ram_block
>   memory: Drop MemoryRegion.ram_addr
>   exec: Pass RAMBlock pointer to qemu_ram_free
>   exec: Factor out section_covers_addr
>   exec: Introduce AddressSpaceDispatch.mru_section
> 
>  cputlb.c                |   4 +-
>  exec.c                  | 106 +++++++++++++++++++++++++-----------------------
>  hw/misc/ivshmem.c       |   9 ++--
>  include/exec/memory.h   |   9 +---
>  include/exec/ram_addr.h |  24 +++++------
>  kvm-all.c               |   3 +-
>  memory.c                |  56 ++++++++++++++-----------
>  7 files changed, 111 insertions(+), 100 deletions(-)
> 

Thanks, queued!

Paolo

Re: [Qemu-devel] [PATCH v2 0/7] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate

[Laszlo Ersek]

(CC Janosch)

Hi,

On 03/01/16 07:18, Fam Zheng wrote:
> v2: In the optimization patch, factor out section_covers_addr() and use it.
>     [Paolo, Peter]
>     Check "ram_block == NULL" in patch 3. [Gonglei]
>     Add Gonglei's rev-by in patches 1, 2, 4 and 5.
> 
> The first four patches drop ram_addr from MemoryRegion on top of Gonglei's
> optimization.
> 
> The next patch simplifies qemu_ram_free a bit by passing the RAMBlock pointer.
> 
> The last patch speeds up address_space_translate with a cache pointer inside
> the AddressSpaceDispatch.
> 
> Fam Zheng (7):
>   exec: Return RAMBlock pointer from allocating functions
>   memory: Move assignment to ram_block to memory_region_init_*
>   memory: Implement memory_region_get_ram_addr with mr->ram_block
>   memory: Drop MemoryRegion.ram_addr
>   exec: Pass RAMBlock pointer to qemu_ram_free
>   exec: Factor out section_covers_addr
>   exec: Introduce AddressSpaceDispatch.mru_section
> 
>  cputlb.c                |   4 +-
>  exec.c                  | 106 +++++++++++++++++++++++++-----------------------
>  hw/misc/ivshmem.c       |   9 ++--
>  include/exec/memory.h   |   9 +---
>  include/exec/ram_addr.h |  24 +++++------
>  kvm-all.c               |   3 +-
>  memory.c                |  56 ++++++++++++++-----------
>  7 files changed, 111 insertions(+), 100 deletions(-)
> 

Does this series preserve "scripts/dump-guest-memory.py" in working
shape? One of the patch titles above says "Drop MemoryRegion.ram_addr",
and I think that might break the memory_region_get_ram_ptr() method.

.. This might prove a false alarm, but I thought I'd ask.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2 0/7] memory: Clean up MemoryRegion.ram_addr and optimize address_space_translate

[Fam Zheng]

On Mon, 03/07 09:53, Laszlo Ersek wrote:
> (CC Janosch)
> 
> Hi,
> 
> On 03/01/16 07:18, Fam Zheng wrote:
> > v2: In the optimization patch, factor out section_covers_addr() and use it.
> >     [Paolo, Peter]
> >     Check "ram_block == NULL" in patch 3. [Gonglei]
> >     Add Gonglei's rev-by in patches 1, 2, 4 and 5.
> > 
> > The first four patches drop ram_addr from MemoryRegion on top of Gonglei's
> > optimization.
> > 
> > The next patch simplifies qemu_ram_free a bit by passing the RAMBlock pointer.
> > 
> > The last patch speeds up address_space_translate with a cache pointer inside
> > the AddressSpaceDispatch.
> > 
> > Fam Zheng (7):
> >   exec: Return RAMBlock pointer from allocating functions
> >   memory: Move assignment to ram_block to memory_region_init_*
> >   memory: Implement memory_region_get_ram_addr with mr->ram_block
> >   memory: Drop MemoryRegion.ram_addr
> >   exec: Pass RAMBlock pointer to qemu_ram_free
> >   exec: Factor out section_covers_addr
> >   exec: Introduce AddressSpaceDispatch.mru_section
> > 
> >  cputlb.c                |   4 +-
> >  exec.c                  | 106 +++++++++++++++++++++++++-----------------------
> >  hw/misc/ivshmem.c       |   9 ++--
> >  include/exec/memory.h   |   9 +---
> >  include/exec/ram_addr.h |  24 +++++------
> >  kvm-all.c               |   3 +-
> >  memory.c                |  56 ++++++++++++++-----------
> >  7 files changed, 111 insertions(+), 100 deletions(-)
> > 
> 
> Does this series preserve "scripts/dump-guest-memory.py" in working
> shape? One of the patch titles above says "Drop MemoryRegion.ram_addr",
> and I think that might break the memory_region_get_ram_ptr() method.

Indeed, the 'memory_region["ram_addr"]' expression is broken. I'll send a
separate patch.

Fam