[PATCH 0/3] vfio-user fixes

[PATCH 0/3] vfio-user fixes

[John Levon]

John Levon (3):
  vfio-user: support VFIO_USER_DEVICE_FEATURE
  vfio-user: correct protocol for DMA reads/writes
  vfio-user: fix DMA write reply

 docs/interop/vfio-user.rst |  8 +++++++-
 hw/vfio-user/protocol.h    | 14 ++++++++++++-
 hw/vfio-user/device.c      | 42 ++++++++++++++++++++++++++++++++++++++
 hw/vfio-user/pci.c         | 30 +++++++++++++++++++--------
 hw/vfio-user/trace-events  | 23 ++++++++++++---------
 5 files changed, 97 insertions(+), 20 deletions(-)

-- 
2.43.0

[PATCH 1/3] 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".

As part of adding a trace point, clean up the trace file.

Signed-off-by: John Levon <john.levon@nutanix.com>
---
 hw/vfio-user/protocol.h   | 12 +++++++++++
 hw/vfio-user/device.c     | 42 +++++++++++++++++++++++++++++++++++++++
 hw/vfio-user/trace-events | 23 +++++++++++----------
 3 files changed, 67 insertions(+), 10 deletions(-)

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..b8d2b7c1a8 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -74,6 +74,47 @@ 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 (which should only occur in the case the feature isn't
+         * actually supported on the server).
+         */
+        if (msgp->hdr.error_reply == EINVAL) {
+            return -ENOTTY;
+        }
+
+        return -msgp->hdr.error_reply;
+    }
+
+    memcpy(feature, &msgp->argsz, feature->argsz);
+
+    trace_vfio_user_device_io_device_feature(msgp->argsz, msgp->flags);
+
+    return 0;
+}
+
 static int vfio_user_get_region_info(VFIOUserProxy *proxy,
                                      struct vfio_region_info *info,
                                      VFIOUserFDs *fds)
@@ -432,6 +473,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,
diff --git a/hw/vfio-user/trace-events b/hw/vfio-user/trace-events
index abb67f4c11..84714781cf 100644
--- a/hw/vfio-user/trace-events
+++ b/hw/vfio-user/trace-events
@@ -2,19 +2,22 @@
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 
-# common.c
+# container.c
+vfio_user_dma_map(uint64_t iova, uint64_t size, uint64_t off, uint32_t flags, bool async_ops) " iova 0x%"PRIx64" size 0x%"PRIx64" off 0x%"PRIx64" flags 0x%x async_ops %d"
+vfio_user_dma_unmap(uint64_t iova, uint64_t size, uint32_t flags,  bool async_ops) " iova 0x%"PRIx64" size 0x%"PRIx64" flags 0x%x async_ops %d"
+
+# device.c
+vfio_user_device_io_device_feature(uint32_t argsz, uint32_t flags);
+vfio_user_get_info(uint32_t nregions, uint32_t nirqs) " #regions %d #irqs %d"
+vfio_user_get_irq_info(uint32_t index, uint32_t flags, uint32_t count) " index %d flags 0x%x count %d"
+vfio_user_set_irqs(uint32_t index, uint32_t start, uint32_t count, uint32_t flags) " index %d start %d count %d flags 0x%x"
+vfio_user_get_region_info(uint32_t index, uint32_t flags, uint64_t size) " index %d flags 0x%x size 0x%"PRIx64
+vfio_user_region_rw(uint32_t region, uint64_t off, uint32_t count) " region %d offset 0x%"PRIx64" count %d"
+
+# proxy.c
 vfio_user_recv_hdr(const char *name, uint16_t id, uint16_t cmd, uint32_t size, uint32_t flags) " (%s) id 0x%x cmd 0x%x size 0x%x flags 0x%x"
 vfio_user_recv_read(uint16_t id, int read) " id 0x%x read 0x%x"
 vfio_user_recv_request(uint16_t cmd) " command 0x%x"
 vfio_user_send_write(uint16_t id, int wrote) " id 0x%x wrote 0x%x"
 vfio_user_version(uint16_t major, uint16_t minor, const char *caps) " major %d minor %d caps: %s"
-vfio_user_get_info(uint32_t nregions, uint32_t nirqs) " #regions %d #irqs %d"
-vfio_user_get_region_info(uint32_t index, uint32_t flags, uint64_t size) " index %d flags 0x%x size 0x%"PRIx64
-vfio_user_region_rw(uint32_t region, uint64_t off, uint32_t count) " region %d offset 0x%"PRIx64" count %d"
-vfio_user_get_irq_info(uint32_t index, uint32_t flags, uint32_t count) " index %d flags 0x%x count %d"
-vfio_user_set_irqs(uint32_t index, uint32_t start, uint32_t count, uint32_t flags) " index %d start %d count %d flags 0x%x"
 vfio_user_wrmulti(const char *s, uint64_t wr_cnt) " %s count 0x%"PRIx64
