diff mbox series

[v1,05/13] hw/virtio/pci: include vdev name in registered PCI sections

Message ID 20200709141327.14631-6-alex.bennee@linaro.org
State Superseded
Headers show
Series misc rc0 fixes (docs, plugins, docker) | expand

Commit Message

Alex Bennée July 9, 2020, 2:13 p.m. UTC 
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(-)

-- 
2.20.1

Comments

Michael S. Tsirkin July 27, 2020, 1:32 p.m. UTC | #1 
On Thu, Jul 09, 2020 at 03:13:19PM +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>



So I don't know what's the plan here. I think ideally core would
just do it for us automatically. Why not?
If it can't my ack stands, anyway, pls
merge with rest of the patches if that is deemed appropriate.

> ---

> 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 8554cf2a038e..215e680c71f4 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
Alex Bennée July 27, 2020, 4:22 p.m. UTC | #2 
Michael S. Tsirkin <mst@redhat.com> writes:

> On Thu, Jul 09, 2020 at 03:13:19PM +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>

>

>

> So I don't know what's the plan here. I think ideally core would

> just do it for us automatically. Why not?

> If it can't my ack stands, anyway, pls

> merge with rest of the patches if that is deemed appropriate.


Yeah currently it's sitting in my plugins/next queue which isn't
targeting 5.1. As the hwprofile plugin is the only one I know using it I
figured I'd keep it with those.

>

>> ---

>> 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 8554cf2a038e..215e680c71f4 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



-- 
Alex Bennée
diff mbox series

Patch

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 8554cf2a038e..215e680c71f4 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);