public inbox for linux-riscv@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API
@ 2022-12-28 16:19 Samuel Holland
  2022-12-28 16:19 ` [PATCH 1/3] kernel/reboot: Use the static sys-off handler for any priority Samuel Holland
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Samuel Holland @ 2022-12-28 16:19 UTC (permalink / raw)
  To: Palmer Dabbelt, Dmitry Osipenko, Rafael J . Wysocki
  Cc: Samuel Holland, Albert Ou, Anup Patel, Atish Patra,
	Geert Uytterhoeven, Heiko Stuebner, Kai-Heng Feng,
	Luis Chamberlain, Paul E. McKenney, Paul Walmsley, Petr Mladek,
	YueHaibing, linux-kernel, linux-riscv, tangmeng

I want to convert the axp20x PMIC poweroff handler to use the sys-off
API, so it can be used as a fallback for if the SBI poweroff handler
is unavailable. But the SBI poweroff handler still uses pm_power_off, so
done alone, this would cause the axp20x callback to be called first,
before the SBI poweroff handler has a chance to run.

In order to prevent this change in behavior, the SBI 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 SBI poweroff handler is registered quite early during boot.

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

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


Samuel Holland (3):
  kernel/reboot: Use the static sys-off handler for any priority
  riscv: sbi: Share the code for unsupported extension warnings
  riscv: sbi: Switch to the sys-off handler API

 arch/riscv/include/asm/sbi.h |  1 -
 arch/riscv/kernel/sbi.c      | 63 +++++++++++++++++++++---------------
 kernel/reboot.c              | 10 +++---
 3 files changed, 41 insertions(+), 33 deletions(-)

-- 
2.37.4


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] kernel/reboot: Use the static sys-off handler for any priority
  2022-12-28 16:19 [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API Samuel Holland
@ 2022-12-28 16:19 ` Samuel Holland
  2023-02-15  0:17   ` Palmer Dabbelt
  2022-12-28 16:19 ` [PATCH 2/3] riscv: sbi: Share the code for unsupported extension warnings Samuel Holland
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Samuel Holland @ 2022-12-28 16:19 UTC (permalink / raw)
  To: Palmer Dabbelt, Dmitry Osipenko, Rafael J . Wysocki
  Cc: Samuel Holland, Albert Ou, Anup Patel, Atish Patra,
	Geert Uytterhoeven, Heiko Stuebner, Kai-Heng Feng,
	Luis Chamberlain, Paul E. McKenney, Paul Walmsley, Petr Mladek,
	YueHaibing, linux-kernel, linux-riscv, tangmeng

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>
---

 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-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/3] riscv: sbi: Share the code for unsupported extension warnings
  2022-12-28 16:19 [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API Samuel Holland
  2022-12-28 16:19 ` [PATCH 1/3] kernel/reboot: Use the static sys-off handler for any priority Samuel Holland
@ 2022-12-28 16:19 ` Samuel Holland
  2023-01-03  8:54   ` Geert Uytterhoeven
  2022-12-28 16:19 ` [PATCH 3/3] riscv: sbi: Switch to the sys-off handler API Samuel Holland
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Samuel Holland @ 2022-12-28 16:19 UTC (permalink / raw)
  To: Palmer Dabbelt, Dmitry Osipenko, Rafael J . Wysocki
  Cc: Samuel Holland, Albert Ou, Anup Patel, Atish Patra,
	Geert Uytterhoeven, Heiko Stuebner, Kai-Heng Feng,
	Luis Chamberlain, Paul E. McKenney, Paul Walmsley, Petr Mladek,
	YueHaibing, linux-kernel, linux-riscv, tangmeng

This reduces the code size by sharing most of the code and the format
string across all affected extensions.

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

 arch/riscv/kernel/sbi.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
index 5c87db8fdff2..1196c12299f6 100644
--- a/arch/riscv/kernel/sbi.c
+++ b/arch/riscv/kernel/sbi.c
@@ -210,16 +210,20 @@ static void sbi_set_power_off(void)
 	pm_power_off = sbi_shutdown;
 }
 #else
+static void __sbi_warn_unsupported(const char *extension)
+{
+	pr_warn("%s extension is not available in SBI v%lu.%lu\n",
+		extension, sbi_major_version(), sbi_minor_version());
+}
+
 static void __sbi_set_timer_v01(uint64_t stime_value)
 {
-	pr_warn("Timer extension is not available in SBI v%lu.%lu\n",
-		sbi_major_version(), sbi_minor_version());
+	__sbi_warn_unsupported("Timer");
 }
 
 static int __sbi_send_ipi_v01(const struct cpumask *cpu_mask)
 {
-	pr_warn("IPI extension is not available in SBI v%lu.%lu\n",
-		sbi_major_version(), sbi_minor_version());
+	__sbi_warn_unsupported("IPI");
 
 	return 0;
 }
@@ -228,8 +232,7 @@ static int __sbi_rfence_v01(int fid, const struct cpumask *cpu_mask,
 			    unsigned long start, unsigned long size,
 			    unsigned long arg4, unsigned long arg5)
 {
-	pr_warn("remote fence extension is not available in SBI v%lu.%lu\n",
-		sbi_major_version(), sbi_minor_version());
+	__sbi_warn_unsupported("Remote fence");
 
 	return 0;
 }
-- 
2.37.4


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/3] riscv: sbi: Switch to the sys-off handler API
  2022-12-28 16:19 [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API Samuel Holland
  2022-12-28 16:19 ` [PATCH 1/3] kernel/reboot: Use the static sys-off handler for any priority Samuel Holland
  2022-12-28 16:19 ` [PATCH 2/3] riscv: sbi: Share the code for unsupported extension warnings Samuel Holland
@ 2022-12-28 16:19 ` Samuel Holland
  2022-12-28 18:30 ` [PATCH 0/3] " Conor Dooley
  2023-05-11 20:53 ` Palmer Dabbelt
  4 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2022-12-28 16:19 UTC (permalink / raw)
  To: Palmer Dabbelt, Dmitry Osipenko, Rafael J . Wysocki
  Cc: Samuel Holland, Albert Ou, Anup Patel, Atish Patra,
	Geert Uytterhoeven, Heiko Stuebner, Kai-Heng Feng,
	Luis Chamberlain, Paul E. McKenney, Paul Walmsley, Petr Mladek,
	YueHaibing, linux-kernel, linux-riscv, tangmeng

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

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

 arch/riscv/include/asm/sbi.h |  1 -
 arch/riscv/kernel/sbi.c      | 52 +++++++++++++++++++++---------------
 2 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 4ca7fbacff42..2915a3ddf7c6 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -267,7 +267,6 @@ long sbi_get_mvendorid(void);
 long sbi_get_marchid(void);
 long sbi_get_mimpid(void);
 void sbi_set_timer(uint64_t stime_value);
-void sbi_shutdown(void);
 void sbi_clear_ipi(void);
 int sbi_send_ipi(const struct cpumask *cpu_mask);
 int sbi_remote_fence_i(const struct cpumask *cpu_mask);
diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
index 1196c12299f6..9574851eae6e 100644
--- a/arch/riscv/kernel/sbi.c
+++ b/arch/riscv/kernel/sbi.c
@@ -7,7 +7,6 @@
 
 #include <linux/bits.h>
 #include <linux/init.h>
-#include <linux/pm.h>
 #include <linux/reboot.h>
 #include <asm/sbi.h>
 #include <asm/smp.h>
@@ -120,15 +119,16 @@ int sbi_console_getchar(void)
 EXPORT_SYMBOL(sbi_console_getchar);
 
 /**
- * sbi_shutdown() - Remove all the harts from executing supervisor code.
+ * __sbi_shutdown_v01() - Remove all the harts from executing supervisor code.
  *
- * Return: None
+ * Return: NOTIFY_DONE
  */
-void sbi_shutdown(void)
+static int __sbi_shutdown_v01(struct sys_off_data *data)
 {
 	sbi_ecall(SBI_EXT_0_1_SHUTDOWN, 0, 0, 0, 0, 0, 0, 0);
+
+	return NOTIFY_DONE;
 }
-EXPORT_SYMBOL(sbi_shutdown);
 
 /**
  * sbi_clear_ipi() - Clear any pending IPIs for the calling hart.
@@ -204,11 +204,6 @@ static int __sbi_rfence_v01(int fid, const struct cpumask *cpu_mask,
 
 	return result;
 }
-
-static void sbi_set_power_off(void)
-{
-	pm_power_off = sbi_shutdown;
-}
 #else
 static void __sbi_warn_unsupported(const char *extension)
 {
@@ -216,6 +211,13 @@ static void __sbi_warn_unsupported(const char *extension)
 		extension, sbi_major_version(), sbi_minor_version());
 }
 
+static int __sbi_shutdown_v01(struct sys_off_data *data)
+{
+	__sbi_warn_unsupported("Shutdown");
+
+	return NOTIFY_DONE;
+}
+
 static void __sbi_set_timer_v01(uint64_t stime_value)
 {
 	__sbi_warn_unsupported("Timer");
@@ -236,8 +238,6 @@ static int __sbi_rfence_v01(int fid, const struct cpumask *cpu_mask,
 
 	return 0;
 }
-
-static void sbi_set_power_off(void) {}
 #endif /* CONFIG_RISCV_SBI_V01 */
 
 static void __sbi_set_timer_v02(uint64_t stime_value)
@@ -574,10 +574,12 @@ static int sbi_srst_reboot(struct notifier_block *this,
 
 static struct notifier_block sbi_srst_reboot_nb;
 
-static void sbi_srst_power_off(void)
+static int sbi_srst_power_off(struct sys_off_data *data)
 {
 	sbi_srst_reset(SBI_SRST_RESET_TYPE_SHUTDOWN,
 		       SBI_SRST_RESET_REASON_NONE);
+
+	return NOTIFY_DONE;
 }
 
 /**
@@ -657,7 +659,6 @@ void __init sbi_init(void)
 {
 	int ret;
 
-	sbi_set_power_off();
 	ret = sbi_get_spec_version();
 	if (ret > 0)
 		sbi_spec_version = ret;
@@ -686,19 +687,26 @@ void __init sbi_init(void)
 		} else {
 			__sbi_rfence	= __sbi_rfence_v01;
 		}
-		if ((sbi_spec_version >= sbi_mk_version(0, 3)) &&
-		    (sbi_probe_extension(SBI_EXT_SRST) > 0)) {
-			pr_info("SBI SRST extension detected\n");
-			pm_power_off = sbi_srst_power_off;
-			sbi_srst_reboot_nb.notifier_call = sbi_srst_reboot;
-			sbi_srst_reboot_nb.priority = 192;
-			register_restart_handler(&sbi_srst_reboot_nb);
-		}
 	} else {
 		__sbi_set_timer = __sbi_set_timer_v01;
 		__sbi_send_ipi	= __sbi_send_ipi_v01;
 		__sbi_rfence	= __sbi_rfence_v01;
 	}
 
+	if ((sbi_spec_version >= sbi_mk_version(0, 3)) &&
+	    (sbi_probe_extension(SBI_EXT_SRST) > 0)) {
+		pr_info("SBI SRST extension detected\n");
+		sbi_srst_reboot_nb.notifier_call = sbi_srst_reboot;
+		sbi_srst_reboot_nb.priority = 192;
+		register_restart_handler(&sbi_srst_reboot_nb);
+		register_sys_off_handler(SYS_OFF_MODE_POWER_OFF,
+					 SYS_OFF_PRIO_FIRMWARE,
+					 sbi_srst_power_off, NULL);
+	} else {
+		register_sys_off_handler(SYS_OFF_MODE_POWER_OFF,
+					 SYS_OFF_PRIO_FIRMWARE,
+					 __sbi_shutdown_v01, NULL);
+	}
+
 	riscv_set_ipi_ops(&sbi_ipi_ops);
 }
-- 
2.37.4


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API
  2022-12-28 16:19 [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API Samuel Holland
                   ` (2 preceding siblings ...)
  2022-12-28 16:19 ` [PATCH 3/3] riscv: sbi: Switch to the sys-off handler API Samuel Holland
@ 2022-12-28 18:30 ` Conor Dooley
  2023-01-03 10:54   ` Conor Dooley
  2023-05-12 21:50   ` Palmer Dabbelt
  2023-05-11 20:53 ` Palmer Dabbelt
  4 siblings, 2 replies; 14+ messages in thread
From: Conor Dooley @ 2022-12-28 18:30 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Palmer Dabbelt, Dmitry Osipenko, Rafael J . Wysocki, Albert Ou,
	Anup Patel, Atish Patra, Geert Uytterhoeven, Heiko Stuebner,
	Kai-Heng Feng, Luis Chamberlain, Paul E. McKenney, Paul Walmsley,
	Petr Mladek, YueHaibing, linux-kernel, linux-riscv, tangmeng


[-- Attachment #1.1: Type: text/plain, Size: 1355 bytes --]

Hey Samuel,

On Wed, Dec 28, 2022 at 10:19:12AM -0600, 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 if the SBI poweroff handler
> is unavailable. But the SBI poweroff handler still uses pm_power_off, so
> done alone, this would cause the axp20x callback to be called first,
> before the SBI poweroff handler has a chance to run.
> 
> In order to prevent this change in behavior, the SBI 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 SBI poweroff handler is registered quite early during boot.
> 
> The first patch is a dependency for both this series and the PSCI
> series[1], so I would like to get at least patch 1 merged soon.
> 
> [1]: https://lore.kernel.org/lkml/20221105214841.7828-1-samuel@sholland.org/
> 
> 
> Samuel Holland (3):
>   kernel/reboot: Use the static sys-off handler for any priority
>   riscv: sbi: Share the code for unsupported extension warnings
>   riscv: sbi: Switch to the sys-off handler API

Not what other stuff has reboot support, so I gave it a whirl on
PolarFire SoC & it seemed to work as expected:
Tested-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] riscv: sbi: Share the code for unsupported extension warnings
  2022-12-28 16:19 ` [PATCH 2/3] riscv: sbi: Share the code for unsupported extension warnings Samuel Holland
@ 2023-01-03  8:54   ` Geert Uytterhoeven
  0 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2023-01-03  8:54 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Palmer Dabbelt, Dmitry Osipenko, Rafael J . Wysocki, Albert Ou,
	Anup Patel, Atish Patra, Heiko Stuebner, Kai-Heng Feng,
	Luis Chamberlain, Paul E. McKenney, Paul Walmsley, Petr Mladek,
	YueHaibing, linux-kernel, linux-riscv, tangmeng

On Wed, Dec 28, 2022 at 5:19 PM Samuel Holland <samuel@sholland.org> wrote:
> This reduces the code size by sharing most of the code and the format
> string across all affected extensions.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API
  2022-12-28 18:30 ` [PATCH 0/3] " Conor Dooley
@ 2023-01-03 10:54   ` Conor Dooley
  2023-05-12 21:50   ` Palmer Dabbelt
  1 sibling, 0 replies; 14+ messages in thread
From: Conor Dooley @ 2023-01-03 10:54 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Palmer Dabbelt, Dmitry Osipenko, Rafael J . Wysocki, Albert Ou,
	Anup Patel, Atish Patra, Geert Uytterhoeven, Heiko Stuebner,
	Kai-Heng Feng, Luis Chamberlain, Paul E. McKenney, Paul Walmsley,
	Petr Mladek, YueHaibing, linux-kernel, linux-riscv, tangmeng


[-- Attachment #1.1: Type: text/plain, Size: 1589 bytes --]

On Wed, Dec 28, 2022 at 06:30:11PM +0000, Conor Dooley wrote:
> Hey Samuel,
> 
> On Wed, Dec 28, 2022 at 10:19:12AM -0600, 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 if the SBI poweroff handler
> > is unavailable. But the SBI poweroff handler still uses pm_power_off, so
> > done alone, this would cause the axp20x callback to be called first,
> > before the SBI poweroff handler has a chance to run.
> > 
> > In order to prevent this change in behavior, the SBI 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 SBI poweroff handler is registered quite early during boot.
> > 
> > The first patch is a dependency for both this series and the PSCI
> > series[1], so I would like to get at least patch 1 merged soon.
> > 
> > [1]: https://lore.kernel.org/lkml/20221105214841.7828-1-samuel@sholland.org/
> > 
> > 
> > Samuel Holland (3):
> >   kernel/reboot: Use the static sys-off handler for any priority
> >   riscv: sbi: Share the code for unsupported extension warnings
> >   riscv: sbi: Switch to the sys-off handler API
> 
> Not what other stuff has reboot support, so I gave it a whirl on

I happened to see this today and could barely understand what I wrote.
s/Not what/Not sure what"

> PolarFire SoC & it seemed to work as expected:
> Tested-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Thanks,
> Conor.



[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] kernel/reboot: Use the static sys-off handler for any priority
  2022-12-28 16:19 ` [PATCH 1/3] kernel/reboot: Use the static sys-off handler for any priority Samuel Holland
@ 2023-02-15  0:17   ` Palmer Dabbelt
  2023-02-18 23:20     ` Samuel Holland
  0 siblings, 1 reply; 14+ messages in thread
From: Palmer Dabbelt @ 2023-02-15  0:17 UTC (permalink / raw)
  To: samuel
  Cc: dmitry.osipenko, rafael.j.wysocki, samuel, aou, apatel,
	Atish Patra, geert, heiko, kai.heng.feng, mcgrof, paulmck,
	Paul Walmsley, pmladek, yuehaibing, linux-kernel, linux-riscv,
	tangmeng

On Wed, 28 Dec 2022 08:19:13 PST (-0800), samuel@sholland.org wrote:
> 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>
> ---
>
>  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);

Sorry for being slow here, I'd been assuming someone would Ack this but 
it looks like maybe there's nobody in the maintainers file for 
kernel/reboot.c?  I'm fine taking this via the RISC-V tree if that's OK 
with people, but the cover letter suggests the patch is necessary for 
multiple patch sets.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] kernel/reboot: Use the static sys-off handler for any priority
  2023-02-15  0:17   ` Palmer Dabbelt
@ 2023-02-18 23:20     ` Samuel Holland
  2023-02-18 23:32       ` Dmitry Osipenko
  0 siblings, 1 reply; 14+ messages in thread
From: Samuel Holland @ 2023-02-18 23:20 UTC (permalink / raw)
  To: Palmer Dabbelt, rafael.j.wysocki
  Cc: dmitry.osipenko, aou, apatel, Atish Patra, geert, heiko,
	kai.heng.feng, mcgrof, paulmck, Paul Walmsley, pmladek,
	yuehaibing, linux-kernel, linux-riscv, tangmeng

On 2/14/23 18:17, Palmer Dabbelt wrote:
> On Wed, 28 Dec 2022 08:19:13 PST (-0800), samuel@sholland.org wrote:
>> 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>
>> ---
>>
>>  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);
> 
> Sorry for being slow here, I'd been assuming someone would Ack this but
> it looks like maybe there's nobody in the maintainers file for
> kernel/reboot.c?  I'm fine taking this via the RISC-V tree if that's OK
> with people, but the cover letter suggests the patch is necessary for
> multiple patch sets.

See also Dmitry's reply[0] to the PSCI thread. (Maybe I should have sent
both conversions as one series?)

I am happy with the patches going through any tree. The kernel/reboot.c
patch is exactly the same between the two series, so it should not hurt
if it gets merged twice. Though if you take this series through the
RISC-V tree, maybe you want to create a tag for it?

I am not sure exactly what needs to be done here; I am happy to do
anything that would assist getting both series merged for v6.3, to avoid
a regression with axp20x[1].

Regards,
Samuel

[0]:
https://lore.kernel.org/lkml/0a180849-ba1b-2a82-ab06-ed1b8155d5ca@collabora.com/
[1]:
https://lore.kernel.org/lkml/e38d29f5-cd3c-4a2b-b355-2bcfad00a24b@sholland.org/

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] kernel/reboot: Use the static sys-off handler for any priority
  2023-02-18 23:20     ` Samuel Holland
@ 2023-02-18 23:32       ` Dmitry Osipenko
  2023-03-15  3:10         ` Palmer Dabbelt
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Osipenko @ 2023-02-18 23:32 UTC (permalink / raw)
  To: Samuel Holland, Palmer Dabbelt, rafael.j.wysocki
  Cc: aou, apatel, Atish Patra, geert, heiko, kai.heng.feng, mcgrof,
	paulmck, Paul Walmsley, pmladek, yuehaibing, linux-kernel,
	linux-riscv, tangmeng

On 2/19/23 02:20, Samuel Holland wrote:
> On 2/14/23 18:17, Palmer Dabbelt wrote:
...
>> Sorry for being slow here, I'd been assuming someone would Ack this but
>> it looks like maybe there's nobody in the maintainers file for
>> kernel/reboot.c?  I'm fine taking this via the RISC-V tree if that's OK
>> with people, but the cover letter suggests the patch is necessary for
>> multiple patch sets.
> 
> See also Dmitry's reply[0] to the PSCI thread. (Maybe I should have sent
> both conversions as one series?)
> 
> I am happy with the patches going through any tree. The kernel/reboot.c
> patch is exactly the same between the two series, so it should not hurt
> if it gets merged twice. Though if you take this series through the
> RISC-V tree, maybe you want to create a tag for it?
> 
> I am not sure exactly what needs to be done here; I am happy to do
> anything that would assist getting both series merged for v6.3, to avoid
> a regression with axp20x[1].
> 
> Regards,
> Samuel
> 
> [0]:
> https://lore.kernel.org/lkml/0a180849-ba1b-2a82-ab06-ed1b8155d5ca@collabora.com/
> [1]:
> https://lore.kernel.org/lkml/e38d29f5-cd3c-4a2b-b355-2bcfad00a24b@sholland.org/

The reboot.c changes should be acked/applied by Rafael.
I noticed that you haven't CC'd the linux-pm ML, maybe that's why it
hasn't got the attention.

-- 
Best regards,
Dmitry


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] kernel/reboot: Use the static sys-off handler for any priority
  2023-02-18 23:32       ` Dmitry Osipenko
@ 2023-03-15  3:10         ` Palmer Dabbelt
  0 siblings, 0 replies; 14+ messages in thread
From: Palmer Dabbelt @ 2023-03-15  3:10 UTC (permalink / raw)
  To: dmitry.osipenko, linux-pm, rafael
  Cc: samuel, rafael.j.wysocki, aou, apatel, Atish Patra, geert, heiko,
	kai.heng.feng, mcgrof, paulmck, Paul Walmsley, pmladek,
	yuehaibing, linux-kernel, linux-riscv, tangmeng

On Sat, 18 Feb 2023 15:32:06 PST (-0800), dmitry.osipenko@collabora.com wrote:
> On 2/19/23 02:20, Samuel Holland wrote:
>> On 2/14/23 18:17, Palmer Dabbelt wrote:
> ...
>>> Sorry for being slow here, I'd been assuming someone would Ack this but
>>> it looks like maybe there's nobody in the maintainers file for
>>> kernel/reboot.c?  I'm fine taking this via the RISC-V tree if that's OK
>>> with people, but the cover letter suggests the patch is necessary for
>>> multiple patch sets.
>>
>> See also Dmitry's reply[0] to the PSCI thread. (Maybe I should have sent
>> both conversions as one series?)
>>
>> I am happy with the patches going through any tree. The kernel/reboot.c
>> patch is exactly the same between the two series, so it should not hurt
>> if it gets merged twice. Though if you take this series through the
>> RISC-V tree, maybe you want to create a tag for it?
>>
>> I am not sure exactly what needs to be done here; I am happy to do
>> anything that would assist getting both series merged for v6.3, to avoid
>> a regression with axp20x[1].
>>
>> Regards,
>> Samuel
>>
>> [0]:
>> https://lore.kernel.org/lkml/0a180849-ba1b-2a82-ab06-ed1b8155d5ca@collabora.com/
>> [1]:
>> https://lore.kernel.org/lkml/e38d29f5-cd3c-4a2b-b355-2bcfad00a24b@sholland.org/
>
> The reboot.c changes should be acked/applied by Rafael.
> I noticed that you haven't CC'd the linux-pm ML, maybe that's why it
> hasn't got the attention.

OK, I'm adding them here.  Not sure if we're ment to re-send it to the 
list, no rush on my end I'm just scrubbing through some old stuff on 
patchwork again.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API
  2022-12-28 16:19 [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API Samuel Holland
                   ` (3 preceding siblings ...)
  2022-12-28 18:30 ` [PATCH 0/3] " Conor Dooley
@ 2023-05-11 20:53 ` Palmer Dabbelt
  4 siblings, 0 replies; 14+ messages in thread
