1. Patchew
  2. QEMU
  3. [PATCH v2 00/11] misc fixes for rc0 (docker, plugins, softfloat)
  4. View patch

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

Alex Bennée posted 11 patches 5 years, 7 months ago
[PATCH v2 09/11] hw/virtio/pci: include vdev name in registered PCI sections
Posted by Alex Bennée 5 years, 7 months ago 
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>

---
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
Re: [PATCH v2 09/11] hw/virtio/pci: include vdev name in registered PCI sections
Posted by Michael S. Tsirkin 5 years, 7 months ago 
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),
>                            &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
Re: [PATCH v2 09/11] hw/virtio/pci: include vdev name in registered PCI sections
Posted by Philippe Mathieu-Daudé 5 years, 7 months ago 
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
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.