[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH qemu v5 16/18] memory: Get rid of address_space_
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH qemu v5 16/18] memory: Get rid of address_space_init_shareable |
Date: |
Fri, 22 Sep 2017 01:06:12 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 21/09/2017 10:51, Alexey Kardashevskiy wrote:
> Since FlatViews are shared now and ASes not, this gets rid of
> address_space_init_shareable().
>
> This should cause no behavioural change.
>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> Changes:
> v3:
> * now removes @malloced and @ref_count, used to be in the previos patch
> ---
> include/exec/memory.h | 19 -------------------
> include/hw/arm/armv7m.h | 2 +-
> cpus.c | 5 +++--
> hw/arm/armv7m.c | 9 ++++-----
> memory.c | 22 +---------------------
> target/arm/cpu.c | 16 ++++++++--------
> target/i386/cpu.c | 5 +++--
> 7 files changed, 20 insertions(+), 58 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 2f4f56cf40..402824c6f2 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -309,8 +309,6 @@ struct AddressSpace {
> struct rcu_head rcu;
> char *name;
> MemoryRegion *root;
> - int ref_count;
> - bool malloced;
>
> /* Accessed via RCU. */
> struct FlatView *current_map;
> @@ -1586,23 +1584,6 @@ MemTxResult memory_region_dispatch_write(MemoryRegion
> *mr,
> void address_space_init(AddressSpace *as, MemoryRegion *root, const char
> *name);
>
> /**
> - * address_space_init_shareable: return an address space for a memory region,
> - * creating it if it does not already exist
> - *
> - * @root: a #MemoryRegion that routes addresses for the address space
> - * @name: an address space name. The name is only used for debugging
> - * output.
> - *
> - * This function will return a pointer to an existing AddressSpace
> - * which was initialized with the specified MemoryRegion, or it will
> - * create and initialize one if it does not already exist. The ASes
> - * are reference-counted, so the memory will be freed automatically
> - * when the AddressSpace is destroyed via address_space_destroy.
> - */
> -AddressSpace *address_space_init_shareable(MemoryRegion *root,
> - const char *name);
> -
> -/**
> * address_space_destroy: destroy an address space
> *
> * Releases all resources associated with an address space. After an
> address space
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
> index 10eb058027..008000fe11 100644
> --- a/include/hw/arm/armv7m.h
> +++ b/include/hw/arm/armv7m.h
> @@ -21,7 +21,7 @@ typedef struct {
> SysBusDevice parent_obj;
> /*< public >*/
>
> - AddressSpace *source_as;
> + AddressSpace source_as;
> MemoryRegion iomem;
> uint32_t base;
> MemoryRegion *source_memory;
> diff --git a/cpus.c b/cpus.c
> index 9bed61eefc..c9a624003a 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1764,8 +1764,9 @@ void qemu_init_vcpu(CPUState *cpu)
> /* If the target cpu hasn't set up any address spaces itself,
> * give it the default one.
> */
> - AddressSpace *as = address_space_init_shareable(cpu->memory,
> - "cpu-memory");
> + AddressSpace *as = g_new0(AddressSpace, 1);
> +
> + address_space_init(as, cpu->memory, "cpu-memory");
> cpu->num_ases = 1;
> cpu_address_space_init(cpu, as, 0);
> }
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index b64a409b40..4900339646 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -41,7 +41,7 @@ static MemTxResult bitband_read(void *opaque, hwaddr offset,
>
> /* Find address in underlying memory and round down to multiple of size
> */
> addr = bitband_addr(s, offset) & (-size);
> - res = address_space_read(s->source_as, addr, attrs, buf, size);
> + res = address_space_read(&s->source_as, addr, attrs, buf, size);
> if (res) {
> return res;
> }
> @@ -66,7 +66,7 @@ static MemTxResult bitband_write(void *opaque, hwaddr
> offset, uint64_t value,
>
> /* Find address in underlying memory and round down to multiple of size
> */
> addr = bitband_addr(s, offset) & (-size);
> - res = address_space_read(s->source_as, addr, attrs, buf, size);
> + res = address_space_read(&s->source_as, addr, attrs, buf, size);
> if (res) {
> return res;
> }
> @@ -79,7 +79,7 @@ static MemTxResult bitband_write(void *opaque, hwaddr
> offset, uint64_t value,
> } else {
> buf[bitpos >> 3] &= ~bit;
> }
> - return address_space_write(s->source_as, addr, attrs, buf, size);
> + return address_space_write(&s->source_as, addr, attrs, buf, size);
> }
>
> static const MemoryRegionOps bitband_ops = {
> @@ -111,8 +111,7 @@ static void bitband_realize(DeviceState *dev, Error
> **errp)
> return;
> }
>
> - s->source_as = address_space_init_shareable(s->source_memory,
> - "bitband-source");
> + address_space_init(&s->source_as, s->source_memory, "bitband-source");
> }
>
> /* Board init. */
> diff --git a/memory.c b/memory.c
> index 4c28b91890..57e47e990f 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -2739,9 +2739,7 @@ void address_space_init(AddressSpace *as, MemoryRegion
> *root, const char *name)
> {
> memory_region_ref(root);
> memory_region_transaction_begin();
> - as->ref_count = 1;
> as->root = root;
> - as->malloced = false;
> as->current_map = NULL;
> as->ioeventfd_nb = 0;
> as->ioeventfds = NULL;
> @@ -2754,37 +2752,19 @@ void address_space_init(AddressSpace *as,
> MemoryRegion *root, const char *name)
>
> static void do_address_space_destroy(AddressSpace *as)
> {
> - bool do_free = as->malloced;
> -
> assert(QTAILQ_EMPTY(&as->listeners));
>
> flatview_unref(as->current_map);
> g_free(as->name);
> g_free(as->ioeventfds);
> memory_region_unref(as->root);
> - if (do_free) {
> - g_free(as);
> - }
> -}
> -
> -AddressSpace *address_space_init_shareable(MemoryRegion *root, const char
> *name)
> -{
> - AddressSpace *as;
> -
> - as = g_malloc0(sizeof *as);
> - address_space_init(as, root, name);
> - as->malloced = true;
> - return as;
> + g_free(as);
This g_free is wrong; /i386/virtio/blk/pci/hotplug in
tests/virtio-blk-test is a pretty reliable reproducer.
Paolo
> }
>
> void address_space_destroy(AddressSpace *as)
> {
> MemoryRegion *root = as->root;
>
> - as->ref_count--;
> - if (as->ref_count) {
> - return;
> - }
> /* Flush out anything from MemoryListeners listening in on this */
> memory_region_transaction_begin();
> as->root = NULL;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 412e94c7ad..bba24f4590 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -684,6 +684,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
> **errp)
> CPUARMState *env = &cpu->env;
> int pagebits;
> Error *local_err = NULL;
> +#ifndef CONFIG_USER_ONLY
> + AddressSpace *as;
> +#endif
>
> cpu_exec_realizefn(cs, &local_err);
> if (local_err != NULL) {
> @@ -874,24 +877,21 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
> **errp)
>
> #ifndef CONFIG_USER_ONLY
> if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
> - AddressSpace *as;
> + as = g_new0(AddressSpace, 1);
>
> cs->num_ases = 2;
>
> if (!cpu->secure_memory) {
> cpu->secure_memory = cs->memory;
> }
> - as = address_space_init_shareable(cpu->secure_memory,
> - "cpu-secure-memory");
> + address_space_init(as, cpu->secure_memory, "cpu-secure-memory");
> cpu_address_space_init(cs, as, ARMASIdx_S);
> } else {
> cs->num_ases = 1;
> }
> -
> - cpu_address_space_init(cs,
> - address_space_init_shareable(cs->memory,
> - "cpu-memory"),
> - ARMASIdx_NS);
> + as = g_new0(AddressSpace, 1);
> + address_space_init(as, cs->memory, "cpu-memory");
> + cpu_address_space_init(cs, as, ARMASIdx_NS);
> #endif
>
> qemu_init_vcpu(cs);
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 4b0fa0613b..b0b123a571 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3741,10 +3741,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error
> **errp)
>
> #ifndef CONFIG_USER_ONLY
> if (tcg_enabled()) {
> - AddressSpace *as_normal = address_space_init_shareable(cs->memory,
> - "cpu-memory");
> + AddressSpace *as_normal = g_new0(AddressSpace, 1);
> AddressSpace *as_smm = g_new(AddressSpace, 1);
>
> + address_space_init(as_normal, cs->memory, "cpu-memory");
> +
> cpu->cpu_as_mem = g_new(MemoryRegion, 1);
> cpu->cpu_as_root = g_new(MemoryRegion, 1);
>
>
- [Qemu-devel] [PATCH qemu v5 06/18] memory: Switch memory from using AddressSpace to FlatView, (continued)
- [Qemu-devel] [PATCH qemu v5 06/18] memory: Switch memory from using AddressSpace to FlatView, Alexey Kardashevskiy, 2017/09/21
- [Qemu-devel] [PATCH qemu v5 12/18] memory: Share FlatView's and dispatch trees between address spaces, Alexey Kardashevskiy, 2017/09/21
- [Qemu-devel] [PATCH qemu v5 08/18] memory: Rename mem_begin/mem_commit/mem_add helpers, Alexey Kardashevskiy, 2017/09/21
- [Qemu-devel] [PATCH qemu v5 11/18] memory: Move address_space_update_ioeventfds, Alexey Kardashevskiy, 2017/09/21
- [Qemu-devel] [PATCH qemu v5 13/18] memory: Do not allocate FlatView in address_space_init, Alexey Kardashevskiy, 2017/09/21
- [Qemu-devel] [PATCH qemu v5 10/18] memory: Alloc dispatch tree where topology is generared, Alexey Kardashevskiy, 2017/09/21
- [Qemu-devel] [PATCH qemu v5 18/18] memory: Avoid temporary FlatView allocation in a single child case, Alexey Kardashevskiy, 2017/09/21
- [Qemu-devel] [PATCH qemu v5 14/18] memory: Rework "info mtree" to print flat views and dispatch trees, Alexey Kardashevskiy, 2017/09/21
- [Qemu-devel] [PATCH qemu v5 15/18] memory: Share special empty FlatView, Alexey Kardashevskiy, 2017/09/21
- [Qemu-devel] [PATCH qemu v5 16/18] memory: Get rid of address_space_init_shareable, Alexey Kardashevskiy, 2017/09/21
- Re: [Qemu-devel] [PATCH qemu v5 16/18] memory: Get rid of address_space_init_shareable,
Paolo Bonzini <=
- [Qemu-devel] [PATCH qemu v5 17/18] memory: Create FlatView directly, Alexey Kardashevskiy, 2017/09/21
- [Qemu-devel] [PATCH qemu v5 01/18] exec: Explicitly export target AS from address_space_translate_internal, Alexey Kardashevskiy, 2017/09/21
- Re: [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView, Paolo Bonzini, 2017/09/21