2020-08-24 09:02:51

by Greg KH

[permalink] [raw]
Subject: [PATCH 4.19 65/71] powerpc/pseries: Do not initiate shutdown when system is running on UPS

From: Vasant Hegde <[email protected]>

commit 90a9b102eddf6a3f987d15f4454e26a2532c1c98 upstream.

As per PAPR we have to look for both EPOW sensor value and event
modifier to identify the type of event and take appropriate action.

In LoPAPR v1.1 section 10.2.2 includes table 136 "EPOW Action Codes":

SYSTEM_SHUTDOWN 3

The system must be shut down. An EPOW-aware OS logs the EPOW error
log information, then schedules the system to be shut down to begin
after an OS defined delay internal (default is 10 minutes.)

Then in section 10.3.2.2.8 there is table 146 "Platform Event Log
Format, Version 6, EPOW Section", which includes the "EPOW Event
Modifier":

For EPOW sensor value = 3
0x01 = Normal system shutdown with no additional delay
0x02 = Loss of utility power, system is running on UPS/Battery
0x03 = Loss of system critical functions, system should be shutdown
0x04 = Ambient temperature too high
All other values = reserved

We have a user space tool (rtas_errd) on LPAR to monitor for
EPOW_SHUTDOWN_ON_UPS. Once it gets an event it initiates shutdown
after predefined time. It also starts monitoring for any new EPOW
events. If it receives "Power restored" event before predefined time
it will cancel the shutdown. Otherwise after predefined time it will
shutdown the system.

Commit 79872e35469b ("powerpc/pseries: All events of
EPOW_SYSTEM_SHUTDOWN must initiate shutdown") changed our handling of
the "on UPS/Battery" case, to immediately shutdown the system. This
breaks existing setups that rely on the userspace tool to delay
shutdown and let the system run on the UPS.

Fixes: 79872e35469b ("powerpc/pseries: All events of EPOW_SYSTEM_SHUTDOWN must initiate shutdown")
Cc: [email protected] # v4.0+
Signed-off-by: Vasant Hegde <[email protected]>
[mpe: Massage change log and add PAPR references]
Signed-off-by: Michael Ellerman <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
arch/powerpc/platforms/pseries/ras.c | 1 -
1 file changed, 1 deletion(-)

--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -118,7 +118,6 @@ static void handle_system_shutdown(char
case EPOW_SHUTDOWN_ON_UPS:
pr_emerg("Loss of system power detected. System is running on"
" UPS/battery. Check RTAS error log for details\n");
- orderly_poweroff(true);
break;

case EPOW_SHUTDOWN_LOSS_OF_CRITICAL_FUNCTIONS:



2020-08-25 19:57:20

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 4.19 65/71] powerpc/pseries: Do not initiate shutdown when system is running on UPS

Hi!

> We have a user space tool (rtas_errd) on LPAR to monitor for
> EPOW_SHUTDOWN_ON_UPS. Once it gets an event it initiates shutdown
> after predefined time. It also starts monitoring for any new EPOW

Yeah, so there's userspace tool, and currently systems _with_ that
tool work poorly with UPS.

So you have fixed that, and now, systems _without_ that tool will work
poorly.

That's not a fix for serious bug, that's behaviour change. You are
fixing one set of systems and breaking another.

I don't believe it is suitable for stable.

Pavel

> @@ -118,7 +118,6 @@ static void handle_system_shutdown(char
> case EPOW_SHUTDOWN_ON_UPS:
> pr_emerg("Loss of system power detected. System is running on"
> " UPS/battery. Check RTAS error log for details\n");
> - orderly_poweroff(true);
> break;
>
> case EPOW_SHUTDOWN_LOSS_OF_CRITICAL_FUNCTIONS:
>

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (1.02 kB)
signature.asc (201.00 B)
Download all attachments

2020-08-26 11:17:02

by Vasant Hegde

[permalink] [raw]
Subject: Re: [PATCH 4.19 65/71] powerpc/pseries: Do not initiate shutdown when system is running on UPS

On 8/26/20 1:26 AM, Pavel Machek wrote:
> Hi!
>

Hi Pavel,

>> We have a user space tool (rtas_errd) on LPAR to monitor for
>> EPOW_SHUTDOWN_ON_UPS. Once it gets an event it initiates shutdown
>> after predefined time. It also starts monitoring for any new EPOW
>
> Yeah, so there's userspace tool, and currently systems _with_ that
> tool work poorly with UPS.
>
> So you have fixed that, and now, systems _without_ that tool will work
> poorly.

User space tool exists for long long time (more than decade) and its default tool
on pseries system. Also user space tool behavior is not changed for long time.

The original design was to forward UPS event to userspace and let user space wait
for predefined time and then initiate shutdown.

Previous fix accidentally initiated shutdown as soon as system switch to UPS power.

>
> That's not a fix for serious bug, that's behaviour change. You are
> fixing one set of systems and breaking another.

Without fix, as soon as system switches to UPS power supply, kernel will start
shutdown process. which is not correct. Its actually impacting customers running
Linux on pseries LPAR mode. Hence I have requested this fix for stable tree.

Hope this clarifies your concern.

-Vasant


>
> I don't believe it is suitable for stable.
>
> Pavel
>
>> @@ -118,7 +118,6 @@ static void handle_system_shutdown(char
>> case EPOW_SHUTDOWN_ON_UPS:
>> pr_emerg("Loss of system power detected. System is running on"
>> " UPS/battery. Check RTAS error log for details\n");
>> - orderly_poweroff(true);
>> break;
>>
>> case EPOW_SHUTDOWN_LOSS_OF_CRITICAL_FUNCTIONS:
>>
>