Received-SPF: pass (google.com: domain of linux-kernel+bounces-111998-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Fri, 22 Mar 2024 20:31:31 +0100
From: Petr =?UTF-8?B?VGVzYcWZw61r?= <petr@tesarici.cz>
To: Michael Kelley <mhklinux@outlook.com>
Cc: Will Deacon <will@kernel.org>, Petr Tesarik
 <petrtesarik@huaweicloud.com>, Christoph Hellwig <hch@lst.de>, Marek
 Szyprowski <m.szyprowski@samsung.com>, Robin Murphy <robin.murphy@arm.com>,
 Petr Tesarik <petr.tesarik1@huawei-partners.com>, open list
 <linux-kernel@vger.kernel.org>, "open list:DMA MAPPING HELPERS"
 <iommu@lists.linux.dev>, Roberto Sassu <roberto.sassu@huaweicloud.com>
Subject: Re: [PATCH v3 0/2] swiotlb: allocate padding slots if necessary
Message-ID: <20240322203131.207519c5@meshulam.tesarici.cz>
In-Reply-To: <SN6PR02MB41574F4837E875F3D158B329D4312@SN6PR02MB4157.namprd02.prod.outlook.com>
References: <20240321171902.85-1-petrtesarik@huaweicloud.com>
	<20240322150941.GA5634@willie-the-truck>
	<SN6PR02MB41574F4837E875F3D158B329D4312@SN6PR02MB4157.namprd02.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

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

> From: Will Deacon <will@kernel.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@huawei-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