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.
Second, "MdeModulePkg/MdeModulePkg.dec" restricts these PCDs to
[PcdsDynamic, PcdsDynamicEx], so no platform DSC can make them
PcdsDynamicHii.
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
