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.
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
