IPMI SEL refactoring comments

IPMI SEL refactoring comments

[Tom Joseph]

Hello Jason,

I reviewed the patches for the IPMI SEL refactoring and wanted to 
summarize the concerns with the proposed design. There are some current 
features that are broken.

1) The sensor numbers are arbitrary in the proposed implementation and 
does not account for the presence/error corresponding to inventory 
objects. On IBM systems the host firmware does not rely on the SDR 
repository in BMC to figure out the sensor numbers, rather the sensor 
numbers are programmed into the  host firmware at the build time. So the 
sensor numbers cannot be arbitrary.

 > I know you are working on a proposal to have a customizable map of 
sensor number to path. The support is only for threshold based sensors( 
targeting only /xyz/openbmc_project/sensors namespace) , it needs to 
extend support for discrete sensors. The sensor types are limited to few 
types(temperature, voltage,current, fan_tach, power), need to support 
other sensor types in the specification. It might not be possible to 
deduce the sensor type from the D-Bus object path for all sensors.

2)  OpenPower systems like Witherspoon and Zaius relies on eSEL(extended 
SEL format) to report debug data from the host firmware. The eSEL data 
is added to the D-Bus native error log via the Add SEL entry command. 
This is not handled with the refactoring.

 > This is an openpower requirement and needs to be handled in an 
openpower IPMI OEM library/application and not in the phosphor 
implementation. A D-Bus error log containing eSEL can be created on 
completion of transfer of eSEL and not depend on Add SEL entry. The 
callouts associated with the eSEL can be added to the D-Bus error log by 
keeping a watch on the IPMI SEL entries. This might need changes to 
phosphor-logging API to support adding callout after creation of error 
log. This is a proposal and looks viable.

3) The current implementation the Get SEL entry is translating the D-Bus 
error log to IPMI SEL format. If there is a FRU callout associated with 
the D-Bus error log(D-Bus object paths in the inventory namespace) it is 
mapped to the sensor number associated with FRU and reported through SEL.

 > The implementation of the Get SEL entry needs to adapt to include all 
the objects paths(the patchsets target only /xyz/openbmc_project/sensors 
namespace) and map the D-Bus object path to the sensor numbers(based on  
the proposed customizable map).

4) In the current implementation all the D-Bus error logs have 1x1 
mapping in the IPMI SEL. In the proposed solution the users will have to 
rely on both D-Bus error logs and IPMI SEL to get the complete picture. 
The customers cannot rely only on IPMI SEL as an event repository.

 > In the current proposal D-Bus error logs reported by BMC will not be 
shown by SEL commands. I am okie with Jason's proposal to set up a D-Bus 
match (either by ipmid or phosphor-sel-logger) that will watch for new 
logging D-Bus objects to be created and add a SEL record for them on 
creation.

https://gerrit.openbmc-project.xyz/#/q/status:open++topic:%22IPMI+SEL%22

Regards,
Tom

Re: IPMI SEL refactoring comments

[Bills, Jason M]

On 11/26/2018 5:26 AM, Tom Joseph wrote:
> Hello Jason,
> 
> I reviewed the patches for the IPMI SEL refactoring and wanted to 
> summarize the concerns with the proposed design. There are some current 
> features that are broken.
> 
> 1) The sensor numbers are arbitrary in the proposed implementation and 
> does not account for the presence/error corresponding to inventory 
> objects. On IBM systems the host firmware does not rely on the SDR 
> repository in BMC to figure out the sensor numbers, rather the sensor 
> numbers are programmed into the  host firmware at the build time. So the 
> sensor numbers cannot be arbitrary.
> 
I think this is the main issue: mapping sensor paths to sensor numbers 
in a flexible way.  Once we can find a way to allow custom sensor number 
mapping, it should allow all types of sensors to be mapped as needed.

One idea we've had would be to somehow add the sensor number into the 
sensor D-Bus object which would mitigate arbitrary numbering as needed.

