[Qemu-devel] [PATCH v2] memory: give name every AddressSpace

[Qemu-devel] [PATCH v2] memory: give name every AddressSpace

[Alexey Kardashevskiy]

The "info mtree" command in QEMU console prints only "memory" and "I/O"
address spaces while there are actually a lot more other AddressSpace
structs created by PCI and VIO devices. Those devices do not normally
have names and therefore not present in "info qtree" output.

The patch fixes this.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

v2:
* removed "const" from AddressSpace::name
* removed redundand check for name!=NULL from mtree_info

---
 exec.c                |    6 ++----
 hw/pci/pci.c          |    3 ++-
 hw/ppc/spapr_vio.c    |    2 +-
 include/exec/memory.h |    5 +++--
 memory.c              |   10 ++++------
 5 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/exec.c b/exec.c
index 180a345..0091272 100644
--- a/exec.c
+++ b/exec.c
@@ -1839,13 +1839,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");
 
     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 fe146dc..0a1acd6 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -823,7 +823,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     memory_region_init_alias(&pci_dev->bus_master_enable_region, "bus master",
                              pci_dev->iommu, 0, memory_region_size(pci_dev->iommu));
     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,
+                       name);
 
     pci_dev->devfn = devfn;
     pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index fbcbd6f..fadde20 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -449,7 +449,7 @@ static int spapr_vio_busdev_init(DeviceState *qdev)
     if (pc->rtce_window_size) {
         uint32_t liobn = SPAPR_VIO_BASE_LIOBN | dev->reg;
         dev->tcet = spapr_tce_new_table(liobn, pc->rtce_window_size);
-        address_space_init(&dev->as, spapr_tce_get_iommu(dev->tcet));
+        address_space_init(&dev->as, spapr_tce_get_iommu(dev->tcet), qdev->id);
     }
 
     return pc->init(dev);
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 99d51b7..907118e 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -177,7 +177,7 @@ struct MemoryRegionPortio {
  */
 struct AddressSpace {
     /* All fields are private. */
-    const char *name;
+    char *name;
     MemoryRegion *root;
     struct FlatView *current_map;
     int ioeventfd_nb;
@@ -839,8 +839,9 @@ void mtree_info(fprintf_function mon_printf, void *f);
  *
  * @as: an uninitialized #AddressSpace
  * @root: a #MemoryRegion that routes addesses for the address space
+ * @name: an address space name
  */
-void address_space_init(AddressSpace *as, MemoryRegion *root);
+void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name);
 
 
 /**
diff --git a/memory.c b/memory.c
index cee3e59..b4117a8 100644
--- a/memory.c
+++ b/memory.c
@@ -1002,7 +1002,7 @@ void memory_region_init_iommu(MemoryRegion *mr,
     mr->terminates = true;  /* then re-forwards */
     mr->destructor = memory_region_destructor_iommu;
     mr->iommu_target_as = g_new(AddressSpace, 1);
-    address_space_init(mr->iommu_target_as, target);
+    address_space_init(mr->iommu_target_as, target, name);
 }
 
 static uint64_t invalid_read(void *opaque, hwaddr addr,
@@ -1599,7 +1599,7 @@ 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;
@@ -1608,7 +1608,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root)
     as->ioeventfd_nb = 0;
     as->ioeventfds = NULL;
     QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
-    as->name = NULL;
+    as->name = g_strdup(name?name:"anonymous");
     address_space_init_dispatch(as);
     memory_region_update_pending |= root->enabled;
     memory_region_transaction_commit();
@@ -1623,6 +1623,7 @@ void address_space_destroy(AddressSpace *as)
     QTAILQ_REMOVE(&address_spaces, as, address_spaces_link);
     address_space_destroy_dispatch(as);
     flatview_destroy(as->current_map);
+    g_free(as->name);
     g_free(as->current_map);
     g_free(as->ioeventfds);
 }
