All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] vhost_vdpa: unset vq irq before freeing irq
@ 2021-09-15  6:39 Wu Zongyong
  2021-09-15  7:26   ` Michael S. Tsirkin
  2021-09-15  8:34   ` Jason Wang
  0 siblings, 2 replies; 8+ messages in thread
From: Wu Zongyong @ 2021-09-15  6:39 UTC (permalink / raw)
  To: wuzongyong, jasowang, virtualization, linux-kernel, mst, tiwei.bie
  Cc: wei.yang1

Currently we unset vq irq after freeing irq and that will result in
error messages:

  pi_update_irte: failed to update PI IRTE
  irq bypass consumer (token 000000005a07a12b) unregistration fails: -22

This patch solves this.

Signed-off-by: Wu Zongyong <wuzongyong@linux.alibaba.com>
---
 drivers/vhost/vdpa.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index f41d081777f5..15bae2290bf9 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -173,6 +173,10 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
 	if (status != 0 && (ops->get_status(vdpa) & ~status) != 0)
 		return -EINVAL;
 
+	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
+		for (i = 0; i < nvqs; i++)
+			vhost_vdpa_unsetup_vq_irq(v, i);
+
 	if (status == 0) {
 		ret = ops->reset(vdpa);
 		if (ret)
@@ -184,10 +188,6 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
 		for (i = 0; i < nvqs; i++)
 			vhost_vdpa_setup_vq_irq(v, i);
 
-	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
-		for (i = 0; i < nvqs; i++)
-			vhost_vdpa_unsetup_vq_irq(v, i);
-
 	return 0;
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] vhost_vdpa: unset vq irq before freeing irq
  2021-09-15  6:39 [PATCH] vhost_vdpa: unset vq irq before freeing irq Wu Zongyong
@ 2021-09-15  7:26   ` Michael S. Tsirkin
  2021-09-15  8:34   ` Jason Wang
  1 sibling, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2021-09-15  7:26 UTC (permalink / raw)
  To: Wu Zongyong; +Cc: jasowang, virtualization, linux-kernel, tiwei.bie, wei.yang1

On Wed, Sep 15, 2021 at 02:39:32PM +0800, Wu Zongyong wrote:
> Currently we unset vq irq after freeing irq and that will result in
> error messages:
> 
>   pi_update_irte: failed to update PI IRTE
>   irq bypass consumer (token 000000005a07a12b) unregistration fails: -22
> 
> This patch solves this.
> 
> Signed-off-by: Wu Zongyong <wuzongyong@linux.alibaba.com>
> ---
>  drivers/vhost/vdpa.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index f41d081777f5..15bae2290bf9 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -173,6 +173,10 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
>  	if (status != 0 && (ops->get_status(vdpa) & ~status) != 0)
>  		return -EINVAL;
>  
> +	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> +		for (i = 0; i < nvqs; i++)
> +			vhost_vdpa_unsetup_vq_irq(v, i);
> +

If we do this before reset like this then the device might assert the
irq, might it not?

>  	if (status == 0) {
>  		ret = ops->reset(vdpa);
>  		if (ret)


> @@ -184,10 +188,6 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
>  		for (i = 0; i < nvqs; i++)
>  			vhost_vdpa_setup_vq_irq(v, i);
>  
> -	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> -		for (i = 0; i < nvqs; i++)
> -			vhost_vdpa_unsetup_vq_irq(v, i);
> -
>  	return 0;



>  }
>  
> -- 
> 2.31.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vhost_vdpa: unset vq irq before freeing irq
@ 2021-09-15  7:26   ` Michael S. Tsirkin
  0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2021-09-15  7:26 UTC (permalink / raw)
  To: Wu Zongyong; +Cc: wei.yang1, linux-kernel, tiwei.bie, virtualization

On Wed, Sep 15, 2021 at 02:39:32PM +0800, Wu Zongyong wrote:
> Currently we unset vq irq after freeing irq and that will result in
> error messages:
> 
>   pi_update_irte: failed to update PI IRTE
>   irq bypass consumer (token 000000005a07a12b) unregistration fails: -22
> 
> This patch solves this.
> 
> Signed-off-by: Wu Zongyong <wuzongyong@linux.alibaba.com>
> ---
>  drivers/vhost/vdpa.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index f41d081777f5..15bae2290bf9 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -173,6 +173,10 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
>  	if (status != 0 && (ops->get_status(vdpa) & ~status) != 0)
>  		return -EINVAL;
>  
> +	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> +		for (i = 0; i < nvqs; i++)
> +			vhost_vdpa_unsetup_vq_irq(v, i);
> +

If we do this before reset like this then the device might assert the
irq, might it not?

>  	if (status == 0) {
>  		ret = ops->reset(vdpa);
>  		if (ret)


> @@ -184,10 +188,6 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
>  		for (i = 0; i < nvqs; i++)
>  			vhost_vdpa_setup_vq_irq(v, i);
>  
> -	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> -		for (i = 0; i < nvqs; i++)
> -			vhost_vdpa_unsetup_vq_irq(v, i);
> -
>  	return 0;



>  }
>  
> -- 
> 2.31.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vhost_vdpa: unset vq irq before freeing irq
  2021-09-15  7:26   ` Michael S. Tsirkin
  (?)
@ 2021-09-15  7:52   ` Wu Zongyong
  2021-09-15 11:07       ` Michael S. Tsirkin
  -1 siblings, 1 reply; 8+ messages in thread
From: Wu Zongyong @ 2021-09-15  7:52 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: jasowang, virtualization, linux-kernel, tiwei.bie, wei.yang1

On Wed, Sep 15, 2021 at 03:26:41AM -0400, Michael S. Tsirkin wrote:
> On Wed, Sep 15, 2021 at 02:39:32PM +0800, Wu Zongyong wrote:
> > Currently we unset vq irq after freeing irq and that will result in
> > error messages:
> > 
> >   pi_update_irte: failed to update PI IRTE
> >   irq bypass consumer (token 000000005a07a12b) unregistration fails: -22
> > 
> > This patch solves this.
> > 
> > Signed-off-by: Wu Zongyong <wuzongyong@linux.alibaba.com>
> > ---
> >  drivers/vhost/vdpa.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> > index f41d081777f5..15bae2290bf9 100644
> > --- a/drivers/vhost/vdpa.c
> > +++ b/drivers/vhost/vdpa.c
> > @@ -173,6 +173,10 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
> >  	if (status != 0 && (ops->get_status(vdpa) & ~status) != 0)
> >  		return -EINVAL;
> >  
> > +	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> > +		for (i = 0; i < nvqs; i++)
> > +			vhost_vdpa_unsetup_vq_irq(v, i);
> > +
> 
> If we do this before reset like this then the device might assert the
> irq, might it not?
> 
This would not be a problem.
AFAIK, vhost_vdpa_unsetup_vq_irq just disables the irq offloading, and the irq
will be handled if there comes an irq.
> >  	if (status == 0) {
> >  		ret = ops->reset(vdpa);
> >  		if (ret)
> 
> 
> > @@ -184,10 +188,6 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
> >  		for (i = 0; i < nvqs; i++)
> >  			vhost_vdpa_setup_vq_irq(v, i);
> >  
> > -	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> > -		for (i = 0; i < nvqs; i++)
> > -			vhost_vdpa_unsetup_vq_irq(v, i);
> > -
> >  	return 0;
> 
> 
> 
> >  }
> >  
> > -- 
> > 2.31.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vhost_vdpa: unset vq irq before freeing irq
  2021-09-15  6:39 [PATCH] vhost_vdpa: unset vq irq before freeing irq Wu Zongyong
@ 2021-09-15  8:34   ` Jason Wang
  2021-09-15  8:34   ` Jason Wang
  1 sibling, 0 replies; 8+ messages in thread
From: Jason Wang @ 2021-09-15  8:34 UTC (permalink / raw)
  To: Wu Zongyong; +Cc: virtualization, linux-kernel, mst, tiwei.bie, wei.yang1

On Wed, Sep 15, 2021 at 2:39 PM Wu Zongyong
<wuzongyong@linux.alibaba.com> wrote:
>
> Currently we unset vq irq after freeing irq and that will result in
> error messages:

Nit: I think for 'we', it actually means the parent driver can free
the irq during reset, set_status(0). vp_vdpa is one example for this.

