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(-)
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;
}
--
2.34.1On 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;
> }
>
© 2016 - 2026 Red Hat, Inc.