diff mbox series

[RFC,2/4] vfio: Implement a common device info helper

Message ID 20230519215739.402729-3-alex.williamson@redhat.com (mailing list archive)
State New, archived
Headers show
Series vfio/pci: Atomic Ops completer support | expand

Commit Message

Alex Williamson May 19, 2023, 9:57 p.m. UTC 
A common helper implementing the realloc algorithm for handling
capabilities.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/s390x/s390-pci-vfio.c      | 37 ++++------------------------
 hw/vfio/common.c              | 46 ++++++++++++++++++++++++++---------
 include/hw/vfio/vfio-common.h |  1 +
 3 files changed, 41 insertions(+), 43 deletions(-)

Comments

Philippe Mathieu-Daudé May 20, 2023, 4:33 a.m. UTC | #1 
On 19/5/23 23:57, Alex Williamson wrote:
> A common helper implementing the realloc algorithm for handling
> capabilities.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>   hw/s390x/s390-pci-vfio.c      | 37 ++++------------------------
>   hw/vfio/common.c              | 46 ++++++++++++++++++++++++++---------
>   include/hw/vfio/vfio-common.h |  1 +
>   3 files changed, 41 insertions(+), 43 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Cédric Le Goater May 22, 2023, 7:19 a.m. UTC | #2 
On 5/19/23 23:57, Alex Williamson wrote:
> A common helper implementing the realloc algorithm for handling
> capabilities.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

Reviewed-by: Cédric Le Goater <clg@redhat.com>

This is a good independent clean up that could be merged now.

Thanks,

C.