-
-# container.c
-vfio_user_dma_map(uint64_t iova, uint64_t size, uint64_t off, uint32_t flags, bool async_ops) " iova 0x%"PRIx64" size 0x%"PRIx64" off 0x%"PRIx64" flags 0x%x async_ops %d"
-vfio_user_dma_unmap(uint64_t iova, uint64_t size, uint32_t flags,  bool async_ops) " iova 0x%"PRIx64" size 0x%"PRIx64" flags 0x%x async_ops %d"
-- 
2.43.0

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

[Cédric Le Goater]

On 4/9/26 12:47, 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".
> 
> As part of adding a trace point, clean up the trace file.
> 
> Signed-off-by: John Levon <john.levon@nutanix.com>
> ---
>   hw/vfio-user/protocol.h   | 12 +++++++++++
>   hw/vfio-user/device.c     | 42 +++++++++++++++++++++++++++++++++++++++
>   hw/vfio-user/trace-events | 23 +++++++++++----------
>   3 files changed, 67 insertions(+), 10 deletions(-)
> 
> 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..b8d2b7c1a8 100644
> --- a/hw/vfio-user/device.c
> +++ b/hw/vfio-user/device.c
> @@ -74,6 +74,47 @@ 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 (which should only occur in the case the feature isn't
> +         * actually supported on the server).
> +         */
> +        if (msgp->hdr.error_reply == EINVAL) {
> +            return -ENOTTY;
> +        }

May be this is not necessary anymore with :

   https://lore.kernel.org/qemu-devel/20260409114312.1704062-1-clg@redhat.com/

> +
> +        return -msgp->hdr.error_reply;
> +    }
> +
> +    memcpy(feature, &msgp->argsz, feature->argsz);
> +
> +    trace_vfio_user_device_io_device_feature(msgp->argsz, msgp->flags);
> +
> +    return 0;
> +}
> +
>   static int vfio_user_get_region_info(VFIOUserProxy *proxy,
>                                        struct vfio_region_info *info,
>                                        VFIOUserFDs *fds)
> @@ -432,6 +473,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,
> diff --git a/hw/vfio-user/trace-events b/hw/vfio-user/trace-events
> index abb67f4c11..84714781cf 100644
> --- a/hw/vfio-user/trace-events
> +++ b/hw/vfio-user/trace-events
> @@ -2,19 +2,22 @@
>   #
>   # SPDX-License-Identifier: GPL-2.0-or-later
>   
> -# common.c
> +# container.c
> +vfio_user_dma_map(uint64_t iova, uint64_t size, uint64_t off, uint32_t flags, bool async_ops) " iova 0x%"PRIx64" size 0x%"PRIx64" off 0x%"PRIx64" flags 0x%x async_ops %d"
> +vfio_user_dma_unmap(uint64_t iova, uint64_t size, uint32_t flags,  bool async_ops) " iova 0x%"PRIx64" size 0x%"PRIx64" flags 0x%x async_ops %d"
> +
> +# device.c
> +vfio_user_device_io_device_feature(uint32_t argsz, uint32_t flags);

missing format here ^


Thanks,

C.


