[PATCH 28/40] vdpa: support iotlb_batch_asid

Si-Wei Liu posted 40 patches 11 months, 3 weeks ago
[PATCH 28/40] vdpa: support iotlb_batch_asid
Posted by Si-Wei Liu 11 months, 3 weeks ago
Then it's possible to specify ASID when calling the DMA
batching API. If the ASID to work on doesn't align with
the ASID for ongoing transaction, the API will fail the
request and return negative, and the transaction will
remain intact as if no failed request ever had occured.

Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 hw/virtio/vhost-vdpa.c         | 25 +++++++++++++++++++------
 include/hw/virtio/vhost-vdpa.h |  1 +
 net/vhost-vdpa.c               |  1 +
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index d3f5721..b7896a8 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -189,15 +189,25 @@ static bool vhost_vdpa_map_batch_begin(VhostVDPAShared *s, uint32_t asid)
 
 static int vhost_vdpa_dma_batch_begin_once(VhostVDPAShared *s, uint32_t asid)
 {
-    if (!(s->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH)) ||
-        s->iotlb_batch_begin_sent) {
+    if (!(s->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) {
         return 0;
     }
 
-    if (vhost_vdpa_map_batch_begin(s, asid)) {
-        s->iotlb_batch_begin_sent = true;
+    if (s->iotlb_batch_begin_sent && s->iotlb_batch_asid != asid) {
+        return -1;
+    }
+
+    if (s->iotlb_batch_begin_sent) {
+        return 0;
     }
 
+    if (!vhost_vdpa_map_batch_begin(s, asid)) {
+        return 0;
+    }
+
+    s->iotlb_batch_begin_sent = true;
+    s->iotlb_batch_asid = asid;
+
     return 0;
 }
 
@@ -237,10 +247,13 @@ static int vhost_vdpa_dma_batch_end_once(VhostVDPAShared *s, uint32_t asid)
         return 0;
     }
 
-    if (vhost_vdpa_dma_batch_end(s, asid)) {
-        s->iotlb_batch_begin_sent = false;
+    if (!vhost_vdpa_dma_batch_end(s, asid)) {
+        return 0;
     }
 
+    s->iotlb_batch_begin_sent = false;
+    s->iotlb_batch_asid = -1;
+
     return 0;
 }
 
diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 0fe0f60..219316f 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -61,6 +61,7 @@ typedef struct vhost_vdpa_shared {
     bool map_thread_enabled;
 
     bool iotlb_batch_begin_sent;
+    uint32_t iotlb_batch_asid;
 
     /*
      * The memory listener has been registered, so DMA maps have been sent to
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index e9b96ed..bc72345 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -1933,6 +1933,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
         s->vhost_vdpa.shared->device_fd = vdpa_device_fd;
         s->vhost_vdpa.shared->iova_range = iova_range;
         s->vhost_vdpa.shared->shadow_data = svq;
+        s->vhost_vdpa.shared->iotlb_batch_asid = -1;
         s->vhost_vdpa.shared->refcnt++;
     } else if (!is_datapath) {
         s->cvq_cmd_out_buffer = mmap(NULL, vhost_vdpa_net_cvq_cmd_page_len(),
-- 
1.8.3.1
Re: [PATCH 28/40] vdpa: support iotlb_batch_asid
Posted by Jason Wang 10 months, 2 weeks ago
On Fri, Dec 8, 2023 at 2:51 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> Then it's possible to specify ASID when calling the DMA
> batching API. If the ASID to work on doesn't align with
> the ASID for ongoing transaction, the API will fail the
> request and return negative, and the transaction will
> remain intact as if no failed request ever had occured.
>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
>  hw/virtio/vhost-vdpa.c         | 25 +++++++++++++++++++------
>  include/hw/virtio/vhost-vdpa.h |  1 +
>  net/vhost-vdpa.c               |  1 +
>  3 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index d3f5721..b7896a8 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -189,15 +189,25 @@ static bool vhost_vdpa_map_batch_begin(VhostVDPAShared *s, uint32_t asid)
>
>  static int vhost_vdpa_dma_batch_begin_once(VhostVDPAShared *s, uint32_t asid)
>  {
> -    if (!(s->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH)) ||
> -        s->iotlb_batch_begin_sent) {
> +    if (!(s->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) {
>          return 0;
>      }
>
> -    if (vhost_vdpa_map_batch_begin(s, asid)) {
> -        s->iotlb_batch_begin_sent = true;
> +    if (s->iotlb_batch_begin_sent && s->iotlb_batch_asid != asid) {
> +        return -1;
> +    }
> +
> +    if (s->iotlb_batch_begin_sent) {
> +        return 0;
>      }
>
> +    if (!vhost_vdpa_map_batch_begin(s, asid)) {
> +        return 0;
> +    }
> +
> +    s->iotlb_batch_begin_sent = true;
> +    s->iotlb_batch_asid = asid;
> +
>      return 0;
>  }
>
> @@ -237,10 +247,13 @@ static int vhost_vdpa_dma_batch_end_once(VhostVDPAShared *s, uint32_t asid)
>          return 0;
>      }
>
> -    if (vhost_vdpa_dma_batch_end(s, asid)) {
> -        s->iotlb_batch_begin_sent = false;
> +    if (!vhost_vdpa_dma_batch_end(s, asid)) {
> +        return 0;
>      }
>
> +    s->iotlb_batch_begin_sent = false;
> +    s->iotlb_batch_asid = -1;
> +
>      return 0;
>  }
>
> diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
> index 0fe0f60..219316f 100644
> --- a/include/hw/virtio/vhost-vdpa.h
> +++ b/include/hw/virtio/vhost-vdpa.h
> @@ -61,6 +61,7 @@ typedef struct vhost_vdpa_shared {
>      bool map_thread_enabled;
>
>      bool iotlb_batch_begin_sent;
> +    uint32_t iotlb_batch_asid;
>
>      /*
>       * The memory listener has been registered, so DMA maps have been sent to
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index e9b96ed..bc72345 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1933,6 +1933,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
>          s->vhost_vdpa.shared->device_fd = vdpa_device_fd;
>          s->vhost_vdpa.shared->iova_range = iova_range;
>          s->vhost_vdpa.shared->shadow_data = svq;
> +        s->vhost_vdpa.shared->iotlb_batch_asid = -1;

This seems a trick, uAPI defines asid as:

        __u32 asid;

So technically -1U is a legal value.

Thanks

>          s->vhost_vdpa.shared->refcnt++;
>      } else if (!is_datapath) {
>          s->cvq_cmd_out_buffer = mmap(NULL, vhost_vdpa_net_cvq_cmd_page_len(),
> --
> 1.8.3.1
>
Re: [PATCH 28/40] vdpa: support iotlb_batch_asid
Posted by Eugenio Perez Martin 11 months, 2 weeks ago
On Thu, Dec 7, 2023 at 7:51 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> Then it's possible to specify ASID when calling the DMA
> batching API. If the ASID to work on doesn't align with
> the ASID for ongoing transaction, the API will fail the
> request and return negative, and the transaction will
> remain intact as if no failed request ever had occured.
>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
>  hw/virtio/vhost-vdpa.c         | 25 +++++++++++++++++++------
>  include/hw/virtio/vhost-vdpa.h |  1 +
>  net/vhost-vdpa.c               |  1 +
>  3 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index d3f5721..b7896a8 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -189,15 +189,25 @@ static bool vhost_vdpa_map_batch_begin(VhostVDPAShared *s, uint32_t asid)
>
>  static int vhost_vdpa_dma_batch_begin_once(VhostVDPAShared *s, uint32_t asid)
>  {
> -    if (!(s->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH)) ||
> -        s->iotlb_batch_begin_sent) {
> +    if (!(s->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) {
>          return 0;
>      }
>
> -    if (vhost_vdpa_map_batch_begin(s, asid)) {
> -        s->iotlb_batch_begin_sent = true;
> +    if (s->iotlb_batch_begin_sent && s->iotlb_batch_asid != asid) {
> +        return -1;
> +    }
> +
> +    if (s->iotlb_batch_begin_sent) {
> +        return 0;
>      }
>
> +    if (!vhost_vdpa_map_batch_begin(s, asid)) {
> +        return 0;
> +    }
> +
> +    s->iotlb_batch_begin_sent = true;
> +    s->iotlb_batch_asid = asid;
> +
>      return 0;
>  }
>
> @@ -237,10 +247,13 @@ static int vhost_vdpa_dma_batch_end_once(VhostVDPAShared *s, uint32_t asid)
>          return 0;
>      }
>
> -    if (vhost_vdpa_dma_batch_end(s, asid)) {
> -        s->iotlb_batch_begin_sent = false;
> +    if (!vhost_vdpa_dma_batch_end(s, asid)) {
> +        return 0;
>      }
>
> +    s->iotlb_batch_begin_sent = false;
> +    s->iotlb_batch_asid = -1;

If we define -1 as "not in batch", iotlb_batch_begin_sent is
redundant. Can we "#define IOTLB_NOT_IN_BATCH -1" and remove
iotlb_batch_begin_sent?

> +
>      return 0;
>  }
>
> diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
> index 0fe0f60..219316f 100644
> --- a/include/hw/virtio/vhost-vdpa.h
> +++ b/include/hw/virtio/vhost-vdpa.h
> @@ -61,6 +61,7 @@ typedef struct vhost_vdpa_shared {
>      bool map_thread_enabled;
>
>      bool iotlb_batch_begin_sent;
> +    uint32_t iotlb_batch_asid;
>
>      /*
>       * The memory listener has been registered, so DMA maps have been sent to
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index e9b96ed..bc72345 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1933,6 +1933,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
>          s->vhost_vdpa.shared->device_fd = vdpa_device_fd;
>          s->vhost_vdpa.shared->iova_range = iova_range;
>          s->vhost_vdpa.shared->shadow_data = svq;
> +        s->vhost_vdpa.shared->iotlb_batch_asid = -1;
>          s->vhost_vdpa.shared->refcnt++;
>      } else if (!is_datapath) {
>          s->cvq_cmd_out_buffer = mmap(NULL, vhost_vdpa_net_cvq_cmd_page_len(),
> --
> 1.8.3.1
>