From: Boris Brezillon <boris.brezillon@collabora.com>
To: Akash Goel <akash.goel@arm.com>
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
Date: Tue, 23 Jun 2026 14:09:42 +0200 [thread overview]
Message-ID: <20260623140942.5055457c@fedora-2.home> (raw)
In-Reply-To: <9f9b372d-b465-4fd4-ab7a-9c399e35b4ed@arm.com>
On Tue, 23 Jun 2026 12:17:51 +0100
Akash Goel <akash.goel@arm.com> wrote:
> Hi Boris
>
> On 6/23/26 10:53, Boris Brezillon wrote:
> > On Tue, 23 Jun 2026 10:24:13 +0100
> > Akash Goel <akash.goel@arm.com> 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 <akash.goel@arm.com>
> >> ---
> >> 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 <boris.brezillon@collabora.com>
next prev parent reply other threads:[~2026-06-23 12:09 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-23 9:24 [PATCH] drm/panthor: Fix NPD issue on partial unmap of an evicted BO Akash Goel
2026-06-23 9:53 ` Boris Brezillon
2026-06-23 11:17 ` Akash Goel
2026-06-23 12:09 ` Boris Brezillon [this message]
2026-06-23 12:41 ` Akash Goel
2026-06-23 12:55 ` Boris Brezillon
2026-06-23 14:14 ` Adrián Larumbe
2026-06-23 14:58 ` Akash Goel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260623140942.5055457c@fedora-2.home \
--to=boris.brezillon@collabora.com \
--cc=airlied@gmail.com \
--cc=akash.goel@arm.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=liviu.dudau@arm.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=nd@arm.com \
--cc=steven.price@arm.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox