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 <kw...@redhat.com>
Reviewed-by: Raphael Norwitz <raphael.norw...@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 >