[1/1] memory: add name in AddressSpace initialization.

Pass the AddressSpace's name (to be used for debugging)
to address_space_init(). If NULL is passed, the name
of root memory region is used instead.

Signed-off-by: Alexander Barabash <alexander_barabash@mentor.com>
---
 exec.c                |    6 ++----
 hw/pci/pci.c          |    3 ++-
 include/exec/memory.h |    5 +++--
 memory.c              |    5 +++--
 4 files changed, 10 insertions(+), 9 deletions(-)

Alexander Barabash <alexander_barabash@mentor.com> writes:

> Pass the AddressSpace's name (to be used for debugging)
> to address_space_init(). If NULL is passed, the name
> of root memory region is used instead.
>
> Signed-off-by: Alexander Barabash <alexander_barabash@mentor.com>
> ---
>  exec.c                |    6 ++----
>  hw/pci/pci.c          |    3 ++-
>  include/exec/memory.h |    5 +++--
>  memory.c              |    5 +++--
>  4 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index a6923ad..4637c28 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1784,13 +1784,11 @@ static void memory_map_init(void)
>  {
>      system_memory = g_malloc(sizeof(*system_memory));
>      memory_region_init(system_memory, "system", INT64_MAX);
> -    address_space_init(&address_space_memory, system_memory);
> -    address_space_memory.name = "memory";
> +    address_space_init(&address_space_memory, system_memory, "memory");
>  
>      system_io = g_malloc(sizeof(*system_io));
>      memory_region_init(system_io, "io", 65536);
> -    address_space_init(&address_space_io, system_io);
> -    address_space_io.name = "I/O";
> +    address_space_init(&address_space_io, system_io, "I/O");

It's best to avoid special characters in names.  It will make a
conversion to QOM later a lot easier.  So s:I/O:IO:g'

>  
>      memory_listener_register(&core_memory_listener, &address_space_memory);
>      memory_listener_register(&io_memory_listener, &address_space_io);
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 94840c4..6cc3abe 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -790,7 +790,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>                                   get_system_memory(), 0,
>                                   memory_region_size(get_system_memory()));
>          memory_region_set_enabled(&pci_dev->bus_master_enable_region, false);
> -        address_space_init(&pci_dev->bus_master_as, &pci_dev->bus_master_enable_region);
> +        address_space_init(&pci_dev->bus_master_as,
> +                           &pci_dev->bus_master_enable_region, NULL);

Please top-post new patches.

Any reason to not name this address space?

Regards,

Anthony Liguori


