On Thu, Nov 05, 2015 at 06:15:53PM +0000, Peter Maydell wrote:
> From: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
>
> This will either create a new AS or return a pointer to an
> already existing equivalent one, if we have already created
> an AS for the specified root memory region.
>
> The motivation is to reuse address spaces as much as possible.
> It's going to be quite common that bus masters out in device land
> have pointers to the same memory region for their mastering yet
> each will need to create its own address space. Let the memory
> API implement sharing for them.
>
> Aside from the perf optimisations, this should reduce the amount
> of redundant output on info mtree as well.
>
> Thee returned value will be malloced, but the malloc will be
> automatically freed when the AS runs out of refs.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
> [PMM: dropped check for NULL root as unused; added doc-comment;
> squashed Peter C's reference-counting patch into this one;
> don't compare name string when deciding if we can share ASes;
> read as->malloced before the unref of as->root to avoid possible
> read-after-free if as->root was the owner of as]
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> include/exec/memory.h | 18 ++++++++++++++++++
> memory.c | 27 +++++++++++++++++++++++++++
> 2 files changed, 45 insertions(+)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index e5d98f8..1d1740a 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -236,6 +236,8 @@ struct AddressSpace {
> struct rcu_head rcu;
> char *name;
> MemoryRegion *root;
> + int ref_count;
> + bool malloced;
>
> /* Accessed via RCU. */
> struct FlatView *current_map;
> @@ -1167,6 +1169,22 @@ 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
> diff --git a/memory.c b/memory.c
> index c435c88..6c98a71 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -2101,7 +2101,9 @@ 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 = g_new(FlatView, 1);
> flatview_init(as->current_map);
> as->ioeventfd_nb = 0;
> @@ -2116,6 +2118,7 @@ void address_space_init(AddressSpace *as, MemoryRegion
> *root, const char *name)
> static void do_address_space_destroy(AddressSpace *as)
> {
> MemoryListener *listener;
> + bool do_free = as->malloced;
>
> address_space_destroy_dispatch(as);
>
> @@ -2127,12 +2130,36 @@ static void do_address_space_destroy(AddressSpace *as)
> 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;
> +
> + QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> + if (root == as->root) {
> + as->ref_count++;
> + return as;
> + }
> + }
> +
> + as = g_malloc0(sizeof *as);
> + address_space_init(as, root, name);
Nit-pick but IIUC, address_space_init does not need AS to be zeroed
so this could be changed to a g_malloc().
either-way:
Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> + as->malloced = true;
> + return as;
> }
>
> 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;
> --
> 1.9.1
>
