[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] virtio-net: clear guest_announce feature if no cvq backend
From: |
Eugenio Perez Martin |
Subject: |
Re: [PATCH] virtio-net: clear guest_announce feature if no cvq backend |
Date: |
Wed, 25 Jan 2023 16:52:09 +0100 |
On Wed, Jan 25, 2023 at 4:20 PM David Edmondson
<david.edmondson@oracle.com> wrote:
>
> On Tuesday, 2023-01-24 at 17:11:59 +01, Eugenio Pérez wrote:
> > Since GUEST_ANNOUNCE is emulated the feature bit could be set without
> > backend support. This happens in the vDPA case.
> >
> > However, backend vDPA parent may not have CVQ support. This causes an
> > incoherent feature set, and the driver may refuse to start. This
> > happens in virtio-net Linux driver.
>
> Could you now simplify the tests in virtio_net_announce() and
> virtio_net_post_load_device() to look only for the presence of
> GUEST_ANNOUNCE, given that you can now presume that it implies CTRL_VQ?
>
That's a good question. As far as I know qemu emits an error if only
GUEST_ANNOUNCE is given in a purely emulated device.
At this moment vhost-kernel and vhost-vdpa do not handle it, but
vhost-user do. Would it be beneficial to preserve previous behavior
and passthrough the features? I guess not, so I think we could
simplify those functions on top of this series.
> But anyway:
>
> Reviewed-by: David Edmondson <david.edmondson@oracle.com>
>
Thanks for the review!
> > This may be solved differently in the future. Qemu is able to emulate a
> > CVQ just for guest_announce purposes, helping guest to notify the new
> > location with vDPA devices that does not support it. However, this is
> > left as a TODO as it is way more complex to backport.
> >
> > Tested with vdpa_net_sim, toggling manually VIRTIO_NET_F_CTRL_VQ in the
> > driver and migrating it with x-svq=on.
> >
> > Fixes: 980003debddd ("vdpa: do not handle VIRTIO_NET_F_GUEST_ANNOUNCE in
> > vhost-vdpa")
> > Reported-by: Dawar, Gautam <gautam.dawar@amd.com>
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> > hw/net/virtio-net.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 3ae909041a..09d5c7a664 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -820,6 +820,21 @@ static uint64_t virtio_net_get_features(VirtIODevice
> > *vdev, uint64_t features,
> > features |= (1ULL << VIRTIO_NET_F_MTU);
> > }
> >
> > + /*
> > + * Since GUEST_ANNOUNCE is emulated the feature bit could be set
> > without
> > + * enabled. This happens in the vDPA case.
> > + *
> > + * Make sure the feature set is not incoherent, as the driver could
> > refuse
> > + * to start.
> > + *
> > + * TODO: QEMU is able to emulate a CVQ just for guest_announce
> > purposes,
> > + * helping guest to notify the new location with vDPA devices that
> > does not
> > + * support it.
> > + */
> > + if (!virtio_has_feature(vdev->backend_features, VIRTIO_NET_F_CTRL_VQ))
> > {
> > + virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ANNOUNCE);
> > + }
> > +
> > return features;
> > }
> --
> Why stay in college? Why go to night school?
>