Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Mi, Dapeng1" <dapeng1.mi@intel.com>
To:     David Stevens <stevensd@chromium.org>,
        Robin Murphy <robin.murphy@arm.com>,
        Will Deacon <will@kernel.org>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Tom Murphy <murphyt7@tcd.ie>,
        "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>
Subject: RE: [PATCH v3 5/5] dma-iommu: account for min_align_mask
Thread-Topic: [PATCH v3 5/5] dma-iommu: account for min_align_mask
Thread-Index: AQHXjlrYm0R+OxTpa0yaHYd4ePRmHatuBj8A
Date:   Wed, 11 Aug 2021 09:26:02 +0000
Message-ID: <PH0PR11MB48249799E5C66534F9C37D8BCDF89@PH0PR11MB4824.namprd11.prod.outlook.com>
References: <20210811024247.1144246-1-stevensd@google.com>
 <20210811024247.1144246-6-stevensd@google.com>
In-Reply-To: <20210811024247.1144246-6-stevensd@google.com>
Accept-Language: en-US
Content-Language: en-US
dlp-reaction: no-action
dlp-version: 11.5.1.3
dlp-product: dlpe-windows
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?06mi3O1SkTnNYmjpqGzbyZXIq1MDJ3aSeyCmjYvHli1GkLyM4bwCWDTFoKBM?=
 =?us-ascii?Q?ueErWltGxMA5rtS87tpRBBLCNJmppcS437Bbs9J5r418rBktm9TPXbN84dtB?=
 =?us-ascii?Q?NZTcEM0SAdPx/G4s2MltcvkN9Y+7nv41VvDfc1FIxJ47aMm15VUjAxtdTLHH?=
 =?us-ascii?Q?JjOgb06cryn2w92f42tzpLjPQFRTM1fnsOZXtRux7F8EN8zbk6rpkou8Io7P?=
 =?us-ascii?Q?wG0Xm0wafJV5Obln5jJFL2hq6xwAH+JaZftNGHZn8IRYuxnTbM3fj0QWKrUN?=
 =?us-ascii?Q?o6HqSd4BurpB6tzRn9IeYgLWrLD1kn0Xdjep+iEzDfBMhXAg52DcvwKwrfc9?=
 =?us-ascii?Q?L6YBsdoKyBvgI1u8Iwwjqw6L3ZOIrKZaJcJXK7eFoAjdpRaLoLMRUtRV5akQ?=
 =?us-ascii?Q?I/Kwq6lxZb+AL5r9phVPQ/6FsL8nt5Xg8oAnkGNtUyBAgopvmArgtcTVK0k2?=
 =?us-ascii?Q?riEw4IBTv4GpczmtGVU74Qc1ZWfnWnscDW7V5hVYVfaYPCyEcvKTdmVpEUHU?=
 =?us-ascii?Q?DxCC0UagnHExWCYoBPG77u7LvOmKgBYrDnFnd6VdPEwcpKtVvBzRbZRGDJar?=
 =?us-ascii?Q?MJN+dp4NZ/itewFteV3iPKLnh9Tp12gvrhoz6yVoTTJAhk/vSPgrcm9v9hhC?=
 =?us-ascii?Q?RQQbKY0bLHVfPuyMXGO2nDN96o6Bq7MGrB2cvOprluQT+4JVHv3IxtSUACPe?=
 =?us-ascii?Q?phci3ypK56VYP9VwV9L5vu/pgBPLqot1bhG8/1xep3WT7jQC05Aw5kou46lz?=
 =?us-ascii?Q?cAos5+ST5GmmmM3Iv33X/rvOoP+mfuvv37lNhv7/TGSu/neeOZDqB+NdvgoD?=
 =?us-ascii?Q?luGXuiyBlWL/zE2nLLcpgtJuO8Rb66c1+ECbVP9M+vocyJukMfQl+0ip3/l7?=
 =?us-ascii?Q?RLz1C6/xSdpdrTZNP85yMeErsThhGTCCp8S7/J+5Wvxdagwa1PrTO2Ya7w85?=
 =?us-ascii?Q?x8+cKeYTnWIm9qWvjgz5IvA1iu06oqaq54feFM7eN1LoqCNCrMm4hNcHWqED?=
 =?us-ascii?Q?2ELDn/vxlEGHF91sKhwvYZ8ujcVNh12XxnmFAMuZmk7ORES0xGRuYfjmGywK?=
 =?us-ascii?Q?Mhl1afLB/dJSX3+QU8SbQnlXC17OmrCJZcQp47FnJyZvdi6qPVJ8+1eIkmCC?=
 =?us-ascii?Q?nfDgjqPMfgLa9pPk6FoC3o+PMi4QEjAOOjXj7Zfm4XnwrmQJlMg1RMm8P8m4?=
 =?us-ascii?Q?PA3jBHp6PT5OfwgLP1Z5tT9ZoWU5kn7E76SZE+K3lQXxi+oy3srgr5PoyaEO?=
 =?us-ascii?Q?zs6lbMQ05RPYTTHy3KUQhlSGxuVt/JS6jxKPc1Z2qBFwtmGMcYvh7OOa4RJn?=
 =?us-ascii?Q?g+a28ToelK5gjsyPLEzFUGHW?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4824.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e8c631df-7eb1-4070-3abc-08d95caa09fd
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Aug 2021 09:26:02.9988
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: yqWcG0VQO5lv5xOw7XQQnPq5+yHKicoVc4Rt8c9tVfqIaIDWeBSIWk4dcwGCnnzDc9t24HfIYWAKOAlgrgLWXg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5128
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: iommu <iommu-bounces@lists.linux-foundation.org> On Behalf Of
> David Stevens
> Sent: Wednesday, August 11, 2021 10:43 AM
> To: Robin Murphy <robin.murphy@arm.com>; Will Deacon <will@kernel.org>
> Cc: linux-kernel@vger.kernel.org; Tom Murphy <murphyt7@tcd.ie>;
> iommu@lists.linux-foundation.org; David Stevens <stevensd@chromium.org>
> Subject: [PATCH v3 5/5] dma-iommu: account for min_align_mask
>=20
> From: David Stevens <stevensd@chromium.org>
>=20
> For devices which set min_align_mask, swiotlb preserves the offset of the
> original physical address within that mask. Since __iommu_dma_map
> accounts for non-aligned addresses, passing a non-aligned swiotlb address
> with the swiotlb aligned size results in the offset being accounted for t=
wice in
> the size passed to iommu_map_atomic. The extra page exposed to DMA is
> also not cleaned up by __iommu_dma_unmap, since tht at function unmaps
> with the correct size. This causes mapping failures if the iova gets reus=
ed,
> due to collisions in the iommu page tables.
>=20
> To fix this, pass the original size to __iommu_dma_map, since that functi=
on
> already handles alignment.
>=20
> Additionally, when swiotlb returns non-aligned addresses, there is paddin=
g at
> the start of the bounce buffer that needs to be cleared.
>=20
> Fixes: 1f221a0d0dbf ("swiotlb: respect min_align_mask")
> Signed-off-by: David Stevens <stevensd@chromium.org>
> ---
>  drivers/iommu/dma-iommu.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
>=20
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 89b689bf801f..ffa7e8ef5db4 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -549,9 +549,8 @@ static dma_addr_t
> __iommu_dma_map_swiotlb(struct device *dev, phys_addr_t phys,
>  	struct iommu_domain *domain =3D iommu_get_dma_domain(dev);
>  	struct iommu_dma_cookie *cookie =3D domain->iova_cookie;
>  	struct iova_domain *iovad =3D &cookie->iovad;
> -	size_t aligned_size =3D org_size;
> -	void *padding_start;
> -	size_t padding_size;
> +	void *tlb_start;
> +	size_t aligned_size, iova_off, mapping_end_off;
>  	dma_addr_t iova;
>=20
>  	/*
> @@ -566,24 +565,26 @@ static dma_addr_t
> __iommu_dma_map_swiotlb(struct device *dev, phys_addr_t phys,
>  		if (phys =3D=3D DMA_MAPPING_ERROR)
>  			return DMA_MAPPING_ERROR;
>=20
> -		/* Cleanup the padding area. */
> -		padding_start =3D phys_to_virt(phys);
> -		padding_size =3D aligned_size;
> +		iova_off =3D iova_offset(iovad, phys);
> +		tlb_start =3D phys_to_virt(phys - iova_off);
>=20
> +		/* Cleanup the padding area. */
>  		if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
>  		    (dir =3D=3D DMA_TO_DEVICE ||
>  		     dir =3D=3D DMA_BIDIRECTIONAL)) {
> -			padding_start +=3D org_size;
> -			padding_size -=3D org_size;
> +			mapping_end_off =3D iova_off + org_size;
> +			memset(tlb_start, 0, iova_off);
> +			memset(tlb_start + mapping_end_off, 0,
> +			       aligned_size - mapping_end_off);
> +		} else {
> +			memset(tlb_start, 0, aligned_size);
>  		}

Nice fix. It's better move the "cleanup ..." comment into if case which loo=
ks more accurate.