diff mbox series

[13/16] vfio/pci: Make capability related functions return bool

Message ID 20240515082041.556571-14-zhenzhong.duan@intel.com (mailing list archive)
State New, archived
Headers show
Series VFIO: misc cleanups part2 | expand

Commit Message

Duan, Zhenzhong May 15, 2024, 8:20 a.m. UTC 
The functions operating on capability don't have a consistent return style.

Below functions are in bool-valued functions style:
vfio_msi_setup()
vfio_msix_setup()
vfio_add_std_cap()
vfio_add_capabilities()

Below two are integer-valued functions:
vfio_add_vendor_specific_cap()
vfio_setup_pcie_cap()

But the returned integer is only used for check succeed/failure.
Change them all to return bool so now all capability related
functions follow the coding standand in qapi/error.h to return
bool.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/pci.c | 77 ++++++++++++++++++++++++---------------------------
 1 file changed, 36 insertions(+), 41 deletions(-)

Comments

Cédric Le Goater May 21, 2024, 12:54 p.m. UTC | #1 
On 5/15/24 10:20, Zhenzhong Duan wrote:
> The functions operating on capability don't have a consistent return style.
> 
> Below functions are in bool-valued functions style:
> vfio_msi_setup()
> vfio_msix_setup()
> vfio_add_std_cap()
> vfio_add_capabilities()
> 
> Below two are integer-valued functions:
> vfio_add_vendor_specific_cap()
> vfio_setup_pcie_cap()
> 
> But the returned integer is only used for check succeed/failure.
> Change them all to return bool so now all capability related
> functions follow the coding standand in qapi/error.h to return
> bool.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


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

Thanks,

C.