>  > I know you are working on a proposal to have a customizable map of 
> sensor number to path. The support is only for threshold based sensors( 
> targeting only /xyz/openbmc_project/sensors namespace) , it needs to 
> extend support for discrete sensors. The sensor types are limited to few 
> types(temperature, voltage,current, fan_tach, power), need to support 
> other sensor types in the specification. It might not be possible to 
> deduce the sensor type from the D-Bus object path for all sensors.
> 
> 2)  OpenPower systems like Witherspoon and Zaius relies on eSEL(extended 
> SEL format) to report debug data from the host firmware. The eSEL data 
> is added to the D-Bus native error log via the Add SEL entry command. 
> This is not handled with the refactoring.
> 
>  > This is an openpower requirement and needs to be handled in an 
> openpower IPMI OEM library/application and not in the phosphor 
> implementation. A D-Bus error log containing eSEL can be created on 
> completion of transfer of eSEL and not depend on Add SEL entry. The 
> callouts associated with the eSEL can be added to the D-Bus error log by 
> keeping a watch on the IPMI SEL entries. This might need changes to 
> phosphor-logging API to support adding callout after creation of error 
> log. This is a proposal and looks viable.
> 
> 3) The current implementation the Get SEL entry is translating the D-Bus 
> error log to IPMI SEL format. If there is a FRU callout associated with 
> the D-Bus error log(D-Bus object paths in the inventory namespace) it is 
> mapped to the sensor number associated with FRU and reported through SEL.
> 
>  > The implementation of the Get SEL entry needs to adapt to include all 
> the objects paths(the patchsets target only /xyz/openbmc_project/sensors 
> namespace) and map the D-Bus object path to the sensor numbers(based on 
> the proposed customizable map).
> 
This should also be solved by fixing the sensor number mapping.

> 4) In the current implementation all the D-Bus error logs have 1x1 
> mapping in the IPMI SEL. In the proposed solution the users will have to 
> rely on both D-Bus error logs and IPMI SEL to get the complete picture. 
> The customers cannot rely only on IPMI SEL as an event repository.
> 
>  > In the current proposal D-Bus error logs reported by BMC will not be 
> shown by SEL commands. I am okie with Jason's proposal to set up a D-Bus 
> match (either by ipmid or phosphor-sel-logger) that will watch for new 
> logging D-Bus objects to be created and add a SEL record for them on 
> creation.
> 
Another thought would be to abandon D-Bus-based logging and see if the 
same information could be placed in the journal.

> https://gerrit.openbmc-project.xyz/#/q/status:open++topic:%22IPMI+SEL%22
> 
> Regards,
> Tom
> 

More work is needed to come up with a flexible generic sensor numbering 
scheme that will work with the various sensor implementations.  I have 
been assigned to a different task and will not be working on this for a 
while.  So, as discussed in the call this week, as a stop-gap, I plan to 
move the journal-based SEL implementation into intel-ipmi-oem for now. 
We can revisit this as we make progress on the sensor numbering issue. 
This will also allow everyone to see the full working implementation 
which may help resolve some questions.

Thanks,
-Jason

Re: IPMI SEL refactoring comments

[Tom Joseph]

Hello Jason,


