[Patch 0/3] vfio: Misc cleanups

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [Patch 0/3] vfio: Misc cleanups
@ 2022-05-02  9:42 Yi Liu
  2022-05-02  9:42 ` [Patch 1/3] hw/vfio/pci: fix vfio_pci_hot_reset_result trace point Yi Liu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Yi Liu @ 2022-05-02  9:42 UTC (permalink / raw)
  To: alex.williamson, qemu-devel
  Cc: eric.auger, kevin.tian, yi.l.liu, eric.auger.pro

The three patches were part of iommufd RFC series, sent separately per
Kevin's suggestion.

https://lore.kernel.org/kvm/BN9PR11MB5276085CDF750807005A775B8CF39@BN9PR11MB5276.namprd11.prod.outlook.com/

Regards,
Yi Liu

Eric Auger (2):
  hw/vfio/pci: fix vfio_pci_hot_reset_result trace point
  vfio/pci: Use vbasedev local variable in vfio_realize()

Yi Liu (1):
  vfio/common: Rename VFIOGuestIOMMU::iommu into ::iommu_mr

 hw/vfio/common.c              | 16 +++++------
 hw/vfio/pci.c                 | 51 ++++++++++++++++++-----------------
 include/hw/vfio/vfio-common.h |  2 +-
 3 files changed, 35 insertions(+), 34 deletions(-)

-- 
2.27.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Patch 1/3] hw/vfio/pci: fix vfio_pci_hot_reset_result trace point
  2022-05-02  9:42 [Patch 0/3] vfio: Misc cleanups Yi Liu
@ 2022-05-02  9:42 ` Yi Liu
  2022-05-02 14:35   ` Alex Williamson
  2022-05-02  9:42 ` [Patch 2/3] vfio/pci: Use vbasedev local variable in vfio_realize() Yi Liu
  2022-05-02  9:42 ` [Patch 3/3] vfio/common: Rename VFIOGuestIOMMU::iommu into ::iommu_mr Yi Liu
  2 siblings, 1 reply; 6+ messages in thread
From: Yi Liu @ 2022-05-02  9:42 UTC (permalink / raw)
  To: alex.williamson, qemu-devel
  Cc: eric.auger, kevin.tian, yi.l.liu, eric.auger.pro

From: Eric Auger <eric.auger@redhat.com>

Properly output the errno string.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 hw/vfio/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 9fd9faee1d..4a66376be6 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2337,7 +2337,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
     g_free(reset);
 
     trace_vfio_pci_hot_reset_result(vdev->vbasedev.name,
-                                    ret ? "%m" : "Success");
+                                    ret ? strerror(errno) : "Success");
 
 out:
     /* Re-enable INTx on affected devices */
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Patch 1/3] hw/vfio/pci: fix vfio_pci_hot_reset_result trace point
  2022-05-02  9:42 ` [Patch 1/3] hw/vfio/pci: fix vfio_pci_hot_reset_result trace point Yi Liu
@ 2022-05-02 14:35   ` Alex Williamson
  2022-05-02 14:43     ` Eric Auger
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Williamson @ 2022-05-02 14:35 UTC (permalink / raw)
  To: Yi Liu; +Cc: eric.auger, kevin.tian, qemu-devel, eric.auger.pro

On Mon,  2 May 2022 02:42:21 -0700
Yi Liu <yi.l.liu@intel.com> wrote:

> From: Eric Auger <eric.auger@redhat.com>
> 
> Properly output the errno string.

More explanation please, why is it broken and how does this fix it?
Thanks,

Alex
 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
