Hi Marvin,
Thanks for reaching out. Please see my comment inline.
Regards,
Kun
On 06/16/2021 00:02, Marvin Häuser wrote:
Good day,
May I ask about two small things?
1) Was there any rationale as to e.g. code compatibility with choosing
UINT64 for the data length? I understand that this is the maximum of the
two as of currently, but I wonder whether a message length that exceeds
the UINT32 range (4 GB!) can possibly be considered sane or a good idea.
I agree that >4GB communication buffer may not be practical as of today.
That is why we modified the SMM communication routine in PiSmmIpl to use
SafeInt functions in Patch 2 of this series. With that change, at least
for 32bit MM, larger than 4GB will be considered insane. But in the
meantime, I do not want to rule out the >4GB communication capability
that a 64bit MM currently already have.
Please let me know if I missed anything in regards to adding SafeInt
functions to SmmCommunication routine.
2) Is it feasible yet with the current set of supported compilers to
support flexible arrays?
My impression is that flexible arrays are already supported (as seen in
UnitTestFrameworkPkg/PrivateInclude/UnitTestFrameworkTypes.h). Please
correct me if I am wrong.
Would you mind letting me know why this is applicable here? We are
trying to seek ideas on how to catch developer mistakes caused by this
change. So any input is appreciated.
Thank you for your work!
Best regards,
Marvin
On 10.06.21 03:42, Kun Qin wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430
In PI Spec v1.7 Errata A, Vol.4, Sec 5.7 MM Communication Protocol, the
MessageLength field of EFI_MM_COMMUNICATE_HEADER (also derived as
EFI_SMM_COMMUNICATE_HEADER) is currently 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 being used.
The suggested change is to make the MessageLength field defined with
definitive size as below:
```
typedef struct {
EFI_GUID HeaderGuid;
UINT64 MessageLength;
UINT8 Data[ANYSIZE_ARRAY];
} EFI_MM_COMMUNICATE_HEADER;
```
Patch v1 branch:
https://github.com/kuqin12/edk2/tree/BZ3398-MmCommunicate-Length
Cc: Jian J Wang <jian.j.w...@intel.com>
Cc: Hao A Wu <hao.a...@intel.com>
Cc: Eric Dong <eric.d...@intel.com>
Cc: Ray Ni <ray...@intel.com>
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>
Kun Qin (5):
EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update
MdeModulePkg: PiSmmIpl: Update MessageLength calculation for
MmCommunicate
MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation
MdeModulePkg: SmiHandlerProfileInfo: Updated MessageLength calculation
MdePkg: MmCommunication: Extend MessageLength field size to UINT64
MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
| 20 +++--
MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
| 8 +-
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
| 13 ++-
BZ3430-SpecChange.md
| 88 ++++++++++++++++++++
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
| 1 +
MdePkg/Include/Protocol/MmCommunication.h
| 3 +-
6 files changed, 124 insertions(+), 9 deletions(-)
create mode 100644 BZ3430-SpecChange.md
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76605): https://edk2.groups.io/g/devel/message/76605
Mute This Topic: https://groups.io/mt/83435921/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