> +vfio_user_get_info(uint32_t nregions, uint32_t nirqs) " #regions %d #irqs %d"
> +vfio_user_get_irq_info(uint32_t index, uint32_t flags, uint32_t count) " index %d flags 0x%x count %d"
> +vfio_user_set_irqs(uint32_t index, uint32_t start, uint32_t count, uint32_t flags) " index %d start %d count %d flags 0x%x"
> +vfio_user_get_region_info(uint32_t index, uint32_t flags, uint64_t size) " index %d flags 0x%x size 0x%"PRIx64
> +vfio_user_region_rw(uint32_t region, uint64_t off, uint32_t count) " region %d offset 0x%"PRIx64" count %d"
> +
> +# proxy.c
>   vfio_user_recv_hdr(const char *name, uint16_t id, uint16_t cmd, uint32_t size, uint32_t flags) " (%s) id 0x%x cmd 0x%x size 0x%x flags 0x%x"
>   vfio_user_recv_read(uint16_t id, int read) " id 0x%x read 0x%x"
>   vfio_user_recv_request(uint16_t cmd) " command 0x%x"
>   vfio_user_send_write(uint16_t id, int wrote) " id 0x%x wrote 0x%x"
>   vfio_user_version(uint16_t major, uint16_t minor, const char *caps) " major %d minor %d caps: %s"
> -vfio_user_get_info(uint32_t nregions, uint32_t nirqs) " #regions %d #irqs %d"
> -vfio_user_get_region_info(uint32_t index, uint32_t flags, uint64_t size) " index %d flags 0x%x size 0x%"PRIx64
> -vfio_user_region_rw(uint32_t region, uint64_t off, uint32_t count) " region %d offset 0x%"PRIx64" count %d"
> -vfio_user_get_irq_info(uint32_t index, uint32_t flags, uint32_t count) " index %d flags 0x%x count %d"
> -vfio_user_set_irqs(uint32_t index, uint32_t start, uint32_t count, uint32_t flags) " index %d start %d count %d flags 0x%x"
>   vfio_user_wrmulti(const char *s, uint64_t wr_cnt) " %s count 0x%"PRIx64
> -
> -# container.c
> -vfio_user_dma_map(uint64_t iova, uint64_t size, uint64_t off, uint32_t flags, bool async_ops) " iova 0x%"PRIx64" size 0x%"PRIx64" off 0x%"PRIx64" flags 0x%x async_ops %d"
> -vfio_user_dma_unmap(uint64_t iova, uint64_t size, uint32_t flags,  bool async_ops) " iova 0x%"PRIx64" size 0x%"PRIx64" flags 0x%x async_ops %d"

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

[John Levon]

On Thu, Apr 09, 2026 at 01:50:58PM +0200, Cédric Le Goater wrote:

> > +        /*
> > +         * Client expects ENOTTY for "not supported", but the protocol may
> > +         * return EINVAL (which should only occur in the case the feature isn't
> > +         * actually supported on the server).
> > +         */
> > +        if (msgp->hdr.error_reply == EINVAL) {
> > +            return -ENOTTY;
> > +        }
> 
> May be this is not necessary anymore with :
> 
>   https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_qemu-2Ddevel_20260409114312.1704062-2D1-2Dclg-40redhat.com_&d=DwICaQ&c=s883GpUCOChKOHiocYtGcg&r=v7SNLJqx7b9Vfc7ZO82Wg4nnZ8O5XkACFQ30bVKxotI&m=lpMrwUH_bH0I_KS0ucNhIf9acrnTdwuaRPGfJXRAj2yImpt5jpWGpP82gP4MJmRZ&s=9mI6zaw_Kt3ZLTto-WtxWPFKpQrqx-H7KpgUyvxnWEQ&e=

I think it's still necessary in general - ENOTTY is what kernel vfio returns so
we need to align for that for any other use cases.

> > +
> > +# device.c
> > +vfio_user_device_io_device_feature(uint32_t argsz, uint32_t flags);
> 
> missing format here ^

Oops, surprised I didn't get errors with this

regards
john

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

[Cédric Le Goater]

>>> +# device.c
>>> +vfio_user_device_io_device_feature(uint32_t argsz, uint32_t flags);
>>
>> missing format here ^
> 
> Oops, surprised I didn't get errors with this

Can you please resend ? I am preparing the vfio queue for the
next cycle.

Thanks,

C.

[PATCH 2/3] vfio-user: correct protocol for DMA reads/writes

[John Levon]

The "count" parameter in the DMA write reply should be 8 bytes in size.

Reported-by: Patrick Mooney <patrick@matx.com>
Signed-off-by: John Levon <john.levon@nutanix.com>
---
 docs/interop/vfio-user.rst | 2 +-
 hw/vfio-user/protocol.h    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/interop/vfio-user.rst b/docs/interop/vfio-user.rst
index 303fc13d6e..d4766487ea 100644
--- a/docs/interop/vfio-user.rst
+++ b/docs/interop/vfio-user.rst
@@ -1463,7 +1463,7 @@ Reply
 +=========+========+==========+
 | address | 0      | 8        |
 +---------+--------+----------+
-| count   | 8      | 4        |
+| count   | 8      | 8        |
 +---------+--------+----------+
 
 * *address* is the client DMA memory address being accessed.
