On Thu, Aug 4, 2022 at 6:17 AM Jason Wang <jasow...@redhat.com> wrote: > > > 在 2022/8/3 01:57, Eugenio Pérez 写道: > > So its generic enough to accept any out sg buffer and we can inject > > NIC state messages. > > > > Signed-off-by: Eugenio Pérez <epere...@redhat.com> > > --- > > v5: Accept out sg instead of dev_buffers[] > > --- > > net/vhost-vdpa.c | 13 +++++++------ > > 1 file changed, 7 insertions(+), 6 deletions(-) > > > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > > index 33bf3d6409..2421bca347 100644 > > --- a/net/vhost-vdpa.c > > +++ b/net/vhost-vdpa.c > > @@ -302,16 +302,16 @@ dma_map_err: > > } > > > > /** > > - * Copy the guest element into a dedicated buffer suitable to be sent to > > NIC > > + * Maps out sg and in buffer into dedicated buffers suitable to be sent to > > NIC > > */ > > -static bool vhost_vdpa_net_cvq_map_elem(VhostVDPAState *s, > > - VirtQueueElement *elem, > > - size_t *out_len) > > +static bool vhost_vdpa_net_cvq_map_sg(VhostVDPAState *s, > > + const struct iovec *out, size_t > > out_num, > > + size_t *out_len) > > > This still looks not genreal as there's no guarantee that we won't have > command-in-specific-data. One example is that Ali is working on the > virtio-net statistics fetching from the control virtqueue. > > So it looks to me we'd better have a general bounce_map here that accepts: > > 1) out_sg and out_num > 2) in_sg and in_num >
We don't need to pass in_sg for that: The only useful information is its size. Since the exposed buffer is an in one, it's enough to expose the s->cvq_cmd_in_buffer buffer in full. The caller already knows the device will write to it, so the only missing piece is to return the written length at vhost_vdpa_net_cvq_add. Is one page the right buffer size for the in buffer? Is it worth worrying about it before implementing the stat control command in qemu virtio-net? > In this level, we'd better not have any special care about the in as the > ack. We need to care about it. If a property has not been updated in the vdpa device (it returned VIRTIO_NET_ERR), we must not update the device model. We can move the processing from vhost_vdpa_net_cvq_add to vhost_vdpa_net_load and vhost_vdpa_net_handle_ctrl_avail, but the code gets duplicated then. > And we need do bouncing: > > 1) for out buffer, during map > 2) for in buffer during unmap > We can move the copy of the in_buffer to the unmap for sure. Thanks! > Thanks > > > > { > > size_t in_copied; > > bool ok; > > > > - ok = vhost_vdpa_cvq_map_buf(&s->vhost_vdpa, elem->out_sg, > > elem->out_num, > > + ok = vhost_vdpa_cvq_map_buf(&s->vhost_vdpa, out, out_num, > > vhost_vdpa_net_cvq_cmd_len(), > > s->cvq_cmd_out_buffer, out_len, false); > > if (unlikely(!ok)) { > > @@ -435,7 +435,8 @@ static int > > vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq, > > }; > > bool ok; > > > > - ok = vhost_vdpa_net_cvq_map_elem(s, elem, &dev_buffers[0].iov_len); > > + ok = vhost_vdpa_net_cvq_map_sg(s, elem->out_sg, elem->out_num, > > + &dev_buffers[0].iov_len); > > if (unlikely(!ok)) { > > goto out; > > } >