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