Re: [PATCH v2 3/5] vduse: add VDUSE_GET_FEATURES ioctl

[Jason Wang]

On Thu, Feb 12, 2026 at 4:23 PM Eugenio Perez Martin
<eperezma@xxxxxxxxxx> wrote:
>
> On Thu, Feb 12, 2026 at 9:07 AM Jason Wang <jasowang@xxxxxxxxxx> wrote:
> >
> > On Tue, Feb 10, 2026 at 4:26 PM Eugenio Pérez <eperezma@xxxxxxxxxx> wrote:
> > >
> > > Add an ioctl to allow VDUSE instances to query the available features
> > > supported by the kernel module.
> > >
> > > Signed-off-by: Eugenio Pérez <eperezma@xxxxxxxxxx>
> > > ---
> > > A simple u64 bitmap is used for feature flags. While a flexible array
> > > could support indefinite expansion, 64 bits is sufficient for the
> > > foreseeable future and simplifies the implementation.
> > >
> > > Also, dev_dbg is used for logging rather than dev_err as these can be
> > > triggered from userspace.
> > > ---
> > > v2:
> > > * return -EINVAL if ioctl called with version < 2, so userland visible
> > >   reply is kept (Jason).
> > > ---
> > >  drivers/vdpa/vdpa_user/vduse_dev.c | 28 ++++++++++++++++++++++++++++
> > >  include/uapi/linux/vduse.h         |  7 ++++++-
> > >  2 files changed, 34 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > index d1da7c15d98b..7458cbaed4c7 100644
> > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > @@ -52,6 +52,9 @@
> > >
> > >  #define IRQ_UNBOUND -1
> > >
> > > +/* Supported VDUSE features */
> > > +static const uint64_t vduse_features;
> > > +
> > >  /*
> > >   * VDUSE instance have not asked the vduse API version, so assume 0.
> > >   *
> > > @@ -1947,6 +1950,19 @@ static bool vduse_validate_config(struct vduse_dev_config *config,
> > >                          sizeof(config->reserved)))
> > >                 return false;
> > >
> > > +       if (api_version < VDUSE_API_VERSION_2) {
> > > +               if (config->vduse_features) {
> > > +                       dev_dbg(vduse_ctrl_dev,
> > > +                               "config->vduse_features with version %llu",
> > > +                               api_version);
> > > +                       return false;
> > > +               }
> > > +       } else {
> > > +               if (config->vduse_features & ~vduse_features)
> > > +                       return false;
> > > +       }
> > > +
> > > +
> > >         if (api_version < VDUSE_API_VERSION_1 &&
> > >             (config->ngroups || config->nas))
> > >                 return false;
> > > @@ -2207,6 +2223,18 @@ static long vduse_ioctl(struct file *file, unsigned int cmd,
> > >                 ret = vduse_destroy_dev(name);
> > >                 break;
> > >         }
> > > +       case VDUSE_GET_FEATURES:
> > > +               if (control->api_version < VDUSE_API_VERSION_2) {
> > > +                       dev_dbg(vduse_ctrl_dev,
> > > +                               "VDUSE_GET_FEATURES ioctl with version %llu",
> > > +                               control->api_version);
> > > +                       ret = -EINVAL;
> > > +                       break;
> > > +               }
> > > +
> > > +               ret = put_user(vduse_features, (u64 __user *)argp);
> > > +               break;
> > > +
> > >         default:
> > >                 ret = -EINVAL;
> > >                 break;
> > > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> > > index 27832d46084c..8898d9daa777 100644
> > > --- a/include/uapi/linux/vduse.h
> > > +++ b/include/uapi/linux/vduse.h
> > > @@ -37,6 +37,7 @@
> > >   * @vq_align: the allocation alignment of virtqueue's metadata
> > >   * @ngroups: number of vq groups that VDUSE device declares
> > >   * @nas: number of address spaces that VDUSE device declares
> > > + * @vduse_features: VDUSE features
> > >   * @reserved: for future use, needs to be initialized to zero
> > >   * @config_size: the size of the configuration space
> > >   * @config: the buffer of the configuration space
> > > @@ -53,7 +54,8 @@ struct vduse_dev_config {
> > >         __u32 vq_align;
> > >         __u32 ngroups; /* if VDUSE_API_VERSION >= 1 */
> > >         __u32 nas; /* if VDUSE_API_VERSION >= 1 */
> > > -       __u32 reserved[11];
> > > +       __u64 vduse_features;
> >
> > Nit: Should we document " /* if VDUSE_API_VERSION >= 2 */
> >
> > But I have an open question, is this better to just introduce a
> > VDUSE_SET_FEATURES then we can do negotiation to avoid bumping API
> > versions?
> >
>
> How do you know if you can call VDUSE_GET_FEATURES and
> VDUSE_SET_FEATURES then? vduse_ioctl does not even return -ENOIOCMD
> but -EINVAL if you call the equivalent of VDUSE_GET_FEATURES on
> current Linux master. Or should the VDUSE instance assume that if the
> ioctl returns -EINVAL the kernel does not support them?

If we go that way, yes.

Thanks

>