| Message ID | 20180901081745.2165-1-vaibhav@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/powernv: Make possible for user to force a full ipl cec reboot | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | warning | Test checkpatch on branch next |
| snowpatch_ozlabs/build-ppc64le | success | Test build-ppc64le on branch next |
| snowpatch_ozlabs/build-ppc64be | success | Test build-ppc64be on branch next |
| snowpatch_ozlabs/build-ppc64e | success | Test build-ppc64e on branch next |
| snowpatch_ozlabs/build-ppc32 | success | Test build-ppc32 on branch next |
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 >
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.
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 <vaibhav@linux.ibm.com> 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 >
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.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?
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().
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
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(-)