| Message ID | 1508390650-19115-2-git-send-email-changpeng.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19/10/2017 07:24, 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.liu@intel.com> > --- > docs/interop/vhost-user.txt | 33 +++++++++++++ > hw/virtio/vhost-user.c | 97 +++++++++++++++++++++++++++++++++++++++ > hw/virtio/vhost.c | 63 +++++++++++++++++++++++++ > include/hw/virtio/vhost-backend.h | 8 ++++ > include/hw/virtio/vhost.h | 16 +++++++ > 5 files changed, 217 insertions(+) > > diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt > index 954771d..ea4c5ca 100644 > --- a/docs/interop/vhost-user.txt > +++ b/docs/interop/vhost-user.txt > @@ -116,6 +116,10 @@ Depending on the request type, payload can be: > - 3: IOTLB invalidate > - 4: IOTLB access fail > > + * Virtio device config space > + > + 256 Bytes static virito device config space "A 256-bytes array holding the contents of the virtio device's configuration space" > +* VHOST_USER_SET_CONFIG_FD > + Id: 26 > + Equivalent ioctl: N/A > + Master payload: N/A > + > + Sets the notifier file descriptor, which is passed as ancillary data. > + Vhost-user slave uses the file descriptor to notify vhost-user master "The vhost-user slave uses the file descriptor to notify the vhost-user master of changes to the virtio configuration space." Paolo > + when the virtio config space changed. The vhost-user master can read > + the virtio config space to get the latest update. > + > Slave message types > ------------------- > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 093675e..4e7bafc 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -26,6 +26,11 @@ > #define VHOST_MEMORY_MAX_NREGIONS 8 > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > > +/* > + * Maximum size of virtio device config space > + */ > +#define VHOST_USER_MAX_CONFIG_SIZE 256 > + > enum VhostUserProtocolFeature { > VHOST_USER_PROTOCOL_F_MQ = 0, > VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1, > @@ -65,6 +70,9 @@ typedef enum VhostUserRequest { > VHOST_USER_SET_SLAVE_REQ_FD = 21, > VHOST_USER_IOTLB_MSG = 22, > VHOST_USER_SET_VRING_ENDIAN = 23, > + VHOST_USER_GET_CONFIG = 24, > + VHOST_USER_SET_CONFIG = 25, > + VHOST_USER_SET_CONFIG_FD = 26, > VHOST_USER_MAX > } VhostUserRequest; > > @@ -109,6 +117,7 @@ typedef struct VhostUserMsg { > VhostUserMemory memory; > VhostUserLog log; > struct vhost_iotlb_msg iotlb; > + uint8_t config[VHOST_USER_MAX_CONFIG_SIZE]; > } payload; > } QEMU_PACKED VhostUserMsg; > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled) > /* No-op as the receive channel is not dedicated to IOTLB messages. */ > } > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, > + size_t config_len) > +{ > + VhostUserMsg msg = { > + .request = VHOST_USER_GET_CONFIG, > + .flags = VHOST_USER_VERSION, > + .size = config_len, > + }; > + > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { > + error_report("bad config length"); > + return -1; > + } > + > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > + > + if (vhost_user_read(dev, &msg) < 0) { > + return -1; > + } > + > + if (msg.request != VHOST_USER_GET_CONFIG) { > + error_report("Received unexpected msg type. Expected %d received %d", > + VHOST_USER_GET_CONFIG, msg.request); > + return -1; > + } > + > + if (msg.size != config_len) { > + error_report("Received bad msg size."); > + return -1; > + } > + > + memcpy(config, &msg.payload.config, config_len); > + > + return 0; > +} > + > +static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *config, > + size_t config_len) > +{ > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + VhostUserMsg msg = { > + .request = VHOST_USER_SET_CONFIG, > + .flags = VHOST_USER_VERSION, > + .size = config_len, > + }; > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_REPLY_MASK; > + } > + > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { > + error_report("bad config length"); > + return -1; > + } > + > + memcpy(&msg.payload.config, config, config_len); > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > + > + if (reply_supported) { > + return process_message_reply(dev, &msg); > + } > + > + return 0; > +} > + > +static int vhost_user_set_config_fd(struct vhost_dev *dev, int fd) > +{ > + VhostUserMsg msg = { > + .request = VHOST_USER_SET_CONFIG_FD, > + .flags = VHOST_USER_VERSION, > + }; > + > + if (vhost_user_write(dev, &msg, &fd, 1) < 0) { > + return -1; > + } > + > + return 0; > +} > + > const VhostOps user_ops = { > .backend_type = VHOST_BACKEND_TYPE_USER, > .vhost_backend_init = vhost_user_init, > @@ -948,4 +1042,7 @@ const VhostOps user_ops = { > .vhost_net_set_mtu = vhost_user_net_set_mtu, > .vhost_set_iotlb_callback = vhost_user_set_iotlb_callback, > .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg, > + .vhost_get_config = vhost_user_get_config, > + .vhost_set_config = vhost_user_set_config, > + .vhost_set_config_fd = vhost_user_set_config_fd, > }; > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index ddc42f0..8079f46 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -1354,6 +1354,9 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) > for (i = 0; i < hdev->nvqs; ++i) { > vhost_virtqueue_cleanup(hdev->vqs + i); > } > + if (hdev->config_ops) { > + event_notifier_cleanup(&hdev->config_notifier); > + } > if (hdev->mem) { > /* those are only safe after successful init */ > memory_listener_unregister(&hdev->memory_listener); > @@ -1501,6 +1504,66 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits, > } > } > > +int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config, > + size_t config_len) > +{ > + assert(hdev->vhost_ops); > + > + if (hdev->vhost_ops->vhost_get_config) { > + return hdev->vhost_ops->vhost_get_config(hdev, config, config_len); > + } > + > + return 0; > +} > + > +int vhost_dev_set_config(struct vhost_dev *hdev, const uint8_t *config, > + size_t config_len) > +{ > + assert(hdev->vhost_ops); > + > + if (hdev->vhost_ops->vhost_set_config) { > + return hdev->vhost_ops->vhost_set_config(hdev, config, config_len); > + } > + > + return 0; > +} > + > +static void vhost_dev_config_notifier_read(EventNotifier *n) > +{ > + struct vhost_dev *hdev = container_of(n, struct vhost_dev, > + config_notifier); > + > + if (event_notifier_test_and_clear(n)) { > + if (hdev->config_ops) { > + hdev->config_ops->vhost_dev_config_notifier(hdev); > + } > + } > +} > + > +int vhost_dev_set_config_notifier(struct vhost_dev *hdev, > + const VhostDevConfigOps *ops) > +{ > + int r, fd; > + > + assert(hdev->vhost_ops); > + > + r = event_notifier_init(&hdev->config_notifier, 0); > + if (r < 0) { > + return r; > + } > + > + hdev->config_ops = ops; > + event_notifier_set_handler(&hdev->config_notifier, > + vhost_dev_config_notifier_read); > + > + if (hdev->vhost_ops->vhost_set_config_fd) { > + fd = event_notifier_get_fd(&hdev->config_notifier); > + return hdev->vhost_ops->vhost_set_config_fd(hdev, fd); > + } > + > + return 0; > +} > + > /* Host notifiers must be enabled at this point. */ > int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) > { > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h > index a7a5f22..df6769e 100644 > --- a/include/hw/virtio/vhost-backend.h > +++ b/include/hw/virtio/vhost-backend.h > @@ -84,6 +84,11 @@ typedef void (*vhost_set_iotlb_callback_op)(struct vhost_dev *dev, > int enabled); > typedef int (*vhost_send_device_iotlb_msg_op)(struct vhost_dev *dev, > struct vhost_iotlb_msg *imsg); > +typedef int (*vhost_set_config_op)(struct vhost_dev *dev, const uint8_t *config, > + size_t config_len); > +typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config, > + size_t config_len); > +typedef int (*vhost_set_config_fd_op)(struct vhost_dev *dev, int fd); > > typedef struct VhostOps { > VhostBackendType backend_type; > @@ -118,6 +123,9 @@ typedef struct VhostOps { > vhost_vsock_set_running_op vhost_vsock_set_running; > vhost_set_iotlb_callback_op vhost_set_iotlb_callback; > vhost_send_device_iotlb_msg_op vhost_send_device_iotlb_msg; > + vhost_get_config_op vhost_get_config; > + vhost_set_config_op vhost_set_config; > + vhost_set_config_fd_op vhost_set_config_fd; > } VhostOps; > > extern const VhostOps user_ops; > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > index 467dc77..ff172f2 100644 > --- a/include/hw/virtio/vhost.h > +++ b/include/hw/virtio/vhost.h > @@ -46,6 +46,12 @@ struct vhost_iommu { > QLIST_ENTRY(vhost_iommu) iommu_next; > }; > > +typedef struct VhostDevConfigOps { > + /* Vhost device config space changed callback > + */ > + void (*vhost_dev_config_notifier)(struct vhost_dev *dev); > +} VhostDevConfigOps; > + > struct vhost_memory; > struct vhost_dev { > VirtIODevice *vdev; > @@ -76,6 +82,8 @@ struct vhost_dev { > QLIST_ENTRY(vhost_dev) entry; > QLIST_HEAD(, vhost_iommu) iommu_list; > IOMMUNotifier n; > + EventNotifier config_notifier; > + const VhostDevConfigOps *config_ops; > }; > > int vhost_dev_init(struct vhost_dev *hdev, void *opaque, > @@ -106,4 +114,12 @@ int vhost_net_set_backend(struct vhost_dev *hdev, > struct vhost_vring_file *file); > > int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write); > +int vhost_dev_get_config(struct vhost_dev *dev, uint8_t *config, > + size_t config_len); > +int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *config, > + size_t config_len); > +/* notifier callback in case vhost device config space changed > + */ > +int vhost_dev_set_config_notifier(struct vhost_dev *dev, > + const VhostDevConfigOps *ops); > #endif >
On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote: > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled) > /* No-op as the receive channel is not dedicated to IOTLB messages. */ > } > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, > + size_t config_len) > +{ > + VhostUserMsg msg = { > + .request = VHOST_USER_GET_CONFIG, > + .flags = VHOST_USER_VERSION, > + .size = config_len, > + }; > + > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { config_len should be limited to 256 bytes: if (config_len == 0 || config_len > sizeof(msg.payload.config) { > + error_report("bad config length"); > + return -1; > + } > + > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > + > + if (vhost_user_read(dev, &msg) < 0) { > + return -1; > + } > + > + if (msg.request != VHOST_USER_GET_CONFIG) { > + error_report("Received unexpected msg type. Expected %d received %d", > + VHOST_USER_GET_CONFIG, msg.request); > + return -1; > + } > + > + if (msg.size != config_len) { > + error_report("Received bad msg size."); > + return -1; > + } > + > + memcpy(config, &msg.payload.config, config_len); There is some complexity here: different virtio devices use different amounts of config space. Devices may append new fields to the config space to support new features. Therefore I think the simplest protocol is to always fetch the full 256-byte configuration space. This way the vhost-user slave process can implement feature bits that the master process does not know about. In other words, I don't think the master process knows how much of the config space is used so it should always request 256 bytes. > + return 0; > +} > + > +static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *config, > + size_t config_len) > +{ > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + VhostUserMsg msg = { > + .request = VHOST_USER_SET_CONFIG, > + .flags = VHOST_USER_VERSION, > + .size = config_len, > + }; > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_REPLY_MASK; > + } > + > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { Same thing here: config_len > sizeof(msg.payload.config)
On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote: > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote: > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled) > > /* No-op as the receive channel is not dedicated to IOTLB messages. */ > > } > > > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, > > + size_t config_len) > > +{ > > + VhostUserMsg msg = { > > + .request = VHOST_USER_GET_CONFIG, > > + .flags = VHOST_USER_VERSION, > > + .size = config_len, > > + }; > > + > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { > > config_len should be limited to 256 bytes: > > if (config_len == 0 || config_len > sizeof(msg.payload.config) { I would just limit it to a reasonable value, acceptable to both master and slave, not fail if it's bigger. > > + error_report("bad config length"); > > + return -1; > > + } > > + > > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > > + return -1; > > + } > > + > > + if (vhost_user_read(dev, &msg) < 0) { > > + return -1; > > + } > > + > > + if (msg.request != VHOST_USER_GET_CONFIG) { > > + error_report("Received unexpected msg type. Expected %d received %d", > > + VHOST_USER_GET_CONFIG, msg.request); > > + return -1; > > + } > > + > > + if (msg.size != config_len) { > > + error_report("Received bad msg size."); > > + return -1; > > + } > > + > > + memcpy(config, &msg.payload.config, config_len); > > There is some complexity here: different virtio devices use different > amounts of config space. Devices may append new fields to the config > space to support new features. > > Therefore I think the simplest protocol is to always fetch the full > 256-byte configuration space. This way the vhost-user slave process can > implement feature bits that the master process does not know about. > > In other words, I don't think the master process knows how much of the > config space is used so it should always request 256 bytes. Each device knows the max config space size. vdev->config_len = config_size; I don't think we need to hard-code 256 bytes in there. > > + return 0; > > +} > > + > > +static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *config, > > + size_t config_len) > > +{ > > + bool reply_supported = virtio_has_feature(dev->protocol_features, > > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > > + > > + VhostUserMsg msg = { > > + .request = VHOST_USER_SET_CONFIG, > > + .flags = VHOST_USER_VERSION, > > + .size = config_len, > > + }; > > + > > + if (reply_supported) { > > + msg.flags |= VHOST_USER_NEED_REPLY_MASK; > > + } > > + > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { > > Same thing here: config_len > sizeof(msg.payload.config)
On Thu, Oct 19, 2017 at 01:24:07PM +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.liu@intel.com> I don't much like it that config is getting passed through. IMO this makes managing things harder not easier. How about specific messages about specific parts of config space that you want to get from the backend? > --- > docs/interop/vhost-user.txt | 33 +++++++++++++ > hw/virtio/vhost-user.c | 97 +++++++++++++++++++++++++++++++++++++++ > hw/virtio/vhost.c | 63 +++++++++++++++++++++++++ > include/hw/virtio/vhost-backend.h | 8 ++++ > include/hw/virtio/vhost.h | 16 +++++++ > 5 files changed, 217 insertions(+) > > diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt > index 954771d..ea4c5ca 100644 > --- a/docs/interop/vhost-user.txt > +++ b/docs/interop/vhost-user.txt > @@ -116,6 +116,10 @@ Depending on the request type, payload can be: > - 3: IOTLB invalidate > - 4: IOTLB access fail > > + * Virtio device config space > + > + 256 Bytes static virito device config space > + > In QEMU the vhost-user message is implemented with the following struct: > > typedef struct VhostUserMsg { > @@ -129,6 +133,7 @@ typedef struct VhostUserMsg { > VhostUserMemory memory; > VhostUserLog log; > struct vhost_iotlb_msg iotlb; > + uint8_t config[256]; > }; > } QEMU_PACKED VhostUserMsg; > > @@ -596,6 +601,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 config 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 config space and also can be used for live migration on the > + destination host. > + > +* VHOST_USER_SET_CONFIG_FD > + Id: 26 > + Equivalent ioctl: N/A > + Master payload: N/A > + > + Sets the notifier file descriptor, which is passed as ancillary data. > + Vhost-user slave uses the file descriptor to notify vhost-user master > + when the virtio config space changed. The vhost-user master can read > + the virtio config space to get the latest update. > + > Slave message types > ------------------- > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 093675e..4e7bafc 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -26,6 +26,11 @@ > #define VHOST_MEMORY_MAX_NREGIONS 8 > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > > +/* > + * Maximum size of virtio device config space > + */ > +#define VHOST_USER_MAX_CONFIG_SIZE 256 > + > enum VhostUserProtocolFeature { > VHOST_USER_PROTOCOL_F_MQ = 0, > VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1, > @@ -65,6 +70,9 @@ typedef enum VhostUserRequest { > VHOST_USER_SET_SLAVE_REQ_FD = 21, > VHOST_USER_IOTLB_MSG = 22, > VHOST_USER_SET_VRING_ENDIAN = 23, > + VHOST_USER_GET_CONFIG = 24, > + VHOST_USER_SET_CONFIG = 25, > + VHOST_USER_SET_CONFIG_FD = 26, > VHOST_USER_MAX > } VhostUserRequest; > > @@ -109,6 +117,7 @@ typedef struct VhostUserMsg { > VhostUserMemory memory; > VhostUserLog log; > struct vhost_iotlb_msg iotlb; > + uint8_t config[VHOST_USER_MAX_CONFIG_SIZE]; > } payload; > } QEMU_PACKED VhostUserMsg; > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled) > /* No-op as the receive channel is not dedicated to IOTLB messages. */ > } > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, > + size_t config_len) > +{ > + VhostUserMsg msg = { > + .request = VHOST_USER_GET_CONFIG, > + .flags = VHOST_USER_VERSION, > + .size = config_len, > + }; > + > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { > + error_report("bad config length"); > + return -1; > + } > + > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > + > + if (vhost_user_read(dev, &msg) < 0) { > + return -1; > + } > + > + if (msg.request != VHOST_USER_GET_CONFIG) { > + error_report("Received unexpected msg type. Expected %d received %d", > + VHOST_USER_GET_CONFIG, msg.request); > + return -1; > + } > + > + if (msg.size != config_len) { > + error_report("Received bad msg size."); > + return -1; > + } > + > + memcpy(config, &msg.payload.config, config_len); > + > + return 0; > +} > + > +static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *config, > + size_t config_len) > +{ > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + VhostUserMsg msg = { > + .request = VHOST_USER_SET_CONFIG, > + .flags = VHOST_USER_VERSION, > + .size = config_len, > + }; > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_REPLY_MASK; > + } > + > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { > + error_report("bad config length"); > + return -1; > + } > + > + memcpy(&msg.payload.config, config, config_len); > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > + > + if (reply_supported) { > + return process_message_reply(dev, &msg); > + } > + > + return 0; > +} > + > +static int vhost_user_set_config_fd(struct vhost_dev *dev, int fd) > +{ > + VhostUserMsg msg = { > + .request = VHOST_USER_SET_CONFIG_FD, > + .flags = VHOST_USER_VERSION, > + }; > + > + if (vhost_user_write(dev, &msg, &fd, 1) < 0) { > + return -1; > + } > + > + return 0; > +} > + > const VhostOps user_ops = { > .backend_type = VHOST_BACKEND_TYPE_USER, > .vhost_backend_init = vhost_user_init, > @@ -948,4 +1042,7 @@ const VhostOps user_ops = { > .vhost_net_set_mtu = vhost_user_net_set_mtu, > .vhost_set_iotlb_callback = vhost_user_set_iotlb_callback, > .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg, > + .vhost_get_config = vhost_user_get_config, > + .vhost_set_config = vhost_user_set_config, > + .vhost_set_config_fd = vhost_user_set_config_fd, > }; > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index ddc42f0..8079f46 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -1354,6 +1354,9 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) > for (i = 0; i < hdev->nvqs; ++i) { > vhost_virtqueue_cleanup(hdev->vqs + i); > } > + if (hdev->config_ops) { > + event_notifier_cleanup(&hdev->config_notifier); > + } > if (hdev->mem) { > /* those are only safe after successful init */ > memory_listener_unregister(&hdev->memory_listener); > @@ -1501,6 +1504,66 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits, > } > } > > +int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config, > + size_t config_len) > +{ > + assert(hdev->vhost_ops); > + > + if (hdev->vhost_ops->vhost_get_config) { > + return hdev->vhost_ops->vhost_get_config(hdev, config, config_len); > + } > + > + return 0; > +} > + > +int vhost_dev_set_config(struct vhost_dev *hdev, const uint8_t *config, > + size_t config_len) > +{ > + assert(hdev->vhost_ops); > + > + if (hdev->vhost_ops->vhost_set_config) { > + return hdev->vhost_ops->vhost_set_config(hdev, config, config_len); > + } > + > + return 0; > +} > + > +static void vhost_dev_config_notifier_read(EventNotifier *n) > +{ > + struct vhost_dev *hdev = container_of(n, struct vhost_dev, > + config_notifier); > + > + if (event_notifier_test_and_clear(n)) { > + if (hdev->config_ops) { > + hdev->config_ops->vhost_dev_config_notifier(hdev); > + } > + } > +} > + > +int vhost_dev_set_config_notifier(struct vhost_dev *hdev, > + const VhostDevConfigOps *ops) > +{ > + int r, fd; > + > + assert(hdev->vhost_ops); > + > + r = event_notifier_init(&hdev->config_notifier, 0); > + if (r < 0) { > + return r; > + } > + > + hdev->config_ops = ops; > + event_notifier_set_handler(&hdev->config_notifier, > + vhost_dev_config_notifier_read); > + > + if (hdev->vhost_ops->vhost_set_config_fd) { > + fd = event_notifier_get_fd(&hdev->config_notifier); > + return hdev->vhost_ops->vhost_set_config_fd(hdev, fd); > + } > + > + return 0; > +} > + > /* Host notifiers must be enabled at this point. */ > int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) > { > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h > index a7a5f22..df6769e 100644 > --- a/include/hw/virtio/vhost-backend.h > +++ b/include/hw/virtio/vhost-backend.h > @@ -84,6 +84,11 @@ typedef void (*vhost_set_iotlb_callback_op)(struct vhost_dev *dev, > int enabled); > typedef int (*vhost_send_device_iotlb_msg_op)(struct vhost_dev *dev, > struct vhost_iotlb_msg *imsg); > +typedef int (*vhost_set_config_op)(struct vhost_dev *dev, const uint8_t *config, > + size_t config_len); > +typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config, > + size_t config_len); > +typedef int (*vhost_set_config_fd_op)(struct vhost_dev *dev, int fd); > > typedef struct VhostOps { > VhostBackendType backend_type; > @@ -118,6 +123,9 @@ typedef struct VhostOps { > vhost_vsock_set_running_op vhost_vsock_set_running; > vhost_set_iotlb_callback_op vhost_set_iotlb_callback; > vhost_send_device_iotlb_msg_op vhost_send_device_iotlb_msg; > + vhost_get_config_op vhost_get_config; > + vhost_set_config_op vhost_set_config; > + vhost_set_config_fd_op vhost_set_config_fd; > } VhostOps; > > extern const VhostOps user_ops; > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > index 467dc77..ff172f2 100644 > --- a/include/hw/virtio/vhost.h > +++ b/include/hw/virtio/vhost.h > @@ -46,6 +46,12 @@ struct vhost_iommu { > QLIST_ENTRY(vhost_iommu) iommu_next; > }; > > +typedef struct VhostDevConfigOps { > + /* Vhost device config space changed callback > + */ > + void (*vhost_dev_config_notifier)(struct vhost_dev *dev); > +} VhostDevConfigOps; > + > struct vhost_memory; > struct vhost_dev { > VirtIODevice *vdev; > @@ -76,6 +82,8 @@ struct vhost_dev { > QLIST_ENTRY(vhost_dev) entry; > QLIST_HEAD(, vhost_iommu) iommu_list; > IOMMUNotifier n; > + EventNotifier config_notifier; > + const VhostDevConfigOps *config_ops; > }; > > int vhost_dev_init(struct vhost_dev *hdev, void *opaque, > @@ -106,4 +114,12 @@ int vhost_net_set_backend(struct vhost_dev *hdev, > struct vhost_vring_file *file); > > int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write); > +int vhost_dev_get_config(struct vhost_dev *dev, uint8_t *config, > + size_t config_len); > +int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *config, > + size_t config_len); > +/* notifier callback in case vhost device config space changed > + */ > +int vhost_dev_set_config_notifier(struct vhost_dev *dev, > + const VhostDevConfigOps *ops); > #endif > -- > 1.9.3
On 19/10/2017 17:39, Michael S. Tsirkin 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.liu@intel.com> > I don't much like it that config is getting passed through. > > IMO this makes managing things harder not easier. > > How about specific messages about specific parts of > config space that you want to get from the backend? In the case of virtio-blk that would be all of it. Do you have a case in mind where some part of the configuration space is owned by QEMU? Paolo
On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote: > On 19/10/2017 17:39, Michael S. Tsirkin 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.liu@intel.com> > > I don't much like it that config is getting passed through. > > > > IMO this makes managing things harder not easier. > > > > How about specific messages about specific parts of > > config space that you want to get from the backend? > > In the case of virtio-blk that would be all of it. Do you have a case > in mind where some part of the configuration space is owned by QEMU? > > Paolo Yes. seg_max
On 19/10/2017 19:43, Michael S. Tsirkin wrote: > On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote: >> On 19/10/2017 17:39, Michael S. Tsirkin 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.liu@intel.com> >>> I don't much like it that config is getting passed through. >>> >>> IMO this makes managing things harder not easier. >>> >>> How about specific messages about specific parts of >>> config space that you want to get from the backend? >> >> In the case of virtio-blk that would be all of it. Do you have a case >> in mind where some part of the configuration space is owned by QEMU? >> >> Paolo > > Yes. seg_max The seg_max limit is established by whoever reads buffers from the vring and passes them down to the lower layer. For vhost-blk that's the device server, not QEMU. Paolo
On Thu, Oct 19, 2017 at 11:04:48PM +0200, Paolo Bonzini wrote: > On 19/10/2017 19:43, Michael S. Tsirkin wrote: > > On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote: > >> On 19/10/2017 17:39, Michael S. Tsirkin 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.liu@intel.com> > >>> I don't much like it that config is getting passed through. > >>> > >>> IMO this makes managing things harder not easier. > >>> > >>> How about specific messages about specific parts of > >>> config space that you want to get from the backend? > >> > >> In the case of virtio-blk that would be all of it. Do you have a case > >> in mind where some part of the configuration space is owned by QEMU? > >> > >> Paolo > > > > Yes. seg_max > > The seg_max limit is established by whoever reads buffers from the vring > and passes them down to the lower layer. For vhost-blk that's the > device server, not QEMU. > > Paolo Good point. How about num_queues though? Also why is there SET_CONFIG? Does not look like blk uses it. And I wonder how do we do it for other devices. E.g. for net there's a bit in the middle of the config field that deals with migration.
> -----Original Message----- > From: Michael S. Tsirkin [mailto:mst@redhat.com] > Sent: Friday, October 20, 2017 8:28 AM > To: Paolo Bonzini <pbonzini@redhat.com> > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org; > stefanha@gmail.com; marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, > James R <james.r.harris@intel.com> > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support > virtio config space > > On Thu, Oct 19, 2017 at 11:04:48PM +0200, Paolo Bonzini wrote: > > On 19/10/2017 19:43, Michael S. Tsirkin wrote: > > > On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote: > > >> On 19/10/2017 17:39, Michael S. Tsirkin 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.liu@intel.com> > > >>> I don't much like it that config is getting passed through. > > >>> > > >>> IMO this makes managing things harder not easier. > > >>> > > >>> How about specific messages about specific parts of > > >>> config space that you want to get from the backend? > > >> > > >> In the case of virtio-blk that would be all of it. Do you have a case > > >> in mind where some part of the configuration space is owned by QEMU? > > >> > > >> Paolo > > > > > > Yes. seg_max > > > > The seg_max limit is established by whoever reads buffers from the vring > > and passes them down to the lower layer. For vhost-blk that's the > > device server, not QEMU. > > > > Paolo > > Good point. How about num_queues though? num_queues is part of virtio_blk config, vhost-user slave can set it, and Qemu driver can rewrite it if user want less IO queues. > > Also why is there SET_CONFIG? Does not look like blk uses it. Only one possible usage when disable write cache to vhost-user slave device. > > And I wonder how do we do it for other devices. > > E.g. for net there's a bit in the middle of the > config field that deals with migration. Well, I'm okay to make those messages only valid for virtio block device, because it's enough for virtio block to be started with vhost-user slave target. > > > -- > MST
On Fri, Oct 20, 2017 at 01:55:20AM +0000, Liu, Changpeng wrote: > > > > -----Original Message----- > > From: Michael S. Tsirkin [mailto:mst@redhat.com] > > Sent: Friday, October 20, 2017 8:28 AM > > To: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org; > > stefanha@gmail.com; marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, > > James R <james.r.harris@intel.com> > > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support > > virtio config space > > > > On Thu, Oct 19, 2017 at 11:04:48PM +0200, Paolo Bonzini wrote: > > > On 19/10/2017 19:43, Michael S. Tsirkin wrote: > > > > On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote: > > > >> On 19/10/2017 17:39, Michael S. Tsirkin 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.liu@intel.com> > > > >>> I don't much like it that config is getting passed through. > > > >>> > > > >>> IMO this makes managing things harder not easier. > > > >>> > > > >>> How about specific messages about specific parts of > > > >>> config space that you want to get from the backend? > > > >> > > > >> In the case of virtio-blk that would be all of it. Do you have a case > > > >> in mind where some part of the configuration space is owned by QEMU? > > > >> > > > >> Paolo > > > > > > > > Yes. seg_max > > > > > > The seg_max limit is established by whoever reads buffers from the vring > > > and passes them down to the lower layer. For vhost-blk that's the > > > device server, not QEMU. > > > > > > Paolo > > > > Good point. How about num_queues though? > num_queues is part of virtio_blk config, vhost-user slave can set it, > and Qemu driver can rewrite it if user want less IO queues. Fundamentally QEMU needs to support this # of queues for this device. So whenever QEMU doesn't always expose config space as-is, need to document the exact semantics. Also, does backend need to know? > > > > Also why is there SET_CONFIG? Does not look like blk uses it. > Only one possible usage when disable write cache to vhost-user slave device. Again need to add documentation what can be written. > > > > And I wonder how do we do it for other devices. > > > > E.g. for net there's a bit in the middle of the > > config field that deals with migration. > Well, I'm okay to make those messages only valid for virtio block device, because it's enough > for virtio block to be started with vhost-user slave target. OK but I'd rather make them at least somewhat generic so we can reuse them down the road. It looks like adding offset/size pair would solve most of the issues. Thoughts? > > > > > > -- > > MST
> -----Original Message----- > From: Michael S. Tsirkin [mailto:mst@redhat.com] > Sent: Friday, October 20, 2017 10:12 AM > To: Liu, Changpeng <changpeng.liu@intel.com> > Cc: Paolo Bonzini <pbonzini@redhat.com>; qemu-devel@nongnu.org; > stefanha@gmail.com; marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, > James R <james.r.harris@intel.com> > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support > virtio config space > > On Fri, Oct 20, 2017 at 01:55:20AM +0000, Liu, Changpeng wrote: > > > > > > > -----Original Message----- > > > From: Michael S. Tsirkin [mailto:mst@redhat.com] > > > Sent: Friday, October 20, 2017 8:28 AM > > > To: Paolo Bonzini <pbonzini@redhat.com> > > > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org; > > > stefanha@gmail.com; marcandre.lureau@redhat.com; felipe@nutanix.com; > Harris, > > > James R <james.r.harris@intel.com> > > > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support > > > virtio config space > > > > > > On Thu, Oct 19, 2017 at 11:04:48PM +0200, Paolo Bonzini wrote: > > > > On 19/10/2017 19:43, Michael S. Tsirkin wrote: > > > > > On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote: > > > > >> On 19/10/2017 17:39, Michael S. Tsirkin 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.liu@intel.com> > > > > >>> I don't much like it that config is getting passed through. > > > > >>> > > > > >>> IMO this makes managing things harder not easier. > > > > >>> > > > > >>> How about specific messages about specific parts of > > > > >>> config space that you want to get from the backend? > > > > >> > > > > >> In the case of virtio-blk that would be all of it. Do you have a case > > > > >> in mind where some part of the configuration space is owned by QEMU? > > > > >> > > > > >> Paolo > > > > > > > > > > Yes. seg_max > > > > > > > > The seg_max limit is established by whoever reads buffers from the vring > > > > and passes them down to the lower layer. For vhost-blk that's the > > > > device server, not QEMU. > > > > > > > > Paolo > > > > > > Good point. How about num_queues though? > > num_queues is part of virtio_blk config, vhost-user slave can set it, > > and Qemu driver can rewrite it if user want less IO queues. > > Fundamentally QEMU needs to support this # of queues for this > device. > > So whenever QEMU doesn't always expose config space as-is, > need to document the exact semantics. Agreed, Qemu vhost block driver should always has a default value, so I also added the value as one of the parameters for vhost block driver. > > Also, does backend need to know? vhost-user slave does know how many queues are used, because vhost-user messages such as SET_VRING_CALL/KICK are related with queues. Here the idea is vhost-user slave provides the maximum io queues supported, and Qemu users can specify lower io queues. > > > > > > > > Also why is there SET_CONFIG? Does not look like blk uses it. > > Only one possible usage when disable write cache to vhost-user slave device. > > Again need to add documentation what can be written. Agreed. > > > > > > > > And I wonder how do we do it for other devices. > > > > > > E.g. for net there's a bit in the middle of the > > > config field that deals with migration. > > Well, I'm okay to make those messages only valid for virtio block device, because > it's enough > > for virtio block to be started with vhost-user slave target. > > OK but I'd rather make them at least somewhat generic so we can reuse > them down the road. It looks like adding offset/size pair would solve > most of the issues. Thoughts? Do you mean SET_CONFIG message followed with offset/size to let vhost-user slave Know which field the master want to change? Yes, sounds good to me. > > > > > > > > > > -- > > > MST
On Thu, Oct 19, 2017 at 06:36:00PM +0300, Michael S. Tsirkin wrote: > On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote: > > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote: > > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled) > > > /* No-op as the receive channel is not dedicated to IOTLB messages. */ > > > } > > > > > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, > > > + size_t config_len) > > > +{ > > > + VhostUserMsg msg = { > > > + .request = VHOST_USER_GET_CONFIG, > > > + .flags = VHOST_USER_VERSION, > > > + .size = config_len, > > > + }; > > > + > > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { > > > > config_len should be limited to 256 bytes: > > > > if (config_len == 0 || config_len > sizeof(msg.payload.config) { > > I would just limit it to a reasonable value, acceptable to > both master and slave, not fail if it's bigger. > > > > > + error_report("bad config length"); > > > + return -1; > > > + } > > > + > > > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > > > + return -1; > > > + } > > > + > > > + if (vhost_user_read(dev, &msg) < 0) { > > > + return -1; > > > + } > > > + > > > + if (msg.request != VHOST_USER_GET_CONFIG) { > > > + error_report("Received unexpected msg type. Expected %d received %d", > > > + VHOST_USER_GET_CONFIG, msg.request); > > > + return -1; > > > + } > > > + > > > + if (msg.size != config_len) { > > > + error_report("Received bad msg size."); > > > + return -1; > > > + } > > > + > > > + memcpy(config, &msg.payload.config, config_len); > > > > There is some complexity here: different virtio devices use different > > amounts of config space. Devices may append new fields to the config > > space to support new features. > > > > Therefore I think the simplest protocol is to always fetch the full > > 256-byte configuration space. This way the vhost-user slave process can > > implement feature bits that the master process does not know about. > > > > In other words, I don't think the master process knows how much of the > > config space is used so it should always request 256 bytes. > > Each device knows the max config space size. > > vdev->config_len = config_size; I see you're referring to the field that is set in: void virtio_init(VirtIODevice *vdev, const char *name, uint16_t device_id, size_t config_size) How does this work for vhost-user where different slave programs may offer different config sizes? The QEMU master process does not know the correct size ahead of time. The size depends on the vhost-user slave process. This is why I suggest using the full 256 bytes that the VIRTIO spec defines.
> -----Original Message----- > From: Stefan Hajnoczi [mailto:stefanha@gmail.com] > Sent: Friday, October 20, 2017 6:01 PM > To: Michael S. Tsirkin <mst@redhat.com> > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org; > pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com; > Harris, James R <james.r.harris@intel.com> > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support > virtio config space > > On Thu, Oct 19, 2017 at 06:36:00PM +0300, Michael S. Tsirkin wrote: > > On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote: > > > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote: > > > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct > vhost_dev *dev, int enabled) > > > > /* No-op as the receive channel is not dedicated to IOTLB messages. */ > > > > } > > > > > > > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, > > > > + size_t config_len) > > > > +{ > > > > + VhostUserMsg msg = { > > > > + .request = VHOST_USER_GET_CONFIG, > > > > + .flags = VHOST_USER_VERSION, > > > > + .size = config_len, > > > > + }; > > > > + > > > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { > > > > > > config_len should be limited to 256 bytes: > > > > > > if (config_len == 0 || config_len > sizeof(msg.payload.config) { > > > > I would just limit it to a reasonable value, acceptable to > > both master and slave, not fail if it's bigger. > > > > > > > > + error_report("bad config length"); > > > > + return -1; > > > > + } > > > > + > > > > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > > > > + return -1; > > > > + } > > > > + > > > > + if (vhost_user_read(dev, &msg) < 0) { > > > > + return -1; > > > > + } > > > > + > > > > + if (msg.request != VHOST_USER_GET_CONFIG) { > > > > + error_report("Received unexpected msg type. Expected %d > received %d", > > > > + VHOST_USER_GET_CONFIG, msg.request); > > > > + return -1; > > > > + } > > > > + > > > > + if (msg.size != config_len) { > > > > + error_report("Received bad msg size."); > > > > + return -1; > > > > + } > > > > + > > > > + memcpy(config, &msg.payload.config, config_len); > > > > > > There is some complexity here: different virtio devices use different > > > amounts of config space. Devices may append new fields to the config > > > space to support new features. > > > > > > Therefore I think the simplest protocol is to always fetch the full > > > 256-byte configuration space. This way the vhost-user slave process can > > > implement feature bits that the master process does not know about. > > > > > > In other words, I don't think the master process knows how much of the > > > config space is used so it should always request 256 bytes. > > > > Each device knows the max config space size. > > > > vdev->config_len = config_size; > > I see you're referring to the field that is set in: > > void virtio_init(VirtIODevice *vdev, const char *name, > uint16_t device_id, size_t config_size) > > How does this work for vhost-user where different slave programs may > offer different config sizes? Each Qemu vhost controller e.g: vhost-user-scsi-pci and vhost-user-blk-pci should has different char devices, so vhost-slave knows those messages are from vhost-scsi or vhost-blk, of course, each UNIX domain socket should be assigned by users with types: vhsot-scsi or vhost-blk. > > The QEMU master process does not know the correct size ahead of time. > The size depends on the vhost-user slave process. This is why I suggest > using the full 256 bytes that the VIRTIO spec defines.
On Mon, Oct 23, 2017 at 04:47:00AM +0000, Liu, Changpeng wrote: > > > > -----Original Message----- > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com] > > Sent: Friday, October 20, 2017 6:01 PM > > To: Michael S. Tsirkin <mst@redhat.com> > > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org; > > pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com; > > Harris, James R <james.r.harris@intel.com> > > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support > > virtio config space > > > > On Thu, Oct 19, 2017 at 06:36:00PM +0300, Michael S. Tsirkin wrote: > > > On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote: > > > > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote: > > > > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct > > vhost_dev *dev, int enabled) > > > > > /* No-op as the receive channel is not dedicated to IOTLB messages. */ > > > > > } > > > > > > > > > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, > > > > > + size_t config_len) > > > > > +{ > > > > > + VhostUserMsg msg = { > > > > > + .request = VHOST_USER_GET_CONFIG, > > > > > + .flags = VHOST_USER_VERSION, > > > > > + .size = config_len, > > > > > + }; > > > > > + > > > > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { > > > > > > > > config_len should be limited to 256 bytes: > > > > > > > > if (config_len == 0 || config_len > sizeof(msg.payload.config) { > > > > > > I would just limit it to a reasonable value, acceptable to > > > both master and slave, not fail if it's bigger. > > > > > > > > > > > + error_report("bad config length"); > > > > > + return -1; > > > > > + } > > > > > + > > > > > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > > > > > + return -1; > > > > > + } > > > > > + > > > > > + if (vhost_user_read(dev, &msg) < 0) { > > > > > + return -1; > > > > > + } > > > > > + > > > > > + if (msg.request != VHOST_USER_GET_CONFIG) { > > > > > + error_report("Received unexpected msg type. Expected %d > > received %d", > > > > > + VHOST_USER_GET_CONFIG, msg.request); > > > > > + return -1; > > > > > + } > > > > > + > > > > > + if (msg.size != config_len) { > > > > > + error_report("Received bad msg size."); > > > > > + return -1; > > > > > + } > > > > > + > > > > > + memcpy(config, &msg.payload.config, config_len); > > > > > > > > There is some complexity here: different virtio devices use different > > > > amounts of config space. Devices may append new fields to the config > > > > space to support new features. > > > > > > > > Therefore I think the simplest protocol is to always fetch the full > > > > 256-byte configuration space. This way the vhost-user slave process can > > > > implement feature bits that the master process does not know about. > > > > > > > > In other words, I don't think the master process knows how much of the > > > > config space is used so it should always request 256 bytes. > > > > > > Each device knows the max config space size. > > > > > > vdev->config_len = config_size; > > > > I see you're referring to the field that is set in: > > > > void virtio_init(VirtIODevice *vdev, const char *name, > > uint16_t device_id, size_t config_size) > > > > How does this work for vhost-user where different slave programs may > > offer different config sizes? > Each Qemu vhost controller e.g: vhost-user-scsi-pci and vhost-user-blk-pci should has different char devices, > so vhost-slave knows those messages are from vhost-scsi or vhost-blk, of course, each UNIX domain socket > should be assigned by users with types: vhsot-scsi or vhost-blk. We're talking about different things. Here is an example illustrating my question: vhost-user-blk slave A only knows about struct virtio_blk_config fields up to wce (VIRTIO 1.0). See http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-2070004. vhost-user-blk slave B implements struct virtio_blk_config with the new num_queues field. See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/virtio_blk.h#n56. Slaves A and B use different struct virtio_blk_config sizes! Which config size should the vhost-master use? There is currently no way to query the size from the slave. What should slave programs do when the master requests configuration space data that is the wrong size? I think the simplest answer is that the master always uses 256 bytes. Slaves also keep the full 256 bytes stored but their device implementation may access fewer bytes. > > The QEMU master process does not know the correct size ahead of time. > > The size depends on the vhost-user slave process. This is why I suggest > > using the full 256 bytes that the VIRTIO spec defines.
> -----Original Message----- > From: Stefan Hajnoczi [mailto:stefanha@gmail.com] > Sent: Tuesday, October 24, 2017 1:26 AM > To: Liu, Changpeng <changpeng.liu@intel.com> > Cc: Michael S. Tsirkin <mst@redhat.com>; qemu-devel@nongnu.org; > pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com; > Harris, James R <james.r.harris@intel.com> > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support > virtio config space > > On Mon, Oct 23, 2017 at 04:47:00AM +0000, Liu, Changpeng wrote: > > > > > > > -----Original Message----- > > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com] > > > Sent: Friday, October 20, 2017 6:01 PM > > > To: Michael S. Tsirkin <mst@redhat.com> > > > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org; > > > pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com; > > > Harris, James R <james.r.harris@intel.com> > > > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to > support > > > virtio config space > > > > > > On Thu, Oct 19, 2017 at 06:36:00PM +0300, Michael S. Tsirkin wrote: > > > > On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote: > > > > > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote: > > > > > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct > > > vhost_dev *dev, int enabled) > > > > > > /* No-op as the receive channel is not dedicated to IOTLB messages. > */ > > > > > > } > > > > > > > > > > > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, > > > > > > + size_t config_len) > > > > > > +{ > > > > > > + VhostUserMsg msg = { > > > > > > + .request = VHOST_USER_GET_CONFIG, > > > > > > + .flags = VHOST_USER_VERSION, > > > > > > + .size = config_len, > > > > > > + }; > > > > > > + > > > > > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { > > > > > > > > > > config_len should be limited to 256 bytes: > > > > > > > > > > if (config_len == 0 || config_len > sizeof(msg.payload.config) { > > > > > > > > I would just limit it to a reasonable value, acceptable to > > > > both master and slave, not fail if it's bigger. > > > > > > > > > > > > > > + error_report("bad config length"); > > > > > > + return -1; > > > > > > + } > > > > > > + > > > > > > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > > > > > > + return -1; > > > > > > + } > > > > > > + > > > > > > + if (vhost_user_read(dev, &msg) < 0) { > > > > > > + return -1; > > > > > > + } > > > > > > + > > > > > > + if (msg.request != VHOST_USER_GET_CONFIG) { > > > > > > + error_report("Received unexpected msg type. Expected %d > > > received %d", > > > > > > + VHOST_USER_GET_CONFIG, msg.request); > > > > > > + return -1; > > > > > > + } > > > > > > + > > > > > > + if (msg.size != config_len) { > > > > > > + error_report("Received bad msg size."); > > > > > > + return -1; > > > > > > + } > > > > > > + > > > > > > + memcpy(config, &msg.payload.config, config_len); > > > > > > > > > > There is some complexity here: different virtio devices use different > > > > > amounts of config space. Devices may append new fields to the config > > > > > space to support new features. > > > > > > > > > > Therefore I think the simplest protocol is to always fetch the full > > > > > 256-byte configuration space. This way the vhost-user slave process can > > > > > implement feature bits that the master process does not know about. > > > > > > > > > > In other words, I don't think the master process knows how much of the > > > > > config space is used so it should always request 256 bytes. > > > > > > > > Each device knows the max config space size. > > > > > > > > vdev->config_len = config_size; > > > > > > I see you're referring to the field that is set in: > > > > > > void virtio_init(VirtIODevice *vdev, const char *name, > > > uint16_t device_id, size_t config_size) > > > > > > How does this work for vhost-user where different slave programs may > > > offer different config sizes? > > Each Qemu vhost controller e.g: vhost-user-scsi-pci and vhost-user-blk-pci > should has different char devices, > > so vhost-slave knows those messages are from vhost-scsi or vhost-blk, of > course, each UNIX domain socket > > should be assigned by users with types: vhsot-scsi or vhost-blk. > > We're talking about different things. Here is an example illustrating > my question: > > vhost-user-blk slave A only knows about struct virtio_blk_config fields > up to wce (VIRTIO 1.0). See > http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1- > 2070004. > > vhost-user-blk slave B implements struct virtio_blk_config with the new > num_queues field. See > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/u > api/linux/virtio_blk.h#n56. > > Slaves A and B use different struct virtio_blk_config sizes! > > Which config size should the vhost-master use? There is currently no > way to query the size from the slave. > > What should slave programs do when the master requests configuration > space data that is the wrong size? > > I think the simplest answer is that the master always uses 256 bytes. > Slaves also keep the full 256 bytes stored but their device > implementation may access fewer bytes. Yes, clear now. How about the following configuration: struct vhost_dev_config { unsigned int offset; unsigned int size; uint8_t config[256]; }; The master always uses 256 Bytes, but with additional 2 parameters to locate detailed configuration fields, I think this combined Michael and your comments about this patch. > > > > The QEMU master process does not know the correct size ahead of time. > > > The size depends on the vhost-user slave process. This is why I suggest > > > using the full 256 bytes that the VIRTIO spec defines.
On Tue, Oct 24, 2017 at 12:52:28AM +0000, Liu, Changpeng wrote: > > > > -----Original Message----- > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com] > > Sent: Tuesday, October 24, 2017 1:26 AM > > To: Liu, Changpeng <changpeng.liu@intel.com> > > Cc: Michael S. Tsirkin <mst@redhat.com>; qemu-devel@nongnu.org; > > pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com; > > Harris, James R <james.r.harris@intel.com> > > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support > > virtio config space > > > > On Mon, Oct 23, 2017 at 04:47:00AM +0000, Liu, Changpeng wrote: > > > > > > > > > > -----Original Message----- > > > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com] > > > > Sent: Friday, October 20, 2017 6:01 PM > > > > To: Michael S. Tsirkin <mst@redhat.com> > > > > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org; > > > > pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com; > > > > Harris, James R <james.r.harris@intel.com> > > > > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to > > support > > > > virtio config space > > > > > > > > On Thu, Oct 19, 2017 at 06:36:00PM +0300, Michael S. Tsirkin wrote: > > > > > On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote: > > > > > > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote: > > > > > > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct > > > > vhost_dev *dev, int enabled) > > > > > > > /* No-op as the receive channel is not dedicated to IOTLB messages. > > */ > > > > > > > } > > > > > > > > > > > > > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, > > > > > > > + size_t config_len) > > > > > > > +{ > > > > > > > + VhostUserMsg msg = { > > > > > > > + .request = VHOST_USER_GET_CONFIG, > > > > > > > + .flags = VHOST_USER_VERSION, > > > > > > > + .size = config_len, > > > > > > > + }; > > > > > > > + > > > > > > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { > > > > > > > > > > > > config_len should be limited to 256 bytes: > > > > > > > > > > > > if (config_len == 0 || config_len > sizeof(msg.payload.config) { > > > > > > > > > > I would just limit it to a reasonable value, acceptable to > > > > > both master and slave, not fail if it's bigger. > > > > > > > > > > > > > > > > > + error_report("bad config length"); > > > > > > > + return -1; > > > > > > > + } > > > > > > > + > > > > > > > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > > > > > > > + return -1; > > > > > > > + } > > > > > > > + > > > > > > > + if (vhost_user_read(dev, &msg) < 0) { > > > > > > > + return -1; > > > > > > > + } > > > > > > > + > > > > > > > + if (msg.request != VHOST_USER_GET_CONFIG) { > > > > > > > + error_report("Received unexpected msg type. Expected %d > > > > received %d", > > > > > > > + VHOST_USER_GET_CONFIG, msg.request); > > > > > > > + return -1; > > > > > > > + } > > > > > > > + > > > > > > > + if (msg.size != config_len) { > > > > > > > + error_report("Received bad msg size."); > > > > > > > + return -1; > > > > > > > + } > > > > > > > + > > > > > > > + memcpy(config, &msg.payload.config, config_len); > > > > > > > > > > > > There is some complexity here: different virtio devices use different > > > > > > amounts of config space. Devices may append new fields to the config > > > > > > space to support new features. > > > > > > > > > > > > Therefore I think the simplest protocol is to always fetch the full > > > > > > 256-byte configuration space. This way the vhost-user slave process can > > > > > > implement feature bits that the master process does not know about. > > > > > > > > > > > > In other words, I don't think the master process knows how much of the > > > > > > config space is used so it should always request 256 bytes. > > > > > > > > > > Each device knows the max config space size. > > > > > > > > > > vdev->config_len = config_size; > > > > > > > > I see you're referring to the field that is set in: > > > > > > > > void virtio_init(VirtIODevice *vdev, const char *name, > > > > uint16_t device_id, size_t config_size) > > > > > > > > How does this work for vhost-user where different slave programs may > > > > offer different config sizes? > > > Each Qemu vhost controller e.g: vhost-user-scsi-pci and vhost-user-blk-pci > > should has different char devices, > > > so vhost-slave knows those messages are from vhost-scsi or vhost-blk, of > > course, each UNIX domain socket > > > should be assigned by users with types: vhsot-scsi or vhost-blk. > > > > We're talking about different things. Here is an example illustrating > > my question: > > > > vhost-user-blk slave A only knows about struct virtio_blk_config fields > > up to wce (VIRTIO 1.0). See > > http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1- > > 2070004. > > > > vhost-user-blk slave B implements struct virtio_blk_config with the new > > num_queues field. See > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/u > > api/linux/virtio_blk.h#n56. > > > > Slaves A and B use different struct virtio_blk_config sizes! > > > > Which config size should the vhost-master use? There is currently no > > way to query the size from the slave. > > > > What should slave programs do when the master requests configuration > > space data that is the wrong size? > > > > I think the simplest answer is that the master always uses 256 bytes. > > Slaves also keep the full 256 bytes stored but their device > > implementation may access fewer bytes. > Yes, clear now. How about the following configuration: > struct vhost_dev_config { > unsigned int offset; > unsigned int size; > uint8_t config[256]; > }; > The master always uses 256 Bytes, but with additional 2 parameters to locate detailed configuration fields, I think this combined > Michael and your comments about this patch. Yes, that's probably a good solution. Stefan
diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt index 954771d..ea4c5ca 100644 --- a/docs/interop/vhost-user.txt +++ b/docs/interop/vhost-user.txt @@ -116,6 +116,10 @@ Depending on the request type, payload can be: - 3: IOTLB invalidate - 4: IOTLB access fail + * Virtio device config space + + 256 Bytes static virito device config space + In QEMU the vhost-user message is implemented with the following struct: typedef struct VhostUserMsg { @@ -129,6 +133,7 @@ typedef struct VhostUserMsg { VhostUserMemory memory; VhostUserLog log; struct vhost_iotlb_msg iotlb; + uint8_t config[256]; }; } QEMU_PACKED VhostUserMsg; @@ -596,6 +601,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 config 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 config space and also can be used for live migration on the + destination host. + +* VHOST_USER_SET_CONFIG_FD + Id: 26 + Equivalent ioctl: N/A + Master payload: N/A + + Sets the notifier file descriptor, which is passed as ancillary data. + Vhost-user slave uses the file descriptor to notify vhost-user master + when the virtio config space changed. The vhost-user master can read + the virtio config space to get the latest update. + Slave message types ------------------- diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 093675e..4e7bafc 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -26,6 +26,11 @@ #define VHOST_MEMORY_MAX_NREGIONS 8 #define VHOST_USER_F_PROTOCOL_FEATURES 30 +/* + * Maximum size of virtio device config space + */ +#define VHOST_USER_MAX_CONFIG_SIZE 256 + enum VhostUserProtocolFeature { VHOST_USER_PROTOCOL_F_MQ = 0, VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1, @@ -65,6 +70,9 @@ typedef enum VhostUserRequest { VHOST_USER_SET_SLAVE_REQ_FD = 21, VHOST_USER_IOTLB_MSG = 22, VHOST_USER_SET_VRING_ENDIAN = 23, + VHOST_USER_GET_CONFIG = 24, + VHOST_USER_SET_CONFIG = 25, + VHOST_USER_SET_CONFIG_FD = 26, VHOST_USER_MAX } VhostUserRequest; @@ -109,6 +117,7 @@ typedef struct VhostUserMsg { VhostUserMemory memory; VhostUserLog log; struct vhost_iotlb_msg iotlb; + uint8_t config[VHOST_USER_MAX_CONFIG_SIZE]; } payload; } QEMU_PACKED VhostUserMsg; @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled) /* No-op as the receive channel is not dedicated to IOTLB messages. */ } +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, + size_t config_len) +{ + VhostUserMsg msg = { + .request = VHOST_USER_GET_CONFIG, + .flags = VHOST_USER_VERSION, + .size = config_len, + }; + + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { + error_report("bad config length"); + return -1; + } + + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { + return -1; + } + + if (vhost_user_read(dev, &msg) < 0) { + return -1; + } + + if (msg.request != VHOST_USER_GET_CONFIG) { + error_report("Received unexpected msg type. Expected %d received %d", + VHOST_USER_GET_CONFIG, msg.request); + return -1; + } + + if (msg.size != config_len) { + error_report("Received bad msg size."); + return -1; + } + + memcpy(config, &msg.payload.config, config_len); + + return 0; +} + +static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *config, + size_t config_len) +{ + bool reply_supported = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_REPLY_ACK); + + VhostUserMsg msg = { + .request = VHOST_USER_SET_CONFIG, + .flags = VHOST_USER_VERSION, + .size = config_len, + }; + + if (reply_supported) { + msg.flags |= VHOST_USER_NEED_REPLY_MASK; + } + + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) { + error_report("bad config length"); + return -1; + } + + memcpy(&msg.payload.config, config, config_len); + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { + return -1; + } + + if (reply_supported) { + return process_message_reply(dev, &msg); + } + + return 0; +} + +static int vhost_user_set_config_fd(struct vhost_dev *dev, int fd) +{ + VhostUserMsg msg = { + .request = VHOST_USER_SET_CONFIG_FD, + .flags = VHOST_USER_VERSION, + }; + + if (vhost_user_write(dev, &msg, &fd, 1) < 0) { + return -1; + } + + return 0; +} + const VhostOps user_ops = { .backend_type = VHOST_BACKEND_TYPE_USER, .vhost_backend_init = vhost_user_init, @@ -948,4 +1042,7 @@ const VhostOps user_ops = { .vhost_net_set_mtu = vhost_user_net_set_mtu, .vhost_set_iotlb_callback = vhost_user_set_iotlb_callback, .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg, + .vhost_get_config = vhost_user_get_config, + .vhost_set_config = vhost_user_set_config, + .vhost_set_config_fd = vhost_user_set_config_fd, }; diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index ddc42f0..8079f46 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1354,6 +1354,9 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) for (i = 0; i < hdev->nvqs; ++i) { vhost_virtqueue_cleanup(hdev->vqs + i); } + if (hdev->config_ops) { + event_notifier_cleanup(&hdev->config_notifier); + } if (hdev->mem) { /* those are only safe after successful init */ memory_listener_unregister(&hdev->memory_listener); @@ -1501,6 +1504,66 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits, } } +int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config, + size_t config_len) +{ + assert(hdev->vhost_ops); + + if (hdev->vhost_ops->vhost_get_config) { + return hdev->vhost_ops->vhost_get_config(hdev, config, config_len); + } + + return 0; +} + +int vhost_dev_set_config(struct vhost_dev *hdev, const uint8_t *config, + size_t config_len) +{ + assert(hdev->vhost_ops); + + if (hdev->vhost_ops->vhost_set_config) { + return hdev->vhost_ops->vhost_set_config(hdev, config, config_len); + } + + return 0; +} + +static void vhost_dev_config_notifier_read(EventNotifier *n) +{ + struct vhost_dev *hdev = container_of(n, struct vhost_dev, + config_notifier); + + if (event_notifier_test_and_clear(n)) { + if (hdev->config_ops) { + hdev->config_ops->vhost_dev_config_notifier(hdev); + } + } +} + +int vhost_dev_set_config_notifier(struct vhost_dev *hdev, + const VhostDevConfigOps *ops) +{ + int r, fd; + + assert(hdev->vhost_ops); + + r = event_notifier_init(&hdev->config_notifier, 0); + if (r < 0) { + return r; + } + + hdev->config_ops = ops; + event_notifier_set_handler(&hdev->config_notifier, + vhost_dev_config_notifier_read); + + if (hdev->vhost_ops->vhost_set_config_fd) { + fd = event_notifier_get_fd(&hdev->config_notifier); + return hdev->vhost_ops->vhost_set_config_fd(hdev, fd); + } + + return 0; +} + /* Host notifiers must be enabled at this point. */ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) { diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h index a7a5f22..df6769e 100644 --- a/include/hw/virtio/vhost-backend.h +++ b/include/hw/virtio/vhost-backend.h @@ -84,6 +84,11 @@ typedef void (*vhost_set_iotlb_callback_op)(struct vhost_dev *dev, int enabled); typedef int (*vhost_send_device_iotlb_msg_op)(struct vhost_dev *dev, struct vhost_iotlb_msg *imsg); +typedef int (*vhost_set_config_op)(struct vhost_dev *dev, const uint8_t *config, + size_t config_len); +typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config, + size_t config_len); +typedef int (*vhost_set_config_fd_op)(struct vhost_dev *dev, int fd); typedef struct VhostOps { VhostBackendType backend_type; @@ -118,6 +123,9 @@ typedef struct VhostOps { vhost_vsock_set_running_op vhost_vsock_set_running; vhost_set_iotlb_callback_op vhost_set_iotlb_callback; vhost_send_device_iotlb_msg_op vhost_send_device_iotlb_msg; + vhost_get_config_op vhost_get_config; + vhost_set_config_op vhost_set_config; + vhost_set_config_fd_op vhost_set_config_fd; } VhostOps; extern const VhostOps user_ops; diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index 467dc77..ff172f2 100644 --- a/include/hw/virtio/vhost.h +++ b/include/hw/virtio/vhost.h @@ -46,6 +46,12 @@ struct vhost_iommu { QLIST_ENTRY(vhost_iommu) iommu_next; }; +typedef struct VhostDevConfigOps { + /* Vhost device config space changed callback + */ + void (*vhost_dev_config_notifier)(struct vhost_dev *dev); +} VhostDevConfigOps; + struct vhost_memory; struct vhost_dev { VirtIODevice *vdev; @@ -76,6 +82,8 @@ struct vhost_dev { QLIST_ENTRY(vhost_dev) entry; QLIST_HEAD(, vhost_iommu) iommu_list; IOMMUNotifier n; + EventNotifier config_notifier; + const VhostDevConfigOps *config_ops; }; int vhost_dev_init(struct vhost_dev *hdev, void *opaque, @@ -106,4 +114,12 @@ int vhost_net_set_backend(struct vhost_dev *hdev, struct vhost_vring_file *file); int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write); +int vhost_dev_get_config(struct vhost_dev *dev, uint8_t *config, + size_t config_len); +int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *config, + size_t config_len); +/* notifier callback in case vhost device config space changed + */ +int vhost_dev_set_config_notifier(struct vhost_dev *dev, + const VhostDevConfigOps *ops); #endif
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.liu@intel.com> --- docs/interop/vhost-user.txt | 33 +++++++++++++ hw/virtio/vhost-user.c | 97 +++++++++++++++++++++++++++++++++++++++ hw/virtio/vhost.c | 63 +++++++++++++++++++++++++ include/hw/virtio/vhost-backend.h | 8 ++++ include/hw/virtio/vhost.h | 16 +++++++ 5 files changed, 217 insertions(+)