Re: [PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot
Thanks for looking into this patch Stewart Stewart Smith writes: > We're about to introduce an MPIPL reboot type (to take a firmware > assisted kdump style thing), and we maybe should have a reboot type to > force attempting a fast-reboot, and this makes me think if we should add > those in now? I will probably let Vasant and others answer that. > If the reboot type isn't supported, what should be the behvaiour? Reboot > the default way or don't reboot at all? Yes, I have addressed that in v3 of this patch at http://patchwork.ozlabs.org/patch/967248/. In case the reboot type isnt supported or if there is an error invoking it, the patch will revert back to calling opal_cec_reboot(). -- Vaibhav Jain Linux Technology Center, IBM India Pvt. Ltd.
Re: [PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot
Vaibhav Jain writes:
> Ever since fast reboot is enabled by default in opal,
> opal_cec_reboot() will use fast-reset instead of full IPL to perform
> system reboot. This leaves the user with no direct way to force a full
> IPL reboot except changing an nvram setting that persistently disables
> fast-reset for all subsequent reboots.
>
> This patch provides a more direct way for the user to force a one-shot
> full IPL reboot by passing the command line argument 'full' to the
> reboot command. So the user will be able to tweak the reboot behavior
> via:
>
> $ sudo reboot full # Force a full ipl reboot skipping fast-reset
>
> or
> $ sudo reboot # default reboot path (usually fast-reset)
>
> The reboot command passes the un-parsed command argument to the kernel
> via the 'Reboot' syscall which is then passed on to the arch function
> pnv_restart(). The patch updates pnv_restart() to handle this cmd-arg
> and issues opal_cec_reboot2 with OPAL_REBOOT_FULL_IPL to force a full
> IPL reset.
We're about to introduce an MPIPL reboot type (to take a firmware
assisted kdump style thing), and we maybe should have a reboot type to
force attempting a fast-reboot, and this makes me think if we should add
those in now?
>
> Signed-off-by: Vaibhav Jain
> ---
> arch/powerpc/include/asm/opal-api.h| 1 +
> arch/powerpc/platforms/powernv/setup.c | 8 +++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/opal-api.h
> b/arch/powerpc/include/asm/opal-api.h
> index 8365353330b4..870fb7b239ea 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -1050,6 +1050,7 @@ enum OpalSysCooling {
> enum {
> OPAL_REBOOT_NORMAL = 0,
> OPAL_REBOOT_PLATFORM_ERROR = 1,
> + OPAL_REBOOT_FULL_IPL= 2,
> };
>
> /* Argument to OPAL_PCI_TCE_KILL */
> diff --git a/arch/powerpc/platforms/powernv/setup.c
> b/arch/powerpc/platforms/powernv/setup.c
> index ae023622..33d2faeacff8 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -224,7 +224,13 @@ static void __noreturn pnv_restart(char *cmd)
> pnv_prepare_going_down();
>
> while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
> - rc = opal_cec_reboot();
> +
> + /* See if we need to do a full IPL reboot */
> + if (cmd && strcmp(cmd, "full") == 0)
> + rc = opal_cec_reboot2(OPAL_REBOOT_FULL_IPL, NULL);
> + else
> + rc = opal_cec_reboot();
> +
If the reboot type isn't supported, what should be the behvaiour? Reboot
the default way or don't reboot at all?
--
Stewart Smith
OPAL Architect, IBM.
Re: [PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot
Thanks for looking into this Andrew,
Andrew Donnellan writes:
> Oh good, someone else has finally picked this up and sent a kernel
> patch, I did the skiboot half and then neglected to make it useful (I
> sent an RFC at https://patchwork.ozlabs.org/patch/697604/ but never
> followed up on it... this approach seems more usable, I think).
Thanks for pointing that out. I wasnt aware of this patch.
> When you say "the reboot command" - is this behaviour of passing the
> argument common to all the important init systems/reboot utils? What's
> the correct systemd way to do it?
Mostly Yes,
On systemd the reboot command is just a symlink to systemctl binary that
checks argv[0] and maintains the a backward compatible behaviour. The
more systemd specific command 'systemctl reboot full' will also do the
same thing. So this patch should work on systemd.
Looking at 'upstart' I see support for adding an arg to the reboot
command since revision
https://bazaar.launchpad.net/~upstart-devel/upstart/trunk/revision/1432.3.1
. So upstart should also be supported.
I dont see support for sending an arg in 'SysVinit' halt command
yet. However this patch wont break the halt command. Just that SysVinit
systems will continue to use fast-reboot like today. Though
theorectially with this patch a SysVinit user can write a tool to issue
reboot syscall with cmd == LINUX_REBOOT_CMD_RESTART2 and arg == "full".
>> while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
>> -rc = opal_cec_reboot();
>> +
>> +/* See if we need to do a full IPL reboot */
>> +if (cmd && strcmp(cmd, "full") == 0)
>> +rc = opal_cec_reboot2(OPAL_REBOOT_FULL_IPL, NULL);
>
> You might want to check for OPAL_UNSUPPORTED here just in case we're
> running on ancient firmware.
Good idea, will fix this in v2.
--
Vaibhav Jain
Linux Technology Center, IBM India Pvt. Ltd.
Re: [PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot
On 01/09/18 18:17, Vaibhav Jain wrote:
Ever since fast reboot is enabled by default in opal,
opal_cec_reboot() will use fast-reset instead of full IPL to perform
system reboot. This leaves the user with no direct way to force a full
IPL reboot except changing an nvram setting that persistently disables
fast-reset for all subsequent reboots.
This patch provides a more direct way for the user to force a one-shot
full IPL reboot by passing the command line argument 'full' to the
reboot command. So the user will be able to tweak the reboot behavior
via:
$ sudo reboot full # Force a full ipl reboot skipping fast-reset
or
$ sudo reboot# default reboot path (usually fast-reset)
The reboot command passes the un-parsed command argument to the kernel
via the 'Reboot' syscall which is then passed on to the arch function
pnv_restart(). The patch updates pnv_restart() to handle this cmd-arg
and issues opal_cec_reboot2 with OPAL_REBOOT_FULL_IPL to force a full
IPL reset.
Signed-off-by: Vaibhav Jain
Oh good, someone else has finally picked this up and sent a kernel
patch, I did the skiboot half and then neglected to make it useful (I
sent an RFC at https://patchwork.ozlabs.org/patch/697604/ but never
followed up on it... this approach seems more usable, I think).
When you say "the reboot command" - is this behaviour of passing the
argument common to all the important init systems/reboot utils? What's
the correct systemd way to do it?
while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
- rc = opal_cec_reboot();
+
+ /* See if we need to do a full IPL reboot */
+ if (cmd && strcmp(cmd, "full") == 0)
+ rc = opal_cec_reboot2(OPAL_REBOOT_FULL_IPL, NULL);
You might want to check for OPAL_UNSUPPORTED here just in case we're
running on ancient firmware.
+ else
+ rc = opal_cec_reboot();
+
if (rc == OPAL_BUSY_EVENT)
opal_poll_events(NULL);
else
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com IBM Australia Limited
Re: [PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot
Thanks for looking into this patch Nick, Nicholas Piggin writes: > This is nice but I wonder if the default should be IPL and fast > should be the option. Fast-reset should work most of the times so I think it should be default. Its also the current default which I am bit relunctant to change. We usually need full IPL reset only when we need to update skiboot or have malfunctioning device that isnt being reset properly during fast-reset. So I am believe full IPL resets will be less frequent hence should be explicitly requested. -- Vaibhav Jain Linux Technology Center, IBM India Pvt. Ltd.
Re: [PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot
On Sat, 1 Sep 2018 13:47:45 +0530
Vaibhav Jain wrote:
> Ever since fast reboot is enabled by default in opal,
> opal_cec_reboot() will use fast-reset instead of full IPL to perform
> system reboot. This leaves the user with no direct way to force a full
> IPL reboot except changing an nvram setting that persistently disables
> fast-reset for all subsequent reboots.
>
> This patch provides a more direct way for the user to force a one-shot
> full IPL reboot by passing the command line argument 'full' to the
> reboot command. So the user will be able to tweak the reboot behavior
> via:
>
> $ sudo reboot full # Force a full ipl reboot skipping fast-reset
>
> or
> $ sudo reboot # default reboot path (usually fast-reset)
>
> The reboot command passes the un-parsed command argument to the kernel
> via the 'Reboot' syscall which is then passed on to the arch function
> pnv_restart(). The patch updates pnv_restart() to handle this cmd-arg
> and issues opal_cec_reboot2 with OPAL_REBOOT_FULL_IPL to force a full
> IPL reset.
>
> Signed-off-by: Vaibhav Jain
This is nice but I wonder if the default should be IPL and fast
should be the option.
> ---
> arch/powerpc/include/asm/opal-api.h| 1 +
> arch/powerpc/platforms/powernv/setup.c | 8 +++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/opal-api.h
> b/arch/powerpc/include/asm/opal-api.h
> index 8365353330b4..870fb7b239ea 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -1050,6 +1050,7 @@ enum OpalSysCooling {
> enum {
> OPAL_REBOOT_NORMAL = 0,
> OPAL_REBOOT_PLATFORM_ERROR = 1,
> + OPAL_REBOOT_FULL_IPL= 2,
> };
>
> /* Argument to OPAL_PCI_TCE_KILL */
> diff --git a/arch/powerpc/platforms/powernv/setup.c
> b/arch/powerpc/platforms/powernv/setup.c
> index ae023622..33d2faeacff8 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -224,7 +224,13 @@ static void __noreturn pnv_restart(char *cmd)
> pnv_prepare_going_down();
>
> while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
> - rc = opal_cec_reboot();
> +
> + /* See if we need to do a full IPL reboot */
> + if (cmd && strcmp(cmd, "full") == 0)
> + rc = opal_cec_reboot2(OPAL_REBOOT_FULL_IPL, NULL);
> + else
> + rc = opal_cec_reboot();
> +
> if (rc == OPAL_BUSY_EVENT)
> opal_poll_events(NULL);
> else
> --
> 2.17.1
>
[PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot
Ever since fast reboot is enabled by default in opal,
opal_cec_reboot() will use fast-reset instead of full IPL to perform
system reboot. This leaves the user with no direct way to force a full
IPL reboot except changing an nvram setting that persistently disables
fast-reset for all subsequent reboots.
This patch provides a more direct way for the user to force a one-shot
full IPL reboot by passing the command line argument 'full' to the
reboot command. So the user will be able to tweak the reboot behavior
via:
$ sudo reboot full# Force a full ipl reboot skipping fast-reset
or
$ sudo reboot # default reboot path (usually fast-reset)
The reboot command passes the un-parsed command argument to the kernel
via the 'Reboot' syscall which is then passed on to the arch function
pnv_restart(). The patch updates pnv_restart() to handle this cmd-arg
and issues opal_cec_reboot2 with OPAL_REBOOT_FULL_IPL to force a full
IPL reset.
Signed-off-by: Vaibhav Jain
---
arch/powerpc/include/asm/opal-api.h| 1 +
arch/powerpc/platforms/powernv/setup.c | 8 +++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/opal-api.h
b/arch/powerpc/include/asm/opal-api.h
index 8365353330b4..870fb7b239ea 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -1050,6 +1050,7 @@ enum OpalSysCooling {
enum {
OPAL_REBOOT_NORMAL = 0,
OPAL_REBOOT_PLATFORM_ERROR = 1,
+ OPAL_REBOOT_FULL_IPL= 2,
};
/* Argument to OPAL_PCI_TCE_KILL */
diff --git a/arch/powerpc/platforms/powernv/setup.c
b/arch/powerpc/platforms/powernv/setup.c
index ae023622..33d2faeacff8 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -224,7 +224,13 @@ static void __noreturn pnv_restart(char *cmd)
pnv_prepare_going_down();
while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
- rc = opal_cec_reboot();
+
+ /* See if we need to do a full IPL reboot */
+ if (cmd && strcmp(cmd, "full") == 0)
+ rc = opal_cec_reboot2(OPAL_REBOOT_FULL_IPL, NULL);
+ else
+ rc = opal_cec_reboot();
+
if (rc == OPAL_BUSY_EVENT)
opal_poll_events(NULL);
else
--
2.17.1
