Re: [edk2-devel] [PATCH V1 3/3] OvmfPkg/TdxDxe: Clear the registers before tdcall
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4696 > > Refer to the [GHCI] spec, TDVF should clear the BIT5 for RBP in the mask. > And TDVF should clear the regitsers to avoid leaking secrets to VMM. > > Reference: > [GHCI]: TDX Guest-Host-Communication Interface v1.5 > https://cdrdv2.intel.com/v1/dl/getContent/726792 > > Cc: Erdem Aktas > Cc: James Bottomley > Cc: Jiewen Yao > Cc: Min Xu > Cc: Tom Lendacky > Cc: Michael Roth > Cc: Gerd Hoffmann > Cc: Erdem Aktas > Cc: Isaku Yamahata > Signed-off-by: Ceping Sun > --- > OvmfPkg/TdxDxe/X64/ApRunLoop.nasm | 30 ++ > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm > b/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm > index 0bef89c48552..57560015f491 100644 > --- a/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm > +++ b/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm > @@ -20,7 +20,7 @@ SECTION .text > > BITS 64 > > -%define TDVMCALL_EXPOSE_REGS_MASK 0xffec > +%define TDVMCALL_EXPOSE_REGS_MASK 0xffcc > %define TDVMCALL0x0 > %define EXIT_REASON_CPUID 0xa > > @@ -28,6 +28,30 @@ BITS 64 >db 0x66, 0x0f, 0x01, 0xcc > %endmacro > > +%macro tdcall_regs_preamble 2 > +mov rax, %1 > + > +xor rcx, rcx > +mov ecx, %2 > + > +; R10 = 0 (standard TDVMCALL) > + > +xor r10d, r10d > + > +; Zero out unused (for standard TDVMCALL) registers to avoid leaking > +; secrets to the VMM. > + > +xor esi, esi > +xor edi, edi > + > +xor edx, edx > +xor ebp, ebp > +xor r8d, r8d > +xor r9d, r9d > +xor r14, r14 > +xor r15, r15 We can just clear the corresponding bit of TDVMCALL_EXPOSE_REGS_MASK in addition to RBP. Same to 1/3 and 3/3. We can eliminate tdcall_regs_postamble. Any reason to bother to zero those registers and pass them to VMM? Thanks, -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#116035): https://edk2.groups.io/g/devel/message/116035 Mute This Topic: https://groups.io/mt/104577524/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] OvmfPkg/BaseMemEncryptTdxLib: Make SetOrClearSharedBit() handle retry error
From: Isaku Yamahata
TDG.VP.VMCALL can return TDG.VP.VMCALL_RETRY when the length is too
large so that VMM can avoid too long processing time. The caller should
retry with the updated starting GPA on the error. Add check
TDG.VP.VMCALL_RETRY check.
Signed-off-by: Isaku Yamahata
---
MdePkg/Include/IndustryStandard/Tdx.h | 4
MdePkg/Library/BaseLib/X64/TdVmcall.nasm | 4 +---
.../Library/BaseMemEncryptTdxLib/MemoryEncryption.c| 10 --
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/MdePkg/Include/IndustryStandard/Tdx.h
b/MdePkg/Include/IndustryStandard/Tdx.h
index 81df1361842b..4ea2960a70f9 100644
--- a/MdePkg/Include/IndustryStandard/Tdx.h
+++ b/MdePkg/Include/IndustryStandard/Tdx.h
@@ -103,6 +103,10 @@
#define TDVMCALL_REPORT_FATAL_ERR0x10003
#define TDVMCALL_SETUP_EVENT_NOTIFY 0x10004
+// TDVMCALL API sub-function Completion Status Codes
+#define TDG_VP_VMCALL_SUCCESS 0x
+#define TDG_VP_VMCALL_RETRY 0x0001
+
#pragma pack(1)
typedef struct {
UINT64Data[6];
diff --git a/MdePkg/Library/BaseLib/X64/TdVmcall.nasm
b/MdePkg/Library/BaseLib/X64/TdVmcall.nasm
index 5ecc10b17193..8dd9bfcbfa14 100644
--- a/MdePkg/Library/BaseLib/X64/TdVmcall.nasm
+++ b/MdePkg/Library/BaseLib/X64/TdVmcall.nasm
@@ -133,9 +133,7 @@ ASM_PFX(TdVmCall):
test r9, r9
jz .no_return_data
- ; On success, propagate TDVMCALL output value to output param
- test rax, rax
- jnz .no_return_data
+ ; Propagate TDVMCALL output value to output param
mov [r9], r11
.no_return_data:
tdcall_regs_postamble
diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
index a01dc98852b8..d55c2a34a44e 100644
--- a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
+++ b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
@@ -526,6 +526,8 @@ SetOrClearSharedBit (
UINT64TdStatus;
EFI_STATUSStatus;
EDKII_MEMORY_ACCEPT_PROTOCOL *MemoryAcceptProtocol;
+ PHYSICAL_ADDRESS TmpPhysicalAddress;
+ UINT64TmpLength;
AddressEncMask = GetMemEncryptionAddressMask ();
@@ -540,8 +542,12 @@ SetOrClearSharedBit (
PhysicalAddress &= ~AddressEncMask;
}
- TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0, NULL);
- if (TdStatus != 0) {
+ TmpPhysicalAddress = PhysicalAddress;
+ do {
+TmpLength = PhysicalAddress + Length - TmpPhysicalAddress;
+TdStatus = TdVmCall (TDVMCALL_MAPGPA, TmpPhysicalAddress, TmpLength, 0, 0,
);
+ } while (TdStatus == TDG_VP_VMCALL_RETRY);
+ if (TdStatus != TDG_VP_VMCALL_SUCCESS) {
DEBUG ((DEBUG_ERROR, "%a: TdVmcall(MAPGPA) failed with %llx\n", __func__,
TdStatus));
ASSERT (FALSE);
return EFI_DEVICE_ERROR;
base-commit: 677f2c6f1509da21258e02957b869b71b008fc61
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107477): https://edk2.groups.io/g/devel/message/107477
Mute This Topic: https://groups.io/mt/100514591/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] OvmfPkg: TdxDxe: Fix AsmRelocateApMailBoxLoop
In TDX, Application Processor busy-loops on Mailbox for OS to issue
MpProtectedModeWakeupCommandWakeup command to UEFI. As the AP acking to
it, it clears the command member according to ACPI spec 6.4, 5.2.12.19
Multiprocessor Wakeup Structure: "The application processor need clear the
command to Noop(0) as the acknowledgement that the command is received."
However, AsmRelocateApMailBoxLoop wrongly clears WakeupVector. Correctly
clear command instead of WakeupVector.
Without this patch, TD guest kernel fails to boot APs.
Fixes: fae5c1464d ("OvmfPkg: Add TdxDxe driver")
Cc: Min Xu
Signed-off-by: Isaku Yamahata
---
OvmfPkg/TdxDxe/X64/ApRunLoop.nasm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm
b/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm
index 49bd04415c..a859375fb8 100644
--- a/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm
+++ b/OvmfPkg/TdxDxe/X64/ApRunLoop.nasm
@@ -69,7 +69,7 @@ MailBoxWakeUp:
movrax, [rbx + WakeupVectorOffset]
; OS sends a wakeup command for a given APIC ID, firmware is supposed to
reset
; the command field back to zero as acknowledgement.
-movqword [rbx + WakeupVectorOffset], 0
+movqword [rbx + CommandOffset], 0
jmprax
MailBoxSleep:
jmp $
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89982): https://edk2.groups.io/g/devel/message/89982
Mute This Topic: https://groups.io/mt/91307432/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