@@ -1749,9 +1750,6 @@ void mtree_info(fprintf_function mon_printf, void *f)
     QTAILQ_INIT(&ml_head);
 
     QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
-        if (!as->name) {
-            continue;
-        }
         mon_printf(f, "%s\n", as->name);
         mtree_print_mr(mon_printf, f, as->root, 0, 0, &ml_head);
     }
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2] memory: give name every AddressSpace

[Paolo Bonzini]

Il 30/04/2013 04:25, Alexey Kardashevskiy ha scritto:
> The "info mtree" command in QEMU console prints only "memory" and "I/O"
> address spaces while there are actually a lot more other AddressSpace
> structs created by PCI and VIO devices. Those devices do not normally
> have names and therefore not present in "info qtree" output.
> 
> The patch fixes this.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> v2:
> * removed "const" from AddressSpace::name
> * removed redundand check for name!=NULL from mtree_info
> 
> ---
>  exec.c                |    6 ++----
>  hw/pci/pci.c          |    3 ++-
>  hw/ppc/spapr_vio.c    |    2 +-
>  include/exec/memory.h |    5 +++--
>  memory.c              |   10 ++++------
>  5 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 180a345..0091272 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1839,13 +1839,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");
>  
>      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 fe146dc..0a1acd6 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -823,7 +823,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>      memory_region_init_alias(&pci_dev->bus_master_enable_region, "bus master",
>                               pci_dev->iommu, 0, memory_region_size(pci_dev->iommu));
>      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,
> +                       name);
>  
>      pci_dev->devfn = devfn;
>      pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index fbcbd6f..fadde20 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -449,7 +449,7 @@ static int spapr_vio_busdev_init(DeviceState *qdev)
>      if (pc->rtce_window_size) {
>          uint32_t liobn = SPAPR_VIO_BASE_LIOBN | dev->reg;
>          dev->tcet = spapr_tce_new_table(liobn, pc->rtce_window_size);
> -        address_space_init(&dev->as, spapr_tce_get_iommu(dev->tcet));
> +        address_space_init(&dev->as, spapr_tce_get_iommu(dev->tcet), qdev->id);
>      }
>  
>      return pc->init(dev);
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 99d51b7..907118e 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -177,7 +177,7 @@ struct MemoryRegionPortio {
>   */
>  struct AddressSpace {
>      /* All fields are private. */
> -    const char *name;
> +    char *name;
>      MemoryRegion *root;
>      struct FlatView *current_map;
>      int ioeventfd_nb;
> @@ -839,8 +839,9 @@ void mtree_info(fprintf_function mon_printf, void *f);
>   *
>   * @as: an uninitialized #AddressSpace
>   * @root: a #MemoryRegion that routes addesses for the address space
> + * @name: an address space name
>   */
> -void address_space_init(AddressSpace *as, MemoryRegion *root);
> +void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name);
>  
>  
>  /**
> diff --git a/memory.c b/memory.c
> index cee3e59..b4117a8 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1002,7 +1002,7 @@ void memory_region_init_iommu(MemoryRegion *mr,
>      mr->terminates = true;  /* then re-forwards */
>      mr->destructor = memory_region_destructor_iommu;
>      mr->iommu_target_as = g_new(AddressSpace, 1);
> -    address_space_init(mr->iommu_target_as, target);
> +    address_space_init(mr->iommu_target_as, target, name);
>  }
>  
>  static uint64_t invalid_read(void *opaque, hwaddr addr,
> @@ -1599,7 +1599,7 @@ 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;
> @@ -1608,7 +1608,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root)
>      as->ioeventfd_nb = 0;
>      as->ioeventfds = NULL;
>      QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
> -    as->name = NULL;
> +    as->name = g_strdup(name?name:"anonymous");
>      address_space_init_dispatch(as);
>      memory_region_update_pending |= root->enabled;
>      memory_region_transaction_commit();
> @@ -1623,6 +1623,7 @@ void address_space_destroy(AddressSpace *as)
>      QTAILQ_REMOVE(&address_spaces, as, address_spaces_link);
>      address_space_destroy_dispatch(as);
>      flatview_destroy(as->current_map);
> +    g_free(as->name);
>      g_free(as->current_map);
>      g_free(as->ioeventfds);
>  }
> @@ -1749,9 +1750,6 @@ void mtree_info(fprintf_function mon_printf, void *f)
>      QTAILQ_INIT(&ml_head);
>  
>      QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> -        if (!as->name) {
> -            continue;
> -        }
>          mon_printf(f, "%s\n", as->name);
>          mtree_print_mr(mon_printf, f, as->root, 0, 0, &ml_head);
>      }
> 