>  hw/vfio/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 9fd9faee1d..4a66376be6 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2337,7 +2337,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
>      g_free(reset);
>  
>      trace_vfio_pci_hot_reset_result(vdev->vbasedev.name,
> -                                    ret ? "%m" : "Success");
> +                                    ret ? strerror(errno) : "Success");
>  
>  out:
>      /* Re-enable INTx on affected devices */



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch 1/3] hw/vfio/pci: fix vfio_pci_hot_reset_result trace point
  2022-05-02 14:35   ` Alex Williamson
@ 2022-05-02 14:43     ` Eric Auger
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Auger @ 2022-05-02 14:43 UTC (permalink / raw)
  To: Alex Williamson, Yi Liu; +Cc: kevin.tian, qemu-devel, eric.auger.pro

Hi Alex,

On 5/2/22 16:35, Alex Williamson wrote:
> On Mon,  2 May 2022 02:42:21 -0700
> Yi Liu <yi.l.liu@intel.com> wrote:
>
>> From: Eric Auger <eric.auger@redhat.com>
>>
>> Properly output the errno string.
> More explanation please, why is it broken and how does this fix it?
> Thanks,
"%m" format specifier is not interpreted by the trace infrastructure and
thus "%m" is output instead of the actual errno string. Fix it by
outputting

strerror(errno).

Thanks

Eric

