On Mon, Jul 11, 2022 at 10:00 AM Jason Wang <jasow...@redhat.com> wrote:
>
>
> 在 2022/7/7 02:39, Eugenio Pérez 写道:
> > VirtQueueElement comes from the guest, but we're heading SVQ to be able
> > to inject element without the guest's knowledge.
> >
> > To do so, make this accept sg buffers directly, instead of using
> > VirtQueueElement.
> >
> > Add vhost_svq_add_element to maintain element convenience
> >
> > Signed-off-by: Eugenio Pérez <epere...@redhat.com>
> > ---
> > hw/virtio/vhost-shadow-virtqueue.c | 38 +++++++++++++++++++++---------
> > 1 file changed, 27 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> > b/hw/virtio/vhost-shadow-virtqueue.c
> > index 2fc5789b73..46d3c1d74f 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > @@ -172,30 +172,32 @@ static bool
> > vhost_svq_vring_write_descs(VhostShadowVirtqueue *svq, hwaddr *sg,
> > }
> >
> > static bool vhost_svq_add_split(VhostShadowVirtqueue *svq,
> > - VirtQueueElement *elem, unsigned *head)
> > + const struct iovec *out_sg, size_t out_num,
> > + const struct iovec *in_sg, size_t in_num,
> > + unsigned *head)
> > {
> > unsigned avail_idx;
> > vring_avail_t *avail = svq->vring.avail;
> > bool ok;
> > - g_autofree hwaddr *sgs = g_new(hwaddr, MAX(elem->out_num,
> > elem->in_num));
> > + g_autofree hwaddr *sgs = NULL;
>
>
> Is this change a must for this patch? (looks not related to the
> decoupling anyhow)
>
Right, the delay on the variable assignment is an artifact I missed in
the cleaning. I can revert for the next version if any.
With that reverted, can I add the acked-by tag from you?
Thanks!
> Other looks good.
>
> Thanks
>
>
> >
> > *head = svq->free_head;
> >
> > /* We need some descriptors here */
> > - if (unlikely(!elem->out_num && !elem->in_num)) {
> > + if (unlikely(!out_num && !in_num)) {
> > qemu_log_mask(LOG_GUEST_ERROR,
> > "Guest provided element with no descriptors");
> > return false;
> > }
> >
> > - ok = vhost_svq_vring_write_descs(svq, sgs, elem->out_sg, elem->out_num,
> > - elem->in_num > 0, false);
> > + sgs = g_new(hwaddr, MAX(out_num, in_num));
> > + ok = vhost_svq_vring_write_descs(svq, sgs, out_sg, out_num, in_num > 0,
> > + false);
> > if (unlikely(!ok)) {
> > return false;
> > }
> >
> > - ok = vhost_svq_vring_write_descs(svq, sgs, elem->in_sg, elem->in_num,
> > false,
> > - true);
> > + ok = vhost_svq_vring_write_descs(svq, sgs, in_sg, in_num, false, true);
> > if (unlikely(!ok)) {
> > /* TODO unwind out_sg */
> > return false;
> > @@ -223,10 +225,13 @@ static bool vhost_svq_add_split(VhostShadowVirtqueue
> > *svq,
> > * takes ownership of the element: In case of failure, it is free and the
> > SVQ
> > * is considered broken.
> > */
> > -static bool vhost_svq_add(VhostShadowVirtqueue *svq, VirtQueueElement
> > *elem)
> > +static bool vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec
> > *out_sg,
> > + size_t out_num, const struct iovec *in_sg,
> > + size_t in_num, VirtQueueElement *elem)
> > {
> > unsigned qemu_head;
> > - bool ok = vhost_svq_add_split(svq, elem, &qemu_head);
> > + bool ok = vhost_svq_add_split(svq, out_sg, out_num, in_sg, in_num,
> > + &qemu_head);
> > if (unlikely(!ok)) {
> > g_free(elem);
> > return false;
> > @@ -250,6 +255,18 @@ static void vhost_svq_kick(VhostShadowVirtqueue *svq)
> > event_notifier_set(&svq->hdev_kick);
> > }
> >
> > +static bool vhost_svq_add_element(VhostShadowVirtqueue *svq,
> > + VirtQueueElement *elem)
> > +{
> > + bool ok = vhost_svq_add(svq, elem->out_sg, elem->out_num, elem->in_sg,
> > + elem->in_num, elem);
> > + if (ok) {
> > + vhost_svq_kick(svq);
> > + }
> > +
> > + return ok;
> > +}
> > +
> > /**
> > * Forward available buffers.
> > *
> > @@ -302,12 +319,11 @@ static void
> > vhost_handle_guest_kick(VhostShadowVirtqueue *svq)
> > return;
> > }
> >
> > - ok = vhost_svq_add(svq, elem);
> > + ok = vhost_svq_add_element(svq, g_steal_pointer(&elem));
> > if (unlikely(!ok)) {
> > /* VQ is broken, just return and ignore any other kicks */
> > return;
> > }
> > - vhost_svq_kick(svq);
> > }
> >
> > virtio_queue_set_notification(svq->vq, true);
>
