Received-SPF: pass (google.com: domain of linux-kernel+bounces-106314-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: Michael Kelley <mhklinux@outlook.com>
To: 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>,
	Will Deacon <will@kernel.org>, open list <linux-kernel@vger.kernel.org>,
	"open list:DMA MAPPING HELPERS" <iommu@lists.linux.dev>
CC: Roberto Sassu <roberto.sassu@huaweicloud.com>, Petr Tesarik
	<petr@tesarici.cz>
Subject: RE: [PATCH v2 1/2] swiotlb: extend buffer pre-padding to
 alloc_align_mask if necessary
Thread-Topic: [PATCH v2 1/2] swiotlb: extend buffer pre-padding to
 alloc_align_mask if necessary
Thread-Index: AQHaeTTv740XxW4sG0CetwBVyXSzY7E9ngKA
Date: Mon, 18 Mar 2024 15:37:16 +0000
Message-ID:
 <SN6PR02MB41571EBB261FA3B5FFCD9BAFD42D2@SN6PR02MB4157.namprd02.prod.outlook.com>
References: <20240318130447.594-1-petrtesarik@huaweicloud.com>
 <20240318130447.594-2-petrtesarik@huaweicloud.com>
In-Reply-To: <20240318130447.594-2-petrtesarik@huaweicloud.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?CgaUYrcue1rnsW2uzNlS+jEXF9B36b3RC5WcTZdtk7dMY2p3tLnlap8BIvdq?=
 =?us-ascii?Q?0bdRIeUXgn5HFr24vzWT0HO/kZROyTZS5nAQ9x+aSKdWAeGpBYSg9UGy2qoC?=
 =?us-ascii?Q?pvqwimV/teIJWCXES8Hs8q1CACmUlmMHA+mnUkYg68uhEQkYtDQVMFS3mo9j?=
 =?us-ascii?Q?JcOge1+Apn9SAcYqr0linECKbVIumqjQP/vTyeH6DOkfeqbk8ccLotFBxPrs?=
 =?us-ascii?Q?XjWPXUGfZeKrDAfZ2TZkyVCIJdhiz9eAIUsJFkrmSh49mLaklCB4nALhDCsM?=
 =?us-ascii?Q?JfH/Tki6oYIHpW/zDgcNSVajqRjAVuDCUkI/5C3Y5DMGKyCz+T6Pe+HrFFl5?=
 =?us-ascii?Q?KjgCyUIng+oL/HnOUzQ+hDoBIVrrBdVv/eaJ3FSIH41prz+Iz8H4EiCwseCS?=
 =?us-ascii?Q?LD0rM8Ajhz+Y7AyX2F1hlC4THWYFtrfzVeiCu4CJVP7ndKX/z5XGmSNkBFDp?=
 =?us-ascii?Q?7aNmizcwlB90utTFuaR6M78xzzzl402NLZ1NLgllcSCI3IC1jHjinVIaJC+U?=
 =?us-ascii?Q?g4jKdRkyLcLNXgDsO039kvfHgFCHPd7B4e3mk5r5/NknMXB7E2cFgGZ2CLJE?=
 =?us-ascii?Q?WEEbY7TohhwRYKHHxe828xREwNCvEkNmm/BBOlMto2olxHcRwcNG2WLBv7RX?=
 =?us-ascii?Q?65X4RT5R0Bp1pnO8Zo7IljnoeEG7pbVd8mtvIVKRnuQF5X6SDkqCTkT9nvw4?=
 =?us-ascii?Q?rBC2UPxbA6/07AHgUJFvo33QeESM9D+poEUhLoYxgQVrRca5EjHSzO7rThSx?=
 =?us-ascii?Q?pfFv7hXqPH5R+G5pbj2FQ0tDdj4bO4rK51SQAbMXSdEh4ZkID3Jqx49lnYHx?=
 =?us-ascii?Q?AFXu0sdbIPcA2Kcnu/r/hkGZYYy7MLvZAmWzzIH28vXWpB+nd6BnSA/eP0ln?=
 =?us-ascii?Q?SnfFsqCXZp/DBy8ys5+1gfwxmkZ/yLHOqkoeBeb2JI1NBus0vP+9bduWdsPR?=
 =?us-ascii?Q?cTDR21SPHejvCNDKmkmFmug1/BGrOUsaGPritxcmzRDDBRaCR3qOH29IC0YO?=
 =?us-ascii?Q?8FNajnCvUSqzf4o0aph5Ig8LhK0xaK2wRXFhm9jhcpll3XQWlKGTns8OHoWR?=
 =?us-ascii?Q?7XfO3ukbUyJCpZ0dwDpdP70F7z/vSHlPcquC0+OD5u3grlUs32QiEMG8woQI?=
 =?us-ascii?Q?Swl3EjrXAfw3EX5YVkGqRuRc6Krv/cvqxZKkWWWnWPIjcbrXwGU4Wfl40HJz?=
 =?us-ascii?Q?s5yuxe05wljpOxX5gGXKo9QLkWvN8XIZrir0aQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR02MB4157.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: 31080dff-a0ee-4b56-c68a-08dc47614a42
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Mar 2024 15:37:16.2845
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR02MB8906

From: Petr Tesarik <petrtesarik@huaweicloud.com> Sent: Monday, March 18, 20=
24 6:05 AM
>=20
> Allow a buffer pre-padding of up to alloc_align_mask. If the allocation
> alignment is bigger than IO_TLB_SIZE and min_align_mask covers any non-ze=
ro
> bits in the original address between IO_TLB_SIZE and alloc_align_mask,
> these bits are not preserved in the swiotlb buffer address.
>=20
> To fix this case, increase the allocation size and use a larger offset
> within the allocated buffer. As a result, extra padding slots may be
> allocated before the mapping start address.
>=20
> Set the orig_addr in these padding slots to INVALID_PHYS_ADDR, because th=
ey
> do not correspond to any CPU buffer and the data must never be synced.

This paragraph is now obsolete.

>=20
> The padding slots should be automatically released when the buffer is
> unmapped. However, swiotlb_tbl_unmap_single() takes only the address of t=
he
> DMA buffer slot, not the first padding slot. Save the number of padding
> slots in struct io_tlb_slot and use it to adjust the slot index in
> swiotlb_release_slots(), so all allocated slots are properly freed.
>=20
> Fixes: 2fd4fa5d3fb5 ("swiotlb: Fix alignment checks when both allocation =
and DMA masks are present")
> Link: https://lore.kernel.org/linux-iommu/20240311210507.217daf8b@meshula=
m.tesarici.cz/
> Signed-off-by: Petr Tesarik <petr.tesarik1@huawei-partners.com>
> ---
>  kernel/dma/swiotlb.c | 35 +++++++++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 6 deletions(-)
>=20
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 86fe172b5958..aefb05ff55e7 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -69,11 +69,14 @@
>   * @alloc_size:	Size of the allocated buffer.
>   * @list:	The free list describing the number of free entries available
>   *		from each index.
> + * @pad_slots:	Number of preceding padding slots. Valid only in the firs=
t
> + *		allocated non-padding slot.
>   */
>  struct io_tlb_slot {
>  	phys_addr_t orig_addr;
>  	size_t alloc_size;
> -	unsigned int list;
> +	unsigned short list;
> +	unsigned short pad_slots;
>  };
>=20
>  static bool swiotlb_force_bounce;
> @@ -287,6 +290,7 @@ static void swiotlb_init_io_tlb_pool(struct io_tlb_po=
ol *mem, phys_addr_t start,
>  					 mem->nslabs - i);
>  		mem->slots[i].orig_addr =3D INVALID_PHYS_ADDR;
>  		mem->slots[i].alloc_size =3D 0;
> +		mem->slots[i].pad_slots =3D 0;
>  	}
>=20
>  	memset(vaddr, 0, bytes);
> @@ -1328,11 +1332,12 @@ phys_addr_t swiotlb_tbl_map_single(struct device =
*dev, phys_addr_t orig_addr,
>  		unsigned long attrs)
>  {
>  	struct io_tlb_mem *mem =3D dev->dma_io_tlb_mem;
> -	unsigned int offset =3D swiotlb_align_offset(dev, orig_addr);
> +	unsigned int offset;
>  	struct io_tlb_pool *pool;
>  	unsigned int i;
>  	int index;
>  	phys_addr_t tlb_addr;
> +	unsigned short pad_slots;
>=20
>  	if (!mem || !mem->nslabs) {
>  		dev_warn_ratelimited(dev,
> @@ -1349,6 +1354,15 @@ phys_addr_t swiotlb_tbl_map_single(struct device *=
dev, phys_addr_t orig_addr,
>  		return (phys_addr_t)DMA_MAPPING_ERROR;
>  	}
>=20
> +	/*
> +	 * Calculate buffer pre-padding within the allocated space. Use it to
> +	 * preserve the low bits of the original address according to device's
> +	 * min_align_mask. Limit the padding to alloc_align_mask or slot size
> +	 * (whichever is bigger); higher bits of the original address are
> +	 * preserved by selecting a suitable IO TLB slot.
> +	 */
> +	offset =3D orig_addr & dma_get_min_align_mask(dev) &
> +		(alloc_align_mask | (IO_TLB_SIZE - 1));
>  	index =3D swiotlb_find_slots(dev, orig_addr,
>  				   alloc_size + offset, alloc_align_mask, &pool);
>  	if (index =3D=3D -1) {
> @@ -1364,6 +1378,10 @@ phys_addr_t swiotlb_tbl_map_single(struct device *=
dev, phys_addr_t orig_addr,
>  	 * This is needed when we sync the memory.  Then we sync the buffer if
>  	 * needed.
>  	 */
> +	pad_slots =3D offset / IO_TLB_SIZE;
> +	offset %=3D IO_TLB_SIZE;
> +	index +=3D pad_slots;
> +	pool->slots[index].pad_slots =3D i;

The above line should be:
	pool->slots[index].pad_slots =3D pad_slots;

>  	for (i =3D 0; i < nr_slots(alloc_size + offset); i++)
>  		pool->slots[index + i].orig_addr =3D slot_addr(orig_addr, i);
>  	tlb_addr =3D slot_addr(pool->start, index) + offset;
> @@ -1385,12 +1403,16 @@ static void swiotlb_release_slots(struct device *=
dev, phys_addr_t tlb_addr)
>  	struct io_tlb_pool *mem =3D swiotlb_find_pool(dev, tlb_addr);
>  	unsigned long flags;
>  	unsigned int offset =3D swiotlb_align_offset(dev, tlb_addr);
> -	int index =3D (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
> -	int nslots =3D nr_slots(mem->slots[index].alloc_size + offset);
> -	int aindex =3D index / mem->area_nslabs;
> -	struct io_tlb_area *area =3D &mem->areas[aindex];
> +	int index, nslots, aindex;
> +	struct io_tlb_area *area;
>  	int count, i;
>=20
> +	index =3D (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
> +	index -=3D mem->slots[index].pad_slots;
> +	nslots =3D nr_slots(mem->slots[index].alloc_size + offset);
> +	aindex =3D index / mem->area_nslabs;
> +	area =3D &mem->areas[aindex];
> +
>  	/*
>  	 * Return the buffer to the free list by setting the corresponding
>  	 * entries to indicate the number of contiguous entries available.
> @@ -1413,6 +1435,7 @@ static void swiotlb_release_slots(struct device *de=
v, phys_addr_t tlb_addr)
>  		mem->slots[i].list =3D ++count;
>  		mem->slots[i].orig_addr =3D INVALID_PHYS_ADDR;
>  		mem->slots[i].alloc_size =3D 0;
> +		mem->slots[i].pad_slots =3D 0;
>  	}
>=20
>  	/*
> --
> 2.34.1