Reviewed-by: Jiewen Yao <jiewen....@intel.com>
> -----Original Message-----
> From: Xu, Min M <min.m...@intel.com>
> Sent: Wednesday, January 18, 2023 7:53 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m...@intel.com>; Aktas, Erdem
> <erdemak...@google.com>; James Bottomley <j...@linux.ibm.com>; Yao,
> Jiewen <jiewen....@intel.com>; Gerd Hoffmann <kra...@redhat.com>;
> Tom Lendacky <thomas.lenda...@amd.com>; Michael Roth
> <michael.r...@amd.com>
> Subject: [PATCH V2 1/1] OvmfPkg/BaseMemEncryptTdxLib: Refactor error
> handle of SetOrClearSharedBit
>
> From: Min M Xu <min.m...@intel.com>
>
> The previous implementation of SetOrClearSharedBit doesn't handle the
> error correctly. In this patch SetOrClearSharedBit is changed to return
> error code so that the caller can handle it.
>
> Cc: Erdem Aktas <erdemak...@google.com>
> Cc: James Bottomley <j...@linux.ibm.com>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> Cc: Tom Lendacky <thomas.lenda...@amd.com>
> Cc: Michael Roth <michael.r...@amd.com>
> Reviewed-by: Jiewen Yao <jiewen....@intel.com>
> Signed-off-by: Min Xu <min.m...@intel.com>
> ---
> .../BaseMemEncryptTdxLib/MemoryEncryption.c | 48 +++++++++++++++----
> 1 file changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> index 503f626d75c6..5b13042512ad 100644
> --- a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> +++ b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> @@ -510,8 +510,11 @@ Split1GPageTo2M (
> @param[in] PagetablePoint Page table entry pointer (PTE).
> @param[in] Mode Set or Clear shared bit
>
> + @retval EFI_SUCCESS Successfully set or clear the memory
> shared bit
> + @retval Others Other error as indicated
> **/
> -STATIC VOID
> +STATIC
> +EFI_STATUS
> SetOrClearSharedBit (
> IN OUT UINT64 *PageTablePointer,
> IN TDX_PAGETABLE_MODE Mode,
> @@ -520,7 +523,8 @@ SetOrClearSharedBit (
> )
> {
> UINT64 AddressEncMask;
> - UINT64 Status;
> + UINT64 TdStatus;
> + EFI_STATUS Status;
> EDKII_MEMORY_ACCEPT_PROTOCOL *MemoryAcceptProtocol;
>
> AddressEncMask = GetMemEncryptionAddressMask ();
> @@ -536,16 +540,30 @@ SetOrClearSharedBit (
> PhysicalAddress &= ~AddressEncMask;
> }
>
> - Status = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0,
> NULL);
> + TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0,
> NULL);
> + if (TdStatus != 0) {
> + DEBUG ((DEBUG_ERROR, "%a: TdVmcall(MAPGPA) failed with %llx\n",
> __FUNCTION__, TdStatus));
> + ASSERT (FALSE);
> + return EFI_DEVICE_ERROR;
> + }
>
> //
> // If changing shared to private, must accept-page again
> //
> if (Mode == ClearSharedBit) {
> Status = gBS->LocateProtocol (&gEdkiiMemoryAcceptProtocolGuid, NULL,
> (VOID **)&MemoryAcceptProtocol);
> - ASSERT (!EFI_ERROR (Status));
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: Failed to locate MemoryAcceptProtocol
> with %r\n", __FUNCTION__, Status));
> + ASSERT (FALSE);
> + return Status;
> + }
> +
> Status = MemoryAcceptProtocol->AcceptMemory (MemoryAcceptProtocol,
> PhysicalAddress, Length);
> - ASSERT (!EFI_ERROR (Status));
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: Failed to AcceptMemory with %r\n",
> __FUNCTION__, Status));
> + ASSERT (FALSE);
> + return Status;
> + }
> }
>
> DEBUG ((
> @@ -558,6 +576,8 @@ SetOrClearSharedBit (
> Mode,
> Status
> ));
> +
> + return EFI_SUCCESS;
> }
>
> /**
> @@ -747,7 +767,11 @@ SetMemorySharedOrPrivate (
> // If we have at least 1GB to go, we can just update this entry
> //
> if (!(PhysicalAddress & (BIT30 - 1)) && (Length >= BIT30)) {
> - SetOrClearSharedBit (&PageDirectory1GEntry->Uint64, Mode,
> PhysicalAddress, BIT30);
> + Status = SetOrClearSharedBit (&PageDirectory1GEntry->Uint64, Mode,
> PhysicalAddress, BIT30);
> + if (EFI_ERROR (Status)) {
> + goto Done;
> + }
> +
> DEBUG ((
> DEBUG_VERBOSE,
> "%a:%a: updated 1GB entry for Physical=0x%Lx\n",
> @@ -809,7 +833,11 @@ SetMemorySharedOrPrivate (
> // If we have at least 2MB left to go, we can just update this entry
> //
> if (!(PhysicalAddress & (BIT21-1)) && (Length >= BIT21)) {
> - SetOrClearSharedBit (&PageDirectory2MEntry->Uint64, Mode,
> PhysicalAddress, BIT21);
> + Status = SetOrClearSharedBit (&PageDirectory2MEntry->Uint64, Mode,
> PhysicalAddress, BIT21);
> + if (EFI_ERROR (Status)) {
> + goto Done;
> + }
> +
> PhysicalAddress += BIT21;
> Length -= BIT21;
> } else {
> @@ -856,7 +884,11 @@ SetMemorySharedOrPrivate (
> goto Done;
> }
>
> - SetOrClearSharedBit (&PageTableEntry->Uint64, Mode, PhysicalAddress,
> EFI_PAGE_SIZE);
> + Status = SetOrClearSharedBit (&PageTableEntry->Uint64, Mode,
> PhysicalAddress, EFI_PAGE_SIZE);
> + if (EFI_ERROR (Status)) {
> + goto Done;
> + }
> +
> PhysicalAddress += EFI_PAGE_SIZE;
> Length -= EFI_PAGE_SIZE;
> }
> --
> 2.29.2.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98739): https://edk2.groups.io/g/devel/message/98739
Mute This Topic: https://groups.io/mt/96343919/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
