linux-kernel - Re: [PATCH v9 3/7] power: reset: Introduce PSCR Recording Framework for Non-Volatile Storage

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 for Android: free password hash cracker in your pocket

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

Message-ID: <bumx6ma3kjanapwaf3oc3mdjnekatvc2cmavt6secfkaapgjpz@kouqjidbl47k>
Date: Fri, 2 May 2025 01:20:50 +0200
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>, 
	Benson Leung <bleung@...omium.org>, Tzung-Bi Shih <tzungbi@...nel.org>, 
	Daniel Lezcano <daniel.lezcano@...aro.org>, Matti Vaittinen <mazziesaccount@...il.com>, 
	kernel@...gutronix.de, linux-kernel@...r.kernel.org, 
	Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, 
	"Rafael J. Wysocki" <rafael@...nel.org>, Zhang Rui <rui.zhang@...el.com>, 
	Lukasz Luba <lukasz.luba@....com>, linux-pm@...r.kernel.org, 
	Søren Andersen <san@...v.dk>, Guenter Roeck <groeck@...omium.org>, 
	Ahmad Fatoum <a.fatoum@...gutronix.de>, Andrew Morton <akpm@...ux-foundation.org>, 
	chrome-platform@...ts.linux.dev
Subject: Re: [PATCH v9 3/7] power: reset: Introduce PSCR Recording Framework
 for Non-Volatile Storage

Hi,

On Tue, Apr 22, 2025 at 10:57:13AM +0200, Oleksij Rempel wrote:
> This commit introduces the Power State Change Reasons Recording (PSCRR)
> framework. It provides a generic mechanism to store shutdown or reboot
> reasons, such as under-voltage, thermal events, or software-triggered
> actions, into non-volatile storage.
> 
> PSCRR is primarily intended for systems where software is able to detect
> a power event in time and store the reason—typically when backup power
> (e.g., capacitors) allows a short window before shutdown. This enables
> reliable postmortem diagnostics, even on devices where traditional storage
> like eMMC or NAND may not survive abrupt power loss.
> 
> In its current form, PSCRR focuses on software-reported reasons. However,
> the framework is also designed with future extensibility in mind and could
> serve as a central frontend for exposing hardware-reported reset reasons
> from sources such as PMICs, watchdogs, or SoC-specific registers.
> 
> This version does not yet integrate such hardware-based reporting.

This adds quite some complex code for a very specific problem while
mostly ignoring the common case of hardware reported reasons. I think
having at least one hardware-reported reasons (e.g. WDOG) is mandatory
to make sure the framework can really handle it.

I also see you extended power_on_reason.h and included it here, but
don't actually use the defined strings at all. Also you create the
new enum, but don't handle the existing reasons and just add the new
codes you need instead of making sure things are properly in-sync :(

> [...]

> +struct pscrr_core {
> +	struct mutex lock;
> +	struct pscrr_backend *backend;
> +	/* Kobject for sysfs */
> +	struct kobject *kobj;
> +	struct notifier_block reboot_nb;
> +} g_pscrr = {
> +	.lock = __MUTEX_INITIALIZER(g_pscrr.lock),
> +};

Apart from the highlevel problems I have with this:

g_pscrr can be static

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)