[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 1/3] vfio: cleanup vfio_get_device error path, remov
From: |
Paolo Bonzini |
Subject: |
[Qemu-devel] [PATCH 1/3] vfio: cleanup vfio_get_device error path, remove vfio_populate_device callback |
Date: |
Wed, 4 Feb 2015 13:11:08 +0100 |
With the next patch vfio_put_base_device will be called unconditionally at
instance_finalize time, which will mean calling it twice if vfio_populate_device
fails. This works, but it is slightly harder to follow.
Change vfio_get_device to not touch the vbasedev struct until it will
definitely succeed, moving the vfio_populate_device call back to vfio-pci.
This way, vfio_put_base_device will only be called once and only on
non-error paths.
Cc: Alex Williamson <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
---
hw/vfio/common.c | 31 ++++++++++++-------------------
hw/vfio/pci.c | 11 +++++++----
include/hw/vfio/vfio-common.h | 1 -
3 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index cf483ff..242b71d 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -867,27 +867,28 @@ int vfio_get_device(VFIOGroup *group, const char *name,
VFIODevice *vbasedev)
{
struct vfio_device_info dev_info = { .argsz = sizeof(dev_info) };
- int ret;
+ int ret, fd;
- ret = ioctl(group->fd, VFIO_GROUP_GET_DEVICE_FD, name);
- if (ret < 0) {
+ fd = ioctl(group->fd, VFIO_GROUP_GET_DEVICE_FD, name);
+ if (fd < 0) {
error_report("vfio: error getting device %s from group %d: %m",
name, group->groupid);
error_printf("Verify all devices in group %d are bound to vfio-<bus> "
"or pci-stub and not already in use\n", group->groupid);
- return ret;
+ return fd;
}
- vbasedev->fd = ret;
- vbasedev->group = group;
- QLIST_INSERT_HEAD(&group->device_list, vbasedev, next);
-
- ret = ioctl(vbasedev->fd, VFIO_DEVICE_GET_INFO, &dev_info);
+ ret = ioctl(fd, VFIO_DEVICE_GET_INFO, &dev_info);
if (ret) {
error_report("vfio: error getting device info: %m");
- goto error;
+ close(fd);
+ return ret;
}
+ vbasedev->fd = fd;
+ 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;
@@ -896,20 +897,12 @@ int vfio_get_device(VFIOGroup *group, const char *name,
dev_info.num_irqs);
vbasedev->reset_works = !!(dev_info.flags & VFIO_DEVICE_FLAGS_RESET);
-
- ret = vbasedev->ops->vfio_populate_device(vbasedev);
-
-error:
- if (ret) {
- vfio_put_base_device(vbasedev);
- }
- return ret;
+ return 0;
}
void vfio_put_base_device(VFIODevice *vbasedev)
{
QLIST_REMOVE(vbasedev, next);
- vbasedev->group = NULL;
trace_vfio_put_base_device(vbasedev->fd);
close(vbasedev->fd);
}
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 014a92c..2f5f718 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -195,7 +195,6 @@ static uint32_t vfio_pci_read_config(PCIDevice *pdev,
uint32_t addr, int len);
static void vfio_pci_write_config(PCIDevice *pdev, uint32_t addr,
uint32_t val, int len);
static void vfio_mmap_set_enabled(VFIOPCIDevice *vdev, bool enabled);
-static int vfio_populate_device(VFIODevice *vbasedev);
/*
* Disabling BAR mmaping can be slow, but toggling it around INTx can
@@ -2934,12 +2933,11 @@ static VFIODeviceOps vfio_pci_ops = {
.vfio_compute_needs_reset = vfio_pci_compute_needs_reset,
.vfio_hot_reset_multi = vfio_pci_hot_reset_multi,
.vfio_eoi = vfio_eoi,
- .vfio_populate_device = vfio_populate_device,
};
-static int vfio_populate_device(VFIODevice *vbasedev)
+static int vfio_pci_populate_device(VFIOPCIDevice *vdev)
{
- VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
+ VFIODevice *vbasedev = &vdev->vbasedev;
struct vfio_region_info reg_info = { .argsz = sizeof(reg_info) };
struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info) };
int i, ret = -1;
@@ -3247,6 +3245,11 @@ static int vfio_initfn(PCIDevice *pdev)
return ret;
}
+ ret = vfio_pci_populate_device(vdev);
+ if (ret) {
+ return ret;
+ }
+
/* Get a copy of config space */
ret = pread(vdev->vbasedev.fd, vdev->pdev.config,
MIN(pci_config_size(&vdev->pdev), vdev->config_size),
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 1d5bfe8..5f3679b 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -112,7 +112,6 @@ struct VFIODeviceOps {
void (*vfio_compute_needs_reset)(VFIODevice *vdev);
int (*vfio_hot_reset_multi)(VFIODevice *vdev);
void (*vfio_eoi)(VFIODevice *vdev);
- int (*vfio_populate_device)(VFIODevice *vdev);
};
typedef struct VFIOGroup {
--
1.8.3.1