From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) (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 80FC23112B0 for ; Thu, 27 Nov 2025 09:11:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.251.105.195 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764234688; cv=none; b=G/8ZfSYc0tH2p32laSB3mFcUjkb33tdcCsmty1p5SvL80n/31xMPoCH3chvt/OZmmt6TGavPW46EXpoAqtxRlLdAjePBbo/I/HI3wTP0dQD0bzkaTpYVhMbFweYbkGbbMC+8UmYQMQPDf0vRhbHQPZ+0b5VfJMOgZNGOogj+WGM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764234688; c=relaxed/simple; bh=CZVDxFPvZpotjQmTJYpwOltVRuRdNm7V2Ux8+c+1gyE=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=MZJPWbKQYSw2ljgxj33Zwqrrml9VFXacWgvo1L92D1MQtubPfi0RwLPMgqJy/PpXXn5qt0aii70o5AdC97ew/LZ3stddu6tUPqUPZwkqPlxU+2mP/t2vOVgF7E+XsORYnAUiRA1ENHsFa0cUqQcGFpPyKAqMIENcM3pP0jD5Ejk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=BxqtYSDn; arc=none smtp.client-ip=148.251.105.195 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="BxqtYSDn" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1764234684; bh=CZVDxFPvZpotjQmTJYpwOltVRuRdNm7V2Ux8+c+1gyE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=BxqtYSDnNPl89BUEVoPlvLkNwcfZvneW6HGO9LnSKJ8XtnDOWegfbmD1QQnqLQ9aS fIET08r/hRRMo7u3ejb2dZVvwxZnGaWjCjLOZjTlLsNmhN6s+4xNdJcKIHPHM+Iul2 LQ77VO6qJNwHN6aiNifJd3+JoD9B4pNSNjSqB1qAlWYyILqy1BK1M0mcetYbqRJwyy No7/SAhwi/9GHaSc+zvjRc13ZN55EG42dMuFOiBxTVFDDCuCi2VgLqWeX1vJrFr8NL K/KzA2sD2anBk76Y9Tgw7JTh+vN9v0Peu+UYFH/nAe6G6dCWd5DNW0M0IlkwP7jNW4 XAAsw3lx/FNzA== Received: from fedora (unknown [IPv6:2a01:e0a:2c:6930:d919:a6e:5ea1:8a9f]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (prime256v1) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id 0D0E517E1122; Thu, 27 Nov 2025 10:11:24 +0100 (CET) Date: Thu, 27 Nov 2025 10:11:16 +0100 From: Boris Brezillon To: =?UTF-8?B?QWRyacOhbg==?= Larumbe Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Steven Price , kernel@collabora.com, Liviu Dudau , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter Subject: Re: [PATCH v2 1/1] drm/panthor: Support partial unmaps of huge pages Message-ID: <20251127101116.012309e7@fedora> In-Reply-To: <20251127035021.624045-2-adrian.larumbe@collabora.com> References: <20251127035021.624045-1-adrian.larumbe@collabora.com> <20251127035021.624045-2-adrian.larumbe@collabora.com> Organization: Collabora X-Mailer: Claws Mail 4.3.1 (GTK 3.24.51; x86_64-redhat-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Thu, 27 Nov 2025 03:50:13 +0000 Adri=C3=A1n Larumbe wrote: > Commit 33729a5fc0ca ("iommu/io-pgtable-arm: Remove split on unmap > behavior") did away with the treatment of partial unmaps of huge IOPTEs. >=20 > In the case of Panthor, that means an attempt to run a VM_BIND unmap > operation on a memory region whose start address and size aren't 2MiB > aligned, in the event it intersects with a huge page, would lead to ARM > IOMMU management code to fail and a warning being raised. >=20 > Presently, and for lack of a better alternative, it's best to have > Panthor handle partial unmaps at the driver level, by unmapping entire > huge pages and remapping the difference between them and the requested > unmap region. >=20 > This could change in the future when the VM_BIND uAPI is expanded to > enforce huge page alignment and map/unmap operational constraints that > render this code unnecessary. >=20 > Signed-off-by: Adri=C3=A1n Larumbe > --- > drivers/gpu/drm/panthor/panthor_mmu.c | 76 +++++++++++++++++++++++++++ > 1 file changed, 76 insertions(+) >=20 > diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/pant= hor/panthor_mmu.c > index 183da30fa500..41d7974c95ea 100644 > --- a/drivers/gpu/drm/panthor/panthor_mmu.c > +++ b/drivers/gpu/drm/panthor/panthor_mmu.c > @@ -2110,6 +2110,57 @@ static int panthor_gpuva_sm_step_map(struct drm_gp= uva_op *op, void *priv) > return 0; > } > =20 > +static bool > +is_huge_page(const struct panthor_vma *unmap_vma, u64 addr) The function name doesn't really match the arguments it's being passed. I'd rename this function iova_mapped_as_huge_page(). I'd also rename unmap_vma into vma (the helper doesn't have to know that the test is used for unmapping), and addr into iova. > +{ > + const struct page *pg; > + pgoff_t bo_offset; > + > + bo_offset =3D addr - unmap_vma->base.va.addr + unmap_vma->base.gem.offs= et; > + pg =3D to_panthor_bo(unmap_vma->base.gem.obj)->base.pages[bo_offset >> = PAGE_SHIFT]; > + > + return (folio_order(page_folio(pg)) >=3D PMD_ORDER); I don't think we should use PMD_ORDER for this test, because the GPU MMU page size might differ from the CPU one, and what we care about here is whether this page is huge from the GPU MMU perspective. IOW, we should have: return folio_size(page_folio(pg)) >=3D SZ_2M; > +} > + > +struct remap_params { > + u64 prev_remap_start, prev_remap_range; > + u64 next_remap_start, next_remap_range; > +}; > + > +static struct remap_params > +get_map_unmap_intervals(const struct drm_gpuva_op_remap *op, > + const struct panthor_vma *unmap_vma, > + u64 *unmap_start, u64 *unmap_range) > +{ > + u64 aligned_unmap_start, aligned_unmap_end, unmap_end; > + struct remap_params params =3D {0}; > + > + drm_gpuva_op_remap_to_unmap_range(op, unmap_start, unmap_range); > + unmap_end =3D *unmap_start + *unmap_range; > + > + aligned_unmap_start =3D ALIGN_DOWN(*unmap_start, SZ_2M); > + > + if (aligned_unmap_start < *unmap_start && > + unmap_vma->base.va.addr <=3D aligned_unmap_start && > + is_huge_page(unmap_vma, *unmap_start)) { > + params.prev_remap_start =3D aligned_unmap_start; > + params.prev_remap_range =3D *unmap_start & (SZ_2M - 1); > + *unmap_range +=3D *unmap_start - aligned_unmap_start; > + *unmap_start =3D aligned_unmap_start; > + } > + > + aligned_unmap_end =3D ALIGN(unmap_end, SZ_2M); > + > + if (aligned_unmap_end > unmap_end && > + (unmap_vma->base.va.addr + unmap_vma->base.va.range >=3D aligned_un= map_end) && > + is_huge_page(unmap_vma, unmap_end - 1)) { > + *unmap_range +=3D params.next_remap_range =3D aligned_unmap_end - unma= p_end; Let's do that in two steps to make it more readable please: params.next_remap_range =3D aligned_unmap_end - unmap_end; *unmap_range +=3D params.next_remap_range; > + params.next_remap_start =3D unmap_end; > + } > + > + return params; > +} > + > static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op, > void *priv) > { > @@ -2118,19 +2169,44 @@ static int panthor_gpuva_sm_step_remap(struct drm= _gpuva_op *op, > struct panthor_vm_op_ctx *op_ctx =3D vm->op_ctx; > struct panthor_vma *prev_vma =3D NULL, *next_vma =3D NULL; > u64 unmap_start, unmap_range; > + struct remap_params params; > int ret; > =20 > drm_gpuva_op_remap_to_unmap_range(&op->remap, &unmap_start, &unmap_rang= e); > + > + /* > + * ARM IOMMU page table management code disallows partial unmaps of hug= e pages, > + * so when a partial unmap is requested, we must first unmap the entire= huge > + * page and then remap the difference between the huge page minus the r= equested > + * unmap region. Calculating the right offsets and ranges for the diffe= rent unmap > + * and map operations is the responsibility of the following function. > + */ > + params =3D get_map_unmap_intervals(&op->remap, unmap_vma, &unmap_start,= &unmap_range); > + > ret =3D panthor_vm_unmap_pages(vm, unmap_start, unmap_range); > if (ret) > return ret; > =20 > if (op->remap.prev) { > + ret =3D panthor_vm_map_pages(vm, params.prev_remap_start, > + flags_to_prot(unmap_vma->flags), > + to_drm_gem_shmem_obj(op->remap.prev->gem.obj)->sgt, > + op->remap.prev->gem.offset, params.prev_remap_range); > + if (ret) > + return ret; > + > prev_vma =3D panthor_vm_op_ctx_get_vma(op_ctx); > panthor_vma_init(prev_vma, unmap_vma->flags); > } > =20 > if (op->remap.next) { > + ret =3D panthor_vm_map_pages(vm, params.next_remap_start, > + flags_to_prot(unmap_vma->flags), > + to_drm_gem_shmem_obj(op->remap.next->gem.obj)->sgt, > + op->remap.next->gem.offset, params.next_remap_range); > + if (ret) > + return ret; > + > next_vma =3D panthor_vm_op_ctx_get_vma(op_ctx); > panthor_vma_init(next_vma, unmap_vma->flags); > }