From: Palmer Dabbelt @ 2023-05-11 20:53 UTC (permalink / raw)
  To: samuel
  Cc: dmitry.osipenko, rafael.j.wysocki, samuel, aou, apatel,
	Atish Patra, geert, heiko, kai.heng.feng, mcgrof, paulmck,
	Paul Walmsley, pmladek, yuehaibing, linux-kernel, linux-riscv,
	tangmeng

On Wed, 28 Dec 2022 08:19:12 PST (-0800), samuel@sholland.org 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 if the SBI poweroff handler
> is unavailable. But the SBI poweroff handler still uses pm_power_off, so
> done alone, this would cause the axp20x callback to be called first,
> before the SBI poweroff handler has a chance to run.
>
> In order to prevent this change in behavior, the SBI 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 SBI poweroff handler is registered quite early during boot.
>
> The first patch is a dependency for both this series and the PSCI
> series[1], so I would like to get at least patch 1 merged soon.
>
> [1]: https://lore.kernel.org/lkml/20221105214841.7828-1-samuel@sholland.org/
>
>
> Samuel Holland (3):
>   kernel/reboot: Use the static sys-off handler for any priority
>   riscv: sbi: Share the code for unsupported extension warnings
>   riscv: sbi: Switch to the sys-off handler API
>
>  arch/riscv/include/asm/sbi.h |  1 -
>  arch/riscv/kernel/sbi.c      | 63 +++++++++++++++++++++---------------
>  kernel/reboot.c              | 10 +++---
>  3 files changed, 41 insertions(+), 33 deletions(-)