> ---
>   hw/vfio/pci.c | 77 ++++++++++++++++++++++++---------------------------
>   1 file changed, 36 insertions(+), 41 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 1922593253..ecfbb9619f 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1339,7 +1339,7 @@ static void vfio_disable_interrupts(VFIOPCIDevice *vdev)
>       }
>   }
>   
> -static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
> +static bool vfio_msi_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>   {
>       uint16_t ctrl;
>       bool msi_64bit, msi_maskbit;
> @@ -1349,7 +1349,7 @@ static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>       if (pread(vdev->vbasedev.fd, &ctrl, sizeof(ctrl),
>                 vdev->config_offset + pos + PCI_CAP_FLAGS) != sizeof(ctrl)) {
>           error_setg_errno(errp, errno, "failed reading MSI PCI_CAP_FLAGS");
> -        return -errno;
> +        return false;
>       }
>       ctrl = le16_to_cpu(ctrl);
>   
> @@ -1362,14 +1362,14 @@ static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>       ret = msi_init(&vdev->pdev, pos, entries, msi_64bit, msi_maskbit, &err);
>       if (ret < 0) {
>           if (ret == -ENOTSUP) {
> -            return 0;
> +            return true;
>           }
>           error_propagate_prepend(errp, err, "msi_init failed: ");
> -        return ret;
> +        return false;
>       }
>       vdev->msi_cap_size = 0xa + (msi_maskbit ? 0xa : 0) + (msi_64bit ? 0x4 : 0);
>   
> -    return 0;
> +    return true;
>   }
>   
>   static void vfio_pci_fixup_msix_region(VFIOPCIDevice *vdev)
> @@ -1644,7 +1644,7 @@ static bool vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
>       return vfio_pci_relocate_msix(vdev, errp);
>   }
>   
> -static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
> +static bool vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>   {
>       int ret;
>       Error *err = NULL;
> @@ -1660,11 +1660,11 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>       if (ret < 0) {
>           if (ret == -ENOTSUP) {
>               warn_report_err(err);
> -            return 0;
> +            return true;
>           }
>   
>           error_propagate(errp, err);
> -        return ret;
> +        return false;
>       }
>   
>       /*
> @@ -1698,7 +1698,7 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>           memory_region_set_enabled(&vdev->pdev.msix_table_mmio, false);
>       }
>   
> -    return 0;
> +    return true;
>   }
>   
>   static void vfio_teardown_msi(VFIOPCIDevice *vdev)
> @@ -1977,8 +1977,8 @@ static void vfio_pci_disable_rp_atomics(VFIOPCIDevice *vdev)
>       }
>   }
>   
> -static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
> -                               Error **errp)
> +static bool vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
> +                                Error **errp)
>   {
>       uint16_t flags;
>       uint8_t type;
> @@ -1992,7 +1992,7 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
>   
>           error_setg(errp, "assignment of PCIe type 0x%x "
>                      "devices is not currently supported", type);
> -        return -EINVAL;
> +        return false;
>       }
>   
>       if (!pci_bus_is_express(pci_get_bus(&vdev->pdev))) {
> @@ -2025,7 +2025,7 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
>           }
>   
>           if (pci_bus_is_express(bus)) {
> -            return 0;
> +            return true;
>           }
>   
>       } else if (pci_bus_is_root(pci_get_bus(&vdev->pdev))) {
> @@ -2063,7 +2063,7 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
>                * Legacy endpoints don't belong on the root complex.  Windows
>                * seems to be happier with devices if we skip the capability.
>                */
> -            return 0;
> +            return true;
>           }
>   
>       } else {
> @@ -2099,12 +2099,12 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
>       pos = pci_add_capability(&vdev->pdev, PCI_CAP_ID_EXP, pos, size,
>                                errp);
>       if (pos < 0) {
> -        return pos;
> +        return false;
>       }
>   
>       vdev->pdev.exp.exp_cap = pos;
>   
> -    return pos;
> +    return true;
>   }
>   
>   static void vfio_check_pcie_flr(VFIOPCIDevice *vdev, uint8_t pos)
> @@ -2137,14 +2137,14 @@ static void vfio_check_af_flr(VFIOPCIDevice *vdev, uint8_t pos)
>       }
>   }
>   
> -static int vfio_add_vendor_specific_cap(VFIOPCIDevice *vdev, int pos,
> -                                        uint8_t size, Error **errp)
> +static bool vfio_add_vendor_specific_cap(VFIOPCIDevice *vdev, int pos,
> +                                         uint8_t size, Error **errp)
>   {
>       PCIDevice *pdev = &vdev->pdev;
>   
>       pos = pci_add_capability(pdev, PCI_CAP_ID_VNDR, pos, size, errp);
>       if (pos < 0) {
> -        return pos;
> +        return false;
>       }
>   
>       /*
> @@ -2156,15 +2156,15 @@ static int vfio_add_vendor_specific_cap(VFIOPCIDevice *vdev, int pos,
>           memset(pdev->cmask + pos + 3, 0, size - 3);
>       }
>   
> -    return pos;
> +    return true;
>   }
>   
> -static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
> +static bool vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
>   {
>       ERRP_GUARD();
>       PCIDevice *pdev = &vdev->pdev;
>       uint8_t cap_id, next, size;
> -    int ret;
> +    bool ret;
>   
>       cap_id = pdev->config[pos];
>       next = pdev->config[pos + PCI_CAP_LIST_NEXT];
> @@ -2185,9 +2185,8 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
>        * will be changed as we unwind the stack.
>        */
>       if (next) {
> -        ret = vfio_add_std_cap(vdev, next, errp);
> -        if (ret) {
> -            return ret;
> +        if (!vfio_add_std_cap(vdev, next, errp)) {
> +            return false;
>           }
>       } else {
>           /* Begin the rebuild, use QEMU emulated list bits */
> @@ -2197,7 +2196,7 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
>   
>           ret = vfio_add_virt_caps(vdev, errp);
>           if (ret) {
> -            return ret;
> +            return false;
>           }
>       }
>   
> @@ -2221,28 +2220,27 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
>       case PCI_CAP_ID_PM:
>           vfio_check_pm_reset(vdev, pos);
>           vdev->pm_cap = pos;
> -        ret = pci_add_capability(pdev, cap_id, pos, size, errp);
> +        ret = pci_add_capability(pdev, cap_id, pos, size, errp) >= 0;
>           break;
>       case PCI_CAP_ID_AF:
>           vfio_check_af_flr(vdev, pos);
> -        ret = pci_add_capability(pdev, cap_id, pos, size, errp);
> +        ret = pci_add_capability(pdev, cap_id, pos, size, errp) >= 0;
>           break;
>       case PCI_CAP_ID_VNDR:
>           ret = vfio_add_vendor_specific_cap(vdev, pos, size, errp);
>           break;
>       default:
> -        ret = pci_add_capability(pdev, cap_id, pos, size, errp);
> +        ret = pci_add_capability(pdev, cap_id, pos, size, errp) >= 0;
>           break;
>       }
>   
> -    if (ret < 0) {
> +    if (!ret) {
>           error_prepend(errp,
>                         "failed to add PCI capability 0x%x[0x%x]@0x%x: ",
>                         cap_id, size, pos);
> -        return ret;
>       }
>   
> -    return 0;
> +    return ret;
>   }
>   
>   static int vfio_setup_rebar_ecap(VFIOPCIDevice *vdev, uint16_t pos)
> @@ -2388,23 +2386,21 @@ static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
>       return;
>   }
>   
> -static int vfio_add_capabilities(VFIOPCIDevice *vdev, Error **errp)
> +static bool vfio_add_capabilities(VFIOPCIDevice *vdev, Error **errp)
>   {
>       PCIDevice *pdev = &vdev->pdev;
> -    int ret;
>   
>       if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST) ||
>           !pdev->config[PCI_CAPABILITY_LIST]) {
> -        return 0; /* Nothing to add */
> +        return true; /* Nothing to add */
>       }
>   
> -    ret = vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST], errp);
> -    if (ret) {
> -        return ret;
> +    if (!vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST], errp)) {
> +        return false;
>       }
>   
>       vfio_add_ext_cap(vdev);
> -    return 0;
> +    return true;
>   }
>   
>   void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
> @@ -3136,8 +3132,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>   
>       vfio_bars_register(vdev);
>   
> -    ret = vfio_add_capabilities(vdev, errp);
> -    if (ret) {
> +    if (!vfio_add_capabilities(vdev, errp)) {
>           goto out_teardown;
>       }
>
diff mbox series

Patch

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 1922593253..ecfbb9619f 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1339,7 +1339,7 @@  static void vfio_disable_interrupts(VFIOPCIDevice *vdev)
     }
 }
 
