Re: [Qemu-devel] [PATCH 4/6] memory: Drop MemoryRegion.ram_addr

[Gonglei (Arei)]

> Subject: [Qemu-devel] [PATCH 4/6] memory: Drop MemoryRegion.ram_addr
> 
> 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).
> 
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  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(-)
> 

Reviewed-by: Gonglei <address@hidden>


> 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 769825e..b221f3c 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
>