[PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot

[PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot

[Vaibhav Jain]

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 <vaibhav@linux.ibm.com>
---
 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 adddde023622..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

Re: [PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot

[Nicholas Piggin]

On Sat,  1 Sep 2018 13:47:45 +0530
Vaibhav Jain <vaibhav@linux.ibm.com> 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 <vaibhav@linux.ibm.com>

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

Re: [PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot

[Vaibhav Jain]

Thanks for looking into this patch Nick,

Nicholas Piggin <npiggin@gmail.com> 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 <vaibhav@linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.

Re: [PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot

[Andrew Donnellan]

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.
>=20
> 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:
>=20
>    $ sudo reboot full	# Force a full ipl reboot skipping fast-reset
>=20
>    or
>    $ sudo reboot  	# default reboot path (usually fast-reset)
>=20
> 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.
>=20
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>

Oh good, someone else has finally picked this up and sent a kernel=20
patch, I did the skiboot half and then neglected to make it useful (I=20
sent an RFC at https://patchwork.ozlabs.org/patch/697604/ but never=20
followed up on it... this approach seems more usable, I think).

When you say "the reboot command" - is this behaviour of passing the=20
argument common to all the important init systems/reboot utils?  What's=20
the correct systemd way to do it?

>   	while (rc =3D=3D OPAL_BUSY || rc =3D=3D OPAL_BUSY_EVENT) {
> -		rc =3D opal_cec_reboot();
> +
> +		/* See if we need to do a full IPL reboot */
> +		if (cmd && strcmp(cmd, "full") =3D=3D 0)
> +			rc =3D opal_cec_reboot2(OPAL_REBOOT_FULL_IPL, NULL);

You might want to check for OPAL_UNSUPPORTED here just in case we're=20
running on ancient firmware.

> +		else
> +			rc =3D opal_cec_reboot();
> +
>   		if (rc =3D=3D OPAL_BUSY_EVENT)
>   			opal_poll_events(NULL);
>   		else
>=20

--=20
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: [PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot

[Vaibhav Jain]

Thanks for looking into this Andrew,

Andrew Donnellan <andrew.donnellan@au1.ibm.com> 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 <vaibhav@linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.

Re: [PATCH] powerpc/powernv: Make possible for user to force a full ipl cec reboot

[Stewart Smith]

Vaibhav Jain <vaibhav@linux.ibm.com> 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 <vaibhav@linux.ibm.com>
> ---
>  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 adddde023622..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

[Vaibhav Jain]

Thanks for looking into this patch Stewart

Stewart Smith <stewart@linux.ibm.com> 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 <vaibhav@linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.