1. Patchew
  2. QEMU
  3. [PATCH 00/40] vdpa-net: improve migration downtime through descriptor ASID and persistent IOTLB
  4. View patch

[PATCH 03/40] vdpa: probe descriptor group index for data vqs

Si-Wei Liu posted 40 patches 11 months, 3 weeks ago
[PATCH 03/40] vdpa: probe descriptor group index for data vqs
Posted by Si-Wei Liu 11 months, 3 weeks ago 
Getting it ahead at initialization time instead of start time allows
decision making independent of device status, while reducing failure
possibility in starting device or during migration.

Adding function vhost_vdpa_probe_desc_group() for that end. This
function will be used to probe the descriptor group for data vqs.

Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 net/vhost-vdpa.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 887c329..0cf3147 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -1688,6 +1688,95 @@ out:
     return r;
 }
 
+static int vhost_vdpa_probe_desc_group(int device_fd, uint64_t features,
+                                       int vq_index, int64_t *desc_grpidx,
+                                       Error **errp)
+{
+    uint64_t backend_features;
+    int64_t vq_group, desc_group;
+    uint8_t saved_status = 0;
+    uint8_t status = 0;
+    int r;
+
+    ERRP_GUARD();
+
+    r = ioctl(device_fd, VHOST_GET_BACKEND_FEATURES, &backend_features);
+    if (unlikely(r < 0)) {
+        error_setg_errno(errp, errno, "Cannot get vdpa backend_features");
+        return r;
+    }
+
+    if (!(backend_features & BIT_ULL(VHOST_BACKEND_F_IOTLB_ASID))) {
+        return 0;
+    }
+
+    if (!(backend_features & BIT_ULL(VHOST_BACKEND_F_DESC_ASID))) {
+        return 0;
+    }
+
+    r = ioctl(device_fd, VHOST_VDPA_GET_STATUS, &saved_status);
+    if (unlikely(r)) {
+        error_setg_errno(errp, -r, "Cannot get device status");
+        goto out;
+    }
+
+    r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status);
+    if (unlikely(r)) {
+        error_setg_errno(errp, -r, "Cannot reset device");
+        goto out;
+    }
+
+    r = ioctl(device_fd, VHOST_SET_FEATURES, &features);
+    if (unlikely(r)) {
+        error_setg_errno(errp, errno, "Cannot set features");
+    }
+
+    status = VIRTIO_CONFIG_S_ACKNOWLEDGE |
+             VIRTIO_CONFIG_S_DRIVER |
+             VIRTIO_CONFIG_S_FEATURES_OK;
+
+    r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status);
+    if (unlikely(r)) {
+        error_setg_errno(errp, -r, "Cannot set device status");
+        goto out;
+    }
+
+    vq_group = vhost_vdpa_get_vring_group(device_fd, vq_index, errp);
+    if (unlikely(vq_group < 0)) {
+        if (vq_group != -ENOTSUP) {
+            r = vq_group;
+            goto out;
+        }
+
+        /*
+         * The kernel report VHOST_BACKEND_F_IOTLB_ASID if the vdpa frontend
+         * support ASID even if the parent driver does not.
+         */
+        error_free(*errp);
+        *errp = NULL;
+        r = 0;
+        goto out;
+    }
+
+    desc_group = vhost_vdpa_get_vring_desc_group(device_fd, vq_index,
+                                                 errp);
+    if (unlikely(desc_group < 0)) {
+        r = desc_group;
+        goto out;
+    } else if (desc_group != vq_group) {
+        *desc_grpidx = desc_group;
+    }
+    r = 1;
+
+out:
+    status = 0;
+    ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status);
+    if (saved_status) {
+        ioctl(device_fd, VHOST_VDPA_SET_STATUS, &saved_status);
+    }
+    return r;
+}
+
 static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
                                        const char *device,
                                        const char *name,
