Re: [edk2] [PATCH v4 10/13] MdeModulePkg/SmmLockBox(PEI): Remove an ASSERT in RestoreLockBox()

2019-02-14 Thread Zeng, Star 
Reviewed-by: Star Zeng 

-Original Message-
From: Wu, Hao A 
Sent: Monday, February 11, 2019 3:58 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Wang, Jian J ; Zeng, 
Star 
Subject: [PATCH v4 10/13] MdeModulePkg/SmmLockBox(PEI): Remove an ASSERT in 
RestoreLockBox()

This commit is out of the scope for BZ-1409. It is a refinement for the PEI 
library instance within SmmLockBoxLib.

For the below ASSERT statement within function RestoreLockBox():
  Status = SmmCommunicationPpi->Communicate (
  SmmCommunicationPpi,
  [0],
  
  );
  if (Status == EFI_NOT_STARTED) {
//
// Pei SMM communication not ready yet, so we access SMRAM directly
//
DEBUG ((DEBUG_INFO, "SmmLockBoxPeiLib Communicate - (%r)\n", Status));
Status = InternalRestoreLockBoxFromSmram (Guid, Buffer, Length);
LockBoxParameterRestore->Header.ReturnStatus = (UINT64)Status;
if (Length != NULL) {
  LockBoxParameterRestore->Length = (UINT64)*Length;
}
  }
  ASSERT_EFI_ERROR (Status);

It is possible for previous codes to return an error status that is possible 
for happen. One example is that, when the 'if' statement 'if (Status == 
EFI_NOT_STARTED) {' is entered, function
InternalRestoreLockBoxFromSmram() is possible to return 'BUFFER_TOO_SMALL'
if the caller of RestoreLockBox() provides a buffer that is too small to hold 
the content of LockBox.

Thus, this commit will remove the ASSERT here.

Please note that the current implementation of RestoreLockBox() is handling the 
above-mentioned error case properly, so no additional error handling codes are 
needed here.

Cc: Jian J Wang 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
Reviewed-by: Laszlo Ersek 
---
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
index 9f73480070..8c3e65bc96 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
@@ -623,7 +623,6 @@ RestoreLockBox (
   LockBoxParameterRestore->Length = (UINT64)*Length;
 }
   }
-  ASSERT_EFI_ERROR (Status);
 
   if (Length != NULL) {
 *Length = (UINTN)LockBoxParameterRestore->Length;
--
2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v4 10/13] MdeModulePkg/SmmLockBox(PEI): Remove an ASSERT in RestoreLockBox()

2019-02-10 Thread Hao Wu 
This commit is out of the scope for BZ-1409. It is a refinement for the
PEI library instance within SmmLockBoxLib.

For the below ASSERT statement within function RestoreLockBox():
  Status = SmmCommunicationPpi->Communicate (
  SmmCommunicationPpi,
  [0],
  
  );
  if (Status == EFI_NOT_STARTED) {
//
// Pei SMM communication not ready yet, so we access SMRAM directly
//
DEBUG ((DEBUG_INFO, "SmmLockBoxPeiLib Communicate - (%r)\n", Status));
Status = InternalRestoreLockBoxFromSmram (Guid, Buffer, Length);
LockBoxParameterRestore->Header.ReturnStatus = (UINT64)Status;
if (Length != NULL) {
  LockBoxParameterRestore->Length = (UINT64)*Length;
}
  }
  ASSERT_EFI_ERROR (Status);

It is possible for previous codes to return an error status that is
possible for happen. One example is that, when the 'if' statement
'if (Status == EFI_NOT_STARTED) {' is entered, function
InternalRestoreLockBoxFromSmram() is possible to return 'BUFFER_TOO_SMALL'
if the caller of RestoreLockBox() provides a buffer that is too small to
hold the content of LockBox.

Thus, this commit will remove the ASSERT here.

Please note that the current implementation of RestoreLockBox() is
handling the above-mentioned error case properly, so no additional error
handling codes are needed here.

Cc: Jian J Wang 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
Reviewed-by: Laszlo Ersek 
---
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
index 9f73480070..8c3e65bc96 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
@@ -623,7 +623,6 @@ RestoreLockBox (
   LockBoxParameterRestore->Length = (UINT64)*Length;
 }
   }
-  ASSERT_EFI_ERROR (Status);
 
   if (Length != NULL) {
 *Length = (UINTN)LockBoxParameterRestore->Length;
-- 
2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel