All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Adrián Larumbe" <adrian.larumbe@collabora.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v8 06/16] drm/panfrost: Fix PM refcnt and autosuspend issues at device probe/remove
Date: Mon, 07 Sep 2026 20:29:58 +0000	[thread overview]
Message-ID: <20260907202958.F251B1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260907-claude-fixes-v8-6-c2bcb5e82184@collabora.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Ignoring the return value of pm_runtime_get_sync() can lead to unclocked register accesses and system crashes.
- [High] Driver teardown races with in-flight GPU jobs, leading to unhandled hardware faults.
--

commit 75e6d8fc2d8a39088a19c0381af15b43a9a4c041
Author: Adrián Larumbe <adrian.larumbe@collabora.com>

drm/panfrost: Fix PM refcnt and autosuspend issues at device probe/remove

During device probe(), failure to do a PM get() will leave the usage_count
set to 0, which is the value assigned at device creation time. That means
when the autosuspend delay expires, runtime suspend callback won't be
invoked, so the device will remain powered on forever.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 55fc22e8d4d4e..a3eff77add557 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c

[ ... ]

> @@ -884,9 +888,11 @@ static void panfrost_remove(struct platform_device *pdev)
>  	drm_dev_unregister(&pfdev->base);
>  
>  	pm_runtime_get_sync(pfdev->base.dev);
> +	pm_runtime_dont_use_autosuspend(pfdev->base.dev);
> +	pm_runtime_put_noidle(pfdev->base.dev);
>  	pm_runtime_disable(pfdev->base.dev);
> -	panfrost_device_fini(pfdev);
>  	pm_runtime_set_suspended(pfdev->base.dev);
> +	panfrost_device_fini(pfdev);

[Severity: High]
This is a pre-existing issue, but does panfrost_device_fini() ignore the
return value of pm_runtime_get_sync() and risk system crashes?

Looking at panfrost_device.c:panfrost_device_fini(), it issues a warning but
unconditionally proceeds with hardware teardown:

	drm_WARN_ON(&pfdev->base, pm_runtime_get_sync(pfdev->base.dev) < 0);
	...
	panfrost_jm_fini(pfdev);
	panfrost_mmu_fini(pfdev);
	panfrost_gpu_fini(pfdev);

If pm_runtime_get_sync() fails (e.g., due to regulator or clock failure),
the device remains physically suspended with clocks disabled.

Can unconditionally writing to MMIO registers in panfrost_jm_fini(),
panfrost_mmu_fini(), and panfrost_gpu_fini() with clocks off cause a bus
fault or kernel panic?

[Severity: High]
This is a pre-existing issue, but does panfrost_device_fini() tear down
hardware while GPU jobs might still be executing?

During device unbind, the driver does not appear to wait for actively
executing jobs on the physical GPU hardware to complete or abort. It proceeds
to unmap pagetables and power off the GPU:

	panfrost_jm_fini(pfdev);
	panfrost_mmu_fini(pfdev);
	panfrost_gpu_fini(pfdev);

Can pulling the memory and clocks out from under a running GPU job during a
hot-unplug or unbind cause catastrophic hardware faults or kernel panics?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-claude-fixes-v8-0-c2bcb5e82184@collabora.com?part=6

  reply	other threads:[~2026-09-07 20:30 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-07 20:16 [PATCH v8 00/16] Collection of fixes for Panfrost: Perfcnt, RPM, refactorings Adrián Larumbe
2026-09-07 20:16 ` [PATCH v8 01/16] drm/panfrost: Move shrinker initialization and unplug one level down Adrián Larumbe
2026-09-07 20:16 ` [PATCH v8 02/16] drm/panfrost: Move lock and modparam initialisations into their subsystems Adrián Larumbe
2026-09-07 20:16 ` [PATCH v8 03/16] drm/panfrost: Move debugfs initialisation to relevant subsystems Adrián Larumbe
2026-09-07 20:28   ` sashiko-bot
2026-09-07 20:16 ` [PATCH v8 04/16] drm/panfrost: Skip NULL checks for clock enable/disabling Adrián Larumbe
2026-09-07 20:16 ` [PATCH v8 05/16] drm/panfrost: Consolidate device clock management and reset Adrián Larumbe
2026-09-07 20:31   ` sashiko-bot
2026-09-07 20:16 ` [PATCH v8 06/16] drm/panfrost: Fix PM refcnt and autosuspend issues at device probe/remove Adrián Larumbe
2026-09-07 20:29   ` sashiko-bot [this message]
2026-09-07 20:16 ` [PATCH v8 07/16] drm/panfrost: Explicitly enable MMU interrupts at device init Adrián Larumbe
2026-09-07 20:31   ` sashiko-bot
2026-09-07 20:16 ` [PATCH v8 08/16] drm/panfrost: Move all DRM device initialisation into device_init() Adrián Larumbe
2026-09-07 20:16 ` [PATCH v8 09/16] drm/panfrost: Add warning messages to fatal error conditions Adrián Larumbe
2026-09-07 20:16 ` [PATCH v8 10/16] drm/panfrost: Add debugfs knob for manually triggering a GPU reset Adrián Larumbe
2026-09-07 20:28   ` sashiko-bot
2026-09-07 20:16 ` [PATCH v8 11/16] drm/panfrost: Move perfcnt GPU disable sequence into a helper Adrián Larumbe
2026-09-07 20:16 ` [PATCH v8 12/16] drm/panfrost: Skip cache flush/invalidate when enabling perfcnt Adrián Larumbe
2026-09-07 20:16 ` [PATCH v8 13/16] drm/panfrost: Avoid cache flush after perfcnt sample in fully coherent systems Adrián Larumbe
2026-09-07 20:35   ` sashiko-bot
2026-09-07 20:16 ` [PATCH v8 14/16] drm/panfrost: Introduce a reset lock Adrián Larumbe
2026-09-07 20:38   ` sashiko-bot
2026-09-07 20:16 ` [PATCH v8 15/16] drm/panfrost: Fix races between perfcnt and reset sequence Adrián Larumbe
2026-09-07 20:36   ` sashiko-bot
2026-09-07 20:16 ` [PATCH v8 16/16] drm/panfrost: Bump driver minor to reflect new DUMP IOCTL req field Adrián Larumbe
2026-09-07 20:33   ` sashiko-bot

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=20260907202958.F251B1F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=adrian.larumbe@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.