On 11/20/19 7:39 AM, Cornelia Huck wrote:
> 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?
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.
>
