| Message ID | 20200713200415.26214-10-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | misc fixes for rc0 (docker, plugins, softfloat) | expand |
On Mon, Jul 13, 2020 at 09:04:13PM +0100, Alex Bennée wrote: > When viewing/debugging memory regions it is sometimes hard to figure > out which PCI device something belongs to. Make the names unique by > including the vdev name in the name string. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> I wonder here: why don't we see the owner name when debugging? Maybe that's a better way to address that. > --- > v2 > - swap ()'s for an extra - > --- > hw/virtio/virtio-pci.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 8554cf2a03..215e680c71 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1406,7 +1406,8 @@ static void virtio_pci_device_write(void *opaque, hwaddr addr, > } > } > > -static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy) > +static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy, > + const char *vdev_name) > { > static const MemoryRegionOps common_ops = { > .read = virtio_pci_common_read, > @@ -1453,36 +1454,41 @@ static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy) > }, > .endianness = DEVICE_LITTLE_ENDIAN, > }; > + g_autoptr(GString) name = g_string_new(NULL); > > - > + g_string_printf(name, "virtio-pci-common-%s", vdev_name); > memory_region_init_io(&proxy->common.mr, OBJECT(proxy), > &common_ops, > proxy, > - "virtio-pci-common", > + name->str, > proxy->common.size); > > + g_string_printf(name, "virtio-pci-isr-%s", vdev_name); > memory_region_init_io(&proxy->isr.mr, OBJECT(proxy), > &isr_ops, > proxy, > - "virtio-pci-isr", > + name->str, > proxy->isr.size); > > + g_string_printf(name, "virtio-pci-device-%s", vdev_name); > memory_region_init_io(&proxy->device.mr, OBJECT(proxy), > &device_ops, > virtio_bus_get_device(&proxy->bus), > - "virtio-pci-device", > + name->str, > proxy->device.size); > > + g_string_printf(name, "virtio-pci-notify-%s", vdev_name); > memory_region_init_io(&proxy->notify.mr, OBJECT(proxy), > ¬ify_ops, > virtio_bus_get_device(&proxy->bus), > - "virtio-pci-notify", > + name->str, > proxy->notify.size); > > + g_string_printf(name, "virtio-pci-notify-pio-%s", vdev_name); > memory_region_init_io(&proxy->notify_pio.mr, OBJECT(proxy), > ¬ify_pio_ops, > virtio_bus_get_device(&proxy->bus), > - "virtio-pci-notify-pio", > + name->str, > proxy->notify_pio.size); > } > > @@ -1623,7 +1629,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) > > struct virtio_pci_cfg_cap *cfg_mask; > > - virtio_pci_modern_regions_init(proxy); > + virtio_pci_modern_regions_init(proxy, vdev->name); > > virtio_pci_modern_mem_region_map(proxy, &proxy->common, &cap); > virtio_pci_modern_mem_region_map(proxy, &proxy->isr, &cap); > -- > 2.20.1
On 7/14/20 11:07 AM, Michael S. Tsirkin wrote: > On Mon, Jul 13, 2020 at 09:04:13PM +0100, Alex Bennée wrote: >> When viewing/debugging memory regions it is sometimes hard to figure >> out which PCI device something belongs to. Make the names unique by >> including the vdev name in the name string. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > > I wonder here: why don't we see the owner name when debugging? Maybe that's a better > way to address that. It depends what you mean mean by "debugging", gdb? trace events? monitor? Something else? From the monitor you can use the '-o' option: (qemu) help info mtree info mtree [-f][-d][-o][-D] -- show memory tree (-f: dump flat view for address spaces;-d: dump dispatch tree, valid with -f only);-o: dump region owners/parents;-D: dump disabled regions (qemu) info mtree -o address-space: memory 0000000000000000-ffffffffffffffff (prio 0, i/o): system parent:{obj path=/machine/unattached} 0000000000000000-0000000007ffffff (prio 0, ram): alias ram-below-4g @pc.ram 0000000000000000-0000000007ffffff parent:{obj path=/machine/unattached} 0000000000000000-ffffffffffffffff (prio -1, i/o): pci parent:{obj path=/machine/unattached} 00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem owner:{dev path=/machine/unattached/device[28]} 00000000000c0000-00000000000dffff (prio 1, rom): pc.rom parent:{obj path=/machine/unattached} 00000000000e0000-00000000000fffff (prio 1, rom): alias isa-bios @pc.bios 0000000000020000-000000000003ffff parent:{obj path=/machine/unattached} 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios parent:{obj path=/machine/unattached} 00000000000a0000-00000000000bffff (prio 1, i/o): alias smram-region @pci 00000000000a0000-00000000000bffff owner:{dev path=/machine/q35/mch} 00000000000c0000-00000000000c3fff (prio 1, i/o): alias pam-pci @pci 00000000000c0000-00000000000c3fff owner:{dev path=/machine/q35/mch} 00000000000c4000-00000000000c7fff (prio 1, i/o): alias pam-pci @pci 00000000000c4000-00000000000c7fff owner:{dev path=/machine/q35/mch} 00000000000c8000-00000000000cbfff (prio 1, i/o): alias pam-pci @pci 00000000000c8000-00000000000cbfff owner:{dev path=/machine/q35/mch} 00000000000cc000-00000000000cffff (prio 1, i/o): alias pam-pci @pci 00000000000cc000-00000000000cffff owner:{dev path=/machine/q35/mch} 00000000000d0000-00000000000d3fff (prio 1, i/o): alias pam-pci @pci 00000000000d0000-00000000000d3fff owner:{dev path=/machine/q35/mch} 00000000000d4000-00000000000d7fff (prio 1, i/o): alias pam-pci @pci 00000000000d4000-00000000000d7fff owner:{dev path=/machine/q35/mch} 00000000000d8000-00000000000dbfff (prio 1, i/o): alias pam-pci @pci 00000000000d8000-00000000000dbfff owner:{dev path=/machine/q35/mch} 00000000000dc000-00000000000dffff (prio 1, i/o): alias pam-pci @pci 00000000000dc000-00000000000dffff owner:{dev path=/machine/q35/mch} 00000000000e0000-00000000000e3fff (prio 1, i/o): alias pam-pci @pci 00000000000e0000-00000000000e3fff owner:{dev path=/machine/q35/mch} 00000000000e4000-00000000000e7fff (prio 1, i/o): alias pam-pci @pci 00000000000e4000-00000000000e7fff owner:{dev path=/machine/q35/mch} 00000000000e8000-00000000000ebfff (prio 1, i/o): alias pam-pci @pci 00000000000e8000-00000000000ebfff owner:{dev path=/machine/q35/mch} 00000000000ec000-00000000000effff (prio 1, i/o): alias pam-pci @pci 00000000000ec000-00000000000effff owner:{dev path=/machine/q35/mch} 00000000000f0000-00000000000fffff (prio 1, i/o): alias pam-pci @pci 00000000000f0000-00000000000fffff owner:{dev path=/machine/q35/mch} 00000000fec00000-00000000fec00fff (prio 0, i/o): ioapic owner:{dev path=/machine/q35/ioapic} 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet owner:{dev path=/machine/unattached/device[5]} 00000000fee00000-00000000feefffff (prio 4096, i/o): apic-msi owner:{dev path=/machine/unattached/device[0]/lapic} For trace events it has been suggested to use object_get_canonical_path_component() but there is a discussion about if it is practical or not in its current state, see: https://www.mail-archive.com/qemu-devel@nongnu.org/msg722110.html For gdb I guess remember Marc-André sent a patch for the Python script, but it hasn't been merged apparently: https://www.mail-archive.com/qemu-devel@nongnu.org/msg659085.html > >> --- >> v2 >> - swap ()'s for an extra - >> --- >> hw/virtio/virtio-pci.c | 22 ++++++++++++++-------- >> 1 file changed, 14 insertions(+), 8 deletions(-) >> >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index 8554cf2a03..215e680c71 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -1406,7 +1406,8 @@ static void virtio_pci_device_write(void *opaque, hwaddr addr, >> } >> } >> >> -static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy) >> +static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy, >> + const char *vdev_name) >> { >> static const MemoryRegionOps common_ops = { >> .read = virtio_pci_common_read, >> @@ -1453,36 +1454,41 @@ static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy) >> }, >> .endianness = DEVICE_LITTLE_ENDIAN, >> }; >> + g_autoptr(GString) name = g_string_new(NULL); >> >> - >> + g_string_printf(name, "virtio-pci-common-%s", vdev_name); >> memory_region_init_io(&proxy->common.mr, OBJECT(proxy), >> &common_ops, >> proxy, >> - "virtio-pci-common", >> + name->str, >> proxy->common.size); >> >> + g_string_printf(name, "virtio-pci-isr-%s", vdev_name); >> memory_region_init_io(&proxy->isr.mr, OBJECT(proxy), >> &isr_ops, >> proxy, >> - "virtio-pci-isr", >> + name->str, >> proxy->isr.size); >> >> + g_string_printf(name, "virtio-pci-device-%s", vdev_name); >> memory_region_init_io(&proxy->device.mr, OBJECT(proxy), >> &device_ops, >> virtio_bus_get_device(&proxy->bus), >> - "virtio-pci-device", >> + name->str, >> proxy->device.size); >> >> + g_string_printf(name, "virtio-pci-notify-%s", vdev_name); >> memory_region_init_io(&proxy->notify.mr, OBJECT(proxy), >> ¬ify_ops, >> virtio_bus_get_device(&proxy->bus), >> - "virtio-pci-notify", >> + name->str, >> proxy->notify.size); >> >> + g_string_printf(name, "virtio-pci-notify-pio-%s", vdev_name); >> memory_region_init_io(&proxy->notify_pio.mr, OBJECT(proxy), >> ¬ify_pio_ops, >> virtio_bus_get_device(&proxy->bus), >> - "virtio-pci-notify-pio", >> + name->str, >> proxy->notify_pio.size); >> } >> >> @@ -1623,7 +1629,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) >> >> struct virtio_pci_cfg_cap *cfg_mask; >> >> - virtio_pci_modern_regions_init(proxy); >> + virtio_pci_modern_regions_init(proxy, vdev->name); >> >> virtio_pci_modern_mem_region_map(proxy, &proxy->common, &cap); >> virtio_pci_modern_mem_region_map(proxy, &proxy->isr, &cap); >> -- >> 2.20.1 >
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 8554cf2a03..215e680c71 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1406,7 +1406,8 @@ static void virtio_pci_device_write(void *opaque, hwaddr addr, } } -static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy) +static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy, + const char *vdev_name) { static const MemoryRegionOps common_ops = { .read = virtio_pci_common_read, @@ -1453,36 +1454,41 @@ static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy) }, .endianness = DEVICE_LITTLE_ENDIAN, }; + g_autoptr(GString) name = g_string_new(NULL); - + g_string_printf(name, "virtio-pci-common-%s", vdev_name); memory_region_init_io(&proxy->common.mr, OBJECT(proxy), &common_ops, proxy, - "virtio-pci-common", + name->str, proxy->common.size); + g_string_printf(name, "virtio-pci-isr-%s", vdev_name); memory_region_init_io(&proxy->isr.mr, OBJECT(proxy), &isr_ops, proxy, - "virtio-pci-isr", + name->str, proxy->isr.size); + g_string_printf(name, "virtio-pci-device-%s", vdev_name); memory_region_init_io(&proxy->device.mr, OBJECT(proxy), &device_ops, virtio_bus_get_device(&proxy->bus), - "virtio-pci-device", + name->str, proxy->device.size); + g_string_printf(name, "virtio-pci-notify-%s", vdev_name); memory_region_init_io(&proxy->notify.mr, OBJECT(proxy), ¬ify_ops, virtio_bus_get_device(&proxy->bus), - "virtio-pci-notify", + name->str, proxy->notify.size); + g_string_printf(name, "virtio-pci-notify-pio-%s", vdev_name); memory_region_init_io(&proxy->notify_pio.mr, OBJECT(proxy), ¬ify_pio_ops, virtio_bus_get_device(&proxy->bus), - "virtio-pci-notify-pio", + name->str, proxy->notify_pio.size); } @@ -1623,7 +1629,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) struct virtio_pci_cfg_cap *cfg_mask; - virtio_pci_modern_regions_init(proxy); + virtio_pci_modern_regions_init(proxy, vdev->name); virtio_pci_modern_mem_region_map(proxy, &proxy->common, &cap); virtio_pci_modern_mem_region_map(proxy, &proxy->isr, &cap);