Applied to iommu branch, thanks.

Paolo

Re: [Qemu-devel] [PATCH v2] memory: give name every AddressSpace

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 6671 bytes --]

On Tue, Apr 30, 2013 at 09:31:52AM +0200, Paolo Bonzini wrote:
> Il 30/04/2013 04:25, Alexey Kardashevskiy ha scritto:
> > The "info mtree" command in QEMU console prints only "memory" and "I/O"
> > address spaces while there are actually a lot more other AddressSpace
> > structs created by PCI and VIO devices. Those devices do not normally
> > have names and therefore not present in "info qtree" output.
> > 
> > The patch fixes this.
> > 
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> > 
> > v2:
> > * removed "const" from AddressSpace::name
> > * removed redundand check for name!=NULL from mtree_info
> > 
> > ---
> >  exec.c                |    6 ++----
> >  hw/pci/pci.c          |    3 ++-
> >  hw/ppc/spapr_vio.c    |    2 +-
> >  include/exec/memory.h |    5 +++--
> >  memory.c              |   10 ++++------
> >  5 files changed, 12 insertions(+), 14 deletions(-)
> > 
> > diff --git a/exec.c b/exec.c
> > index 180a345..0091272 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1839,13 +1839,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");
> >  
> >      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 fe146dc..0a1acd6 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -823,7 +823,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> >      memory_region_init_alias(&pci_dev->bus_master_enable_region, "bus master",
> >                               pci_dev->iommu, 0, memory_region_size(pci_dev->iommu));
> >      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,
> > +                       name);
> >  
> >      pci_dev->devfn = devfn;
> >      pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
> > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> > index fbcbd6f..fadde20 100644
> > --- a/hw/ppc/spapr_vio.c
> > +++ b/hw/ppc/spapr_vio.c
> > @@ -449,7 +449,7 @@ static int spapr_vio_busdev_init(DeviceState *qdev)
> >      if (pc->rtce_window_size) {
> >          uint32_t liobn = SPAPR_VIO_BASE_LIOBN | dev->reg;
> >          dev->tcet = spapr_tce_new_table(liobn, pc->rtce_window_size);
> > -        address_space_init(&dev->as, spapr_tce_get_iommu(dev->tcet));
> > +        address_space_init(&dev->as, spapr_tce_get_iommu(dev->tcet), qdev->id);
> >      }
> >  
> >      return pc->init(dev);
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index 99d51b7..907118e 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -177,7 +177,7 @@ struct MemoryRegionPortio {
> >   */
> >  struct AddressSpace {
> >      /* All fields are private. */
> > -    const char *name;
> > +    char *name;
> >      MemoryRegion *root;
> >      struct FlatView *current_map;
> >      int ioeventfd_nb;
> > @@ -839,8 +839,9 @@ void mtree_info(fprintf_function mon_printf, void *f);
> >   *
> >   * @as: an uninitialized #AddressSpace
> >   * @root: a #MemoryRegion that routes addesses for the address space
> > + * @name: an address space name
> >   */
> > -void address_space_init(AddressSpace *as, MemoryRegion *root);
> > +void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name);
> >  
> >  
> >  /**
> > diff --git a/memory.c b/memory.c
> > index cee3e59..b4117a8 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -1002,7 +1002,7 @@ void memory_region_init_iommu(MemoryRegion *mr,
> >      mr->terminates = true;  /* then re-forwards */
> >      mr->destructor = memory_region_destructor_iommu;
> >      mr->iommu_target_as = g_new(AddressSpace, 1);
> > -    address_space_init(mr->iommu_target_as, target);
> > +    address_space_init(mr->iommu_target_as, target, name);
> >  }
> >  
> >  static uint64_t invalid_read(void *opaque, hwaddr addr,
> > @@ -1599,7 +1599,7 @@ 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;
> > @@ -1608,7 +1608,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root)
> >      as->ioeventfd_nb = 0;
> >      as->ioeventfds = NULL;
> >      QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
> > -    as->name = NULL;
> > +    as->name = g_strdup(name?name:"anonymous");
> >      address_space_init_dispatch(as);
> >      memory_region_update_pending |= root->enabled;
> >      memory_region_transaction_commit();
> > @@ -1623,6 +1623,7 @@ void address_space_destroy(AddressSpace *as)
> >      QTAILQ_REMOVE(&address_spaces, as, address_spaces_link);
> >      address_space_destroy_dispatch(as);
> >      flatview_destroy(as->current_map);
> > +    g_free(as->name);
> >      g_free(as->current_map);
> >      g_free(as->ioeventfds);
> >  }
> > @@ -1749,9 +1750,6 @@ void mtree_info(fprintf_function mon_printf, void *f)
> >      QTAILQ_INIT(&ml_head);
> >  
> >      QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> > -        if (!as->name) {
> > -            continue;
> > -        }
> >          mon_printf(f, "%s\n", as->name);
> >          mtree_print_mr(mon_printf, f, as->root, 0, 0, &ml_head);
> >      }
> > 
> 
> Applied to iommu branch, thanks.

I don't see this patch at git://github.com/bonzini/qemu.git yet.  Did
you need to do something to push it out?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [Qemu-devel] [PATCH v2] memory: give name every AddressSpace

[Paolo Bonzini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 01/05/2013 05:25, David Gibson ha scritto:
>>> Applied to iommu branch, thanks.
> I don't see this patch at git://github.com/bonzini/qemu.git yet.
> Did you need to do something to push it out?

I just applied it locally for now.

Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRgS/NAAoJEBvWZb6bTYbyr2wP/0SE8WDVpcteMf0oO+W/o1jg
nav1QqSiVWs6KbFR5uQsiLK3l+Rv1BKKf5FZoGiOSvHxyr7/tYpv6MmOAMYyKqVF
+KsYZ5J8snL9OyZviSkUPvmYF/v8+MZxBAMJHB5ryjpZoDk/4y5aT0LrPl6NLeqy
fkXToabzPFWHEWVwa6JkHgyS1IFxYM0zgobCv82TGypSAebded02BvrIzUV6jSI0
iEjIAKdZiSK2pdgHnZFRHqd0I8+NHjeSaXXX5YxhtE0R3uCMNoRBiam9t5aGVIYb
HLwGq94/ikf4gDnntkE6/J9zAwVcHltp97InGEhOQRP/71xgGjTbRVDn7v5kadwC
UMZzGA+fBVhI80Zjnb280hQVPHihgicO3koaaEkW6zBDGuCZZtr2yocQFnVYRZhH
CnQ5yUG6SZCgH+RlsIZ8mh/bEknH21+IF4xdTv5j3gSH8R5Y3I+l3KShTMDCcEfc
+ItrdQQLJXMlPXXRCeG6N1+Hkg1VpcnhGBpfIXqUTgfJtvGqi+TzEMqt8KGeIMEf
7YLfpHm7vd5kseBo6u85XScAcxPazfoSn8vTb0yFiU+YeSUstZzoAwoQg/7lzarB
1ouWvNrT9plNywg2aMGO34tCQCSCDwMlsQ47HMGxKiCTlVEczz758Xki1NAURCTy
uza3CdJb9OrYDQ5qeDOi
=jcUq
-----END PGP SIGNATURE-----