[OpenBMC] [phosphor-sel-logger] Property type conversion and the value of getScaledIPMIValue

[George Hung (洪忠敬)]

Hi all,
 
Considering there are two style of sensors (scaled int64 and double values) for sensor value DBus properties, we would like to add both of them support for phosphor-sel-logger.

The attached file is the patch for making sel-logger work with int64 and double.

If it's OK, I will push the patch to OpenBMC Gerrit.

Hi Jason,

Thanks for your kind advices.

About the first question I mentioned before, "The sensor DBus properties , “/MaxValue/MinValue/Value/” are always 0", it's because type conversion cause always get 0, not their values are invalid. I'm sorry for my confused description.


Thanks.

Best Regard 
George Hung
Research Division - Software Engineer
Cloud Computing Business Unit
Quanta Computer Inc.
Ext: 16830
E-Mail : George.Hung at QuantaTW.com

-----Original Message-----
From: Bills, Jason M [mailto:jason.m.bills at linux.intel.com] 
Sent: Saturday, March 23, 2019 6:50 AM
To: George Hung (洪忠敬)
Cc: Richard Tung (董彥屏); Buddy Huang (黃天鴻); Will Liang (梁永鉉); Fran Hsu (徐誌謙)
Subject: Re: [OpenBMC] [phosphor-sel-logger] Property type conversion and the value of getScaledIPMIValue

Hi George,

It is better to post these types of questions to the full mailing list as I am not the right person to answer all of them.

On 3/22/2019 12:20 AM, George Hung (洪忠敬) wrote:
> Hi Jason,
> 
> Since our OpenBMC project needs to support IPMI SEL, I try to use 
> phosphor-sel-logger to add new IPMI SEL to journal, but I found some 
> issues need to discuss:
> 
> 1.The sensor DBus properties , “/MaxValue/MinValue/Value/” are always 
> 0

This means that something is wrong in the sensor configuration.  These values should all be valid.
> 
> 2.The /value/ parameter of /getScaledIPMIValue/ function uses the 
> scaled value, not actual value,

The threshold event monitor is currently designed to monitor only the double-style sensors that store the actual value.
> 
> it may cause the result of /scaleIPMIValueFromDouble /function is 
> “value out of range”
> 
> After tracing the code, we may find the root cause, but need your help 
> to check if it’s the correct fix. I listed as follows:
> 
> 1.The original type declaration is
> “sdbusplus::message::variant<double>”, but should be 
> “sdbusplus::message::variant<int64_t>”

There are two styles of sensors, one style stores scaled int64 values and another style stores actual double values.  The threshold event monitor currently handles only double style sensors.

If you need to support both styles of sensors, you can look here for an example visitor that can handle both int64 and double: 
https://github.com/openbmc/phosphor-pid-control/commit/c065cf1b195f9ccd8fea882ce76416473d4fee7c.
> 
> 2.Modify to use actual sensor value for “getScaledIPMIValue” function
> 
> The attached file is the fix patch, it should be more clear.
> 
> If it’s the correct fix, will you patch to OpenBMC Gerrit, or we do it 
> directly ?

Please feel free to push your patch to OpenBMC Gerrit for review and discussion.
> 
> BTW, the phosphor-sel-logger supports add IPMI SEL to journal, but 
> seems only threshold-based sensor event logs, no discrete sensors event logs.

That is correct.
> 
> Is there any plan to add discrete sensors event logs to 
> phosphor-sel-logger ?

Intel plans to only support threshold sensor events in the IPMI SEL. 
The phosphor-sel-logger should be extensible to support additional types of events.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Support-both-double-and-int64-sensor-values.patch
Type: application/octet-stream
Size: 2990 bytes
Desc: Support-both-double-and-int64-sensor-values.patch
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20190326/a4951b88/attachment.obj>