On Fri, 15 Nov 2019 04:34:36 +0100
Eric Farman <far...@linux.ibm.com> wrote:
> Make it easier to add new ones in the future.
>
> Signed-off-by: Eric Farman <far...@linux.ibm.com>
> ---
> hw/vfio/ccw.c | 55 ++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 39 insertions(+), 16 deletions(-)
>
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 2b1a83b94c..b16526d5de 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -334,22 +334,35 @@ read_err:
> css_inject_io_interrupt(sch);
> }
>
> -static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp)
> +static void vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev, int irq,
Maybe make this unsigned?
> + Error **errp)
> {
> VFIODevice *vdev = &vcdev->vdev;
> struct vfio_irq_info *irq_info;
> size_t argsz;
> int fd;
> + EventNotifier *notifier;
> + IOHandler *fd_read;
> +
> + switch (irq) {
> + case VFIO_CCW_IO_IRQ_INDEX:
> + notifier = &vcdev->io_notifier;
> + fd_read = vfio_ccw_io_notifier_handler;
> + break;
> + default:
> + error_setg(errp, "vfio: Unsupported device irq(%d) fd: %m", irq);
Hm, which errno is this supposed to print?
> + return;
> + }
>
> - if (vdev->num_irqs < VFIO_CCW_IO_IRQ_INDEX + 1) {
> - error_setg(errp, "vfio: unexpected number of io irqs %u",
> + if (vdev->num_irqs < irq + 1) {
> + error_setg(errp, "vfio: unexpected number of irqs %u",
> vdev->num_irqs);
> return;
> }
>
> argsz = sizeof(*irq_info);
> irq_info = g_malloc0(argsz);
> - irq_info->index = VFIO_CCW_IO_IRQ_INDEX;
> + irq_info->index = irq;
> irq_info->argsz = argsz;
> if (ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO,
> irq_info) < 0 || irq_info->count < 1) {
> @@ -357,37 +370,47 @@ static void vfio_ccw_register_io_notifier(VFIOCCWDevice
> *vcdev, Error **errp)
> goto out_free_info;
> }
>
> - if (event_notifier_init(&vcdev->io_notifier, 0)) {
> + if (event_notifier_init(notifier, 0)) {
> error_setg_errno(errp, errno,
> - "vfio: Unable to init event notifier for IO");
> + "vfio: Unable to init event notifier for irq (%d)",
> irq);
> goto out_free_info;
> }
>
> - fd = event_notifier_get_fd(&vcdev->io_notifier);
> - qemu_set_fd_handler(fd, vfio_ccw_io_notifier_handler, NULL, vcdev);
> + fd = event_notifier_get_fd(notifier);
> + qemu_set_fd_handler(fd, fd_read, NULL, vcdev);
>
> - if (vfio_set_irq_signaling(vdev, VFIO_CCW_IO_IRQ_INDEX, 0,
> + if (vfio_set_irq_signaling(vdev, irq, 0,
> VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
> qemu_set_fd_handler(fd, NULL, NULL, vcdev);
> - event_notifier_cleanup(&vcdev->io_notifier);
> + event_notifier_cleanup(notifier);
> }
>
> out_free_info:
> g_free(irq_info);
> }
>
> -static void vfio_ccw_unregister_io_notifier(VFIOCCWDevice *vcdev)
> +static void vfio_ccw_unregister_irq_notifier(VFIOCCWDevice *vcdev, int irq)
Also unsigned here?
> {
> Error *err = NULL;
> + EventNotifier *notifier;
> +
> + switch (irq) {
> + case VFIO_CCW_IO_IRQ_INDEX:
> + notifier = &vcdev->io_notifier;
> + break;
> + default:
> + error_report("vfio: Unsupported device irq(%d) fd: %m", irq);
Same comment for the %m.
> + return;
> + }
>
> - if (vfio_set_irq_signaling(&vcdev->vdev, VFIO_CCW_IO_IRQ_INDEX, 0,
> + if (vfio_set_irq_signaling(&vcdev->vdev, irq, 0,
> VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
> error_reportf_err(err, VFIO_MSG_PREFIX, vcdev->vdev.name);
> }
>
> - qemu_set_fd_handler(event_notifier_get_fd(&vcdev->io_notifier),
> + qemu_set_fd_handler(event_notifier_get_fd(notifier),
> NULL, NULL, vcdev);
> - event_notifier_cleanup(&vcdev->io_notifier);
> + event_notifier_cleanup(notifier);
> }
>
> static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
> @@ -590,7 +613,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error
> **errp)
> goto out_region_err;
> }
>
> - vfio_ccw_register_io_notifier(vcdev, &err);
> + vfio_ccw_register_irq_notifier(vcdev, VFIO_CCW_IO_IRQ_INDEX, &err);
> if (err) {
> goto out_notifier_err;
> }
> @@ -619,7 +642,7 @@ static void vfio_ccw_unrealize(DeviceState *dev, Error
> **errp)
> S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
> VFIOGroup *group = vcdev->vdev.group;
>
> - vfio_ccw_unregister_io_notifier(vcdev);
> + vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_IO_IRQ_INDEX);
> vfio_ccw_put_region(vcdev);
> vfio_ccw_put_device(vcdev);
> vfio_put_group(group);
Otherwise, looks good.
