[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 6/9] cosmetic changes preparing for the followin
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH 6/9] cosmetic changes preparing for the following patches |
Date: |
Wed, 4 Feb 2015 11:10:36 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Tue, 02/03 13:52, Paolo Bonzini wrote:
> From: Mike Day <address@hidden>
>
> Signed-off-by: Mike Day <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> arch_init.c | 5 +--
> exec.c | 84
> +++++++++++++++++++++++++++++++++-----------------
> include/exec/cpu-all.h | 1 +
> 3 files changed, 57 insertions(+), 33 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 89c8fa4..b13f74b 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -688,9 +688,9 @@ static int ram_find_and_save_block(QEMUFile *f, bool
> last_stage)
> }
> }
> }
> +
> last_seen_block = block;
> last_offset = offset;
> -
> return bytes_sent;
> }
>
> @@ -1117,7 +1117,6 @@ static int ram_load(QEMUFile *f, void *opaque, int
> version_id)
> ret = -EINVAL;
> break;
> }
> -
> ch = qemu_get_byte(f);
> ram_handle_compressed(host, ch, TARGET_PAGE_SIZE);
> break;
> @@ -1128,7 +1127,6 @@ static int ram_load(QEMUFile *f, void *opaque, int
> version_id)
> ret = -EINVAL;
> break;
> }
> -
> qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
> break;
> case RAM_SAVE_FLAG_XBZRLE:
> @@ -1138,7 +1136,6 @@ static int ram_load(QEMUFile *f, void *opaque, int
> version_id)
> ret = -EINVAL;
> break;
> }
> -
> if (load_xbzrle(f, addr, host) < 0) {
> error_report("Failed to decompress XBZRLE page at "
> RAM_ADDR_FMT, addr);
> diff --git a/exec.c b/exec.c
> index 05c5b44..8239370 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1265,11 +1265,12 @@ static RAMBlock *find_ram_block(ram_addr_t addr)
> return NULL;
> }
>
> +/* Called with iothread lock held. */
> void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
> {
> - RAMBlock *new_block = find_ram_block(addr);
> - RAMBlock *block;
> + RAMBlock *new_block, *block;
>
> + new_block = find_ram_block(addr);
> assert(new_block);
> assert(!new_block->idstr[0]);
>
> @@ -1282,7 +1283,6 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char
> *name, DeviceState *dev)
> }
> pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
>
> - /* This assumes the iothread lock is taken here too. */
> qemu_mutex_lock_ramlist();
> QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> if (block != new_block && !strcmp(block->idstr, new_block->idstr)) {
> @@ -1294,10 +1294,17 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char
> *name, DeviceState *dev)
> qemu_mutex_unlock_ramlist();
> }
>
> +/* Called with iothread lock held. */
> void qemu_ram_unset_idstr(ram_addr_t addr)
> {
> - RAMBlock *block = find_ram_block(addr);
> + RAMBlock *block;
>
> + /* FIXME: arch_init.c assumes that this is not called throughout
> + * migration. Ignore the problem since hot-unplug during migration
> + * does not work anyway.
> + */
> +
> + block = find_ram_block(addr);
> if (block) {
> memset(block->idstr, 0, sizeof(block->idstr));
> }
> @@ -1585,7 +1592,6 @@ void qemu_ram_free(ram_addr_t addr)
> }
> }
> qemu_mutex_unlock_ramlist();
> -
> }
>
> #ifndef _WIN32
> @@ -1633,7 +1639,6 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
> memory_try_enable_merging(vaddr, length);
> qemu_ram_setup_dump(vaddr, length);
> }
> - return;
Other changes are equivalent, but not quite for this one. But I think it is
still correct, so:
Reviewed-by: Fam Zheng <address@hidden>
> }
> }
> }
> @@ -1641,49 +1646,60 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t
> length)
>
> int qemu_get_ram_fd(ram_addr_t addr)
> {
> - RAMBlock *block = qemu_get_ram_block(addr);
> + RAMBlock *block;
> + int fd;
>
> - return block->fd;
> + block = qemu_get_ram_block(addr);
> + fd = block->fd;
> + return fd;
> }
>
> void *qemu_get_ram_block_host_ptr(ram_addr_t addr)
> {
> - RAMBlock *block = qemu_get_ram_block(addr);
> + RAMBlock *block;
> + void *ptr;
>
> - return ramblock_ptr(block, 0);
> + block = qemu_get_ram_block(addr);
> + ptr = ramblock_ptr(block, 0);
> + return ptr;
> }
>
> /* Return a host pointer to ram allocated with qemu_ram_alloc.
> - With the exception of the softmmu code in this file, this should
> - only be used for local memory (e.g. video ram) that the device owns,
> - and knows it isn't going to access beyond the end of the block.
> -
> - It should not be used for general purpose DMA.
> - Use cpu_physical_memory_map/cpu_physical_memory_rw instead.
> + * This should not be used for general purpose DMA. Use address_space_map
> + * or address_space_rw instead. For local memory (e.g. video ram) that the
> + * device owns, use memory_region_get_ram_ptr.
> */
> void *qemu_get_ram_ptr(ram_addr_t addr)
> {
> - RAMBlock *block = qemu_get_ram_block(addr);
> + RAMBlock *block;
> + void *ptr;
>
> - if (xen_enabled()) {
> + block = qemu_get_ram_block(addr);
> +
> + if (xen_enabled() && block->host == NULL) {
> /* We need to check if the requested address is in the RAM
> * because we don't want to map the entire memory in QEMU.
> * In that case just map until the end of the page.
> */
> if (block->offset == 0) {
> - return xen_map_cache(addr, 0, 0);
> - } else if (block->host == NULL) {
> - block->host =
> - xen_map_cache(block->offset, block->max_length, 1);
> + ptr = xen_map_cache(addr, 0, 0);
> + goto done;
> }
> +
> + block->host = xen_map_cache(block->offset, block->max_length, 1);
> }
> - return ramblock_ptr(block, addr - block->offset);
> + ptr = ramblock_ptr(block, addr - block->offset);
> +
> +done:
> + return ptr;
> }
>
> /* Return a host pointer to guest's ram. Similar to qemu_get_ram_ptr
> - * but takes a size argument */
> + * but takes a size argument.
> + */
> static void *qemu_ram_ptr_length(ram_addr_t addr, hwaddr *size)
> {
> + void *ptr;
> if (*size == 0) {
> return NULL;
> }
> @@ -1691,12 +1707,12 @@ static void *qemu_ram_ptr_length(ram_addr_t addr,
> hwaddr *size)
> return xen_map_cache(addr, *size, 1);
> } else {
> RAMBlock *block;
> -
> QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> if (addr - block->offset < block->max_length) {
> if (addr - block->offset + *size > block->max_length)
> *size = block->max_length - addr + block->offset;
> - return ramblock_ptr(block, addr - block->offset);
> + ptr = ramblock_ptr(block, addr - block->offset);
> + return ptr;
> }
> }
>
> @@ -1706,15 +1722,24 @@ static void *qemu_ram_ptr_length(ram_addr_t addr,
> hwaddr *size)
> }
>
> /* Some of the softmmu routines need to translate from a host pointer
> - (typically a TLB entry) back to a ram offset. */
> + * (typically a TLB entry) back to a ram offset.
> + *
> + * By the time this function returns, the returned pointer is not protected
> + * by RCU anymore. If the caller is not within an RCU critical section and
> + * does not hold the iothread lock, it must have other means of protecting
> the
> + * pointer, such as a reference to the region that includes the incoming
> + * ram_addr_t.
> + */
> MemoryRegion *qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
> {
> RAMBlock *block;
> uint8_t *host = ptr;
> + MemoryRegion *mr;
>
> if (xen_enabled()) {
> *ram_addr = xen_ram_addr_from_mapcache(ptr);
> - return qemu_get_ram_block(*ram_addr)->mr;
> + mr = qemu_get_ram_block(*ram_addr)->mr;
> + return mr;
> }
>
> block = ram_list.mru_block;
> @@ -1736,7 +1761,8 @@ MemoryRegion *qemu_ram_addr_from_host(void *ptr,
> ram_addr_t *ram_addr)
>
> found:
> *ram_addr = block->offset + (host - block->host);
> - return block->mr;
> + mr = block->mr;
> + return mr;
> }
>
> static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index b8781d1..f7a3625 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -277,6 +277,7 @@ struct RAMBlock {
> ram_addr_t max_length;
> void (*resized)(const char*, uint64_t length, void *host);
> uint32_t flags;
> + /* Protected by iothread lock. */
> char idstr[256];
> /* Reads can take either the iothread or the ramlist lock.
> * Writes must take both locks.
> --
> 1.8.3.1
>
>
[Qemu-devel] [PATCH 6/9] cosmetic changes preparing for the following patches, Paolo Bonzini, 2015/02/03
- Re: [Qemu-devel] [PATCH 6/9] cosmetic changes preparing for the following patches,
Fam Zheng <=
[Qemu-devel] [PATCH 8/9] exec: convert ram_list to QLIST, Paolo Bonzini, 2015/02/03
[Qemu-devel] [PATCH 2/9] exec: make iotlb RCU-friendly, Paolo Bonzini, 2015/02/03
[Qemu-devel] [PATCH 7/9] rcu: prod call_rcu thread when calling synchronize_rcu, Paolo Bonzini, 2015/02/03
[Qemu-devel] [PATCH 9/9] Convert ram_list to RCU, Paolo Bonzini, 2015/02/03