On Thursday 29 November 2018 04:51 AM, Bills, Jason M wrote:
>
>
> On 11/26/2018 5:26 AM, Tom Joseph wrote:
>> Hello Jason,
>>
>> I reviewed the patches for the IPMI SEL refactoring and wanted to 
>> summarize the concerns with the proposed design. There are some 
>> current features that are broken.
>>
>> 1) The sensor numbers are arbitrary in the proposed implementation 
>> and does not account for the presence/error corresponding to 
>> inventory objects. On IBM systems the host firmware does not rely on 
>> the SDR repository in BMC to figure out the sensor numbers, rather 
>> the sensor numbers are programmed into the  host firmware at the 
>> build time. So the sensor numbers cannot be arbitrary.
>>
> I think this is the main issue: mapping sensor paths to sensor numbers 
> in a flexible way.  Once we can find a way to allow custom sensor 
> number mapping, it should allow all types of sensors to be mapped as 
> needed.
>
> One idea we've had would be to somehow add the sensor number into the 
> sensor D-Bus object which would mitigate arbitrary numbering as needed.
>
>>  > I know you are working on a proposal to have a customizable map of 
>> sensor number to path. The support is only for threshold based 
>> sensors( targeting only /xyz/openbmc_project/sensors namespace) , it 
>> needs to extend support for discrete sensors. The sensor types are 
>> limited to few types(temperature, voltage,current, fan_tach, power), 
>> need to support other sensor types in the specification. It might not 
>> be possible to deduce the sensor type from the D-Bus object path for 
>> all sensors.
>>
>> 2)  OpenPower systems like Witherspoon and Zaius relies on 
>> eSEL(extended SEL format) to report debug data from the host 
>> firmware. The eSEL data is added to the D-Bus native error log via 
>> the Add SEL entry command. This is not handled with the refactoring.
>>
>>  > This is an openpower requirement and needs to be handled in an 
>> openpower IPMI OEM library/application and not in the phosphor 
>> implementation. A D-Bus error log containing eSEL can be created on 
>> completion of transfer of eSEL and not depend on Add SEL entry. The 
>> callouts associated with the eSEL can be added to the D-Bus error log 
>> by keeping a watch on the IPMI SEL entries. This might need changes 
>> to phosphor-logging API to support adding callout after creation of 
>> error log. This is a proposal and looks viable.
I made progress with moving the openpower specific code from 
phosphor-host-ipmid to openpower IPMI OEM library.
>>
>> 3) The current implementation the Get SEL entry is translating the 
>> D-Bus error log to IPMI SEL format. If there is a FRU callout 
>> associated with the D-Bus error log(D-Bus object paths in the 
>> inventory namespace) it is mapped to the sensor number associated 
>> with FRU and reported through SEL.
>>
>>  > The implementation of the Get SEL entry needs to adapt to include 
>> all the objects paths(the patchsets target only 
>> /xyz/openbmc_project/sensors namespace) and map the D-Bus object path 
>> to the sensor numbers(based on the proposed customizable map).
>>
> This should also be solved by fixing the sensor number mapping.
>
>> 4) In the current implementation all the D-Bus error logs have 1x1 
>> mapping in the IPMI SEL. In the proposed solution the users will have 
>> to rely on both D-Bus error logs and IPMI SEL to get the complete 
>> picture. The customers cannot rely only on IPMI SEL as an event 
>> repository.
>>
>>  > In the current proposal D-Bus error logs reported by BMC will not 
>> be shown by SEL commands. I am okie with Jason's proposal to set up a 
>> D-Bus match (either by ipmid or phosphor-sel-logger) that will watch 
>> for new logging D-Bus objects to be created and add a SEL record for 
>> them on creation.
>>
> Another thought would be to abandon D-Bus-based logging and see if the 
> same information could be placed in the journal.
>
>> https://gerrit.openbmc-project.xyz/#/q/status:open++topic:%22IPMI+SEL%22
>>
>> Regards,
>> Tom
>>
>
> More work is needed to come up with a flexible generic sensor 
> numbering scheme that will work with the various sensor 
> implementations.  I have been assigned to a different task and will 
> not be working on this for a while.  So, as discussed in the call this 
> week, as a stop-gap, I plan to move the journal-based SEL 
> implementation into intel-ipmi-oem for now. We can revisit this as we 
> make progress on the sensor numbering issue. This will also allow 
> everyone to see the full working implementation which may help resolve 
> some questions.
Did you get to make  progress with the flexible generic sensor numbering 
scheme? We will be glad to have your journal based SEL implementation 
upstreamed.
>
> Thanks,
> -Jason
>

Re: IPMI SEL refactoring comments

[Bills, Jason M]