>          pci_dev->dma = g_new(DMAContext, 1);
>          dma_context_init(pci_dev->dma, &pci_dev->bus_master_as, NULL, NULL, NULL);
>      }
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 2322732..cb79a65 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -817,9 +817,10 @@ void mtree_info(fprintf_function mon_printf, void *f);
>   *
>   * @as: an uninitialized #AddressSpace
>   * @root: a #MemoryRegion that routes addesses for the address space
> + * @name: used for debugging. If NULL, name of the root memory region is used.
>   */
> -void address_space_init(AddressSpace *as, MemoryRegion *root);
> -
> +void address_space_init(AddressSpace *as, MemoryRegion *root,
> +                         const char *name);
>  
>  /**
>   * address_space_destroy: destroy an address space
> diff --git a/memory.c b/memory.c
> index 410c5f8..ab3b136 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1562,14 +1562,14 @@ void memory_listener_unregister(MemoryListener *listener)
>      QTAILQ_REMOVE(&memory_listeners, listener, link);
>  }
>  
> -void address_space_init(AddressSpace *as, MemoryRegion *root)
> +void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
>  {
>      memory_region_transaction_begin();
>      as->root = root;
>      as->current_map = g_new(FlatView, 1);
>      flatview_init(as->current_map);
>      QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
> -    as->name = NULL;
> +    as->name = g_strdup(name ? name : root->name);
>      memory_region_transaction_commit();
>      address_space_init_dispatch(as);
>  }
> @@ -1584,6 +1584,7 @@ void address_space_destroy(AddressSpace *as)
>      address_space_destroy_dispatch(as);
>      flatview_destroy(as->current_map);
>      g_free(as->current_map);
> +    g_free((char *)as->name);
>  }
>  
>  uint64_t io_mem_read(MemoryRegion *mr, hwaddr addr, unsigned size)
> -- 
> 1.7.9.5

On 01/08/2013 04:00 PM, Anthony Liguori wrote:
> Alexander Barabash <alexander_barabash@mentor.com> writes:
>
>> Pass the AddressSpace's name (to be used for debugging)
>> to address_space_init(). If NULL is passed, the name
>> of root memory region is used instead.
>>
>> @@ -1784,13 +1784,11 @@ static void memory_map_init(void)
...
>>       memory_region_init(system_io, "io", 65536);
>> -    address_space_init(&address_space_io, system_io);
>> -    address_space_io.name = "I/O";
>> +    address_space_init(&address_space_io, system_io, "I/O");
> It's best to avoid special characters in names.  It will make a
> conversion to QOM later a lot easier.  So s:I/O:IO:g'

I did not change the name of this address space.
We can do it in a separate patch.
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -790,7 +790,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>>                                    get_system_memory(), 0,
>>                                    memory_region_size(get_system_memory()));
>>           memory_region_set_enabled(&pci_dev->bus_master_enable_region, false);
>> -        address_space_init(&pci_dev->bus_master_as, &pci_dev->bus_master_enable_region);
>> +        address_space_init(&pci_dev->bus_master_as,
>> +                           &pci_dev->bus_master_enable_region, NULL);
> Please top-post new patches.

I assume you this to mean, "post new patches as reply to the previous 
one". ) OK

> Any reason to not name this address space?
It was not named previously. Now, it will pick up the name from the root 
memory region,
passed to the same call.
> Regards,
>
> Anthony Liguori

Regards,
Alex Barabash

>
>
>>           pci_dev->dma = g_new(DMAContext, 1);
>>           dma_context_init(pci_dev->dma, &pci_dev->bus_master_as, NULL, NULL, NULL);
>>       }
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index 2322732..cb79a65 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -817,9 +817,10 @@ void mtree_info(fprintf_function mon_printf, void *f);
>>    *
>>    * @as: an uninitialized #AddressSpace
>>    * @root: a #MemoryRegion that routes addesses for the address space
>> + * @name: used for debugging. If NULL, name of the root memory region is used.
>>    */
>> -void address_space_init(AddressSpace *as, MemoryRegion *root);
>> -
>> +void address_space_init(AddressSpace *as, MemoryRegion *root,
>> +                         const char *name);
>>   
>>   /**
>>    * address_space_destroy: destroy an address space
>> diff --git a/memory.c b/memory.c
>> index 410c5f8..ab3b136 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -1562,14 +1562,14 @@ void memory_listener_unregister(MemoryListener *listener)
>>       QTAILQ_REMOVE(&memory_listeners, listener, link);
>>   }
>>   
>> -void address_space_init(AddressSpace *as, MemoryRegion *root)
>> +void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
>>   {
>>       memory_region_transaction_begin();
>>       as->root = root;
>>       as->current_map = g_new(FlatView, 1);
>>       flatview_init(as->current_map);
>>       QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
>> -    as->name = NULL;
>> +    as->name = g_strdup(name ? name : root->name);
>>       memory_region_transaction_commit();
>>       address_space_init_dispatch(as);
>>   }
>> @@ -1584,6 +1584,7 @@ void address_space_destroy(AddressSpace *as)
>>       address_space_destroy_dispatch(as);
>>       flatview_destroy(as->current_map);
>>       g_free(as->current_map);
>> +    g_free((char *)as->name);
>>   }
>>   
>>   uint64_t io_mem_read(MemoryRegion *mr, hwaddr addr, unsigned size)
>> -- 
>> 1.7.9.5