> ---
>   hw/s390x/s390-pci-vfio.c      | 37 ++++------------------------
>   hw/vfio/common.c              | 46 ++++++++++++++++++++++++++---------
>   include/hw/vfio/vfio-common.h |  1 +
>   3 files changed, 41 insertions(+), 43 deletions(-)
> 
> diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
> index f51190d4662f..66e8ec3bdef9 100644
> --- a/hw/s390x/s390-pci-vfio.c
> +++ b/hw/s390x/s390-pci-vfio.c
> @@ -289,38 +289,11 @@ static void s390_pci_read_pfip(S390PCIBusDevice *pbdev,
>       memcpy(pbdev->zpci_fn.pfip, cap->pfip, CLP_PFIP_NR_SEGMENTS);
>   }
>   
> -static struct vfio_device_info *get_device_info(S390PCIBusDevice *pbdev,
> -                                                uint32_t argsz)
> +static struct vfio_device_info *get_device_info(S390PCIBusDevice *pbdev);
>   {
> -    struct vfio_device_info *info = g_malloc0(argsz);
> -    VFIOPCIDevice *vfio_pci;
> -    int fd;
> +    VFIOPCIDevice *vfio_pci = container_of(pbdev->pdev, VFIOPCIDevice, pdev);
>   
> -    vfio_pci = container_of(pbdev->pdev, VFIOPCIDevice, pdev);
> -    fd = vfio_pci->vbasedev.fd;
> -
> -    /*
> -     * If the specified argsz is not large enough to contain all capabilities
> -     * it will be updated upon return from the ioctl.  Retry until we have
> -     * a big enough buffer to hold the entire capability chain.  On error,
> -     * just exit and rely on CLP defaults.
> -     */
> -retry:
> -    info->argsz = argsz;
> -
> -    if (ioctl(fd, VFIO_DEVICE_GET_INFO, info)) {
> -        trace_s390_pci_clp_dev_info(vfio_pci->vbasedev.name);
> -        g_free(info);
> -        return NULL;
> -    }
> -
> -    if (info->argsz > argsz) {
> -        argsz = info->argsz;
> -        info = g_realloc(info, argsz);
> -        goto retry;
> -    }
> -
> -    return info;
> +    return vfio_get_device_info(vfio_pci->vbasedev.fd);
>   }
>   
>   /*
> @@ -335,7 +308,7 @@ bool s390_pci_get_host_fh(S390PCIBusDevice *pbdev, uint32_t *fh)
>   
>       assert(fh);
>   
> -    info = get_device_info(pbdev, sizeof(*info));
> +    info = get_device_info(pbdev);
>       if (!info) {
>           return false;
>       }
> @@ -356,7 +329,7 @@ void s390_pci_get_clp_info(S390PCIBusDevice *pbdev)
>   {
>       g_autofree struct vfio_device_info *info = NULL;
>   
> -    info = get_device_info(pbdev, sizeof(*info));
> +    info = get_device_info(pbdev);
>       if (!info) {
>           return;
>       }
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 78358ede2764..ed142296e9fe 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -2843,11 +2843,35 @@ void vfio_put_group(VFIOGroup *group)
>       }
>   }
>   
> +struct vfio_device_info *vfio_get_device_info(int fd)
> +{
> +    struct vfio_device_info *info;
> +    uint32_t argsz = sizeof(*info);
> +
> +    info = g_malloc0(argsz);
> +
> +retry:
> +    info->argsz = argsz;
> +
> +    if (ioctl(fd, VFIO_DEVICE_GET_INFO, info)) {
> +        g_free(info);
> +        return NULL;
> +    }
> +
> +    if (info->argsz > argsz) {
> +        argsz = info->argsz;
> +        info = g_realloc(info, argsz);
> +        goto retry;
> +    }
> +
> +    return info;
> +}
> +
>   int vfio_get_device(VFIOGroup *group, const char *name,
>                       VFIODevice *vbasedev, Error **errp)
>   {
> -    struct vfio_device_info dev_info = { .argsz = sizeof(dev_info) };
> -    int ret, fd;
> +    g_autofree struct vfio_device_info *info = NULL;
> +    int fd;
>   
>       fd = ioctl(group->fd, VFIO_GROUP_GET_DEVICE_FD, name);
>       if (fd < 0) {
> @@ -2859,11 +2883,11 @@ int vfio_get_device(VFIOGroup *group, const char *name,
>           return fd;
>       }
>   
> -    ret = ioctl(fd, VFIO_DEVICE_GET_INFO, &dev_info);
> -    if (ret) {
> +    info = vfio_get_device_info(fd);
> +    if (!info) {
>           error_setg_errno(errp, errno, "error getting device info");
>           close(fd);
> -        return ret;
> +        return -1;
>       }
>   
>       /*
> @@ -2891,14 +2915,14 @@ int vfio_get_device(VFIOGroup *group, const char *name,
>       vbasedev->group = group;
>       QLIST_INSERT_HEAD(&group->device_list, vbasedev, next);
>   
> -    vbasedev->num_irqs = dev_info.num_irqs;
> -    vbasedev->num_regions = dev_info.num_regions;
> -    vbasedev->flags = dev_info.flags;
> +    vbasedev->num_irqs = info->num_irqs;
> +    vbasedev->num_regions = info->num_regions;
> +    vbasedev->flags = info->flags;
> +
> +    trace_vfio_get_device(name, info->flags, info->num_regions, info->num_irqs);
>   
> -    trace_vfio_get_device(name, dev_info.flags, dev_info.num_regions,
> -                          dev_info.num_irqs);
> +    vbasedev->reset_works = !!(info->flags & VFIO_DEVICE_FLAGS_RESET);
>   
> -    vbasedev->reset_works = !!(dev_info.flags & VFIO_DEVICE_FLAGS_RESET);
>       return 0;
>   }
>   
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index eed244f25f34..a8dcda592c08 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -212,6 +212,7 @@ void vfio_region_finalize(VFIORegion *region);
>   void vfio_reset_handler(void *opaque);
>   VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp);
>   void vfio_put_group(VFIOGroup *group);
> +struct vfio_device_info *vfio_get_device_info(int fd);
>   int vfio_get_device(VFIOGroup *group, const char *name,
>                       VFIODevice *vbasedev, Error **errp);
>
diff mbox series

Patch

diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index f51190d4662f..66e8ec3bdef9 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -289,38 +289,11 @@  static void s390_pci_read_pfip(S390PCIBusDevice *pbdev,
     memcpy(pbdev->zpci_fn.pfip, cap->pfip, CLP_PFIP_NR_SEGMENTS);
 }
 
-static struct vfio_device_info *get_device_info(S390PCIBusDevice *pbdev,
-                                                uint32_t argsz)
+static struct vfio_device_info *get_device_info(S390PCIBusDevice *pbdev);
 {
-    struct vfio_device_info *info = g_malloc0(argsz);
-    VFIOPCIDevice *vfio_pci;
-    int fd;
+    VFIOPCIDevice *vfio_pci = container_of(pbdev->pdev, VFIOPCIDevice, pdev);
 
-    vfio_pci = container_of(pbdev->pdev, VFIOPCIDevice, pdev);
-    fd = vfio_pci->vbasedev.fd;
-
-    /*
-     * If the specified argsz is not large enough to contain all capabilities
-     * it will be updated upon return from the ioctl.  Retry until we have
-     * a big enough buffer to hold the entire capability chain.  On error,
-     * just exit and rely on CLP defaults.
-     */
-retry:
-    info->argsz = argsz;
-
-    if (ioctl(fd, VFIO_DEVICE_GET_INFO, info)) {
-        trace_s390_pci_clp_dev_info(vfio_pci->vbasedev.name);
-        g_free(info);
-        return NULL;
-    }
-
-    if (info->argsz > argsz) {
-        argsz = info->argsz;
-        info = g_realloc(info, argsz);
-        goto retry;
-    }
-
-    return info;
+    return vfio_get_device_info(vfio_pci->vbasedev.fd);
 }
 
 /*
@@ -335,7 +308,7 @@  bool s390_pci_get_host_fh(S390PCIBusDevice *pbdev, uint32_t *fh)
 
     assert(fh);
 
-    info = get_device_info(pbdev, sizeof(*info));
+    info = get_device_info(pbdev);
     if (!info) {
         return false;
     }
@@ -356,7 +329,7 @@  void s390_pci_get_clp_info(S390PCIBusDevice *pbdev)
 {
     g_autofree struct vfio_device_info *info = NULL;
 
-    info = get_device_info(pbdev, sizeof(*info));
+    info = get_device_info(pbdev);
     if (!info) {
         return;
     }
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 78358ede2764..ed142296e9fe 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -2843,11 +2843,35 @@  void vfio_put_group(VFIOGroup *group)
     }
 }
 
+struct vfio_device_info *vfio_get_device_info(int fd)
+{
+    struct vfio_device_info *info;
+    uint32_t argsz = sizeof(*info);
+
+    info = g_malloc0(argsz);
+
+retry:
+    info->argsz = argsz;
+
+    if (ioctl(fd, VFIO_DEVICE_GET_INFO, info)) {
+        g_free(info);
+        return NULL;
+    }
+
+    if (info->argsz > argsz) {
+        argsz = info->argsz;
+        info = g_realloc(info, argsz);
+        goto retry;
+    }
+
+    return info;
+}
+
 int vfio_get_device(VFIOGroup *group, const char *name,
                     VFIODevice *vbasedev, Error **errp)
 {
-    struct vfio_device_info dev_info = { .argsz = sizeof(dev_info) };
-    int ret, fd;
+    g_autofree struct vfio_device_info *info = NULL;
+    int fd;
 
     fd = ioctl(group->fd, VFIO_GROUP_GET_DEVICE_FD, name);
     if (fd < 0) {
@@ -2859,11 +2883,11 @@  int vfio_get_device(VFIOGroup *group, const char *name,
         return fd;
     }
 
-    ret = ioctl(fd, VFIO_DEVICE_GET_INFO, &dev_info);
-    if (ret) {
+    info = vfio_get_device_info(fd);
+    if (!info) {
         error_setg_errno(errp, errno, "error getting device info");
         close(fd);
-        return ret;
+        return -1;
     }
 
     /*
@@ -2891,14 +2915,14 @@  int vfio_get_device(VFIOGroup *group, const char *name,
     vbasedev->group = group;
     QLIST_INSERT_HEAD(&group->device_list, vbasedev, next);
 
-    vbasedev->num_irqs = dev_info.num_irqs;
-    vbasedev->num_regions = dev_info.num_regions;
-    vbasedev->flags = dev_info.flags;
+    vbasedev->num_irqs = info->num_irqs;
+    vbasedev->num_regions = info->num_regions;
+    vbasedev->flags = info->flags;
+
+    trace_vfio_get_device(name, info->flags, info->num_regions, info->num_irqs);
 
-    trace_vfio_get_device(name, dev_info.flags, dev_info.num_regions,
-                          dev_info.num_irqs);
+    vbasedev->reset_works = !!(info->flags & VFIO_DEVICE_FLAGS_RESET);
 
-    vbasedev->reset_works = !!(dev_info.flags & VFIO_DEVICE_FLAGS_RESET);
     return 0;
 }
 
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index eed244f25f34..a8dcda592c08 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -212,6 +212,7 @@  void vfio_region_finalize(VFIORegion *region);
 void vfio_reset_handler(void *opaque);
 VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp);
 void vfio_put_group(VFIOGroup *group);
+struct vfio_device_info *vfio_get_device_info(int fd);
 int vfio_get_device(VFIOGroup *group, const char *name,
                     VFIODevice *vbasedev, Error **errp);