linux-kernel - Re: [PATCH v3 0/2] swiotlb: allocate padding slots if necessary

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]

Date: Fri, 22 Mar 2024 20:31:31 +0100
From: Petr Tesařík <petr@...arici.cz>
To: Michael Kelley <mhklinux@...look.com>
Cc: Will Deacon <will@...nel.org>, Petr Tesarik
 <petrtesarik@...weicloud.com>, Christoph Hellwig <hch@....de>, Marek
 Szyprowski <m.szyprowski@...sung.com>, Robin Murphy <robin.murphy@....com>,
 Petr Tesarik <petr.tesarik1@...wei-partners.com>, open list
 <linux-kernel@...r.kernel.org>, "open list:DMA MAPPING HELPERS"
 <iommu@...ts.linux.dev>, Roberto Sassu <roberto.sassu@...weicloud.com>
Subject: Re: [PATCH v3 0/2] swiotlb: allocate padding slots if necessary

On Fri, 22 Mar 2024 18:11:50 +0000
Michael Kelley <mhklinux@...look.com> wrote:

> From: Will Deacon <will@...nel.org> Sent: Friday, March 22, 2024 8:10 AM
> > 
> > Hi Petr,
> > 
> > On Thu, Mar 21, 2024 at 06:19:00PM +0100, Petr Tesarik wrote:  
> > > From: Petr Tesarik <petr.tesarik1@...wei-partners.com>
> > >
> > > If the allocation alignment is bigger than IO_TLB_SIZE and min_align_mask
> > > covers some bits in the original address between IO_TLB_SIZE and
> > > alloc_align_mask, preserve these bits by allocating additional padding
> > > slots before the actual swiotlb buffer.  
> > 
> > Thanks for fixing this! I was out at a conference last week, so I didn't
> > get very far with it myself, but I ended up in a pickle trying to avoid
> > extending 'struct io_tlb_slot'. Your solution is much better than the
> > crazy avenue I started going down...
> > 
> > With your changes, can we now simplify swiotlb_align_offset() to ignore
> > dma_get_min_align_mask() altogether and just:
> > 
> > 	return addr & (IO_TLB_SIZE - 1);
> > 
> > ?
> >   
> 
> I don't think such a change is correct, since we want to allow the
> DMA min_align_mask to work if it is set to 0x3FF or 0x1FF or
> something else smaller than IO_TLB_SIZE - 1.
> 
> Petr's new offset calculation in swiotlb_tbl_map_single() is this:
> 
> offset = orig_addr & dma_get_min_align_mask(dev) &
>                 (alloc_align_mask | (IO_TLB_SIZE - 1));
> 
> In the normal stream mapping case, where alloc_align_mask is
> zero, Petr's new statement is equivalent to swiotlb_align_offset().
> And I think it needs to continue to be equivalent so that
> swiotlb_search_pool_area(), swiotlb_bounce()  and
> swiotlb_release_slots() calculate the same offset as
> swiotlb_tbl_map_single() uses after it separately processes
> the padding slots.
> 
> Perhaps a better approach to maintaining the equivalence is
> to modify swiotlb_align_offset() to be Petr's new calculation,
> with alloc_align_mask passed as an argument.
> swiotlb_search_pool_area(), swiotlb_bounce(), and
> swiotlb_release_slots() would all pass 0 as the alloc_align_mask
> argument.

I like this idea.

Petr T