On 2015/10/9 6:33, Andrew Fish wrote:
On Oct 8, 2015, at 3:11 PM, Laszlo Ersek <ler...@redhat.com> wrote:
On 10/08/15 09:39, Star Zeng wrote:
PcdSet## has no error status returned, then the caller has no idea about
whether the set operation is successful or not.
PcdSet##S were added to return error status and PcdSet## APIs were put in
ifndef DISABLE_NEW_DEPRECATED_INTERFACES condition.
To adopt PcdSet##S and further code development with
DISABLE_NEW_DEPRECATED_INTERFACES defined, we need to Replace PcdSet## usage
with PcdSet##S.
Normally, DynamicDefault PCD set is expected to be success, but DynamicHii PCD
set failure is a legal case.
I almost had a question here ("why?"), but looking at
"MdeModulePkg/Universal/PCD/Dxe/Pcd.inf", I know why -- because
PcdsDynamicHii is stored in the non-volatile varstore.
For this case, PcdS3BootScriptTablePrivateDataPtr and
PcdS3BootScriptTablePrivateSmmDataPtr are expected to be DynamicDefault,
so use PcdSet64S to instead of PcdSet64 and assert when set failure.
Okay, so this I don't understand.
First, I don't think that any platform would like to store S3
bootscript-related stuff in nv variables.
Historically I think there is an ACPI NVS memory region that the variable
pointed to (going from memory so I may be a little off).
The newer code uses the LockBox that is usually stored in SMM.
Second, "MdeModulePkg/MdeModulePkg.dec" restricts these PCDs to
[PcdsDynamic, PcdsDynamicEx], so no platform DSC can make them
PcdsDynamicHii.
This is just confusing terminology/syntax….
From the DEC and INF point of view:
PcdDynamic means you use the PCD PPI/Protocol with the build generated Token
PcdDynamicEx means you use the PCD PPI/Protocol with the fixed Token and
EFI_GUID from the .DEC.
The DSC file also has a knob that controls how dynamic PCDs are stored. So
PcdsDynamicHii is a PcdDynamic form, but the driver that implements the
PPI/Protocol stores the data in an alternate location.
Right, thanks for the explanation.
Star
Thanks,
Andrew Fish
Therefore, I guess the patch is correct, but why is it necessary? Is it
just to be compatible with DISABLE_NEW_DEPRECATED_INTERFACES?
Ah, I see. The commit message has three parts / arguments actually:
(1) general description of status checking vs. non-checking interfaces
(2) argument why we should move to checked interfaces
(3) argument why the check will always succeed.
Thanks
Laszlo
Cc: Jiewen Yao <jiewen....@intel.com>
Cc: Liming Gao <liming....@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.z...@intel.com>
---
MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
index 24c6798..e7d2a24 100644
--- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
+++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
@@ -454,7 +454,8 @@ S3BootScriptLibInitialize (
}
S3TablePtr = (VOID *) (UINTN) Buffer;
- PcdSet64 (PcdS3BootScriptTablePrivateDataPtr, (UINT64) (UINTN)S3TablePtr);
+ Status = PcdSet64S (PcdS3BootScriptTablePrivateDataPtr, (UINT64)
(UINTN)S3TablePtr);
+ ASSERT_EFI_ERROR (Status);
ZeroMem (S3TablePtr, sizeof(SCRIPT_TABLE_PRIVATE_DATA));
//
// Create event to notify the library system enter the SmmLocked phase.
@@ -506,7 +507,8 @@ S3BootScriptLibInitialize (
return RETURN_OUT_OF_RESOURCES;
}
- PcdSet64 (PcdS3BootScriptTablePrivateSmmDataPtr, (UINT64)
(UINTN)S3TableSmmPtr);
+ Status = PcdSet64S (PcdS3BootScriptTablePrivateSmmDataPtr, (UINT64)
(UINTN)S3TableSmmPtr);
+ ASSERT_EFI_ERROR (Status);
ZeroMem (S3TableSmmPtr, sizeof(SCRIPT_TABLE_PRIVATE_DATA));
//
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
