linux-kernel - Re: [PATCH v4] vfio/type1: optimize vfio_pin_pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250526033737.7657-1-lizhe.67@bytedance.com>
Date: Mon, 26 May 2025 11:37:37 +0800
From: lizhe.67@...edance.com
To: alex.williamson@...hat.com
Cc: david@...hat.com,
	kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	lizhe.67@...edance.com,
	muchun.song@...ux.dev,
	peterx@...hat.com
Subject: Re: [PATCH v4] vfio/type1: optimize vfio_pin_pages_remote() for large folio

On Fri, 23 May 2025 08:54:15 -0600, alex.williamson@...hat.com wrote: 

> > > +static long vpfn_pages(struct vfio_dma *dma,
> > > +		       dma_addr_t iova_start, long nr_pages)
> > > +{
> > > +	dma_addr_t iova_end = iova_start + (nr_pages << PAGE_SHIFT);
> > > +	struct vfio_pfn *vpfn;
> > > +	long count = 0;
> > > +
> > > +	do {
> > > +		vpfn = vfio_find_vpfn_range(dma, iova_start, iova_end);
> > 
> > I am somehow confused here. Function vfio_find_vpfn_range()is designed
> > to find, through the rbtree, the node that is closest to the root node
> > and satisfies the condition within the range [iova_start, iova_end),
> > rather than the node closest to iova_start? Or perhaps I have
> > misunderstood something?
> 
> Sorry, that's an oversight on my part.  We might forego the _range
> version and just do an inline walk of the tree counting the number of
> already accounted pfns within the range.  Thanks,
> 
> Alex
> 
> > > +		if (likely(!vpfn))
> > > +			break;
> > > +
> > > +		count++;
> > > +		iova_start = vpfn->iova + PAGE_SIZE;
> > > +	} while (iova_start < iova_end);
> > > +
> > > +	return count;
> > > +}

The utilization of the function vpfn_pages() is undoubtedly a
good idea. It can swiftly determine the num of vpfn pages
within a specified range, which will evidently expedite the
process of vfio_pin_pages_remote(). Given that the function
vfio_find_vpfn_range() returns the "top" node in the rb tree
that satisfies the condition within the range
[iova_start, iova_end), might we consider implementing the
functionality of vpfn_pages() using the following approach?

+static long _vpfn_pages(struct vfio_pfn *vpfn,
+               dma_addr_t iova_start, dma_addr_t iova_end)
+{
+       struct vfio_pfn *left;
+       struct vfio_pfn *right;
+
+       if (!vpfn)
+               return 0;
+
+       left = vpfn->node.rb_left ?
+               rb_entry(vpfn->node.rb_left, struct vfio_pfn, node) : NULL;
+       right = vpfn->node.rb_right ?
+               rb_entry(vpfn->node.rb_right, struct vfio_pfn, node) : NULL;
+
+       if ((vpfn->iova >= iova_start) && (vpfn->iova < iova_end))
+               return 1 + _vpfn_pages(left, iova_start, iova_end) +
+                               _vpfn_pages(right, iova_start, iova_end);
+
+       if (vpfn->iova >= iova_end)
+               return _vpfn_pages(left, iova_start, iova_end);
+
+       return _vpfn_pages(right, iova_start, iova_end);
+}
+
+static long vpfn_pages(struct vfio_dma *dma,
+               dma_addr_t iova_start, long nr_pages)
+{
+       dma_addr_t iova_end = iova_start + (nr_pages << PAGE_SHIFT);
+       struct vfio_pfn *top = vfio_find_vpfn_range(dma, iova_start, iova_end);
+
+       return _vpfn_pages(top, iova_start, iova_end);
+}

Thanks,
Zhe