[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/3] docs/vhost-user: Clarifications for VHOST_USER_ADD/REM_M
From: |
Raphael Norwitz |
Subject: |
Re: [PATCH 1/3] docs/vhost-user: Clarifications for VHOST_USER_ADD/REM_MEM_REG |
Date: |
Sat, 9 Apr 2022 00:02:24 +0000 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
On Thu, Apr 07, 2022 at 03:36:55PM +0200, Kevin Wolf wrote:
> The specification for VHOST_USER_ADD/REM_MEM_REG messages is unclear
> in several points, which has led to clients having incompatible
> implementations. This changes the specification to be more explicit
> about them:
>
> * VHOST_USER_ADD_MEM_REG is not specified as receiving a file
> descriptor, though it obviously does need to do so. All
> implementations agree on this one, fix the specification.
>
> * VHOST_USER_REM_MEM_REG is not specified as receiving a file
> descriptor either, and it also has no reason to do so. rust-vmm does
> not send file descriptors for removing a memory region (in agreement
> with the specification), libvhost-user and QEMU do (which is a bug),
> though libvhost-user doesn't actually make any use of it.
>
> Change the specification so that for compatibility QEMU's behaviour
> becomes legal, even if discouraged, but rust-vmm's behaviour becomes
> the explicitly recommended mode of operation.
>
> * VHOST_USER_ADD_MEM_REG doesn't have a documented return value, which
> is the desired behaviour in the non-postcopy case. It also implemented
> like this in QEMU and rust-vmm, though libvhost-user is buggy and
> sometimes sends an unexpected reply. This will be fixed in a separate
> patch.
>
> However, in postcopy mode it does reply like VHOST_USER_SET_MEM_TABLE.
> This behaviour is shared between libvhost-user and QEMU; rust-vmm
> doesn't implement postcopy mode yet. Mention it explicitly in the
> spec.
>
> * The specification doesn't mention how VHOST_USER_REM_MEM_REG
> identifies the memory region to be removed. Change it to describe the
> existing behaviour of libvhost-user (guest address, user address and
> size must match).
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> ---
> docs/interop/vhost-user.rst | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index 4dbc84fd00..f9e721ba5f 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -308,6 +308,7 @@ replies. Here is a list of the ones that do:
> There are several messages that the master sends with file descriptors passed
> in the ancillary data:
>
> +* ``VHOST_USER_ADD_MEM_REG``
> * ``VHOST_USER_SET_MEM_TABLE``
> * ``VHOST_USER_SET_LOG_BASE`` (if ``VHOST_USER_PROTOCOL_F_LOG_SHMFD``)
> * ``VHOST_USER_SET_LOG_FD``
> @@ -1334,6 +1335,14 @@ Master message types
> ``VHOST_USER_REM_MEM_REG`` message, this message is used to set and
> update the memory tables of the slave device.
>
> + Exactly one file descriptor from which the memory is mapped is
> + passed in the ancillary data.
> +
> + In postcopy mode (see ``VHOST_USER_POSTCOPY_LISTEN``), the slave
> + replies with the bases of the memory mapped region to the master.
> + For further details on postcopy, see ``VHOST_USER_SET_MEM_TABLE``.
> + They apply to ``VHOST_USER_ADD_MEM_REG`` accordingly.
> +
> ``VHOST_USER_REM_MEM_REG``
> :id: 38
> :equivalent ioctl: N/A
> @@ -1349,6 +1358,14 @@ Master message types
> ``VHOST_USER_ADD_MEM_REG`` message, this message is used to set and
> update the memory tables of the slave device.
>
> + The memory region to be removed is identified by its guest address,
> + user address and size. The mmap offset is ignored.
> +
> + No file descriptors SHOULD be passed in the ancillary data. For
> + compatibility with existing incorrect implementations, the slave MAY
> + accept messages with one file descriptor. If a file descriptor is
> + passed, the slave MUST close it without using it otherwise.
> +
> ``VHOST_USER_SET_STATUS``
> :id: 39
> :equivalent ioctl: VHOST_VDPA_SET_STATUS
> --
> 2.35.1
>