| Message ID | 20221019125210.226291-7-eperezma@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Guest announce feature emulation using Shadow VirtQueue | expand |
On Wed, Oct 19, 2022 at 8:52 PM Eugenio Pérez <eperezma@redhat.com> wrote: > > Since this capability is emulated by qemu shadowed CVQ cannot forward it > to the device. I wonder what happens for a device that has GUEST_ANNOUNCE support on its own? > Process all that command within qemu. > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > net/vhost-vdpa.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > index 3374c21b4d..5fda405a66 100644 > --- a/net/vhost-vdpa.c > +++ b/net/vhost-vdpa.c > @@ -488,9 +488,18 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq, > out.iov_len = iov_to_buf(elem->out_sg, elem->out_num, 0, > s->cvq_cmd_out_buffer, > vhost_vdpa_net_cvq_cmd_len()); > - dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); > - if (unlikely(dev_written < 0)) { > - goto out; > + if (*(uint8_t *)s->cvq_cmd_out_buffer == VIRTIO_NET_CTRL_ANNOUNCE) { Interesting, I thought we can do better by forbidding the code that goes into vhost-vDPA specific code, everything should be set at virtio-net.c level. Thanks > + /* > + * Guest announce capability is emulated by qemu, so dont forward to > + * the device. > + */ > + dev_written = sizeof(status); > + *s->status = VIRTIO_NET_OK; > + } else { > + dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); > + if (unlikely(dev_written < 0)) { > + goto out; > + } > } > > if (unlikely(dev_written < sizeof(status))) { > -- > 2.31.1 >
On Thu, Oct 20, 2022 at 6:35 AM Jason Wang <jasowang@redhat.com> wrote: > > On Wed, Oct 19, 2022 at 8:52 PM Eugenio Pérez <eperezma@redhat.com> wrote: > > > > Since this capability is emulated by qemu shadowed CVQ cannot forward it > > to the device. > > I wonder what happens for a device that has GUEST_ANNOUNCE support on its own? > If SVQ is enabled the feature is always emulated by qemu by this series. if SVQ is disabled then the device is the one in charge of all of it. > > Process all that command within qemu. > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > --- > > net/vhost-vdpa.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > > index 3374c21b4d..5fda405a66 100644 > > --- a/net/vhost-vdpa.c > > +++ b/net/vhost-vdpa.c > > @@ -488,9 +488,18 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq, > > out.iov_len = iov_to_buf(elem->out_sg, elem->out_num, 0, > > s->cvq_cmd_out_buffer, > > vhost_vdpa_net_cvq_cmd_len()); > > - dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); > > - if (unlikely(dev_written < 0)) { > > - goto out; > > + if (*(uint8_t *)s->cvq_cmd_out_buffer == VIRTIO_NET_CTRL_ANNOUNCE) { > > Interesting, I thought we can do better by forbidding the code that > goes into vhost-vDPA specific code, everything should be set at > virtio-net.c level. > Do you mean to move the SVQ processing to each handle_output? It's somehow on the roadmap but I'm not sure if it has more priority than implementing the different features. Thanks! > Thanks > > > + /* > > + * Guest announce capability is emulated by qemu, so dont forward to > > + * the device. > > + */ > > + dev_written = sizeof(status); > > + *s->status = VIRTIO_NET_OK; > > + } else { > > + dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); > > + if (unlikely(dev_written < 0)) { > > + goto out; > > + } > > } > > > > if (unlikely(dev_written < sizeof(status))) { > > -- > > 2.31.1 > > >
On Thu, Oct 20, 2022 at 3:01 PM Eugenio Perez Martin <eperezma@redhat.com> wrote: > > On Thu, Oct 20, 2022 at 6:35 AM Jason Wang <jasowang@redhat.com> wrote: > > > > On Wed, Oct 19, 2022 at 8:52 PM Eugenio Pérez <eperezma@redhat.com> wrote: > > > > > > Since this capability is emulated by qemu shadowed CVQ cannot forward it > > > to the device. > > > > I wonder what happens for a device that has GUEST_ANNOUNCE support on its own? > > > > If SVQ is enabled the feature is always emulated by qemu by this series. > > if SVQ is disabled then the device is the one in charge of all of it. Ok, I see. > > > > Process all that command within qemu. > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > > --- > > > net/vhost-vdpa.c | 15 ++++++++++++--- > > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > > > index 3374c21b4d..5fda405a66 100644 > > > --- a/net/vhost-vdpa.c > > > +++ b/net/vhost-vdpa.c > > > @@ -488,9 +488,18 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq, > > > out.iov_len = iov_to_buf(elem->out_sg, elem->out_num, 0, > > > s->cvq_cmd_out_buffer, > > > vhost_vdpa_net_cvq_cmd_len()); > > > - dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); > > > - if (unlikely(dev_written < 0)) { > > > - goto out; > > > + if (*(uint8_t *)s->cvq_cmd_out_buffer == VIRTIO_NET_CTRL_ANNOUNCE) { > > > > Interesting, I thought we can do better by forbidding the code that > > goes into vhost-vDPA specific code, everything should be set at > > virtio-net.c level. > > > > Do you mean to move the SVQ processing to each handle_output? It's > somehow on the roadmap but I'm not sure if it has more priority than > implementing the different features. Right, but I think we need to find a way to eliminate the casting here. Thanks > > Thanks! > > > > Thanks > > > > > + /* > > > + * Guest announce capability is emulated by qemu, so dont forward to > > > + * the device. > > > + */ > > > + dev_written = sizeof(status); > > > + *s->status = VIRTIO_NET_OK; > > > + } else { > > > + dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); > > > + if (unlikely(dev_written < 0)) { > > > + goto out; > > > + } > > > } > > > > > > if (unlikely(dev_written < sizeof(status))) { > > > -- > > > 2.31.1 > > > > > >
On Fri, Oct 21, 2022 at 5:02 AM Jason Wang <jasowang@redhat.com> wrote: > > On Thu, Oct 20, 2022 at 3:01 PM Eugenio Perez Martin > <eperezma@redhat.com> wrote: > > > > On Thu, Oct 20, 2022 at 6:35 AM Jason Wang <jasowang@redhat.com> wrote: > > > > > > On Wed, Oct 19, 2022 at 8:52 PM Eugenio Pérez <eperezma@redhat.com> wrote: > > > > > > > > Since this capability is emulated by qemu shadowed CVQ cannot forward it > > > > to the device. > > > > > > I wonder what happens for a device that has GUEST_ANNOUNCE support on its own? > > > > > > > If SVQ is enabled the feature is always emulated by qemu by this series. > > > > if SVQ is disabled then the device is the one in charge of all of it. > > Ok, I see. > > > > > > > Process all that command within qemu. > > > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > > > --- > > > > net/vhost-vdpa.c | 15 ++++++++++++--- > > > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > > > > index 3374c21b4d..5fda405a66 100644 > > > > --- a/net/vhost-vdpa.c > > > > +++ b/net/vhost-vdpa.c > > > > @@ -488,9 +488,18 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq, > > > > out.iov_len = iov_to_buf(elem->out_sg, elem->out_num, 0, > > > > s->cvq_cmd_out_buffer, > > > > vhost_vdpa_net_cvq_cmd_len()); > > > > - dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); > > > > - if (unlikely(dev_written < 0)) { > > > > - goto out; > > > > + if (*(uint8_t *)s->cvq_cmd_out_buffer == VIRTIO_NET_CTRL_ANNOUNCE) { > > > > > > Interesting, I thought we can do better by forbidding the code that > > > goes into vhost-vDPA specific code, everything should be set at > > > virtio-net.c level. > > > > > > > Do you mean to move the SVQ processing to each handle_output? It's > > somehow on the roadmap but I'm not sure if it has more priority than > > implementing the different features. > > Right, but I think we need to find a way to eliminate the casting here. > Would it work to use it this way? uint8_t *virtio_net_ctrl_class = s->cvq_cmd_out_buffer if (*virtio_net_ctrl_class == VIRTIO_NET_CTRL_ANNOUNCE) { ... } > Thanks > > > > > Thanks! > > > > > > > Thanks > > > > > > > + /* > > > > + * Guest announce capability is emulated by qemu, so dont forward to > > > > + * the device. > > > > + */ > > > > + dev_written = sizeof(status); > > > > + *s->status = VIRTIO_NET_OK; > > > > + } else { > > > > + dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); > > > > + if (unlikely(dev_written < 0)) { > > > > + goto out; > > > > + } > > > > } > > > > > > > > if (unlikely(dev_written < sizeof(status))) { > > > > -- > > > > 2.31.1 > > > > > > > > > >
On Fri, Oct 21, 2022 at 5:05 PM Eugenio Perez Martin <eperezma@redhat.com> wrote: > > On Fri, Oct 21, 2022 at 5:02 AM Jason Wang <jasowang@redhat.com> wrote: > > > > On Thu, Oct 20, 2022 at 3:01 PM Eugenio Perez Martin > > <eperezma@redhat.com> wrote: > > > > > > On Thu, Oct 20, 2022 at 6:35 AM Jason Wang <jasowang@redhat.com> wrote: > > > > > > > > On Wed, Oct 19, 2022 at 8:52 PM Eugenio Pérez <eperezma@redhat.com> wrote: > > > > > > > > > > Since this capability is emulated by qemu shadowed CVQ cannot forward it > > > > > to the device. > > > > > > > > I wonder what happens for a device that has GUEST_ANNOUNCE support on its own? > > > > > > > > > > If SVQ is enabled the feature is always emulated by qemu by this series. > > > > > > if SVQ is disabled then the device is the one in charge of all of it. > > > > Ok, I see. > > > > > > > > > > Process all that command within qemu. > > > > > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > > > > --- > > > > > net/vhost-vdpa.c | 15 ++++++++++++--- > > > > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > > > > > index 3374c21b4d..5fda405a66 100644 > > > > > --- a/net/vhost-vdpa.c > > > > > +++ b/net/vhost-vdpa.c > > > > > @@ -488,9 +488,18 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq, > > > > > out.iov_len = iov_to_buf(elem->out_sg, elem->out_num, 0, > > > > > s->cvq_cmd_out_buffer, > > > > > vhost_vdpa_net_cvq_cmd_len()); > > > > > - dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); > > > > > - if (unlikely(dev_written < 0)) { > > > > > - goto out; > > > > > + if (*(uint8_t *)s->cvq_cmd_out_buffer == VIRTIO_NET_CTRL_ANNOUNCE) { > > > > > > > > Interesting, I thought we can do better by forbidding the code that > > > > goes into vhost-vDPA specific code, everything should be set at > > > > virtio-net.c level. > > > > > > > > > > Do you mean to move the SVQ processing to each handle_output? It's > > > somehow on the roadmap but I'm not sure if it has more priority than > > > implementing the different features. > > > > Right, but I think we need to find a way to eliminate the casting here. > > > > Would it work to use it this way? > uint8_t *virtio_net_ctrl_class = s->cvq_cmd_out_buffer > if (*virtio_net_ctrl_class == VIRTIO_NET_CTRL_ANNOUNCE) { > ... > } Something like this, yes. Thanks > > > Thanks > > > > > > > > Thanks! > > > > > > > > > > Thanks > > > > > > > > > + /* > > > > > + * Guest announce capability is emulated by qemu, so dont forward to > > > > > + * the device. > > > > > + */ > > > > > + dev_written = sizeof(status); > > > > > + *s->status = VIRTIO_NET_OK; > > > > > + } else { > > > > > + dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); > > > > > + if (unlikely(dev_written < 0)) { > > > > > + goto out; > > > > > + } > > > > > } > > > > > > > > > > if (unlikely(dev_written < sizeof(status))) { > > > > > -- > > > > > 2.31.1 > > > > > > > > > > > > > > >
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index 3374c21b4d..5fda405a66 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -488,9 +488,18 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq, out.iov_len = iov_to_buf(elem->out_sg, elem->out_num, 0, s->cvq_cmd_out_buffer, vhost_vdpa_net_cvq_cmd_len()); - dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); - if (unlikely(dev_written < 0)) { - goto out; + if (*(uint8_t *)s->cvq_cmd_out_buffer == VIRTIO_NET_CTRL_ANNOUNCE) { + /* + * Guest announce capability is emulated by qemu, so dont forward to + * the device. + */ + dev_written = sizeof(status); + *s->status = VIRTIO_NET_OK; + } else { + dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status)); + if (unlikely(dev_written < 0)) { + goto out; + } } if (unlikely(dev_written < sizeof(status))) {
Since this capability is emulated by qemu shadowed CVQ cannot forward it to the device. Process all that command within qemu. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- net/vhost-vdpa.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)