Hi,

On 11/16/21 09:47, Ding, Xuan wrote:
Hi Chenbo,

-----Original Message-----
From: Xia, Chenbo <chenbo....@intel.com>
Sent: 2021年11月16日 16:07
To: Xia, Chenbo <chenbo....@intel.com>; Wang, YuanX
<yuanx.w...@intel.com>; maxime.coque...@redhat.com
Cc: dev@dpdk.org; Hu, Jiayu <jiayu...@intel.com>; Ding, Xuan
<xuan.d...@intel.com>; Ma, WenwuX <wenwux...@intel.com>; He,
Xingguang <xingguang...@intel.com>; Yang, YvonneX
<yvonnex.y...@intel.com>
Subject: RE: [PATCH] vhost: fix get hpa fail from guest pages

-----Original Message-----
From: Xia, Chenbo <chenbo....@intel.com>
Sent: Monday, November 15, 2021 4:04 PM
To: Wang, YuanX <yuanx.w...@intel.com>; maxime.coque...@redhat.com
Cc: dev@dpdk.org; Hu, Jiayu <jiayu...@intel.com>; Ding, Xuan
<xuan.d...@intel.com>; Ma, WenwuX <wenwux...@intel.com>; He,
Xingguang
<xingguang...@intel.com>; Yang, YvonneX <yvonnex.y...@intel.com>
Subject: RE: [PATCH] vhost: fix get hpa fail from guest pages

-----Original Message-----
From: Wang, YuanX <yuanx.w...@intel.com>
Sent: Thursday, November 11, 2021 2:27 PM
To: maxime.coque...@redhat.com; Xia, Chenbo <chenbo....@intel.com>
Cc: dev@dpdk.org; Hu, Jiayu <jiayu...@intel.com>; Ding, Xuan
<xuan.d...@intel.com>; Ma, WenwuX <wenwux...@intel.com>; He,
Xingguang <xingguang...@intel.com>; Yang, YvonneX
<yvonnex.y...@intel.com>; Wang,
YuanX
<yuanx.w...@intel.com>
Subject: [PATCH] vhost: fix get hpa fail from guest pages

When processing front-end memory regions messages, vhost saves the
guest/host physical address mappings to guest pages and merges
adjacent contiguous pages if hpa is contiguous, however gpa is
likely not contiguous in PA mode and merging will cause the gpa
range to change.
This patch distinguishes the case of discontinuous gpa and does a
range lookup on gpa when doing a binary search.

Fixes: e246896178e("vhost: get guest/host physical address
mappings")
Fixes: 6563cf92380 ("vhost: fix async copy on multi-page buffers")

Signed-off-by: Yuan Wang <yuanx.w...@intel.com>
---
  lib/vhost/vhost.h      | 18 ++++++++++++++++--
  lib/vhost/vhost_user.c | 15 +++++++++++----
  2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index
7085e0885c..b3f0c1d07c 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -587,6 +587,20 @@ static __rte_always_inline int
guest_page_addrcmp(const void *p1,
        return 0;
  }

+static __rte_always_inline int guest_page_rangecmp(const void *p1,
+const
void
*p2)
+{
+       const struct guest_page *page1 = (const struct guest_page *)p1;
+       const struct guest_page *page2 = (const struct guest_page *)p2;
+
+       if (page1->guest_phys_addr >= page2->guest_phys_addr) {
+               if (page1->guest_phys_addr < page2->guest_phys_addr +
page2->size)
+                       return 0;
+               else
+                       return 1;
+       } else
+               return -1;
+}
+
  static __rte_always_inline rte_iova_t  gpa_to_first_hpa(struct
virtio_net *dev, uint64_t gpa,
        uint64_t gpa_size, uint64_t *hpa_size) @@ -597,9 +611,9 @@
gpa_to_first_hpa(struct virtio_net *dev, uint64_t gpa,

        *hpa_size = gpa_size;
        if (dev->nr_guest_pages >= VHOST_BINARY_SEARCH_THRESH) {
-               key.guest_phys_addr = gpa & ~(dev->guest_pages[0].size - 1);
+               key.guest_phys_addr = gpa;
                page = bsearch(&key, dev->guest_pages, dev->nr_guest_pages,
-                              sizeof(struct guest_page), guest_page_addrcmp);
+                              sizeof(struct guest_page), guest_page_rangecmp);
                if (page) {
                        if (gpa + gpa_size <=
                                        page->guest_phys_addr + page->size)
{ diff --git
a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index
a781346c4d..7d58fde458 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -999,10 +999,17 @@ add_one_guest_page(struct virtio_net *dev,
uint64_t guest_phys_addr,
        if (dev->nr_guest_pages > 0) {
                last_page = &dev->guest_pages[dev->nr_guest_pages - 1];
                /* merge if the two pages are continuous */
-               if (host_phys_addr == last_page->host_phys_addr +
-                                     last_page->size) {
-                       last_page->size += size;
-                       return 0;
+               if (host_phys_addr == last_page->host_phys_addr +
+last_page->size)
{
+                       if (rte_eal_iova_mode() == RTE_IOVA_VA) {
+                               last_page->size += size;
+                               return 0;
+                       }

This makes me think about a question: In IOVA_VA mode, what ensures
HPA and GPA are both contiguous?

When I wrote this email, I thought host_phys_addr is HPA but in VA mode, it's
Host IOVA, aka VA in DPDK's case. So in most cases GPA and VA will be both
contiguous when the contiguous pages are all in one memory region. But I think
we should not make such assumption as some vhost master may send GPA-
contiguous pages in different memory region, then the VA after mmap may not
be contiguous.

Now we do async vfio mapping at page granularity.
For 4K/2M pages, without this assumption, pages not be merged may
exceed the IOMMU's capability. This limits only use 1G hugepage for DMA device.

I'm sorry, but I'm not sure to understand your explanation.
Could you please rephrase it?

Thanks,
Maxime

Thanks,
Xuan


Maxime, What do you think?

Thanks,
Chenbo


Maxime & Yuan, any thought?

Thanks,
Chenbo

+
+                       if (rte_eal_iova_mode() == RTE_IOVA_PA &&
+                               guest_phys_addr == last_page-
guest_phys_addr +
last_page->size) {
+                               last_page->size += size;
+                               return 0;
+                       }
                }
        }

--
2.25.1


Reply via email to