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

[Alex Bennée]

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