| Message ID | 3a002790e6c880af928c6470ecbf03e7c65a68bb.1689748694.git.yin31149@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/8] vhost: Add argument to vhost_svq_poll() | expand |
On Wed, Jul 19, 2023 at 9:54 AM Hawkins Jiawei <yin31149@gmail.com> wrote: > > This patch enables sending CVQ state load commands > in parallel at device startup by following steps: > > * Refactor vhost_vdpa_net_load_cmd() to iterate through > the control commands shadow buffers. This allows different > CVQ state load commands to use their own unique buffers. > > * Delay the polling and checking of buffers until either > the SVQ is full or control commands shadow buffers are full. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1578 > Signed-off-by: Hawkins Jiawei <yin31149@gmail.com> > --- > net/vhost-vdpa.c | 157 +++++++++++++++++++++++++++++------------------ > 1 file changed, 96 insertions(+), 61 deletions(-) > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > index 795c9c1fd2..1ebb58f7f6 100644 > --- a/net/vhost-vdpa.c > +++ b/net/vhost-vdpa.c > @@ -633,6 +633,26 @@ static uint16_t vhost_vdpa_net_svq_available_slots(VhostVDPAState *s) > return vhost_svq_available_slots(svq); > } > > +/* > + * Poll SVQ for multiple pending control commands and check the device's ack. > + * > + * Caller should hold the BQL when invoking this function. > + */ > +static ssize_t vhost_vdpa_net_svq_flush(VhostVDPAState *s, > + size_t cmds_in_flight) > +{ > + vhost_vdpa_net_svq_poll(s, cmds_in_flight); > + > + /* Device should and must use only one byte ack each control command */ > + assert(cmds_in_flight < vhost_vdpa_net_cvq_cmd_page_len()); > + for (int i = 0; i < cmds_in_flight; ++i) { > + if (s->status[i] != VIRTIO_NET_OK) { > + return -EIO; > + } > + } > + return 0; > +} > + > static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, void **out_cursor, > void **in_cursor, uint8_t class, > uint8_t cmd, const struct iovec *data_sg, > @@ -642,19 +662,41 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, void **out_cursor, > .class = class, > .cmd = cmd, > }; > - size_t data_size = iov_size(data_sg, data_num); > + size_t data_size = iov_size(data_sg, data_num), > + left_bytes = vhost_vdpa_net_cvq_cmd_page_len() - > + (*out_cursor - s->cvq_cmd_out_buffer); > /* Buffers for the device */ > struct iovec out = { > - .iov_base = *out_cursor, > .iov_len = sizeof(ctrl) + data_size, > }; > struct iovec in = { > - .iov_base = *in_cursor, > .iov_len = sizeof(*s->status), > }; > ssize_t r; > > - assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl)); > + if (sizeof(ctrl) > left_bytes || data_size > left_bytes - sizeof(ctrl) || I'm ok with this code, but maybe we can simplify the code if we use two struct iovec as cursors instead of a void **? I think functions like iov_size and iov_copy already take care of a few checks here. Apart from that it would be great to merge this call to vhost_vdpa_net_svq_flush, but I find it very hard to do unless we scatter it through all callers of vhost_vdpa_net_load_cmd. Apart from the minor comments I think the series is great, thanks! > + vhost_vdpa_net_svq_available_slots(s) < 2) { > + /* > + * It is time to flush all pending control commands if SVQ is full > + * or control commands shadow buffers are full. > + * > + * We can poll here since we've had BQL from the time > + * we sent the descriptor. > + */ > + r = vhost_vdpa_net_svq_flush(s, *in_cursor - (void *)s->status); > + if (unlikely(r < 0)) { > + return r; > + } > + > + *out_cursor = s->cvq_cmd_out_buffer; > + *in_cursor = s->status; > + left_bytes = vhost_vdpa_net_cvq_cmd_page_len(); > + } > + > + out.iov_base = *out_cursor; > + in.iov_base = *in_cursor; > + > + assert(data_size <= left_bytes - sizeof(ctrl)); > /* Each CVQ command has one out descriptor and one in descriptor */ > assert(vhost_vdpa_net_svq_available_slots(s) >= 2); > > @@ -670,11 +712,11 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, void **out_cursor, > return r; > } > > - /* > - * We can poll here since we've had BQL from the time > - * we sent the descriptor. > - */ > - return vhost_vdpa_net_svq_poll(s, 1); > + /* iterate the cursors */ > + *out_cursor += out.iov_len; > + *in_cursor += in.iov_len; > + > + return 0; > } > > static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n, > @@ -685,15 +727,12 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n, > .iov_base = (void *)n->mac, > .iov_len = sizeof(n->mac), > }; > - ssize_t dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > - VIRTIO_NET_CTRL_MAC, > - VIRTIO_NET_CTRL_MAC_ADDR_SET, > - &data, 1); > - if (unlikely(dev_written < 0)) { > - return dev_written; > - } > - if (*s->status != VIRTIO_NET_OK) { > - return -EIO; > + ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > + VIRTIO_NET_CTRL_MAC, > + VIRTIO_NET_CTRL_MAC_ADDR_SET, > + &data, 1); > + if (unlikely(r < 0)) { > + return r; > } > } > > @@ -738,15 +777,12 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n, > .iov_len = mul_macs_size, > }, > }; > - ssize_t dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > + ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > VIRTIO_NET_CTRL_MAC, > VIRTIO_NET_CTRL_MAC_TABLE_SET, > data, ARRAY_SIZE(data)); > - if (unlikely(dev_written < 0)) { > - return dev_written; > - } > - if (*s->status != VIRTIO_NET_OK) { > - return -EIO; > + if (unlikely(r < 0)) { > + return r; > } > > return 0; > @@ -757,7 +793,7 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s, > void **out_cursor, void **in_cursor) > { > struct virtio_net_ctrl_mq mq; > - ssize_t dev_written; > + ssize_t r; > > if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_MQ)) { > return 0; > @@ -768,15 +804,12 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s, > .iov_base = &mq, > .iov_len = sizeof(mq), > }; > - dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > - VIRTIO_NET_CTRL_MQ, > - VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, > - &data, 1); > - if (unlikely(dev_written < 0)) { > - return dev_written; > - } > - if (*s->status != VIRTIO_NET_OK) { > - return -EIO; > + r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > + VIRTIO_NET_CTRL_MQ, > + VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, > + &data, 1); > + if (unlikely(r < 0)) { > + return r; > } > > return 0; > @@ -787,7 +820,7 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s, > void **out_cursor, void **in_cursor) > { > uint64_t offloads; > - ssize_t dev_written; > + ssize_t r; > > if (!virtio_vdev_has_feature(&n->parent_obj, > VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) { > @@ -815,15 +848,12 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s, > .iov_base = &offloads, > .iov_len = sizeof(offloads), > }; > - dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > - VIRTIO_NET_CTRL_GUEST_OFFLOADS, > - VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, > - &data, 1); > - if (unlikely(dev_written < 0)) { > - return dev_written; > - } > - if (*s->status != VIRTIO_NET_OK) { > - return -EIO; > + r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > + VIRTIO_NET_CTRL_GUEST_OFFLOADS, > + VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, > + &data, 1); > + if (unlikely(r < 0)) { > + return r; > } > > return 0; > @@ -838,15 +868,12 @@ static int vhost_vdpa_net_load_rx_mode(VhostVDPAState *s, > .iov_base = &on, > .iov_len = sizeof(on), > }; > - ssize_t dev_written; > + ssize_t r; > > - dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > - VIRTIO_NET_CTRL_RX, cmd, &data, 1); > - if (unlikely(dev_written < 0)) { > - return dev_written; > - } > - if (*s->status != VIRTIO_NET_OK) { > - return -EIO; > + r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > + VIRTIO_NET_CTRL_RX, cmd, &data, 1); > + if (unlikely(r < 0)) { > + return r; > } > > return 0; > @@ -1001,15 +1028,12 @@ static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s, > .iov_base = &vid, > .iov_len = sizeof(vid), > }; > - ssize_t dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > - VIRTIO_NET_CTRL_VLAN, > - VIRTIO_NET_CTRL_VLAN_ADD, > - &data, 1); > - if (unlikely(dev_written < 0)) { > - return dev_written; > - } > - if (unlikely(*s->status != VIRTIO_NET_OK)) { > - return -EIO; > + ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, > + VIRTIO_NET_CTRL_VLAN, > + VIRTIO_NET_CTRL_VLAN_ADD, > + &data, 1); > + if (unlikely(r < 0)) { > + return r; > } > > return 0; > @@ -1078,6 +1102,17 @@ static int vhost_vdpa_net_load(NetClientState *nc) > return r; > } > > + /* > + * We need to poll and check all pending device's used buffers. > + * > + * We can poll here since we've had BQL from the time > + * we sent the descriptor. > + */ > + r = vhost_vdpa_net_svq_flush(s, in_cursor - (void *)s->status); > + if (unlikely(r)) { > + return r; > + } > + > return 0; > } > > -- > 2.25.1 >
On 2023/8/19 01:27, Eugenio Perez Martin wrote: > On Wed, Jul 19, 2023 at 9:54 AM Hawkins Jiawei <yin31149@gmail.com> wrote: >> >> This patch enables sending CVQ state load commands >> in parallel at device startup by following steps: >> >> * Refactor vhost_vdpa_net_load_cmd() to iterate through >> the control commands shadow buffers. This allows different >> CVQ state load commands to use their own unique buffers. >> >> * Delay the polling and checking of buffers until either >> the SVQ is full or control commands shadow buffers are full. >> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1578 >> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com> >> --- >> net/vhost-vdpa.c | 157 +++++++++++++++++++++++++++++------------------ >> 1 file changed, 96 insertions(+), 61 deletions(-) >> >> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c >> index 795c9c1fd2..1ebb58f7f6 100644 >> --- a/net/vhost-vdpa.c >> +++ b/net/vhost-vdpa.c >> @@ -633,6 +633,26 @@ static uint16_t vhost_vdpa_net_svq_available_slots(VhostVDPAState *s) >> return vhost_svq_available_slots(svq); >> } >> >> +/* >> + * Poll SVQ for multiple pending control commands and check the device's ack. >> + * >> + * Caller should hold the BQL when invoking this function. >> + */ >> +static ssize_t vhost_vdpa_net_svq_flush(VhostVDPAState *s, >> + size_t cmds_in_flight) >> +{ >> + vhost_vdpa_net_svq_poll(s, cmds_in_flight); >> + >> + /* Device should and must use only one byte ack each control command */ >> + assert(cmds_in_flight < vhost_vdpa_net_cvq_cmd_page_len()); >> + for (int i = 0; i < cmds_in_flight; ++i) { >> + if (s->status[i] != VIRTIO_NET_OK) { >> + return -EIO; >> + } >> + } >> + return 0; >> +} >> + >> static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, void **out_cursor, >> void **in_cursor, uint8_t class, >> uint8_t cmd, const struct iovec *data_sg, >> @@ -642,19 +662,41 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, void **out_cursor, >> .class = class, >> .cmd = cmd, >> }; >> - size_t data_size = iov_size(data_sg, data_num); >> + size_t data_size = iov_size(data_sg, data_num), >> + left_bytes = vhost_vdpa_net_cvq_cmd_page_len() - >> + (*out_cursor - s->cvq_cmd_out_buffer); >> /* Buffers for the device */ >> struct iovec out = { >> - .iov_base = *out_cursor, >> .iov_len = sizeof(ctrl) + data_size, >> }; >> struct iovec in = { >> - .iov_base = *in_cursor, >> .iov_len = sizeof(*s->status), >> }; >> ssize_t r; >> >> - assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl)); >> + if (sizeof(ctrl) > left_bytes || data_size > left_bytes - sizeof(ctrl) || > > I'm ok with this code, but maybe we can simplify the code if we use > two struct iovec as cursors instead of a void **? I think functions > like iov_size and iov_copy already take care of a few checks here. Hi Eugenio, Thanks for the explanation, I will refactor the patch according to your suggestion! > > Apart from that it would be great to merge this call to > vhost_vdpa_net_svq_flush, but I find it very hard to do unless we > scatter it through all callers of vhost_vdpa_net_load_cmd. Yes, I agree with you. Maybe we can consider refactoring like this in the future if needed. > > Apart from the minor comments I think the series is great, thanks! Thanks for your review:)! > >> + vhost_vdpa_net_svq_available_slots(s) < 2) { >> + /* >> + * It is time to flush all pending control commands if SVQ is full >> + * or control commands shadow buffers are full. >> + * >> + * We can poll here since we've had BQL from the time >> + * we sent the descriptor. >> + */ >> + r = vhost_vdpa_net_svq_flush(s, *in_cursor - (void *)s->status); >> + if (unlikely(r < 0)) { >> + return r; >> + } >> + >> + *out_cursor = s->cvq_cmd_out_buffer; >> + *in_cursor = s->status; >> + left_bytes = vhost_vdpa_net_cvq_cmd_page_len(); >> + } >> + >> + out.iov_base = *out_cursor; >> + in.iov_base = *in_cursor; >> + >> + assert(data_size <= left_bytes - sizeof(ctrl)); >> /* Each CVQ command has one out descriptor and one in descriptor */ >> assert(vhost_vdpa_net_svq_available_slots(s) >= 2); >> >> @@ -670,11 +712,11 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, void **out_cursor, >> return r; >> } >> >> - /* >> - * We can poll here since we've had BQL from the time >> - * we sent the descriptor. >> - */ >> - return vhost_vdpa_net_svq_poll(s, 1); >> + /* iterate the cursors */ >> + *out_cursor += out.iov_len; >> + *in_cursor += in.iov_len; >> + >> + return 0; >> } >> >> static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n, >> @@ -685,15 +727,12 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n, >> .iov_base = (void *)n->mac, >> .iov_len = sizeof(n->mac), >> }; >> - ssize_t dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> - VIRTIO_NET_CTRL_MAC, >> - VIRTIO_NET_CTRL_MAC_ADDR_SET, >> - &data, 1); >> - if (unlikely(dev_written < 0)) { >> - return dev_written; >> - } >> - if (*s->status != VIRTIO_NET_OK) { >> - return -EIO; >> + ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> + VIRTIO_NET_CTRL_MAC, >> + VIRTIO_NET_CTRL_MAC_ADDR_SET, >> + &data, 1); >> + if (unlikely(r < 0)) { >> + return r; >> } >> } >> >> @@ -738,15 +777,12 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n, >> .iov_len = mul_macs_size, >> }, >> }; >> - ssize_t dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> + ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> VIRTIO_NET_CTRL_MAC, >> VIRTIO_NET_CTRL_MAC_TABLE_SET, >> data, ARRAY_SIZE(data)); >> - if (unlikely(dev_written < 0)) { >> - return dev_written; >> - } >> - if (*s->status != VIRTIO_NET_OK) { >> - return -EIO; >> + if (unlikely(r < 0)) { >> + return r; >> } >> >> return 0; >> @@ -757,7 +793,7 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s, >> void **out_cursor, void **in_cursor) >> { >> struct virtio_net_ctrl_mq mq; >> - ssize_t dev_written; >> + ssize_t r; >> >> if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_MQ)) { >> return 0; >> @@ -768,15 +804,12 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s, >> .iov_base = &mq, >> .iov_len = sizeof(mq), >> }; >> - dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> - VIRTIO_NET_CTRL_MQ, >> - VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, >> - &data, 1); >> - if (unlikely(dev_written < 0)) { >> - return dev_written; >> - } >> - if (*s->status != VIRTIO_NET_OK) { >> - return -EIO; >> + r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> + VIRTIO_NET_CTRL_MQ, >> + VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, >> + &data, 1); >> + if (unlikely(r < 0)) { >> + return r; >> } >> >> return 0; >> @@ -787,7 +820,7 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s, >> void **out_cursor, void **in_cursor) >> { >> uint64_t offloads; >> - ssize_t dev_written; >> + ssize_t r; >> >> if (!virtio_vdev_has_feature(&n->parent_obj, >> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) { >> @@ -815,15 +848,12 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s, >> .iov_base = &offloads, >> .iov_len = sizeof(offloads), >> }; >> - dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> - VIRTIO_NET_CTRL_GUEST_OFFLOADS, >> - VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, >> - &data, 1); >> - if (unlikely(dev_written < 0)) { >> - return dev_written; >> - } >> - if (*s->status != VIRTIO_NET_OK) { >> - return -EIO; >> + r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> + VIRTIO_NET_CTRL_GUEST_OFFLOADS, >> + VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, >> + &data, 1); >> + if (unlikely(r < 0)) { >> + return r; >> } >> >> return 0; >> @@ -838,15 +868,12 @@ static int vhost_vdpa_net_load_rx_mode(VhostVDPAState *s, >> .iov_base = &on, >> .iov_len = sizeof(on), >> }; >> - ssize_t dev_written; >> + ssize_t r; >> >> - dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> - VIRTIO_NET_CTRL_RX, cmd, &data, 1); >> - if (unlikely(dev_written < 0)) { >> - return dev_written; >> - } >> - if (*s->status != VIRTIO_NET_OK) { >> - return -EIO; >> + r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> + VIRTIO_NET_CTRL_RX, cmd, &data, 1); >> + if (unlikely(r < 0)) { >> + return r; >> } >> >> return 0; >> @@ -1001,15 +1028,12 @@ static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s, >> .iov_base = &vid, >> .iov_len = sizeof(vid), >> }; >> - ssize_t dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> - VIRTIO_NET_CTRL_VLAN, >> - VIRTIO_NET_CTRL_VLAN_ADD, >> - &data, 1); >> - if (unlikely(dev_written < 0)) { >> - return dev_written; >> - } >> - if (unlikely(*s->status != VIRTIO_NET_OK)) { >> - return -EIO; >> + ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, >> + VIRTIO_NET_CTRL_VLAN, >> + VIRTIO_NET_CTRL_VLAN_ADD, >> + &data, 1); >> + if (unlikely(r < 0)) { >> + return r; >> } >> >> return 0; >> @@ -1078,6 +1102,17 @@ static int vhost_vdpa_net_load(NetClientState *nc) >> return r; >> } >> >> + /* >> + * We need to poll and check all pending device's used buffers. >> + * >> + * We can poll here since we've had BQL from the time >> + * we sent the descriptor. >> + */ >> + r = vhost_vdpa_net_svq_flush(s, in_cursor - (void *)s->status); >> + if (unlikely(r)) { >> + return r; >> + } >> + >> return 0; >> } >> >> -- >> 2.25.1 >> >
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index 795c9c1fd2..1ebb58f7f6 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -633,6 +633,26 @@ static uint16_t vhost_vdpa_net_svq_available_slots(VhostVDPAState *s) return vhost_svq_available_slots(svq); } +/* + * Poll SVQ for multiple pending control commands and check the device's ack. + * + * Caller should hold the BQL when invoking this function. + */ +static ssize_t vhost_vdpa_net_svq_flush(VhostVDPAState *s, + size_t cmds_in_flight) +{ + vhost_vdpa_net_svq_poll(s, cmds_in_flight); + + /* Device should and must use only one byte ack each control command */ + assert(cmds_in_flight < vhost_vdpa_net_cvq_cmd_page_len()); + for (int i = 0; i < cmds_in_flight; ++i) { + if (s->status[i] != VIRTIO_NET_OK) { + return -EIO; + } + } + return 0; +} + static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, void **out_cursor, void **in_cursor, uint8_t class, uint8_t cmd, const struct iovec *data_sg, @@ -642,19 +662,41 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, void **out_cursor, .class = class, .cmd = cmd, }; - size_t data_size = iov_size(data_sg, data_num); + size_t data_size = iov_size(data_sg, data_num), + left_bytes = vhost_vdpa_net_cvq_cmd_page_len() - + (*out_cursor - s->cvq_cmd_out_buffer); /* Buffers for the device */ struct iovec out = { - .iov_base = *out_cursor, .iov_len = sizeof(ctrl) + data_size, }; struct iovec in = { - .iov_base = *in_cursor, .iov_len = sizeof(*s->status), }; ssize_t r; - assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl)); + if (sizeof(ctrl) > left_bytes || data_size > left_bytes - sizeof(ctrl) || + vhost_vdpa_net_svq_available_slots(s) < 2) { + /* + * It is time to flush all pending control commands if SVQ is full + * or control commands shadow buffers are full. + * + * We can poll here since we've had BQL from the time + * we sent the descriptor. + */ + r = vhost_vdpa_net_svq_flush(s, *in_cursor - (void *)s->status); + if (unlikely(r < 0)) { + return r; + } + + *out_cursor = s->cvq_cmd_out_buffer; + *in_cursor = s->status; + left_bytes = vhost_vdpa_net_cvq_cmd_page_len(); + } + + out.iov_base = *out_cursor; + in.iov_base = *in_cursor; + + assert(data_size <= left_bytes - sizeof(ctrl)); /* Each CVQ command has one out descriptor and one in descriptor */ assert(vhost_vdpa_net_svq_available_slots(s) >= 2); @@ -670,11 +712,11 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, void **out_cursor, return r; } - /* - * We can poll here since we've had BQL from the time - * we sent the descriptor. - */ - return vhost_vdpa_net_svq_poll(s, 1); + /* iterate the cursors */ + *out_cursor += out.iov_len; + *in_cursor += in.iov_len; + + return 0; } static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n, @@ -685,15 +727,12 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n, .iov_base = (void *)n->mac, .iov_len = sizeof(n->mac), }; - ssize_t dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, - VIRTIO_NET_CTRL_MAC, - VIRTIO_NET_CTRL_MAC_ADDR_SET, - &data, 1); - if (unlikely(dev_written < 0)) { - return dev_written; - } - if (*s->status != VIRTIO_NET_OK) { - return -EIO; + ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, + VIRTIO_NET_CTRL_MAC, + VIRTIO_NET_CTRL_MAC_ADDR_SET, + &data, 1); + if (unlikely(r < 0)) { + return r; } } @@ -738,15 +777,12 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n, .iov_len = mul_macs_size, }, }; - ssize_t dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, + ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, VIRTIO_NET_CTRL_MAC, VIRTIO_NET_CTRL_MAC_TABLE_SET, data, ARRAY_SIZE(data)); - if (unlikely(dev_written < 0)) { - return dev_written; - } - if (*s->status != VIRTIO_NET_OK) { - return -EIO; + if (unlikely(r < 0)) { + return r; } return 0; @@ -757,7 +793,7 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s, void **out_cursor, void **in_cursor) { struct virtio_net_ctrl_mq mq; - ssize_t dev_written; + ssize_t r; if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_MQ)) { return 0; @@ -768,15 +804,12 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s, .iov_base = &mq, .iov_len = sizeof(mq), }; - dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, - VIRTIO_NET_CTRL_MQ, - VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, - &data, 1); - if (unlikely(dev_written < 0)) { - return dev_written; - } - if (*s->status != VIRTIO_NET_OK) { - return -EIO; + r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, + VIRTIO_NET_CTRL_MQ, + VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, + &data, 1); + if (unlikely(r < 0)) { + return r; } return 0; @@ -787,7 +820,7 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s, void **out_cursor, void **in_cursor) { uint64_t offloads; - ssize_t dev_written; + ssize_t r; if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) { @@ -815,15 +848,12 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s, .iov_base = &offloads, .iov_len = sizeof(offloads), }; - dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, - VIRTIO_NET_CTRL_GUEST_OFFLOADS, - VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, - &data, 1); - if (unlikely(dev_written < 0)) { - return dev_written; - } - if (*s->status != VIRTIO_NET_OK) { - return -EIO; + r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, + VIRTIO_NET_CTRL_GUEST_OFFLOADS, + VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, + &data, 1); + if (unlikely(r < 0)) { + return r; } return 0; @@ -838,15 +868,12 @@ static int vhost_vdpa_net_load_rx_mode(VhostVDPAState *s, .iov_base = &on, .iov_len = sizeof(on), }; - ssize_t dev_written; + ssize_t r; - dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, - VIRTIO_NET_CTRL_RX, cmd, &data, 1); - if (unlikely(dev_written < 0)) { - return dev_written; - } - if (*s->status != VIRTIO_NET_OK) { - return -EIO; + r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, + VIRTIO_NET_CTRL_RX, cmd, &data, 1); + if (unlikely(r < 0)) { + return r; } return 0; @@ -1001,15 +1028,12 @@ static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s, .iov_base = &vid, .iov_len = sizeof(vid), }; - ssize_t dev_written = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, - VIRTIO_NET_CTRL_VLAN, - VIRTIO_NET_CTRL_VLAN_ADD, - &data, 1); - if (unlikely(dev_written < 0)) { - return dev_written; - } - if (unlikely(*s->status != VIRTIO_NET_OK)) { - return -EIO; + ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor, + VIRTIO_NET_CTRL_VLAN, + VIRTIO_NET_CTRL_VLAN_ADD, + &data, 1); + if (unlikely(r < 0)) { + return r; } return 0; @@ -1078,6 +1102,17 @@ static int vhost_vdpa_net_load(NetClientState *nc) return r; } + /* + * We need to poll and check all pending device's used buffers. + * + * We can poll here since we've had BQL from the time + * we sent the descriptor. + */ + r = vhost_vdpa_net_svq_flush(s, in_cursor - (void *)s->status); + if (unlikely(r)) { + return r; + } + return 0; }
This patch enables sending CVQ state load commands in parallel at device startup by following steps: * Refactor vhost_vdpa_net_load_cmd() to iterate through the control commands shadow buffers. This allows different CVQ state load commands to use their own unique buffers. * Delay the polling and checking of buffers until either the SVQ is full or control commands shadow buffers are full. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1578 Signed-off-by: Hawkins Jiawei <yin31149@gmail.com> --- net/vhost-vdpa.c | 157 +++++++++++++++++++++++++++++------------------ 1 file changed, 96 insertions(+), 61 deletions(-)