Re: [REGRESSION] vhost_iotlb: bound map allocation in add_range breaks vhost-vdpa on hosts with fragmented memory

[Dragos Tatulea <dtatulea@nvidia.com>]

On 28.10.26 16:17, Michael S. Tsirkin wrote:
> On Wed, Oct 28, 2026 at 03:46:05PM +0100, Dragos Tatulea wrote:
>> On 28.10.26 14:05, Michael S. Tsirkin wrote:
>>> On Wed, Oct 28, 2026 at 01:44:30PM +0100, Dragos Tatulea wrote:
>>>> Hi,
>>>>
>>>> We've started seeing regressions for a while in our verification CI for
>>>> vDPA Live Migration tests. Occasionally the FW returns a syndrome
>>>> related to invalid mappings during device reset.
>>>>
>>>> We've narrowed this down to commit 1ed35ac7f3fe "vhost_iotlb: bound map
>>>> allocation in add_range" which limits the iotlbs to 2K by default. On a
>>>> host with fragmented memory you can get easily more than 2K entries.
>>>> This results in truncated/incomplete iotlbs mapped to the device and when
>>>> the rings happen to be outside of the first 2K mappings the FW will complain
>>>> because it got addresses that are not mapped.
>>>>
>>>> I'm not sure how to fix this though:
>>>>
>>>> - LLM suggests setting the 2K limit only in VA mode and using 0 as
>>>>   before in PA mode because PA mode is limited anyway. Does this
>>>>   work?
>>>
>>> Well no, we do need the limit in PA mode I think.
>>>
>>> But we should maybe  bound from what's actually being mapped rather than
>>> a fixed count. e.g. X entries per pinned page?
>> You could have many 1 PAGE_SIZE mappings though. It wouldn't help
>> then.
> 
> it still mostly is 1 entry=1 pinned page
> 
> sharing is possible but uncommon
> 
>>> Bounding by bytes instead of entry count is another option.How to get this limit from userspace though? Would RLIMIT_MEMLOCK
>> be an option? Although `ulimit -l unlimited` would need to be
>> capped to some value...
> 
> guest ram no?
> 
>>
>> Also, if the entry limit is still there some kind of precedence between
>> memory and entry limit needs to be decided.
> 
> I'd steal a page from rlimit then use that for the entries, when out of
> that - steal another page.
> 
Do you mean something like this [1]?

Remaining questions:
1) Is it ok to delete max_iotlb_entries? Or keep it initialized to max?
2) Is it ok to delete the module specific max_iotlb_entries param? For
  example mlx5_vdpa_max_iotlb_entries. I don't see much use for it ...
3) Shouldn't the core or the driver track holes encounter due to
  incomplete mappings? It would be good to not give bad addresses
  to the hw/fw...

Thanks,
Dragos

[1]:
---
 drivers/vhost/vdpa.c | 72 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 54 insertions(+), 18 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index a4d0ce3dac04..224f950a22bd 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -34,19 +34,18 @@ enum {
 
 #define VHOST_VDPA_DEV_MAX (1U << MINORBITS)
 
-static int max_iotlb_entries = 2048;
-module_param(max_iotlb_entries, int, 0444);
-MODULE_PARM_DESC(max_iotlb_entries,
-		 "Maximum number of iotlb entries. (default: 2048)");
-
 #define VHOST_VDPA_IOTLB_BUCKETS 16
 
 struct vhost_vdpa_as {
 	struct hlist_node hash_link;
 	struct vhost_iotlb iotlb;
+	/* pages charged to RLIMIT_MEMLOCK for the IOTLB entries */
+	unsigned long charged_pages;
 	u32 id;
 };

 struct vhost_vdpa {
 	struct vhost_dev vdev;
 	struct iommu_domain *domain;
@@ -75,11 +74,41 @@ static void vhost_vdpa_iotlb_unmap(struct vhost_vdpa *v,
 				   struct vhost_iotlb *iotlb, u64 start,
 				   u64 last, u32 asid);
 
+static inline struct vhost_vdpa_as *iotlb_to_as(struct vhost_iotlb *iotlb)
+{
+	return container_of(iotlb, struct vhost_vdpa_as, iotlb);
+}
+
 static inline u32 iotlb_to_asid(struct vhost_iotlb *iotlb)
 {
-	struct vhost_vdpa_as *as = container_of(iotlb, struct
-						vhost_vdpa_as, iotlb);
-	return as->id;
+	return iotlb_to_as(iotlb)->id;
+}
+
+static int vhost_vdpa_charge_maps(struct vhost_vdpa *v,
+				  struct vhost_vdpa_as *as,
+				  unsigned int nmaps)
+{
+	unsigned long pages = DIV_ROUND_UP(nmaps, (PAGE_SIZE / sizeof(struct vhost_iotlb_map)));
+	struct mm_struct *mm = v->vdev.mm;
+	unsigned long lock_limit;
+
+	if (pages > as->charged_pages) {
+		lock_limit = PFN_DOWN(rlimit(RLIMIT_MEMLOCK));
+		if (pages - as->charged_pages +
+		    atomic64_read(&mm->pinned_vm) > lock_limit)
+			return -ENOMEM;
+		atomic64_add(pages - as->charged_pages, &mm->pinned_vm);
+	} else {
+		atomic64_sub(as->charged_pages - pages, &mm->pinned_vm);
+	}
+	as->charged_pages = pages;
+
+	return 0;
 }
 
 static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 asid)
@@ -114,14 +143,11 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
 
 	if (asid >= v->vdpa->nas)
 		return NULL;
-	if (max_iotlb_entries <= 0)
-		return NULL;
-
 	as = kmalloc_obj(*as);
 	if (!as)
 		return NULL;
 
-	vhost_iotlb_init(&as->iotlb, max_iotlb_entries, 0);
+	vhost_iotlb_init(&as->iotlb, UINT_MAX, 0);
 	as->id = asid;
 	hlist_add_head(&as->hash_link, head);
 
@@ -978,9 +1004,11 @@ static void vhost_vdpa_iotlb_unmap(struct vhost_vdpa *v,
 	struct vdpa_device *vdpa = v->vdpa;
 
 	if (vdpa->use_va)
-		return vhost_vdpa_va_unmap(v, iotlb, start, last, asid);
+		vhost_vdpa_va_unmap(v, iotlb, start, last, asid);
+	else
+		vhost_vdpa_pa_unmap(v, iotlb, start, last, asid);
 
-	return vhost_vdpa_pa_unmap(v, iotlb, start, last, asid);
+	vhost_vdpa_charge_maps(v, iotlb_to_as(iotlb), iotlb->nmaps);
 }
 
 static int perm_to_iommu_flags(u32 perm)
@@ -1011,13 +1039,18 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
 	struct vhost_dev *dev = &v->vdev;
 	struct vdpa_device *vdpa = v->vdpa;
 	const struct vdpa_config_ops *ops = vdpa->config;
-	u32 asid = iotlb_to_asid(iotlb);
+	struct vhost_vdpa_as *as = iotlb_to_as(iotlb);
+	u32 asid = as->id;
 	int r = 0;
 
+	r = vhost_vdpa_charge_maps(v, as, iotlb->nmaps + 1);
+	if (r)
+		return r;
+
 	r = vhost_iotlb_add_range_ctx(iotlb, iova, iova + size - 1,
 				      pa, perm, opaque);
 	if (r)
-		return r;
+		goto uncharge;
 
 	if (ops->dma_map) {
 		r = ops->dma_map(vdpa, asid, iova, size, pa, perm, opaque);
@@ -1031,13 +1064,16 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
 	}
 	if (r) {
 		vhost_iotlb_del_range(iotlb, iova, iova + size - 1);
-		return r;
+		goto uncharge;
 	}
 
 	if (!vdpa->use_va)
 		atomic64_add(PFN_DOWN(size), &dev->mm->pinned_vm);
 
-	return 0;
+uncharge:
+	vhost_vdpa_charge_maps(v, as, iotlb->nmaps);
+	return r;
 }
 
 static void vhost_vdpa_unmap(struct vhost_vdpa *v,
-- 
2.55.0