Adding support for MM Communicate v3 #10854

kuqin12 · 2025-03-13T06:57:43Z

Description

As PI spec v1.9 introduced MM communicate v3, this change adds the support for MM communicate v3 in MdePkg, MdeModulePkg, and StandaloneMmPkg.

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

This change is tested on QEMU virtual platform and verified variable service can operate through both v1 and v3 mm communicate protocols.

Integration Instructions

Locate MM communicate v3 PPI/Protocol, and populate the communication buffer as defined in the spec.

tianocore-assign-reviewers · 2025-03-25T07:55:44Z

⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.

Non-collaborators requested:

@sweeaun

Attn Admins:

Admin Instructions:

Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
If they are no longer needed as reviewers, remove them from Maintainers.txt

MdePkg/Include/Protocol/MmCommunication3.h

…ePkg REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 This change introduces a new definition for MM communicate header structure, intending to provide better portability between different architectures (IA32 & X64) and adapt to flexible array supported by modern compilers. The original MessageLength field of EFI_MM_COMMUNICATE_HEADER, as a generic definition, was used for both PEI and DXE MM communication. On a system that supports PEI MM launch, but operates PEI in 32bit mode and MM foundation in 64bit, the current EFI_MM_COMMUNICATE_HEADER definition will cause structure parse error due to UINTN used. This introduction removes the architecture dependent field by defining this field as UINT64. The new signature could help identifying whether the data received is compiliant with this new data structure, which will help for binary release modules to identify usage of legacy data structure. BufferSize field is also added to indicate the full range of communicate region available to the SMI handler. The data field of MM communicate message is replaced with flexible array to allow users not having to consume extra data during communicate and author code more intrinsically. Cc: Michael D Kinney <[email protected]> Cc: Liming Gao <[email protected]> Cc: Zhiguang Liu <[email protected]> Signed-off-by: Kun Qin <[email protected]>

…MdePkg REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 This change introduces a new definition for MM communicate protocol v3. This protocol will be installed under a new GUID in contrast to exisiting EFI_MM_COMMUNICATION_PROTOCOL. Data communicated to MM through EFI_MM_COMMUNICATION3_PROTOCOL should always start with EFI_MM_COMMUNICATE_HEADER_V3 with its HeaderGuid, Signature and Version fields properly populated. Cc: Michael D Kinney <[email protected]> Cc: Liming Gao <[email protected]> Cc: Zhiguang Liu <[email protected]> Signed-off-by: Kun Qin <[email protected]>

…dePkg REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 This change introduces a new definition for MM communicate PPI v3. This PPI will be installed under a new GUID in contrast to exisiting EFI_PEI_MM_COMMUNICATION_PPI. Data communicated to MM through EFI_PEI_MM_COMMUNICATION3_PPI should always start with EFI_MM_COMMUNICATE_HEADER_V3 with its HeaderGuid, Signature and Version fields properly populated. Signed-off-by: Kun Qin <[email protected]>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 MM communicate protocols are expanded with EFI_MM_COMMUNICATE_HEADER_V3 structure that cooperates with updated field types and flexible array. The PiSmmCore implementation is updated to detect and process incoming data accordingly. Two checks are also performed to prevent legacy communicate data or unsupported data is fed into MM core under agreed header guid. Cc: Jian J Wang <[email protected]> Cc: Hao A Wu <[email protected]> Cc: Eric Dong <[email protected]> Cc: Ray Ni <[email protected]> Signed-off-by: Kun Qin <[email protected]>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 MM communicate protocols are expanded with EFI_MM_COMMUNICATE_HEADER_V3 structure that cooperates with updated field types and flexible array. The PiSmmCore implementation is updated to detect and process incoming data accordingly. Two checks are also performed to prevent legacy communicate data or unsupported data is fed into MM core under agreed header guid. Signed-off-by: Kun Qin <[email protected]>

…icate REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 This change added support of installing `EFI_MM_COMMUNICATION3_PROTOCOL`. MmCommunicate v3 routine that calculates message length is also updated to remove ambiguity in contrast to v1 routine. Signed-off-by: Kun Qin <[email protected]>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 This change added support of installing `EFI_PEI_MM_COMMUNICATION3_PPI`. MmCommunicate v3 routine will no longer need to supply buffer length in the input parameters but instead inference from the returned communicate buffer. Signed-off-by: Kun Qin <[email protected]>

lgao4 · 2025-03-31T07:09:18Z

This patch is good to me .

jiaxinwu · 2025-04-01T08:13:34Z

MdePkg/Include/Protocol/MmCommunication3.h

+  EFI_MM_COMMUNICATE3    Communicate;
+};
+
+extern EFI_GUID  gEfiMmCommunication3ProtocolGuid;


