Re: [Qemu-devel] [PATCH 1/2] memory: remove unnecessary masking of MemoryRegion ram_addr

[Michael S. Tsirkin]

On Thu, Mar 24, 2016 at 12:03:34PM +0100, Paolo Bonzini wrote:
> mr->ram_block->offset is already aligned to both host and target size
> (see qemu_ram_alloc_internal).  Remove further masking as it is
> unnecessary.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>

Reviewed-by: Michael S. Tsirkin <address@hidden>

> ---
>  exec.c             | 21 +++++++--------------
>  memory.c           |  5 ++---
>  migration/savevm.c |  4 ++--
>  translate-all.c    |  3 +--
>  4 files changed, 12 insertions(+), 21 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index f398d21..001b669 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1042,8 +1042,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
>  
>      if (memory_region_is_ram(section->mr)) {
>          /* Normal RAM.  */
> -        iotlb = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
> -            + xlat;
> +        iotlb = memory_region_get_ram_addr(section->mr) + xlat;
>          if (!section->readonly) {
>              iotlb |= PHYS_SECTION_NOTDIRTY;
>          } else {
> @@ -3093,9 +3092,7 @@ static inline uint32_t 
> address_space_ldl_internal(AddressSpace *as, hwaddr addr,
>      } else {
>          /* RAM case */
>          ptr = qemu_get_ram_ptr(mr->ram_block,
> -                               (memory_region_get_ram_addr(mr)
> -                                & TARGET_PAGE_MASK)
> -                               + addr1);
> +                               memory_region_get_ram_addr(mr) + addr1);
>          switch (endian) {
>          case DEVICE_LITTLE_ENDIAN:
>              val = ldl_le_p(ptr);
> @@ -3189,9 +3186,7 @@ static inline uint64_t 
> address_space_ldq_internal(AddressSpace *as, hwaddr addr,
>      } else {
>          /* RAM case */
>          ptr = qemu_get_ram_ptr(mr->ram_block,
> -                               (memory_region_get_ram_addr(mr)
> -                                & TARGET_PAGE_MASK)
> -                               + addr1);
> +                               memory_region_get_ram_addr(mr) + addr1);
>          switch (endian) {
>          case DEVICE_LITTLE_ENDIAN:
>              val = ldq_le_p(ptr);
> @@ -3305,9 +3300,7 @@ static inline uint32_t 
> address_space_lduw_internal(AddressSpace *as,
>      } else {
>          /* RAM case */
>          ptr = qemu_get_ram_ptr(mr->ram_block,
> -                               (memory_region_get_ram_addr(mr)
> -                                & TARGET_PAGE_MASK)
> -                               + addr1);
> +                               memory_region_get_ram_addr(mr) + addr1);
>          switch (endian) {
>          case DEVICE_LITTLE_ENDIAN:
>              val = lduw_le_p(ptr);
> @@ -3389,7 +3382,7 @@ void address_space_stl_notdirty(AddressSpace *as, 
> hwaddr addr, uint32_t val,
>  
>          r = memory_region_dispatch_write(mr, addr1, val, 4, attrs);
>      } else {
> -        addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK;
> +        addr1 += memory_region_get_ram_addr(mr);
>          ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>          stl_p(ptr, val);
>  
> @@ -3444,7 +3437,7 @@ static inline void 
> address_space_stl_internal(AddressSpace *as,
>          r = memory_region_dispatch_write(mr, addr1, val, 4, attrs);
>      } else {
>          /* RAM case */
> -        addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK;
> +        addr1 += memory_region_get_ram_addr(mr);
>          ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>          switch (endian) {
>          case DEVICE_LITTLE_ENDIAN:
> @@ -3554,7 +3547,7 @@ static inline void 
> address_space_stw_internal(AddressSpace *as,
>          r = memory_region_dispatch_write(mr, addr1, val, 2, attrs);
>      } else {
>          /* RAM case */
> -        addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK;
> +        addr1 += memory_region_get_ram_addr(mr);
>          ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>          switch (endian) {
>          case DEVICE_LITTLE_ENDIAN:
> diff --git a/memory.c b/memory.c
> index 95f7209..49c9b14 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1640,7 +1640,7 @@ int memory_region_get_fd(MemoryRegion *mr)
>  
>      assert(mr->ram_block);
>  
> -    return qemu_get_ram_fd(memory_region_get_ram_addr(mr) & 
> TARGET_PAGE_MASK);
> +    return qemu_get_ram_fd(memory_region_get_ram_addr(mr));
>  }
>  
>  void *memory_region_get_ram_ptr(MemoryRegion *mr)
> @@ -1654,8 +1654,7 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr)
>          mr = mr->alias;
>      }
>      assert(mr->ram_block);
> -    ptr = qemu_get_ram_ptr(mr->ram_block,
> -                           memory_region_get_ram_addr(mr) & 
> TARGET_PAGE_MASK);
> +    ptr = qemu_get_ram_ptr(mr->ram_block, memory_region_get_ram_addr(mr));
>      rcu_read_unlock();
>  
>      return ptr + offset;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 0a33c22..cbba062 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2229,13 +2229,13 @@ void hmp_info_snapshots(Monitor *mon, const QDict 
> *qdict)
>  
>  void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
>  {
> -    qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK,
> +    qemu_ram_set_idstr(memory_region_get_ram_addr(mr),
>                         memory_region_name(mr), dev);
>  }
>  
>  void vmstate_unregister_ram(MemoryRegion *mr, DeviceState *dev)
>  {
> -    qemu_ram_unset_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK);
> +    qemu_ram_unset_idstr(memory_region_get_ram_addr(mr));
>  }
>  
>  void vmstate_register_ram_global(MemoryRegion *mr)
> diff --git a/translate-all.c b/translate-all.c
> index e9f409b..6820d8e 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -1550,8 +1550,7 @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr 
> addr)
>          rcu_read_unlock();
>          return;
>      }
> -    ram_addr = (memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK)
> -        + addr;
> +    ram_addr = memory_region_get_ram_addr(mr) + addr;
>      tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0);
>      rcu_read_unlock();
>  }
> -- 
> 1.8.3.1
>