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 663653D45CF for ; Tue, 23 Jun 2026 12:09:49 +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=1782216590; cv=none; b=lZObCtaLD6J5nQNkC7HpHXLYevucPae4F4EgOY0EvBz4wV+lyC85i+KiyNWG+ZO7lPdX58zBSpS5by+QcuTmITplC9MfZOBW3HZDLalCuFSVgtC1+M9Engo3pDkkRNO3Mf/sJP2WtaY232COnB9iG94xJQYExXVYEuCpdS6mJ44= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782216590; c=relaxed/simple; bh=/TZ1zUVv9qX46VAgMsPsPlEdJt9LZFVakRYi1wlWw0M=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=ktAGce3AhRSRmKolqeiNKglRZilJb0TK7YawqKRcX6raWGtd3FyVrkjAE2ba3UM9UUB+YBXJF33qOpXL/6i6PSSMvGsQHfaboXgUotoJLnKMDb6IUJCFlP3DU/HDuy1lyKgXYNGN30PdhcqloUWBEuih9qXsoast1Js6anZU9JI= 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=WOuerfJQ; 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="WOuerfJQ" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1782216587; bh=/TZ1zUVv9qX46VAgMsPsPlEdJt9LZFVakRYi1wlWw0M=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=WOuerfJQT8r+WBDJTIyz1Ier7k+9Im5Y5fHm51mveX82HSHfb4e0AtWeN7mv4Lnr2 Uj8BSjU9Q/2cK/t7iY1Rlpa1GO/iyjj1pp6h80lZPYaOEwvVrWzJnC6AbkpEknYXUd bNcufi/I2LVfR0dl88U4BaTs953Lz7PRCMhPgLNNVoNTWAzNj5EoIEhBqI95hm5Vq7 ydOxE82sRCSq4QUuEfYZsiz0YgfjjmOOuj8xIVkQZ0WiBBbrkaMJi6gEooDtIB1i/4 JU244u8X5ZJT2tVTe5Xt4IiSVhptjboz7OrYbTe1xkr8Pagl7xz8UMk8ybuq3pSA6I FErQzi41e2B3g== Received: from fedora-2.home (unknown [100.64.0.11]) (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 182EE17E062C; Tue, 23 Jun 2026 14:09:47 +0200 (CEST) Date: Tue, 23 Jun 2026 14:09:42 +0200 From: Boris Brezillon To: Akash Goel Cc: liviu.dudau@arm.com, steven.price@arm.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch, nd@arm.com Subject: Re: [PATCH] drm/panthor: Fix NPD issue on partial unmap of an evicted BO Message-ID: <20260623140942.5055457c@fedora-2.home> In-Reply-To: <9f9b372d-b465-4fd4-ab7a-9c399e35b4ed@arm.com> References: <20260623092413.2710066-1-akash.goel@arm.com> <20260623115332.7e89f56b@fedora-2.home> <9f9b372d-b465-4fd4-ab7a-9c399e35b4ed@arm.com> Organization: Collabora X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; 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=US-ASCII Content-Transfer-Encoding: 7bit On Tue, 23 Jun 2026 12:17:51 +0100 Akash Goel wrote: > Hi Boris > > On 6/23/26 10:53, Boris Brezillon wrote: > > On Tue, 23 Jun 2026 10:24:13 +0100 > > Akash Goel wrote: > > > >> This commit fixes the NULL pointer dereference issue that would have > >> happened on the split of GPU mapping due to partial unmap of an evicted > >> BO. There is a logic to handle the partial unmap of huge pages when the > >> GPU mapping is split. That logic was not being completely skipped for > >> the VMA of an evicted BO and that resulted in a NPD possibility for the > >> 'bo->backing.pages' pointer, which is set to NULL when pages of a > >> BO are released on eviction. > >> > > >> > >> Fixes: 8e7460eac786 ("drm/panthor: Support partial unmaps of huge pages") > >> Signed-off-by: Akash Goel > >> --- > >> drivers/gpu/drm/panthor/panthor_mmu.c | 26 +++++++++++++------------- > >> 1 file changed, 13 insertions(+), 13 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c > >> index 31cc57029c12..285e7b9bc100 100644 > >> --- a/drivers/gpu/drm/panthor/panthor_mmu.c > >> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c > >> @@ -2358,20 +2358,20 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op, > >> */ > >> panthor_fix_sparse_map_offset(op->remap.next, unmap_vma->flags); > >> > >> - /* > >> - * ARM IOMMU page table management code disallows partial unmaps of huge 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 requested > >> - * unmap region. Calculating the right start address and range for the expanded > >> - * unmap operation is the responsibility of the following function. > >> - */ > >> - unmap_hugepage_align(&op->remap, &unmap_start, &unmap_range); > >> - > >> - /* If the range changed, we might have to lock a wider region to guarantee > >> - * atomicity. panthor_vm_lock_region() bails out early if the new region > >> - * is already part of the locked region, so no need to do this check here. > >> - */ > >> if (!unmap_vma->evicted) { > >> + /* > >> + * ARM IOMMU page table management code disallows partial unmaps of huge 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 requested > >> + * unmap region. Calculating the right start address and range for the expanded > >> + * unmap operation is the responsibility of the following function. > >> + */ > >> + unmap_hugepage_align(&op->remap, &unmap_start, &unmap_range); > >> + > >> + /* If the range changed, we might have to lock a wider region to guarantee > >> + * atomicity. panthor_vm_lock_region() bails out early if the new region > >> + * is already part of the locked region, so no need to do this check here. > >> + */ > >> panthor_vm_lock_region(vm, unmap_start, unmap_range); > >> panthor_vm_unmap_pages(vm, unmap_start, unmap_range); > >> } > > > > > > I think we want something like that instead, so we can keep the > > 2M alignment for sparse mappings which go recently introduced. > > > > Thanks for the suggestion. But sorry I didn't get it. > > I see that the patching of 'op->remap.next->gem.offset' would still be > done with my change. > > panthor_fix_sparse_map_offset(op->remap.next, unmap_vma->flags); > > if (!unmap_vma->evicted) { > unmap_hugepage_align(&op->remap, &unmap_start, > > IIUC, the 2M alignment is done to avoid a potential partial unmap of 2M > page. But if the VMA is in evicted state then already the unmap would > have happened for the whole virtual range covered by the VMA. Nah, you're correct, the patching of the drm_gpuva is independent of the adjusted unmap range, so we should be good even if we don't adjust this range for evicted sparse mappings. Sorry for the noise. Reviewed-by: Boris Brezillon