MdeModulePkg/DxeIpmiLibIpmiProtocol: Add depex

[nhivp]

Description

The library should always consume the gIpmiProtocolGuid protocol. This patch is to correct it and add the protocol GUID to Depex section to improve driver boot order.

• Breaking change?
  • Breaking change - Does this PR cause a break in build or boot behavior?
  • Examples: Does it add a new library class or move a module to a different repo.
• Impacts security?
  • Security - Does this PR have a direct security impact?
  • Examples: Crypto algorithm change or buffer overflow fix.
• Includes tests?
  • Tests - Does this PR include any explicit test code?
  • Examples: Unit tests or integration tests.

How This Was Tested

<Describe the test(s) that were run to verify the changes.>

Integration Instructions

<Describe how these changes should be integrated. Use N/A if nothing is required.>

[mdkinney]

If this is made an unconditional dependency, then the lookup of the protocol should be done in a CONSTRUCTOR and IpmiSubmitCommand() can be simplified to assume mIpmiProtocol is always valid.

[nhivp]

Hi @mdkinney, thanks for your reviewing.

The DxeIpmiLibIpmiProtocol always consumes the gIpmiProtocolGuid protocol, and declaring it in the [Depex] section ensures that the library consumers execute only after the protocol is available. Without this, there is no guarantee of proper driver execution order.

Regarding your suggestion to initialize mIpmiProtocol in a LIBRARY_CONSTRUCTOR, I agree this approach would simplify IpmiSubmitCommand() by removing the need for runtime protocol lookup. However, the current design expects the protocol to be available at the time of library use, and if it is not, calls to IpmiSubmitCommand() will fail as designed.

[mdkinney]

If you add to [Depex], then the protocol is guaranteed to be present when the entry point of this module is executed and CONSTRUCTOR services are executed. So looking up time of use is not required if it is added to [Depex]. If it is not added to [Depex], then the protocol will be checked on each use and can fail if the protocol is not present yet. If platforms depend on lazy evaluation of this protocol being present, then this change can not be accepted.

[nhivp]

Thanks @mdkinney for the clarification.

The check for mIpmiProtocol == NULL in the IpmiSubmitCommand () function is intended to perform a one-time lookup of the gIpmiProtocolGuid protocol. Once the protocol is located, mIpmiProtocol is cached for subsequent use, ensuring minimal overhead after the initial lookup.

If it is not added to [Depex], then the protocol will be checked on each use and can fail if the protocol is not present yet.

The primary intent of this change is to ensure that the DxeIpmiLibIpmiProtocol library consumes the gIpmiProtocolGuid protocol reliably. Adding the protocol to the [Depex] section consistently enforces proper driver execution order and guarantees the protocol is available when the consumer's DXE Entry Point is executed.

If platforms depend on lazy evaluation of this protocol being present, then this change can not be accepted.

I'm unsure if I understand correctly. Adding the gIpmiProtocolGuid protocol to the [Depex] section of individual consumer drivers could address the issue. However, this change aims to simplify dependency management by moving the responsibility to the DxeIpmiLibIpmiProtocol library itself.

By declaring the protocol in the library’s [Depex] section:

1. Existing and future consumers of the library are relieved of the need to explicitly handle the gIpmiProtocolGuid dependency.

2. The library fully abstracts the protocol dependency, adhering to the principle of encapsulation and ensuring a cleaner, more maintainable design.

Additionally, this change ensures consistent behavior across all consumers without requiring them to have knowledge of the dependent protocol.