pci: Make capability related functions return bool

Cédric Le Goater Tue, 21 May 2024 05:55:00 -0700

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.d...@intel.com>



Reviewed-by: Cédric Le Goater <c...@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;
      }

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

Reply via email to