On 1/23/2019 8:11 AM, Tom Joseph wrote:
> Hello Jason,
> 
> 
> On Thursday 29 November 2018 04:51 AM, Bills, Jason M wrote:
>>
>>
>> On 11/26/2018 5:26 AM, Tom Joseph wrote:
>>> Hello Jason,
>>>
>>> I reviewed the patches for the IPMI SEL refactoring and wanted to 
>>> summarize the concerns with the proposed design. There are some 
>>> current features that are broken.
>>>
>>> 1) The sensor numbers are arbitrary in the proposed implementation 
>>> and does not account for the presence/error corresponding to 
>>> inventory objects. On IBM systems the host firmware does not rely on 
>>> the SDR repository in BMC to figure out the sensor numbers, rather 
>>> the sensor numbers are programmed into the  host firmware at the 
>>> build time. So the sensor numbers cannot be arbitrary.
>>>
>> I think this is the main issue: mapping sensor paths to sensor numbers 
>> in a flexible way.  Once we can find a way to allow custom sensor 
>> number mapping, it should allow all types of sensors to be mapped as 
>> needed.
>>
>> One idea we've had would be to somehow add the sensor number into the 
>> sensor D-Bus object which would mitigate arbitrary numbering as needed.
>>
>>>  > I know you are working on a proposal to have a customizable map of 
>>> sensor number to path. The support is only for threshold based 
>>> sensors( targeting only /xyz/openbmc_project/sensors namespace) , it 
>>> needs to extend support for discrete sensors. The sensor types are 
>>> limited to few types(temperature, voltage,current, fan_tach, power), 
>>> need to support other sensor types in the specification. It might not 
>>> be possible to deduce the sensor type from the D-Bus object path for 
>>> all sensors.
>>>
>>> 2)  OpenPower systems like Witherspoon and Zaius relies on 
>>> eSEL(extended SEL format) to report debug data from the host 
>>> firmware. The eSEL data is added to the D-Bus native error log via 
>>> the Add SEL entry command. This is not handled with the refactoring.
>>>
>>>  > This is an openpower requirement and needs to be handled in an 
>>> openpower IPMI OEM library/application and not in the phosphor 
>>> implementation. A D-Bus error log containing eSEL can be created on 
>>> completion of transfer of eSEL and not depend on Add SEL entry. The 
>>> callouts associated with the eSEL can be added to the D-Bus error log 
>>> by keeping a watch on the IPMI SEL entries. This might need changes 
>>> to phosphor-logging API to support adding callout after creation of 
>>> error log. This is a proposal and looks viable.
> I made progress with moving the openpower specific code from 
> phosphor-host-ipmid to openpower IPMI OEM library.
>>>
>>> 3) The current implementation the Get SEL entry is translating the 
>>> D-Bus error log to IPMI SEL format. If there is a FRU callout 
>>> associated with the D-Bus error log(D-Bus object paths in the 
>>> inventory namespace) it is mapped to the sensor number associated 
>>> with FRU and reported through SEL.
>>>
>>>  > The implementation of the Get SEL entry needs to adapt to include 
>>> all the objects paths(the patchsets target only 
>>> /xyz/openbmc_project/sensors namespace) and map the D-Bus object path 
>>> to the sensor numbers(based on the proposed customizable map).
>>>
>> This should also be solved by fixing the sensor number mapping.
>>
>>> 4) In the current implementation all the D-Bus error logs have 1x1 
>>> mapping in the IPMI SEL. In the proposed solution the users will have 
>>> to rely on both D-Bus error logs and IPMI SEL to get the complete 
>>> picture. The customers cannot rely only on IPMI SEL as an event 
>>> repository.
>>>
>>>  > In the current proposal D-Bus error logs reported by BMC will not 
>>> be shown by SEL commands. I am okie with Jason's proposal to set up a 
>>> D-Bus match (either by ipmid or phosphor-sel-logger) that will watch 
>>> for new logging D-Bus objects to be created and add a SEL record for 
>>> them on creation.
>>>
>> Another thought would be to abandon D-Bus-based logging and see if the 
>> same information could be placed in the journal.
>>
>>> https://gerrit.openbmc-project.xyz/#/q/status:open++topic:%22IPMI+SEL%22
>>>
>>> Regards,
>>> Tom
>>>
>>
>> More work is needed to come up with a flexible generic sensor 
>> numbering scheme that will work with the various sensor 
>> implementations.  I have been assigned to a different task and will 
>> not be working on this for a while.  So, as discussed in the call this 
>> week, as a stop-gap, I plan to move the journal-based SEL 
>> implementation into intel-ipmi-oem for now. We can revisit this as we 
>> make progress on the sensor numbering issue. This will also allow 
>> everyone to see the full working implementation which may help resolve 
>> some questions.
> Did you get to make  progress with the flexible generic sensor numbering 
> scheme? We will be glad to have your journal based SEL implementation 
> upstreamed.