>
>   pi_update_irte: failed to update PI IRTE
>   irq bypass consumer (token 000000005a07a12b) unregistration fails: -22
>
> This patch solves this.
>
> Signed-off-by: Wu Zongyong <wuzongyong@linux.alibaba.com>

Other than this.

Acked-by: Jason Wang <jasowang@redhat.com>

> ---
>  drivers/vhost/vdpa.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index f41d081777f5..15bae2290bf9 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -173,6 +173,10 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
>         if (status != 0 && (ops->get_status(vdpa) & ~status) != 0)
>                 return -EINVAL;
>
> +       if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> +               for (i = 0; i < nvqs; i++)
> +                       vhost_vdpa_unsetup_vq_irq(v, i);
> +
>         if (status == 0) {
>                 ret = ops->reset(vdpa);
>                 if (ret)
> @@ -184,10 +188,6 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
>                 for (i = 0; i < nvqs; i++)
>                         vhost_vdpa_setup_vq_irq(v, i);
>
> -       if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> -               for (i = 0; i < nvqs; i++)
> -                       vhost_vdpa_unsetup_vq_irq(v, i);
> -
>         return 0;
>  }
>
> --
> 2.31.1
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vhost_vdpa: unset vq irq before freeing irq
@ 2021-09-15  8:34   ` Jason Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Jason Wang @ 2021-09-15  8:34 UTC (permalink / raw)
  To: Wu Zongyong; +Cc: wei.yang1, mst, linux-kernel, tiwei.bie, virtualization

On Wed, Sep 15, 2021 at 2:39 PM Wu Zongyong
<wuzongyong@linux.alibaba.com> wrote:
>
> Currently we unset vq irq after freeing irq and that will result in
> error messages:

Nit: I think for 'we', it actually means the parent driver can free
the irq during reset, set_status(0). vp_vdpa is one example for this.

>
>   pi_update_irte: failed to update PI IRTE
>   irq bypass consumer (token 000000005a07a12b) unregistration fails: -22
>
> This patch solves this.
>
> Signed-off-by: Wu Zongyong <wuzongyong@linux.alibaba.com>

Other than this.

Acked-by: Jason Wang <jasowang@redhat.com>

> ---
>  drivers/vhost/vdpa.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index f41d081777f5..15bae2290bf9 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -173,6 +173,10 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
>         if (status != 0 && (ops->get_status(vdpa) & ~status) != 0)
>                 return -EINVAL;
>
> +       if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> +               for (i = 0; i < nvqs; i++)
> +                       vhost_vdpa_unsetup_vq_irq(v, i);
> +
>         if (status == 0) {
>                 ret = ops->reset(vdpa);
>                 if (ret)
> @@ -184,10 +188,6 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
>                 for (i = 0; i < nvqs; i++)
>                         vhost_vdpa_setup_vq_irq(v, i);
>
> -       if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> -               for (i = 0; i < nvqs; i++)
> -                       vhost_vdpa_unsetup_vq_irq(v, i);
> -
>         return 0;
>  }
>
> --
> 2.31.1
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vhost_vdpa: unset vq irq before freeing irq
  2021-09-15  7:52   ` Wu Zongyong
