Re: [PATCH] virtio: rng: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Mathieu,

On 25/10/23 19:18, Mathieu Poirier wrote:
> Since the driver doesn't support interrupts, we must return early when
> index is set to VIRTIO_CONFIG_IRQ_IDX.  Basically the same thing Viresh
> did for "91208dd297f2 virtio: i2c: Check notifier helpers for
> VIRTIO_CONFIG_IRQ_IDX".
> 
> Fixes: 544f0278afca ("virtio: introduce macro VIRTIO_CONFIG_IRQ_IDX")
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>   hw/virtio/vhost-user-rng.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/hw/virtio/vhost-user-rng.c b/hw/virtio/vhost-user-rng.c
> index 201a39e220c5..62142b717f73 100644
> --- a/hw/virtio/vhost-user-rng.c
> +++ b/hw/virtio/vhost-user-rng.c
> @@ -129,6 +129,14 @@ static void vu_rng_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
>   {
>       VHostUserRNG *rng = VHOST_USER_RNG(vdev);
>   
> +    /*
> +     * We don't support interrupts, return early if index is set to
> +     * VIRTIO_CONFIG_IRQ_IDX.
> +     */
> +    if (idx == VIRTIO_CONFIG_IRQ_IDX) {
> +        return;
> +    }
> +
>       vhost_virtqueue_mask(&rng->vhost_dev, vdev, idx, mask);
>   }
>   
> @@ -136,6 +144,14 @@ static bool vu_rng_guest_notifier_pending(VirtIODevice *vdev, int idx)
>   {
>       VHostUserRNG *rng = VHOST_USER_RNG(vdev);
>   
> +    /*
> +     * We don't support interrupts, return early if index is set to
> +     * VIRTIO_CONFIG_IRQ_IDX.
> +     */
> +    if (idx == VIRTIO_CONFIG_IRQ_IDX) {
> +        return false;
> +    }
> +
>       return vhost_virtqueue_pending(&rng->vhost_dev, idx);
>   }

The patch looks fine, so:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Now I wonder if we shouldn't add that check one layer above in the
caller, eventually checking a VirtioDeviceClass::irq_supported bool.
This is out of the scope of this patch.

Regards,

Phil.