diff --git a/hw/vfio-user/protocol.h b/hw/vfio-user/protocol.h
index 2a0c31e7c5..c478d1353f 100644
--- a/hw/vfio-user/protocol.h
+++ b/hw/vfio-user/protocol.h
@@ -210,7 +210,7 @@ typedef struct {
 typedef struct {
     VFIOUserHdr hdr;
     uint64_t offset;
-    uint32_t count;
+    uint64_t count;
     char data[];
 } VFIOUserDMARW;
 
-- 
2.43.0

[PATCH 3/3] vfio-user: fix DMA write reply

[John Levon]

The protocol specifies that DMA write replies should include
address+count, but the client code was only doing so for read. Fix that
up.

In addition, add a protocol clarification over how short writes may be
reported in that reply. QEMU never reports a short write via the
count field.

Reported-by: Patrick Mooney <patrick@matx.com>
Signed-off-by: John Levon <john.levon@nutanix.com>
---
 docs/interop/vfio-user.rst |  6 ++++++
 hw/vfio-user/pci.c         | 30 ++++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/docs/interop/vfio-user.rst b/docs/interop/vfio-user.rst
index d4766487ea..12deb25102 100644
--- a/docs/interop/vfio-user.rst
+++ b/docs/interop/vfio-user.rst
@@ -1429,6 +1429,9 @@ Reply
 * *count* is the size of the data transferred.
 * *data* is the data read.
 
+Note that whether short reads return an error or just set count appropriately is
+a client-side choice; servers should be prepared to handle both cases.
+
 ``VFIO_USER_DMA_WRITE``
 -----------------------
 
@@ -1469,6 +1472,9 @@ Reply
 * *address* is the client DMA memory address being accessed.
 * *count* is the size of the data transferred.
 
+Note that whether short writes return an error or just set count appropriately
+is a client-side choice; servers should be prepared to handle both cases.
+
 ``VFIO_USER_DEVICE_RESET``
 --------------------------
 
diff --git a/hw/vfio-user/pci.c b/hw/vfio-user/pci.c
index 64b8b3cb8c..facc79727a 100644
--- a/hw/vfio-user/pci.c
+++ b/hw/vfio-user/pci.c
@@ -109,6 +109,10 @@ static void vfio_user_dma_read(VFIOPCIDevice *vdev, VFIOUserDMARW *msg)
 
     r = pci_dma_read(pdev, res->offset, &res->data, res->count);
 
+    /*
+     * pci_dma_read() doesn't support reporting short reads via the reply's
+     * count parameter; in this case, we'll reply with an error instead.
+     */
     switch (r) {
     case MEMTX_OK:
         if (res->hdr.flags & VFIO_USER_NO_REPLY) {
@@ -136,6 +140,7 @@ static void vfio_user_dma_write(VFIOPCIDevice *vdev, VFIOUserDMARW *msg)
 {
     PCIDevice *pdev = PCI_DEVICE(vdev);
     VFIOUserProxy *proxy = vdev->vbasedev.proxy;
+    VFIOUserDMARW *res;
     MemTxResult r;
 
     if (msg->hdr.size < sizeof(*msg)) {
@@ -150,26 +155,35 @@ static void vfio_user_dma_write(VFIOPCIDevice *vdev, VFIOUserDMARW *msg)
 
     r = pci_dma_write(pdev, msg->offset, &msg->data, msg->count);
 
+    res = g_malloc0(sizeof(*res));
+    memcpy(res, msg, sizeof(*res));
+    g_free(msg);
+
+    /*
+     * pci_dma_write() doesn't support reporting short writes via the reply's
+     * count parameter; in this case, we'll reply with an error instead.
+     */
     switch (r) {
     case MEMTX_OK:
-        if ((msg->hdr.flags & VFIO_USER_NO_REPLY) == 0) {
-            vfio_user_send_reply(proxy, &msg->hdr, sizeof(msg->hdr));
-        } else {
-            g_free(msg);
+        if (res->hdr.flags & VFIO_USER_NO_REPLY) {
+            g_free(res);
+            return;
         }
+
+        vfio_user_send_reply(proxy, &res->hdr, sizeof(*res));
         break;
     case MEMTX_ERROR:
-        vfio_user_send_error(proxy, &msg->hdr, EFAULT);
+        vfio_user_send_error(proxy, &res->hdr, EFAULT);
         break;
     case MEMTX_DECODE_ERROR:
-        vfio_user_send_error(proxy, &msg->hdr, ENODEV);
+        vfio_user_send_error(proxy, &res->hdr, ENODEV);
         break;
     case MEMTX_ACCESS_ERROR:
-        vfio_user_send_error(proxy, &msg->hdr, EPERM);
+        vfio_user_send_error(proxy, &res->hdr, EPERM);
         break;
     default:
         error_printf("vfio_user_dma_write unknown error %d\n", r);
-        vfio_user_send_error(vdev->vbasedev.proxy, &msg->hdr, EINVAL);
+        vfio_user_send_error(vdev->vbasedev.proxy, &res->hdr, EINVAL);
     }
 }
 
-- 
2.43.0