Linux-mediatek Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Yong Wu (吴勇)" <Yong.Wu@mediatek.com>
To: "valentin.haudiquet@canonical.com" <valentin.haudiquet@canonical.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mediatek@lists.infradead.org"
	<linux-mediatek@lists.infradead.org>,
	"krzk@kernel.org" <krzk@kernel.org>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	Jian Hui Lee <jianhui.lee@canonical.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>
Subject: Re: [PATCH v2] memory: mtk-smi: fail larb runtime resume until component bind
Date: Thu, 8 Oct 2026 10:35:40 +0000	[thread overview]
Message-ID: <adb82cfed14bb682fe3250d22bee806bf35b4a78.camel@mediatek.com> (raw)
In-Reply-To: <20260926192417.1448813-1-valentin.haudiquet@canonical.com>

On Sat, 2026-09-26 at 21:24 +0200, Valentin Haudiquet wrote:
> With the mtk-smi and mtk-iommu drivers built as modules, a consumer
> device probe (e.g. mediatek-drm on MT8183) can race the iommu's
> component master bind.  The consumer's probe path runtime-resumes its
> smi-larb suppliers, and the larb's runtime resume then runs
> mtk_smi_larb_config_port_gen2_general(), which dereferences
> larb->mmu and larb->bank.  Those pointers are only populated by
> mtk_smi_larb_bind() when the component master completes, so a larb
> resumed inside the window dereferences NULL and oopses in the module
> loader's initcall context.  The module loader is then stuck and the
> rest of the boot dies.  On MT8183 (krane), mediatek-drm module init
> hits this:
> 
>   Internal error: Oops: 0000000096000004 [#1] SMP
>   CPU: 4 UID: 0 PID: 270 Comm: (udev-worker) Tainted: G W
>   pc : mtk_smi_larb_config_port_gen2_general+0xe0/0x660 [mtk_smi]
>   lr : mtk_smi_larb_resume+0xb8/0x1b8 [mtk_smi]
>   Call trace:
>    mtk_smi_larb_config_port_gen2_general+0xe0/0x660 [mtk_smi] (P)
>    mtk_smi_larb_resume+0xb8/0x1b8 [mtk_smi]
>    pm_generic_runtime_resume+0x38/0x80
>    genpd_runtime_resume+0x118/0x320
>    rpm_callback+0x8c/0xc8
>    rpm_resume+0x558/0x758
>    __pm_runtime_resume+0x64/0xd8
>    pm_runtime_get_suppliers+0x6c/0xb8
>    __driver_probe_device+0x68/0x230
>    driver_probe_device+0xdc/0x1b0
>    __driver_attach+0x10c/0x2c8
>    bus_add_driver+0x17c/0x2e0
>    __platform_register_drivers+0x7c/0x1c0
>    mtk_drm_init+0x38/0xfd0 [mediatek_drm]
>    do_one_initcall+0x60/0x4e0
>    do_init_module+0xa0/0x308
>    init_module_from_file+0x10c/0x180
>    idempotent_init_module+0x1f8/0x2f0
>    __arm64_sys_finit_module+0x74/0x120
> 
> The oopsing udev worker exits holding the module loader's locks, so
> every later module load soft-locks and the boot dies in the initrd.
> The same oops was reported on MT8188 (Genio 700 EVK) from mtk_mdp3
> module init.
> 
> Publish the bind state with smp_store_release() and check it with
> smp_load_acquire() in mtk_smi_larb_resume(); that answers how the
> check synchronizes between CPUs.  Fail the resume with -EAGAIN: the
> larb stays RPM_SUSPENDED and retryable, the error is dropped by
> pm_runtime_get_suppliers() on the driver-core probe path, and the
> consumer's first runtime resume after the bind (at display enable,
> mtk_crtc_atomic_enable()) re-runs the callback, which configures the
> larb ports then.  Failing rather than skipping keeps the larb
> suspended instead of marking it active with unconfigured MMU ports,
> which would only turn the crash into silent iommu faults.
> 
> A failed supplier resume does not break the consumer's probe.
> pm_runtime_get_suppliers() is void and drops the error, so a probe
> inside the window just proceeds.  The display components do not touch
> the larb in probe: mtk_disp_ovl_probe() and its siblings only call
> pm_runtime_enable(), and mdp3 only enables runtime PM for its
> DMA-capable components (mtk-mdp3-comp.c).  The first real runtime
> resume happens later, at display enable: mtk_crtc_atomic_enable()
> does pm_runtime_resume_and_get() and the device links resume the
> larb first.  By then the component master bind has completed, so the
> larb configures its ports and the resume succeeds.  There is no
> deadlock either: the bind only records the larb pointers, it does not
> resume the larb, so the window closes when the bind completes
> regardless of what consumers do.  -ENODEV is not an alternative;
> like -EPROBE_DEFER it is recorded in dev->power.runtime_error
> instead of being exempted.
> 
> The bind state is a separate flag rather than a NULL test on
> larb->mmu so that the publication explicitly covers both larb->mmu
> and larb->bank.  mtk_smi_larb_unbind() clears it again with
> WRITE_ONCE(), which is all the clear side needs since it publishes
> nothing, so an iommu removal or module unload can not leave a stale
> bound state pointing at the freed iommu data.  A resume in flight
> while the iommu goes away is a pre-existing teardown hazard (the
> data it reads belongs to the iommu device, whose removal breaks
> bound consumers anyway); clearing the flag makes every resume that
> starts after the clear fail safely instead.
> 
> Tested on MT8183 (krane) with the full MTK stack built as modules:
> with this change applied the machine boots reliably, the iommu
> probes and binds all larbs, and the display stack comes up fully
> functional.
> 
> The Fixes: tag points at 4f0a1a1ae351, which moved the port
> configuration into the runtime resume.  The device links that open
> the race window are added on the mtk-iommu side, but the dereference
> guarded here is the one that commit introduced.
> 
> Fixes: 4f0a1a1ae351 ("memory: mtk-smi: Invoke pm runtime_callback to
> enable clocks")
> Reported-by: Jian Hui Lee <jianhui.lee@canonical.com>
> Closes: 
> https://lore.kernel.org/linux-arm-kernel/20251111124853.2916889-1-jianhui.lee@canonical.com/
>  
> Cc: stable@vger.kernel.org
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Jian Hui Lee <jianhui.lee@canonical.com>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Cc: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: linux-mediatek@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Valentin Haudiquet <valentin.haudiquet@canonical.com>
> ---
> 
> A fix for this was posted by Jian Hui Lee in November 2025, but the
> thread went quiet after review.  This revision is not from the
> original poster: it is an independent rework of the same fix,
> submitted because the same problem was hit on krane and the machine
> needed a working kernel.
> 
> Changes in v2:
>  - publish the bind state with smp_store_release()/smp_load_acquire()
>    instead of a bare NULL check, answering the "how does this
>    synchronize between CPUs" question from the v1 review
>  - fail the resume with -EAGAIN instead of silently skipping the port
>    configuration; the larb stays RPM_SUSPENDED and the retry is
>    demand-driven (the consumer's next runtime resume, after the bind)
>  - clear the flag in mtk_smi_larb_unbind() so an iommu removal can
> not
>    leave a stale bound state pointing at the freed iommu data
>  - quote the oops backtrace and add proper tags
>  drivers/memory/mtk-smi.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)

Reviewed-by: Yong Wu <yong.wu@mediatek.com>

Thanks very much for the patch.

      reply	other threads:[~2026-10-08 10:36 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-26 19:24 [PATCH v2] memory: mtk-smi: fail larb runtime resume until component bind Valentin Haudiquet
2026-10-08 10:35 ` Yong Wu (吴勇) [this message]

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=adb82cfed14bb682fe3250d22bee806bf35b4a78.camel@mediatek.com \
    --to=yong.wu@mediatek.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=jianhui.lee@canonical.com \
    --cc=krzk@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=rafael@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=ulf.hansson@linaro.org \
    --cc=valentin.haudiquet@canonical.com \
    /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