Samuel: do you mind rebasing this and resending it with the reboot folks 
in the To list?  That way we might be able to get an Ack on the generic 
bits.

Thanks!

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API
  2022-12-28 18:30 ` [PATCH 0/3] " Conor Dooley
  2023-01-03 10:54   ` Conor Dooley
@ 2023-05-12 21:50   ` Palmer Dabbelt
  2023-05-12 21:51     ` Palmer Dabbelt
  1 sibling, 1 reply; 14+ messages in thread
From: Palmer Dabbelt @ 2023-05-12 21:50 UTC (permalink / raw)
  To: Conor Dooley
  Cc: samuel, dmitry.osipenko, rafael.j.wysocki, aou, apatel,
	Atish Patra, geert, heiko, kai.heng.feng, mcgrof, paulmck,
	Paul Walmsley, pmladek, yuehaibing, linux-kernel, linux-riscv,
	tangmeng

On Wed, 28 Dec 2022 10:30:11 PST (-0800), Conor Dooley wrote:
> Hey Samuel,
>
> On Wed, Dec 28, 2022 at 10:19:12AM -0600, 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 if the SBI poweroff handler
>> is unavailable. But the SBI poweroff handler still uses pm_power_off, so
>> done alone, this would cause the axp20x callback to be called first,
>> before the SBI poweroff handler has a chance to run.
>> 
>> In order to prevent this change in behavior, the SBI 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 SBI poweroff handler is registered quite early during boot.
>> 
>> The first patch is a dependency for both this series and the PSCI
>> series[1], so I would like to get at least patch 1 merged soon.
>> 
>> [1]: https://lore.kernel.org/lkml/20221105214841.7828-1-samuel@sholland.org/
>> 
>> 
>> Samuel Holland (3):
>>   kernel/reboot: Use the static sys-off handler for any priority
>>   riscv: sbi: Share the code for unsupported extension warnings
>>   riscv: sbi: Switch to the sys-off handler API
>
> Not what other stuff has reboot support, so I gave it a whirl on
> PolarFire SoC & it seemed to work as expected:
> Tested-by: Conor Dooley <conor.dooley@microchip.com>

Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

in case the reboot folks want to take these.  I'm also happy to take the 
reboot change through the RISC-V tree with an Ack.  There's some 
discussion about this in the previous patches.

>
> Thanks,
> Conor.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API
  2023-05-12 21:50   ` Palmer Dabbelt
@ 2023-05-12 21:51     ` Palmer Dabbelt
  0 siblings, 0 replies; 14+ messages in thread
From: Palmer Dabbelt @ 2023-05-12 21:51 UTC (permalink / raw)
  To: Conor Dooley
  Cc: samuel, dmitry.osipenko, rafael.j.wysocki, aou, apatel,
	Atish Patra, geert, heiko, kai.heng.feng, mcgrof, paulmck,
	Paul Walmsley, pmladek, yuehaibing, linux-kernel, linux-riscv,
	tangmeng

On Fri, 12 May 2023 14:50:17 PDT (-0700), Palmer Dabbelt wrote:
> On Wed, 28 Dec 2022 10:30:11 PST (-0800), Conor Dooley wrote:
>> Hey Samuel,
>>
>> On Wed, Dec 28, 2022 at 10:19:12AM -0600, 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 if the SBI poweroff handler
>>> is unavailable. But the SBI poweroff handler still uses pm_power_off, so
>>> done alone, this would cause the axp20x callback to be called first,
>>> before the SBI poweroff handler has a chance to run.
>>>
>>> In order to prevent this change in behavior, the SBI 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 SBI poweroff handler is registered quite early during boot.
>>>
>>> The first patch is a dependency for both this series and the PSCI
>>> series[1], so I would like to get at least patch 1 merged soon.
>>>
>>> [1]: https://lore.kernel.org/lkml/20221105214841.7828-1-samuel@sholland.org/
>>>
>>>
>>> Samuel Holland (3):
>>>   kernel/reboot: Use the static sys-off handler for any priority
>>>   riscv: sbi: Share the code for unsupported extension warnings
>>>   riscv: sbi: Switch to the sys-off handler API
>>
>> Not what other stuff has reboot support, so I gave it a whirl on
>> PolarFire SoC & it seemed to work as expected:
>> Tested-by: Conor Dooley <conor.dooley@microchip.com>
>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
>
> in case the reboot folks want to take these.  I'm also happy to take the
> reboot change through the RISC-V tree with an Ack.  There's some
> discussion about this in the previous patches.

sorry, this is the old version -- I just got lost...

>
>>
>> Thanks,
>> Conor.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-05-12 21:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-28 16:19 [PATCH 0/3] riscv: sbi: Switch to the sys-off handler API Samuel Holland
2022-12-28 16:19 ` [PATCH 1/3] kernel/reboot: Use the static sys-off handler for any priority Samuel Holland
2023-02-15  0:17   ` Palmer Dabbelt
2023-02-18 23:20     ` Samuel Holland
2023-02-18 23:32       ` Dmitry Osipenko
2023-03-15  3:10         ` Palmer Dabbelt
2022-12-28 16:19 ` [PATCH 2/3] riscv: sbi: Share the code for unsupported extension warnings Samuel Holland
2023-01-03  8:54   ` Geert Uytterhoeven
2022-12-28 16:19 ` [PATCH 3/3] riscv: sbi: Switch to the sys-off handler API Samuel Holland
2022-12-28 18:30 ` [PATCH 0/3] " Conor Dooley
2023-01-03 10:54   ` Conor Dooley
2023-05-12 21:50   ` Palmer Dabbelt
2023-05-12 21:51     ` Palmer Dabbelt
2023-05-11 20:53 ` Palmer Dabbelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox