在 2023/10/4 01:44, Eugenio Perez Martin 写道: > On Tue, Aug 29, 2023 at 7:55 AM Hawkins Jiawei <yin31...@gmail.com> wrote: >> >> Next patches in this series will delay the polling >> and checking of buffers until either the SVQ is >> full or control commands shadow buffers are full, >> no longer perform an immediate poll and check of >> the device's used buffers for each CVQ state load command. >> >> To achieve this, this patch exposes >> vhost_svq_available_slots() and introduces a helper function, >> allowing QEMU to know whether the SVQ is full. >> >> Signed-off-by: Hawkins Jiawei <yin31...@gmail.com> >> Acked-by: Eugenio Pérez <epere...@redhat.com> >> --- >> hw/virtio/vhost-shadow-virtqueue.c | 2 +- >> hw/virtio/vhost-shadow-virtqueue.h | 1 + >> net/vhost-vdpa.c | 9 +++++++++ >> 3 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/hw/virtio/vhost-shadow-virtqueue.c >> b/hw/virtio/vhost-shadow-virtqueue.c >> index e731b1d2ea..fc5f408f77 100644 >> --- a/hw/virtio/vhost-shadow-virtqueue.c >> +++ b/hw/virtio/vhost-shadow-virtqueue.c >> @@ -66,7 +66,7 @@ bool vhost_svq_valid_features(uint64_t features, Error >> **errp) >> * >> * @svq: The svq >> */ >> -static uint16_t vhost_svq_available_slots(const VhostShadowVirtqueue *svq) >> +uint16_t vhost_svq_available_slots(const VhostShadowVirtqueue *svq) >> { >> return svq->num_free; >> } >> diff --git a/hw/virtio/vhost-shadow-virtqueue.h >> b/hw/virtio/vhost-shadow-virtqueue.h >> index 5bce67837b..19c842a15b 100644 >> --- a/hw/virtio/vhost-shadow-virtqueue.h >> +++ b/hw/virtio/vhost-shadow-virtqueue.h >> @@ -114,6 +114,7 @@ typedef struct VhostShadowVirtqueue { >> >> bool vhost_svq_valid_features(uint64_t features, Error **errp); >> >> +uint16_t vhost_svq_available_slots(const VhostShadowVirtqueue *svq); >> void vhost_svq_push_elem(VhostShadowVirtqueue *svq, >> const VirtQueueElement *elem, uint32_t len); >> int vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec *out_sg, > > I think it is ok to split this export in its own patch. If you decide > to do it that way, you can add my Acked-by.
I will split this in its own patch, thanks for your suggestion! > >> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c >> index a875767ee9..e6342b213f 100644 >> --- a/net/vhost-vdpa.c >> +++ b/net/vhost-vdpa.c >> @@ -620,6 +620,13 @@ static ssize_t vhost_vdpa_net_cvq_add(VhostVDPAState *s, >> return vhost_svq_poll(svq, 1); >> } >> >> +/* Convenience wrapper to get number of available SVQ descriptors */ >> +static uint16_t vhost_vdpa_net_svq_available_slots(VhostVDPAState *s) >> +{ >> + VhostShadowVirtqueue *svq = g_ptr_array_index(s->vhost_vdpa.shadow_vqs, >> 0); > > This is not really generic enough for all VhostVDPAState, as dataplane > ones have two svqs. > > I think the best is to just inline the function in the caller, as > there is only one, isn't it? If not, would it work to just replace > _net_ by _cvq_ or similar? > Yes, there should be only one user for this function, I will inline the function in the caller. >> + return vhost_svq_available_slots(svq); >> +} >> + >> static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, uint8_t class, >> uint8_t cmd, const struct iovec >> *data_sg, >> size_t data_num) >> @@ -640,6 +647,8 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState >> *s, uint8_t class, >> }; >> >> assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl)); >> + /* Each CVQ command has one out descriptor and one in descriptor */ >> + assert(vhost_vdpa_net_svq_available_slots(s) >= 2); >> > > I think we should remove this assertion. By the end of the series > there is an "if" checks explicitly for the opposite condition, and > flushing the queue in that case, so the code can never reach it. > Yes, you are right. I will remove this assertion. Thanks! >> /* pack the CVQ command header */ >> memcpy(s->cvq_cmd_out_buffer, &ctrl, sizeof(ctrl)); >> -- >> 2.25.1 >> >