Re: [PATCH v2 09/11] hw/virtio/pci: include vdev name in registered PCI sections

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

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),
>>                            &notify_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),
>>                            &notify_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
>