Message-ID: <20251110113757.22b320b8.alex@shazbot.org>
Date: Mon, 10 Nov 2025 11:37:57 -0700
From: Alex Williamson <alex@...zbot.org>
To: David Matlack <dmatlack@...gle.com>
Cc: Alex Mastro <amastro@...com>, Alex Williamson
 <alex.williamson@...hat.com>, Jason Gunthorpe <jgg@...pe.ca>,
 kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vfio: selftests: Skip vfio_dma_map_limit_test if
 mapping returns -EINVAL

On Mon, 10 Nov 2025 18:00:08 +0000
David Matlack <dmatlack@...gle.com> wrote:

> On 2025-11-10 08:48 AM, Alex Mastro wrote:
> > On Mon, Nov 10, 2025 at 08:17:09AM -0700, Alex Williamson wrote:  
> > > On Sat, 8 Nov 2025 17:20:10 -0800
> > > Alex Mastro <amastro@...com> wrote:
> > >   
> > > > On Sat, Nov 08, 2025 at 02:37:10PM -0700, Alex Williamson wrote:  
> > > > > On Sat, 8 Nov 2025 12:19:48 -0800
> > > > > Alex Mastro <amastro@...com> wrote:    
> > > > > > Here's my attempt at adding some machinery to query iova ranges, with
> > > > > > normalization to iommufd's struct. I kept the vfio capability chain stuff
> > > > > > relatively generic so we can use it for other things in the future if needed.    
> > > > > 
> > > > > Seems we were both hacking on this, I hadn't seen you posted this
> > > > > before sending:
> > > > > 
> > > > > https://lore.kernel.org/kvm/20251108212954.26477-1-alex@shazbot.org/T/#u
> > > > > 
> > > > > Maybe we can combine the best merits of each.  Thanks,    
> > > > 
> > > > Yes! I have been thinking along the following lines
> > > > - Your idea to change the end of address space test to allocate at the end of
> > > >   the supported range is better and more general than my idea of skipping the
> > > >   test if ~(iova_t)0 is out of bounds. We should do that.
> > > > - Introducing the concept iova allocator makes sense.
> > > > - I think it's worthwhile to keep common test concepts like vfio_pci_device
> > > >   less opinionated/stateful so as not to close the door on certain categories of
> > > >   testing in the future. For example, if we ever wanted to test IOVA range
> > > >   contraction after binding additional devices to an IOAS or vfio container.  
> > > 
> > > Yes, fetching the IOVA ranges should really occur after all the devices
> > > are attached to the container/ioas rather than in device init.  We need
> > > another layer of abstraction for the shared IOMMU state.  We can
> > > probably work on that incrementally.  
> 
> I am working on pulling the iommu state out of struct vfio_pci_device
> here:
> 
>   https://lore.kernel.org/kvm/20251008232531.1152035-5-dmatlack@google.com/
> 
> But if we keep the iova allocator a separate object, then we can
> introduce it mosty indepently from this series. I imagine the only thing
> that will change is passing a struct iommu * instead of a struct
> vfio_pci_device * when initializing the allocator.
> 
> > > 
> > > I certainly like the idea of testing range contraction, but I don't
> > > know where we can reliably see that behavior.  
> > 
> > I'm not sure about the exact testing strategy for that yet either actually.
> >   
> > > > - What do you think about making the concept of an IOVA allocator something
> > > >   standalone for which tests that need it can create one? I think it would
> > > >   compose pretty cleanly on top of my vfio_pci_iova_ranges().  
> > > 
> > > Yep, that sounds good.  Obviously what's there is just the simplest
> > > possible linear, aligned allocator with no attempt to fill gaps or
> > > track allocations for freeing.  We're not likely to exhaust the address
> > > space in an individual unit test, I just wanted to relieve the test
> > > from the burden of coming up with a valid IOVA, while leaving some
> > > degree of geometry info for exploring the boundaries.  
> > 
> > Keeping the simple linear allocator makes sense to me.
> >   
> > > Are you interested in generating a combined v2?  
> > 
> > Sure -- I can put up a v2 series which stages like so
> > - adds stateless low level iova ranges queries
> > - adds iova allocator utility object
> > - fixes end of ranges tests, uses iova allocator instead of iova=vaddr   
> 
> +1 to getting rid of iova=vaddr.
> 
> But note that the HugeTLB tests in vfio_dma_mapping_test.c have
> alignment requirements to pass on Intel (since it validates the pages
> are mapped at the right level in the I/O page tables using the Intel
> debugfs interface).
> 
> > > TBH I'm not sure that just marking a test as skipped based on the DMA
> > > mapping return is worthwhile with a couple proposals to add IOVA range
> > > support already on the table.  Thanks,  
> > 
> > I'll put up the new series rooted on linux-vfio/next soon.  
> 
> I think we should try to get vfio_dma_mapping_test back to passing in
> time for Linux 6.18, since the newly failing test was added in 6.18.
> 
> The sequence I was imagining was:
> 
>  1. Merge the quick fix to skip the test into 6.18.

We'd still have the iova=vaddr failure on some platforms, but could
hack around that by hard coding some "well supporteD" IOVA like 0 or
4GB.

>  2. Split struct iommu from struct vfio_pci_device.
>  3. Add iova allocator.
> 
> AlexW, how much time do we have to get AlexM's series ready? I am fine
> with doing (3), then (2), and dropping (1) if there's enough time.

I'll certainly agree that it'd be a much better precedent if the self
test were initially working, but also we should not increase the scope
beyond what we need to make it work for v6.18.  If we can get that done
in the next day or two, add it to linux-next mid-week, and get Linus to
pull for rc6, I think that'd be reasonable.  Thanks,

Alex

Powered by blists - more mailing lists