linux-kernel - Re: [PATCH v3 21/24] pmdomain: core: Leave powered-on genpds on until late_initcall

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: <212a1a56-08a5-48a5-9e98-23de632168d0@samsung.com>
Date: Thu, 10 Jul 2025 14:26:52 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Ulf Hansson <ulf.hansson@...aro.org>, Saravana Kannan
	<saravanak@...gle.com>, Stephen Boyd <sboyd@...nel.org>,
	linux-pm@...r.kernel.org
Cc: "Rafael J . Wysocki" <rafael@...nel.org>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>, Michael Grzeschik
	<m.grzeschik@...gutronix.de>, Bjorn Andersson <andersson@...nel.org>, Abel
	Vesa <abel.vesa@...aro.org>, Peng Fan <peng.fan@....nxp.com>, Tomi Valkeinen
	<tomi.valkeinen@...asonboard.com>, Johan Hovold <johan@...nel.org>, Maulik
	Shah <maulik.shah@....qualcomm.com>, Michal Simek <michal.simek@....com>,
	Konrad Dybcio <konradybcio@...nel.org>, Thierry Reding
	<thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>, Hiago De
	Franco <hiago.franco@...adex.com>, Geert Uytterhoeven
	<geert@...ux-m68k.org>, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 21/24] pmdomain: core: Leave powered-on genpds on
 until late_initcall_sync

On 01.07.2025 13:47, Ulf Hansson wrote:
> Powering-off a genpd that was on during boot, before all of its consumer
> devices have been probed, is certainly prone to problems.
>
> As a step to improve this situation, let's prevent these genpds from being
> powered-off until genpd_power_off_unused() gets called, which is a
> late_initcall_sync().
>
> Note that, this still doesn't guarantee that all the consumer devices has
> been probed before we allow to power-off the genpds. Yet, this should be a
> step in the right direction.
>
> Suggested-by: Saravana Kannan <saravanak@...gle.com>
> Tested-by: Hiago De Franco <hiago.franco@...adex.com> # Colibri iMX8X
> Tested-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com> # TI AM62A,Xilinx ZynqMP ZCU106
> Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>

This change has a side effect on some Exynos based boards, which have 
display and bootloader is configured to setup a splash screen on it. 
Since today's linux-next, those boards fails to boot, because of the 
IOMMU page fault.

This happens because the display controller is enabled and configured to 
perform the scanout from the spash-screen buffer until the respective 
driver will reset it in driver probe() function. This however doesn't 
work with IOMMU, which is being probed earlier than the display 
controller driver, what in turn causes IOMMU page fault once the IOMMU 
driver gets attached. This worked before applying this patch, because 
the power domain of display controller was simply turned off early 
effectively reseting the display controller.

This has been discussed a bit recently: 
https://lore.kernel.org/all/544ad69cba52a9b87447e3ac1c7fa8c3@disroot.org/ 
and I can add a workaround for this issue in the bootloaders of those 
boards, but this is something that has to be somehow addressed in a 
generic way.

> ---
>   drivers/pmdomain/core.c   | 10 ++++++++--
>   include/linux/pm_domain.h |  1 +
>   2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 5cef6de60c72..18951ed6295d 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -931,11 +931,12 @@ static void genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
>   	 * The domain is already in the "power off" state.
>   	 * System suspend is in progress.
>   	 * The domain is configured as always on.
> +	 * The domain was on at boot and still need to stay on.
>   	 * The domain has a subdomain being powered on.
>   	 */
>   	if (!genpd_status_on(genpd) || genpd->prepared_count > 0 ||
>   	    genpd_is_always_on(genpd) || genpd_is_rpm_always_on(genpd) ||
> -	    atomic_read(&genpd->sd_count) > 0)
> +	    genpd->stay_on || atomic_read(&genpd->sd_count) > 0)
>   		return;
>   
>   	/*
> @@ -1346,8 +1347,12 @@ static int __init genpd_power_off_unused(void)
>   	pr_info("genpd: Disabling unused power domains\n");
>   	mutex_lock(&gpd_list_lock);
>   
> -	list_for_each_entry(genpd, &gpd_list, gpd_list_node)
> +	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
> +		genpd_lock(genpd);
> +		genpd->stay_on = false;
> +		genpd_unlock(genpd);
>   		genpd_queue_power_off_work(genpd);
> +	}
>   
>   	mutex_unlock(&gpd_list_lock);
>   
> @@ -2352,6 +2357,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
>   	INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
>   	atomic_set(&genpd->sd_count, 0);
>   	genpd->status = is_off ? GENPD_STATE_OFF : GENPD_STATE_ON;
> +	genpd->stay_on = !is_off;
>   	genpd->sync_state = GENPD_SYNC_STATE_OFF;
>   	genpd->device_count = 0;
>   	genpd->provider = NULL;
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index d68e07dadc99..99556589f45e 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -199,6 +199,7 @@ struct generic_pm_domain {
>   	unsigned int performance_state;	/* Aggregated max performance state */
>   	cpumask_var_t cpus;		/* A cpumask of the attached CPUs */
>   	bool synced_poweroff;		/* A consumer needs a synced poweroff */
> +	bool stay_on;			/* Stay powered-on during boot. */
>   	enum genpd_sync_state sync_state; /* How sync_state is managed. */
>   	int (*power_off)(struct generic_pm_domain *domain);
>   	int (*power_on)(struct generic_pm_domain *domain);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland