A couple of minor comments below:
> -----Original Message----- > From: Kun Qin <kuqi...@gmail.com> > Sent: Thursday, June 10, 2021 9:43 AM > To: devel@edk2.groups.io > Cc: Wang, Jian J <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>; > Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com> > Subject: [PATCH v1 2/5] MdeModulePkg: PiSmmIpl: Update MessageLength > calculation for MmCommunicate > > 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.w...@intel.com> > Cc: Hao A Wu <hao.a...@intel.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > > Signed-off-by: Kun Qin <kuqi...@gmail.com> > --- > MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 13 ++++++++++++- > MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 + > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > index 599a0cd01d80..9508715fda24 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> // BZ3398 I suggest to remove the comment "// BZ3398" here. I think users can use the 'blame' feature of the version control systems together with the commit log message to find out the information. > > #include "PiSmmCorePrivateData.h" > > @@ -515,6 +516,7 @@ SmmCommunicationCommunicate ( > EFI_STATUS Status; > EFI_SMM_COMMUNICATE_HEADER *CommunicateHeader; > BOOLEAN OldInSmm; > + UINT64 BZ3398_LongCommSize; Suggest to drop the "BZ3398_" prefix for the variable name. > UINTN TempCommSize; > > // > @@ -527,7 +529,16 @@ SmmCommunicationCommunicate ( > CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *) > CommBuffer; > > if (CommSize == NULL) { > - TempCommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) > + CommunicateHeader->MessageLength; > + // BZ3398 Starts: Make MessageLength the same size in > EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. Suggest to drop the "// BZ3398 Starts:" and "// BZ3398 Ends" comments pair here. With above handled: Reviewed-by: Hao A Wu <hao.a...@intel.com> Best Regards, Hao Wu > + Status = SafeUint64Add (OFFSET_OF > (EFI_SMM_COMMUNICATE_HEADER, Data), CommunicateHeader- > >MessageLength, &BZ3398_LongCommSize); > + if (EFI_ERROR (Status)) { > + return EFI_INVALID_PARAMETER; > + } > + Status = SafeUint64ToUintn (BZ3398_LongCommSize, &TempCommSize); > + if (EFI_ERROR (Status)) { > + return EFI_INVALID_PARAMETER; > + } > + // BZ3398 Ends > } else { > TempCommSize = *CommSize; > // > diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf > b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf > index 6109d6b5449c..87142e27fa47 100644 > --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf > +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf > @@ -46,6 +46,7 @@ [LibraryClasses] > DxeServicesLib > PcdLib > ReportStatusCodeLib > + SafeIntLib #BZ3398 > > [Protocols] > gEfiSmmBase2ProtocolGuid ## PRODUCES > -- > 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76373): https://edk2.groups.io/g/devel/message/76373 Mute This Topic: https://groups.io/mt/83435923/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