@ 2021-09-15 11:07       ` Michael S. Tsirkin
  0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2021-09-15 11:07 UTC (permalink / raw)
  To: Wu Zongyong; +Cc: jasowang, virtualization, linux-kernel, tiwei.bie, wei.yang1

On Wed, Sep 15, 2021 at 03:52:32PM +0800, Wu Zongyong wrote:
> On Wed, Sep 15, 2021 at 03:26:41AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Sep 15, 2021 at 02:39:32PM +0800, Wu Zongyong wrote:
> > > Currently we unset vq irq after freeing irq and that will result in
> > > error messages:
> > > 
> > >   pi_update_irte: failed to update PI IRTE
> > >   irq bypass consumer (token 000000005a07a12b) unregistration fails: -22
> > > 
> > > This patch solves this.
> > > 
> > > Signed-off-by: Wu Zongyong <wuzongyong@linux.alibaba.com>
> > > ---
> > >  drivers/vhost/vdpa.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> > > index f41d081777f5..15bae2290bf9 100644
> > > --- a/drivers/vhost/vdpa.c
> > > +++ b/drivers/vhost/vdpa.c
> > > @@ -173,6 +173,10 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
> > >  	if (status != 0 && (ops->get_status(vdpa) & ~status) != 0)
> > >  		return -EINVAL;
> > >  
> > > +	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> > > +		for (i = 0; i < nvqs; i++)
> > > +			vhost_vdpa_unsetup_vq_irq(v, i);
> > > +
> > 
> > If we do this before reset like this then the device might assert the
> > irq, might it not?
> > 
> This would not be a problem.
> AFAIK, vhost_vdpa_unsetup_vq_irq just disables the irq offloading, and the irq
> will be handled if there comes an irq.

Oh, right. ACK.

> > >  	if (status == 0) {
> > >  		ret = ops->reset(vdpa);
> > >  		if (ret)
> > 
> > 
> > > @@ -184,10 +188,6 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
> > >  		for (i = 0; i < nvqs; i++)
> > >  			vhost_vdpa_setup_vq_irq(v, i);
> > >  
> > > -	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> > > -		for (i = 0; i < nvqs; i++)
> > > -			vhost_vdpa_unsetup_vq_irq(v, i);
> > > -
> > >  	return 0;
> > 
> > 
> > 
> > >  }
> > >  
> > > -- 
> > > 2.31.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vhost_vdpa: unset vq irq before freeing irq
@ 2021-09-15 11:07       ` Michael S. Tsirkin
  0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2021-09-15 11:07 UTC (permalink / raw)
  To: Wu Zongyong; +Cc: wei.yang1, linux-kernel, tiwei.bie, virtualization

On Wed, Sep 15, 2021 at 03:52:32PM +0800, Wu Zongyong wrote:
> On Wed, Sep 15, 2021 at 03:26:41AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Sep 15, 2021 at 02:39:32PM +0800, Wu Zongyong wrote:
> > > Currently we unset vq irq after freeing irq and that will result in
> > > error messages:
> > > 
> > >   pi_update_irte: failed to update PI IRTE
> > >   irq bypass consumer (token 000000005a07a12b) unregistration fails: -22
> > > 
> > > This patch solves this.
> > > 
> > > Signed-off-by: Wu Zongyong <wuzongyong@linux.alibaba.com>
> > > ---
> > >  drivers/vhost/vdpa.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> > > index f41d081777f5..15bae2290bf9 100644
> > > --- a/drivers/vhost/vdpa.c
> > > +++ b/drivers/vhost/vdpa.c
> > > @@ -173,6 +173,10 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
> > >  	if (status != 0 && (ops->get_status(vdpa) & ~status) != 0)
> > >  		return -EINVAL;
> > >  
> > > +	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> > > +		for (i = 0; i < nvqs; i++)
> > > +			vhost_vdpa_unsetup_vq_irq(v, i);
> > > +
> > 
> > If we do this before reset like this then the device might assert the
> > irq, might it not?
> > 
> This would not be a problem.
> AFAIK, vhost_vdpa_unsetup_vq_irq just disables the irq offloading, and the irq
> will be handled if there comes an irq.

Oh, right. ACK.

> > >  	if (status == 0) {
> > >  		ret = ops->reset(vdpa);
> > >  		if (ret)
> > 
> > 
> > > @@ -184,10 +188,6 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
> > >  		for (i = 0; i < nvqs; i++)
> > >  			vhost_vdpa_setup_vq_irq(v, i);
> > >  
> > > -	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK))
> > > -		for (i = 0; i < nvqs; i++)
> > > -			vhost_vdpa_unsetup_vq_irq(v, i);
> > > -
> > >  	return 0;
> > 
> > 
> > 
> > >  }
> > >  
> > > -- 
> > > 2.31.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-09-15 11:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-15  6:39 [PATCH] vhost_vdpa: unset vq irq before freeing irq Wu Zongyong
2021-09-15  7:26 ` Michael S. Tsirkin
2021-09-15  7:26   ` Michael S. Tsirkin
2021-09-15  7:52   ` Wu Zongyong
2021-09-15 11:07     ` Michael S. Tsirkin
2021-09-15 11:07       ` Michael S. Tsirkin
2021-09-15  8:34 ` Jason Wang
2021-09-15  8:34   ` Jason Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.