>
> Alex
>  
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>>  hw/vfio/pci.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 9fd9faee1d..4a66376be6 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -2337,7 +2337,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
>>      g_free(reset);
>>  
>>      trace_vfio_pci_hot_reset_result(vdev->vbasedev.name,
>> -                                    ret ? "%m" : "Success");
>> +                                    ret ? strerror(errno) : "Success");
>>  
>>  out:
>>      /* Re-enable INTx on affected devices */



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Patch 2/3] vfio/pci: Use vbasedev local variable in vfio_realize()
  2022-05-02  9:42 [Patch 0/3] vfio: Misc cleanups Yi Liu
  2022-05-02  9:42 ` [Patch 1/3] hw/vfio/pci: fix vfio_pci_hot_reset_result trace point Yi Liu
@ 2022-05-02  9:42 ` Yi Liu
  2022-05-02  9:42 ` [Patch 3/3] vfio/common: Rename VFIOGuestIOMMU::iommu into ::iommu_mr Yi Liu
  2 siblings, 0 replies; 6+ messages in thread
From: Yi Liu @ 2022-05-02  9:42 UTC (permalink / raw)
  To: alex.williamson, qemu-devel
  Cc: eric.auger, kevin.tian, yi.l.liu, eric.auger.pro

From: Eric Auger <eric.auger@redhat.com>

Using a VFIODevice handle local variable to improve the code readability.

no functional change intended

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 hw/vfio/pci.c | 49 +++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 4a66376be6..d71823fd05 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2803,6 +2803,7 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice *vdev)
 static void vfio_realize(PCIDevice *pdev, Error **errp)
 {
     VFIOPCIDevice *vdev = VFIO_PCI(pdev);
+    VFIODevice *vbasedev = &vdev->vbasedev;
     VFIODevice *vbasedev_iter;
     VFIOGroup *group;
     char *tmp, *subsys, group_path[PATH_MAX], *group_name;
@@ -2813,7 +2814,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
     int i, ret;
     bool is_mdev;
 
-    if (!vdev->vbasedev.sysfsdev) {
+    if (!vbasedev->sysfsdev) {
         if (!(~vdev->host.domain || ~vdev->host.bus ||
               ~vdev->host.slot || ~vdev->host.function)) {
             error_setg(errp, "No provided host device");
@@ -2821,24 +2822,24 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
                               "or -device vfio-pci,sysfsdev=PATH_TO_DEVICE\n");
             return;
         }
-        vdev->vbasedev.sysfsdev =
+        vbasedev->sysfsdev =
             g_strdup_printf("/sys/bus/pci/devices/%04x:%02x:%02x.%01x",
                             vdev->host.domain, vdev->host.bus,
                             vdev->host.slot, vdev->host.function);
     }
 
-    if (stat(vdev->vbasedev.sysfsdev, &st) < 0) {
+    if (stat(vbasedev->sysfsdev, &st) < 0) {
         error_setg_errno(errp, errno, "no such host device");
-        error_prepend(errp, VFIO_MSG_PREFIX, vdev->vbasedev.sysfsdev);
+        error_prepend(errp, VFIO_MSG_PREFIX, vbasedev->sysfsdev);
         return;
     }
 
-    vdev->vbasedev.name = g_path_get_basename(vdev->vbasedev.sysfsdev);
-    vdev->vbasedev.ops = &vfio_pci_ops;
-    vdev->vbasedev.type = VFIO_DEVICE_TYPE_PCI;
-    vdev->vbasedev.dev = DEVICE(vdev);
+    vbasedev->name = g_path_get_basename(vbasedev->sysfsdev);
+    vbasedev->ops = &vfio_pci_ops;
+    vbasedev->type = VFIO_DEVICE_TYPE_PCI;
+    vbasedev->dev = DEVICE(vdev);
 
-    tmp = g_strdup_printf("%s/iommu_group", vdev->vbasedev.sysfsdev);
+    tmp = g_strdup_printf("%s/iommu_group", vbasedev->sysfsdev);
     len = readlink(tmp, group_path, sizeof(group_path));
     g_free(tmp);
 
@@ -2856,7 +2857,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
         goto error;
     }
 
-    trace_vfio_realize(vdev->vbasedev.name, groupid);
+    trace_vfio_realize(vbasedev->name, groupid);
 
     group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), errp);
     if (!group) {
@@ -2864,7 +2865,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
     }
 
     QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
-        if (strcmp(vbasedev_iter->name, vdev->vbasedev.name) == 0) {
+        if (strcmp(vbasedev_iter->name, vbasedev->name) == 0) {
             error_setg(errp, "device is already attached");
             vfio_put_group(group);
             goto error;
@@ -2877,22 +2878,22 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
      * stays in sync with the active working set of the guest driver.  Prevent
      * the x-balloon-allowed option unless this is minimally an mdev device.
      */
-    tmp = g_strdup_printf("%s/subsystem", vdev->vbasedev.sysfsdev);
+    tmp = g_strdup_printf("%s/subsystem", vbasedev->sysfsdev);
     subsys = realpath(tmp, NULL);
     g_free(tmp);
     is_mdev = subsys && (strcmp(subsys, "/sys/bus/mdev") == 0);
     free(subsys);
 
-    trace_vfio_mdev(vdev->vbasedev.name, is_mdev);
+    trace_vfio_mdev(vbasedev->name, is_mdev);
 
-    if (vdev->vbasedev.ram_block_discard_allowed && !is_mdev) {
+    if (vbasedev->ram_block_discard_allowed && !is_mdev) {
         error_setg(errp, "x-balloon-allowed only potentially compatible "
                    "with mdev devices");
         vfio_put_group(group);
         goto error;
     }
 
-    ret = vfio_get_device(group, vdev->vbasedev.name, &vdev->vbasedev, errp);
+    ret = vfio_get_device(group, vbasedev->name, vbasedev, errp);
     if (ret) {
         vfio_put_group(group);
         goto error;
@@ -2905,7 +2906,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
     }
 
     /* Get a copy of config space */
-    ret = pread(vdev->vbasedev.fd, vdev->pdev.config,
+    ret = pread(vbasedev->fd, vdev->pdev.config,
                 MIN(pci_config_size(&vdev->pdev), vdev->config_size),
                 vdev->config_offset);
     if (ret < (int)MIN(pci_config_size(&vdev->pdev), vdev->config_size)) {
@@ -2933,7 +2934,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
             goto error;
         }
         vfio_add_emulated_word(vdev, PCI_VENDOR_ID, vdev->vendor_id, ~0);
-        trace_vfio_pci_emulated_vendor_id(vdev->vbasedev.name, vdev->vendor_id);
+        trace_vfio_pci_emulated_vendor_id(vbasedev->name, vdev->vendor_id);
     } else {
         vdev->vendor_id = pci_get_word(pdev->config + PCI_VENDOR_ID);
     }
@@ -2944,7 +2945,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
             goto error;
         }
         vfio_add_emulated_word(vdev, PCI_DEVICE_ID, vdev->device_id, ~0);
-        trace_vfio_pci_emulated_device_id(vdev->vbasedev.name, vdev->device_id);
+        trace_vfio_pci_emulated_device_id(vbasedev->name, vdev->device_id);
     } else {
         vdev->device_id = pci_get_word(pdev->config + PCI_DEVICE_ID);
     }
@@ -2956,7 +2957,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
         }
         vfio_add_emulated_word(vdev, PCI_SUBSYSTEM_VENDOR_ID,
                                vdev->sub_vendor_id, ~0);
-        trace_vfio_pci_emulated_sub_vendor_id(vdev->vbasedev.name,
+        trace_vfio_pci_emulated_sub_vendor_id(vbasedev->name,
                                               vdev->sub_vendor_id);
     }
 
@@ -2966,7 +2967,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
             goto error;
         }
         vfio_add_emulated_word(vdev, PCI_SUBSYSTEM_ID, vdev->sub_device_id, ~0);
-        trace_vfio_pci_emulated_sub_device_id(vdev->vbasedev.name,
+        trace_vfio_pci_emulated_sub_device_id(vbasedev->name,
                                               vdev->sub_device_id);
     }
 
@@ -3025,7 +3026,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
             goto out_teardown;
         }
 
-        ret = vfio_get_dev_region_info(&vdev->vbasedev,
+        ret = vfio_get_dev_region_info(vbasedev,
                         VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
                         VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, &opregion);
         if (ret) {
@@ -3101,9 +3102,9 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
     }
 
     if (!pdev->failover_pair_id) {
-        ret = vfio_migration_probe(&vdev->vbasedev, errp);
+        ret = vfio_migration_probe(vbasedev, errp);
         if (ret) {
-            error_report("%s: Migration disabled", vdev->vbasedev.name);
+            error_report("%s: Migration disabled", vbasedev->name);
         }
     }
 
@@ -3120,7 +3121,7 @@ out_teardown:
     vfio_teardown_msi(vdev);
     vfio_bars_exit(vdev);
 error:
-    error_prepend(errp, VFIO_MSG_PREFIX, vdev->vbasedev.name);
+    error_prepend(errp, VFIO_MSG_PREFIX, vbasedev->name);
 }
 
 static void vfio_instance_finalize(Object *obj)
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Patch 3/3] vfio/common: Rename VFIOGuestIOMMU::iommu into ::iommu_mr
  2022-05-02  9:42 [Patch 0/3] vfio: Misc cleanups Yi Liu
  2022-05-02  9:42 ` [Patch 1/3] hw/vfio/pci: fix vfio_pci_hot_reset_result trace point Yi Liu
  2022-05-02  9:42 ` [Patch 2/3] vfio/pci: Use vbasedev local variable in vfio_realize() Yi Liu
@ 2022-05-02  9:42 ` Yi Liu
  2 siblings, 0 replies; 6+ messages in thread
