linux-kernel - Re: [PATCH v2 3/3] remoteproc: imx_rproc: add power mode check for remote core attachment

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250523191713.nylhi74jq6z4hqmr@hiago-nb>
Date: Fri, 23 May 2025 16:17:13 -0300
From: Hiago De Franco <hiagofranco@...il.com>
To: Ulf Hansson <ulf.hansson@...aro.org>
Cc: Peng Fan <peng.fan@....nxp.com>,
	Mathieu Poirier <mathieu.poirier@...aro.org>,
	linux-pm@...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,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	daniel.baluta@....com, iuliana.prodan@....nxp.com,
	Fabio Estevam <festevam@...il.com>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Peng Fan <peng.fan@....com>
Subject: Re: [PATCH v2 3/3] remoteproc: imx_rproc: add power mode check for
 remote core attachment

Hi Ulf,

On Wed, May 21, 2025 at 02:11:02PM +0200, Ulf Hansson wrote:
> You should not provide any flag (or attach_data to
> dev_pm_domain_attach_list()) at all. In other words just call
> dev_pm_domain_attach_list(dev, NULL, &priv->pd_list), similar to how
> drivers/remoteproc/imx_dsp_rproc.c does it.
> 
> In this way, the device_link is created by making the platform->dev
> the consumer and by keeping the supplier-devices (corresponding to the
> genpds) in RPM_SUSPENDED state.
> 
> The PM domains (genpds) are then left in their current state, which
> should allow us to call dev_pm_genpd_is_on() for the corresponding
> supplier-devices, to figure out whether the bootloader turned them on
> or not, I think.
> 
> Moreover, to make sure the genpds are turned on when needed, we also
> need to call pm_runtime_enable(platform->dev) and
> pm_runtime_get_sync(platform->dev). The easiest approach is probably
> to do that during ->probe() - and then as an improvement on top you
> may want to implement more fine-grained support for runtime PM.
> 
> [...]
> 
> Kind regards
> Uffe

I did some tests here and I might be missing something. I used the
dev_pm_genpd_is_on() inside imx_rproc.c with the following changes:

@@ -902,7 +902,12 @@ 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);
+       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);
+       }
        return ret < 0 ? ret : 0;
 }

This was a quick test to check the returned value, and it always return
1 for both pds, even if I did not boot the remote core.

So I was wondering if it was because of PD_FLAG_DEV_LINK_ON, I removed
it and passed NULL to dev_pm_domain_attach_list(). Booting the kernel
now it correctly reports 0 for both pds, however when I start the
remote core with a hello world firmware and boot the kernel, the CPU
resets with a fault reset ("Reset cause: SCFW fault reset").

I added both pm functions to probe, just to test:

@@ -1152,6 +1158,9 @@ static int imx_rproc_probe(struct platform_device *pdev)
                goto err_put_clk;
        }

+       pm_runtime_enable(dev);
+       pm_runtime_get_sync(dev);
+
        return 0

Now the kernel boot with the remote core running, but it still returns
0 from dev_pm_genpd_is_on(). So basically now it always returns 0, with
or without the remote core running.

I tried to move pm_runtime_get_sync() to .prepare function but it make
the kernel not boot anymore (with the SCU fault reset).

Do you have any suggestions? Am I doing something wrong with these PDs?

Best regards,
Hiago.