libpldmresponder comments

[Supreeth Venkatesh]

On Fri, 2019-10-04 at 09:28 -0500, Deepak Kodihalli wrote:
> On 04/10/19 7:26 PM, Supreeth Venkatesh wrote:
> > Hello Deepak/all,
> > 
> > Sorry for the late comments/feedback.
> 
> No problem, thanks for the feedback Supreeth!
> 
> > I was looking at porting libpldmresponder and libpldm to an Arm
> > based 
> > platform.
> 
> libpldm - sure, it is meant to be portable across OpenBMC, but 
> libpldmresponder is not :)
> 
> > These are  few observations with the design, some of them were
> > discussed 
> > during OSF OpenBMC Hackathon, summarizing them here:
> > 
> > Assumption was that libpldmresponder can be easily ported to
> > Host/other 
> > Satellite/Service Management controller,
> 
> libpldmresponder was not meant to be ported outside OpenBMC - it's
> the 
> OpenBMC implementation of the BMC as a PLDM responder.

>From our previous discussion, I was under the impression you wanted
libPLDMresponder to get publicity and ported over as well.
If I was mistaken, the rest of discussion is moot.



> 
> > However, in the current design,
> > 
> >  1. libpldmresponder implements standard Commands/APIs defined by
> > PLDM
> >     specifications in C++.
> >  2. libpldmresponder PDR, BIOS config structures are defined by
> > PLDM
> >     specifications, However, the library uses Json format, thus
> > making
> >     JSON parser mandatory for 
> > Host/Service management controller firmware.
> > 
> >  3. libpldmresponder has DBUS/other OpenBMC implementation
> > dependencies,
> >     thus making portability harder. >  4. I guess the expectation
> > when we started with the design was that
> >     there will be one **single** library which will handle all pldm
> >     requests/responses and 
> > 
> > upper layer application/Daemon will call the APIs provided by PLDM 
> > library to implement use cases as they fit.
> 
> https://github.com/openbmc/docs/blob/master/designs/pldm-stack.md
> makes 
> the distinction between a portable libpldm (which handles the
> protocol 
> encode/decode), and an OpenBMC specific responder implementation.
> 
> >  5. Libpldm also has dependencies on OpenBMC structures/DBUS
> > objects,
> >     making it a little harder to port.
> 
> No, libpldm has no OpenBMC dependencies. It for example is used by 
If I remember right, in libpldm, for getting date and time, there was
dependency on OpenBMC object to get it from the timer as opposed to
upper level app calling into libpldm and providing date and time?
Now that I look at the code in repo, it does not seem to be the case. 
Please ignore my rant. 

> IBM's 
> host firmware stacks today without any code changes. Can you point me
> to 
> where you see D-Bus dependencies in libpldm? Do you just mean you
> find 
> this hard to build outside an OpenBMC environment?


> 
> > Please let me know, how I can help fix some of these, so that it
> > is 
> > easily portable.
> 
> No change should be needed to libpldm code, it should already be 
> portable like noted earlier. As far as the libpldmresponder is 
> considered, like we discussed at the OSFC, one thing we could do is
> to 
> write a responder API layer, the implementation of which is platform 
> (for eg OpenBMC/ARM) specific. Would you like to propose a design
> update 
> for this to the existing document? I think we need to understand the 
> usefulness of such a layer though. I mean for example if you consider
> a 
> PLDM command along the lines of ReadSensor, such a sensor maybe a D-
> Bus 
> object on OpenBMC and something else on the ARM platform, those 
> specifics *must* be implemented on the platform, so what would the 
> portable layer consist of? Those details would be good to capture in
> the 
> design doc.
Ok. Thanks for the example command.
Let's take the example of "GetSensorReading" as specified by one of the
PLDM specification,

Use case: BMC is trying to get the current reading from a platform
smart (MCTP enabled) sensor.

Request is:
GetSensorReading (uint16 sensorID, bool8 rearmEventStatus) 

Request Message to the smart MCTP enabled sensor (not including headers
and other field for illustration only) will be somthing like this
GetSensorReading | Sensor ID | rearmEventStatus

Response will contain the following fields:
enum8 completionCode, 
enum8 sensorDataSize, 
enum8 sensorOperationalState,
bool8 sensorEventMessageEnable,
enum8 presentState,
enum8 previousState,
enum8 eventState,
var   presentReading
These fields will be filled in by smart MCTP enabled sensor or
microcontroller(MCTP endpoint) which controls the sensor.

Hence for this use case, my current thinking is that PLDM library will
provide the standard template for GetSensorReading API as a function
pointer.
The implementation of this GetSensorReading in openBMC and in smart
sensor software/firmware will be different. 
To elaborate a little further, the implementation of this API in
OpenBMC for this particular use case is to fill in the PLDM request
fields and send it over the  to the smart sensor over MCTP over sensor
specific physical media and get the response. (or can use DBUS
specifics to get the response, if possible)

The smart sensor implementation of GetSensorReading will be different
and it will be to parse PLDM request and then to fill in the response
fields and send back the PLDM message over MCTP over sensor specific
physical media. 

So, my thinking is to have a wrapper with function pointers, which will
have all standard  commands defined by the PLDM specification.
This wrapper can then be ported across management controller software
and devices alike.

I am thinking from the specification view point, but from your email it
looks like to you want merge DBUS and PLDM concept. please confirm.

I can add more details when I propose a design.

 
> 
> > Thanks,
> > 
> > Supreeth
> > 
> > IMPORTANT NOTICE: The contents of this email and any attachments
> > are 
> > confidential and may also be privileged. If you are not the
> > intended 
> > recipient, please notify the sender immediately and do not disclose
> > the 
> > contents to any other person, use it for any purpose, or store or
> > copy 
> > the information in any medium. Thank you.
> 
>