From: Yi Liu @ 2022-05-02  9:42 UTC (permalink / raw)
  To: alex.williamson, qemu-devel
  Cc: eric.auger, kevin.tian, yi.l.liu, eric.auger.pro

Rename VFIOGuestIOMMU iommu field into iommu_mr. Then it becomes clearer
it is an IOMMU memory region.

no functional change intended

Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 hw/vfio/common.c              | 16 ++++++++--------
 include/hw/vfio/vfio-common.h |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 2b1f78fdfa..287a2b6828 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -992,7 +992,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
          * device emulation the VFIO iommu handles to use).
          */
         giommu = g_malloc0(sizeof(*giommu));
-        giommu->iommu = iommu_mr;
+        giommu->iommu_mr = iommu_mr;
         giommu->iommu_offset = section->offset_within_address_space -
                                section->offset_within_region;
         giommu->container = container;
@@ -1007,7 +1007,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
                             int128_get64(llend),
                             iommu_idx);
 
-        ret = memory_region_iommu_set_page_size_mask(giommu->iommu,
+        ret = memory_region_iommu_set_page_size_mask(giommu->iommu_mr,
                                                      container->pgsizes,
                                                      &err);
         if (ret) {
@@ -1022,7 +1022,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
             goto fail;
         }
         QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
-        memory_region_iommu_replay(giommu->iommu, &giommu->n);
+        memory_region_iommu_replay(giommu->iommu_mr, &giommu->n);
 
         return;
     }
