[PATCH 0/2] vfio: Implement device features for vfio-user

[PATCH 0/2] vfio: Implement device features for vfio-user

[John Levon]

John Levon (2):
  vfio-user: support VFIO_USER_DEVICE_FEATURE
  vfio: quieten dma-buf warning

 hw/vfio-user/protocol.h | 12 ++++++++++++
 hw/vfio-user/device.c   | 39 +++++++++++++++++++++++++++++++++++++++
 hw/vfio/region.c        |  2 --
 3 files changed, 51 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH 1/2] vfio-user: support VFIO_USER_DEVICE_FEATURE

[John Levon]

Plumb through vfio_device_get_feature to the vfio-user server. Note that
we translate EINVAL into ENOTTY, as the existing generic vfio code is
expecting the latter to mean "unsupported".

Signed-off-by: John Levon <john.levon@nutanix.com>
---
 hw/vfio-user/protocol.h | 12 ++++++++++++
 hw/vfio-user/device.c   | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/hw/vfio-user/protocol.h b/hw/vfio-user/protocol.h
index 3249a4a6b6..2a0c31e7c5 100644
--- a/hw/vfio-user/protocol.h
+++ b/hw/vfio-user/protocol.h
@@ -40,6 +40,7 @@ enum vfio_user_command {
     VFIO_USER_DEVICE_RESET              = 13,
     VFIO_USER_DIRTY_PAGES               = 14,
     VFIO_USER_REGION_WRITE_MULTI        = 15,
+    VFIO_USER_DEVICE_FEATURE            = 16,
     VFIO_USER_MAX,
 };
 
@@ -239,4 +240,15 @@ typedef struct {
     VFIOUserWROne wrs[VFIO_USER_MULTI_MAX];
 } VFIOUserWRMulti;
 
+/*
+ * VFIO_USER_DEVICE_FEATURE
+ * imported from struct vfio_device_feature
+ */
+typedef struct {
+    VFIOUserHdr hdr;
+    uint32_t argsz;
+    uint32_t flags;
+    char data[];
+} VFIOUserDeviceFeature;
+
 #endif /* VFIO_USER_PROTOCOL_H */
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 64ef35b320..6910d183d7 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -74,6 +74,44 @@ void vfio_user_device_reset(VFIOUserProxy *proxy)
     }
 }
 
+static int
+vfio_user_device_io_device_feature(VFIODevice *vbasedev,
+                                   struct vfio_device_feature *feature)
+{
+    g_autofree VFIOUserDeviceFeature *msgp = NULL;
+    int size = sizeof(VFIOUserHdr) + feature->argsz;
+    VFIOUserProxy *proxy = vbasedev->proxy;
+    Error *local_err = NULL;
+
+    msgp = g_malloc0(size);
+
+    vfio_user_request_msg(&msgp->hdr, VFIO_USER_DEVICE_FEATURE, size, 0);
+
+    memcpy(&msgp->argsz, &feature->argsz, feature->argsz);
+
+    if (!vfio_user_send_wait(proxy, &msgp->hdr, NULL, size, &local_err)) {
+        error_prepend(&local_err, "%s: ", __func__);
+        error_report_err(local_err);
+        return -EFAULT;
+    }
+
+    if (msgp->hdr.flags & VFIO_USER_ERROR) {
+        /*
+         * Client expects ENOTTY for "not supported", but the protocol may
+         * return EINVAL.
+         */
+        if (msgp->hdr.error_reply == EINVAL) {
+            return -ENOTTY;
+        }
+
+        return -msgp->hdr.error_reply;
+    }
+
+    memcpy(feature, &msgp->argsz, feature->argsz);
+
+    return 0;
+}
+
 static int vfio_user_get_region_info(VFIOUserProxy *proxy,
                                      struct vfio_region_info *info,
                                      VFIOUserFDs *fds)
