On 10/28/2023 10:03 AM, Laszlo Ersek wrote:
On 10/27/23 18:08, Michael Kubacki wrote:
This allows ambiguous "platform" code in the critical path of the MM
core. Is this necessary?

Do you need this for one feature that others might too and can be
abstracted? Or, do you plan to perform an unknown and arbitrary number
of changes behind the hook over time?

Not sure if it's necessary, but it's somewhat "customary". Platform hook
libs are not uncommon; for example PiSmmCpuDxeSmm consumes
SmmCpuPlatformHookLib and SmmCpuFeaturesLib.

My request would be for Wei to file a TianoCore Feature Request
bugzilla, with a bit more information than "We need this library to
implement our feature". Reference the BZ in the commit messages, then
add the BZ to the
<https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning>.

That would be helpful. Hooks are sometimes necessary of course, but I generally consider these open-ended hook libraries a design wart. They introduce an inherently incohesive interface and a convenient place for new APIs to land instead of more focused interfaces that may be more appropriate. This extends to the implementation where, over time, it can become a mess of dependencies and unrelated code coupled together.

I'd like to see if there's a way to avoid that or such a library interface is the best choice.

Laszlo


Thanks,
Michael

On 10/26/2023 11:28 PM, Xu, Wei6 wrote:
This patch set is to add StandaloneMmCorePlatformHookLib into
StandaloneMmCore.

This library class defines a set of platform hooks called by the
Standalone Mm Core. With this library, platform can perform specific
tasks before and after invoking registered MMI handlers.
We need this library to implement our feature.

PR: https://github.com/tianocore/edk2/pull/4949



Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>

Cc: Sami Mujawar <sami.muja...@arm.com>

Cc: Ray Ni <ray...@intel.com>


Wei6 Xu (2):
    StandaloneMmPkg: Add Standalone Mm Core platform hook lib.
    StandaloneMmPkg/Core: Consumes Standalone Mm Core Platform Hook Lib.

   StandaloneMmPkg/Core/StandaloneMmCore.c       |  7 ++-
   .../StandaloneMmCorePlatformHookLibNull.c     | 45 +++++++++++++++++++
   StandaloneMmPkg/Core/StandaloneMmCore.h       |  1 +
   StandaloneMmPkg/Core/StandaloneMmCore.inf     |  1 +
   .../Library/StandaloneMmCorePlatformHookLib.h | 44 ++++++++++++++++++
   .../StandaloneMmCorePlatformHookLibNull.inf   | 30 +++++++++++++
   StandaloneMmPkg/StandaloneMmPkg.dec           |  4 ++
   StandaloneMmPkg/StandaloneMmPkg.dsc           |  2 +
   8 files changed, 133 insertions(+), 1 deletion(-)
   create mode 100644
StandaloneMmPkg/Library/StandaloneMmCorePlatformHookLibNull/StandaloneMmCorePlatformHookLibNull.c
   create mode 100644
StandaloneMmPkg/Include/Library/StandaloneMmCorePlatformHookLib.h
   create mode 100644
StandaloneMmPkg/Library/StandaloneMmCorePlatformHookLibNull/StandaloneMmCorePlatformHookLibNull.inf













-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110336): https://edk2.groups.io/g/devel/message/110336
Mute This Topic: https://groups.io/mt/102214566/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to