On Fri, Nov 17, 2017 at 04:44:37AM +0800, Changpeng Liu wrote: > Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages which can be > used for live migration of vhost user devices, also vhost user devices > can benefit from the messages to get/set virtio config space from/to the > I/O target. For the purpose to support virtio config space change, > VHOST_USER_SET_CONFIG_FD message is added as the event notifier > in case virtio config space change in the I/O target. > > Signed-off-by: Changpeng Liu <changpeng....@intel.com> > --- > docs/interop/vhost-user.txt | 39 ++++++++++++++++ > hw/virtio/vhost-user.c | 98 > +++++++++++++++++++++++++++++++++++++++ > hw/virtio/vhost.c | 63 +++++++++++++++++++++++++ > include/hw/virtio/vhost-backend.h | 8 ++++ > include/hw/virtio/vhost.h | 16 +++++++ > 5 files changed, 224 insertions(+) > > diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt > index 954771d..1b98388 100644 > --- a/docs/interop/vhost-user.txt > +++ b/docs/interop/vhost-user.txt > @@ -116,6 +116,16 @@ Depending on the request type, payload can be: > - 3: IOTLB invalidate > - 4: IOTLB access fail > > + * Virtio device config space > + --------------------------- > + | offset | size | payload | > + --------------------------- > + > + Offset: a 32-bit offset of virtio device's configuration space
s/of/in the/
> + Size: a 32-bit size of configuration space that master wanted to change
Is this also used for GET_CONFIG? If yes, I suggest "a 32-bit
configuration space access size in bytes".
Please mention that Size must be <= 256 bytes.
> + Payload: a 256-bytes array holding the contents of the virtio
> + device's configuration space
What about bytes outside the [offset, offset+size) range? I guess they
must be 0 and are ignored by the master/slave.
Would it be cleaner to make Payload a variable-sized field with Size
bytes? That way it's not necessary to transfer 0s and memcpy() a subset
of the payload array.
> +
> In QEMU the vhost-user message is implemented with the following struct:
>
> typedef struct VhostUserMsg {
> @@ -129,6 +139,7 @@ typedef struct VhostUserMsg {
> VhostUserMemory memory;
> VhostUserLog log;
> struct vhost_iotlb_msg iotlb;
> + VhostUserConfig config;
> };
> } QEMU_PACKED VhostUserMsg;
>
> @@ -596,6 +607,34 @@ Master message types
> and expect this message once (per VQ) during device configuration
> (ie. before the master starts the VQ).
>
> + * VHOST_USER_GET_CONFIG
> + Id: 24
> + Equivalent ioctl: N/A
> + Master payload: virtio device config space
> +
> + Submitted by the vhost-user master to fetch the contents of the virtio
> + device configuration space. The vhost-user master may cache the
> contents
> + to avoid repeated VHOST_USER_GET_CONFIG calls.
> +
> +* VHOST_USER_SET_CONFIG
> + Id: 25
> + Equivalent ioctl: N/A
> + Master payload: virtio device config space
> +
> + Submitted by the vhost-user master when the Guest changes the virtio
> + device configuration space and also can be used for live migration
> + on the destination host.
There might be security issues if the vhost slave cannot tell whether
SET_CONFIG is coming from the guest driver or from the master process
(live migration). Typically certain fields are read-only for the guest
driver. Maybe those fields need to be set by the master after live
migration.
One way to solve this is adding a flags field to the message. A special
flag can be used for live migration so the slave knows that this
SET_CONFIG message is allowed to write to read-only fields.
It's also worth documenting that slaves MUST NOT accept SET_CONFIG for
read-only configuration space fields unless the live migration bit is
set. Hopefully this will remind implementors to think through the
security issues.
signature.asc
Description: PGP signature
