Received-SPF: pass (google.com: domain of linux-kernel+bounces-125576-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Sat, 30 Mar 2024 11:55:33 +0900
From: Dominique Martinet <dominique.martinet@atmark-techno.com>
To: Michael Kelley <mhklinux@outlook.com>
Cc: "hch@lst.de" <hch@lst.de>,
	"m.szyprowski@samsung.com" <m.szyprowski@samsung.com>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>,
	"konrad.wilk@oracle.com" <konrad.wilk@oracle.com>,
	"bumyong.lee@samsung.com" <bumyong.lee@samsung.com>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"will@kernel.org" <will@kernel.org>,
	"petr@tesarici.cz" <petr@tesarici.cz>,
	"roberto.sassu@huaweicloud.com" <roberto.sassu@huaweicloud.com>,
	"lukas@mntmn.com" <lukas@mntmn.com>
Subject: Re: [PATCH 1/1] swiotlb: Fix swiotlb_bounce() to do partial sync's
 correctly
Message-ID: <Zgd_JaCHzOOLqWUM@atmark-techno.com>
References: <20240327034548.1959-1-mhklinux@outlook.com>
 <ZgO3HlYWo6qXaGs8@atmark-techno.com>
 <ZgZNAM337-UEY1DH@atmark-techno.com>
 <SN6PR02MB41574B5BC91B70AE74E51FE6D43A2@SN6PR02MB4157.namprd02.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <SN6PR02MB41574B5BC91B70AE74E51FE6D43A2@SN6PR02MB4157.namprd02.prod.outlook.com>

Michael Kelley wrote on Fri, Mar 29, 2024 at 03:18:16PM +0000:
> * tlb_offset = 1 - 3 = -2, as you describe above
> * orig_addr = 39 + -2 = 37.  The computation uses 39 from
> slot[1], not the 7 from slot[0].  This computed 37 is the
> correct orig_addr to use for the memcpy().

There are two things I don't understand here:
1/ Why orig_addr would come from slot[1] ?

We have index = (tlb_addr - mem->start) >> IO_TLB_SHIFT,
so index = (33 - 7) >> 5 = 26 >> 5 = 0

As such, orig_addr = mem->slots[0].orig_addr and we'd need the offset to
be 30, not -2 ?
Well, either work - if we fix index to point to the next slot in the
negative case that's also acceptable if we're sure it's valid, but I'm
worried it might not be in cases there was only one slot e.g. mapping
[7; 34] and calling with 33 size 2 would try to access slot 1 with a
negative offset in your example, but slot[0] is the last valid slot.


2/ Why is orig_addr 37 the correct address to use for memcpy, and not
33? I'd think it's off by a "minimum alignment page", for me this
computation only works if the dma_get_min_align size is bigger than io
tlb size.


> * size is still 4.  There's no computation in swiotlb_bounce()
> that changes "size".
> * alloc_size is pulled from slot[1], and is adjusted by tlb_offset.
> This adjusted alloc_size isn't used for anything except as a sanity
> check against "size".

Right, sorry - so size is ok (assuming slot[1] is used, I conflated the
two sizes.


I'm probably still missing something here, thanks for bearing with me.
-- 
Dominique