* [PATCH v2 1/6] EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update
2021-06-18 9:02 [PATCH v2 0/6] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER Kun Qin
@ 2021-06-18 9:02 ` Kun Qin
2021-06-23 10:02 ` [edk2-devel] " Laszlo Ersek
2021-06-18 9:02 ` [PATCH v2 2/6] MdeModulePkg: PiSmmIpl: Update MessageLength calculation for MmCommunicate Kun Qin
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Kun Qin @ 2021-06-18 9:02 UTC (permalink / raw)
To: devel
Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Andrew Fish,
Laszlo Ersek, Leif Lindholm, Hao A Wu
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.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
Notes:
v2:
- Updated change impact analysis regarding SmmLockBoxDxeLib [Hao]
BZ3430-SpecChange.md | 90 ++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/BZ3430-SpecChange.md b/BZ3430-SpecChange.md
new file mode 100644
index 000000000000..a4f8b397e822
--- /dev/null
+++ b/BZ3430-SpecChange.md
@@ -0,0 +1,90 @@
+# 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
+MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib
+```
+
+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 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
+MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH v2 1/6] EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update
2021-06-18 9:02 ` [PATCH v2 1/6] EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update Kun Qin
@ 2021-06-23 10:02 ` Laszlo Ersek
2021-06-24 0:28 ` Kun Qin
0 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2021-06-23 10:02 UTC (permalink / raw)
To: devel, kuqin12
Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Andrew Fish,
Leif Lindholm, Hao A Wu
On 06/18/21 11:02, Kun Qin wrote:
> 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.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
>
> Notes:
> v2:
> - Updated change impact analysis regarding SmmLockBoxDxeLib [Hao]
>
> BZ3430-SpecChange.md | 90 ++++++++++++++++++++
> 1 file changed, 90 insertions(+)
Placing such a document in the edk2 root directory looks very strange to me.
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-First-Process#tianocore-github
[...] Specification text changes are held within the affected source
repository, using the GitHub flavor of markdown, in a file (or split
across several files) with .md suffix [...]
The wiki does not seem to specify what directory should contain the spec
change document.
Should we create a "CodeFirst" top-level directory?
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH v2 1/6] EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update
2021-06-23 10:02 ` [edk2-devel] " Laszlo Ersek
@ 2021-06-24 0:28 ` Kun Qin
0 siblings, 0 replies; 12+ messages in thread
From: Kun Qin @ 2021-06-24 0:28 UTC (permalink / raw)
To: Laszlo Ersek, devel
Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Andrew Fish,
Leif Lindholm, Hao A Wu
I can place this file under "CodeFirst" folder on the next round of
patch series.
It would be helpful to update the code-first process page so that others
can be consistent on the process next time.
Thanks,
Kun
On 06/23/2021 03:02, Laszlo Ersek wrote:
> On 06/18/21 11:02, Kun Qin wrote:
>> 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.kinney@intel.com>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>> Cc: Andrew Fish <afish@apple.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Leif Lindholm <leif@nuviainc.com>
>> Cc: Hao A Wu <hao.a.wu@intel.com>
>>
>> Signed-off-by: Kun Qin <kuqin12@gmail.com>
>> ---
>>
>> Notes:
>> v2:
>> - Updated change impact analysis regarding SmmLockBoxDxeLib [Hao]
>>
>> BZ3430-SpecChange.md | 90 ++++++++++++++++++++
>> 1 file changed, 90 insertions(+)
>
> Placing such a document in the edk2 root directory looks very strange to me.
>
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-First-Process#tianocore-github
>
> [...] Specification text changes are held within the affected source
> repository, using the GitHub flavor of markdown, in a file (or split
> across several files) with .md suffix [...]
>
> The wiki does not seem to specify what directory should contain the spec
> change document.
>
> Should we create a "CodeFirst" top-level directory?
>
> Thanks,
> Laszlo
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/6] MdeModulePkg: PiSmmIpl: Update MessageLength calculation for MmCommunicate
2021-06-18 9:02 [PATCH v2 0/6] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER Kun Qin
2021-06-18 9:02 ` [PATCH v2 1/6] EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update Kun Qin
@ 2021-06-18 9:02 ` Kun Qin
2021-06-18 9:02 ` [PATCH v2 3/6] MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation Kun Qin
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Kun Qin @ 2021-06-18 9:02 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Eric Dong, Ray Ni
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
This change updated calculation routine for MM communication in PiSmmIpl.
It removes ambiguity brought in by UINTN variables from this routine and
paves way for updating definition of field MessageLength in
EFI_MM_COMMUNICATE_HEADER to definitive size.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
---
Notes:
v2:
- Removed "BZ" tags from comments and variables [Hao]
- Added "Reviewed-by" tag [Hao]
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 11 ++++++++++-
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 +
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
index 599a0cd01d80..01cde6cfc3e4 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
@@ -34,6 +34,7 @@
#include <Library/UefiRuntimeLib.h>
#include <Library/PcdLib.h>
#include <Library/ReportStatusCodeLib.h>
+#include <Library/SafeIntLib.h>
#include "PiSmmCorePrivateData.h"
@@ -515,6 +516,7 @@ SmmCommunicationCommunicate (
EFI_STATUS Status;
EFI_SMM_COMMUNICATE_HEADER *CommunicateHeader;
BOOLEAN OldInSmm;
+ UINT64 LongCommSize;
UINTN TempCommSize;
//
@@ -527,7 +529,14 @@ SmmCommunicationCommunicate (
CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *) CommBuffer;
if (CommSize == NULL) {
- TempCommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + CommunicateHeader->MessageLength;
+ Status = SafeUint64Add (OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data), CommunicateHeader->MessageLength, &LongCommSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ Status = SafeUint64ToUintn (LongCommSize, &TempCommSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
} else {
TempCommSize = *CommSize;
//
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
index 6109d6b5449c..ddeb39cee266 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
@@ -46,6 +46,7 @@ [LibraryClasses]
DxeServicesLib
PcdLib
ReportStatusCodeLib
+ SafeIntLib
[Protocols]
gEfiSmmBase2ProtocolGuid ## PRODUCES
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 3/6] MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation
2021-06-18 9:02 [PATCH v2 0/6] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER Kun Qin
2021-06-18 9:02 ` [PATCH v2 1/6] EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update Kun Qin
2021-06-18 9:02 ` [PATCH v2 2/6] MdeModulePkg: PiSmmIpl: Update MessageLength calculation for MmCommunicate Kun Qin
@ 2021-06-18 9:02 ` Kun Qin
2021-06-23 1:33 ` [edk2-devel] " Wu, Hao A
2021-06-18 9:02 ` [PATCH v2 4/6] MdeModulePkg: SmiHandlerProfileInfo: " Kun Qin
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Kun Qin @ 2021-06-18 9:02 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
This change replaced the calculation of communication buffer size from
explicitly adding the size of each member with the OFFSET macro function.
This will make the structure field defition change transparent to
consumers.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
Notes:
v2:
- Added a missed case this change should cover [Hao]
- Removed "BZ" tags from comments [Hao]
MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c | 28 +++++++++++++++-----
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
index 191c31068545..69f78c090e7c 100644
--- a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
+++ b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
@@ -1140,8 +1140,7 @@ GetSmramProfileData (
return Status;
}
- MinimalSizeNeeded = sizeof (EFI_GUID) +
- sizeof (UINTN) +
+ MinimalSizeNeeded = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
MAX (sizeof (SMRAM_PROFILE_PARAMETER_GET_PROFILE_INFO),
MAX (sizeof (SMRAM_PROFILE_PARAMETER_GET_PROFILE_DATA_BY_OFFSET),
sizeof (SMRAM_PROFILE_PARAMETER_RECORDING_STATE)));
@@ -1190,7 +1189,10 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_DISABLE;
- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "SmramProfile: SmmCommunication - %r\n", Status));
@@ -1213,7 +1215,10 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_DISABLE;
- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize);
}
@@ -1230,7 +1235,10 @@ GetSmramProfileData (
CommGetProfileInfo->Header.ReturnStatus = (UINT64)-1;
CommGetProfileInfo->ProfileSize = 0;
- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize);
ASSERT_EFI_ERROR (Status);
@@ -1261,7 +1269,10 @@ GetSmramProfileData (
CommGetProfileData->Header.DataLength = sizeof (*CommGetProfileData);
CommGetProfileData->Header.ReturnStatus = (UINT64)-1;
- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Buffer = (UINT8 *) CommHeader + CommSize;
Size -= CommSize;
@@ -1320,7 +1331,10 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_ENABLE;
- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize);
}
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH v2 3/6] MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation
2021-06-18 9:02 ` [PATCH v2 3/6] MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation Kun Qin
@ 2021-06-23 1:33 ` Wu, Hao A
0 siblings, 0 replies; 12+ messages in thread
From: Wu, Hao A @ 2021-06-23 1:33 UTC (permalink / raw)
To: devel@edk2.groups.io, kuqin12@gmail.com; +Cc: Wang, Jian J
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun Qin
> Sent: Friday, June 18, 2021 5:03 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [edk2-devel] [PATCH v2 3/6] MdeModulePkg: MemoryProfileInfo:
> Updated MessageLength calculation
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
>
> This change replaced the calculation of communication buffer size from
> explicitly adding the size of each member with the OFFSET macro function.
> This will make the structure field defition change transparent to
> consumers.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
>
> Notes:
> v2:
> - Added a missed case this change should cover [Hao]
> - Removed "BZ" tags from comments [Hao]
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
>
> MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c | 28
> +++++++++++++++-----
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git
> a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
> b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
> index 191c31068545..69f78c090e7c 100644
> --- a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
> +++
> b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
> @@ -1140,8 +1140,7 @@ GetSmramProfileData (
> return Status;
> }
>
> - MinimalSizeNeeded = sizeof (EFI_GUID) +
> - sizeof (UINTN) +
> + MinimalSizeNeeded = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER,
> Data) +
> MAX (sizeof
> (SMRAM_PROFILE_PARAMETER_GET_PROFILE_INFO),
> MAX (sizeof
> (SMRAM_PROFILE_PARAMETER_GET_PROFILE_DATA_BY_OFFSET),
> sizeof
> (SMRAM_PROFILE_PARAMETER_RECORDING_STATE)));
> @@ -1190,7 +1189,10 @@ GetSmramProfileData (
> CommRecordingState->Header.ReturnStatus = (UINT64)-1;
> CommRecordingState->RecordingState =
> MEMORY_PROFILE_RECORDING_DISABLE;
>
> - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
> >MessageLength;
> + //
> + // The CommHeader->MessageLength contains a definitive value, thus
> UINTN cast is safe here.
> + //
> + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
> (UINTN)CommHeader->MessageLength;
> Status = SmmCommunication->Communicate (SmmCommunication,
> CommBuffer, &CommSize);
> if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "SmramProfile: SmmCommunication - %r\n",
> Status));
> @@ -1213,7 +1215,10 @@ GetSmramProfileData (
> CommRecordingState->Header.ReturnStatus = (UINT64)-1;
> CommRecordingState->RecordingState =
> MEMORY_PROFILE_RECORDING_DISABLE;
>
> - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
> >MessageLength;
> + //
> + // The CommHeader->MessageLength contains a definitive value, thus
> UINTN cast is safe here.
> + //
> + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
> (UINTN)CommHeader->MessageLength;
> SmmCommunication->Communicate (SmmCommunication, CommBuffer,
> &CommSize);
> }
>
> @@ -1230,7 +1235,10 @@ GetSmramProfileData (
> CommGetProfileInfo->Header.ReturnStatus = (UINT64)-1;
> CommGetProfileInfo->ProfileSize = 0;
>
> - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
> >MessageLength;
> + //
> + // The CommHeader->MessageLength contains a definitive value, thus
> UINTN cast is safe here.
> + //
> + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
> (UINTN)CommHeader->MessageLength;
> Status = SmmCommunication->Communicate (SmmCommunication,
> CommBuffer, &CommSize);
> ASSERT_EFI_ERROR (Status);
>
> @@ -1261,7 +1269,10 @@ GetSmramProfileData (
> CommGetProfileData->Header.DataLength = sizeof
> (*CommGetProfileData);
> CommGetProfileData->Header.ReturnStatus = (UINT64)-1;
>
> - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
> >MessageLength;
> + //
> + // The CommHeader->MessageLength contains a definitive value, thus
> UINTN cast is safe here.
> + //
> + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
> (UINTN)CommHeader->MessageLength;
> Buffer = (UINT8 *) CommHeader + CommSize;
> Size -= CommSize;
>
> @@ -1320,7 +1331,10 @@ GetSmramProfileData (
> CommRecordingState->Header.ReturnStatus = (UINT64)-1;
> CommRecordingState->RecordingState =
> MEMORY_PROFILE_RECORDING_ENABLE;
>
> - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
> >MessageLength;
> + //
> + // The CommHeader->MessageLength contains a definitive value, thus
> UINTN cast is safe here.
> + //
> + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
> (UINTN)CommHeader->MessageLength;
> SmmCommunication->Communicate (SmmCommunication, CommBuffer,
> &CommSize);
> }
>
> --
> 2.31.1.windows.1
>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/6] MdeModulePkg: SmiHandlerProfileInfo: Updated MessageLength calculation
2021-06-18 9:02 [PATCH v2 0/6] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER Kun Qin
` (2 preceding siblings ...)
2021-06-18 9:02 ` [PATCH v2 3/6] MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation Kun Qin
@ 2021-06-18 9:02 ` Kun Qin
2021-06-23 1:33 ` [edk2-devel] " Wu, Hao A
2021-06-18 9:02 ` [PATCH v2 5/6] MdeModulePkg: SmmLockBoxDxeLib: " Kun Qin
2021-06-18 9:02 ` [PATCH v2 6/6] MdePkg: MmCommunication: Extend MessageLength field size to UINT64 Kun Qin
5 siblings, 1 reply; 12+ messages in thread
From: Kun Qin @ 2021-06-18 9:02 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Eric Dong, Ray Ni
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
This change replaced the calculation of communication buffer size from
explicitly adding the size of each member with the OFFSET macro function.
This will make the structure field defition change transparent to
consumers.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
Notes:
v2:
- Updated comments by removing "BZ" tags [Hao]
MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
index 4153074b7a80..4bfd5946caba 100644
--- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
+++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
@@ -116,7 +116,10 @@ GetSmiHandlerProfileDatabase(
CommGetInfo->Header.ReturnStatus = (UINT64)-1;
CommGetInfo->DataSize = 0;
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate(SmmCommunication, CommBuffer, &CommSize);
if (EFI_ERROR(Status)) {
Print(L"SmiHandlerProfile: SmmCommunication - %r\n", Status);
@@ -149,7 +152,10 @@ GetSmiHandlerProfileDatabase(
CommGetData->Header.DataLength = sizeof(*CommGetData);
CommGetData->Header.ReturnStatus = (UINT64)-1;
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Buffer = (UINT8 *)CommHeader + CommSize;
Size -= CommSize;
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH v2 4/6] MdeModulePkg: SmiHandlerProfileInfo: Updated MessageLength calculation
2021-06-18 9:02 ` [PATCH v2 4/6] MdeModulePkg: SmiHandlerProfileInfo: " Kun Qin
@ 2021-06-23 1:33 ` Wu, Hao A
0 siblings, 0 replies; 12+ messages in thread
From: Wu, Hao A @ 2021-06-23 1:33 UTC (permalink / raw)
To: devel@edk2.groups.io, kuqin12@gmail.com; +Cc: Wang, Jian J, Dong, Eric, Ni, Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun Qin
> Sent: Friday, June 18, 2021 5:03 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH v2 4/6] MdeModulePkg: SmiHandlerProfileInfo:
> Updated MessageLength calculation
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
>
> This change replaced the calculation of communication buffer size from
> explicitly adding the size of each member with the OFFSET macro function.
> This will make the structure field defition change transparent to consumers.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
>
> Notes:
> v2:
> - Updated comments by removing "BZ" tags [Hao]
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
>
> MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
> | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git
> a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo
> .c
> b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo
> .c
> index 4153074b7a80..4bfd5946caba 100644
> ---
> a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo
> .c
> +++
> b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileIn
> +++ fo.c
> @@ -116,7 +116,10 @@ GetSmiHandlerProfileDatabase(
> CommGetInfo->Header.ReturnStatus = (UINT64)-1;
> CommGetInfo->DataSize = 0;
>
> - CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader-
> >MessageLength;
> + //
> + // The CommHeader->MessageLength contains a definitive value, thus
> UINTN cast is safe here.
> + //
> + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
> + (UINTN)CommHeader->MessageLength;
> Status = SmmCommunication->Communicate(SmmCommunication,
> CommBuffer, &CommSize);
> if (EFI_ERROR(Status)) {
> Print(L"SmiHandlerProfile: SmmCommunication - %r\n", Status); @@ -
> 149,7 +152,10 @@ GetSmiHandlerProfileDatabase(
> CommGetData->Header.DataLength = sizeof(*CommGetData);
> CommGetData->Header.ReturnStatus = (UINT64)-1;
>
> - CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader-
> >MessageLength;
> + //
> + // The CommHeader->MessageLength contains a definitive value, thus
> UINTN cast is safe here.
> + //
> + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
> + (UINTN)CommHeader->MessageLength;
> Buffer = (UINT8 *)CommHeader + CommSize;
> Size -= CommSize;
>
> --
> 2.31.1.windows.1
>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 5/6] MdeModulePkg: SmmLockBoxDxeLib: Updated MessageLength calculation
2021-06-18 9:02 [PATCH v2 0/6] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER Kun Qin
` (3 preceding siblings ...)
2021-06-18 9:02 ` [PATCH v2 4/6] MdeModulePkg: SmiHandlerProfileInfo: " Kun Qin
@ 2021-06-18 9:02 ` Kun Qin
2021-06-23 1:33 ` Wu, Hao A
2021-06-18 9:02 ` [PATCH v2 6/6] MdePkg: MmCommunication: Extend MessageLength field size to UINT64 Kun Qin
5 siblings, 1 reply; 12+ messages in thread
From: Kun Qin @ 2021-06-18 9:02 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Eric Dong, Ray Ni
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
This change replaced the calculation of communication buffer size from
explicitly adding the size of each member with the OFFSET macro function.
This will make the structure field defition change transparent to
consumers.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
Notes:
v2:
- Newly added in v2
MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c | 23 ++++++++++----------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
index 2cbffe889e1f..66fbe4dd961c 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
@@ -79,8 +79,7 @@ LockBoxGetSmmCommBuffer (
return mLockBoxSmmCommBuffer;
}
- MinimalSizeNeeded = sizeof (EFI_GUID) +
- sizeof (UINTN) +
+ MinimalSizeNeeded = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) +
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),
@@ -142,7 +141,7 @@ SaveLockBox (
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
EFI_SMM_LOCK_BOX_PARAMETER_SAVE *LockBoxParameterSave;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
- UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)];
+ UINT8 TempCommBuffer[OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)];
UINT8 *CommBuffer;
UINTN CommSize;
@@ -182,7 +181,7 @@ SaveLockBox (
//
// Send command
//
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE);
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE);
Status = SmmCommunication->Communicate (
SmmCommunication,
&CommBuffer[0],
@@ -224,7 +223,7 @@ SetLockBoxAttributes (
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES *LockBoxParameterSetAttributes;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
- UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)];
+ UINT8 TempCommBuffer[OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)];
UINT8 *CommBuffer;
UINTN CommSize;
@@ -264,7 +263,7 @@ SetLockBoxAttributes (
//
// Send command
//
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES);
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES);
Status = SmmCommunication->Communicate (
SmmCommunication,
&CommBuffer[0],
@@ -314,7 +313,7 @@ UpdateLockBox (
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
EFI_SMM_LOCK_BOX_PARAMETER_UPDATE *LockBoxParameterUpdate;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
- UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)];
+ UINT8 TempCommBuffer[OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)];
UINT8 *CommBuffer;
UINTN CommSize;
@@ -355,7 +354,7 @@ UpdateLockBox (
//
// Send command
//
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE);
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE);
Status = SmmCommunication->Communicate (
SmmCommunication,
&CommBuffer[0],
@@ -403,7 +402,7 @@ RestoreLockBox (
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
EFI_SMM_LOCK_BOX_PARAMETER_RESTORE *LockBoxParameterRestore;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
- UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)];
+ UINT8 TempCommBuffer[OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)];
UINT8 *CommBuffer;
UINTN CommSize;
@@ -449,7 +448,7 @@ RestoreLockBox (
//
// Send command
//
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE);
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE);
Status = SmmCommunication->Communicate (
SmmCommunication,
&CommBuffer[0],
@@ -488,7 +487,7 @@ RestoreAllLockBoxInPlace (
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE *LockBoxParameterRestoreAllInPlace;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
- UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)];
+ UINT8 TempCommBuffer[OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)];
UINT8 *CommBuffer;
UINTN CommSize;
@@ -518,7 +517,7 @@ RestoreAllLockBoxInPlace (
//
// Send command
//
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE);
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE);
Status = SmmCommunication->Communicate (
SmmCommunication,
&CommBuffer[0],
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 5/6] MdeModulePkg: SmmLockBoxDxeLib: Updated MessageLength calculation
2021-06-18 9:02 ` [PATCH v2 5/6] MdeModulePkg: SmmLockBoxDxeLib: " Kun Qin
@ 2021-06-23 1:33 ` Wu, Hao A
0 siblings, 0 replies; 12+ messages in thread
From: Wu, Hao A @ 2021-06-23 1:33 UTC (permalink / raw)
To: Kun Qin, devel@edk2.groups.io; +Cc: Wang, Jian J, Dong, Eric, Ni, Ray
> -----Original Message-----
> From: Kun Qin <kuqin12@gmail.com>
> Sent: Friday, June 18, 2021 5:03 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v2 5/6] MdeModulePkg: SmmLockBoxDxeLib: Updated
> MessageLength calculation
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
>
> This change replaced the calculation of communication buffer size from
> explicitly adding the size of each member with the OFFSET macro function.
> This will make the structure field defition change transparent to
> consumers.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
>
> Notes:
> v2:
> - Newly added in v2
>
> MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c | 23
> ++++++++++----------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
> b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
> index 2cbffe889e1f..66fbe4dd961c 100644
> --- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
> +++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
> @@ -79,8 +79,7 @@ LockBoxGetSmmCommBuffer (
> return mLockBoxSmmCommBuffer;
> }
>
> - MinimalSizeNeeded = sizeof (EFI_GUID) +
> - sizeof (UINTN) +
> + MinimalSizeNeeded = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER,
> Data) +
> 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),
> @@ -142,7 +141,7 @@ SaveLockBox (
> EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
> EFI_SMM_LOCK_BOX_PARAMETER_SAVE *LockBoxParameterSave;
> EFI_SMM_COMMUNICATE_HEADER *CommHeader;
> - UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)];
> + UINT8 TempCommBuffer[OFFSET_OF
> (EFI_SMM_COMMUNICATE_HEADER, Data) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)];
> UINT8 *CommBuffer;
> UINTN CommSize;
>
> @@ -182,7 +181,7 @@ SaveLockBox (
> //
> // Send command
> //
> - CommSize = sizeof(EFI_GUID) + sizeof(UINTN) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE);
> + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE);
> Status = SmmCommunication->Communicate (
> SmmCommunication,
> &CommBuffer[0],
> @@ -224,7 +223,7 @@ SetLockBoxAttributes (
> EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
> EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES
> *LockBoxParameterSetAttributes;
> EFI_SMM_COMMUNICATE_HEADER *CommHeader;
> - UINT8 TempCommBuffer[sizeof(EFI_GUID) +
> sizeof(UINTN) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)];
> + UINT8 TempCommBuffer[OFFSET_OF
> (EFI_SMM_COMMUNICATE_HEADER, Data) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)];
> UINT8 *CommBuffer;
> UINTN CommSize;
>
> @@ -264,7 +263,7 @@ SetLockBoxAttributes (
> //
> // Send command
> //
> - CommSize = sizeof(EFI_GUID) + sizeof(UINTN) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES);
> + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES);
> Status = SmmCommunication->Communicate (
> SmmCommunication,
> &CommBuffer[0],
> @@ -314,7 +313,7 @@ UpdateLockBox (
> EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
> EFI_SMM_LOCK_BOX_PARAMETER_UPDATE *LockBoxParameterUpdate;
> EFI_SMM_COMMUNICATE_HEADER *CommHeader;
> - UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN)
> + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)];
> + UINT8 TempCommBuffer[OFFSET_OF
> (EFI_SMM_COMMUNICATE_HEADER, Data) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)];
> UINT8 *CommBuffer;
> UINTN CommSize;
>
> @@ -355,7 +354,7 @@ UpdateLockBox (
> //
> // Send command
> //
> - CommSize = sizeof(EFI_GUID) + sizeof(UINTN) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE);
> + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE);
> Status = SmmCommunication->Communicate (
> SmmCommunication,
> &CommBuffer[0],
> @@ -403,7 +402,7 @@ RestoreLockBox (
> EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
> EFI_SMM_LOCK_BOX_PARAMETER_RESTORE *LockBoxParameterRestore;
> EFI_SMM_COMMUNICATE_HEADER *CommHeader;
> - UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN)
> + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)];
> + UINT8 TempCommBuffer[OFFSET_OF
> (EFI_SMM_COMMUNICATE_HEADER, Data) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)];
> UINT8 *CommBuffer;
> UINTN CommSize;
>
> @@ -449,7 +448,7 @@ RestoreLockBox (
> //
> // Send command
> //
> - CommSize = sizeof(EFI_GUID) + sizeof(UINTN) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE);
> + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE);
> Status = SmmCommunication->Communicate (
> SmmCommunication,
> &CommBuffer[0],
> @@ -488,7 +487,7 @@ RestoreAllLockBoxInPlace (
> EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
> EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE
> *LockBoxParameterRestoreAllInPlace;
> EFI_SMM_COMMUNICATE_HEADER *CommHeader;
> - UINT8 TempCommBuffer[sizeof(EFI_GUID) +
> sizeof(UINTN) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)];
> + UINT8 TempCommBuffer[OFFSET_OF
> (EFI_SMM_COMMUNICATE_HEADER, Data) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)];
> UINT8 *CommBuffer;
> UINTN CommSize;
>
> @@ -518,7 +517,7 @@ RestoreAllLockBoxInPlace (
> //
> // Send command
> //
> - CommSize = sizeof(EFI_GUID) + sizeof(UINTN) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE);
> + CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) +
> sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE);
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
> Status = SmmCommunication->Communicate (
> SmmCommunication,
> &CommBuffer[0],
> --
> 2.31.1.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 6/6] MdePkg: MmCommunication: Extend MessageLength field size to UINT64
2021-06-18 9:02 [PATCH v2 0/6] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER Kun Qin
` (4 preceding siblings ...)
2021-06-18 9:02 ` [PATCH v2 5/6] MdeModulePkg: SmmLockBoxDxeLib: " Kun Qin
@ 2021-06-18 9:02 ` Kun Qin
5 siblings, 0 replies; 12+ messages in thread
From: Kun Qin @ 2021-06-18 9:02 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430
The MessageLength field of EFI_MM_COMMUNICATE_HEADER, as a generic
definition, could be 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 change removes the architecture dependent field by extending this
field definition as UINT64.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
Notes:
v2:
- Removed comments with "BZ"
MdePkg/Include/Protocol/MmCommunication.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MdePkg/Include/Protocol/MmCommunication.h b/MdePkg/Include/Protocol/MmCommunication.h
index 34c3e2b5a9e3..d42b00bbeb7c 100644
--- a/MdePkg/Include/Protocol/MmCommunication.h
+++ b/MdePkg/Include/Protocol/MmCommunication.h
@@ -26,7 +26,7 @@ typedef struct {
///
/// Describes the size of Data (in bytes) and does not include the size of the header.
///
- UINTN MessageLength;
+ UINT64 MessageLength;
///
/// Designates an array of bytes that is MessageLength in size.
///
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread