Re: [PATCH 0/2] Fix double allocation in swiotlb_alloc()

[Will Deacon <will@kernel.org>]

On Mon, Jan 29, 2024 at 08:26:19PM +0100, Petr Tesařík wrote:
> On Mon, 29 Jan 2024 18:42:55 +0000
> Will Deacon <will@kernel.org> wrote:
> 
> > On Fri, Jan 26, 2024 at 05:20:59PM +0100, Petr Tesařík wrote:
> > > On Fri, 26 Jan 2024 15:19:54 +0000
> > > Will Deacon <will@kernel.org> wrote:
> > >   
> > > > Hi folks,
> > > > 
> > > > These two patches fix a nasty double allocation problem in swiotlb_alloc()
> > > > and add a diagnostic to help catch any similar issues in future. This was
> > > > a royal pain to track down and I've had to make a bit of a leap at the
> > > > correct alignment semantics (i.e. iotlb_align_mask vs alloc_align_mask).  
> > > 
> > > Welcome to the club. I believe you had to re-discover what I described here:
> > > 
> > >   https://lore.kernel.org/linux-iommu/20231108101347.77cab795@meshulam.tesarici.cz/  
> > 
> > Lucky me...
> > 
> > > The relevant part would be this:
> > > 
> > >   To sum it up, there are two types of alignment:
> > > 
> > >   1. specified by a device's min_align_mask; this says how many low
> > >      bits of a buffer's physical address must be preserved,
> > > 
> > >   2. specified by allocation size and/or the alignment parameter;
> > >      this says how many low bits in the first IO TLB slot's physical
> > >      address must be zero.  
> > > 
> > > Fix for that has been sitting on my TODO list for too long. :-(  
> > 
> > FWIW, it did _used_ to work (or appear to work), so it would be good to
> > at least get it back to the old behaviour if nothing else.
> 
> Yes, now that I look at the code, it was probably misunderstanding on
> my side as to how the three different alignment requirements are
> supposed to work together.
> 
> AFAICT your patch addresses everything that has ever worked. The rest
> needs some more thought, and before I touch this loop again, I'll write
> a proper test case.

Thanks, that would be much appreciated!

Will