RE: [PATCH] vhost: fix get hpa fail from guest pages

["Xia, Chenbo" <chenbo.xia@intel.com>]

> -----Original Message-----
> From: Wang, YuanX <yuanx.wang@intel.com>
> Sent: Thursday, November 11, 2021 2:27 PM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan
> <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; He, Xingguang
> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>; Wang, YuanX
> <yuanx.wang@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.wang@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?

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