[PATCH v2 0/2] firmware/psci: Switch to the sys-off handler API

[PATCH v2 0/2] firmware/psci: Switch to the sys-off handler API

[Samuel Holland]

I want to convert the axp20x PMIC poweroff handler to use the sys-off
API, so it can be used as a fallback for the SBI poweroff handler on
RISC-V. But the PSCI poweroff handler still uses pm_power_off, so done
alone, this conversion would cause the axp20x callback to be called
first, before the PSCI poweroff handler.

In order to prevent this change in behavior, the PSCI poweroff handler
needs to be converted to the sys-off API first, at a higher priority.

This series performs the conversion, after accounting for the fact that
the PSCI poweroff handler is registered quite early during boot.

The first patch is a dependency for both this series and the SBI
series[1], so I would like to get at least patch 1 merged soon.

[1]: https://lore.kernel.org/lkml/20221228161915.13194-1-samuel@sholland.org/

Changes in v2:
 - Update commit messages

Samuel Holland (2):
  kernel/reboot: Use the static sys-off handler for any priority
  firmware/psci: Switch to the sys-off handler API

 drivers/firmware/psci/psci.c |  9 ++++++---
 kernel/reboot.c              | 10 ++++------
 2 files changed, 10 insertions(+), 9 deletions(-)

-- 
2.37.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/2] kernel/reboot: Use the static sys-off handler for any priority

[Samuel Holland]

commit 587b9bfe0668 ("kernel/reboot: Use static handler for
register_platform_power_off()") addded a statically-allocated handler
so register_sys_off_handler() could be called before the slab allocator
is available.

That behavior was limited to the SYS_OFF_PRIO_PLATFORM priority.
However, it is also required for handlers such as PSCI on ARM and SBI on
RISC-V, which should be registered at SYS_OFF_PRIO_FIRMWARE. Currently,
this call stack crashes:

  start_kernel()
    setup_arch()
      psci_dt_init()
        psci_0_2_init()
          register_sys_off_handler()
            kmem_cache_alloc()

Generalize the code to use the statically-allocated handler for the
first registration, regardless of priority. Check .sys_off_cb for
conflicts instead of .cb_data; some callbacks (e.g. firmware drivers)
do not need any per-instance data, so .cb_data could be NULL.

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - Update commit messages

 kernel/reboot.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/kernel/reboot.c b/kernel/reboot.c
index 3bba88c7ffc6..38c18d4f0a36 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -327,7 +327,7 @@ static int sys_off_notify(struct notifier_block *nb,
 	return handler->sys_off_cb(&data);
 }
 
-static struct sys_off_handler platform_sys_off_handler;
+static struct sys_off_handler early_sys_off_handler;
 
 static struct sys_off_handler *alloc_sys_off_handler(int priority)
 {
@@ -338,10 +338,8 @@ static struct sys_off_handler *alloc_sys_off_handler(int priority)
 	 * Platforms like m68k can't allocate sys_off handler dynamically
 	 * at the early boot time because memory allocator isn't available yet.
 	 */
-	if (priority == SYS_OFF_PRIO_PLATFORM) {
-		handler = &platform_sys_off_handler;
-		if (handler->cb_data)
-			return ERR_PTR(-EBUSY);
+	if (!early_sys_off_handler.sys_off_cb) {
+		handler = &early_sys_off_handler;
 	} else {
 		if (system_state > SYSTEM_RUNNING)
 			flags = GFP_ATOMIC;
@@ -358,7 +356,7 @@ static struct sys_off_handler *alloc_sys_off_handler(int priority)
 
 static void free_sys_off_handler(struct sys_off_handler *handler)
 {
-	if (handler == &platform_sys_off_handler)
+	if (handler == &early_sys_off_handler)
 		memset(handler, 0, sizeof(*handler));
 	else
 		kfree(handler);
-- 
2.37.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/2] firmware/psci: Switch to the sys-off handler API

[Samuel Holland]

Any other poweroff handlers registered at the default priority will
run before the legacy pm_power_off() function. Register the PSCI
poweroff handler with the correct priority to ensure it runs first.

PSCI_0_2_FN_SYSTEM_OFF never returns, so the value returned from
psci_sys_poweroff() is meaningless, but that function must return
some value to have the right prototype for a notifier callback.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - Update commit messages

 drivers/firmware/psci/psci.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index e7bcfca4159f..6d528021925d 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -13,7 +13,6 @@
 #include <linux/errno.h>
 #include <linux/linkage.h>
 #include <linux/of.h>
-#include <linux/pm.h>
 #include <linux/printk.h>
 #include <linux/psci.h>
 #include <linux/reboot.h>
@@ -322,9 +321,11 @@ static struct notifier_block psci_sys_reset_nb = {
 	.priority = 129,
 };
 
-static void psci_sys_poweroff(void)
+static int psci_sys_poweroff(struct sys_off_data *data)
 {
 	invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
+
+	return NOTIFY_DONE;
 }
 
 static int psci_features(u32 psci_func_id)
@@ -603,7 +604,9 @@ static void __init psci_0_2_set_functions(void)
 
 	register_restart_handler(&psci_sys_reset_nb);
 
-	pm_power_off = psci_sys_poweroff;
+	register_sys_off_handler(SYS_OFF_MODE_POWER_OFF,
+				 SYS_OFF_PRIO_FIRMWARE,
+				 psci_sys_poweroff, NULL);
 }
 
 /*
-- 
2.37.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/2] firmware/psci: Switch to the sys-off handler API

[Sudeep Holla]

On Sun, Jan 01, 2023 at 12:17:15PM -0600, Samuel Holland wrote:
> Any other poweroff handlers registered at the default priority will
> run before the legacy pm_power_off() function. Register the PSCI
> poweroff handler with the correct priority to ensure it runs first.
>
> PSCI_0_2_FN_SYSTEM_OFF never returns, so the value returned from
> psci_sys_poweroff() is meaningless, but that function must return
> some value to have the right prototype for a notifier callback.
>

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Tested-by: Sudeep Holla <sudeep.holla@arm.com>

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/2] firmware/psci: Switch to the sys-off handler API

[Dmitry Osipenko]

On 1/1/23 21:17, Samuel Holland wrote:
> I want to convert the axp20x PMIC poweroff handler to use the sys-off
> API, so it can be used as a fallback for the SBI poweroff handler on
> RISC-V. But the PSCI poweroff handler still uses pm_power_off, so done
> alone, this conversion would cause the axp20x callback to be called
> first, before the PSCI poweroff handler.
> 
> In order to prevent this change in behavior, the PSCI poweroff handler
> needs to be converted to the sys-off API first, at a higher priority.
> 
> This series performs the conversion, after accounting for the fact that
> the PSCI poweroff handler is registered quite early during boot.
> 
> The first patch is a dependency for both this series and the SBI
> series[1], so I would like to get at least patch 1 merged soon.
> 
> [1]: https://lore.kernel.org/lkml/20221228161915.13194-1-samuel@sholland.org/
> 
> Changes in v2:
>  - Update commit messages
> 
> Samuel Holland (2):
>   kernel/reboot: Use the static sys-off handler for any priority
>   firmware/psci: Switch to the sys-off handler API
> 
>  drivers/firmware/psci/psci.c |  9 ++++++---
>  kernel/reboot.c              | 10 ++++------
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 

Hello Rafael,

Do you think you will be able to pick up this series for 6.3? I'm going
to continue removing the pm_power_off from kernel, the new power-off API
feels stable now to me. I think the Samuel's improvement for the early
boot memory allocation will be good to have to avoid similar problem for
other drivers.

Ideally, the PSCI patch should get an ack, though the code change is
about the PM stuff, so perhaps will be fine to take it via PM tree if FW
maintainers will show no interest in the nearest time.

Thanks!

-- 
Best regards,
Dmitry


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/2] firmware/psci: Switch to the sys-off handler API

[Samuel Holland]

On 1/25/23 06:53, Dmitry Osipenko wrote:
> On 1/1/23 21:17, Samuel Holland wrote:
>> I want to convert the axp20x PMIC poweroff handler to use the sys-off
>> API, so it can be used as a fallback for the SBI poweroff handler on
>> RISC-V. But the PSCI poweroff handler still uses pm_power_off, so done
>> alone, this conversion would cause the axp20x callback to be called
>> first, before the PSCI poweroff handler.
>>
>> In order to prevent this change in behavior, the PSCI poweroff handler
>> needs to be converted to the sys-off API first, at a higher priority.
>>
>> This series performs the conversion, after accounting for the fact that
>> the PSCI poweroff handler is registered quite early during boot.
>>
>> The first patch is a dependency for both this series and the SBI
>> series[1], so I would like to get at least patch 1 merged soon.
>>
>> [1]: https://lore.kernel.org/lkml/20221228161915.13194-1-samuel@sholland.org/
>>
>> Changes in v2:
>>  - Update commit messages
>>
>> Samuel Holland (2):
>>   kernel/reboot: Use the static sys-off handler for any priority
>>   firmware/psci: Switch to the sys-off handler API
>>
>>  drivers/firmware/psci/psci.c |  9 ++++++---
>>  kernel/reboot.c              | 10 ++++------
>>  2 files changed, 10 insertions(+), 9 deletions(-)
>>
> 
> Hello Rafael,
> 
> Do you think you will be able to pick up this series for 6.3? I'm going
> to continue removing the pm_power_off from kernel, the new power-off API
> feels stable now to me. I think the Samuel's improvement for the early
> boot memory allocation will be good to have to avoid similar problem for
> other drivers.
> 
> Ideally, the PSCI patch should get an ack, though the code change is
> about the PM stuff, so perhaps will be fine to take it via PM tree if FW
> maintainers will show no interest in the nearest time.

Additionally, a patch which depends on this[1] series has already been
merged[2], so if this series does not make 6.3, that change would need
to be reverted.

Regards,
Samuel

[1]:
https://lore.kernel.org/lkml/20221228162752.14204-1-samuel@sholland.org/
[2]: https://git.kernel.org/next/linux-next/c/a4755a1374ba


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel