> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun Qin
> Sent: Thursday, June 10, 2021 9:43 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Liming Gao
> <gaolim...@byosoft.com.cn>; Liu, Zhiguang <zhiguang....@intel.com>;
> Andrew Fish <af...@apple.com>; Laszlo Ersek <ler...@redhat.com>; Leif
> Lindholm <l...@nuviainc.com>
> Subject: [edk2-devel] [PATCH v1 1/5] EDK2 Code First: PI Specification:
> EFI_MM_COMMUNICATE_HEADER Update
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430
>
> This change includes specification update markdown file that describes the
> proposed PI Specification v1.7 Errata A in detail and potential impact to the
> existing codebase.
>
> Cc: Michael D Kinney <michael.d.kin...@intel.com>
> Cc: Liming Gao <gaolim...@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang....@intel.com>
> Cc: Andrew Fish <af...@apple.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Cc: Leif Lindholm <l...@nuviainc.com>
>
> Signed-off-by: Kun Qin <kuqi...@gmail.com>
> ---
> BZ3430-SpecChange.md | 88 ++++++++++++++++++++
> 1 file changed, 88 insertions(+)
>
> diff --git a/BZ3430-SpecChange.md b/BZ3430-SpecChange.md new file mode
> 100644 index 000000000000..33a1ffda447b
> --- /dev/null
> +++ b/BZ3430-SpecChange.md
> @@ -0,0 +1,88 @@
> +# Title: Change MessageLength Field of EFI_MM_COMMUNICATE_HEADER
> to
> +UINT64
> +
> +## Status: Draft
> +
> +## Document: UEFI Platform Initialization Specification Version 1.7
> +Errata A
> +
> +## License
> +
> +SPDX-License-Identifier: CC-BY-4.0
> +
> +## Submitter: [TianoCore Community](https://www.tianocore.org)
> +
> +## Summary of the change
> +
> +Change the `MessageLength` Field of `EFI_MM_COMMUNICATE_HEADER`
> from UINTN to UINT64 to remove architecture dependency:
> +
> +```c
> +typedef struct {
> + EFI_GUID HeaderGuid;
> + UINT64 MessageLength;
> + UINT8 Data[ANYSIZE_ARRAY];
> +} EFI_MM_COMMUNICATE_HEADER;
> +```
> +
> +## Benefits of the change
> +
> +In PI Spec v1.7 Errata A, Vol.4, Sec 5.7 MM Communication Protocol, the
> MessageLength field of `EFI_MM_COMMUNICATE_HEADER` (also defined as
> `EFI_SMM_COMMUNICATE_HEADER`) is defined as type UINTN.
> +
> +But this structure, as a generic definition, could be used for both PEI and
> DXE MM communication. Thus for 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.
> +
> +## Impact of the change
> +
> +This change will impact the known structure consumers including:
> +
> +```bash
> +MdeModulePkg/Core/PiSmmCore/PiSmmIpl
> +MdeModulePkg/Application/SmiHandlerProfileInfo
> +MdeModulePkg/Application/MemoryProfileInfo
> +```
> +
> +For consumers that are not using
> `OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)`, but performing
> explicit addition such as the existing
> MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c,
> one will need to change code implementation to match new structure
> definition. Otherwise, the code compiled on IA32 architecture will
> experience structure field dereference error.
> +
> +User who currently uses UINTN local variables as place holder of
> MessageLength will need to use caution to make cast from UINTN to UINT64
> and vice versa. It is recommended to use `SafeUint64ToUintn` for such
> operations when the value is indeterministic.
> +
> +Note: MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib is also
> consuming this structure, but it handled this size discrepancy internally. If
> this
Hello Kun,
Sorry for a question.
I am not sure why the current codes in file SmmLockBoxDxeLib.c will work
properly after the UINTN -> UINT64 change.
For example:
LockBoxGetSmmCommBuffer():
MinimalSizeNeeded = sizeof (EFI_GUID) +
sizeof (UINTN) +
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_SAVE),
MAX (sizeof
(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES),
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_UPDATE),
MAX (sizeof
(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE),
sizeof
(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)))));
SaveLockBox():
UINT8 TempCommBuffer[sizeof(EFI_GUID) +
sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)];
Is the series missed changes for SmmLockBoxDxeLib or I got something wrong?
Best Regards,
Hao Wu
> potential spec change is not applied, all applicable PEI MM communicate
> callers will need to use the same routine as that of SmmLockBoxPeiLib to
> invoke a properly populated EFI_MM_COMMUNICATE_HEADER to be used
> in X64 MM foundation.
> +
> +## Detailed description of the change [normative updates]
> +
> +### Specification Changes
> +
> +1. In PI Specification v1.7 Errata A: Vol. 4 Page-91, the definition of
> `EFI_MM_COMMUNICATE_HEADER` should be changed from current:
> +
> +```c
> +typedef struct {
> + EFI_GUID HeaderGuid;
> + UINTN MessageLength;
> + UINT8 Data[ANYSIZE_ARRAY];
> +} EFI_MM_COMMUNICATE_HEADER;
> +```
> +
> +to:
> +
> +```c
> +typedef struct {
> + EFI_GUID HeaderGuid;
> + UINT64 MessageLength;
> + UINT8 Data[ANYSIZE_ARRAY];
> +} EFI_MM_COMMUNICATE_HEADER;
> +```
> +
> +### Code Changes
> +
> +1. Remove the explicit calculation of the offset of `Data` in
> `EFI_MM_COMMUNICATE_HEADER`. Thus applicable calculations of
> `sizeof(EFI_GUID) + sizeof(UINTN)` should be replaced with
> `OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)` or similar
> alternatives. These calculations are identified in:
> +
> +```bash
> +MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.
> c
> +MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
> +```
> +
> +1. Resolve potentially mismatched type between `UINTN` and `UINT64`.
> This would occur when `MessageLength` or its derivitive are used for local
> calculation with existing `UINTN` typed variables. Code change regarding this
> perspective is per case evaluation: if the variables involved are all
> deterministic values, and there is no overflow or underflow risk, a cast
> operation (from `UINTN` to `UINT64`) can be safely used. Otherwise, the
> calculation will be performed in `UINT64` bitwidth and then convert to
> `UINTN` using `SafeUint64*` and `SafeUint64ToUintn`, respectively. These
> operations are identified in:
> +
> +```bash
> +MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
> +MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.
> c
> +MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
> +```
> +
> +1. After all above changes applied and specification updated,
> `MdePkg/Include/Protocol/MmCommunication.h` will need to be updated to
> match new definition that includes the field type update.
> --
> 2.31.1.windows.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76370): https://edk2.groups.io/g/devel/message/76370
Mute This Topic: https://groups.io/mt/83435922/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
