| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPDyKFrDvxpFeBU5noRDBZCA1N96iPNYYjM0kqd1R4z_4CUV3w@mail.gmail.com>
Date: Fri, 30 May 2025 11:45:48 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Hiago De Franco <hiagofranco@...il.com>
Cc: Peng Fan <peng.fan@....com>, "Peng Fan (OSS)" <peng.fan@....nxp.com>,
Mathieu Poirier <mathieu.poirier@...aro.org>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-remoteproc@...r.kernel.org" <linux-remoteproc@...r.kernel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>, Bjorn Andersson <andersson@...nel.org>,
Hiago De Franco <hiago.franco@...adex.com>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Daniel Baluta <daniel.baluta@....com>,
"Iuliana Prodan (OSS)" <iuliana.prodan@....nxp.com>, Fabio Estevam <festevam@...il.com>,
Pengutronix Kernel Team <kernel@...gutronix.de>
Subject: Re: [PATCH v2 3/3] remoteproc: imx_rproc: add power mode check for
remote core attachment
On Thu, 29 May 2025 at 22:15, Hiago De Franco <hiagofranco@...il.com> wrote:
>
> On Thu, May 29, 2025 at 03:54:47AM +0000, Peng Fan wrote:
>
> [...]
>
> > > We are making progress ;-)
> > >
> > > With the patches you shared Ulf (I added them on top of the current
> > > master branch), it works as expected, dev_pm_genpd_is_on() returns 0
> > > when I boot the kernel without M4 running and it returns 1 when I
> > > boot the kernel with M4 running with a hello-world demo.
> > >
> > > However now I tried to, if dev_pm_genpd_is_on() returns 1, put the
> > > DETACHED state, something as
> > >
> > > if (dev_pm_genpd_is_on(priv->pd_list->pd_devs[0]))
> > > priv->rproc->state = RPROC_DETACHED;
> > >
> > > In this case I used 0 because I understand this is the
> > > IMX_SC_R_M4_0_PID0 defined in my device tree overlay:
> > >
> > > power-domains = <&pd IMX_SC_R_M4_0_PID0>,
> > > <&pd IMX_SC_R_M4_0_MU_1A>;
> > >
> > > But in this case, the kernel does not boot anymore, I see the "Starting
> > > kernel..." and nothing else.
> >
> > Please add "earlycon" in bootargs to see where it hangs.
>
> Thanks Peng! I was able to catch the kernel panic yesterday, however I
> must say that today I was doing the tests again and the issue is gone.
> Sorry, I might have done something wrong yesterday with the tests.
> Anyway, here is the log:
>
> [ 1.271163] remoteproc remoteproc0: imx-rproc is available
> [ 1.280296] remoteproc remoteproc0: attaching to imx-rproc
> [ 1.285756] Unable to handle kernel paging request at virtual address ffff80005ae3dd79
> [ 1.293624] Mem abort info:
> [ 1.294655] mmc0: SDHCI controller on 5b010000.mmc [5b010000.mmc] using ADMA
> [ 1.296386] ESR = 0x0000000096000005
> [ 1.307194] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 1.312473] SET = 0, FnV = 0
> [ 1.315566] EA = 0, S1PTW = 0
> [ 1.318649] FSC = 0x05: level 1 translation fault
> [ 1.323510] Data abort info:
> [ 1.326370] ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
> [ 1.331846] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [ 1.336882] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [ 1.342182] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000096bc1000
> [ 1.348870] [ffff80005ae3dd79] pgd=0000000000000000, p4d=1000000097054003, pud=0000000000000000
> [ 1.357565] Internal error: Oops: 0000000096000005 [#1] SMP
> [ 1.363198] Modules linked in:
> [ 1.366236] CPU: 2 UID: 0 PID: 47 Comm: kworker/u16:3 Not tainted 6.15.0-03667-g3f5f09105c40-dirty #826 PREEMPT
> [ 1.376405] Hardware name: Toradex Colibri iMX8QXP on Colibri Evaluation Board V3 (DT)
> [ 1.384313] Workqueue: events_unbound deferred_probe_work_func
> [ 1.390128] pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 1.397076] pc : rproc_handle_resources.constprop.0+0x78/0x1d0
> [ 1.402896] lr : rproc_boot+0x368/0x56c
> [ 1.406717] sp : ffff8000819c3990
> [ 1.410017] x29: ffff8000819c3990 x28: ffff80005ae3dd7d x27: 0000000000000000
> [ 1.417145] x26: 0000000000000000 x25: ffff0000015ec038 x24: ffff800080f0c0a8
> [ 1.424268] x23: ffff8000813a6110 x22: 00000000d999ad79 x21: ffff0000015ec000
> [ 1.431392] x20: 0000000026665683 x19: ffff80005ae3dd79 x18: 0000000000000006
> [ 1.438516] x17: ffff000001799400 x16: ffff000001798e00 x15: 4addd15cca11c529
> [ 1.445639] x14: 53ebce6d5564d787 x13: 4addd15cca11c529 x12: 53ebce6d5564d787
> [ 1.452763] x11: 95a1e33b6b190674 x10: 9e3c9abdb41ca345 x9 : ab17b4eaffd6fd1c
> [ 1.459887] x8 : d5da055de4cfbb87 x7 : dfd7fa31596acbbc x6 : 9946d97107d0dcca
> [ 1.467011] x5 : ffff0000010c7800 x4 : 00000000000003fc x3 : ffff0000010c7780
> [ 1.474134] x2 : fffffffffffffff0 x1 : ffff8000814a3000 x0 : ffff8000814a3000
> [ 1.481261] Call trace:
> [ 1.483690] rproc_handle_resources.constprop.0+0x78/0x1d0 (P)
> [ 1.487705] mmc0: new HS400 MMC card at address 0001
> [ 1.489502] rproc_boot+0x368/0x56c
> [ 1.495349] mmcblk0: mmc0:0001 Q2J55L 7.09 GiB
> [ 1.497929] rproc_add+0x184/0x190
> [ 1.504356] mmcblk0: p1 p2
> [ 1.505747] imx_rproc_probe+0x458/0x528
> [ 1.509238] mmcblk0boot0: mmc0:0001 Q2J55L 16.0 MiB
> [ 1.512437] platform_probe+0x68/0xc0
> [ 1.512452] really_probe+0xc0/0x38c
> [ 1.520584] mmcblk0boot1: mmc0:0001 Q2J55L 16.0 MiB
> [ 1.520951] __driver_probe_device+0x7c/0x15c
> [ 1.527522] mmcblk0rpmb: mmc0:0001 Q2J55L 4.00 MiB, chardev (242:0)
> [ 1.529377] driver_probe_device+0x3c/0x10c
> [ 1.544263] __device_attach_driver+0xbc/0x158
> [ 1.548586] bus_for_each_drv+0x84/0xe0
> [ 1.552407] __device_attach+0x9c/0x1ac
> [ 1.556231] device_initial_probe+0x14/0x20
> [ 1.560401] bus_probe_device+0xac/0xb0
> [ 1.564221] deferred_probe_work_func+0x9c/0xec
> [ 1.568741] process_one_work+0x14c/0x28c
> [ 1.572735] worker_thread+0x2cc/0x3d4
> [ 1.576473] kthread+0x12c/0x208
> [ 1.579687] ret_from_fork+0x10/0x20
> [ 1.583253] Code: 8b36c033 9100127c 54000924 d503201f (b9400261)
> [ 1.589337] ---[ end trace 0000000000000000 ]---
>
> But again, the issue is not happening anymore ;-) I will keep testing to
> see if the issue happens again, but for now is working fine, I can now
> attach to the remote processor.
>
> This is the git diff on top of Ulf patches I have been testing:
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 6da25e2c81d2..661a6aad40a8 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -599,6 +599,23 @@ int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state)
> }
> EXPORT_SYMBOL_GPL(dev_pm_genpd_set_performance_state);
>
> +bool dev_pm_genpd_is_on(struct device *dev)
> +{
> + struct generic_pm_domain *genpd;
> + bool is_on;
> +
> + genpd = dev_to_genpd_safe(dev);
> + if (!genpd)
> + return false;
> +
> + genpd_lock(genpd);
> + is_on = genpd_status_on(genpd);
> + genpd_unlock(genpd);
> +
> + return is_on;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_genpd_is_on);
> +
> /**
> * dev_pm_genpd_set_next_wakeup - Notify PM framework of an impending wakeup.
> *
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 627e57a88db2..9688370f9bb5 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -18,6 +18,7 @@
> #include <linux/of_reserved_mem.h>
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> #include <linux/reboot.h>
> #include <linux/regmap.h>
> #include <linux/remoteproc.h>
> @@ -891,9 +892,7 @@ static int imx_rproc_attach_pd(struct imx_rproc *priv)
> {
> struct device *dev = priv->dev;
> int ret;
> - struct dev_pm_domain_attach_data pd_data = {
> - .pd_flags = PD_FLAG_DEV_LINK_ON,
> - };
> + bool test;
>
> /*
> * If there is only one power-domain entry, the platform driver framework
> @@ -902,7 +901,16 @@ static int imx_rproc_attach_pd(struct imx_rproc *priv)
> if (dev->pm_domain)
> return 0;
>
> - ret = dev_pm_domain_attach_list(dev, &pd_data, &priv->pd_list);
> + ret = dev_pm_domain_attach_list(dev, NULL, &priv->pd_list);
> + printk("hfranco: returned pd devs is %d", ret);
> + for (int i = 0; i < ret; i++) {
> + test = dev_pm_genpd_is_on(priv->pd_list->pd_devs[i]);
> + printk("hfranco: returned value is %d", test);
> + if (test) {
> + priv->rproc->state = RPROC_DETACHED;
> + break;
> + }
> + }
> return ret < 0 ? ret : 0;
> }
>
> @@ -1146,6 +1154,9 @@ static int imx_rproc_probe(struct platform_device *pdev)
> }
> }
>
> + pm_runtime_enable(dev);
> + pm_runtime_get_sync(dev);
> +
> ret = rproc_add(rproc);
> if (ret) {
> dev_err(dev, "rproc_add failed\n");
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index 756b842dcd30..16d1fca2a8c5 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -95,6 +95,7 @@ extern void pm_runtime_put_suppliers(struct device *dev);
> extern void pm_runtime_new_link(struct device *dev);
> extern void pm_runtime_drop_link(struct device_link *link);
> extern void pm_runtime_release_supplier(struct device_link *link);
> +bool dev_pm_genpd_is_on(struct device *dev);
>
> int devm_pm_runtime_set_active_enabled(struct device *dev);
> extern int devm_pm_runtime_enable(struct device *dev);
>
> This is the rproc output when bootaux is used:
>
> root@...ibri-imx8x-07308754:~# dmesg | grep hfranco
> [ 0.478475] hfranco: returned pd devs is 2
> [ 0.478496] hfranco: returned value is 1
> root@...ibri-imx8x-07308754:~# dmesg | grep rproc
> [ 0.478797] remoteproc remoteproc0: imx-rproc is available
> [ 0.478878] remoteproc remoteproc0: attaching to imx-rproc
> [ 0.478961] remoteproc remoteproc0: remote processor imx-rproc is now attached
>
> I will cleanup everything and try to come up with a patch. Ulf, in this
> case, as your patches have not yet been merged, should I wait for them?
I think you can state in the cover-letter that your series depends on
mine, so please go ahead and submit them.
>
> Thanks for all the help guys.
>
> >
> > >
> > > I am using the pm_runtime functions before rproc_add():
> > >
> > > @@ -1146,6 +1154,9 @@ static int imx_rproc_probe(struct
> > > platform_device *pdev)
> > > }
> > > }
> > >
> > > + pm_runtime_enable(dev);
> > > + pm_runtime_get_sync(dev);
> >
> > I think only make this apply for i.MX8QX/8QM/DX, then no
> > impact to other patforms.
In general I think we should avoid such quirks in drivers, unless it's
really needed. Just wanted to share my opinion, but it's totally up to
you.
[...]
Kind regards
Uffe
Powered by blists - more mailing lists