On 12/12/22 23:48, Min Xu wrote:
From: Min M Xu <min.m...@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4171
A typical QEMU fw_cfg read bytes with IOMMU for td guest is that:
(QemuFwCfgReadBytes@QemuFwCfgLib.c is the example)
1) Allocate DMA Access buffer
2) Map actual data buffer
3) start the transfer and wait for the transfer to complete
4) Free DMA Access buffer
5) Un-map actual data buffer
In step 1/2, Private memories are allocated, converted to shared memories.
In Step 4/5 the shared memories are converted to private memories and
accepted again. The final step is to free the pages.
This is time-consuming and impacts td guest's boot perf (both direct boot
and grub boot) badly.
In a typical grub boot, there are about 5000 calls of page allocation and
private/share conversion. Most of page size is less than 32KB.
This patch allocates a memory region and initializes it into pieces of
memory with different sizes. A piece of such memory consists of 2 parts:
the first page is of private memory, and the other pages are shared
memory. This is to meet the layout of common buffer.
When allocating bounce buffer in IoMmuMap(), IoMmuAllocateBounceBuffer()
is called to allocate the buffer. Accordingly when freeing bounce buffer
in IoMmuUnmapWorker(), IoMmuFreeBounceBuffer() is called to free the
bounce buffer. CommonBuffer is allocated by IoMmuAllocateCommonBuffer
and accordingly freed by IoMmuFreeCommonBuffer.
This feature is tested in Intel TDX pre-production platform. It saves up
to hundreds of ms in a grub boot.
This patch causes crashes for SEV guests and breaks bisect-ability of the
EDK2 tree. See below...
Cc: Erdem Aktas <erdemak...@google.com>
Cc: James Bottomley <j...@linux.ibm.com>
Cc: Jiewen Yao <jiewen....@intel.com>
Cc: Tom Lendacky <thomas.lenda...@amd.com>
Cc: Gerd Hoffmann <kra...@redhat.com>
Reviewed-by: Jiewen Yao <jiewen....@intel.com>
Signed-off-by: Min Xu <min.m...@intel.com>
---
OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 142 +++++-----
OvmfPkg/IoMmuDxe/IoMmuBuffer.c | 459 +++++++++++++++++++++++++++++++
OvmfPkg/IoMmuDxe/IoMmuDxe.inf | 1 +
OvmfPkg/IoMmuDxe/IoMmuInternal.h | 179 ++++++++++++
4 files changed, 710 insertions(+), 71 deletions(-)
create mode 100644 OvmfPkg/IoMmuDxe/IoMmuBuffer.c
create mode 100644 OvmfPkg/IoMmuDxe/IoMmuInternal.h
diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
index 6b65897f032a..77e46bbf4a60 100644
--- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
+++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
@@ -633,8 +614,9 @@ IoMmuAllocateBuffer (
CommonBufferHeader = (VOID *)(UINTN)PhysicalAddress;
PhysicalAddress += EFI_PAGE_SIZE;
- CommonBufferHeader->Signature = COMMON_BUFFER_SIG;
- CommonBufferHeader->StashBuffer = StashBuffer;
+ CommonBufferHeader->Signature = COMMON_BUFFER_SIG;
+ CommonBufferHeader->StashBuffer = StashBuffer;
+ CommonBufferHeader->ReservedMemBitmap = ReservedMemBitmap;
*HostAddress = (VOID *)(UINTN)PhysicalAddress;
@@ -707,7 +689,7 @@ IoMmuFreeBuffer (
// Release the common buffer itself. Unmap() has re-encrypted it in-place,
so
// no need to zero it.
//
- return gBS->FreePages ((UINTN)CommonBufferHeader, CommonBufferPages);
+ return IoMmuFreeCommonBuffer (CommonBufferHeader, CommonBufferPages);
}
/**
@@ -878,6 +860,11 @@ IoMmuUnmapAllMappings (
TRUE // MemoryMapLocked
);
}
+
+ //
+ // Release the reserved shared memory as well.
+ //
+ IoMmuReleaseReservedSharedMem (TRUE);
This call is the reason for the crash. You'll need to check for whether
there has been any shared memory reserved before attempting to free it (in
the case of SEV that doesn't happen until patch #3 of the series). I think
adding the check in IoMmuReleaseReservedSharedMem() itself might be best,
since you can also experience this crash should the below allocation fail,
too.
Thanks,
Tom
}
/**
@@ -936,6 +923,19 @@ InstallIoMmuProtocol (
goto CloseExitBootEvent;
}
+ //
+ // Currently only Tdx guest support Reserved shared memory for DMA operation.
+ //
+ if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
+ mReservedSharedMemSupported = TRUE;
+ Status = IoMmuInitReservedSharedMem ();
+ if (EFI_ERROR (Status)) {
+ mReservedSharedMemSupported = FALSE;
+ } else {
+ DEBUG ((DEBUG_INFO, "%a: Feature of reserved memory for DMA is
supported.\n", __FUNCTION__));
+ }
+ }
+
return EFI_SUCCESS;
CloseExitBootEvent:
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97312): https://edk2.groups.io/g/devel/message/97312
Mute This Topic: https://groups.io/mt/95639824/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
