From: Will Deacon <will@kernel.org>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Robin Murphy <robin.murphy@arm.com>,
Jean-Philippe Brucker <jean-philippe@linaro.org>,
linux-arm-msm@vger.kernel.org, iommu@lists.linux-foundation.org,
linux-kernel@vger.kernel.org,
Amey Narkhede <ameynarkhede03@gmail.com>,
Jon Hunter <jonathanh@nvidia.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] iommu/arm: Cleanup resources in case of probe error path
Date: Thu, 1 Jul 2021 10:01:13 +0100 [thread overview]
Message-ID: <20210701090112.GA9489@willie-the-truck> (raw)
In-Reply-To: <d3fee5f3-a435-7aa8-62ae-060e9b2aa532@samsung.com>
On Thu, Jul 01, 2021 at 10:29:29AM +0200, Marek Szyprowski wrote:
> Hi Robin,
>
> On 30.06.2021 16:01, Robin Murphy wrote:
> > On 2021-06-30 14:48, Marek Szyprowski wrote:
> >> On 30.06.2021 14:59, Will Deacon wrote:
> >>> On Wed, Jun 30, 2021 at 02:48:15PM +0200, Marek Szyprowski wrote:
> >>>> On 08.06.2021 18:45, Amey Narkhede wrote:
> >>>>> If device registration fails, remove sysfs attribute
> >>>>> and if setting bus callbacks fails, unregister the device
> >>>>> and cleanup the sysfs attribute.
> >>>>>
> >>>>> Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
> >>>> This patch landed in linux-next some time ago as commit 249c9dc6aa0d
> >>>> ("iommu/arm: Cleanup resources in case of probe error path"). After
> >>>> bisecting and some manual searching I finally found that it is
> >>>> responsible for breaking s2idle on DragonBoard 410c. Here is the log
> >>>> (captured with no_console_suspend):
> >>>>
> >>>> # time rtcwake -s10 -mmem
> >>>> rtcwake: wakeup from "mem" using /dev/rtc0 at Thu Jan 1 00:02:13 1970
> >>>> PM: suspend entry (s2idle)
> >>>> Filesystems sync: 0.002 seconds
> >>>> Freezing user space processes ... (elapsed 0.006 seconds) done.
> >>>> OOM killer disabled.
> >>>> Freezing remaining freezable tasks ... (elapsed 0.004 seconds) done.
> >>>> Unable to handle kernel NULL pointer dereference at virtual address
> >>>> 0000000000000070
> >>>> Mem abort info:
> >>>> ESR = 0x96000006
> >>>> EC = 0x25: DABT (current EL), IL = 32 bits
> >>>> SET = 0, FnV = 0
> >>>> EA = 0, S1PTW = 0
> >>>> FSC = 0x06: level 2 translation fault
> >>>> Data abort info:
> >>>> ISV = 0, ISS = 0x00000006
> >>>> CM = 0, WnR = 0
> >>>> user pgtable: 4k pages, 48-bit VAs, pgdp=000000008ad08000
> >>>> [0000000000000070] pgd=0800000085c3c003, p4d=0800000085c3c003,
> >>>> pud=0800000088dcf003, pmd=0000000000000000
> >>>> Internal error: Oops: 96000006 [#1] PREEMPT SMP
> >>>> Modules linked in: bluetooth ecdh_generic ecc rfkill ipv6 ax88796b
> >>>> venus_enc venus_dec videobuf2_dma_contig asix crct10dif_ce adv7511
> >>>> snd_soc_msm8916_analog qcom_spmi_temp_alarm rtc_pm8xxx qcom_pon
> >>>> qcom_camss qcom_spmi_vadc videobuf2_dma_sg qcom_vadc_common msm
> >>>> venus_core v4l2_fwnode v4l2_async snd_soc_msm8916_digital
> >>>> videobuf2_memops snd_soc_lpass_apq8016 snd_soc_lpass_cpu v4l2_mem2mem
> >>>> snd_soc_lpass_platform snd_soc_apq8016_sbc videobuf2_v4l2
> >>>> snd_soc_qcom_common qcom_rng videobuf2_common i2c_qcom_cci
> >>>> qnoc_msm8916
> >>>> videodev mc icc_smd_rpm mdt_loader socinfo display_connector rmtfs_mem
> >>>> CPU: 1 PID: 1522 Comm: rtcwake Not tainted 5.13.0-next-20210629 #3592
> >>>> Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
> >>>> pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> >>>> pc : msm_runtime_suspend+0x1c/0x60 [msm]
> >>>> lr : msm_pm_suspend+0x18/0x38 [msm]
> >>>> ...
> >>>> Call trace:
> >>>> msm_runtime_suspend+0x1c/0x60 [msm]
> >>>> msm_pm_suspend+0x18/0x38 [msm]
> >>>> dpm_run_callback+0x84/0x378
> >>> I wonder if we're missing a pm_runtime_disable() call on the failure
> >>> path?
> >>> i.e. something like the diff below...
> >>
> >> I've checked and it doesn't fix anything.
> >
> > What's happened previously? Has an IOMMU actually failed to probe, or
> > is this a fiddly "code movement unveils latent bug elsewhere" kind of
> > thing? There doesn't look to be much capable of going wrong in
> > msm_runtime_suspend() itself, so is the DRM driver also in a broken
> > half-probed state where it's left its pm_runtime_ops behind without
> > its drvdata being valid?
> >
> I finally had some time to analyze this issue. It turned out that with
> this patch, iommu fails to probe for soc:iommu@1f08000 device, while it
> worked fine before. This happens because this patch adds a check for the
> return value of the bus_set_iommu() in
> drivers/iommu/arm/arm-smmu/qcom_iommu.c. When I removed that check, it
> probes successfully again. It looks that there are already iommu ops
> registered for platform bus, before qcom_iommu probes. On the other
> hand, if I remember correctly they are not used during the device
> registration, but they are needed for some legacy stuff. I can send a
> patch restoring old code flow if you think that this is a right solution.
Yes, let's just revert the qcom_iommu.c changes from that patch for now.
The pm runtime stuff looks dodgy anyway so I think this needs more thought.
Will
next prev parent reply other threads:[~2021-07-01 9:01 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-08 16:45 [PATCH] iommu/arm: Cleanup resources in case of probe error path Amey Narkhede
2021-06-08 17:16 ` Will Deacon
[not found] ` <CGME20210630124816eucas1p27563f0a456c0196e20937619aa2f8d26@eucas1p2.samsung.com>
2021-06-30 12:48 ` Marek Szyprowski
2021-06-30 12:59 ` Will Deacon
2021-06-30 13:48 ` Marek Szyprowski
2021-06-30 14:01 ` Robin Murphy
2021-07-01 8:29 ` Marek Szyprowski
2021-07-01 9:01 ` Will Deacon [this message]
2021-07-01 9:11 ` Robin Murphy
2021-07-01 9:26 ` Marek Szyprowski
2021-07-01 0:13 ` Krishna Reddy
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=20210701090112.GA9489@willie-the-truck \
--to=will@kernel.org \
--cc=ameynarkhede03@gmail.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jean-philippe@linaro.org \
--cc=jonathanh@nvidia.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=robin.murphy@arm.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;
as well as URLs for NNTP newsgroup(s).