[v1] virtio: i2c: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX

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

Posted by Viresh Kumar 2 years, 9 months ago

Since the driver doesn't support interrupts, we must return early when
index is set to VIRTIO_CONFIG_IRQ_IDX.

Fixes: 544f0278afca ("virtio: introduce macro VIRTIO_CONFIG_IRQ_IDX")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 hw/virtio/vhost-user-i2c.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c
index 60eaf0d95be0..45100a24953c 100644
--- a/hw/virtio/vhost-user-i2c.c
+++ b/hw/virtio/vhost-user-i2c.c
@@ -128,6 +128,16 @@ static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
 {
     VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
 
+    /*
+     * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1
+     * as the Marco of configure interrupt's IDX, If this driver does not
+     * support, the function will return
+     */
+
+    if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+        return;
+    }
+
     vhost_virtqueue_mask(&i2c->vhost_dev, vdev, idx, mask);
 }
 
@@ -135,6 +145,16 @@ static bool vu_i2c_guest_notifier_pending(VirtIODevice *vdev, int idx)
 {
     VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
 
+    /*
+     * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1
+     * as the Marco of configure interrupt's IDX, If this driver does not
+     * support, the function will return
+     */
+
+    if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+        return false;
+    }
+
     return vhost_virtqueue_pending(&i2c->vhost_dev, idx);
 }
 
-- 
2.31.1.272.g89b43f80a514

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

Posted by Philippe Mathieu-Daudé 2 years, 9 months ago

On 17/4/23 08:02, Viresh Kumar wrote:
> Since the driver doesn't support interrupts, we must return early when
> index is set to VIRTIO_CONFIG_IRQ_IDX.
> 
> Fixes: 544f0278afca ("virtio: introduce macro VIRTIO_CONFIG_IRQ_IDX")
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   hw/virtio/vhost-user-i2c.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c
> index 60eaf0d95be0..45100a24953c 100644
> --- a/hw/virtio/vhost-user-i2c.c
> +++ b/hw/virtio/vhost-user-i2c.c
> @@ -128,6 +128,16 @@ static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
>   {
>       VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
>   
> +    /*
> +     * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1
> +     * as the Marco of configure interrupt's IDX, If this driver does not

Copy/paste of pre-existing comment, still I wonder who is "the Marco of 
configure" :P

> +     * support, the function will return
> +     */

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

Posted by Alex Bennée 2 years, 9 months ago

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 17/4/23 08:02, Viresh Kumar wrote:
>> Since the driver doesn't support interrupts, we must return early when
>> index is set to VIRTIO_CONFIG_IRQ_IDX.
>> Fixes: 544f0278afca ("virtio: introduce macro
>> VIRTIO_CONFIG_IRQ_IDX")
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>   hw/virtio/vhost-user-i2c.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>> diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c
>> index 60eaf0d95be0..45100a24953c 100644
>> --- a/hw/virtio/vhost-user-i2c.c
>> +++ b/hw/virtio/vhost-user-i2c.c
>> @@ -128,6 +128,16 @@ static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
>>   {
>>       VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
>>   +    /*
>> +     * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1
>> +     * as the Marco of configure interrupt's IDX, If this driver does not
>
> Copy/paste of pre-existing comment, still I wonder who is "the Marco
> of configure" :P
>
>> +     * support, the function will return
>> +     */

First patch of my last VirtIO series fixes these all up. 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

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

Posted by Viresh Kumar 2 years, 9 months ago

On 17-04-23, 09:36, Philippe Mathieu-Daudé wrote:
> On 17/4/23 08:02, Viresh Kumar wrote:
> > Since the driver doesn't support interrupts, we must return early when
> > index is set to VIRTIO_CONFIG_IRQ_IDX.
> > 
> > Fixes: 544f0278afca ("virtio: introduce macro VIRTIO_CONFIG_IRQ_IDX")
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >   hw/virtio/vhost-user-i2c.c | 20 ++++++++++++++++++++
> >   1 file changed, 20 insertions(+)
> > 
> > diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c
> > index 60eaf0d95be0..45100a24953c 100644
> > --- a/hw/virtio/vhost-user-i2c.c
> > +++ b/hw/virtio/vhost-user-i2c.c
> > @@ -128,6 +128,16 @@ static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
> >   {
> >       VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
> > +    /*
> > +     * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1
> > +     * as the Marco of configure interrupt's IDX, If this driver does not
> 
> Copy/paste of pre-existing comment, still I wonder who is "the Marco of
> configure" :P
> 
> > +     * support, the function will return
> > +     */

Yeah the comment could be improved, I didn't touch it as it was
written this way for many drivers :)

Maybe a simple comment like is all we need:

/*
 * We don't support interrupts, return early if index is set to
 * VIRTIO_CONFIG_IRQ_IDX.
 */

-- 
viresh