no plan to support the V3 protocol in edk2\StandaloneMmPkg\Drivers\MmCommunicationDxe\MmCommunicationDxe.inf?

jiaxinwu · 2025-04-02T07:33:30Z

StandaloneMmPkg/Core/StandaloneMmCore.c

+        Status      = MmiManage (
+                        CommGuid,
+                        NULL,
+                        CommData,


Bug here!!! CommData should be pointed to the mInternalCommBufferCopy, which is the shadow MM Communication Buffer, not the original MM Communication Buffer from non-MMRAM.

have you passed the standalonemm test? above bug will block the standalonemm solution.

niruiyu · 2025-04-02T07:44:07Z

MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c

+MmCommunicationMmCommunicate3 (
+  IN CONST EFI_MM_COMMUNICATION3_PROTOCOL  *This,
+  IN OUT VOID                              *CommBufferPhysical,
+  IN OUT VOID                              *CommBufferVirtual
+  )
+{
+  EFI_STATUS                    Status;
+  EFI_MM_COMMUNICATE_HEADER_V3  *CommunicateHeader;
+  BOOLEAN                       OldInSmm;
+  UINT64                        MinCommSize;
+  UINT64                        TempCommSize;


Can you implement a common InternalCommunicate() function and all 1-2-3 Communicate() call the internal function?

niruiyu · 2025-04-02T07:45:41Z

MdePkg/MdePkg.dec

+  #
+  # GUID indicates the MM communication data is compliant with v3 communication header.
+  #
+  gCommunicateHeaderV3Guid       = { 0x68e8c853, 0x2ba9, 0x4dd7, { 0x9a, 0xc0, 0x91, 0xe1, 0x61, 0x55, 0xc9, 0x35 } }
+


A spec-defined GUID should have Efi prefix: gEfiMmCommunicateHeaderV3Guid

kuqin12 mentioned this pull request Mar 13, 2025

EFI_MM_COMMUNICATE_HEADER has architecture dependent sized field (Bugzilla Bug 3398) #10094

Open

kuqin12 force-pushed the BZ3398-MmCommunicate-Length-v5 branch 6 times, most recently from 85e5f61 to a272678 Compare March 14, 2025 06:36

kuqin12 force-pushed the BZ3398-MmCommunicate-Length-v5 branch from a272678 to a36eb3a Compare March 25, 2025 05:39

github-actions bot added the impact:security This change has a direct security impact such as changing a crypto algorithm. label Mar 25, 2025

kuqin12 force-pushed the BZ3398-MmCommunicate-Length-v5 branch from a36eb3a to 35f405c Compare March 25, 2025 07:55

kuqin12 marked this pull request as ready for review March 25, 2025 07:55

tianocore-assign-reviewers bot requested review from ardbiesheuvel, jiaxinwu, lgao4, mdkinney, niruiyu, samimujawar and td36 March 25, 2025 07:55

lgao4 reviewed Mar 26, 2025

View reviewed changes

MdePkg/Include/Protocol/MmCommunication3.h Outdated Show resolved Hide resolved

kuqin12 and others added 7 commits March 27, 2025 00:28

kuqin12 force-pushed the BZ3398-MmCommunicate-Length-v5 branch from 35f405c to d1c3359 Compare March 27, 2025 07:29

lgao4 approved these changes Mar 31, 2025

View reviewed changes

jiaxinwu reviewed Apr 1, 2025

View reviewed changes

jiaxinwu suggested changes Apr 2, 2025

View reviewed changes

niruiyu reviewed Apr 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for MM Communicate v3 #10854

Adding support for MM Communicate v3 #10854

kuqin12 commented Mar 13, 2025 •

edited

Loading

tianocore-assign-reviewers bot commented Mar 25, 2025

lgao4 commented Mar 31, 2025

jiaxinwu Apr 1, 2025

jiaxinwu Apr 2, 2025

jiaxinwu Apr 2, 2025

niruiyu Apr 2, 2025

niruiyu Apr 2, 2025

Adding support for MM Communicate v3 #10854

Are you sure you want to change the base?

Adding support for MM Communicate v3 #10854

Conversation

kuqin12 commented Mar 13, 2025 • edited Loading

Description

How This Was Tested

Integration Instructions

tianocore-assign-reviewers bot commented Mar 25, 2025

lgao4 commented Mar 31, 2025

jiaxinwu Apr 1, 2025

Choose a reason for hiding this comment

jiaxinwu Apr 2, 2025

Choose a reason for hiding this comment

jiaxinwu Apr 2, 2025

Choose a reason for hiding this comment

niruiyu Apr 2, 2025

Choose a reason for hiding this comment

niruiyu Apr 2, 2025

Choose a reason for hiding this comment

kuqin12 commented Mar 13, 2025 •

edited

Loading