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

[Michael S. Tsirkin]

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