| Message ID | 1466756228-27490-2-git-send-email-saxenap.ltc@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
CC'ing MarcAndre and Michael S Tsirkin. On Fri, Jun 24, 2016 at 1:47 PM, Prerna Saxena <saxenap.ltc@gmail.com> wrote: > From: Prerna Saxena <prerna.saxena@nutanix.com> > > Signed-off-by: Prerna Saxena <prerna.saxena@nutanix.com> > --- > docs/specs/vhost-user.txt | 36 +++++++++++ > hw/virtio/vhost-user.c | 153 > +++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 186 insertions(+), 3 deletions(-) > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > index 777c49c..e5388b2 100644 > --- a/docs/specs/vhost-user.txt > +++ b/docs/specs/vhost-user.txt > @@ -37,6 +37,7 @@ consists of 3 header fields and a payload: > * Flags: 32-bit bit field: > - Lower 2 bits are the version (currently 0x01) > - Bit 2 is the reply flag - needs to be sent on each reply from the > slave > + - Bit 3 is the need_response flag - see > VHOST_USER_PROTOCOL_F_REPLY_ACK for details. > * Size - 32-bit size of the payload > > > @@ -126,6 +127,8 @@ the ones that do: > * VHOST_GET_VRING_BASE > * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD) > > +[ Also see the section on REPLY_ACK protocol extension] > + > There are several messages that the master sends with file descriptors > passed > in the ancillary data: > > @@ -254,6 +257,7 @@ Protocol features > #define VHOST_USER_PROTOCOL_F_MQ 0 > #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1 > #define VHOST_USER_PROTOCOL_F_RARP 2 > +#define VHOST_USER_PROTOCOL_F_REPLY_ACK 3 > > Message types > ------------- > @@ -464,3 +468,35 @@ Message types > is present in VHOST_USER_GET_PROTOCOL_FEATURES. > The first 6 bytes of the payload contain the mac address of the > guest to > allow the vhost user backend to construct and broadcast the fake > RARP. > + > +VHOST_USER_PROTOCOL_F_REPLY_ACK: > +-------------------------------- > +The original vhost-user specification only demands responses for certain > +commands. This differs from the vhost protocol implementation where > commands > +are sent over an ioctl() call and block until the client has completed. > Not > +receiving a response for commands like VHOST_SET_MEM_TABLE makes the > sender > +unable to tell when the client has finished (re)mapping the GPA, or > whether it > +has failed altogether. > + > +With this protocol extension negotiated, the sender can set the newly > +introduced "need_response" [Bit 3] flag to any command. This indicates > that > +the client MUST to respond with a Payload VhostUserMsg indicating success > or > +failure. The payload should be set to zero on success or non-zero on > failure. > +In other words, response must be in the following format : > +------------------------------------ > +| request | flags | size | payload | > +------------------------------------ > + > + * Request: 32-bit type of the original request which is being responded > to. > + * Flags: 32-bit bit field: (VHOST_USER_VERSION | VHOST_USER_REPLY_MASK) > + * Size: size of the payload ( see below) > + * Payload : a u64 integer, where a non-zero value indicates a failure. > + > +Note that as per the original vhost-user protocol, the following four > messages anyway > +require distinct responses from the vhost-user client process : > + * VHOST_GET_FEATURES > + * VHOST_GET_PROTOCOL_FEATURES > + * VHOST_GET_VRING_BASE > + * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD) > +For these message types, the presence of VHOST_USER_PROTOCOL_F_REPLY_ACK > or > +need_response bit being set brings no behaviourial change. > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 495e09f..f01ebb4 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -31,6 +31,7 @@ enum VhostUserProtocolFeature { > VHOST_USER_PROTOCOL_F_MQ = 0, > VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1, > VHOST_USER_PROTOCOL_F_RARP = 2, > + VHOST_USER_PROTOCOL_F_REPLY_ACK = 3, > > VHOST_USER_PROTOCOL_F_MAX > }; > @@ -82,8 +83,9 @@ typedef struct VhostUserLog { > typedef struct VhostUserMsg { > VhostUserRequest request; > > -#define VHOST_USER_VERSION_MASK (0x3) > -#define VHOST_USER_REPLY_MASK (0x1<<2) > +#define VHOST_USER_VERSION_MASK (0x3) > +#define VHOST_USER_REPLY_MASK (0x1 << 2) > +#define VHOST_USER_NEED_RESPONSE_MASK (0x1 << 3) > uint32_t flags; > uint32_t size; /* the following payload size */ > union { > @@ -239,10 +241,17 @@ static int vhost_user_set_mem_table(struct vhost_dev > *dev, > int fds[VHOST_MEMORY_MAX_NREGIONS]; > int i, fd; > size_t fd_num = 0; > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > VhostUserMsg msg = { > .request = VHOST_USER_SET_MEM_TABLE, > .flags = VHOST_USER_VERSION, > }; > + VhostUserRequest request = msg.request; > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > > for (i = 0; i < dev->mem->nregions; ++i) { > struct vhost_memory_region *reg = dev->mem->regions + i; > @@ -277,6 +286,20 @@ static int vhost_user_set_mem_table(struct vhost_dev > *dev, > > vhost_user_write(dev, &msg, fds, fd_num); > > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > + > return 0; > } > > @@ -289,9 +312,29 @@ static int vhost_user_set_vring_addr(struct vhost_dev > *dev, > .payload.addr = *addr, > .size = sizeof(msg.payload.addr), > }; > + VhostUserRequest request = msg.request; > + > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > vhost_user_write(dev, &msg, NULL, 0); > > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > return 0; > } > > @@ -313,8 +356,28 @@ static int vhost_set_vring(struct vhost_dev *dev, > .size = sizeof(msg.payload.state), > }; > > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > + > vhost_user_write(dev, &msg, NULL, 0); > > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %lu received %lu", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > return 0; > } > > @@ -395,6 +458,13 @@ static int vhost_set_vring_file(struct vhost_dev *dev, > .size = sizeof(msg.payload.u64), > }; > > + bool reply_Supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > + > if (ioeventfd_enabled() && file->fd > 0) { > fds[fd_num++] = file->fd; > } else { > @@ -403,6 +473,20 @@ static int vhost_set_vring_file(struct vhost_dev *dev, > > vhost_user_write(dev, &msg, fds, fd_num); > > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > + > return 0; > } > > @@ -489,8 +573,30 @@ static int vhost_user_set_owner(struct vhost_dev *dev) > .flags = VHOST_USER_VERSION, > }; > > + VhostUserRequest request = msg.request; > + > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > + > vhost_user_write(dev, &msg, NULL, 0); > > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > return 0; > } > > @@ -500,9 +606,30 @@ static int vhost_user_reset_device(struct vhost_dev > *dev) > .request = VHOST_USER_RESET_OWNER, > .flags = VHOST_USER_VERSION, > }; > + VhostUserRequest request = msg.request; > + > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > > vhost_user_write(dev, &msg, NULL, 0); > > + if (reply_suported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > return 0; > } > > @@ -589,9 +716,15 @@ static int vhost_user_migration_done(struct vhost_dev > *dev, char* mac_addr) > { > VhostUserMsg msg = { 0 }; > int err; > + VhostUserRequest request = msg.request; > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > /* If guest supports GUEST_ANNOUNCE do nothing */ > if (virtio_has_feature(dev->acked_features, > VIRTIO_NET_F_GUEST_ANNOUNCE)) { > return 0; > @@ -606,7 +739,21 @@ static int vhost_user_migration_done(struct vhost_dev > *dev, char* mac_addr) > msg.size = sizeof(msg.payload.u64); > > err = vhost_user_write(dev, &msg, NULL, 0); > - return err; > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } else { > + return err; > + } > } > return -1; > } > -- > 1.8.1.2 > >
On Fri, Jun 24, 2016 at 01:17:08AM -0700, Prerna Saxena wrote: > From: Prerna Saxena <prerna.saxena@nutanix.com> > > Signed-off-by: Prerna Saxena <prerna.saxena@nutanix.com> > --- > docs/specs/vhost-user.txt | 36 +++++++++++ > hw/virtio/vhost-user.c | 153 +++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 186 insertions(+), 3 deletions(-) > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > index 777c49c..e5388b2 100644 > --- a/docs/specs/vhost-user.txt > +++ b/docs/specs/vhost-user.txt > @@ -37,6 +37,7 @@ consists of 3 header fields and a payload: > * Flags: 32-bit bit field: > - Lower 2 bits are the version (currently 0x01) > - Bit 2 is the reply flag - needs to be sent on each reply from the slave > + - Bit 3 is the need_response flag - see VHOST_USER_PROTOCOL_F_REPLY_ACK for details. > * Size - 32-bit size of the payload > > > @@ -126,6 +127,8 @@ the ones that do: > * VHOST_GET_VRING_BASE > * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD) > > +[ Also see the section on REPLY_ACK protocol extension] > + > There are several messages that the master sends with file descriptors passed > in the ancillary data: > > @@ -254,6 +257,7 @@ Protocol features > #define VHOST_USER_PROTOCOL_F_MQ 0 > #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1 > #define VHOST_USER_PROTOCOL_F_RARP 2 > +#define VHOST_USER_PROTOCOL_F_REPLY_ACK 3 > > Message types > ------------- > @@ -464,3 +468,35 @@ Message types > is present in VHOST_USER_GET_PROTOCOL_FEATURES. > The first 6 bytes of the payload contain the mac address of the guest to > allow the vhost user backend to construct and broadcast the fake RARP. > + > +VHOST_USER_PROTOCOL_F_REPLY_ACK: > +-------------------------------- > +The original vhost-user specification only demands responses for certain > +commands. This differs from the vhost protocol implementation where commands > +are sent over an ioctl() call and block until the client has completed. Not > +receiving a response for commands like VHOST_SET_MEM_TABLE makes the sender > +unable to tell when the client has finished (re)mapping the GPA, or whether it > +has failed altogether. I'd just drop this from spec. > + > +With this protocol extension negotiated, the sender can set the newly > +introduced "need_response" [Bit 3] flag to any command. This indicates that > +the client MUST to respond with a Payload VhostUserMsg indicating success or > +failure. The payload should be set to zero on success or non-zero on failure. Are there any cases where qemu can handle failure? If not, please specify that it's for debugging. Let's add a string for debugging as well? > +In other words, response must be in the following format : > +------------------------------------ > +| request | flags | size | payload | > +------------------------------------ > + > + * Request: 32-bit type of the original request which is being responded to. > + * Flags: 32-bit bit field: (VHOST_USER_VERSION | VHOST_USER_REPLY_MASK) > + * Size: size of the payload ( see below) > + * Payload : a u64 integer, where a non-zero value indicates a failure. > + > +Note that as per the original vhost-user protocol, the following four messages anyway > +require distinct responses from the vhost-user client process : > + * VHOST_GET_FEATURES > + * VHOST_GET_PROTOCOL_FEATURES > + * VHOST_GET_VRING_BASE > + * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD) > +For these message types, the presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or > +need_response bit being set brings no behaviourial change. This needs to be reworded to avoid distinction between original and new. Basically compare behaviour when VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated and not. > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 495e09f..f01ebb4 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -31,6 +31,7 @@ enum VhostUserProtocolFeature { > VHOST_USER_PROTOCOL_F_MQ = 0, > VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1, > VHOST_USER_PROTOCOL_F_RARP = 2, > + VHOST_USER_PROTOCOL_F_REPLY_ACK = 3, > > VHOST_USER_PROTOCOL_F_MAX > }; > @@ -82,8 +83,9 @@ typedef struct VhostUserLog { > typedef struct VhostUserMsg { > VhostUserRequest request; > > -#define VHOST_USER_VERSION_MASK (0x3) > -#define VHOST_USER_REPLY_MASK (0x1<<2) > +#define VHOST_USER_VERSION_MASK (0x3) > +#define VHOST_USER_REPLY_MASK (0x1 << 2) > +#define VHOST_USER_NEED_RESPONSE_MASK (0x1 << 3) > uint32_t flags; > uint32_t size; /* the following payload size */ > union { > @@ -239,10 +241,17 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, > int fds[VHOST_MEMORY_MAX_NREGIONS]; > int i, fd; > size_t fd_num = 0; > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > VhostUserMsg msg = { > .request = VHOST_USER_SET_MEM_TABLE, > .flags = VHOST_USER_VERSION, > }; > + VhostUserRequest request = msg.request; > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > > for (i = 0; i < dev->mem->nregions; ++i) { > struct vhost_memory_region *reg = dev->mem->regions + i; > @@ -277,6 +286,20 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, > > vhost_user_write(dev, &msg, fds, fd_num); > > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > + > return 0; > } > > @@ -289,9 +312,29 @@ static int vhost_user_set_vring_addr(struct vhost_dev *dev, > .payload.addr = *addr, > .size = sizeof(msg.payload.addr), > }; > + VhostUserRequest request = msg.request; > + > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > vhost_user_write(dev, &msg, NULL, 0); > > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > return 0; > } > > @@ -313,8 +356,28 @@ static int vhost_set_vring(struct vhost_dev *dev, > .size = sizeof(msg.payload.state), > }; > > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > + > vhost_user_write(dev, &msg, NULL, 0); > > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %lu received %lu", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > return 0; > } > > @@ -395,6 +458,13 @@ static int vhost_set_vring_file(struct vhost_dev *dev, > .size = sizeof(msg.payload.u64), > }; > > + bool reply_Supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > + > if (ioeventfd_enabled() && file->fd > 0) { > fds[fd_num++] = file->fd; > } else { > @@ -403,6 +473,20 @@ static int vhost_set_vring_file(struct vhost_dev *dev, > > vhost_user_write(dev, &msg, fds, fd_num); > > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > + > return 0; > } > > @@ -489,8 +573,30 @@ static int vhost_user_set_owner(struct vhost_dev *dev) > .flags = VHOST_USER_VERSION, > }; > > + VhostUserRequest request = msg.request; > + > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > + > vhost_user_write(dev, &msg, NULL, 0); > > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > return 0; > } > > @@ -500,9 +606,30 @@ static int vhost_user_reset_device(struct vhost_dev *dev) > .request = VHOST_USER_RESET_OWNER, > .flags = VHOST_USER_VERSION, > }; > + VhostUserRequest request = msg.request; > + > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > > vhost_user_write(dev, &msg, NULL, 0); > > + if (reply_suported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } > return 0; > } > > @@ -589,9 +716,15 @@ static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr) > { > VhostUserMsg msg = { 0 }; > int err; > + VhostUserRequest request = msg.request; > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > > + if (reply_supported) { > + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; > + } > /* If guest supports GUEST_ANNOUNCE do nothing */ > if (virtio_has_feature(dev->acked_features, VIRTIO_NET_F_GUEST_ANNOUNCE)) { > return 0; > @@ -606,7 +739,21 @@ static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr) > msg.size = sizeof(msg.payload.u64); > > err = vhost_user_write(dev, &msg, NULL, 0); > - return err; > + if (reply_supported) { > + if (vhost_user_read(dev, &msg) < 0) { > + return 0; > + } > + > + if (msg.request != request) { > + error_report("Received unexpected msg type." > + "Expected %d received %d", > + request, msg.request); > + return -1; > + } > + return msg.payload.u64 ? -1 : 0; > + } else { > + return err; > + } > } > return -1; Please add comments documenting why we are waiting for a response here. Also, pls add a helper to reduce code duplication, and also to support !VHOST_USER_PROTOCOL_F_REPLY_ACK. > } > -- > 1.8.1.2 >
diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt index 777c49c..e5388b2 100644 --- a/docs/specs/vhost-user.txt +++ b/docs/specs/vhost-user.txt @@ -37,6 +37,7 @@ consists of 3 header fields and a payload: * Flags: 32-bit bit field: - Lower 2 bits are the version (currently 0x01) - Bit 2 is the reply flag - needs to be sent on each reply from the slave + - Bit 3 is the need_response flag - see VHOST_USER_PROTOCOL_F_REPLY_ACK for details. * Size - 32-bit size of the payload @@ -126,6 +127,8 @@ the ones that do: * VHOST_GET_VRING_BASE * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD) +[ Also see the section on REPLY_ACK protocol extension] + There are several messages that the master sends with file descriptors passed in the ancillary data: @@ -254,6 +257,7 @@ Protocol features #define VHOST_USER_PROTOCOL_F_MQ 0 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1 #define VHOST_USER_PROTOCOL_F_RARP 2 +#define VHOST_USER_PROTOCOL_F_REPLY_ACK 3 Message types ------------- @@ -464,3 +468,35 @@ Message types is present in VHOST_USER_GET_PROTOCOL_FEATURES. The first 6 bytes of the payload contain the mac address of the guest to allow the vhost user backend to construct and broadcast the fake RARP. + +VHOST_USER_PROTOCOL_F_REPLY_ACK: +-------------------------------- +The original vhost-user specification only demands responses for certain +commands. This differs from the vhost protocol implementation where commands +are sent over an ioctl() call and block until the client has completed. Not +receiving a response for commands like VHOST_SET_MEM_TABLE makes the sender +unable to tell when the client has finished (re)mapping the GPA, or whether it +has failed altogether. + +With this protocol extension negotiated, the sender can set the newly +introduced "need_response" [Bit 3] flag to any command. This indicates that +the client MUST to respond with a Payload VhostUserMsg indicating success or +failure. The payload should be set to zero on success or non-zero on failure. +In other words, response must be in the following format : +------------------------------------ +| request | flags | size | payload | +------------------------------------ + + * Request: 32-bit type of the original request which is being responded to. + * Flags: 32-bit bit field: (VHOST_USER_VERSION | VHOST_USER_REPLY_MASK) + * Size: size of the payload ( see below) + * Payload : a u64 integer, where a non-zero value indicates a failure. + +Note that as per the original vhost-user protocol, the following four messages anyway +require distinct responses from the vhost-user client process : + * VHOST_GET_FEATURES + * VHOST_GET_PROTOCOL_FEATURES + * VHOST_GET_VRING_BASE + * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD) +For these message types, the presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or +need_response bit being set brings no behaviourial change. diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 495e09f..f01ebb4 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -31,6 +31,7 @@ enum VhostUserProtocolFeature { VHOST_USER_PROTOCOL_F_MQ = 0, VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1, VHOST_USER_PROTOCOL_F_RARP = 2, + VHOST_USER_PROTOCOL_F_REPLY_ACK = 3, VHOST_USER_PROTOCOL_F_MAX }; @@ -82,8 +83,9 @@ typedef struct VhostUserLog { typedef struct VhostUserMsg { VhostUserRequest request; -#define VHOST_USER_VERSION_MASK (0x3) -#define VHOST_USER_REPLY_MASK (0x1<<2) +#define VHOST_USER_VERSION_MASK (0x3) +#define VHOST_USER_REPLY_MASK (0x1 << 2) +#define VHOST_USER_NEED_RESPONSE_MASK (0x1 << 3) uint32_t flags; uint32_t size; /* the following payload size */ union { @@ -239,10 +241,17 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, int fds[VHOST_MEMORY_MAX_NREGIONS]; int i, fd; size_t fd_num = 0; + bool reply_supported = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_REPLY_ACK); VhostUserMsg msg = { .request = VHOST_USER_SET_MEM_TABLE, .flags = VHOST_USER_VERSION, }; + VhostUserRequest request = msg.request; + + if (reply_supported) { + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; + } for (i = 0; i < dev->mem->nregions; ++i) { struct vhost_memory_region *reg = dev->mem->regions + i; @@ -277,6 +286,20 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, vhost_user_write(dev, &msg, fds, fd_num); + if (reply_supported) { + if (vhost_user_read(dev, &msg) < 0) { + return 0; + } + + if (msg.request != request) { + error_report("Received unexpected msg type." + "Expected %d received %d", + request, msg.request); + return -1; + } + return msg.payload.u64 ? -1 : 0; + } + return 0; } @@ -289,9 +312,29 @@ static int vhost_user_set_vring_addr(struct vhost_dev *dev, .payload.addr = *addr, .size = sizeof(msg.payload.addr), }; + VhostUserRequest request = msg.request; + + bool reply_supported = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_REPLY_ACK); + if (reply_supported) { + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; + } vhost_user_write(dev, &msg, NULL, 0); + if (reply_supported) { + if (vhost_user_read(dev, &msg) < 0) { + return 0; + } + + if (msg.request != request) { + error_report("Received unexpected msg type." + "Expected %d received %d", + request, msg.request); + return -1; + } + return msg.payload.u64 ? -1 : 0; + } return 0; } @@ -313,8 +356,28 @@ static int vhost_set_vring(struct vhost_dev *dev, .size = sizeof(msg.payload.state), }; + bool reply_supported = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_REPLY_ACK); + + if (reply_supported) { + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; + } + vhost_user_write(dev, &msg, NULL, 0); + if (reply_supported) { + if (vhost_user_read(dev, &msg) < 0) { + return 0; + } + + if (msg.request != request) { + error_report("Received unexpected msg type." + "Expected %lu received %lu", + request, msg.request); + return -1; + } + return msg.payload.u64 ? -1 : 0; + } return 0; } @@ -395,6 +458,13 @@ static int vhost_set_vring_file(struct vhost_dev *dev, .size = sizeof(msg.payload.u64), }; + bool reply_Supported = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_REPLY_ACK); + + if (reply_supported) { + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; + } + if (ioeventfd_enabled() && file->fd > 0) { fds[fd_num++] = file->fd; } else { @@ -403,6 +473,20 @@ static int vhost_set_vring_file(struct vhost_dev *dev, vhost_user_write(dev, &msg, fds, fd_num); + if (reply_supported) { + if (vhost_user_read(dev, &msg) < 0) { + return 0; + } + + if (msg.request != request) { + error_report("Received unexpected msg type." + "Expected %d received %d", + request, msg.request); + return -1; + } + return msg.payload.u64 ? -1 : 0; + } + return 0; } @@ -489,8 +573,30 @@ static int vhost_user_set_owner(struct vhost_dev *dev) .flags = VHOST_USER_VERSION, }; + VhostUserRequest request = msg.request; + + bool reply_supported = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_REPLY_ACK); + + if (reply_supported) { + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; + } + vhost_user_write(dev, &msg, NULL, 0); + if (reply_supported) { + if (vhost_user_read(dev, &msg) < 0) { + return 0; + } + + if (msg.request != request) { + error_report("Received unexpected msg type." + "Expected %d received %d", + request, msg.request); + return -1; + } + return msg.payload.u64 ? -1 : 0; + } return 0; } @@ -500,9 +606,30 @@ static int vhost_user_reset_device(struct vhost_dev *dev) .request = VHOST_USER_RESET_OWNER, .flags = VHOST_USER_VERSION, }; + VhostUserRequest request = msg.request; + + bool reply_supported = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_REPLY_ACK); + + if (reply_supported) { + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; + } vhost_user_write(dev, &msg, NULL, 0); + if (reply_suported) { + if (vhost_user_read(dev, &msg) < 0) { + return 0; + } + + if (msg.request != request) { + error_report("Received unexpected msg type." + "Expected %d received %d", + request, msg.request); + return -1; + } + return msg.payload.u64 ? -1 : 0; + } return 0; } @@ -589,9 +716,15 @@ static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr) { VhostUserMsg msg = { 0 }; int err; + VhostUserRequest request = msg.request; + bool reply_supported = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_REPLY_ACK); assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); + if (reply_supported) { + msg.flags |= VHOST_USER_NEED_RESPONSE_MASK; + } /* If guest supports GUEST_ANNOUNCE do nothing */ if (virtio_has_feature(dev->acked_features, VIRTIO_NET_F_GUEST_ANNOUNCE)) { return 0; @@ -606,7 +739,21 @@ static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr) msg.size = sizeof(msg.payload.u64); err = vhost_user_write(dev, &msg, NULL, 0); - return err; + if (reply_supported) { + if (vhost_user_read(dev, &msg) < 0) { + return 0; + } + + if (msg.request != request) { + error_report("Received unexpected msg type." + "Expected %d received %d", + request, msg.request); + return -1; + } + return msg.payload.u64 ? -1 : 0; + } else { + return err; + } } return -1; }