@@ -432,6 +470,7 @@ static int vfio_user_device_io_region_write(VFIODevice *vbasedev, uint8_t index,
  * Socket-based io_ops
  */
 VFIODeviceIOOps vfio_user_device_io_ops_sock = {
+    .device_feature = vfio_user_device_io_device_feature,
     .get_region_info = vfio_user_device_io_get_region_info,
     .get_irq_info = vfio_user_device_io_get_irq_info,
     .set_irqs = vfio_user_device_io_set_irqs,
-- 
2.43.0

Re: [PATCH 1/2] vfio-user: support VFIO_USER_DEVICE_FEATURE

[Cédric Le Goater]

On 3/17/26 20:53, John Levon wrote:
> Plumb through vfio_device_get_feature to the vfio-user server. Note that
> we translate EINVAL into ENOTTY, as the existing generic vfio code is
> expecting the latter to mean "unsupported".
> 
> Signed-off-by: John Levon <john.levon@nutanix.com>
> ---
>   hw/vfio-user/protocol.h | 12 ++++++++++++
>   hw/vfio-user/device.c   | 39 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 51 insertions(+)
> 
> diff --git a/hw/vfio-user/protocol.h b/hw/vfio-user/protocol.h
> index 3249a4a6b6..2a0c31e7c5 100644
> --- a/hw/vfio-user/protocol.h
> +++ b/hw/vfio-user/protocol.h
> @@ -40,6 +40,7 @@ enum vfio_user_command {
>       VFIO_USER_DEVICE_RESET              = 13,
>       VFIO_USER_DIRTY_PAGES               = 14,
>       VFIO_USER_REGION_WRITE_MULTI        = 15,
> +    VFIO_USER_DEVICE_FEATURE            = 16,
>       VFIO_USER_MAX,
>   };
>   
> @@ -239,4 +240,15 @@ typedef struct {
>       VFIOUserWROne wrs[VFIO_USER_MULTI_MAX];
>   } VFIOUserWRMulti;
>   
> +/*
> + * VFIO_USER_DEVICE_FEATURE
> + * imported from struct vfio_device_feature
> + */
> +typedef struct {
> +    VFIOUserHdr hdr;
> +    uint32_t argsz;
> +    uint32_t flags;
> +    char data[];
> +} VFIOUserDeviceFeature;
> +
>   #endif /* VFIO_USER_PROTOCOL_H */
> diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
> index 64ef35b320..6910d183d7 100644
> --- a/hw/vfio-user/device.c
> +++ b/hw/vfio-user/device.c
> @@ -74,6 +74,44 @@ void vfio_user_device_reset(VFIOUserProxy *proxy)
>       }
>   }
>   
> +static int
> +vfio_user_device_io_device_feature(VFIODevice *vbasedev,
> +                                   struct vfio_device_feature *feature)
> +{
> +    g_autofree VFIOUserDeviceFeature *msgp = NULL;
> +    int size = sizeof(VFIOUserHdr) + feature->argsz;
> +    VFIOUserProxy *proxy = vbasedev->proxy;
> +    Error *local_err = NULL;
> +
> +    msgp = g_malloc0(size);
> +
> +    vfio_user_request_msg(&msgp->hdr, VFIO_USER_DEVICE_FEATURE, size, 0);
> +
> +    memcpy(&msgp->argsz, &feature->argsz, feature->argsz);
> +
> +    if (!vfio_user_send_wait(proxy, &msgp->hdr, NULL, size, &local_err)) {
> +        error_prepend(&local_err, "%s: ", __func__);
> +        error_report_err(local_err);
> +        return -EFAULT;
> +    }
> +
> +    if (msgp->hdr.flags & VFIO_USER_ERROR) {
> +        /*
> +         * Client expects ENOTTY for "not supported", but the protocol may
> +         * return EINVAL.
> +         */

Is this errno tanslation due to the call in vfio_region_create_dma_buf() ?

EINVAL could indicate other issues. Is is safe to mask it ?


> +        if (msgp->hdr.error_reply == EINVAL) {
> +            return -ENOTTY;
> +        }
> +
> +        return -msgp->hdr.error_reply;
> +    }


We could add a trace event here ?

Thanks,

C.

> +    memcpy(feature, &msgp->argsz, feature->argsz);
> +
> +    return 0;
> +}
> +
>   static int vfio_user_get_region_info(VFIOUserProxy *proxy,
>                                        struct vfio_region_info *info,
>                                        VFIOUserFDs *fds)
> @@ -432,6 +470,7 @@ static int vfio_user_device_io_region_write(VFIODevice *vbasedev, uint8_t index,
>    * Socket-based io_ops
>    */
>   VFIODeviceIOOps vfio_user_device_io_ops_sock = {
> +    .device_feature = vfio_user_device_io_device_feature,
>       .get_region_info = vfio_user_device_io_get_region_info,
>       .get_irq_info = vfio_user_device_io_get_irq_info,
>       .set_irqs = vfio_user_device_io_set_irqs,

Re: [PATCH 1/2] vfio-user: support VFIO_USER_DEVICE_FEATURE

[John Levon]

On Wed, Apr 08, 2026 at 09:42:30AM +0200, Cédric Le Goater wrote:

> > +    if (msgp->hdr.flags & VFIO_USER_ERROR) {
> > +        /*
> > +         * Client expects ENOTTY for "not supported", but the protocol may
> > +         * return EINVAL.
> > +         */
> 
> Is this errno tanslation due to the call in vfio_region_create_dma_buf() ?

Yes.

> EINVAL could indicate other issues. Is is safe to mask it ?

Yes - this should only occur otherwise in the case of incorrect protocol
messages (i.e. qemu bugs).

regards
john

[PATCH 2/2] vfio: quieten dma-buf warning

[John Levon]

Don't report this error every time, as it is specific to the kernel vfio
implementation; we shouldn't warn about things that always happen, such
as when this is backed by vfio-user.

Signed-off-by: John Levon <john.levon@nutanix.com>
---
 hw/vfio/region.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/vfio/region.c b/hw/vfio/region.c
index 47fdc2df34..1d442f3f48 100644
--- a/hw/vfio/region.c
+++ b/hw/vfio/region.c
@@ -316,8 +316,6 @@ static bool vfio_region_create_dma_buf(VFIORegion *region, Error **errp)
     ret = vfio_device_get_feature(vbasedev, feature);
     if (ret < 0) {
         if (ret == -ENOTTY) {
-            warn_report_once("VFIO dma-buf not supported in kernel: "
-                             "PCI BAR IOMMU mappings may fail");
             return true;
         }
         /* P2P DMA or exposing device memory use cases are not supported. */
-- 
2.43.0

Re: [PATCH 2/2] vfio: quieten dma-buf warning

[Cédric Le Goater]

On 3/17/26 20:53, John Levon wrote:
> Don't report this error every time, 

warn_report_once() is not every time.

> as it is specific to the kernel vfio
> implementation; we shouldn't warn about things that always happen, such
> as when this is backed by vfio-user.

What bothers you is the "... kernel: " message ?

We could use (!vbasedev->proxy) to change the "kernel" string to
an another one related to the device implementation. What do you
think ?

Thanks,

C.

> Signed-off-by: John Levon <john.levon@nutanix.com>
> ---
>   hw/vfio/region.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/hw/vfio/region.c b/hw/vfio/region.c
> index 47fdc2df34..1d442f3f48 100644
> --- a/hw/vfio/region.c
> +++ b/hw/vfio/region.c
> @@ -316,8 +316,6 @@ static bool vfio_region_create_dma_buf(VFIORegion *region, Error **errp)
>       ret = vfio_device_get_feature(vbasedev, feature);
>       if (ret < 0) {
>           if (ret == -ENOTTY) {
> -            warn_report_once("VFIO dma-buf not supported in kernel: "
> -                             "PCI BAR IOMMU mappings may fail");
>               return true;
>           }
>           /* P2P DMA or exposing device memory use cases are not supported. */

Re: [PATCH 2/2] vfio: quieten dma-buf warning

[John Levon]

On Wed, Apr 08, 2026 at 09:52:53AM +0200, Cédric Le Goater wrote:

> On 3/17/26 20:53, John Levon wrote:
> > Don't report this error every time,
> 
> warn_report_once() is not every time.

It's every time the VM boots for an obscure feature that is totally irrelevant
to non-kernel-vfio backends.

> > as it is specific to the kernel vfio
> > implementation; we shouldn't warn about things that always happen, such
> > as when this is backed by vfio-user.
> 
> What bothers you is the "... kernel: " message ?

The existence of an incorrect and meaningless warning is my complaint.

> We could use (!vbasedev->proxy) to change the "kernel" string to
> an another one related to the device implementation. What do you
> think ?

If I'm "allowed" to check for this to suppress the message entirely, then works
for me.

regards
john

Re: [PATCH 2/2] vfio: quieten dma-buf warning

[Cédric Le Goater]

On 4/8/26 10:57, John Levon wrote:
> On Wed, Apr 08, 2026 at 09:52:53AM +0200, Cédric Le Goater wrote:
> 
>> On 3/17/26 20:53, John Levon wrote:
>>> Don't report this error every time,
>>
>> warn_report_once() is not every time.
> 
> It's every time the VM boots for an obscure feature that is totally irrelevant
> to non-kernel-vfio backends.

I agree. It's irrelevant for vfio-user.
  
>>> as it is specific to the kernel vfio
>>> implementation; we shouldn't warn about things that always happen, such
>>> as when this is backed by vfio-user.
>>
>> What bothers you is the "... kernel: " message ?
> 
> The existence of an incorrect and meaningless warning is my complaint.

I sense your irritation from across the Channel :)

>> We could use (!vbasedev->proxy) to change the "kernel" string to
>> an another one related to the device implementation. What do you
>> think ?
> 
> If I'm "allowed" to check for this to suppress the message entirely, then works
> for me.


VFIO_DEVICE_FEATURE_DMA_BUF should be available in Linux v6.19 and
QEMU 11.0. It is an important feature for large GPUs, PCI P2P DMAs
and most entreprise distros should have a KVM/VFIO and QEMU in sync
for it. The lack of dmabuf support is not fatal but it deserves some
"warning". I'd rather keep it that way.

The vfio-user implementation doesn't care. I agree. Sorry it was not
my focus. Anyhow, this raises an interesting question on the lack of
support of a feature for a specific backend.

I see 2 possible approaches,

1. quick one:

   Return -ENOTSUP and test the value in vfio_region_create_dma_buf().
   It's cleaner than masking EINVAL IMO.

2. cleaner one:

   Add a capability field to VFIODeviceIOOps to advertise supported
   features:
                                                              
     #define VFIO_IO_CAP_DMA_BUF        (1 << 0)
     #define VFIO_IO_CAP_MIGRATION      (1 << 1)
                                                                    
     struct VFIODeviceIOOps {
       uint64_t capabilities;  /* Bitmask of VFIO_IO_CAP_* */
       ..
                                                                   
and test vbasedev->io_ops->capabilities where needed.

How's that ? I can implement option 2.

Thanks,

C.

Re: [PATCH 2/2] vfio: quieten dma-buf warning

[John Levon]

On Thu, Apr 09, 2026 at 11:53:58AM +0200, Cédric Le Goater wrote:

> > The existence of an incorrect and meaningless warning is my complaint.
> 
> I sense your irritation from across the Channel :)

:)

> 1. quick one:
> 
>   Return -ENOTSUP and test the value in vfio_region_create_dma_buf().
>   It's cleaner than masking EINVAL IMO.

Let me do this one now, and we can look at the capability one later perhaps? You
mention migration but I want to get that working, so might be a bit silly just
for one error message.

regards
john

Re: [PATCH 2/2] vfio: quieten dma-buf warning

[John Levon]

On Thu, Apr 09, 2026 at 11:53:58AM +0200, Cédric Le Goater wrote:

> 1. quick one:
> 
>   Return -ENOTSUP and test the value in vfio_region_create_dma_buf().
>   It's cleaner than masking EINVAL IMO.

Scratch that, vfio can return this error too:

220         if (!vdev->pci_ops || !vdev->pci_ops->get_dmabuf_phys)
221                 return -EOPNOTSUPP;

> 2. cleaner one:
> 
>   Add a capability field to VFIODeviceIOOps to advertise supported
>   features:
>     #define VFIO_IO_CAP_DMA_BUF        (1 << 0)
>     #define VFIO_IO_CAP_MIGRATION      (1 << 1)
>     struct VFIODeviceIOOps {
>       uint64_t capabilities;  /* Bitmask of VFIO_IO_CAP_* */
>       ..
> and test vbasedev->io_ops->capabilities where needed.
> 
> How's that ? I can implement option 2.

I'd appreciate it, thanks, please let me know.

regards
john