* [PATCH V1 0/3] Fix TDVF issues
@ 2023-01-15 23:27 Min Xu
2023-01-15 23:27 ` [PATCH V1 1/3] OvmfPkg/CcExitLib: Initialize Status in IoExit Min Xu
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Min Xu @ 2023-01-15 23:27 UTC (permalink / raw)
To: devel
Cc: Min Xu, Jiewen Yao, Jian J Wang, Erdem Aktas, James Bottomley,
Gerd Hoffmann, Tom Lendacky, Michael Roth
This patch-set fix below TDVF issues:
Patch#1: Initialize Status in IoExit
Patch#2: Extend EFI boot variable to PCR[1] according to TCG PC Client
PFP spec.
Patch#3: Refactor error handle of SetOrClearSharedBit so that the caller
can handle the returned error.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
Min M Xu (3):
OvmfPkg/CcExitLib: Initialize Status in IoExit
SecurityPkg/TdTcg2Dxe: Extend EFI boot variable to PCR[1]
OvmfPkg/BaseMemEncryptTdxLib: Refactor error handle of
SetOrClearSharedBit
.../BaseMemEncryptTdxLib/MemoryEncryption.c | 48 +++++++++++++++----
OvmfPkg/Library/CcExitLib/CcExitVeHandler.c | 9 ++--
SecurityPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 6 +--
3 files changed, 46 insertions(+), 17 deletions(-)
--
2.29.2.windows.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V1 1/3] OvmfPkg/CcExitLib: Initialize Status in IoExit
2023-01-15 23:27 [PATCH V1 0/3] Fix TDVF issues Min Xu
@ 2023-01-15 23:27 ` Min Xu
2023-01-15 23:27 ` [PATCH V1 2/3] SecurityPkg/TdTcg2Dxe: Extend EFI boot variable to PCR[1] Min Xu
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Min Xu @ 2023-01-15 23:27 UTC (permalink / raw)
To: devel
Cc: Min M Xu, Erdem Aktas, James Bottomley, Jiewen Yao, Gerd Hoffmann,
Tom Lendacky, Michael Roth
From: Min M Xu <min.m.xu@intel.com>
Status should be initialized otherwise it may return unexpected value.
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/Library/CcExitLib/CcExitVeHandler.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c b/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
index 30d547d5fe55..872f772a5ac8 100644
--- a/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
+++ b/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
@@ -105,10 +105,11 @@ IoExit (
UINT64 RepCnt;
UINT64 Status;
- Val = 0;
- Write = Veinfo->ExitQualification.Io.Direction ? FALSE : TRUE;
- Size = Veinfo->ExitQualification.Io.Size + 1;
- Port = Veinfo->ExitQualification.Io.Port;
+ Val = 0;
+ Status = 0;
+ Write = Veinfo->ExitQualification.Io.Direction ? FALSE : TRUE;
+ Size = Veinfo->ExitQualification.Io.Size + 1;
+ Port = Veinfo->ExitQualification.Io.Port;
if (Veinfo->ExitQualification.Io.String) {
//
--
2.29.2.windows.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V1 2/3] SecurityPkg/TdTcg2Dxe: Extend EFI boot variable to PCR[1]
2023-01-15 23:27 [PATCH V1 0/3] Fix TDVF issues Min Xu
2023-01-15 23:27 ` [PATCH V1 1/3] OvmfPkg/CcExitLib: Initialize Status in IoExit Min Xu
@ 2023-01-15 23:27 ` Min Xu
2023-01-15 23:27 ` [PATCH V1 3/3] OvmfPkg/BaseMemEncryptTdxLib: Refactor error handle of SetOrClearSharedBit Min Xu
2023-01-16 0:44 ` [PATCH V1 0/3] Fix TDVF issues Yao, Jiewen
3 siblings, 0 replies; 6+ messages in thread
From: Min Xu @ 2023-01-15 23:27 UTC (permalink / raw)
To: devel; +Cc: Min M Xu, Jiewen Yao, Jian J Wang
From: Min M Xu <min.m.xu@intel.com>
According to TCG PC Client PFP spec 0021 Section 2.4.4.2 EFI boot variable
should be measured and extended to PCR[1], not PCR[5]. This patch is
proposed to fix this error.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
SecurityPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/SecurityPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c b/SecurityPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
index d19923b0c682..59341a8c0250 100644
--- a/SecurityPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
+++ b/SecurityPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c
@@ -1873,12 +1873,8 @@ ReadAndMeasureBootVariable (
OUT VOID **VarData
)
{
- //
- // Boot variables are measured into (PCR[5]) RTMR[1],
- // details in section 8.1 of TDVF design guide.
- //
return ReadAndMeasureVariable (
- MapPcrToMrIndex (5),
+ MapPcrToMrIndex (1),
EV_EFI_VARIABLE_BOOT,
VarName,
VendorGuid,
--
2.29.2.windows.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V1 3/3] OvmfPkg/BaseMemEncryptTdxLib: Refactor error handle of SetOrClearSharedBit
2023-01-15 23:27 [PATCH V1 0/3] Fix TDVF issues Min Xu
2023-01-15 23:27 ` [PATCH V1 1/3] OvmfPkg/CcExitLib: Initialize Status in IoExit Min Xu
2023-01-15 23:27 ` [PATCH V1 2/3] SecurityPkg/TdTcg2Dxe: Extend EFI boot variable to PCR[1] Min Xu
@ 2023-01-15 23:27 ` Min Xu
2023-01-16 0:44 ` [PATCH V1 0/3] Fix TDVF issues Yao, Jiewen
3 siblings, 0 replies; 6+ messages in thread
From: Min Xu @ 2023-01-15 23:27 UTC (permalink / raw)
To: devel
Cc: Min M Xu, Erdem Aktas, James Bottomley, Jiewen Yao, Gerd Hoffmann,
Tom Lendacky, Michael Roth
From: Min M Xu <min.m.xu@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 <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Signed-off-by: Min Xu <min.m.xu@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V1 0/3] Fix TDVF issues
2023-01-15 23:27 [PATCH V1 0/3] Fix TDVF issues Min Xu
` (2 preceding siblings ...)
2023-01-15 23:27 ` [PATCH V1 3/3] OvmfPkg/BaseMemEncryptTdxLib: Refactor error handle of SetOrClearSharedBit Min Xu
@ 2023-01-16 0:44 ` Yao, Jiewen
2023-01-17 23:54 ` Min Xu
3 siblings, 1 reply; 6+ messages in thread
From: Yao, Jiewen @ 2023-01-16 0:44 UTC (permalink / raw)
To: Xu, Min M, devel@edk2.groups.io
Cc: Wang, Jian J, Aktas, Erdem, James Bottomley, Gerd Hoffmann,
Tom Lendacky, Michael Roth, Yao, Jiewen
>From process perspective, I see no reason to combine them into one patch set, because each patch is individual, and they are handling different problem.
Also, there is no reason to mix the fix in SecurityPkg with the fix in OvmfPkg, if they are not related.
Please split them into 3 different patches.
With comment above, reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Monday, January 16, 2023 7:28 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Michael Roth <michael.roth@amd.com>
> Subject: [PATCH V1 0/3] Fix TDVF issues
>
> This patch-set fix below TDVF issues:
> Patch#1: Initialize Status in IoExit
> Patch#2: Extend EFI boot variable to PCR[1] according to TCG PC Client
> PFP spec.
> Patch#3: Refactor error handle of SetOrClearSharedBit so that the caller
> can handle the returned error.
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Michael Roth <michael.roth@amd.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
>
> Min M Xu (3):
> OvmfPkg/CcExitLib: Initialize Status in IoExit
> SecurityPkg/TdTcg2Dxe: Extend EFI boot variable to PCR[1]
> OvmfPkg/BaseMemEncryptTdxLib: Refactor error handle of
> SetOrClearSharedBit
>
> .../BaseMemEncryptTdxLib/MemoryEncryption.c | 48 +++++++++++++++----
> OvmfPkg/Library/CcExitLib/CcExitVeHandler.c | 9 ++--
> SecurityPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 6 +--
> 3 files changed, 46 insertions(+), 17 deletions(-)
>
> --
> 2.29.2.windows.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V1 0/3] Fix TDVF issues
2023-01-16 0:44 ` [PATCH V1 0/3] Fix TDVF issues Yao, Jiewen
@ 2023-01-17 23:54 ` Min Xu
0 siblings, 0 replies; 6+ messages in thread
From: Min Xu @ 2023-01-17 23:54 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io
Cc: Wang, Jian J, Aktas, Erdem, James Bottomley, Gerd Hoffmann,
Tom Lendacky, Michael Roth
Thanks for the comments. They're will be sent for review in separate patches.
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Monday, January 16, 2023 8:45 AM
> To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Michael Roth <michael.roth@amd.com>; Yao,
> Jiewen <jiewen.yao@intel.com>
> Subject: RE: [PATCH V1 0/3] Fix TDVF issues
>
> From process perspective, I see no reason to combine them into one patch set,
> because each patch is individual, and they are handling different problem.
> Also, there is no reason to mix the fix in SecurityPkg with the fix in OvmfPkg, if
> they are not related.
>
> Please split them into 3 different patches.
>
> With comment above, reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
>
> Thank you
> Yao, Jiewen
>
> > -----Original Message-----
> > From: Xu, Min M <min.m.xu@intel.com>
> > Sent: Monday, January 16, 2023 7:28 AM
> > To: devel@edk2.groups.io
> > Cc: Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Aktas,
> > Erdem <erdemaktas@google.com>; James Bottomley
> <jejb@linux.ibm.com>;
> > Gerd Hoffmann <kraxel@redhat.com>; Tom Lendacky
> > <thomas.lendacky@amd.com>; Michael Roth <michael.roth@amd.com>
> > Subject: [PATCH V1 0/3] Fix TDVF issues
> >
> > This patch-set fix below TDVF issues:
> > Patch#1: Initialize Status in IoExit
> > Patch#2: Extend EFI boot variable to PCR[1] according to TCG PC Client
> > PFP spec.
> > Patch#3: Refactor error handle of SetOrClearSharedBit so that the caller
> > can handle the returned error.
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Erdem Aktas <erdemaktas@google.com>
> > Cc: James Bottomley <jejb@linux.ibm.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: Michael Roth <michael.roth@amd.com>
> > Signed-off-by: Min Xu <min.m.xu@intel.com>
> >
> > Min M Xu (3):
> > OvmfPkg/CcExitLib: Initialize Status in IoExit
> > SecurityPkg/TdTcg2Dxe: Extend EFI boot variable to PCR[1]
> > OvmfPkg/BaseMemEncryptTdxLib: Refactor error handle of
> > SetOrClearSharedBit
> >
> > .../BaseMemEncryptTdxLib/MemoryEncryption.c | 48 +++++++++++++++---
> -
> > OvmfPkg/Library/CcExitLib/CcExitVeHandler.c | 9 ++--
> > SecurityPkg/Tcg/TdTcg2Dxe/TdTcg2Dxe.c | 6 +--
> > 3 files changed, 46 insertions(+), 17 deletions(-)
> >
> > --
> > 2.29.2.windows.2
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-17 23:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-15 23:27 [PATCH V1 0/3] Fix TDVF issues Min Xu
2023-01-15 23:27 ` [PATCH V1 1/3] OvmfPkg/CcExitLib: Initialize Status in IoExit Min Xu
2023-01-15 23:27 ` [PATCH V1 2/3] SecurityPkg/TdTcg2Dxe: Extend EFI boot variable to PCR[1] Min Xu
2023-01-15 23:27 ` [PATCH V1 3/3] OvmfPkg/BaseMemEncryptTdxLib: Refactor error handle of SetOrClearSharedBit Min Xu
2023-01-16 0:44 ` [PATCH V1 0/3] Fix TDVF issues Yao, Jiewen
2023-01-17 23:54 ` Min Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox