search

openbmc / phosphor-pid-control

OpenBMC PID-based Thermal Control Daemon
Apache License 2.0
16 stars 21 forks source link

Need Redfish equivalents to our IPMI extensions #21

Open Krellan opened 2 years ago

Krellan commented 2 years ago

This is a request for discussion.

We currently have these IPMI extensions:

https://github.com/openbmc/phosphor-pid-control/blob/master/ipmi.md

We should have Redfish equivalents for these, for feature parity with IPMI.

Are there existing efforts to expose thermal zones (both read-only and read-write parameters) to Redfish? I know sensors and fans are currently exposed, but I'm not sure about thermal zones.

edtanous commented 2 years ago

This already exists in Redfish, and has significantly more functionality than even the IPMI commands that exist:

https://github.com/openbmc/bmcweb/blob/039a47e3474d5667d295984f330e876aef309eac/static/redfish/v1/schema/OemManager_v1.xml#L113

Is an example of one of the schemas for it.

Krellan commented 2 years ago

As for OemManager, it's strange, wouldn't there be separate schemas for fans, PID loops, and such, instead of grouping them all into the OemManager schema?

I see this is a way to expose the configuration parameters of the PID loops (what is in the JSON file for entity-manager now) over Redfish.

That's useful, but doesn't address the question. Are there Redfish equivalents to the existing IPMI extensions? I could not find them, after searching.

In particular, these are needed:

A challenge is that these parameters are "live", in that they come directly from the phosphor-pid-control program when queried. They aren't merely configuration parameters. They only exist at run time, they don't exist at configuration time.

If these are already in OemManager, where are they? I couldn't find them.

edtanous commented 2 years ago

As for OemManager, it's strange, wouldn't there be separate schemas for fans, PID loops, and such, instead of grouping them all into the OemManager schema?

What you described is what's already there. All of the answers to the above can be found by looking through he code.

I see this is a way to expose the configuration parameters of the PID loops (what is in the JSON file for entity-manager now) over Redfish.

That's useful, but doesn't address the question. Are there Redfish equivalents to the existing IPMI extensions? I could not find them, after searching.

The existing schemas cover everything that I'm aware of (and more) that the IPMI extensions did, so I think the answer is, yes, there are equivalents, but most things are covered in the standard now.

In particular, these are needed:

  • Failsafe status. This is a boolean flag, per thermal zone. True if in failsafe mode, false if in normal mode. It's read-only from the user's point of view.

This I think is covered as part of "health", but we'd have to go read the code.

  • Auto versus manual setting. This is an enum with 2 possibilities (it could also be a boolean flag if desired). Auto setting indicates the PID loops are active, manual setting indicates the PID loops are bypassed (and thus, the user's manual fan speed selection has a much higher chance of being honored). It's read-write from the user's point of view.

We don't have an explicit "manual" setting, we just have a PATCH to the PWM value you'd like to set puts it in manual mode. I believe a patch of null puts it back in automatic for any given zone.

A challenge is that these parameters are "live", in that they come directly from the phosphor-pid-control program when queried. They aren't merely configuration parameters. They only exist at run time, they don't exist at configuration time.

When in dbus mode, the "value" of a PWM is owned by pwmsensor in dbus-sensors, which redfish is capable of reading and controlling. You're right, the failsafe status might not be handled.

If these are already in OemManager, where are they? I couldn't find them.

Answers above.

Krellan commented 4 months ago

There was an earlier effort to add this functionality:

https://gerrit.openbmc.org/c/openbmc/bmcweb/+/45181

This patch is incomplete (no exposure of the FailSafe boolean, improper behavior when phosphor-pid-control is not running, and so on). However, it is a good start.