From nobody Fri Nov 22 22:03:44 2024 Received: from out-179.mta0.migadu.com (out-179.mta0.migadu.com [91.218.175.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9452E1B0F24 for ; Fri, 15 Nov 2024 12:33:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.179 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731674028; cv=none; b=Zk9a9YNKQM+lLX0Iz7U2Jvm0LeIWDke8zgQDWmjkYqVCOk5xqAxm+0aT5YgxmVCgaCANkDzNu30K8AZgav+Bfx8Aw2+S6gQYdUO8aYJAHF0ixXrE7+vUyKqZOwpYmzle8HoWucHo9TjdtQERQDDsM37JCkZbhazIOltg8CIEBLQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731674028; c=relaxed/simple; bh=p1FBURnBl3WmoZ6QfnFAvJt2S5vJOEikeqPxrbq9oU0=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=rFCrk5XDEsSWXtuD+ElnVaY36jPFTmVjBEmpMRf4eOtLmFdR+y22LfnViaNaFL3v1RCvCGde2Sgg5j5Pqy0LXWiLhpfZMYkI8kaEOozZ5EYo2pT4FcM6ZeIUjPyA5TeGDWb7Wa2AFj3M7iOrUVFErmGMz7waO5Mz9400YN1Liwk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=TvhwZEUd; arc=none smtp.client-ip=91.218.175.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="TvhwZEUd" X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1731674023; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=lFm/QsqGtNpOQ5SfKjk7QqN4kTAcwma8p9hnqSEIdIw=; b=TvhwZEUdErRZSOAllia1psVl6aQXD4WEscpoQXA5ebV9ZcfBj5NHRKWvdRc428Wc4268bW 9OB95aE1FjavmQYR3MdwO/a4JiQswaPBDKdZJte4MA3tdBXJD475cvi3oQRNLz90qhDjPD TOq2mPpmMQOBaWL45IFaIBe2qZqMOdQ= From: Sui Jingfeng To: Lucas Stach , Russell King , Christian Gmeiner Cc: David Airlie , Simona Vetter , etnaviv@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Sui Jingfeng Subject: [PATCH v3 1/3] drm/etnaviv: Drop the offset in page manipulation Date: Fri, 15 Nov 2024 20:32:44 +0800 Message-Id: <20241115123246.111346-2-sui.jingfeng@linux.dev> In-Reply-To: <20241115123246.111346-1-sui.jingfeng@linux.dev> References: <20241115123246.111346-1-sui.jingfeng@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Migadu-Flow: FLOW_OUT Content-Type: text/plain; charset="utf-8" The etnaviv driver, both kernel space and user space, assumes that GPU page size is 4KiB. Its IOMMU map/unmap 4KiB physical address range once a time. If 'sg->offset !=3D 0' is true, then the current implementation will map the IOVA to a wrong area, which may lead to coherency problem. Picture 0 and 1 give the illustration, see below. PA start drifted | |<--- 'sg_dma_address(sg) - sg->offset' | .------ sg_dma_address(sg) | | .---- sg_dma_len(sg) |<-sg->offset->| | V |<-->| Another one cpu page +----+----+----+----+ +----+----+----+----+ |xxxx| |||||| ||||||||||||||||||||| +----+----+----+----+ +----+----+----+----+ ^ ^ ^ ^ |<--- da_len --->| | | | | | | | .--------------' | | | | .----------------' | | | | .----------------' | | | | | | +----+----+----+----+ | | ||||||||||||||||||||| | | +----+----+----+----+ | | | '--------------. da_len =3D sg_dma_len(sg) + sg->offset, using | | 'sg_dma_len(sg) + sg->offset' will lead to GPUVA +----+ ~~~~~~~~~~~~~+ collision, but min_t(unsigned int, da_len, va_len) |xxxx| | will clamp it to correct size. But the IOVA will +----+ ~~~~~~~~~~~~~+ be redirect to wrong area. ^ | Picture 0: Possibly wrong implementation. GPUVA (IOVA) -------------------------------------------------------------------------- .------- sg_dma_address(sg) | .---- sg_dma_len(sg) |<-sg->offset->| | | |<-->| another one cpu page +----+----+----+----+ +----+----+----+----+ | |||||| ||||||||||||||||||||| +----+----+----+----+ +----+----+----+----+ ^ ^ ^ ^ | | | | .--------------' | | | | | | | | .--------------' | | | | .----------------' | | | | .----------------' | | | | +----+ +----+----+----+----+ |||||| ||||||||||||||||||||| The first one is SZ_4K, the second is SZ_16K +----+ +----+----+----+----+ ^ | Picture 1: Perfectly correct implementation. GPUVA (IOVA) If sg->offset !=3D 0 is true, IOVA will be mapped to wrong physical address. Either because there doesn't contain the data or there contains wrong data. Strictly speaking, the memory area that before sg_dma_address(sg) doesn't belong to us, and it's likely that the area is being used by other process. Because we don't want to introduce confusions about which part is visible to the GPU, we assumes that the size of GPUVA is always 4KiB aligned. This is very relaxed requirement, since we already made the decision that GPU page size is 4KiB (as a canonical decision). And softpin feature is landed, Mesa's util_vma_heap_alloc() will certainly report correct length of GPUVA to kernel with desired alignment ensured. With above statements agreed, drop the "offset in page" manipulation will return us a correct implementation at any case. Fixes: a8c21a5451d8 ("drm/etnaviv: add initial etnaviv DRM driver") Signed-off-by: Sui Jingfeng --- drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnavi= v/etnaviv_mmu.c index 7e065b3723cf..c786df840a18 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c @@ -82,8 +82,8 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_context= *context, return -EINVAL; =20 for_each_sgtable_dma_sg(sgt, sg, i) { - phys_addr_t pa =3D sg_dma_address(sg) - sg->offset; - unsigned int da_len =3D sg_dma_len(sg) + sg->offset; + phys_addr_t pa =3D sg_dma_address(sg); + unsigned int da_len =3D sg_dma_len(sg); unsigned int bytes =3D min_t(unsigned int, da_len, va_len); =20 VERB("map[%d]: %08x %pap(%x)", i, iova, &pa, bytes); --=20 2.34.1 From nobody Fri Nov 22 22:03:44 2024 Received: from out-181.mta0.migadu.com (out-181.mta0.migadu.com [91.218.175.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EBA071BD00A for ; Fri, 15 Nov 2024 12:33:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.181 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731674032; cv=none; b=WjF2A0EVZPxfPFuY10SdhUYaLGeT8YRrAVZQBxV3w1wiGw6Y/M1aJZZh5E0WBHYetuzwkO1TZL2r2ELB73w6+niywQNKdV7XdyYKjHxY2P2zLAwkZ72RkSmy06qd+Xl5Cep7lPwx8bYq3VydJIeUdLak+9JBMpOhsVn+sA6XI5Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731674032; c=relaxed/simple; bh=QR+yyH9uPJ/OqaN5IdQwKdfdKmwO+NOSoIP1GccCk24=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=YGBCLxVEsf83irKho7caB98CL31jQVMv7a+GmGTVMOlejIxOwoEljcE5QThkxn3K98Jy1wCHYYe9TVZt1soZGhbuap3+UFVBlMa7/3d3WTiKW06Lvax6A+3Ud6mSWfEq3myJRzKbNLb6WGQouRHF9d3ff/t/L7d0KnNnD5rkN6M= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=tEpmfXTA; arc=none smtp.client-ip=91.218.175.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="tEpmfXTA" X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1731674028; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5Q1NNXm8xCGlaeYRhrzTJWYfqyDhwYjiiGjFlTLExYI=; b=tEpmfXTAbP1X+EpnZ/kMxs8+Keov1Wrn1IC2phl4qaNcaMwKv+I2ursKhEvzRdc2kLo+4M 9jdZHHnkPlpE/CBM6tIedljxu+eVgnZ3MqJAZPS0UgV6A8+EVRV+StvEwtY+U/mufIdHmr +3aSAl+isWcrlvU7oIm+k2gX2fsbS7A= From: Sui Jingfeng To: Lucas Stach , Russell King , Christian Gmeiner Cc: David Airlie , Simona Vetter , etnaviv@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Sui Jingfeng Subject: [PATCH v3 2/3] drm/etnaviv: Fix the debug log of the etnaviv_iommu_map() Date: Fri, 15 Nov 2024 20:32:45 +0800 Message-Id: <20241115123246.111346-3-sui.jingfeng@linux.dev> In-Reply-To: <20241115123246.111346-1-sui.jingfeng@linux.dev> References: <20241115123246.111346-1-sui.jingfeng@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Migadu-Flow: FLOW_OUT Content-Type: text/plain; charset="utf-8" The value of the 'iova' variable is the start GPUVA that is going to be mapped, its value doesn't changed when the mapping is on going. Replace it with the 'da' variable, which is incremental and it reflects the actual address being mapped exactly. Signed-off-by: Sui Jingfeng --- drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnavi= v/etnaviv_mmu.c index c786df840a18..ff90bf85c156 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c @@ -86,7 +86,7 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_context= *context, unsigned int da_len =3D sg_dma_len(sg); unsigned int bytes =3D min_t(unsigned int, da_len, va_len); =20 - VERB("map[%d]: %08x %pap(%x)", i, iova, &pa, bytes); + VERB("map[%d]: %08x %pap(%x)", i, da, &pa, bytes); =20 ret =3D etnaviv_context_map(context, da, pa, bytes, prot); if (ret) --=20 2.34.1 From nobody Fri Nov 22 22:03:44 2024 Received: from out-181.mta0.migadu.com (out-181.mta0.migadu.com [91.218.175.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E88B81BBBCC for ; Fri, 15 Nov 2024 12:33:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.181 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731674037; cv=none; b=hYbTYGLu2Sh1/uWHIHkICnZa4XALdn/dJXUaH7C/VL/Rj+iq+XjEMvHXAG83Vqp3zhb5Sqk5qitPWZ3xPyKsCnH56gOd3KqzDabcOL1S/F7Xac6B+kWt57mO6qvdtpXM0ND/vHQwXm9CCU7JxKD8RPHRVLcoJ3tDYEYMryqtbAU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731674037; c=relaxed/simple; bh=dOSC6eUGC/in0o9pWwfngkUERvWaWcLQ205GavFwqhk=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=uUSaXHsFmMEfu3L5LbXs7uOIz0VoWi77ojbIfOs0vzSNstRAWsd8atXz6sH8GD35FXMSTLRegsUPwWWRGrJ7hS8GYsKUWpvQNCYX9+ZsSgxhb0WRfigKgbbFQwFyG6+kdh7brZYdRyFtlZygZE0dXFS/c0sow8D75auPZg8etmU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=PAIFC22J; arc=none smtp.client-ip=91.218.175.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="PAIFC22J" X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1731674032; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=GH0nS2nq3ekeLQm+XUvi36Oo7y7L7oC9zjgTOfOD4BI=; b=PAIFC22JH4MhlCMgShy7Zb4gzL9CCZ5iyW6Pf4+Jq+VOU62gyDJJrL8o/E9Sed0yiZCG4/ 2jGTHatUPYtsueFT5r28dJNk0AAVEVjn7imsYAsvOIqhpLlydG0HwuO4acng9KOJLiuY1X 7F9ip2JLsGY3G7pzH9sma9AG5jihgSY= From: Sui Jingfeng To: Lucas Stach , Russell King , Christian Gmeiner Cc: David Airlie , Simona Vetter , etnaviv@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Sui Jingfeng Subject: [PATCH v3 3/3] drm/etnaviv: Improve VA, PA, SIZE alignment checking Date: Fri, 15 Nov 2024 20:32:46 +0800 Message-Id: <20241115123246.111346-4-sui.jingfeng@linux.dev> In-Reply-To: <20241115123246.111346-1-sui.jingfeng@linux.dev> References: <20241115123246.111346-1-sui.jingfeng@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Migadu-Flow: FLOW_OUT Content-Type: text/plain; charset="utf-8" Alignment checking is only needed to be done in the upper caller function. If those address and sizes are able to pass the check, it will certainly pass the same test in the etnaviv_context_unmap() function. We don't need examine it more than once. Remove redundant alignment tests, move the those useless to upper caller function. Signed-off-by: Sui Jingfeng --- drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnavi= v/etnaviv_mmu.c index ff90bf85c156..df5192083b20 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c @@ -19,12 +19,6 @@ static void etnaviv_context_unmap(struct etnaviv_iommu_c= ontext *context, size_t unmapped_page, unmapped =3D 0; size_t pgsize =3D SZ_4K; =20 - if (!IS_ALIGNED(iova | size, pgsize)) { - pr_err("unaligned: iova 0x%lx size 0x%zx min_pagesz 0x%zx\n", - iova, size, pgsize); - return; - } - while (unmapped < size) { unmapped_page =3D context->global->ops->unmap(context, iova, pgsize); @@ -45,12 +39,6 @@ static int etnaviv_context_map(struct etnaviv_iommu_cont= ext *context, size_t orig_size =3D size; int ret =3D 0; =20 - if (!IS_ALIGNED(iova | paddr | size, pgsize)) { - pr_err("unaligned: iova 0x%lx pa %pa size 0x%zx min_pagesz 0x%zx\n", - iova, &paddr, size, pgsize); - return -EINVAL; - } - while (size) { ret =3D context->global->ops->map(context, iova, paddr, pgsize, prot); @@ -88,6 +76,14 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_contex= t *context, =20 VERB("map[%d]: %08x %pap(%x)", i, da, &pa, bytes); =20 + if (!IS_ALIGNED(iova | pa | bytes, SZ_4K)) { + dev_err(context->global->dev, + "unaligned: iova 0x%x pa %pa size 0x%x\n", + iova, &pa, bytes); + ret =3D -EINVAL; + goto fail; + } + ret =3D etnaviv_context_map(context, da, pa, bytes, prot); if (ret) goto fail; --=20 2.34.1