-static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
+static bool vfio_msi_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
 {
     uint16_t ctrl;
     bool msi_64bit, msi_maskbit;
@@ -1349,7 +1349,7 @@  static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
     if (pread(vdev->vbasedev.fd, &ctrl, sizeof(ctrl),
               vdev->config_offset + pos + PCI_CAP_FLAGS) != sizeof(ctrl)) {
         error_setg_errno(errp, errno, "failed reading MSI PCI_CAP_FLAGS");
-        return -errno;
+        return false;
     }
     ctrl = le16_to_cpu(ctrl);
 
@@ -1362,14 +1362,14 @@  static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
     ret = msi_init(&vdev->pdev, pos, entries, msi_64bit, msi_maskbit, &err);
     if (ret < 0) {
         if (ret == -ENOTSUP) {
-            return 0;
+            return true;
         }
         error_propagate_prepend(errp, err, "msi_init failed: ");
-        return ret;
+        return false;
     }
     vdev->msi_cap_size = 0xa + (msi_maskbit ? 0xa : 0) + (msi_64bit ? 0x4 : 0);
 
-    return 0;
+    return true;
 }
 
 static void vfio_pci_fixup_msix_region(VFIOPCIDevice *vdev)
@@ -1644,7 +1644,7 @@  static bool vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
     return vfio_pci_relocate_msix(vdev, errp);
 }
 
-static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
+static bool vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
 {
     int ret;
     Error *err = NULL;
@@ -1660,11 +1660,11 @@  static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
     if (ret < 0) {
         if (ret == -ENOTSUP) {
             warn_report_err(err);
-            return 0;
+            return true;
         }
 
         error_propagate(errp, err);
-        return ret;
+        return false;
     }
 
     /*
@@ -1698,7 +1698,7 @@  static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
         memory_region_set_enabled(&vdev->pdev.msix_table_mmio, false);
     }
 
-    return 0;
+    return true;
 }
 
 static void vfio_teardown_msi(VFIOPCIDevice *vdev)
@@ -1977,8 +1977,8 @@  static void vfio_pci_disable_rp_atomics(VFIOPCIDevice *vdev)
     }
 }
 
-static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
-                               Error **errp)
+static bool vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
+                                Error **errp)
 {
     uint16_t flags;
     uint8_t type;
@@ -1992,7 +1992,7 @@  static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
 
         error_setg(errp, "assignment of PCIe type 0x%x "
                    "devices is not currently supported", type);
-        return -EINVAL;
+        return false;
     }
 
     if (!pci_bus_is_express(pci_get_bus(&vdev->pdev))) {
@@ -2025,7 +2025,7 @@  static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
         }
 
         if (pci_bus_is_express(bus)) {
-            return 0;
+            return true;
         }
 
     } else if (pci_bus_is_root(pci_get_bus(&vdev->pdev))) {
@@ -2063,7 +2063,7 @@  static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
              * Legacy endpoints don't belong on the root complex.  Windows
              * seems to be happier with devices if we skip the capability.
              */
-            return 0;
+            return true;
         }
 
     } else {
@@ -2099,12 +2099,12 @@  static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
     pos = pci_add_capability(&vdev->pdev, PCI_CAP_ID_EXP, pos, size,
                              errp);
     if (pos < 0) {
-        return pos;
+        return false;
     }
 
     vdev->pdev.exp.exp_cap = pos;
 
-    return pos;
+    return true;
 }
 
 static void vfio_check_pcie_flr(VFIOPCIDevice *vdev, uint8_t pos)
