[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 05/10] vdpa: move SVQ vring features check to net/
From: |
Eugenio Perez Martin |
Subject: |
Re: [PATCH v6 05/10] vdpa: move SVQ vring features check to net/ |
Date: |
Fri, 11 Nov 2022 08:55:48 +0100 |
On Fri, Nov 11, 2022 at 8:34 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2022/11/10 21:09, Eugenio Perez Martin 写道:
> > On Thu, Nov 10, 2022 at 6:40 AM Jason Wang <jasowang@redhat.com> wrote:
> >>
> >> 在 2022/11/9 01:07, Eugenio Pérez 写道:
> >>> The next patches will start control SVQ if possible. However, we don't
> >>> know if that will be possible at qemu boot anymore.
> >>
> >> If I was not wrong, there's no device specific feature that is checked
> >> in the function. So it should be general enough to be used by devices
> >> other than net. Then I don't see any advantage of doing this.
> >>
> > Because vhost_vdpa_init_svq is called at qemu boot, failing if it is
> > not possible to shadow the Virtqueue.
> >
> > Now the CVQ will be shadowed if possible, so we need to check this at
> > device start, not at initialization.
>
>
> Any reason we can't check this at device start? We don't need
> driver_features and we can do any probing to make sure cvq has an unique
> group during initialization time.
>
We need the CVQ index to check if it has an independent group. CVQ
index depends on the features the guest's ack:
* If it acks _F_MQ, it is the last one.
* If it doesn't, CVQ idx is 2.
We cannot have acked features at initialization, and they could
change: It is valid for a guest to ack _F_MQ, then reset the device,
then not ack it.
>
> > To store this information at boot
> > time is not valid anymore, because v->shadow_vqs_enabled is not valid
> > at this time anymore.
>
>
> Ok, but this doesn't explain why it is net specific but vhost-vdpa specific.
>
We can try to move it to a vhost op, but we have the same problem as
the svq array allocation: We don't have the right place in vhost ops
to check this. Maybe vhost_set_features is the right one here?
Thanks!
> Thanks
>
>
> >
> > Thanks!
> >
> >> Thanks
> >>
> >>
> >>> Since the moved checks will be already evaluated at net/ to know if it
> >>> is ok to shadow CVQ, move them.
> >>>
> >>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> >>> ---
> >>> hw/virtio/vhost-vdpa.c | 33 ++-------------------------------
> >>> net/vhost-vdpa.c | 3 ++-
> >>> 2 files changed, 4 insertions(+), 32 deletions(-)
> >>>
> >>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> >>> index 3df2775760..146f0dcb40 100644
> >>> --- a/hw/virtio/vhost-vdpa.c
> >>> +++ b/hw/virtio/vhost-vdpa.c
> >>> @@ -402,29 +402,9 @@ static int vhost_vdpa_get_dev_features(struct
> >>> vhost_dev *dev,
> >>> return ret;
> >>> }
> >>>
> >>> -static int vhost_vdpa_init_svq(struct vhost_dev *hdev, struct vhost_vdpa
> >>> *v,
> >>> - Error **errp)
> >>> +static void vhost_vdpa_init_svq(struct vhost_dev *hdev, struct
> >>> vhost_vdpa *v)
> >>> {
> >>> g_autoptr(GPtrArray) shadow_vqs = NULL;
> >>> - uint64_t dev_features, svq_features;
> >>> - int r;
> >>> - bool ok;
> >>> -
> >>> - if (!v->shadow_vqs_enabled) {
> >>> - return 0;
> >>> - }
> >>> -
> >>> - r = vhost_vdpa_get_dev_features(hdev, &dev_features);
> >>> - if (r != 0) {
> >>> - error_setg_errno(errp, -r, "Can't get vdpa device features");
> >>> - return r;
> >>> - }
> >>> -
> >>> - svq_features = dev_features;
> >>> - ok = vhost_svq_valid_features(svq_features, errp);
> >>> - if (unlikely(!ok)) {
> >>> - return -1;
> >>> - }
> >>>
> >>> shadow_vqs = g_ptr_array_new_full(hdev->nvqs, vhost_svq_free);
> >>> for (unsigned n = 0; n < hdev->nvqs; ++n) {
> >>> @@ -436,7 +416,6 @@ static int vhost_vdpa_init_svq(struct vhost_dev
> >>> *hdev, struct vhost_vdpa *v,
> >>> }
> >>>
> >>> v->shadow_vqs = g_steal_pointer(&shadow_vqs);
> >>> - return 0;
> >>> }
> >>>
> >>> static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error
> >>> **errp)
> >>> @@ -461,11 +440,7 @@ static int vhost_vdpa_init(struct vhost_dev *dev,
> >>> void *opaque, Error **errp)
> >>> dev->opaque = opaque ;
> >>> v->listener = vhost_vdpa_memory_listener;
> >>> v->msg_type = VHOST_IOTLB_MSG_V2;
> >>> - ret = vhost_vdpa_init_svq(dev, v, errp);
> >>> - if (ret) {
> >>> - goto err;
> >>> - }
> >>> -
> >>> + vhost_vdpa_init_svq(dev, v);
> >>> vhost_vdpa_get_iova_range(v);
> >>>
> >>> if (!vhost_vdpa_first_dev(dev)) {
> >>> @@ -476,10 +451,6 @@ static int vhost_vdpa_init(struct vhost_dev *dev,
> >>> void *opaque, Error **errp)
> >>> VIRTIO_CONFIG_S_DRIVER);
> >>>
> >>> return 0;
> >>> -
> >>> -err:
> >>> - ram_block_discard_disable(false);
> >>> - return ret;
> >>> }
> >>>
> >>> static void vhost_vdpa_host_notifier_uninit(struct vhost_dev *dev,
> >>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> >>> index d3b1de481b..fb35b17ab4 100644
> >>> --- a/net/vhost-vdpa.c
> >>> +++ b/net/vhost-vdpa.c
> >>> @@ -117,9 +117,10 @@ static bool
> >>> vhost_vdpa_net_valid_svq_features(uint64_t features, Error **errp)
> >>> if (invalid_dev_features) {
> >>> error_setg(errp, "vdpa svq does not work with features 0x%"
> >>> PRIx64,
> >>> invalid_dev_features);
> >>> + return false;
> >>> }
> >>>
> >>> - return !invalid_dev_features;
> >>> + return vhost_svq_valid_features(features, errp);
> >>> }
> >>>
> >>> static int vhost_vdpa_net_check_device_id(struct vhost_net *net)
>
- Re: [PATCH v6 01/10] vdpa: Use v->shadow_vqs_enabled in vhost_vdpa_svqs_start & stop, (continued)
[PATCH v6 03/10] vhost: Allocate SVQ device file descriptors at device start, Eugenio Pérez, 2022/11/08
[PATCH v6 04/10] vdpa: add vhost_vdpa_net_valid_svq_features, Eugenio Pérez, 2022/11/08
[PATCH v6 05/10] vdpa: move SVQ vring features check to net/, Eugenio Pérez, 2022/11/08
- Re: [PATCH v6 05/10] vdpa: move SVQ vring features check to net/, Jason Wang, 2022/11/10
- Re: [PATCH v6 05/10] vdpa: move SVQ vring features check to net/, Eugenio Perez Martin, 2022/11/10
- Re: [PATCH v6 05/10] vdpa: move SVQ vring features check to net/, Jason Wang, 2022/11/11
- Re: [PATCH v6 05/10] vdpa: move SVQ vring features check to net/,
Eugenio Perez Martin <=
- Re: [PATCH v6 05/10] vdpa: move SVQ vring features check to net/, Jason Wang, 2022/11/11
- Re: [PATCH v6 05/10] vdpa: move SVQ vring features check to net/, Eugenio Perez Martin, 2022/11/11
- Re: [PATCH v6 05/10] vdpa: move SVQ vring features check to net/, Jason Wang, 2022/11/14
- Re: [PATCH v6 05/10] vdpa: move SVQ vring features check to net/, Eugenio Perez Martin, 2022/11/14
[PATCH v6 06/10] vdpa: Allocate SVQ unconditionally, Eugenio Pérez, 2022/11/08
[PATCH v6 07/10] vdpa: Add asid parameter to vhost_vdpa_dma_map/unmap, Eugenio Pérez, 2022/11/08