-- 
1.8.3.1
Re: [PATCH 03/40] vdpa: probe descriptor group index for data vqs
Posted by Jason Wang 10 months, 2 weeks ago 
On Fri, Dec 8, 2023 at 2:53 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> Getting it ahead at initialization time instead of start time allows
> decision making independent of device status, while reducing failure
> possibility in starting device or during migration.
>
> Adding function vhost_vdpa_probe_desc_group() for that end. This
> function will be used to probe the descriptor group for data vqs.
>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
>  net/vhost-vdpa.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 887c329..0cf3147 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1688,6 +1688,95 @@ out:
>      return r;
>  }
>
> +static int vhost_vdpa_probe_desc_group(int device_fd, uint64_t features,
> +                                       int vq_index, int64_t *desc_grpidx,
> +                                       Error **errp)
> +{
> +    uint64_t backend_features;
> +    int64_t vq_group, desc_group;
> +    uint8_t saved_status = 0;
> +    uint8_t status = 0;
> +    int r;
> +
> +    ERRP_GUARD();
> +
> +    r = ioctl(device_fd, VHOST_GET_BACKEND_FEATURES, &backend_features);
> +    if (unlikely(r < 0)) {
> +        error_setg_errno(errp, errno, "Cannot get vdpa backend_features");
> +        return r;
> +    }
> +
> +    if (!(backend_features & BIT_ULL(VHOST_BACKEND_F_IOTLB_ASID))) {
> +        return 0;
> +    }
> +
> +    if (!(backend_features & BIT_ULL(VHOST_BACKEND_F_DESC_ASID))) {
> +        return 0;
> +    }
> +
> +    r = ioctl(device_fd, VHOST_VDPA_GET_STATUS, &saved_status);
> +    if (unlikely(r)) {
> +        error_setg_errno(errp, -r, "Cannot get device status");
> +        goto out;
> +    }

I wonder what's the reason for the status being saved and restored?

We don't do this in vhost_vdpa_probe_cvq_isolation().

Thanks
Re: [PATCH 03/40] vdpa: probe descriptor group index for data vqs
Posted by Eugenio Perez Martin 11 months, 3 weeks ago 
On Thu, Dec 7, 2023 at 7:53 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> Getting it ahead at initialization time instead of start time allows
> decision making independent of device status, while reducing failure
> possibility in starting device or during migration.
>
> Adding function vhost_vdpa_probe_desc_group() for that end. This
> function will be used to probe the descriptor group for data vqs.
>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
>  net/vhost-vdpa.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 887c329..0cf3147 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1688,6 +1688,95 @@ out:
>      return r;
>  }
>
> +static int vhost_vdpa_probe_desc_group(int device_fd, uint64_t features,
> +                                       int vq_index, int64_t *desc_grpidx,
> +                                       Error **errp)
> +{
> +    uint64_t backend_features;
> +    int64_t vq_group, desc_group;
> +    uint8_t saved_status = 0;
> +    uint8_t status = 0;
> +    int r;
> +
> +    ERRP_GUARD();
> +
> +    r = ioctl(device_fd, VHOST_GET_BACKEND_FEATURES, &backend_features);
> +    if (unlikely(r < 0)) {
> +        error_setg_errno(errp, errno, "Cannot get vdpa backend_features");
> +        return r;
> +    }
> +
> +    if (!(backend_features & BIT_ULL(VHOST_BACKEND_F_IOTLB_ASID))) {
> +        return 0;
> +    }
> +
> +    if (!(backend_features & BIT_ULL(VHOST_BACKEND_F_DESC_ASID))) {
> +        return 0;
> +    }
> +
> +    r = ioctl(device_fd, VHOST_VDPA_GET_STATUS, &saved_status);
> +    if (unlikely(r)) {
> +        error_setg_errno(errp, -r, "Cannot get device status");
> +        goto out;

Nit, we could return here directly, can't we?

> +    }
> +
> +    r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status);
> +    if (unlikely(r)) {
> +        error_setg_errno(errp, -r, "Cannot reset device");
> +        goto out;
> +    }
> +
> +    r = ioctl(device_fd, VHOST_SET_FEATURES, &features);
> +    if (unlikely(r)) {
> +        error_setg_errno(errp, errno, "Cannot set features");

missing goto out?

> +    }
> +
> +    status = VIRTIO_CONFIG_S_ACKNOWLEDGE |
> +             VIRTIO_CONFIG_S_DRIVER |
> +             VIRTIO_CONFIG_S_FEATURES_OK;
> +
> +    r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status);
> +    if (unlikely(r)) {
> +        error_setg_errno(errp, -r, "Cannot set device status");
> +        goto out;
> +    }
> +
> +    vq_group = vhost_vdpa_get_vring_group(device_fd, vq_index, errp);
> +    if (unlikely(vq_group < 0)) {
> +        if (vq_group != -ENOTSUP) {
> +            r = vq_group;
> +            goto out;
> +        }
> +
> +        /*
> +         * The kernel report VHOST_BACKEND_F_IOTLB_ASID if the vdpa frontend
> +         * support ASID even if the parent driver does not.
> +         */
> +        error_free(*errp);
> +        *errp = NULL;
> +        r = 0;
> +        goto out;
> +    }
> +
> +    desc_group = vhost_vdpa_get_vring_desc_group(device_fd, vq_index,
> +                                                 errp);
> +    if (unlikely(desc_group < 0)) {
> +        r = desc_group;
> +        goto out;
> +    } else if (desc_group != vq_group) {
> +        *desc_grpidx = desc_group;
> +    }
> +    r = 1;
> +
> +out:
> +    status = 0;
> +    ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status);
> +    if (saved_status) {
> +        ioctl(device_fd, VHOST_VDPA_SET_STATUS, &saved_status);
> +    }
> +    return r;
> +}
> +

It is invalid to add static functions without a caller, I think the
compiler will complain about this.

>  static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
>                                         const char *device,
>                                         const char *name,
> --
> 1.8.3.1
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.