diff mbox series

[v2,2/4] vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature

Message ID 20230609092127.170673-3-eperezma@redhat.com
State New
Headers
Series Add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag to vdpa backend |

Commit Message

Eugenio Perez Martin June 9, 2023, 9:21 a.m. UTC 
  Accepting VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature if
userland sets it.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Shannon Nelson <shannon.nelson@amd.com>
---
 drivers/vhost/vdpa.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Michael S. Tsirkin June 9, 2023, 4:13 p.m. UTC | #1 
On Fri, Jun 09, 2023 at 11:21:25AM +0200, Eugenio Pérez wrote:
> Accepting VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature if
> userland sets it.
> 
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> Acked-by: Shannon Nelson <shannon.nelson@amd.com>

I don't get it, so all vdpa devices accept this automatically?
Should this not be up to the parent?

> ---
>  drivers/vhost/vdpa.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index bf77924d5b60..a3204406b73d 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -680,7 +680,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>  			return -EFAULT;
>  		if (features & ~(VHOST_VDPA_BACKEND_FEATURES |
>  				 BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
> -				 BIT_ULL(VHOST_BACKEND_F_RESUME)))
> +				 BIT_ULL(VHOST_BACKEND_F_RESUME) |
> +				 BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK)))
>  			return -EOPNOTSUPP;
>  		if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) &&
>  		     !vhost_vdpa_can_suspend(v))
> -- 
> 2.31.1
Eugenio Perez Martin July 3, 2023, 8:21 a.m. UTC | #2 
On Fri, Jun 9, 2023 at 6:13 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Fri, Jun 09, 2023 at 11:21:25AM +0200, Eugenio Pérez wrote:
> > Accepting VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature if
> > userland sets it.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > Acked-by: Shannon Nelson <shannon.nelson@amd.com>
>
> I don't get it, so all vdpa devices accept this automatically?
> Should this not be up to the parent?
>

At the moment I don't see a reason why if a parent offers this
feature, it could reject it afterwards. However I think we can add a
fail if userland acks the backend feature but the parent does not
offer it however.

Would it work to add such fail in vdpa frontend and move it to the
backend if and when any parent driver needs it in the future?

Thanks!

> > ---
> >  drivers/vhost/vdpa.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> > index bf77924d5b60..a3204406b73d 100644
> > --- a/drivers/vhost/vdpa.c
> > +++ b/drivers/vhost/vdpa.c
> > @@ -680,7 +680,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> >                       return -EFAULT;
> >               if (features & ~(VHOST_VDPA_BACKEND_FEATURES |
> >                                BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
> > -                              BIT_ULL(VHOST_BACKEND_F_RESUME)))
> > +                              BIT_ULL(VHOST_BACKEND_F_RESUME) |
> > +                              BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK)))
> >                       return -EOPNOTSUPP;
> >               if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) &&
> >                    !vhost_vdpa_can_suspend(v))
> > --
> > 2.31.1
>
diff mbox series

Patch

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index bf77924d5b60..a3204406b73d 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -680,7 +680,8 @@  static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 			return -EFAULT;
 		if (features & ~(VHOST_VDPA_BACKEND_FEATURES |
 				 BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
-				 BIT_ULL(VHOST_BACKEND_F_RESUME)))
+				 BIT_ULL(VHOST_BACKEND_F_RESUME) |
+				 BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK)))
 			return -EOPNOTSUPP;
 		if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) &&
 		     !vhost_vdpa_can_suspend(v))