Re: [PATCH v7 07/10] virtio-mmio: add support for configure interrupt

[Jason Wang]

在 2021/6/2 上午11:47, Cindy Lu 写道:
> Add configure interrupt support for virtio-mmio bus. This
> interrupt will working while backend is vhost-vdpa
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>   hw/virtio/virtio-mmio.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
>
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 13772d52bb..423267d51c 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -670,7 +670,26 @@ static int virtio_mmio_set_guest_notifier(DeviceState *d, 
> int n, bool assign,
>   
>       return 0;
>   }
> +static int virtio_mmio_set_config_notifier(DeviceState *d, bool assign)
> +{
> +    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
>   
> +    EventNotifier *notifier = virtio_get_config_notifier(vdev);
> +    int r = 0;
> +    if (assign) {
> +        r = event_notifier_init(notifier, 0);


The return value is ignored.


> +        virtio_set_notifier_fd_handler(vdev, -1, true, false);


You'd better use the macro you introduced in patch 1 here?


> +    } else {
> +        virtio_set_notifier_fd_handler(vdev, -1, false, false);
> +        event_notifier_cleanup(notifier);
> +    }
> +    if (vdc->guest_notifier_mask && vdev->use_guest_notifier_mask) {
> +        vdc->guest_notifier_mask(vdev, -1, !assign);
> +    }
> +    return r;
> +}
>   static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
>                                              bool assign)
>   {
> @@ -692,8 +711,15 @@ static int virtio_mmio_set_guest_notifiers(DeviceState *d, 
> int nvqs,
>               goto assign_error;
>           }
>       }
> +    r = virtio_mmio_set_config_notifier(d, assign);
> +    if (r < 0) {
> +        goto config_assign_error;
> +    }
>   
>       return 0;
> +config_assign_error:
> +    assert(assign);
> +    r = virtio_mmio_set_config_notifier(d, false);


This looks wired. We only have a single configure interrupt, so assign 
fails should mean unassigned?

Thanks


>   
>   assign_error:
>       /* We get here on assignment failure. Recover by undoing for VQs 0 .. n. 
> */