qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram


From: Michael S. Tsirkin
Subject: Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users
Date: Sun, 3 Apr 2016 16:49:47 +0300

On Thu, Mar 24, 2016 at 12:03:35PM +0100, Paolo Bonzini wrote:
> Let users of qemu_get_ram_ptr and qemu_ram_ptr_length pass in an
> address that is relative to the MemoryRegion.  This basically means
> what address_space_translate returns.
> 
> invalidate_and_set_dirty has to add back mr->ram_addr, but reads do
> not need it at all.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>


I agree but I think we need a better name for this function.
qemu_ram_offset_to_ptr?
Will also serve to make sure backporting patches across this
API change does not cause issues.


> ---
>  exec.c                       | 40 +++++++++++++++-------------------------
>  include/exec/memory.h        |  1 -
>  memory.c                     |  4 ++--
>  scripts/dump-guest-memory.py | 19 +++----------------
>  4 files changed, 20 insertions(+), 44 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 001b669..ca9e3b6 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1876,6 +1876,7 @@ void *qemu_get_ram_ptr(RAMBlock *ram_block, ram_addr_t 
> addr)
>  
>      if (block == NULL) {
>          block = qemu_get_ram_block(addr);
> +        addr -= block->offset;
>      }
>  
>      if (xen_enabled() && block->host == NULL) {
> @@ -1889,7 +1890,7 @@ void *qemu_get_ram_ptr(RAMBlock *ram_block, ram_addr_t 
> addr)
>  
>          block->host = xen_map_cache(block->offset, block->max_length, 1);
>      }
> -    return ramblock_ptr(block, addr - block->offset);
> +    return ramblock_ptr(block, addr);
>  }
>  
>  /* Return a host pointer to guest's ram. Similar to qemu_get_ram_ptr
> @@ -1901,16 +1902,15 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block, 
> ram_addr_t addr,
>                                   hwaddr *size)
>  {
>      RAMBlock *block = ram_block;
> -    ram_addr_t offset_inside_block;
>      if (*size == 0) {
>          return NULL;
>      }
>  
>      if (block == NULL) {
>          block = qemu_get_ram_block(addr);
> +        addr -= block->offset;
>      }
> -    offset_inside_block = addr - block->offset;
> -    *size = MIN(*size, block->max_length - offset_inside_block);
> +    *size = MIN(*size, block->max_length - addr);
>  
>      if (xen_enabled() && block->host == NULL) {
>          /* We need to check if the requested address is in the RAM
> @@ -1924,7 +1924,7 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block, 
> ram_addr_t addr,
>          block->host = xen_map_cache(block->offset, block->max_length, 1);
>      }
>  
> -    return ramblock_ptr(block, offset_inside_block);
> +    return ramblock_ptr(block, addr);
>  }
>  
>  /*
> @@ -2504,6 +2504,8 @@ static void invalidate_and_set_dirty(MemoryRegion *mr, 
> hwaddr addr,
>                                       hwaddr length)
>  {
>      uint8_t dirty_log_mask = memory_region_get_dirty_log_mask(mr);
> +    addr += memory_region_get_ram_addr(mr);
> +
>      /* No early return if dirty_log_mask is or becomes 0, because
>       * cpu_physical_memory_set_dirty_range will still call
>       * xen_modified_memory.
> @@ -2616,7 +2618,6 @@ static MemTxResult 
> address_space_write_continue(AddressSpace *as, hwaddr addr,
>                  abort();
>              }
>          } else {
> -            addr1 += memory_region_get_ram_addr(mr);
>              /* RAM case */
>              ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>              memcpy(ptr, buf, l);
> @@ -2709,8 +2710,7 @@ MemTxResult address_space_read_continue(AddressSpace 
> *as, hwaddr addr,
>              }
>          } else {
>              /* RAM case */
> -            ptr = qemu_get_ram_ptr(mr->ram_block,
> -                                   memory_region_get_ram_addr(mr) + addr1);
> +            ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>              memcpy(buf, ptr, l);
>          }
>  
> @@ -2793,7 +2793,6 @@ static inline void 
> cpu_physical_memory_write_rom_internal(AddressSpace *as,
>                memory_region_is_romd(mr))) {
>              l = memory_access_size(mr, l, addr1);
>          } else {
> -            addr1 += memory_region_get_ram_addr(mr);
>              /* ROM/RAM case */
>              ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>              switch (type) {
> @@ -2953,7 +2952,6 @@ void *address_space_map(AddressSpace *as,
>      hwaddr done = 0;
>      hwaddr l, xlat, base;
>      MemoryRegion *mr, *this_mr;
> -    ram_addr_t raddr;
>      void *ptr;
>  
>      if (len == 0) {
> @@ -2962,7 +2960,7 @@ void *address_space_map(AddressSpace *as,
>  
>      l = len;
>      rcu_read_lock();
> -    mr = address_space_translate(as, addr, &xlat, &l, is_write);
> +    mr = address_space_translate(as, addr, &base, &l, is_write);
>  
>      if (!memory_access_is_direct(mr, is_write)) {
>          if (atomic_xchg(&bounce.in_use, true)) {
> @@ -2987,9 +2985,6 @@ void *address_space_map(AddressSpace *as,
>          return bounce.buffer;
>      }
>  
> -    base = xlat;
> -    raddr = memory_region_get_ram_addr(mr);
> -
>      for (;;) {
>          len -= l;
>          addr += l;
> @@ -3007,7 +3002,7 @@ void *address_space_map(AddressSpace *as,
>  
>      memory_region_ref(mr);
>      *plen = done;
> -    ptr = qemu_ram_ptr_length(mr->ram_block, raddr + base, plen);
> +    ptr = qemu_ram_ptr_length(mr->ram_block, base, plen);
>      rcu_read_unlock();
>  
>      return ptr;
> @@ -3091,8 +3086,7 @@ static inline uint32_t 
> address_space_ldl_internal(AddressSpace *as, hwaddr addr,
>  #endif
>      } else {
>          /* RAM case */
> -        ptr = qemu_get_ram_ptr(mr->ram_block,
> -                               memory_region_get_ram_addr(mr) + addr1);
> +        ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>          switch (endian) {
>          case DEVICE_LITTLE_ENDIAN:
>              val = ldl_le_p(ptr);
> @@ -3185,8 +3179,7 @@ static inline uint64_t 
> address_space_ldq_internal(AddressSpace *as, hwaddr addr,
>  #endif
>      } else {
>          /* RAM case */
> -        ptr = qemu_get_ram_ptr(mr->ram_block,
> -                               memory_region_get_ram_addr(mr) + addr1);
> +        ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>          switch (endian) {
>          case DEVICE_LITTLE_ENDIAN:
>              val = ldq_le_p(ptr);
> @@ -3299,8 +3292,7 @@ static inline uint32_t 
> address_space_lduw_internal(AddressSpace *as,
>  #endif
>      } else {
>          /* RAM case */
> -        ptr = qemu_get_ram_ptr(mr->ram_block,
> -                               memory_region_get_ram_addr(mr) + addr1);
> +        ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>          switch (endian) {
>          case DEVICE_LITTLE_ENDIAN:
>              val = lduw_le_p(ptr);
> @@ -3382,13 +3374,13 @@ 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);
>          ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>          stl_p(ptr, val);
>  
>          dirty_log_mask = memory_region_get_dirty_log_mask(mr);
>          dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE);
> -        cpu_physical_memory_set_dirty_range(addr1, 4, dirty_log_mask);
> +        cpu_physical_memory_set_dirty_range(memory_region_get_ram_addr(mr) + 
> addr,
> +                                            4, dirty_log_mask);
>          r = MEMTX_OK;
>      }
>      if (result) {
> @@ -3437,7 +3429,6 @@ 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);
>          ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>          switch (endian) {
>          case DEVICE_LITTLE_ENDIAN:
> @@ -3547,7 +3538,6 @@ 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);
>          ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>          switch (endian) {
>          case DEVICE_LITTLE_ENDIAN:
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 2de7898..326c2a2 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1419,7 +1419,6 @@ MemTxResult address_space_read(AddressSpace *as, hwaddr 
> addr, MemTxAttrs attrs,
>              l = len;
>              mr = address_space_translate(as, addr, &addr1, &l, false);
>              if (len == l && memory_access_is_direct(mr, false)) {
> -                addr1 += memory_region_get_ram_addr(mr);
>                  ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
>                  memcpy(buf, ptr, len);
>              } else {
> diff --git a/memory.c b/memory.c
> index 49c9b14..bc47d0b 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1654,10 +1654,10 @@ 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));
> +    ptr = qemu_get_ram_ptr(mr->ram_block, offset);
>      rcu_read_unlock();
>  
> -    return ptr + offset;
> +    return ptr;
>  }
>  
>  ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index c0a2e99..ae21f97 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -328,23 +328,10 @@ def qlist_foreach(head, field_str):
>          yield var
>  
>  
> -def qemu_get_ram_block(ram_addr):
> -    """Returns the RAMBlock struct to which the given address belongs."""
> -
> -    ram_blocks = gdb.parse_and_eval("ram_list.blocks")
> -
> -    for block in qlist_foreach(ram_blocks, "next"):
> -        if (ram_addr - block["offset"]) < block["used_length"]:
> -            return block
> -
> -    raise gdb.GdbError("Bad ram offset %x" % ram_addr)
> -
> -
> -def qemu_get_ram_ptr(ram_addr):
> +def qemu_get_ram_ptr(block, offset):
>      """Returns qemu vaddr for given guest physical address."""
>  
> -    block = qemu_get_ram_block(ram_addr)
> -    return block["host"] + (ram_addr - block["offset"])
> +    return block["host"] + offset
>  
>  
>  def memory_region_get_ram_ptr(memory_region):
> @@ -352,7 +339,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_block"]["offset"])
> +    return qemu_get_ram_ptr(memory_region["ram_block"], 0)
>  
>  
>  def get_guest_phys_blocks():
> -- 
> 1.8.3.1



reply via email to

[Prev in Thread] Current Thread [Next in Thread]