Commit 217e9fdc authored by Paolo Bonzini's avatar Paolo Bonzini Committed by Alex Williamson
Browse files

vfio: cleanup vfio_get_device error path, remove vfio_populate_device callback 



Now that vfio_put_base_device is called unconditionally at instance_finalize
time, it can be called 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.

Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
Signed-off-by: default avatarAlex Williamson <alex.williamson@redhat.com>
parent 6e48e8f9

hw/vfio/common.c

+12 −18
Original line number Diff line number Diff line
@@ -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,14 +897,7 @@ 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)

hw/vfio/pci.c

+7 −4
Original line number Diff line number Diff line
@@ -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_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;
@@ -3248,6 +3246,11 @@ static int vfio_initfn(PCIDevice *pdev) 
        return ret;

    }



    ret = vfio_populate_device(vdev);

    if (ret) {

        goto out_put;

    }



    /* Get a copy of config space */

    ret = pread(vdev->vbasedev.fd, vdev->pdev.config,

                MIN(pci_config_size(&vdev->pdev), vdev->config_size),

include/hw/vfio/vfio-common.h

+0 −1
Original line number Diff line number Diff line
@@ -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 {