| Message ID | 20231025171841.3379663-1-mathieu.poirier@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | virtio: rng: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX | expand |
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>
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>
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.
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
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); }
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(+)