virtio: rng: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX

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

Posted by Mathieu Poirier 1 year, 1 month ago

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);
 }
 
-- 
2.34.1

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

Posted by Michael S. Tsirkin 1 year ago

On Wed, Oct 25, 2023 at 11:18:41AM -0600, 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>

So vhost-user-rng is now switching to use the generic base device.
Alex could you help suggest how to rebase this on top of your series please?

> ---
>  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);
>  }
>  
> -- 
> 2.34.1

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

Posted by Philippe Mathieu-Daudé 1 year, 1 month ago

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.

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

Posted by Leo Yan 1 year, 1 month ago

On Wed, Oct 25, 2023 at 11:18:41AM -0600, 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>

Tested this patch and the vhost-user-i2c device works with it:

Tested-by: Leo Yan <leo.yan@linaro.org>

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

Posted by Leo Yan 1 year, 1 month ago

On Thu, Oct 26, 2023 at 01:06:21PM +0800, Leo Yan wrote:
> On Wed, Oct 25, 2023 at 11:18:41AM -0600, 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>
> 
> Tested this patch and the vhost-user-i2c device works with it:

Sorry for typo.  Here I am meaning the vhost-device-rng device.

> Tested-by: Leo Yan <leo.yan@linaro.org>