Sorry, I'm still mostly consumed with other tasks and haven't had much 
chance to focus on this.  We've had a few thoughts on ways that we could 
make the sensor numbering more flexible and able to handle dynamic and 
hardcoded numbering.

One thought is to use the map proposed here 
https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-host-ipmid/+/17637/1 
and offer a method to update the sensor numbers with hardcoded sensor 
data.  This map could then be referenced for all sensor numbers in the 
system.

Another thought would be to add an optional sensor number value into the 
sensor D-Bus object.  Then if a sensor number is found, use it; 
otherwise, assign a dynamic number.

Will either of these options provide a feasible solution for sensor 
numbering?

>>
>> Thanks,
>> -Jason
>>
> 

Re: IPMI SEL refactoring comments

[Tom Joseph]

On Thursday 24 January 2019 06:18 AM, Bills, Jason M wrote:
>
>
> On 1/23/2019 8:11 AM, Tom Joseph wrote:
>> Hello Jason,
>>
>>
>> On Thursday 29 November 2018 04:51 AM, Bills, Jason M wrote:
>>>
>>>
>>> On 11/26/2018 5:26 AM, Tom Joseph wrote:
>>>> Hello Jason,
>>>>
>>>> I reviewed the patches for the IPMI SEL refactoring and wanted to 
>>>> summarize the concerns with the proposed design. There are some 
>>>> current features that are broken.
>>>>
>>>> 1) The sensor numbers are arbitrary in the proposed implementation 
>>>> and does not account for the presence/error corresponding to 
>>>> inventory objects. On IBM systems the host firmware does not rely 
>>>> on the SDR repository in BMC to figure out the sensor numbers, 
>>>> rather the sensor numbers are programmed into the  host firmware at 
>>>> the build time. So the sensor numbers cannot be arbitrary.
>>>>
>>> I think this is the main issue: mapping sensor paths to sensor 
>>> numbers in a flexible way.  Once we can find a way to allow custom 
>>> sensor number mapping, it should allow all types of sensors to be 
>>> mapped as needed.
>>>
>>> One idea we've had would be to somehow add the sensor number into 
>>> the sensor D-Bus object which would mitigate arbitrary numbering as 
>>> needed.
>>>
>>>>  > I know you are working on a proposal to have a customizable map 
>>>> of sensor number to path. The support is only for threshold based 
>>>> sensors( targeting only /xyz/openbmc_project/sensors namespace) , 
>>>> it needs to extend support for discrete sensors. The sensor types 
>>>> are limited to few types(temperature, voltage,current, fan_tach, 
>>>> power), need to support other sensor types in the specification. It 
>>>> might not be possible to deduce the sensor type from the D-Bus 
>>>> object path for all sensors.
>>>>
>>>> 2)  OpenPower systems like Witherspoon and Zaius relies on 
>>>> eSEL(extended SEL format) to report debug data from the host 
>>>> firmware. The eSEL data is added to the D-Bus native error log via 
>>>> the Add SEL entry command. This is not handled with the refactoring.
>>>>
>>>>  > This is an openpower requirement and needs to be handled in an 
>>>> openpower IPMI OEM library/application and not in the phosphor 
>>>> implementation. A D-Bus error log containing eSEL can be created on 
>>>> completion of transfer of eSEL and not depend on Add SEL entry. The 
>>>> callouts associated with the eSEL can be added to the D-Bus error 
>>>> log by keeping a watch on the IPMI SEL entries. This might need 
>>>> changes to phosphor-logging API to support adding callout after 
>>>> creation of error log. This is a proposal and looks viable.
>> I made progress with moving the openpower specific code from 
>> phosphor-host-ipmid to openpower IPMI OEM library.
>>>>
>>>> 3) The current implementation the Get SEL entry is translating the 
>>>> D-Bus error log to IPMI SEL format. If there is a FRU callout 
>>>> associated with the D-Bus error log(D-Bus object paths in the 
>>>> inventory namespace) it is mapped to the sensor number associated 
>>>> with FRU and reported through SEL.
>>>>
>>>>  > The implementation of the Get SEL entry needs to adapt to 
>>>> include all the objects paths(the patchsets target only 
>>>> /xyz/openbmc_project/sensors namespace) and map the D-Bus object 
>>>> path to the sensor numbers(based on the proposed customizable map).
>>>>
>>> This should also be solved by fixing the sensor number mapping.
>>>
>>>> 4) In the current implementation all the D-Bus error logs have 1x1 
>>>> mapping in the IPMI SEL. In the proposed solution the users will 
>>>> have to rely on both D-Bus error logs and IPMI SEL to get the 
>>>> complete picture. The customers cannot rely only on IPMI SEL as an 
>>>> event repository.
>>>>
>>>>  > In the current proposal D-Bus error logs reported by BMC will 
>>>> not be shown by SEL commands. I am okie with Jason's proposal to 
>>>> set up a D-Bus match (either by ipmid or phosphor-sel-logger) that 
>>>> will watch for new logging D-Bus objects to be created and add a 
>>>> SEL record for them on creation.
>>>>
>>> Another thought would be to abandon D-Bus-based logging and see if 
>>> the same information could be placed in the journal.
>>>
>>>> https://gerrit.openbmc-project.xyz/#/q/status:open++topic:%22IPMI+SEL%22 
>>>>
>>>>
>>>> Regards,
>>>> Tom
>>>>
>>>
>>> More work is needed to come up with a flexible generic sensor 
>>> numbering scheme that will work with the various sensor 
>>> implementations.  I have been assigned to a different task and will 
>>> not be working on this for a while.  So, as discussed in the call 
>>> this week, as a stop-gap, I plan to move the journal-based SEL 
>>> implementation into intel-ipmi-oem for now. We can revisit this as 
>>> we make progress on the sensor numbering issue. This will also allow 
>>> everyone to see the full working implementation which may help 
>>> resolve some questions.
>> Did you get to make  progress with the flexible generic sensor 
>> numbering scheme? We will be glad to have your journal based SEL 
>> implementation upstreamed.
>
> Sorry, I'm still mostly consumed with other tasks and haven't had much 
> chance to focus on this.  We've had a few thoughts on ways that we 
> could make the sensor numbering more flexible and able to handle 
> dynamic and hardcoded numbering.
>
> One thought is to use the map proposed here 
> https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-host-ipmid/+/17637/1 
> and offer a method to update the sensor numbers with hardcoded sensor 
> data.  This map could then be referenced for all sensor numbers in the 
> system.

My thought is to have a JSON file that has sensor number to object 
mappings( and probably more relevant IPMI information) that can be 
consumed by any IPMI relevant applications. So the systems that rely on 
hardcoded sensor numbers will provide the mappings otherwise generate 
dynamic sensor numbers. The map described in the patch can be updated 
with the hardcoded sensor data.

> Another thought would be to add an optional sensor number value into 
> the sensor D-Bus object.  Then if a sensor number is found, use it; 
> otherwise, assign a dynamic number.

I am not excited having IPMI sensor information in the sensor D-Bus 
object. It is one of the design goals to have the D-Bus representation 
separated from the external interface representations(like IPMI).

>
> Will either of these options provide a feasible solution for sensor 
> numbering?
>
>>>
>>> Thanks,
>>> -Jason
>>>
>>
>