From 1f3b34e403b90292d0bfceb4109937511d525b61 Mon Sep 17 00:00:00 2001 From: John Levon Date: Fri, 10 Oct 2025 12:24:53 +0200 Subject: [PATCH 1/6] docs/interop/vfio-user: update protocol specification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refresh the protocol specification to the latest version implemented by libvfio-user. All changes are backward compatible. Note that QEMU client itself does not yet implement these extensions, but as this is now the canonical specification, it needs to be kept up to date. Signed-off-by: John Levon Link: https://lore.kernel.org/qemu-devel/20251010102453.711072-1-john.levon@nutanix.com Signed-off-by: Cédric Le Goater --- docs/interop/vfio-user.rst | 550 +++++++++++++++++++++++++++++++++---- 1 file changed, 504 insertions(+), 46 deletions(-) diff --git a/docs/interop/vfio-user.rst b/docs/interop/vfio-user.rst index 0b06f026b0..303fc13d6e 100644 --- a/docs/interop/vfio-user.rst +++ b/docs/interop/vfio-user.rst @@ -76,7 +76,7 @@ structures used in the ``ioctl()`` implementation. Messages derived from the ``ioctl()`` will have a name derived from the ``ioctl()`` command name. E.g., the ``VFIO_DEVICE_GET_INFO`` ``ioctl()`` command becomes a ``VFIO_USER_DEVICE_GET_INFO`` message. The purpose of this reuse is to share as -much code as feasible with the ``ioctl()`` implementation``. +much code as feasible with the ``ioctl()`` implementation. Connection Initiation ^^^^^^^^^^^^^^^^^^^^^ @@ -201,12 +201,32 @@ A server can serve: 1) one or more clients, and/or 2) one or more virtual devices, belonging to one or more clients. -The current protocol specification requires a dedicated socket per -client/server connection. It is a server-side implementation detail whether a -single server handles multiple virtual devices from the same or multiple -clients. The location of the socket is implementation-specific. Multiplexing -clients, devices, and servers over the same socket is not supported in this -version of the protocol. +The current protocol specification requires dedicated sockets per +client/server connection. Commands in the client-to-server direction are +handled on the main communication socket which the client connects to, and +replies to these commands are passed on the same socket. Commands sent in the +other direction from the server to the client as well as their corresponding +replies can optionally be passed across a separate socket, which is set up +during negotiation (AF_UNIX servers just pass the file descriptor). + +Using separate sockets for each command channel avoids introducing an +artificial point of synchronization between the channels. This simplifies +implementations since it obviates the need to demultiplex incoming messages +into commands and replies and interleave command handling and reply processing. +Note that it is still illegal for implementations to stall command or reply +processing indefinitely while waiting for replies on the other channel, as this +may lead to deadlocks. However, since incoming commands and requests arrive on +different sockets, it's possible to meet this requirement e.g. by running two +independent request processing threads that can internally operate +synchronously. It is expected that this is simpler to implement than fully +asynchronous message handling code. Implementations may still choose a fully +asynchronous, event-based design for other reasons, and the protocol fully +supports it. + +It is a server-side implementation detail whether a single server handles +multiple virtual devices from the same or multiple clients. The location of the +socket is implementation-specific. Multiplexing clients, devices, and servers +over the same socket is not supported in this version of the protocol. Authentication -------------- @@ -364,6 +384,9 @@ Name Command Request Direction ``VFIO_USER_DMA_WRITE`` 12 server -> client ``VFIO_USER_DEVICE_RESET`` 13 client -> server ``VFIO_USER_REGION_WRITE_MULTI`` 15 client -> server +``VFIO_USER_DEVICE_FEATURE`` 16 client -> server +``VFIO_USER_MIG_DATA_READ`` 17 client -> server +``VFIO_USER_MIG_DATA_WRITE`` 18 client -> server ====================================== ========= ================= Header @@ -486,45 +509,48 @@ format: Capabilities: -+--------------------+---------+------------------------------------------------+ -| Name | Type | Description | -+====================+=========+================================================+ -| max_msg_fds | number | Maximum number of file descriptors that can be | -| | | received by the sender in one message. | -| | | Optional. If not specified then the receiver | -| | | must assume a value of ``1``. | -+--------------------+---------+------------------------------------------------+ -| max_data_xfer_size | number | Maximum ``count`` for data transfer messages; | -| | | see `Read and Write Operations`_. Optional, | -| | | with a default value of 1048576 bytes. | -+--------------------+---------+------------------------------------------------+ -| pgsizes | number | Page sizes supported in DMA map operations | -| | | or'ed together. Optional, with a default value | -| | | of supporting only 4k pages. | -+--------------------+---------+------------------------------------------------+ -| max_dma_maps | number | Maximum number DMA map windows that can be | -| | | valid simultaneously. Optional, with a | -| | | value of 65535 (64k-1). | -+--------------------+---------+------------------------------------------------+ -| migration | object | Migration capability parameters. If missing | -| | | then migration is not supported by the sender. | -+--------------------+---------+------------------------------------------------+ -| write_multiple | boolean | ``VFIO_USER_REGION_WRITE_MULTI`` messages | -| | | are supported if the value is ``true``. | -+--------------------+---------+------------------------------------------------+ ++--------------------+---------+-----------------------------------------------+ +| Name | Type | Description | ++====================+=========+===============================================+ +| max_msg_fds | number | Maximum number of file descriptors that can | +| | | be received by the sender in one message. | +| | | Optional. If not specified then the receiver | +| | | must assume a value of ``1``. | ++--------------------+---------+-----------------------------------------------+ +| max_data_xfer_size | number | Maximum ``count`` for data transfer messages; | +| | | see `Read and Write Operations`_. Optional, | +| | | with a default value of 1048576 bytes. | ++--------------------+---------+-----------------------------------------------+ +| max_dma_maps | number | Maximum number DMA map windows that can be | +| | | valid simultaneously. Optional, with a | +| | | value of 65535 (64k-1). | ++--------------------+---------+-----------------------------------------------+ +| pgsizes | number | Page sizes supported in DMA map operations | +| | | or'ed together. Optional, with a default | +| | | value of supporting only 4k pages. | ++--------------------+---------+-----------------------------------------------+ +| twin_socket | object | Parameters for twin-socket mode, which | +| | | handles server-to-client commands and their | +| | | replies on a separate socket. Optional. | ++--------------------+---------+-----------------------------------------------+ +| write_multiple | boolean | ``VFIO_USER_REGION_WRITE_MULTI`` messages | +| | | are supported if the value is ``true``. | ++--------------------+---------+-----------------------------------------------+ -The migration capability contains the following name/value pairs: +The ``twin_socket`` capability object holds these name/value pairs: -+-----------------+--------+--------------------------------------------------+ -| Name | Type | Description | -+=================+========+==================================================+ -| pgsize | number | Page size of dirty pages bitmap. The smallest | -| | | between the client and the server is used. | -+-----------------+--------+--------------------------------------------------+ -| max_bitmap_size | number | Maximum bitmap size in ``VFIO_USER_DIRTY_PAGES`` | -| | | and ``VFIO_DMA_UNMAP`` messages. Optional, | -| | | with a default value of 256MB. | -+-----------------+--------+--------------------------------------------------+ ++-----------+---------+--------------------------------------------------------+ +| Name | Type | Description | ++===========+=========+========================================================+ +| supported | boolean | Indicates whether the sender supports twin-socket | +| | | mode. Optional, defaults to false. | ++-----------+---------+--------------------------------------------------------+ +| fd_index | number | Specifies an index in the file descriptor array | +| | | included with the message. The designated file | +| | | descriptor is a socket which is to be used for the | +| | | server-to-client command channel. Optional, only valid | +| | | in the reply message. | ++-----------+---------+--------------------------------------------------------+ Reply ^^^^^ @@ -532,6 +558,38 @@ Reply The same message format is used in the server's reply with the semantics described above. +If and only if the client has indicated support for twin-socket mode by setting +``twin_socket.supported`` to true in its capabilities, the server may optionally +set up a separate command channel for server-to-client commands and their +replies. The server enables twin-socket mode as follows: + +* Create a fresh socket pair. +* Keep the server end of the socket pair and pass the client end in the file + descriptor array included with the reply message. +* Set ``twin_socket.supported`` to true in the reply. +* Indicate the index in the file descriptor array by the + ``twin_socket.fd_index`` capability field in the reply, so the client can + identify the correct file descriptor to use. + +A client requesting twin-socket mode must examine the ``twin_socket`` capability +in the reply: + +* If ``twin_socket.supported`` is false, the field is missing, or the entire + ``twin_socket`` object is absent, the server does not support twin-socket mode + or decided not to enable it. The client can choose whether it wants to proceed + without twin-socket mode, or close the connection if not. +* If ``twin_socket.supported`` is true and ``twin_socket.fd_index`` is present + and refers to a valid file descriptor, twin-socket mode negotiation has + succeeded. The client monitors the provided file descriptor for commands from + the server. +* Otherwise, the reply from the server is inconsistent. The client must abort + and close the connection since it is potentially unable to receive commands + from the server. + +The twin-socket feature is optional, so some servers may not support it. +However, for server implementations that do send server-to-client commands it is +strongly recommended to implement twin-socket support. + ``VFIO_USER_DMA_MAP`` --------------------- @@ -1335,7 +1393,9 @@ Reply ----------------------- If the client has not shared mappable memory, the server can use this message to -read from guest memory. +read from guest memory. This message and its reply are passed over the separate +server-to-client socket if twin-socket mode has been negotiated during +connection setup. Request ^^^^^^^ @@ -1373,7 +1433,9 @@ Reply ----------------------- If the client has not shared mappable memory, the server can use this message to -write to guest memory. +write to guest memory. This message and its reply are passed over the separate +server-to-client socket if twin-socket mode has been negotiated during +connection setup. Request ^^^^^^^ @@ -1466,6 +1528,402 @@ Reply * *wr_cnt* is the number of device writes completed. +``VFIO_USER_DEVICE_FEATURE`` +---------------------------- + +This command is analogous to ``VFIO_DEVICE_FEATURE``. It is used to get, set, or +probe feature data of the device. + +Request +^^^^^^^ + +The request payload for this message is a structure of the following format. + ++-------+--------+--------------------------------+ +| Name | Offset | Size | ++=======+========+================================+ +| argsz | 0 | 4 | ++-------+--------+--------------------------------+ +| flags | 4 | 4 | ++-------+--------+--------------------------------+ +| | +---------+---------------------------+ | +| | | Bit | Definition | | +| | +=========+===========================+ | +| | | 0 to 15 | Feature index | | +| | +---------+---------------------------+ | +| | | 16 | VFIO_DEVICE_FEATURE_GET | | +| | +---------+---------------------------+ | +| | | 17 | VFIO_DEVICE_FEATURE_SET | | +| | +---------+---------------------------+ | +| | | 18 | VFIO_DEVICE_FEATURE_PROBE | | +| | +---------+---------------------------+ | ++-------+--------+--------------------------------+ +| data | 8 | variable | ++-------+--------+--------------------------------+ + +* *argsz* is the maximum size of the reply payload. + +* *flags* defines the action to be performed by the server and upon which + feature: + + * The feature index consists of the least significant 16 bits of the flags + field, and can be accessed using the ``VFIO_DEVICE_FEATURE_MASK`` bit mask. + + * ``VFIO_DEVICE_FEATURE_GET`` instructs the server to get the data for the + given feature. + + * ``VFIO_DEVICE_FEATURE_SET`` instructs the server to set the feature data to + that given in the ``data`` field of the payload. + + * ``VFIO_DEVICE_FEATURE_PROBE`` instructs the server to probe for feature + support. If ``VFIO_DEVICE_FEATURE_GET`` and/or ``VFIO_DEVICE_FEATURE_SET`` + are also set, the probe will only return success if all of the indicated + methods are supported. + + ``VFIO_DEVICE_FEATURE_GET`` and ``VFIO_DEVICE_FEATURE_SET`` are mutually + exclusive, except for use with ``VFIO_DEVICE_FEATURE_PROBE``. + +* *data* is specific to the particular feature. It is not used for probing. + +This part of the request is analogous to VFIO's ``struct vfio_device_feature``. + +Reply +^^^^^ + +The reply payload must be the same as the request payload for setting or +probing a feature. For getting a feature's data, the data is added in the data +section and its length is added to ``argsz``. + +Device Features +^^^^^^^^^^^^^^^ + +The only device features supported by vfio-user are those related to migration, +although this may change in the future. They are a subset of those supported in +the VFIO implementation of the Linux kernel. + ++----------------------------------------+---------------+ +| Name | Feature Index | ++========================================+===============+ +| VFIO_DEVICE_FEATURE_MIGRATION | 1 | ++----------------------------------------+---------------+ +| VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE | 2 | ++----------------------------------------+---------------+ +| VFIO_DEVICE_FEATURE_DMA_LOGGING_START | 6 | ++----------------------------------------+---------------+ +| VFIO_DEVICE_FEATURE_DMA_LOGGING_STOP | 7 | ++----------------------------------------+---------------+ +| VFIO_DEVICE_FEATURE_DMA_LOGGING_REPORT | 8 | ++----------------------------------------+---------------+ + +``VFIO_DEVICE_FEATURE_MIGRATION`` +""""""""""""""""""""""""""""""""" + +This feature indicates that the device can support the migration API through +``VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE``. If ``GET`` succeeds, the ``RUNNING`` +and ``ERROR`` states are always supported. Support for additional states is +indicated via the flags field; at least ``VFIO_MIGRATION_STOP_COPY`` must be +set. + +There is no data field of the request message. + +The data field of the reply message is structured as follows: + ++-------+--------+---------------------------+ +| Name | Offset | Size | ++=======+========+===========================+ +| flags | 0 | 8 | ++-------+--------+---------------------------+ +| | +-----+--------------------------+ | +| | | Bit | Definition | | +| | +=====+==========================+ | +| | | 0 | VFIO_MIGRATION_STOP_COPY | | +| | +-----+--------------------------+ | +| | | 1 | VFIO_MIGRATION_P2P | | +| | +-----+--------------------------+ | +| | | 2 | VFIO_MIGRATION_PRE_COPY | | +| | +-----+--------------------------+ | ++-------+--------+---------------------------+ + +These flags are interpreted in the same way as VFIO. + +``VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE`` +"""""""""""""""""""""""""""""""""""""""" + +Upon ``VFIO_DEVICE_FEATURE_SET``, execute a migration state change on the VFIO +device. The new state is supplied in ``device_state``. The state transition must +fully complete before the reply is sent. + +The data field of the reply message, as well as the ``SET`` request message, is +structured as follows: + ++--------------+--------+------+ +| Name | Offset | Size | ++==============+========+======+ +| device_state | 0 | 4 | ++--------------+--------+------+ +| data_fd | 4 | 4 | ++--------------+--------+------+ + +* *device_state* is the current state of the device (for ``GET``) or the + state to transition to (for ``SET``). It is defined by the + ``vfio_device_mig_state`` enum as detailed below. These states are the states + of the device migration Finite State Machine. + ++--------------------------------+-------+---------------------------------------------------------------------+ +| Name | State | Description | ++================================+=======+=====================================================================+ +| VFIO_DEVICE_STATE_ERROR | 0 | The device has failed and must be reset. | ++--------------------------------+-------+---------------------------------------------------------------------+ +| VFIO_DEVICE_STATE_STOP | 1 | The device does not change the internal or external state. | ++--------------------------------+-------+---------------------------------------------------------------------+ +| VFIO_DEVICE_STATE_RUNNING | 2 | The device is running normally. | ++--------------------------------+-------+---------------------------------------------------------------------+ +| VFIO_DEVICE_STATE_STOP_COPY | 3 | The device internal state can be read out. | ++--------------------------------+-------+---------------------------------------------------------------------+ +| VFIO_DEVICE_STATE_RESUMING | 4 | The device is stopped and is loading a new internal state. | ++--------------------------------+-------+---------------------------------------------------------------------+ +| VFIO_DEVICE_STATE_RUNNING_P2P | 5 | (not used in vfio-user) | ++--------------------------------+-------+---------------------------------------------------------------------+ +| VFIO_DEVICE_STATE_PRE_COPY | 6 | The device is running normally but tracking internal state changes. | ++--------------------------------+-------+---------------------------------------------------------------------+ +| VFIO_DEVICE_STATE_PRE_COPY_P2P | 7 | (not used in vfio-user) | ++--------------------------------+-------+---------------------------------------------------------------------+ + +* *data_fd* is unused in vfio-user, as the ``VFIO_USER_MIG_DATA_READ`` and + ``VFIO_USER_MIG_DATA_WRITE`` messages are used instead for migration data + transport. + +Direct State Transitions +"""""""""""""""""""""""" + +The device migration FSM is a Mealy machine, so actions are taken upon the arcs +between FSM states. The following transitions need to be supported by the +server, a subset of those defined in ```` +(``enum vfio_device_mig_state``). + +* ``RUNNING -> STOP``, ``STOP_COPY -> STOP``: Stop the operation of the device. + The ``STOP_COPY`` arc terminates the data transfer session. + +* ``RESUMING -> STOP``: Terminate the data transfer session. Complete processing + of the migration data. Stop the operation of the device. If the delivered data + is found to be incomplete, inconsistent, or otherwise invalid, fail the + ``SET`` command and optionally transition to the ``ERROR`` state. + +* ``PRE_COPY -> RUNNING``: Terminate the data transfer session. The device is + now fully operational. + +* ``STOP -> RUNNING``: Start the operation of the device. + +* ``RUNNING -> PRE_COPY``, ``STOP -> STOP_COPY``: Begin the process of saving + the device state. The device operation is unchanged, but data transfer begins. + ``PRE_COPY`` and ``STOP_COPY`` are referred to as the "saving group" of + states. + +* ``PRE_COPY -> STOP_COPY``: Continue to transfer migration data, but stop + device operation. + +* ``STOP -> RESUMING``: Start the process of restoring the device state. The + internal device state may be changed to prepare the device to receive the + migration data. + +The ``STOP_COPY -> PRE_COPY`` transition is explicitly not allowed and should +return an error if requested. + +``ERROR`` cannot be specified as a device state, but any transition request can +be failed and then move the state into ``ERROR`` if the server was unable to +execute the requested arc AND was unable to restore the device into any valid +state. To recover from ``ERROR``, ``VFIO_USER_DEVICE_RESET`` must be used to +return back to ``RUNNING``. + +If ``PRE_COPY`` is not supported, arcs touching it are removed. + +Complex State Transitions +""""""""""""""""""""""""" + +The remaining possible transitions are to be implemented as combinations of the +above FSM arcs. As there are multiple paths, the path should be selected based +on the following rules: + +* Select the shortest path. + +* The path cannot have saving group states as interior arcs, only start/end + states. + +``VFIO_DEVICE_FEATURE_DMA_LOGGING_START`` / ``VFIO_DEVICE_FEATURE_DMA_LOGGING_STOP`` +"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" + +Upon ``VFIO_DEVICE_FEATURE_SET``, start/stop DMA logging. These features can +also be probed to determine whether the device supports DMA logging. + +When DMA logging is started, a range of IOVAs to monitor is provided and the +device can optimize its logging to cover only the IOVA range given. Only DMA +writes are logged. + +The data field of the ``SET`` request is structured as follows: + ++------------+--------+----------+ +| Name | Offset | Size | ++============+========+==========+ +| page_size | 0 | 8 | ++------------+--------+----------+ +| num_ranges | 8 | 4 | ++------------+--------+----------+ +| reserved | 12 | 4 | ++------------+--------+----------+ +| ranges | 16 | variable | ++------------+--------+----------+ + +* *page_size* hints what tracking granularity the device should try to achieve. + If the device cannot do the hinted page size then it's the driver's choice + which page size to pick based on its support. On output the device will return + the page size it selected. + +* *num_ranges* is the number of IOVA ranges to monitor. A value of zero + indicates that all writes should be logged. + +* *ranges* is an array of ``vfio_user_device_feature_dma_logging_range`` + entries: + ++--------+--------+------+ +| Name | Offset | Size | ++========+========+======+ +| iova | 0 | 8 | ++--------+--------+------+ +| length | 8 | 8 | ++--------+--------+------+ + + * *iova* is the base IO virtual address + * *length* is the length of the range to log + +Upon success, the response data field will be the same as the request, unless +the page size was changed, in which case this will be reflected in the response. + +``VFIO_DEVICE_FEATURE_DMA_LOGGING_REPORT`` +"""""""""""""""""""""""""""""""""""""""""" + +Upon ``VFIO_DEVICE_FEATURE_GET``, returns the dirty bitmap for a specific IOVA +range. This operation is only valid if logging of dirty pages has been +previously started by setting ``VFIO_DEVICE_FEATURE_DMA_LOGGING_START``. + +The data field of the request is structured as follows: + ++-----------+--------+------+ +| Name | Offset | Size | ++===========+========+======+ +| iova | 0 | 8 | ++-----------+--------+------+ +| length | 8 | 8 | ++-----------+--------+------+ +| page_size | 16 | 8 | ++-----------+--------+------+ + +* *iova* is the base IO virtual address + +* *length* is the length of the range + +* *page_size* is the unit of granularity of the bitmap, and must be a power of + two. It doesn't have to match the value given to + ``VFIO_DEVICE_FEATURE_DMA_LOGGING_START`` because the driver will format its + internal logging to match the reporting page size possibly by replicating bits + if the internal page size is lower than requested + +The data field of the response is identical, except with the bitmap added on +the end at offset 24. + +The bitmap is an array of u64s that holds the output bitmap, with 1 bit +reporting a *page_size* unit of IOVA. The bits outside of the requested range +must be zero. + +The mapping of IOVA to bits is given by: + +``bitmap[(addr - iova)/page_size] & (1ULL << (addr % 64))`` + +``VFIO_USER_MIG_DATA_READ`` +--------------------------- + +This command is used to read data from the source migration server while it is +in a saving group state (``PRE_COPY`` or ``STOP_COPY``). + +This command, and ``VFIO_USER_MIG_DATA_WRITE``, are used in place of the +``data_fd`` file descriptor in ```` +(``struct vfio_device_feature_mig_state``) to enable all data transport to use +the single already-established UNIX socket. Hence, the migration data is +treated like a stream, so the client must continue reading until no more +migration data remains. + +Request +^^^^^^^ + +The request payload for this message is a structure of the following format. + ++-------+--------+------+ +| Name | Offset | Size | ++=======+========+======+ +| argsz | 0 | 4 | ++-------+--------+------+ +| size | 4 | 4 | ++-------+--------+------+ + +* *argsz* is the maximum size of the reply payload. + +* *size* is the size of the migration data to read. + +Reply +^^^^^ + +The reply payload for this message is a structure of the following format. + ++-------+--------+----------+ +| Name | Offset | Size | ++=======+========+==========+ +| argsz | 0 | 4 | ++-------+--------+----------+ +| size | 4 | 4 | ++-------+--------+----------+ +| data | 8 | variable | ++-------+--------+----------+ + +* *argsz* is the size of the above structure, including the size of the data. + +* *size* indicates the size of returned migration data. If this is less than the + requested size, there is no more migration data to read. + +* *data* contains the migration data. + +``VFIO_USER_MIG_DATA_WRITE`` +---------------------------- + +This command is used to write data to the destination migration server while it +is in the ``RESUMING`` state. + +As above, this replaces the ``data_fd`` file descriptor for transport of +migration data, and as such, the migration data is treated like a stream. + +Request +^^^^^^^ + +The request payload for this message is a structure of the following format. + ++-------+--------+----------+ +| Name | Offset | Size | ++=======+========+==========+ +| argsz | 0 | 4 | ++-------+--------+----------+ +| size | 4 | 4 | ++-------+--------+----------+ +| data | 8 | variable | ++-------+--------+----------+ + +* *argsz* is the maximum size of the reply payload. + +* *size* is the size of the migration data to be written. + +* *data* contains the migration data. + +Reply +^^^^^ + +There is no reply payload for this message. Appendices ========== From a8731f691df2bd9efd041d836ece27e3173555f2 Mon Sep 17 00:00:00 2001 From: John Levon Date: Wed, 3 Dec 2025 15:33:11 +0530 Subject: [PATCH 2/6] vfio-user: simplify vfio_user_process() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It can figure out if it's a reply by itself, rather than passing that information in. Signed-off-by: John Levon Reviewed-by: Cédric Le Goater Reviewed-by: Mark Cave-Ayland Link: https://lore.kernel.org/qemu-devel/20251203100316.3604456-2-john.levon@nutanix.com Signed-off-by: Cédric Le Goater --- hw/vfio-user/proxy.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c index bbd7ec243d..75845d7c89 100644 --- a/hw/vfio-user/proxy.c +++ b/hw/vfio-user/proxy.c @@ -147,8 +147,7 @@ VFIOUserFDs *vfio_user_getfds(int numfds) /* * Process a received message. */ -static void vfio_user_process(VFIOUserProxy *proxy, VFIOUserMsg *msg, - bool isreply) +static void vfio_user_process(VFIOUserProxy *proxy, VFIOUserMsg *msg) { /* @@ -157,7 +156,7 @@ static void vfio_user_process(VFIOUserProxy *proxy, VFIOUserMsg *msg, * * Requests get queued for the BH. */ - if (isreply) { + if ((msg->hdr->flags & VFIO_USER_TYPE) == VFIO_USER_REPLY) { msg->complete = true; if (msg->type == VFIO_MSG_WAIT) { qemu_cond_signal(&msg->cv); @@ -187,7 +186,6 @@ static int vfio_user_complete(VFIOUserProxy *proxy, Error **errp) { VFIOUserMsg *msg = proxy->part_recv; size_t msgleft = proxy->recv_left; - bool isreply; char *data; int ret; @@ -214,8 +212,7 @@ static int vfio_user_complete(VFIOUserProxy *proxy, Error **errp) */ proxy->part_recv = NULL; proxy->recv_left = 0; - isreply = (msg->hdr->flags & VFIO_USER_TYPE) == VFIO_USER_REPLY; - vfio_user_process(proxy, msg, isreply); + vfio_user_process(proxy, msg); /* return positive value */ return 1; @@ -381,7 +378,7 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error **errp) data += ret; } - vfio_user_process(proxy, msg, isreply); + vfio_user_process(proxy, msg); return 0; /* From 356c7b175258f29429fb91a7641f775080ab6b49 Mon Sep 17 00:00:00 2001 From: John Levon Date: Wed, 3 Dec 2025 15:33:12 +0530 Subject: [PATCH 3/6] vfio-user: clarify partial message handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improve a comment for this. Signed-off-by: John Levon Reviewed-by: Cédric Le Goater Reviewed-by: Mark Cave-Ayland Link: https://lore.kernel.org/qemu-devel/20251203100316.3604456-3-john.levon@nutanix.com Signed-off-by: Cédric Le Goater --- hw/vfio-user/proxy.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c index 75845d7c89..82c76c6665 100644 --- a/hw/vfio-user/proxy.c +++ b/hw/vfio-user/proxy.c @@ -362,7 +362,10 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error **errp) while (msgleft > 0) { ret = qio_channel_read(proxy->ioc, data, msgleft, errp); - /* prepare to complete read on next iternation */ + /* + * We'll complete this read on the next go around; keep track of the + * partial message until then. + */ if (ret == QIO_CHANNEL_ERR_BLOCK) { proxy->part_recv = msg; proxy->recv_left = msgleft; From 7b884e2a27793e0ff5817ad04ecce85d0a90149d Mon Sep 17 00:00:00 2001 From: John Levon Date: Wed, 3 Dec 2025 15:33:13 +0530 Subject: [PATCH 4/6] vfio-user: refactor out header handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify vfio_user_recv_one() by moving the header handling out to a helper function. Signed-off-by: John Levon Reviewed-by: Cédric Le Goater Link: https://lore.kernel.org/qemu-devel/20251203100316.3604456-4-john.levon@nutanix.com Signed-off-by: Cédric Le Goater --- hw/vfio-user/proxy.c | 101 +++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 41 deletions(-) diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c index 82c76c6665..e0f9202535 100644 --- a/hw/vfio-user/proxy.c +++ b/hw/vfio-user/proxy.c @@ -218,6 +218,61 @@ static int vfio_user_complete(VFIOUserProxy *proxy, Error **errp) return 1; } +static int vfio_user_recv_hdr(VFIOUserProxy *proxy, Error **errp, + VFIOUserHdr *hdr, int **fdp, size_t *numfdp, + bool *isreply) +{ + struct iovec iov = { + .iov_base = hdr, + .iov_len = sizeof(*hdr), + }; + int ret; + + /* + * Read header + */ + ret = qio_channel_readv_full(proxy->ioc, &iov, 1, fdp, numfdp, 0, + errp); + if (ret == QIO_CHANNEL_ERR_BLOCK) { + return ret; + } + + if (ret < 0) { + error_setg_errno(errp, errno, "failed to read header"); + return -1; + } else if (ret == 0) { + error_setg(errp, "failed to read header: EOF"); + return -1; + } else if (ret < sizeof(*hdr)) { + error_setg(errp, "short read of header"); + return -1; + } + + /* + * Validate header + */ + if (hdr->size < sizeof(*hdr)) { + error_setg(errp, "bad header size"); + return -1; + } + + switch (hdr->flags & VFIO_USER_TYPE) { + case VFIO_USER_REQUEST: + *isreply = false; + break; + case VFIO_USER_REPLY: + *isreply = true; + break; + default: + error_setg(errp, "unknown message type"); + return -1; + } + + trace_vfio_user_recv_hdr(proxy->sockname, hdr->id, hdr->command, hdr->size, + hdr->flags); + return 0; +} + /* * Receive and process one incoming message. * @@ -230,10 +285,6 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error **errp) g_autofree int *fdp = NULL; VFIOUserFDs *reqfds; VFIOUserHdr hdr; - struct iovec iov = { - .iov_base = &hdr, - .iov_len = sizeof(hdr), - }; bool isreply = false; int i, ret; size_t msgleft, numfds = 0; @@ -257,46 +308,14 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error **errp) /* else fall into reading another msg */ } - /* - * Read header - */ - ret = qio_channel_readv_full(proxy->ioc, &iov, 1, &fdp, &numfds, 0, - errp); - if (ret == QIO_CHANNEL_ERR_BLOCK) { - return ret; - } - - /* read error or other side closed connection */ - if (ret <= 0) { + ret = vfio_user_recv_hdr(proxy, errp, &hdr, &fdp, &numfds, &isreply); + if (ret < 0) { + if (ret == QIO_CHANNEL_ERR_BLOCK) { + return ret; + } goto fatal; } - if (ret < sizeof(hdr)) { - error_setg(errp, "short read of header"); - goto fatal; - } - - /* - * Validate header - */ - if (hdr.size < sizeof(VFIOUserHdr)) { - error_setg(errp, "bad header size"); - goto fatal; - } - switch (hdr.flags & VFIO_USER_TYPE) { - case VFIO_USER_REQUEST: - isreply = false; - break; - case VFIO_USER_REPLY: - isreply = true; - break; - default: - error_setg(errp, "unknown message type"); - goto fatal; - } - trace_vfio_user_recv_hdr(proxy->sockname, hdr.id, hdr.command, hdr.size, - hdr.flags); - /* * For replies, find the matching pending request. * For requests, reap incoming FDs. From 0df8baec95b76c93093efde37d1644508c3c7d2c Mon Sep 17 00:00:00 2001 From: John Levon Date: Wed, 3 Dec 2025 15:33:14 +0530 Subject: [PATCH 5/6] vfio-user: simplify vfio_user_recv_one() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function was unnecessarily difficult to understand due to the separate handling of request and reply messages. Use common code for both where we can. Signed-off-by: John Levon Reviewed-by: Mark Cave-Ayland Link: https://lore.kernel.org/qemu-devel/20251203100316.3604456-5-john.levon@nutanix.com Signed-off-by: Cédric Le Goater --- hw/vfio-user/proxy.c | 68 +++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 38 deletions(-) diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c index e0f9202535..f2601eada5 100644 --- a/hw/vfio-user/proxy.c +++ b/hw/vfio-user/proxy.c @@ -281,15 +281,14 @@ static int vfio_user_recv_hdr(VFIOUserProxy *proxy, Error **errp, */ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error **errp) { - VFIOUserMsg *msg = NULL; g_autofree int *fdp = NULL; - VFIOUserFDs *reqfds; - VFIOUserHdr hdr; + VFIOUserMsg *msg = NULL; bool isreply = false; - int i, ret; - size_t msgleft, numfds = 0; + size_t msgleft = 0; + size_t numfds = 0; char *data = NULL; - char *buf = NULL; + VFIOUserHdr hdr; + int i, ret; /* * Complete any partial reads @@ -317,8 +316,8 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error **errp) } /* - * For replies, find the matching pending request. - * For requests, reap incoming FDs. + * Find the matching request if this is a reply, or initialize a new + * server->client request. */ if (isreply) { QTAILQ_FOREACH(msg, &proxy->pending, next) { @@ -332,51 +331,44 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error **errp) } QTAILQ_REMOVE(&proxy->pending, msg, next); - /* - * Process any received FDs - */ - if (numfds != 0) { - if (msg->fds == NULL || msg->fds->recv_fds < numfds) { - error_setg(errp, "unexpected FDs"); - goto err; - } - msg->fds->recv_fds = numfds; - memcpy(msg->fds->fds, fdp, numfds * sizeof(int)); - } - } else { - if (numfds != 0) { - reqfds = vfio_user_getfds(numfds); - memcpy(reqfds->fds, fdp, numfds * sizeof(int)); - } else { - reqfds = NULL; - } - } - - /* - * Put the whole message into a single buffer. - */ - if (isreply) { if (hdr.size > msg->rsize) { error_setg(errp, "reply larger than recv buffer"); goto err; } - *msg->hdr = hdr; - data = (char *)msg->hdr + sizeof(hdr); } else { + void *buf; + if (hdr.size > proxy->max_xfer_size + sizeof(VFIOUserDMARW)) { error_setg(errp, "vfio_user_recv request larger than max"); goto err; } + buf = g_malloc0(hdr.size); - memcpy(buf, &hdr, sizeof(hdr)); - data = buf + sizeof(hdr); - msg = vfio_user_getmsg(proxy, (VFIOUserHdr *)buf, reqfds); + msg = vfio_user_getmsg(proxy, buf, NULL); msg->type = VFIO_MSG_REQ; } + *msg->hdr = hdr; + data = (char *)msg->hdr + sizeof(hdr); + + if (numfds != 0) { + if (msg->type == VFIO_MSG_REQ) { + msg->fds = vfio_user_getfds(numfds); + } else { + if (msg->fds == NULL || msg->fds->recv_fds < numfds) { + error_setg(errp, "unexpected FDs in reply"); + goto err; + } + msg->fds->recv_fds = numfds; + } + + memcpy(msg->fds->fds, fdp, numfds * sizeof(int)); + } + /* - * Read rest of message. + * Read rest of message into the data buffer. */ + msgleft = hdr.size - sizeof(hdr); while (msgleft > 0) { ret = qio_channel_read(proxy->ioc, data, msgleft, errp); From 23c586abf2e12843894189c4742c8ea55c594cd5 Mon Sep 17 00:00:00 2001 From: John Levon Date: Wed, 3 Dec 2025 15:33:15 +0530 Subject: [PATCH 6/6] vfio-user: recycle msg on failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we fail to read an incoming request, recycle the message. Resolves: Coverity CID 1611807 Resolves: Coverity CID 1611808 Signed-off-by: John Levon Reviewed-by: Mark Cave-Ayland Link: https://lore.kernel.org/qemu-devel/20251203100316.3604456-6-john.levon@nutanix.com Signed-off-by: Cédric Le Goater --- hw/vfio-user/proxy.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c index f2601eada5..314dfd23d8 100644 --- a/hw/vfio-user/proxy.c +++ b/hw/vfio-user/proxy.c @@ -412,11 +412,22 @@ err: for (i = 0; i < numfds; i++) { close(fdp[i]); } - if (isreply && msg != NULL) { - /* force an error to keep sending thread from hanging */ - vfio_user_set_error(msg->hdr, EINVAL); - msg->complete = true; - qemu_cond_signal(&msg->cv); + if (msg != NULL) { + if (msg->type == VFIO_MSG_REQ) { + /* + * Clean up the request message on failure. Change type back to + * NOWAIT to free. + */ + msg->type = VFIO_MSG_NOWAIT; + vfio_user_recycle(proxy, msg); + } else { + /* + * Report an error back to the sender. Sender will recycle msg. + */ + vfio_user_set_error(msg->hdr, EINVAL); + msg->complete = true; + qemu_cond_signal(&msg->cv); + } } return -1; }