From: "Zeng, Star" <star.zeng@intel.com>
To: Hao Wu <hao.a.wu@intel.com>, edk2-devel@lists.01.org
Cc: star.zeng@intel.com
Subject: Re: [PATCH v4 11/13] MdeModulePkg/SmmLockBoxLib: Support LockBox enlarge in UpdateLockBox()
Date: Fri, 15 Feb 2019 13:13:58 +0800 [thread overview]
Message-ID: <35f5e873-7b32-ba28-c9cb-0d8d4050a067@intel.com> (raw)
In-Reply-To: <20190211075738.16676-13-hao.a.wu@intel.com>
Some minor comments.
With them addressed, Reviewed-by: Star Zeng <star.zeng@intel.com>.
On 2019/2/11 15:57, Hao Wu wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1409
>
> This commit will add the support to enlarge a LockBox when using the
> LockBoxLib API UpdateLockBox().
>
> Please note that the new support will ONLY work for LockBox with attribute
> LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY set.
>
> The functional uni-test for the commit is available at:
> https://github.com/hwu25/edk2/tree/lockbox_unitest
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> Reviewed-by: Ray Ni <ray.ni@intel.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> ---
> MdeModulePkg/Include/Library/LockBoxLib.h | 7 +-
> MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c | 7 +-
> MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c | 5 +-
> MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c | 5 +-
> MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c | 72 ++++++++++++++++++--
> 5 files changed, 86 insertions(+), 10 deletions(-)
>
> diff --git a/MdeModulePkg/Include/Library/LockBoxLib.h b/MdeModulePkg/Include/Library/LockBoxLib.h
> index 5921731419..addce3bd4a 100644
> --- a/MdeModulePkg/Include/Library/LockBoxLib.h
> +++ b/MdeModulePkg/Include/Library/LockBoxLib.h
> @@ -2,7 +2,7 @@
> This library is only intended to be used by DXE modules that need save
> confidential information to LockBox and get it by PEI modules in S3 phase.
>
> -Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions
> @@ -85,7 +85,10 @@ SetLockBoxAttributes (
> @retval RETURN_SUCCESS the information is saved successfully.
> @retval RETURN_INVALID_PARAMETER the Guid is NULL, or Buffer is NULL, or Length is 0.
> @retval RETURN_NOT_FOUND the requested GUID not found.
> - @retval RETURN_BUFFER_TOO_SMALL the original buffer to too small to hold new information.
> + @retval RETURN_BUFFER_TOO_SMALL for lockbox with attribute LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE,
Here, the description should be "without attribute
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY".
> + the original buffer to too small to hold new information.
> + @retval RETURN_OUT_OF_RESOURCES for lockbox with attribute LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
> + no enough resource to save the information.
> @retval RETURN_ACCESS_DENIED it is too late to invoke this interface
> @retval RETURN_NOT_STARTED it is too early to invoke this interface
> @retval RETURN_UNSUPPORTED the service is not supported by implementaion.
> diff --git a/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c b/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c
> index c40dfea398..0adda1e2a9 100644
> --- a/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c
> +++ b/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c
> @@ -1,6 +1,6 @@
> /** @file
>
> -Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions
> @@ -76,7 +76,10 @@ SetLockBoxAttributes (
> @retval RETURN_SUCCESS the information is saved successfully.
> @retval RETURN_INVALID_PARAMETER the Guid is NULL, or Buffer is NULL, or Length is 0.
> @retval RETURN_NOT_FOUND the requested GUID not found.
> - @retval RETURN_BUFFER_TOO_SMALL the original buffer to too small to hold new information.
> + @retval RETURN_BUFFER_TOO_SMALL for lockbox with attribute LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE,
> + the original buffer to too small to hold new information.
> + @retval RETURN_OUT_OF_RESOURCES for lockbox with attribute LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
> + no enough resource to save the information.
> @retval RETURN_ACCESS_DENIED it is too late to invoke this interface
> @retval RETURN_NOT_STARTED it is too early to invoke this interface
> @retval RETURN_UNSUPPORTED the service is not supported by implementaion.
> diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
> index 0428decbac..5ee563b71f 100644
> --- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
> +++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
> @@ -300,7 +300,10 @@ SetLockBoxAttributes (
> @retval RETURN_SUCCESS the information is saved successfully.
> @retval RETURN_INVALID_PARAMETER the Guid is NULL, or Buffer is NULL, or Length is 0.
> @retval RETURN_NOT_FOUND the requested GUID not found.
> - @retval RETURN_BUFFER_TOO_SMALL the original buffer to too small to hold new information.
> + @retval RETURN_BUFFER_TOO_SMALL for lockbox with attribute LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE,
> + the original buffer to too small to hold new information.
> + @retval RETURN_OUT_OF_RESOURCES for lockbox with attribute LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
> + no enough resource to save the information.
> @retval RETURN_ACCESS_DENIED it is too late to invoke this interface
> @retval RETURN_NOT_STARTED it is too early to invoke this interface
> @retval RETURN_UNSUPPORTED the service is not supported by implementaion.
> diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
> index 8c3e65bc96..19fdd995c6 100644
> --- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
> +++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
> @@ -477,7 +477,10 @@ SetLockBoxAttributes (
> @retval RETURN_SUCCESS the information is saved successfully.
> @retval RETURN_INVALID_PARAMETER the Guid is NULL, or Buffer is NULL, or Length is 0.
> @retval RETURN_NOT_FOUND the requested GUID not found.
> - @retval RETURN_BUFFER_TOO_SMALL the original buffer to too small to hold new information.
> + @retval RETURN_BUFFER_TOO_SMALL for lockbox with attribute LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE,
> + the original buffer to too small to hold new information.
> + @retval RETURN_OUT_OF_RESOURCES for lockbox with attribute LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
> + no enough resource to save the information.
> @retval RETURN_ACCESS_DENIED it is too late to invoke this interface
> @retval RETURN_NOT_STARTED it is too early to invoke this interface
> @retval RETURN_UNSUPPORTED the service is not supported by implementaion.
> diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> index c912d187a4..d1cff97ba1 100644
> --- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> +++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> @@ -604,7 +604,10 @@ SetLockBoxAttributes (
> @retval RETURN_SUCCESS the information is saved successfully.
> @retval RETURN_INVALID_PARAMETER the Guid is NULL, or Buffer is NULL, or Length is 0.
> @retval RETURN_NOT_FOUND the requested GUID not found.
> - @retval RETURN_BUFFER_TOO_SMALL the original buffer to too small to hold new information.
> + @retval RETURN_BUFFER_TOO_SMALL for lockbox with attribute LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE,
> + the original buffer to too small to hold new information.
> + @retval RETURN_OUT_OF_RESOURCES for lockbox with attribute LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
> + no enough resource to save the information.
> @retval RETURN_ACCESS_DENIED it is too late to invoke this interface
> @retval RETURN_NOT_STARTED it is too early to invoke this interface
> @retval RETURN_UNSUPPORTED the service is not supported by implementaion.
> @@ -619,13 +622,16 @@ UpdateLockBox (
> )
> {
> SMM_LOCK_BOX_DATA *LockBox;
> + EFI_PHYSICAL_ADDRESS SmramBuffer;
> + EFI_STATUS Status;
>
> DEBUG ((DEBUG_INFO, "SmmLockBoxSmmLib UpdateLockBox - Enter\n"));
>
> //
> // Basic check
> //
> - if ((Guid == NULL) || (Buffer == NULL) || (Length == 0)) {
> + if ((Guid == NULL) || (Buffer == NULL) || (Length == 0) ||
> + (Length > MAX_UINTN - Offset)) {
> DEBUG ((DEBUG_INFO, "SmmLockBoxSmmLib UpdateLockBox - Exit (%r)\n", EFI_INVALID_PARAMETER));
> return EFI_INVALID_PARAMETER;
> }
> @@ -643,8 +649,66 @@ UpdateLockBox (
> // Update data
> //
> if (LockBox->Length < Offset + Length) {
> - DEBUG ((DEBUG_INFO, "SmmLockBoxSmmLib UpdateLockBox - Exit (%r)\n", EFI_BUFFER_TOO_SMALL));
> - return EFI_BUFFER_TOO_SMALL;
> + if ((LockBox->Attributes & LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY) != 0) {
> + //
> + // If 'LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY' attribute is set, enlarge the
> + // LockBox.
> + //
> + DEBUG ((
> + DEBUG_INFO,
> + "SmmLockBoxSmmLib UpdateLockBox - Origin LockBox too small, enlarge.\n"
> + ));
> +
> + if (EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (LockBox->Length)) < Offset + Length) {
Need add (UINTN) type cast before LockBox->Length.
Thanks,
Star
> + //
> + // In SaveLockBox(), the SMRAM buffer allocated for LockBox is of page
> + // granularity. Here, if the required size is larger than the origin size
> + // of the pages, allocate new buffer from SMRAM to enlarge the LockBox.
> + //
> + DEBUG ((
> + DEBUG_INFO,
> + "SmmLockBoxSmmLib UpdateLockBox - Allocate new buffer to enlarge.\n"
> + ));
> + Status = gSmst->SmmAllocatePages (
> + AllocateAnyPages,
> + EfiRuntimeServicesData,
> + EFI_SIZE_TO_PAGES (Offset + Length),
> + &SmramBuffer
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_INFO, "SmmLockBoxSmmLib UpdateLockBox - Exit (%r)\n", EFI_OUT_OF_RESOURCES));
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + //
> + // Copy origin data to the new SMRAM buffer and wipe the content in the
> + // origin SMRAM buffer.
> + //
> + CopyMem ((VOID *)(UINTN)SmramBuffer, (VOID *)(UINTN)LockBox->SmramBuffer, (UINTN)LockBox->Length);
> + ZeroMem ((VOID *)(UINTN)LockBox->SmramBuffer, (UINTN)LockBox->Length);
> + gSmst->SmmFreePages (LockBox->SmramBuffer, EFI_SIZE_TO_PAGES ((UINTN)LockBox->Length));
> +
> + LockBox->SmramBuffer = SmramBuffer;
> + }
> +
> + //
> + // Handle potential uninitialized content in the LockBox.
> + //
> + if (Offset > LockBox->Length) {
> + ZeroMem (
> + (VOID *)((UINTN)LockBox->SmramBuffer + (UINTN)LockBox->Length),
> + Offset - (UINTN)LockBox->Length
> + );
> + }
> + LockBox->Length = Offset + Length;
> + } else {
> + //
> + // If 'LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY' attribute is NOT set, return
> + // EFI_BUFFER_TOO_SMALL directly.
> + //
> + DEBUG ((DEBUG_INFO, "SmmLockBoxSmmLib UpdateLockBox - Exit (%r)\n", EFI_BUFFER_TOO_SMALL));
> + return EFI_BUFFER_TOO_SMALL;
> + }
> }
> ASSERT ((UINTN)LockBox->SmramBuffer <= (MAX_ADDRESS - Offset));
> CopyMem ((VOID *)((UINTN)LockBox->SmramBuffer + Offset), Buffer, Length);
>
next prev parent reply other threads:[~2019-02-15 5:14 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-11 7:57 [PATCH v4 00/13] Split the S3 PEI phase HW init codes from Opal driver Hao Wu
2019-02-11 7:57 ` [PATCH v4 01/13] MdeModulePkg: Add definitions for ATA AHCI host controller PPI Hao Wu
2019-02-11 7:57 ` [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Report StatusCode for device init failure Hao Wu
2019-02-13 0:51 ` Ni, Ray
2019-02-11 7:57 ` [PATCH v4 02/13] MdeModulePkg: Add definitions for EDKII PEI ATA PassThru PPI Hao Wu
2019-02-13 0:53 ` Ni, Ray
2019-02-11 7:57 ` [PATCH v4 03/13] MdeModulePkg: Add definitions for Storage Security Command PPI Hao Wu
2019-02-11 7:57 ` [PATCH v4 04/13] MdeModulePkg: Add GUID for LockBox to save storage dev to init in S3 Hao Wu
2019-02-13 0:54 ` Ni, Ray
2019-02-11 7:57 ` [PATCH v4 05/13] MdeModulePkg/NvmExpressPei: Avoid updating the module-level variable Hao Wu
2019-02-11 7:57 ` [PATCH v4 06/13] MdeModulePkg/NvmExpressPei: Add logic to produce SSC PPI Hao Wu
2019-02-11 7:57 ` [PATCH v4 07/13] MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList LockBox Hao Wu
2019-02-11 7:57 ` [PATCH v4 08/13] MdeModulePkg/AhciPei: Add AHCI mode ATA device support in PEI Hao Wu
2019-02-11 7:57 ` [PATCH v4 09/13] MdeModulePkg/SmmLockBoxLib: Use 'DEBUG_' prefix instead of 'EFI_D_' Hao Wu
2019-02-15 5:09 ` Zeng, Star
2019-02-11 7:57 ` [PATCH v4 10/13] MdeModulePkg/SmmLockBox(PEI): Remove an ASSERT in RestoreLockBox() Hao Wu
2019-02-15 5:09 ` Zeng, Star
2019-02-11 7:57 ` [PATCH v4 11/13] MdeModulePkg/SmmLockBoxLib: Support LockBox enlarge in UpdateLockBox() Hao Wu
2019-02-15 5:13 ` Zeng, Star [this message]
2019-02-11 7:57 ` [PATCH v4 12/13] OvmfPkg/LockBoxLib: Update the comments for API UpdateLockBox() Hao Wu
2019-02-11 10:55 ` Laszlo Ersek
2019-02-13 1:01 ` Ni, Ray
2019-02-11 7:57 ` [PATCH v4 13/13] SecurityPkg/OpalPassword: Remove HW init codes and consume SSC PPI Hao Wu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=35f5e873-7b32-ba28-c9cb-0d8d4050a067@intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox