On 26/07/2017 18:53, Anthony PERARD wrote:
> Commit 04bf2526ce87f21b32c9acba1c5518708c243ad0 (exec: use
> qemu_ram_ptr_length to access guest ram) start using qemu_ram_ptr_length
> instead of qemu_map_ram_ptr, but when used with Xen, the behavior of
> both function is different. They both call xen_map_cache, but one with
> "lock", meaning the mapping of guest memory is never released
> implicitly, and the second one without, which means, mapping can be
> release later, when needed.
>
> In the context of address_space_{read,write}_continue, the ptr to those
> mapping should not be locked because it is used immediatly and never
> used again.
>
> The lock parameter make it explicit in which context qemu_ram_ptr_length
> is called.
>
> Signed-off-by: Anthony PERARD <anthony.per...@citrix.com>
> ---
> exec.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 01ac21e3cd..63508cd35e 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2203,7 +2203,7 @@ void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t
> addr)
> * Called within RCU critical section.
> */
> static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr,
> - hwaddr *size)
> + hwaddr *size, bool lock)
> {
> RAMBlock *block = ram_block;
> if (*size == 0) {
> @@ -2222,10 +2222,10 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block,
> ram_addr_t addr,
> * In that case just map the requested area.
> */
> if (block->offset == 0) {
> - return xen_map_cache(addr, *size, 1, true);
> + return xen_map_cache(addr, *size, lock ? 1 : 0, lock);
> }
>
> - block->host = xen_map_cache(block->offset, block->max_length, 1,
> true);
> + block->host = xen_map_cache(block->offset, block->max_length, 1,
> lock);
> }
>
> return ramblock_ptr(block, addr);
> @@ -2947,7 +2947,7 @@ static MemTxResult
> address_space_write_continue(AddressSpace *as, hwaddr addr,
> }
> } else {
> /* RAM case */
> - ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l);
> + ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
> memcpy(ptr, buf, l);
> invalidate_and_set_dirty(mr, addr1, l);
> }
> @@ -3038,7 +3038,7 @@ MemTxResult address_space_read_continue(AddressSpace
> *as, hwaddr addr,
> }
> } else {
> /* RAM case */
> - ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l);
> + ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
> memcpy(buf, ptr, l);
> }
>
> @@ -3349,7 +3349,7 @@ void *address_space_map(AddressSpace *as,
>
> memory_region_ref(mr);
> *plen = address_space_extend_translation(as, addr, len, mr, xlat, l,
> is_write);
> - ptr = qemu_ram_ptr_length(mr->ram_block, xlat, plen);
> + ptr = qemu_ram_ptr_length(mr->ram_block, xlat, plen, true);
> rcu_read_unlock();
>
> return ptr;
>
Queued, thanks.
Paolo
