| Message ID | 20190614092705.11025-1-cohuck@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | vfio-ccw: use vfio_set_irq_signaling | expand |
Hi Connie, On 6/14/19 11:27 AM, Cornelia Huck wrote: > Use the new helper. > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks Eric > --- > hw/vfio/ccw.c | 68 +++++++++++---------------------------------------- > 1 file changed, 14 insertions(+), 54 deletions(-) > > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > index 03a2becb3ec9..3dc08721a3db 100644 > --- a/hw/vfio/ccw.c > +++ b/hw/vfio/ccw.c > @@ -197,10 +197,7 @@ read_err: > static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) > { > VFIODevice *vdev = &vcdev->vdev; > - struct vfio_irq_info *irq_info; > - struct vfio_irq_set *irq_set; > - size_t argsz; > - int32_t *pfd; > + int fd; > > if (vdev->num_irqs < VFIO_CCW_IO_IRQ_INDEX + 1) { > error_setg(errp, "vfio: unexpected number of io irqs %u", > @@ -208,72 +205,35 @@ static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) > return; > } > > - argsz = sizeof(*irq_info); > - irq_info = g_malloc0(argsz); > - irq_info->index = VFIO_CCW_IO_IRQ_INDEX; > - 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"); > - goto out_free_info; > - } > - > if (event_notifier_init(&vcdev->io_notifier, 0)) { > error_setg_errno(errp, errno, > "vfio: Unable to init event notifier for IO"); > - goto out_free_info; > + return; > } > > - argsz = sizeof(*irq_set) + sizeof(*pfd); > - irq_set = g_malloc0(argsz); > - irq_set->argsz = argsz; > - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | > - VFIO_IRQ_SET_ACTION_TRIGGER; > - irq_set->index = VFIO_CCW_IO_IRQ_INDEX; > - irq_set->start = 0; > - irq_set->count = 1; > - pfd = (int32_t *) &irq_set->data; > - > - *pfd = event_notifier_get_fd(&vcdev->io_notifier); > - qemu_set_fd_handler(*pfd, vfio_ccw_io_notifier_handler, NULL, vcdev); > - if (ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) { > - error_setg(errp, "vfio: Failed to set up io notification"); > - qemu_set_fd_handler(*pfd, NULL, NULL, vcdev); > + fd = event_notifier_get_fd(&vcdev->io_notifier); > + qemu_set_fd_handler(fd, vfio_ccw_io_notifier_handler, NULL, vcdev); > + > + if (vfio_set_irq_signaling(vdev, VFIO_CCW_IO_IRQ_INDEX, 0, > + VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) { > + qemu_set_fd_handler(fd, NULL, NULL, vcdev); > event_notifier_cleanup(&vcdev->io_notifier); > } > - > - g_free(irq_set); > - > -out_free_info: > - g_free(irq_info); > } > > static void vfio_ccw_unregister_io_notifier(VFIOCCWDevice *vcdev) > { > - struct vfio_irq_set *irq_set; > - size_t argsz; > - int32_t *pfd; > - > - argsz = sizeof(*irq_set) + sizeof(*pfd); > - irq_set = g_malloc0(argsz); > - irq_set->argsz = argsz; > - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | > - VFIO_IRQ_SET_ACTION_TRIGGER; > - irq_set->index = VFIO_CCW_IO_IRQ_INDEX; > - irq_set->start = 0; > - irq_set->count = 1; > - pfd = (int32_t *) &irq_set->data; > - *pfd = -1; > - > - if (ioctl(vcdev->vdev.fd, VFIO_DEVICE_SET_IRQS, irq_set)) { > - error_report("vfio: Failed to de-assign device io fd: %m"); > + Error *err = NULL; > + > + vfio_set_irq_signaling(&vcdev->vdev, VFIO_CCW_IO_IRQ_INDEX, 0, > + VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err); > + if (err) { > + error_reportf_err(err, VFIO_MSG_PREFIX, vcdev->vdev.name); > } > > qemu_set_fd_handler(event_notifier_get_fd(&vcdev->io_notifier), > NULL, NULL, vcdev); > event_notifier_cleanup(&vcdev->io_notifier); > - > - g_free(irq_set); > } > > static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp) >
On 6/14/19 5:27 AM, Cornelia Huck wrote: > Use the new helper. > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > hw/vfio/ccw.c | 68 +++++++++++---------------------------------------- > 1 file changed, 14 insertions(+), 54 deletions(-) > > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > index 03a2becb3ec9..3dc08721a3db 100644 > --- a/hw/vfio/ccw.c > +++ b/hw/vfio/ccw.c > @@ -197,10 +197,7 @@ read_err: > static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) > { > VFIODevice *vdev = &vcdev->vdev; > - struct vfio_irq_info *irq_info; > - struct vfio_irq_set *irq_set; > - size_t argsz; > - int32_t *pfd; > + int fd; > > if (vdev->num_irqs < VFIO_CCW_IO_IRQ_INDEX + 1) { > error_setg(errp, "vfio: unexpected number of io irqs %u", > @@ -208,72 +205,35 @@ static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) > return; > } > > - argsz = sizeof(*irq_info); > - irq_info = g_malloc0(argsz); > - irq_info->index = VFIO_CCW_IO_IRQ_INDEX; > - 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"); > - goto out_free_info; > - } > - Don't we still need this hunk? (And the out_free_info label stuff that cleans it up.) I don't see vfio_set_irq_signaling() covering it. > if (event_notifier_init(&vcdev->io_notifier, 0)) { > error_setg_errno(errp, errno, > "vfio: Unable to init event notifier for IO"); > - goto out_free_info; > + return; > } > > - argsz = sizeof(*irq_set) + sizeof(*pfd); > - irq_set = g_malloc0(argsz); > - irq_set->argsz = argsz; > - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | > - VFIO_IRQ_SET_ACTION_TRIGGER; > - irq_set->index = VFIO_CCW_IO_IRQ_INDEX; > - irq_set->start = 0; > - irq_set->count = 1; > - pfd = (int32_t *) &irq_set->data; > - > - *pfd = event_notifier_get_fd(&vcdev->io_notifier); > - qemu_set_fd_handler(*pfd, vfio_ccw_io_notifier_handler, NULL, vcdev); > - if (ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) { > - error_setg(errp, "vfio: Failed to set up io notification"); > - qemu_set_fd_handler(*pfd, NULL, NULL, vcdev); > + fd = event_notifier_get_fd(&vcdev->io_notifier); > + qemu_set_fd_handler(fd, vfio_ccw_io_notifier_handler, NULL, vcdev); > + > + if (vfio_set_irq_signaling(vdev, VFIO_CCW_IO_IRQ_INDEX, 0, > + VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) { > + qemu_set_fd_handler(fd, NULL, NULL, vcdev); This sure looks nice though. :) > event_notifier_cleanup(&vcdev->io_notifier); > } > - > - g_free(irq_set); > - > -out_free_info: > - g_free(irq_info); > } > > static void vfio_ccw_unregister_io_notifier(VFIOCCWDevice *vcdev) > { > - struct vfio_irq_set *irq_set; > - size_t argsz; > - int32_t *pfd; > - > - argsz = sizeof(*irq_set) + sizeof(*pfd); > - irq_set = g_malloc0(argsz); > - irq_set->argsz = argsz; > - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | > - VFIO_IRQ_SET_ACTION_TRIGGER; > - irq_set->index = VFIO_CCW_IO_IRQ_INDEX; > - irq_set->start = 0; > - irq_set->count = 1; > - pfd = (int32_t *) &irq_set->data; > - *pfd = -1; > - > - if (ioctl(vcdev->vdev.fd, VFIO_DEVICE_SET_IRQS, irq_set)) { > - error_report("vfio: Failed to de-assign device io fd: %m"); > + Error *err = NULL; > + > + vfio_set_irq_signaling(&vcdev->vdev, VFIO_CCW_IO_IRQ_INDEX, 0, > + VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err); > + if (err) { > + error_reportf_err(err, VFIO_MSG_PREFIX, vcdev->vdev.name); > } > > qemu_set_fd_handler(event_notifier_get_fd(&vcdev->io_notifier), > NULL, NULL, vcdev); > event_notifier_cleanup(&vcdev->io_notifier); > - > - g_free(irq_set); > } > > static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp) >
Hi Eric, On 6/14/19 4:30 PM, Eric Farman wrote: > > > On 6/14/19 5:27 AM, Cornelia Huck wrote: >> Use the new helper. >> >> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >> --- >> hw/vfio/ccw.c | 68 +++++++++++---------------------------------------- >> 1 file changed, 14 insertions(+), 54 deletions(-) >> >> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c >> index 03a2becb3ec9..3dc08721a3db 100644 >> --- a/hw/vfio/ccw.c >> +++ b/hw/vfio/ccw.c >> @@ -197,10 +197,7 @@ read_err: >> static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) >> { >> VFIODevice *vdev = &vcdev->vdev; >> - struct vfio_irq_info *irq_info; >> - struct vfio_irq_set *irq_set; >> - size_t argsz; >> - int32_t *pfd; >> + int fd; >> >> if (vdev->num_irqs < VFIO_CCW_IO_IRQ_INDEX + 1) { >> error_setg(errp, "vfio: unexpected number of io irqs %u", >> @@ -208,72 +205,35 @@ static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) >> return; >> } >> >> - argsz = sizeof(*irq_info); >> - irq_info = g_malloc0(argsz); >> - irq_info->index = VFIO_CCW_IO_IRQ_INDEX; >> - 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"); >> - goto out_free_info; >> - } >> - > > Don't we still need this hunk? (And the out_free_info label stuff that > cleans it up.) I don't see vfio_set_irq_signaling() covering it. Looks this IRQ index is always implemented and exposed by the driver, isn't it? In such a case it shouldn't be needed to test its presence? Thanks Eric > >> if (event_notifier_init(&vcdev->io_notifier, 0)) { >> error_setg_errno(errp, errno, >> "vfio: Unable to init event notifier for IO"); >> - goto out_free_info; >> + return; >> } >> >> - argsz = sizeof(*irq_set) + sizeof(*pfd); >> - irq_set = g_malloc0(argsz); >> - irq_set->argsz = argsz; >> - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | >> - VFIO_IRQ_SET_ACTION_TRIGGER; >> - irq_set->index = VFIO_CCW_IO_IRQ_INDEX; >> - irq_set->start = 0; >> - irq_set->count = 1; >> - pfd = (int32_t *) &irq_set->data; >> - >> - *pfd = event_notifier_get_fd(&vcdev->io_notifier); >> - qemu_set_fd_handler(*pfd, vfio_ccw_io_notifier_handler, NULL, vcdev); >> - if (ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) { >> - error_setg(errp, "vfio: Failed to set up io notification"); >> - qemu_set_fd_handler(*pfd, NULL, NULL, vcdev); >> + fd = event_notifier_get_fd(&vcdev->io_notifier); >> + qemu_set_fd_handler(fd, vfio_ccw_io_notifier_handler, NULL, vcdev); >> + >> + if (vfio_set_irq_signaling(vdev, VFIO_CCW_IO_IRQ_INDEX, 0, >> + VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) { >> + qemu_set_fd_handler(fd, NULL, NULL, vcdev); > > This sure looks nice though. :) > >> event_notifier_cleanup(&vcdev->io_notifier); >> } >> - >> - g_free(irq_set); >> - >> -out_free_info: >> - g_free(irq_info); >> } >> >> static void vfio_ccw_unregister_io_notifier(VFIOCCWDevice *vcdev) >> { >> - struct vfio_irq_set *irq_set; >> - size_t argsz; >> - int32_t *pfd; >> - >> - argsz = sizeof(*irq_set) + sizeof(*pfd); >> - irq_set = g_malloc0(argsz); >> - irq_set->argsz = argsz; >> - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | >> - VFIO_IRQ_SET_ACTION_TRIGGER; >> - irq_set->index = VFIO_CCW_IO_IRQ_INDEX; >> - irq_set->start = 0; >> - irq_set->count = 1; >> - pfd = (int32_t *) &irq_set->data; >> - *pfd = -1; >> - >> - if (ioctl(vcdev->vdev.fd, VFIO_DEVICE_SET_IRQS, irq_set)) { >> - error_report("vfio: Failed to de-assign device io fd: %m"); >> + Error *err = NULL; >> + >> + vfio_set_irq_signaling(&vcdev->vdev, VFIO_CCW_IO_IRQ_INDEX, 0, >> + VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err); >> + if (err) { >> + error_reportf_err(err, VFIO_MSG_PREFIX, vcdev->vdev.name); >> } >> >> qemu_set_fd_handler(event_notifier_get_fd(&vcdev->io_notifier), >> NULL, NULL, vcdev); >> event_notifier_cleanup(&vcdev->io_notifier); >> - >> - g_free(irq_set); >> } >> >> static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp) >> >
On 6/14/19 11:06 AM, Auger Eric wrote: > Hi Eric, > > On 6/14/19 4:30 PM, Eric Farman wrote: >> >> >> On 6/14/19 5:27 AM, Cornelia Huck wrote: >>> Use the new helper. >>> >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >>> --- >>> hw/vfio/ccw.c | 68 +++++++++++---------------------------------------- >>> 1 file changed, 14 insertions(+), 54 deletions(-) >>> >>> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c >>> index 03a2becb3ec9..3dc08721a3db 100644 >>> --- a/hw/vfio/ccw.c >>> +++ b/hw/vfio/ccw.c >>> @@ -197,10 +197,7 @@ read_err: >>> static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) >>> { >>> VFIODevice *vdev = &vcdev->vdev; >>> - struct vfio_irq_info *irq_info; >>> - struct vfio_irq_set *irq_set; >>> - size_t argsz; >>> - int32_t *pfd; >>> + int fd; >>> >>> if (vdev->num_irqs < VFIO_CCW_IO_IRQ_INDEX + 1) { >>> error_setg(errp, "vfio: unexpected number of io irqs %u", >>> @@ -208,72 +205,35 @@ static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) >>> return; >>> } >>> >>> - argsz = sizeof(*irq_info); >>> - irq_info = g_malloc0(argsz); >>> - irq_info->index = VFIO_CCW_IO_IRQ_INDEX; >>> - 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"); >>> - goto out_free_info; >>> - } >>> - >> >> Don't we still need this hunk? (And the out_free_info label stuff that >> cleans it up.) I don't see vfio_set_irq_signaling() covering it. > > Looks this IRQ index is always implemented and exposed by the driver, > isn't it? In such a case it shouldn't be needed to test its presence? Right; if we were running on an old kernel, both ioctl's would fail the same way since they were added concurrently. So the check today doesn't seem very useful. But since it's there, and we're taking it out, it got me wondering whether there are/were intentions to expand GET_IRQ_INFO in the future. I'm not aware of any reason vfio-ccw would need to, but want some confirmation that I'm not overlooking anything. Cheers, Other Eric > > Thanks > > Eric >> >>> if (event_notifier_init(&vcdev->io_notifier, 0)) { >>> error_setg_errno(errp, errno, >>> "vfio: Unable to init event notifier for IO"); >>> - goto out_free_info; >>> + return; >>> } >>> >>> - argsz = sizeof(*irq_set) + sizeof(*pfd); >>> - irq_set = g_malloc0(argsz); >>> - irq_set->argsz = argsz; >>> - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | >>> - VFIO_IRQ_SET_ACTION_TRIGGER; >>> - irq_set->index = VFIO_CCW_IO_IRQ_INDEX; >>> - irq_set->start = 0; >>> - irq_set->count = 1; >>> - pfd = (int32_t *) &irq_set->data; >>> - >>> - *pfd = event_notifier_get_fd(&vcdev->io_notifier); >>> - qemu_set_fd_handler(*pfd, vfio_ccw_io_notifier_handler, NULL, vcdev); >>> - if (ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) { >>> - error_setg(errp, "vfio: Failed to set up io notification"); >>> - qemu_set_fd_handler(*pfd, NULL, NULL, vcdev); >>> + fd = event_notifier_get_fd(&vcdev->io_notifier); >>> + qemu_set_fd_handler(fd, vfio_ccw_io_notifier_handler, NULL, vcdev); >>> + >>> + if (vfio_set_irq_signaling(vdev, VFIO_CCW_IO_IRQ_INDEX, 0, >>> + VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) { >>> + qemu_set_fd_handler(fd, NULL, NULL, vcdev); >> >> This sure looks nice though. :) >> >>> event_notifier_cleanup(&vcdev->io_notifier); >>> } >>> - >>> - g_free(irq_set); >>> - >>> -out_free_info: >>> - g_free(irq_info); >>> } >>> >>> static void vfio_ccw_unregister_io_notifier(VFIOCCWDevice *vcdev) >>> { >>> - struct vfio_irq_set *irq_set; >>> - size_t argsz; >>> - int32_t *pfd; >>> - >>> - argsz = sizeof(*irq_set) + sizeof(*pfd); >>> - irq_set = g_malloc0(argsz); >>> - irq_set->argsz = argsz; >>> - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | >>> - VFIO_IRQ_SET_ACTION_TRIGGER; >>> - irq_set->index = VFIO_CCW_IO_IRQ_INDEX; >>> - irq_set->start = 0; >>> - irq_set->count = 1; >>> - pfd = (int32_t *) &irq_set->data; >>> - *pfd = -1; >>> - >>> - if (ioctl(vcdev->vdev.fd, VFIO_DEVICE_SET_IRQS, irq_set)) { >>> - error_report("vfio: Failed to de-assign device io fd: %m"); >>> + Error *err = NULL; >>> + >>> + vfio_set_irq_signaling(&vcdev->vdev, VFIO_CCW_IO_IRQ_INDEX, 0, >>> + VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err); >>> + if (err) { >>> + error_reportf_err(err, VFIO_MSG_PREFIX, vcdev->vdev.name); >>> } >>> >>> qemu_set_fd_handler(event_notifier_get_fd(&vcdev->io_notifier), >>> NULL, NULL, vcdev); >>> event_notifier_cleanup(&vcdev->io_notifier); >>> - >>> - g_free(irq_set); >>> } >>> >>> static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp) >>> >> >
On Fri, 14 Jun 2019 11:41:41 -0400 Eric Farman <farman@linux.ibm.com> wrote: > On 6/14/19 11:06 AM, Auger Eric wrote: > > Hi Eric, > > > > On 6/14/19 4:30 PM, Eric Farman wrote: > >> > >> > >> On 6/14/19 5:27 AM, Cornelia Huck wrote: > >>> Use the new helper. > >>> > >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> > >>> --- > >>> hw/vfio/ccw.c | 68 +++++++++++---------------------------------------- > >>> 1 file changed, 14 insertions(+), 54 deletions(-) > >>> > >>> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > >>> index 03a2becb3ec9..3dc08721a3db 100644 > >>> --- a/hw/vfio/ccw.c > >>> +++ b/hw/vfio/ccw.c > >>> @@ -197,10 +197,7 @@ read_err: > >>> static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) > >>> { > >>> VFIODevice *vdev = &vcdev->vdev; > >>> - struct vfio_irq_info *irq_info; > >>> - struct vfio_irq_set *irq_set; > >>> - size_t argsz; > >>> - int32_t *pfd; > >>> + int fd; > >>> > >>> if (vdev->num_irqs < VFIO_CCW_IO_IRQ_INDEX + 1) { > >>> error_setg(errp, "vfio: unexpected number of io irqs %u", > >>> @@ -208,72 +205,35 @@ static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) > >>> return; > >>> } > >>> > >>> - argsz = sizeof(*irq_info); > >>> - irq_info = g_malloc0(argsz); > >>> - irq_info->index = VFIO_CCW_IO_IRQ_INDEX; > >>> - 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"); > >>> - goto out_free_info; > >>> - } > >>> - > >> > >> Don't we still need this hunk? (And the out_free_info label stuff that > >> cleans it up.) I don't see vfio_set_irq_signaling() covering it. > > > > Looks this IRQ index is always implemented and exposed by the driver, > > isn't it? In such a case it shouldn't be needed to test its presence? > > Right; if we were running on an old kernel, both ioctl's would fail the > same way since they were added concurrently. So the check today doesn't > seem very useful. > > But since it's there, and we're taking it out, it got me wondering > whether there are/were intentions to expand GET_IRQ_INFO in the future. > I'm not aware of any reason vfio-ccw would need to, but want some > confirmation that I'm not overlooking anything. It seems I actually was a bit too delete-happy... I did not intend to rip out that code at all, it just somehow was suddenly gone :) Checking is probably not strictly needed, but let me send a v2 that does not delete it (and only converts to the new interface).
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c index 03a2becb3ec9..3dc08721a3db 100644 --- a/hw/vfio/ccw.c +++ b/hw/vfio/ccw.c @@ -197,10 +197,7 @@ read_err: static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) { VFIODevice *vdev = &vcdev->vdev; - struct vfio_irq_info *irq_info; - struct vfio_irq_set *irq_set; - size_t argsz; - int32_t *pfd; + int fd; if (vdev->num_irqs < VFIO_CCW_IO_IRQ_INDEX + 1) { error_setg(errp, "vfio: unexpected number of io irqs %u", @@ -208,72 +205,35 @@ static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp) return; } - argsz = sizeof(*irq_info); - irq_info = g_malloc0(argsz); - irq_info->index = VFIO_CCW_IO_IRQ_INDEX; - 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"); - goto out_free_info; - } - if (event_notifier_init(&vcdev->io_notifier, 0)) { error_setg_errno(errp, errno, "vfio: Unable to init event notifier for IO"); - goto out_free_info; + return; } - argsz = sizeof(*irq_set) + sizeof(*pfd); - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | - VFIO_IRQ_SET_ACTION_TRIGGER; - irq_set->index = VFIO_CCW_IO_IRQ_INDEX; - irq_set->start = 0; - irq_set->count = 1; - pfd = (int32_t *) &irq_set->data; - - *pfd = event_notifier_get_fd(&vcdev->io_notifier); - qemu_set_fd_handler(*pfd, vfio_ccw_io_notifier_handler, NULL, vcdev); - if (ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) { - error_setg(errp, "vfio: Failed to set up io notification"); - qemu_set_fd_handler(*pfd, NULL, NULL, vcdev); + fd = event_notifier_get_fd(&vcdev->io_notifier); + qemu_set_fd_handler(fd, vfio_ccw_io_notifier_handler, NULL, vcdev); + + if (vfio_set_irq_signaling(vdev, VFIO_CCW_IO_IRQ_INDEX, 0, + VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) { + qemu_set_fd_handler(fd, NULL, NULL, vcdev); event_notifier_cleanup(&vcdev->io_notifier); } - - g_free(irq_set); - -out_free_info: - g_free(irq_info); } static void vfio_ccw_unregister_io_notifier(VFIOCCWDevice *vcdev) { - struct vfio_irq_set *irq_set; - size_t argsz; - int32_t *pfd; - - argsz = sizeof(*irq_set) + sizeof(*pfd); - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | - VFIO_IRQ_SET_ACTION_TRIGGER; - irq_set->index = VFIO_CCW_IO_IRQ_INDEX; - irq_set->start = 0; - irq_set->count = 1; - pfd = (int32_t *) &irq_set->data; - *pfd = -1; - - if (ioctl(vcdev->vdev.fd, VFIO_DEVICE_SET_IRQS, irq_set)) { - error_report("vfio: Failed to de-assign device io fd: %m"); + Error *err = NULL; + + vfio_set_irq_signaling(&vcdev->vdev, VFIO_CCW_IO_IRQ_INDEX, 0, + VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err); + if (err) { + error_reportf_err(err, VFIO_MSG_PREFIX, vcdev->vdev.name); } qemu_set_fd_handler(event_notifier_get_fd(&vcdev->io_notifier), NULL, NULL, vcdev); event_notifier_cleanup(&vcdev->io_notifier); - - g_free(irq_set); } static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
Use the new helper. Signed-off-by: Cornelia Huck <cohuck@redhat.com> --- hw/vfio/ccw.c | 68 +++++++++++---------------------------------------- 1 file changed, 14 insertions(+), 54 deletions(-)