* ipmi command implementation mismatch with the design document
@ 2020-11-23 6:39 Tung Nguyen OS
2020-11-24 19:06 ` Vernon Mauery
0 siblings, 1 reply; 4+ messages in thread
From: Tung Nguyen OS @ 2020-11-23 6:39 UTC (permalink / raw)
To: openbmc
[-- Attachment #1: Type: text/plain, Size: 3419 bytes --]
Hi everyone,
I’m Tung Nguyen, developer for AmpereComputing - Altra system. While working with the IPMI commands I have a concern when comparing the design document and the implementation like the following:
The state-management-and-external-interfaces.md
The full mapping of Redfish and IPMI to xyz.openbmc_project.State.* is as follows:
Redfish IPMI xyz.openbmc_project.State.Transition
ForceOff power down Chassis.Off
ForceOn power up Host.On
ForceRestart hard reset Host.ForceWarmReboot
GracefulRestart Host.GracefulWarmReboot
GracefulShutdown soft off Host.Off
On power up Host.On
PowerCycle (host on) power cycle Host.Reboot
PowerCycle (host off) Chassis.PowerCycle
the IPMI – chassishandler.cpp:
ipmi::RspType<> ipmiChassisControl(uint8_t chassisControl)
{
int rc = 0;
switch (chassisControl)
{
case CMD_POWER_ON:
rc = initiate_state_transition(State::Host::Transition::On);
break;
case CMD_POWER_OFF:
// This path would be hit in 2 conditions.
// 1: When user asks for power off using ipmi chassis command 0x04
// 2: Host asking for power off post shutting down.
// If it's a host requested power off, then need to nudge Softoff
// application that it needs to stop the watchdog timer if running.
// If it is a user requested power off, then this is not really
// needed. But then we need to differentiate between user and host
// calling this same command
// For now, we are going ahead with trying to nudge the soft off and
// interpret the failure to do so as a non softoff case
rc = stop_soft_off_timer();
// Only request the Off transition if the soft power off
// application is not running
if (rc < 0)
{
// First create a file to indicate to the soft off application
// that it should not run. Not doing this will result in State
// manager doing a default soft power off when asked for power
// off.
indicate_no_softoff_needed();
// Now request the shutdown
rc = initiate_state_transition(State::Host::Transition::Off);
}
else
{
log<level::INFO>("Soft off is running, so let shutdown target "
"stop the host");
}
break;
The redfish – systems.hpp:
else if (resetType == "ForceOff")
{
command = "xyz.openbmc_project.State.Chassis.Transition.Off";
hostCommand = false;
}
Although the indicate_no_softoff_needed() can prevent the host from soft off, but it seems like a mismatch b/w the design document and the IPMI implementation.
So, my question: is it reasonable for IPMI command ?
Reference:
https://github.com/openbmc/docs/blob/master/designs/state-management-and-external-interfaces.md
Best regards,
Tung
[-- Attachment #2: Type: text/html, Size: 20099 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ipmi command implementation mismatch with the design document
2020-11-23 6:39 ipmi command implementation mismatch with the design document Tung Nguyen OS
@ 2020-11-24 19:06 ` Vernon Mauery
2020-11-30 17:36 ` Andrew Geissler
0 siblings, 1 reply; 4+ messages in thread
From: Vernon Mauery @ 2020-11-24 19:06 UTC (permalink / raw)
To: Tung Nguyen OS; +Cc: Andrew Geissler, openbmc
On 23-Nov-2020 06:39 AM, Tung Nguyen OS wrote:
>Hi everyone,
>I’m Tung Nguyen, developer for AmpereComputing - Altra system. While working with the IPMI commands I have a concern when comparing the design document and the implementation like the following:
>The state-management-and-external-interfaces.md
>The full mapping of Redfish and IPMI to xyz.openbmc_project.State.* is as follows:
>Redfish IPMI xyz.openbmc_project.State.Transition
>
>ForceOff power down Chassis.Off
>ForceOn power up Host.On
>ForceRestart hard reset Host.ForceWarmReboot
>GracefulRestart Host.GracefulWarmReboot
>GracefulShutdown soft off Host.Off
>On power up Host.On
>PowerCycle (host on) power cycle Host.Reboot
>PowerCycle (host off) Chassis.PowerCycle
>
>the IPMI – chassishandler.cpp:
>
>ipmi::RspType<> ipmiChassisControl(uint8_t chassisControl)
>
>{
>
> int rc = 0;
>
> switch (chassisControl)
>
> {
>
> case CMD_POWER_ON:
>
> rc = initiate_state_transition(State::Host::Transition::On);
>
> break;
>
> case CMD_POWER_OFF:
>
> // This path would be hit in 2 conditions.
>
> // 1: When user asks for power off using ipmi chassis command 0x04
>
> // 2: Host asking for power off post shutting down.
>
>
>
> // If it's a host requested power off, then need to nudge Softoff
>
> // application that it needs to stop the watchdog timer if running.
>
> // If it is a user requested power off, then this is not really
>
> // needed. But then we need to differentiate between user and host
>
> // calling this same command
>
>
>
> // For now, we are going ahead with trying to nudge the soft off and
>
> // interpret the failure to do so as a non softoff case
>
> rc = stop_soft_off_timer();
>
>
>
> // Only request the Off transition if the soft power off
>
> // application is not running
>
> if (rc < 0)
>
> {
>
> // First create a file to indicate to the soft off application
>
> // that it should not run. Not doing this will result in State
>
> // manager doing a default soft power off when asked for power
>
> // off.
>
> indicate_no_softoff_needed();
>
>
>
> // Now request the shutdown
>
> rc = initiate_state_transition(State::Host::Transition::Off);
>
> }
>
> else
>
> {
>
> log<level::INFO>("Soft off is running, so let shutdown target "
>
> "stop the host");
>
> }
>
> break;
>
>
>
>The redfish – systems.hpp:
> else if (resetType == "ForceOff")
> {
> command = "xyz.openbmc_project.State.Chassis.Transition.Off";
> hostCommand = false;
> }
>
>
>
>
>
>Although the indicate_no_softoff_needed() can prevent the host from soft off, but it seems like a mismatch b/w the design document and the IPMI implementation.
>
>So, my question: is it reasonable for IPMI command ?
This code has been in place for quite some time now, so I am not sure if
the original authors have the context at this point in time. But nobody
else has raised this question. git blame says that Andrew Geissler added
this feature, but we would have to see if he still knows why.
commit a6e3a3080d532536e02e304c819c1e17214e038a
Author: Andrew Geissler <andrewg@us.ibm.com>
Date: Wed May 31 19:34:00 2017 -0500
Create file to indicate host requested off/reboot
Create a file to ensure the soft power off service is
not run when the host is requesting a power off
or reboot. There's no need to notify the host (i.e.
soft power off) when they are initiating it.
Change-Id: Ic9f8e7110d30f477ceae38bba9d684559d9503d3
Signed-off-by: Andrew Geissler <andrewg@us.ibm.com>
--Vernon
>Reference:
>https://github.com/openbmc/docs/blob/master/designs/state-management-and-external-interfaces.md
>
>Best regards,
>Tung
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ipmi command implementation mismatch with the design document
2020-11-24 19:06 ` Vernon Mauery
@ 2020-11-30 17:36 ` Andrew Geissler
2020-12-10 8:34 ` George Hung (洪忠敬)
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Geissler @ 2020-11-30 17:36 UTC (permalink / raw)
To: Vernon Mauery; +Cc: Tung Nguyen OS, openbmc
> On Nov 24, 2020, at 1:06 PM, Vernon Mauery <vernon.mauery@linux.intel.com> wrote:
>
> On 23-Nov-2020 06:39 AM, Tung Nguyen OS wrote:
>>
>> Although the indicate_no_softoff_needed() can prevent the host from soft off, but it seems like a mismatch b/w the design document and the IPMI implementation.
>>
>> So, my question: is it reasonable for IPMI command ?
>
> This code has been in place for quite some time now, so I am not sure if the original authors have the context at this point in time. But nobody else has raised this question. git blame says that Andrew Geissler added this feature, but we would have to see if he still knows why.
> commit a6e3a3080d532536e02e304c819c1e17214e038a
> Author: Andrew Geissler <andrewg@us.ibm.com>
> Date: Wed May 31 19:34:00 2017 -0500
>
> Create file to indicate host requested off/reboot
>
> Create a file to ensure the soft power off service is
> not run when the host is requesting a power off
> or reboot. There's no need to notify the host (i.e.
> soft power off) when they are initiating it.
>
> Change-Id: Ic9f8e7110d30f477ceae38bba9d684559d9503d3
> Signed-off-by: Andrew Geissler <andrewg@us.ibm.com>
>
I’m wondering if this ties into the discussion in https://github.com/openbmc/phosphor-host-ipmid/issues/158?
Are you having the same issue Tung? May be time to just put up a commit
that does this correctly and discuss via gerrit.
>
> --Vernon
>
>> Reference:
>> https://github.com/openbmc/docs/blob/master/designs/state-management-and-external-interfaces.md
>>
>> Best regards,
>> Tung
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: ipmi command implementation mismatch with the design document
2020-11-30 17:36 ` Andrew Geissler
@ 2020-12-10 8:34 ` George Hung (洪忠敬)
0 siblings, 0 replies; 4+ messages in thread
From: George Hung (洪忠敬) @ 2020-12-10 8:34 UTC (permalink / raw)
To: Andrew Geissler, Vernon Mauery; +Cc: Tung Nguyen OS, openbmc
> On Nov 24, 2020, at 1:06 PM, Vernon Mauery <vernon.mauery@linux.intel.com> wrote:
>
> On 23-Nov-2020 06:39 AM, Tung Nguyen OS wrote:
>>
>> Although the indicate_no_softoff_needed() can prevent the host from soft off, but it seems like a mismatch b/w the design document and the IPMI implementation.
>>
>> So, my question: is it reasonable for IPMI command ?
>
> This code has been in place for quite some time now, so I am not sure if the original authors have the context at this point in time. But nobody else has raised this question. git blame says that Andrew Geissler added this feature, but we would have to see if he still knows why.
> commit a6e3a3080d532536e02e304c819c1e17214e038a
> Author: Andrew Geissler <andrewg@us.ibm.com>
> Date: Wed May 31 19:34:00 2017 -0500
>
> Create file to indicate host requested off/reboot
>
> Create a file to ensure the soft power off service is
> not run when the host is requesting a power off
> or reboot. There's no need to notify the host (i.e.
> soft power off) when they are initiating it.
>
> Change-Id: Ic9f8e7110d30f477ceae38bba9d684559d9503d3
> Signed-off-by: Andrew Geissler <andrewg@us.ibm.com>
>
> I’m wondering if this ties into the discussion in https://github.com/openbmc/phosphor-host-ipmid/issues/158?
> Are you having the same issue Tung? May be time to just put up a commit that does this correctly and discuss via gerrit.
Hi Andrew, I have the same question about ipmi chassis power off,
because according to https://github.com/openbmc/docs/blob/master/designs/state-management-and-external-interfaces.md
Should it be set state transition to "State::Chassis::Transition::Off", not "State::Host::Transition::Off" ?
Or do I miss something I don't know.
Best Regards
George Hung
>
> --Vernon
>
>> Reference:
>> https://github.com/openbmc/docs/blob/master/designs/state-management-
>> and-external-interfaces.md
>>
>> Best regards,
>> Tung
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-10 8:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-23 6:39 ipmi command implementation mismatch with the design document Tung Nguyen OS
2020-11-24 19:06 ` Vernon Mauery
2020-11-30 17:36 ` Andrew Geissler
2020-12-10 8:34 ` George Hung (洪忠敬)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).