@@ -2137,14 +2137,14 @@  static void vfio_check_af_flr(VFIOPCIDevice *vdev, uint8_t pos)
     }
 }
 
-static int vfio_add_vendor_specific_cap(VFIOPCIDevice *vdev, int pos,
-                                        uint8_t size, Error **errp)
+static bool vfio_add_vendor_specific_cap(VFIOPCIDevice *vdev, int pos,
+                                         uint8_t size, Error **errp)
 {
     PCIDevice *pdev = &vdev->pdev;
 
     pos = pci_add_capability(pdev, PCI_CAP_ID_VNDR, pos, size, errp);
     if (pos < 0) {
-        return pos;
+        return false;
     }
 
     /*
@@ -2156,15 +2156,15 @@  static int vfio_add_vendor_specific_cap(VFIOPCIDevice *vdev, int pos,
         memset(pdev->cmask + pos + 3, 0, size - 3);
     }
 
-    return pos;
+    return true;
 }
 
-static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
+static bool vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
 {
     ERRP_GUARD();
     PCIDevice *pdev = &vdev->pdev;
     uint8_t cap_id, next, size;
-    int ret;
+    bool ret;
 
     cap_id = pdev->config[pos];
     next = pdev->config[pos + PCI_CAP_LIST_NEXT];
@@ -2185,9 +2185,8 @@  static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
      * will be changed as we unwind the stack.
      */
     if (next) {
-        ret = vfio_add_std_cap(vdev, next, errp);
-        if (ret) {
-            return ret;
+        if (!vfio_add_std_cap(vdev, next, errp)) {
+            return false;
         }
     } else {
         /* Begin the rebuild, use QEMU emulated list bits */
@@ -2197,7 +2196,7 @@  static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
 
         ret = vfio_add_virt_caps(vdev, errp);
         if (ret) {
-            return ret;
+            return false;
         }
     }
 
@@ -2221,28 +2220,27 @@  static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
     case PCI_CAP_ID_PM:
         vfio_check_pm_reset(vdev, pos);
         vdev->pm_cap = pos;
-        ret = pci_add_capability(pdev, cap_id, pos, size, errp);
+        ret = pci_add_capability(pdev, cap_id, pos, size, errp) >= 0;
         break;
     case PCI_CAP_ID_AF:
         vfio_check_af_flr(vdev, pos);
-        ret = pci_add_capability(pdev, cap_id, pos, size, errp);
+        ret = pci_add_capability(pdev, cap_id, pos, size, errp) >= 0;
         break;
     case PCI_CAP_ID_VNDR:
         ret = vfio_add_vendor_specific_cap(vdev, pos, size, errp);
         break;
     default:
-        ret = pci_add_capability(pdev, cap_id, pos, size, errp);
+        ret = pci_add_capability(pdev, cap_id, pos, size, errp) >= 0;
         break;
     }
 
-    if (ret < 0) {
+    if (!ret) {
         error_prepend(errp,
                       "failed to add PCI capability 0x%x[0x%x]@0x%x: ",
                       cap_id, size, pos);
-        return ret;
     }
 
-    return 0;
+    return ret;
 }
 
 static int vfio_setup_rebar_ecap(VFIOPCIDevice *vdev, uint16_t pos)
@@ -2388,23 +2386,21 @@  static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
     return;
 }
 
-static int vfio_add_capabilities(VFIOPCIDevice *vdev, Error **errp)
+static bool vfio_add_capabilities(VFIOPCIDevice *vdev, Error **errp)
 {
     PCIDevice *pdev = &vdev->pdev;
-    int ret;
 
     if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST) ||
         !pdev->config[PCI_CAPABILITY_LIST]) {
-        return 0; /* Nothing to add */
+        return true; /* Nothing to add */
     }
 
-    ret = vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST], errp);
-    if (ret) {
-        return ret;
+    if (!vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST], errp)) {
+        return false;
     }
 
     vfio_add_ext_cap(vdev);
-    return 0;
+    return true;
 }
 
 void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
@@ -3136,8 +3132,7 @@  static void vfio_realize(PCIDevice *pdev, Error **errp)
 
     vfio_bars_register(vdev);
 
-    ret = vfio_add_capabilities(vdev, errp);
-    if (ret) {
+    if (!vfio_add_capabilities(vdev, errp)) {
         goto out_teardown;
     }