@@ -1128,7 +1128,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
         VFIOGuestIOMMU *giommu;
 
         QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
-            if (MEMORY_REGION(giommu->iommu) == section->mr &&
+            if (MEMORY_REGION(giommu->iommu_mr) == section->mr &&
                 giommu->n.start == section->offset_within_region) {
                 memory_region_unregister_iommu_notifier(section->mr,
                                                         &giommu->n);
@@ -1393,11 +1393,11 @@ static int vfio_sync_dirty_bitmap(VFIOContainer *container,
         VFIOGuestIOMMU *giommu;
 
         QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
-            if (MEMORY_REGION(giommu->iommu) == section->mr &&
+            if (MEMORY_REGION(giommu->iommu_mr) == section->mr &&
                 giommu->n.start == section->offset_within_region) {
                 Int128 llend;
                 vfio_giommu_dirty_notifier gdn = { .giommu = giommu };
-                int idx = memory_region_iommu_attrs_to_index(giommu->iommu,
+                int idx = memory_region_iommu_attrs_to_index(giommu->iommu_mr,
                                                        MEMTXATTRS_UNSPECIFIED);
 
                 llend = int128_add(int128_make64(section->offset_within_region),
@@ -1410,7 +1410,7 @@ static int vfio_sync_dirty_bitmap(VFIOContainer *container,
                                     section->offset_within_region,
                                     int128_get64(llend),
                                     idx);
-                memory_region_iommu_replay(giommu->iommu, &gdn.n);
+                memory_region_iommu_replay(giommu->iommu_mr, &gdn.n);
                 break;
             }
         }
@@ -2246,7 +2246,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
 
         QLIST_FOREACH_SAFE(giommu, &container->giommu_list, giommu_next, tmp) {
             memory_region_unregister_iommu_notifier(
-                    MEMORY_REGION(giommu->iommu), &giommu->n);
+                    MEMORY_REGION(giommu->iommu_mr), &giommu->n);
             QLIST_REMOVE(giommu, giommu_next);
             g_free(giommu);
         }
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 8af11b0a76..e573f5a9f1 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -98,7 +98,7 @@ typedef struct VFIOContainer {
 
 typedef struct VFIOGuestIOMMU {
     VFIOContainer *container;
-    IOMMUMemoryRegion *iommu;
+    IOMMUMemoryRegion *iommu_mr;
     hwaddr iommu_offset;
     IOMMUNotifier n;
     QLIST_ENTRY(VFIOGuestIOMMU) giommu_next;
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-05-02 14:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-02  9:42 [Patch 0/3] vfio: Misc cleanups Yi Liu
2022-05-02  9:42 ` [Patch 1/3] hw/vfio/pci: fix vfio_pci_hot_reset_result trace point Yi Liu
2022-05-02 14:35   ` Alex Williamson
2022-05-02 14:43     ` Eric Auger
2022-05-02  9:42 ` [Patch 2/3] vfio/pci: Use vbasedev local variable in vfio_realize() Yi Liu
2022-05-02  9:42 ` [Patch 3/3] vfio/common: Rename VFIOGuestIOMMU::iommu into ::iommu_mr Yi Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).