In looking at the code, this is what I see:
edk2:
There is an IpmiLib.h library class which abstracts the
IpmiPpi.h/IpmiProtocol.h use
There is an IpmiPpi.h/IpmiProtocol.h which provide an interface to send
a command "via IPMI"
Current IpmiPpi.h/IpmiProtocol.h implementations use IpmiBaseLib
library class to send commands
edk2-platforms/Features/Intel/OutOfBandManagement/IpmiFeaturePkg:
There is an IpmiBaseLib library class which abstracts
IpmiTransportPpi.h/IpmiTransportProtocol.h use
There is an IpmiTransportPpi.h/IpmiTransportProtocol.h that abstracts
the transport protocol.
Currently IpmiTransportPpi.h/IpmiTransportProtocol.h are implemented in
"GenericIpmi".
It appears to me that they are meant to be the same API. The IpmiFeaturePkg
transport version adds a function to GetBmcStatus, but it seems like
IpmiSubmitCommand is meant to be the same function throughout.
I don't find any use of the GetBmcStatus function that is in the IpmiFeaturePkg
but not the edk2 API. I am not sure what to make of that. I think we need
more feedback from current consumers to resolve that.
When I look at our internal reference version which is an ancestor to the
edk2-platforms/Feature/Intel.../IpmiFeaturePkg version, there is no use of the
IpmiLib.h/IpmiPpi.h/IpmiProtocol.h. There are additional fields in interfaces,
like GetBmcStatus and data values, but they seem to have been redesigned out
when open sourcing the interfaces.
My main questions:
To your understanding, are the IpmiLib.h/IpmiPpi.h/IpmiProtocol.h and the
IpmiBaseLib.h/IpmiTransportPpi.h/IpmiTransportProtocol.h simply two versions of
basically the same with a library API abstracting a dynamic phase specific IPMI
transport API?
Why does ManageabilityPkg use IpmiBaseLib and not IpmiLib? Is it more than a
temporary solution?
High level feedback:
0001 - I think we should not make this change. It impacts any existing users
for no reason I can justify.
0002 - Not sure - per my understanding of the stack, it doesn't quite fit the
code in an obvious way.
0003 - More description of the design and duplication, the roles of the library
class and the PPI/protocols, and the relationship to other IPMI code in the
repos.
0004 - Minor feedback only
0005/0006 - I think that we should skip the step of using IpmiBaseLib and just
implement the modified version of GenericIpmi to support the edk2 API stack
design.
I will send more specific details in response to each commit shortly.
Regards,
Isaac
-----Original Message-----
From: abner.ch...@amd.com <abner.ch...@amd.com>
Sent: Tuesday, February 7, 2023 8:22 AM
To: devel@edk2.groups.io
Cc: Gao, Liming <gaolim...@byosoft.com.cn>; Oram, Isaac W
<isaac.w.o...@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desim...@intel.com>; Nickle Wang <nick...@nvidia.com>; Igor
Kulchytskyy <ig...@ami.com>; Abdul Lateef Attar <abdat...@amd.com>; Leif
Lindholm <quic_llind...@quicinc.com>; Kinney, Michael D
<michael.d.kin...@intel.com>
Subject: [edk2-platforms][PATCH 0/7] Implementation of IPMI Protocol
From: Abner Chang <abner.ch...@amd.com>
This change implementes IPMI Protocol and PPI in the new introduced
ManageabilityPkg (described in below email)
https://edk2.groups.io/g/devel/message/95579?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3ACreated%2C%2CManageability%2C20%2C2%2C0%2C94572748
BZ #4336:
The change also fixes the confusion (Patch 1/7) of IpmiSubmitCommand() deinfed
in IPMI Transport protocol.
You can skip reviewing on patch 2/7 as it is an image file.
Signed-off-by: Abner Chang <abner.ch...@amd.com>
Cc: Liming Gao <gaolim...@byosoft.com.cn>
Cc: Isaac Oram <isaac.w.o...@intel.com>
Cc: Nate DeSimone <nathaniel.l.desim...@intel.com>
Cc: Nickle Wang <nick...@nvidia.com>
Cc: Igor Kulchytskyy <ig...@ami.com>
Cc: Abdul Lateef Attar <abdat...@amd.com>
Cc: Leif Lindholm <quic_llind...@quicinc.com>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Abner Chang (7):
IpmiFeaturePkg: Rename IpmiSubmitCommand function
ManageabilityPkg: Add diagrams
ManageabilityPkg: Add Readme file
ManageabilityPkg: Initial package
ManageabilityPkg: Implement Ipmi Protocol/Ppi
IpmiProtocol: Add to Manageability Package
edk2-platforms: Maintainers.txt
.../ManageabilityPkg/ManageabilityPkg.dec | 18 ++++
.../Include/CommonLibs.dsc.inc | 52 +++++++++
.../ManageabilityPkg/ManageabilityPkg.dsc | 27 +++++
.../IpmiProtocol/Dxe/IpmiProtocolDxe.inf | 37 +++++++
.../Universal/IpmiProtocol/Pei/IpmiPpiPei.inf | 38 +++++++
.../IpmiProtocol/Smm/IpmiProtocolSmm.inf | 40 +++++++
.../Include/Library/IpmiBaseLib.h | 2 +-
.../Include/Ppi/IpmiTransportPpi.h | 2 +-
.../Include/Protocol/IpmiTransportProtocol.h | 2 +-
.../IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c | 2 +-
.../GenericIpmi/Pei/PeiGenericIpmi.c | 2 +-
.../GenericIpmi/Smm/SmmGenericIpmi.c | 2 +-
.../Library/IpmiBaseLib/IpmiBaseLib.c | 4 +-
.../Library/IpmiBaseLibNull/IpmiBaseLibNull.c | 2 +-
.../IpmiCommandLib/IpmiCommandLibNetFnApp.c | 26 ++---
.../IpmiCommandLibNetFnChassis.c | 12 +--
.../IpmiCommandLibNetFnStorage.c | 24 ++---
.../IpmiCommandLibNetFnTransport.c | 8 +-
.../Library/PeiIpmiBaseLib/PeiIpmiBaseLib.c | 4 +-
.../Library/SmmIpmiBaseLib/SmmIpmiBaseLib.c | 4 +-
.../Universal/IpmiProtocol/Dxe/IpmiProtocol.c | 97 +++++++++++++++++
.../Universal/IpmiProtocol/Pei/IpmiPpi.c | 102 ++++++++++++++++++
.../Universal/IpmiProtocol/Smm/IpmiProtocol.c | 98 +++++++++++++++++
.../Ipmi/Library/IpmiLibKcs/IpmiLibKcs.c | 12 +--
Features/ManageabilityPkg/Readme.md | 37 +++++++
.../Media/ManageabilityDriverStack.svg | 1 +
Maintainers.txt | 9 +-
27 files changed, 608 insertions(+), 56 deletions(-) create mode 100644
Features/ManageabilityPkg/ManageabilityPkg.dec
create mode 100644 Features/ManageabilityPkg/Include/CommonLibs.dsc.inc
create mode 100644 Features/ManageabilityPkg/ManageabilityPkg.dsc
create mode 100644
Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocolDxe.inf
create mode 100644
Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiPei.inf
create mode 100644
Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocolSmm.inf
create mode 100644
Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol.c
create mode 100644
Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c
create mode 100644
Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.c
create mode 100644 Features/ManageabilityPkg/Readme.md
create mode 100644
Features/ManageabilityPkg/Documents/Media/ManageabilityDriverStack.svg
--
2.37.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100253): https://edk2.groups.io/g/devel/message/100253
Mute This Topic: https://groups.io/mt/96810540/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
