Add a helper similar to vfio_device_get_region_info() and use it
everywhere.
Replace a couple of needless allocations with stack variables.
As a side-effect, this fixes a minor error reporting issue in the call
from vfio_msix_early_setup().
Signed-off-by: John Levon <john.levon@nutanix.com>
---
hw/vfio/ap.c | 19 ++++++++++---------
hw/vfio/ccw.c | 20 +++++++++++---------
hw/vfio/device.c | 15 +++++++++++++++
hw/vfio/pci.c | 23 +++++++++++------------
hw/vfio/platform.c | 6 +++---
include/hw/vfio/vfio-device.h | 3 +++
6 files changed, 53 insertions(+), 33 deletions(-)
diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index 4af7379d4f..f311bca5b6 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -74,10 +74,10 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
unsigned int irq, Error **errp)
{
int fd;
- size_t argsz;
+ int ret;
IOHandler *fd_read;
EventNotifier *notifier;
- g_autofree struct vfio_irq_info *irq_info = NULL;
+ struct vfio_irq_info irq_info;
VFIODevice *vdev = &vapdev->vdev;
switch (irq) {
@@ -96,14 +96,15 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
return false;
}
- argsz = sizeof(*irq_info);
- irq_info = g_malloc0(argsz);
- irq_info->index = irq;
- irq_info->argsz = argsz;
+ ret = vfio_device_get_irq_info(vdev, irq, &irq_info);
+
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "vfio: Error getting irq info");
+ return false;
+ }
- if (ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO,
- irq_info) < 0 || irq_info->count < 1) {
- error_setg_errno(errp, errno, "vfio: Error getting irq info");
+ if (irq_info.count < 1) {
+ error_setg_errno(errp, EINVAL, "vfio: Error getting irq info, count=0");
return false;
}
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 98aa0000da..dac8769925 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -376,8 +376,8 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
Error **errp)
{
VFIODevice *vdev = &vcdev->vdev;
- g_autofree struct vfio_irq_info *irq_info = NULL;
- size_t argsz;
+ struct vfio_irq_info irq_info;
+ int ret;
int fd;
EventNotifier *notifier;
IOHandler *fd_read;
@@ -406,13 +406,15 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
return false;
}
- argsz = sizeof(*irq_info);
- irq_info = g_malloc0(argsz);
- irq_info->index = irq;
- irq_info->argsz = argsz;
- if (ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO,
- irq_info) < 0 || irq_info->count < 1) {
- error_setg_errno(errp, errno, "vfio: Error getting irq info");
+ ret = vfio_device_get_irq_info(vdev, irq, &irq_info);
+
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "vfio: Error getting irq info");
+ return false;
+ }
+
+ if (irq_info.count < 1) {
+ error_setg_errno(errp, EINVAL, "vfio: Error getting irq info, count=0");
return false;
}
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index b9473878fc..2966171118 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -185,6 +185,21 @@ bool vfio_device_irq_set_signaling(VFIODevice *vbasedev, int index, int subindex
return false;
}
+int vfio_device_get_irq_info(VFIODevice *vbasedev, int index,
+ struct vfio_irq_info *info)
+{
+ int ret;
+
+ memset(info, 0, sizeof(*info));
+
+ info->argsz = sizeof(*info);
+ info->index = index;
+
+ ret = ioctl(vbasedev->fd, VFIO_DEVICE_GET_IRQ_INFO, info);
+
+ return ret < 0 ? -errno : ret;
+}
+
int vfio_device_get_region_info(VFIODevice *vbasedev, int index,
struct vfio_region_info **info)
{
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 090b2f2ef0..ac53c43f2b 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1555,8 +1555,7 @@ static bool vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
uint16_t ctrl;
uint32_t table, pba;
int ret, fd = vdev->vbasedev.fd;
- struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info),
- .index = VFIO_PCI_MSIX_IRQ_INDEX };
+ struct vfio_irq_info irq_info;
VFIOMSIXInfo *msix;
pos = pci_find_capability(&vdev->pdev, PCI_CAP_ID_MSIX);
@@ -1593,7 +1592,8 @@ static bool vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
msix->pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
msix->entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
- ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info);
+ ret = vfio_device_get_irq_info(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX,
+ &irq_info);
if (ret < 0) {
error_setg_errno(errp, -ret, "failed to get MSI-X irq info");
g_free(msix);
@@ -2737,7 +2737,7 @@ static bool vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
{
VFIODevice *vbasedev = &vdev->vbasedev;
g_autofree struct vfio_region_info *reg_info = NULL;
- struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info) };
+ struct vfio_irq_info irq_info;
int i, ret = -1;
/* Sanity check device */
@@ -2798,12 +2798,10 @@ static bool vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
}
}
- irq_info.index = VFIO_PCI_ERR_IRQ_INDEX;
-
- ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info);
+ ret = vfio_device_get_irq_info(vbasedev, VFIO_PCI_ERR_IRQ_INDEX, &irq_info);
if (ret) {
/* This can fail for an old kernel or legacy PCI dev */
- trace_vfio_populate_device_get_irq_info_failure(strerror(errno));
+ trace_vfio_populate_device_get_irq_info_failure(strerror(-ret));
} else if (irq_info.count == 1) {
vdev->pci_aer = true;
} else {
@@ -2912,17 +2910,18 @@ static void vfio_req_notifier_handler(void *opaque)
static void vfio_register_req_notifier(VFIOPCIDevice *vdev)
{
- struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info),
- .index = VFIO_PCI_REQ_IRQ_INDEX };
+ struct vfio_irq_info irq_info;
Error *err = NULL;
int32_t fd;
+ int ret;
if (!(vdev->features & VFIO_FEATURE_ENABLE_REQ)) {
return;
}
- if (ioctl(vdev->vbasedev.fd,
- VFIO_DEVICE_GET_IRQ_INFO, &irq_info) < 0 || irq_info.count < 1) {
+ ret = vfio_device_get_irq_info(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX,
+ &irq_info);
+ if (ret < 0 || irq_info.count < 1) {
return;
}
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 877d69b7aa..fd176c18a4 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -475,10 +475,10 @@ static bool vfio_populate_device(VFIODevice *vbasedev, Error **errp)
QSIMPLEQ_INIT(&vdev->pending_intp_queue);
for (i = 0; i < vbasedev->num_irqs; i++) {
- struct vfio_irq_info irq = { .argsz = sizeof(irq) };
+ struct vfio_irq_info irq;
+
+ ret = vfio_device_get_irq_info(vbasedev, i, &irq);
- irq.index = i;
- ret = ioctl(vbasedev->fd, VFIO_DEVICE_GET_IRQ_INFO, &irq);
if (ret) {
error_setg_errno(errp, -ret, "failed to get device irq info");
goto irq_err;
diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
index 3563a82ede..9522a09c48 100644
--- a/include/hw/vfio/vfio-device.h
+++ b/include/hw/vfio/vfio-device.h
@@ -144,6 +144,9 @@ int vfio_device_get_region_info(VFIODevice *vbasedev, int index,
int vfio_device_get_region_info_type(VFIODevice *vbasedev, uint32_t type,
uint32_t subtype, struct vfio_region_info **info);
bool vfio_device_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_type);
+
+int vfio_device_get_irq_info(VFIODevice *vbasedev, int index,
+ struct vfio_irq_info *info);
#endif
/* Returns 0 on success, or a negative errno. */
--
2.34.1On 4/9/25 15:48, John Levon wrote:
> Add a helper similar to vfio_device_get_region_info() and use it
> everywhere.
>
> Replace a couple of needless allocations with stack variables.
>
> As a side-effect, this fixes a minor error reporting issue in the call
> from vfio_msix_early_setup().
>
> Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Some comments below,
> ---> hw/vfio/ap.c | 19 ++++++++++---------
> hw/vfio/ccw.c | 20 +++++++++++---------
> hw/vfio/device.c | 15 +++++++++++++++
> hw/vfio/pci.c | 23 +++++++++++------------
> hw/vfio/platform.c | 6 +++---
> include/hw/vfio/vfio-device.h | 3 +++
> 6 files changed, 53 insertions(+), 33 deletions(-)
>
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index 4af7379d4f..f311bca5b6 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -74,10 +74,10 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
> unsigned int irq, Error **errp)
> {
> int fd;
> - size_t argsz;
> + int ret;
> IOHandler *fd_read;
> EventNotifier *notifier;
> - g_autofree struct vfio_irq_info *irq_info = NULL;
> + struct vfio_irq_info irq_info;
> VFIODevice *vdev = &vapdev->vdev;
>
> switch (irq) {
> @@ -96,14 +96,15 @@ static bool vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
> return false;
> }
>
> - argsz = sizeof(*irq_info);
> - irq_info = g_malloc0(argsz);
> - irq_info->index = irq;
> - irq_info->argsz = argsz;
> + ret = vfio_device_get_irq_info(vdev, irq, &irq_info);
> +
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "vfio: Error getting irq info");
> + return false;
> + }
>
> - if (ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO,
> - irq_info) < 0 || irq_info->count < 1) {
> - error_setg_errno(errp, errno, "vfio: Error getting irq info");
> + if (irq_info.count < 1) {
> + error_setg_errno(errp, EINVAL, "vfio: Error getting irq info, count=0");
I am not sure using error_setg_errno() is interesting in that case. May be simply
use error_setg(). Same below.
Thanks,
C.
> return false;
> }
>
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 98aa0000da..dac8769925 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -376,8 +376,8 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
> Error **errp)
> {
> VFIODevice *vdev = &vcdev->vdev;
> - g_autofree struct vfio_irq_info *irq_info = NULL;
> - size_t argsz;
> + struct vfio_irq_info irq_info;
> + int ret;
> int fd;
> EventNotifier *notifier;
> IOHandler *fd_read;
> @@ -406,13 +406,15 @@ static bool vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev,
> return false;
> }
>
> - argsz = sizeof(*irq_info);
> - irq_info = g_malloc0(argsz);
> - irq_info->index = irq;
> - irq_info->argsz = argsz;
> - if (ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO,
> - irq_info) < 0 || irq_info->count < 1) {
> - error_setg_errno(errp, errno, "vfio: Error getting irq info");
> + ret = vfio_device_get_irq_info(vdev, irq, &irq_info);
> +
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "vfio: Error getting irq info");
> + return false;
> + }
> +
> + if (irq_info.count < 1) {
> + error_setg_errno(errp, EINVAL, "vfio: Error getting irq info, count=0");
> return false;
> }
>
> diff --git a/hw/vfio/device.c b/hw/vfio/device.c
> index b9473878fc..2966171118 100644
> --- a/hw/vfio/device.c
> +++ b/hw/vfio/device.c
> @@ -185,6 +185,21 @@ bool vfio_device_irq_set_signaling(VFIODevice *vbasedev, int index, int subindex
> return false;
> }
>
> +int vfio_device_get_irq_info(VFIODevice *vbasedev, int index,
> + struct vfio_irq_info *info)
> +{
> + int ret;
> +
> + memset(info, 0, sizeof(*info));
> +
> + info->argsz = sizeof(*info);
> + info->index = index;
> +
> + ret = ioctl(vbasedev->fd, VFIO_DEVICE_GET_IRQ_INFO, info);
> +
> + return ret < 0 ? -errno : ret;
> +}
> +
> int vfio_device_get_region_info(VFIODevice *vbasedev, int index,
> struct vfio_region_info **info)
> {
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 090b2f2ef0..ac53c43f2b 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1555,8 +1555,7 @@ static bool vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
> uint16_t ctrl;
> uint32_t table, pba;
> int ret, fd = vdev->vbasedev.fd;
> - struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info),
> - .index = VFIO_PCI_MSIX_IRQ_INDEX };
> + struct vfio_irq_info irq_info;
> VFIOMSIXInfo *msix;
>
> pos = pci_find_capability(&vdev->pdev, PCI_CAP_ID_MSIX);
> @@ -1593,7 +1592,8 @@ static bool vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
> msix->pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
> msix->entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
>
> - ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info);
> + ret = vfio_device_get_irq_info(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX,
> + &irq_info);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "failed to get MSI-X irq info");
> g_free(msix);
> @@ -2737,7 +2737,7 @@ static bool vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
> {
> VFIODevice *vbasedev = &vdev->vbasedev;
> g_autofree struct vfio_region_info *reg_info = NULL;
> - struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info) };
> + struct vfio_irq_info irq_info;
> int i, ret = -1;
>
> /* Sanity check device */
> @@ -2798,12 +2798,10 @@ static bool vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
> }
> }
>
> - irq_info.index = VFIO_PCI_ERR_IRQ_INDEX;
> -
> - ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info);
> + ret = vfio_device_get_irq_info(vbasedev, VFIO_PCI_ERR_IRQ_INDEX, &irq_info);
> if (ret) {
> /* This can fail for an old kernel or legacy PCI dev */
> - trace_vfio_populate_device_get_irq_info_failure(strerror(errno));
> + trace_vfio_populate_device_get_irq_info_failure(strerror(-ret));
> } else if (irq_info.count == 1) {
> vdev->pci_aer = true;
> } else {
> @@ -2912,17 +2910,18 @@ static void vfio_req_notifier_handler(void *opaque)
>
> static void vfio_register_req_notifier(VFIOPCIDevice *vdev)
> {
> - struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info),
> - .index = VFIO_PCI_REQ_IRQ_INDEX };
> + struct vfio_irq_info irq_info;
> Error *err = NULL;
> int32_t fd;
> + int ret;
>
> if (!(vdev->features & VFIO_FEATURE_ENABLE_REQ)) {
> return;
> }
>
> - if (ioctl(vdev->vbasedev.fd,
> - VFIO_DEVICE_GET_IRQ_INFO, &irq_info) < 0 || irq_info.count < 1) {
> + ret = vfio_device_get_irq_info(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX,
> + &irq_info);
> + if (ret < 0 || irq_info.count < 1) {
> return;
> }
>
> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index 877d69b7aa..fd176c18a4 100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -475,10 +475,10 @@ static bool vfio_populate_device(VFIODevice *vbasedev, Error **errp)
> QSIMPLEQ_INIT(&vdev->pending_intp_queue);
>
> for (i = 0; i < vbasedev->num_irqs; i++) {
> - struct vfio_irq_info irq = { .argsz = sizeof(irq) };
> + struct vfio_irq_info irq;
> +
> + ret = vfio_device_get_irq_info(vbasedev, i, &irq);
>
> - irq.index = i;
> - ret = ioctl(vbasedev->fd, VFIO_DEVICE_GET_IRQ_INFO, &irq);
> if (ret) {
> error_setg_errno(errp, -ret, "failed to get device irq info");
> goto irq_err;
> diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
> index 3563a82ede..9522a09c48 100644
> --- a/include/hw/vfio/vfio-device.h
> +++ b/include/hw/vfio/vfio-device.h
> @@ -144,6 +144,9 @@ int vfio_device_get_region_info(VFIODevice *vbasedev, int index,
> int vfio_device_get_region_info_type(VFIODevice *vbasedev, uint32_t type,
> uint32_t subtype, struct vfio_region_info **info);
> bool vfio_device_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_type);
> +
> +int vfio_device_get_irq_info(VFIODevice *vbasedev, int index,
> + struct vfio_irq_info *info);
> #endif
>
> /* Returns 0 on success, or a negative errno. */
© 2016 - 2025 Red Hat, Inc.