diff mbox series

[RFC,v1,7/8] vfio-ccw: Refactor ccw irq handler

Message ID 20191115033437.37926-8-farman@linux.ibm.com
State New
Headers show
Series s390x/vfio-ccw: Channel Path Handling | expand

Commit Message

Eric Farman Nov. 15, 2019, 3:34 a.m. UTC 
Make it easier to add new ones in the future.

Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 hw/vfio/ccw.c | 55 ++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 16 deletions(-)

Comments

Cornelia Huck Nov. 20, 2019, 12:39 p.m. UTC | #1 
On Fri, 15 Nov 2019 04:34:36 +0100
Eric Farman <farman@linux.ibm.com> wrote:

> Make it easier to add new ones in the future.
> 
> Signed-off-by: Eric Farman <farman@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.
Eric Farman Dec. 3, 2019, 8:01 p.m. UTC | #2 
On 11/20/19 7:39 AM, Cornelia Huck wrote:
> On Fri, 15 Nov 2019 04:34:36 +0100
> Eric Farman <farman@linux.ibm.com> wrote:
> 
>> Make it easier to add new ones in the future.
>>
>> Signed-off-by: Eric Farman <farman@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?

Yeah, seems proper.

> 
>> +                                           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?

Um...  Dealers choice?  :)  I'll fix this up.

> 
>> +        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.
>
diff mbox series

Patch

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,
+                                           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);
+        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)
 {
     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);
+        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);