* [Qemu-trivial] [PATCH] memory: give name every AddressSpace [not found] <517BC1F2.70405@redhat.com> @ 2013-04-29 2:11 ` Alexey Kardashevskiy 2013-04-29 8:16 ` Paolo Bonzini 0 siblings, 1 reply; 5+ messages in thread From: Alexey Kardashevskiy @ 2013-04-29 2:11 UTC (permalink / raw) To: Paolo Bonzini Cc: Alexey Kardashevskiy, qemu-trivial, qemu-devel, David Gibson 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> --- The number of AddressSpace structs is constantly growing (even without IOMMU) and it is getting harder to trace them so this is why I came up with this patch. Or there is a reason to hide those AddressSpace structs which I do not see, is not it? --- exec.c | 6 ++---- hw/pci/pci.c | 3 ++- hw/ppc/spapr_vio.c | 2 +- include/exec/memory.h | 2 +- memory.c | 7 ++++--- 5 files changed, 10 insertions(+), 10 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..b4f1182 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -840,7 +840,7 @@ void mtree_info(fprintf_function mon_printf, void *f); * @as: an uninitialized #AddressSpace * @root: a #MemoryRegion that routes addesses for the address space */ -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..b50f5e6 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((void *)as->name); g_free(as->current_map); g_free(as->ioeventfds); } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-trivial] [PATCH] memory: give name every AddressSpace 2013-04-29 2:11 ` [Qemu-trivial] [PATCH] memory: give name every AddressSpace Alexey Kardashevskiy @ 2013-04-29 8:16 ` Paolo Bonzini 2013-04-29 8:21 ` Alexey Kardashevskiy 0 siblings, 1 reply; 5+ messages in thread From: Paolo Bonzini @ 2013-04-29 8:16 UTC (permalink / raw) To: Alexey Kardashevskiy; +Cc: qemu-trivial, qemu-devel, David Gibson Il 29/04/2013 04:11, 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> > --- > > > The number of AddressSpace structs is constantly growing (even without IOMMU) > and it is getting harder to trace them so this is why I came up with this patch. > Or there is a reason to hide those AddressSpace structs which I do not see, > is not it? > > > --- > exec.c | 6 ++---- > hw/pci/pci.c | 3 ++- > hw/ppc/spapr_vio.c | 2 +- > include/exec/memory.h | 2 +- > memory.c | 7 ++++--- > 5 files changed, 10 insertions(+), 10 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..b4f1182 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -840,7 +840,7 @@ void mtree_info(fprintf_function mon_printf, void *f); > * @as: an uninitialized #AddressSpace > * @root: a #MemoryRegion that routes addesses for the address space > */ > -void address_space_init(AddressSpace *as, MemoryRegion *root); > +void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name); > > Please update the doc comment. > /** > diff --git a/memory.c b/memory.c > index cee3e59..b50f5e6 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((void *)as->name); No cast here. Paolo > g_free(as->current_map); > g_free(as->ioeventfds); > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-trivial] [PATCH] memory: give name every AddressSpace 2013-04-29 8:16 ` Paolo Bonzini @ 2013-04-29 8:21 ` Alexey Kardashevskiy 2013-04-29 9:25 ` Paolo Bonzini 0 siblings, 1 reply; 5+ messages in thread From: Alexey Kardashevskiy @ 2013-04-29 8:21 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-trivial, qemu-devel, David Gibson On 04/29/2013 06:16 PM, Paolo Bonzini wrote: > Il 29/04/2013 04:11, 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> >> --- >> >> >> The number of AddressSpace structs is constantly growing (even without IOMMU) >> and it is getting harder to trace them so this is why I came up with this patch. >> Or there is a reason to hide those AddressSpace structs which I do not see, >> is not it? >> >> >> --- >> exec.c | 6 ++---- >> hw/pci/pci.c | 3 ++- >> hw/ppc/spapr_vio.c | 2 +- >> include/exec/memory.h | 2 +- >> memory.c | 7 ++++--- >> 5 files changed, 10 insertions(+), 10 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..b4f1182 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -840,7 +840,7 @@ void mtree_info(fprintf_function mon_printf, void *f); >> * @as: an uninitialized #AddressSpace >> * @root: a #MemoryRegion that routes addesses for the address space >> */ >> -void address_space_init(AddressSpace *as, MemoryRegion *root); >> +void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name); >> >> > > Please update the doc comment. > >> /** >> diff --git a/memory.c b/memory.c >> index cee3e59..b50f5e6 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((void *)as->name); > > No cast here. ? CC ppc64-softmmu/memory.o /home/alexey/pcipassthru/qemu-impreza/memory.c: In function 'address_space_destroy': /home/alexey/pcipassthru/qemu-impreza/memory.c:1626:5: warning: passing argument 1 of 'g_free' discards 'const' qualifier from pointer target type [enabled by default] g_free(/*(void *)*/as->name); ^ In file included from /home/alexey/pcipassthru/lib4qemu/usr/include/glib-2.0/glib/glist.h:34:0, from /home/alexey/pcipassthru/lib4qemu/usr/include/glib-2.0/glib/ghash.h:35, from /home/alexey/pcipassthru/lib4qemu/usr/include/glib-2.0/glib.h:52, from /home/alexey/pcipassthru/qemu-impreza/include/glib-compat.h:17, from /home/alexey/pcipassthru/qemu-impreza/include/qemu-common.h:43, from /home/alexey/pcipassthru/qemu-impreza/include/exec/memory.h:21, from /home/alexey/pcipassthru/qemu-impreza/memory.c:16: /home/alexey/pcipassthru/lib4qemu/usr/include/glib-2.0/glib/gmem.h:70:7: note: expected 'gpointer' but argument is of type 'const char *' void g_free (gpointer mem); ^ > Paolo > >> g_free(as->current_map); >> g_free(as->ioeventfds); >> } >> > -- Alexey ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-trivial] [PATCH] memory: give name every AddressSpace 2013-04-29 8:21 ` Alexey Kardashevskiy @ 2013-04-29 9:25 ` Paolo Bonzini 2013-04-29 11:09 ` David Gibson 0 siblings, 1 reply; 5+ messages in thread From: Paolo Bonzini @ 2013-04-29 9:25 UTC (permalink / raw) To: Alexey Kardashevskiy; +Cc: qemu-trivial, qemu-devel, David Gibson Il 29/04/2013 10:21, Alexey Kardashevskiy ha scritto: >>> >> + g_free((void *)as->name); >> > >> > No cast here. > ? > > CC ppc64-softmmu/memory.o > /home/alexey/pcipassthru/qemu-impreza/memory.c: In function > 'address_space_destroy': > /home/alexey/pcipassthru/qemu-impreza/memory.c:1626:5: warning: passing > argument 1 of 'g_free' discards 'const' qualifier from pointer target type > [enabled by default] > g_free(/*(void *)*/as->name); > ^ Please remove the const from as->name instead. Since you are strdup-ing it, and the field is meant to be private to memory.c anyway, you do not need protection against changing it. Paolo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-trivial] [PATCH] memory: give name every AddressSpace 2013-04-29 9:25 ` Paolo Bonzini @ 2013-04-29 11:09 ` David Gibson 0 siblings, 0 replies; 5+ messages in thread From: David Gibson @ 2013-04-29 11:09 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Alexey Kardashevskiy, qemu-trivial, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1046 bytes --] On Mon, Apr 29, 2013 at 11:25:16AM +0200, Paolo Bonzini wrote: > Il 29/04/2013 10:21, Alexey Kardashevskiy ha scritto: > >>> >> + g_free((void *)as->name); > >> > > >> > No cast here. > > ? > > > > CC ppc64-softmmu/memory.o > > /home/alexey/pcipassthru/qemu-impreza/memory.c: In function > > 'address_space_destroy': > > /home/alexey/pcipassthru/qemu-impreza/memory.c:1626:5: warning: passing > > argument 1 of 'g_free' discards 'const' qualifier from pointer target type > > [enabled by default] > > g_free(/*(void *)*/as->name); > > ^ > > Please remove the const from as->name instead. Since you are strdup-ing > it, and the field is meant to be private to memory.c anyway, you do not > need protection against changing it. As a general rule, pointer variables that you malloc()ate should not be const. -- 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 --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-04-29 11:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <517BC1F2.70405@redhat.com>
2013-04-29 2:11 ` [Qemu-trivial] [PATCH] memory: give name every AddressSpace Alexey Kardashevskiy
2013-04-29 8:16 ` Paolo Bonzini
2013-04-29 8:21 ` Alexey Kardashevskiy
2013-04-29 9:25 ` Paolo Bonzini
2013-04-29 11:09 ` David Gibson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).