| Message ID | 20180903102624.24344-1-vaibhav@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] 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 | success | 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 03/09/18 20:26, Vaibhav Jain wrote: > diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c > index adddde023622..650484e0940b 100644 > --- a/arch/powerpc/platforms/powernv/setup.c > +++ b/arch/powerpc/platforms/powernv/setup.c > @@ -224,7 +224,22 @@ 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_UNSUPPORTED; > + > + if (!(rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT)) { > + if (cmd) { > + pr_warn("Couldn't do a '%s' reboot. Err=%ld\n", > + cmd, rc); > + /* reset cmd to force a cec_reboot now */ > + cmd = NULL; > + } > + rc = opal_cec_reboot(); > + } > + The logic here is a bit hard to follow, relying on if (cmd && strcmp...) failing then setting rc = OPAL_UNSUPPORTED in order to handle the normal case seems a bit backwards. I think I would slightly prefer it if the !cmd case were handled first to make it clear that opal_cec_reboot() is the normal case. But either way: Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Thanks for reviewing this patch Andrew, Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes: > The logic here is a bit hard to follow, relying on if (cmd && strcmp...) > failing then setting rc = OPAL_UNSUPPORTED in order to handle the normal > case seems a bit backwards. Agree. With this change I am trying to make adding new reboot types in future simpler, without resorting to major refactoring of the function. opal_cec_reboot() should be a fallback for any failures in doing a typed reboot. So adding a new foobar reboot type just needs adding a new branch in the if-else ladder. if (cmd && strcmp(cmd, "full") == 0) rc = opal_cec_reboot2(OPAL_REBOOT_FULL_IPL, NULL); else if(cmd && strcmd(cmd, "foobar") == 0) rc = /* new code here */ else rc = OPAL_UNSUPPORTED; > I think I would slightly prefer it if the !cmd case were handled first > to make it clear that opal_cec_reboot() is the normal case. Fair suggestion. But then I will still have to handle the errors just after the if-else ladder is finished to determine if I need to fallback. > Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Thanks
On 09/03/2018 03:56 PM, 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: > .../... > > /* Argument to OPAL_PCI_TCE_KILL */ > diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c > index adddde023622..650484e0940b 100644 > --- a/arch/powerpc/platforms/powernv/setup.c > +++ b/arch/powerpc/platforms/powernv/setup.c > @@ -224,7 +224,22 @@ 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_UNSUPPORTED; As discussed offline, please handle OPAL_SUCCESS case well. -Vasant
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..650484e0940b 100644 --- a/arch/powerpc/platforms/powernv/setup.c +++ b/arch/powerpc/platforms/powernv/setup.c @@ -224,7 +224,22 @@ 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_UNSUPPORTED; + + if (!(rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT)) { + if (cmd) { + pr_warn("Couldn't do a '%s' reboot. Err=%ld\n", + cmd, rc); + /* reset cmd to force a cec_reboot now */ + cmd = NULL; + } + 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> --- Change-log: v2 -> Updated the code to handle case when opal_cec_reboot2() is not supported by opal. [Andrew] -> Force a call to opal_cec_reboot() is opal_cec_reboot2() fails or the action verb 'cmd' is not recognized. --- arch/powerpc/include/asm/opal-api.h | 1 + arch/powerpc/platforms/powernv/setup.c | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-)