| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
A recent change to the PciIoMap() function now propagates the return code
from the IoMmu protocol SetAttribute() operation. The implementation of
this operation in OvmfPkg/IoMmuDxe/CcIoMmu.c returns EFI_UNSUPPORTED,
resulting in a failure to boot the guest.
Provide an implementation for SetAttribute() that validates the IoMmu
access method being requested against the IoMmu mapping operation.
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <c0f9e95f557b601a045da015c1a97201e8aec2ab.1706634932.git.thomas.lendacky@amd.com>
Tested-by: Min Xu <min.m.xu@intel.com>
Reviewed-by: Min Xu <min.m.xu@intel.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Instead of relying on raising the TPL to protect the critical sections
that manipulate the global bitmask that keeps track of bounce buffer
allocations, use compare-and-exchange to manage the global variable, and
tweak the logic to line up with that.
Given that IoMmuDxe implements a singleton protocol that is shared
between multiple drivers, and considering the elaborate and confusing
requirements in the UEFP spec regarding TPL levels at which protocol
methods may be invoked, not relying on TPL levels at all is a more
robust approach in this case.
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2211060
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Pedro Falcato <pedro.falcato@gmail.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Searching for an unused bounce buffer in mReservedMemBitmap and
reserving the buffer by flipping the bit is a critical section
which must not be interrupted. Raise the TPL level to ensure
that.
Without this fix it can happen that IoMmuDxe hands out the same
bounce buffer twice, causing trouble down the road. Seen happening
in practice with VirtioNetDxe setting up the network interface (and
calling into IoMmuDxe from a polling timer callback) in parallel with
Boot Manager doing some disk I/O. An ASSERT() in VirtioNet caught
the buffer inconsistency.
Full story with lots of details and discussions is available here:
https://bugzilla.redhat.com/show_bug.cgi?id=2211060
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
__FUNCTION__ is a pre-standard extension that gcc and Visual C++ among
others support, while __func__ was standardized in C99.
Since it's more standard, replace __FUNCTION__ with __func__ throughout
OvmfPkg.
Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Add support to use the reserved shared memory within the IoMmu library.
This improves boot times for all SEV guests, with SEV-SNP benefiting the
most as it avoids the page state change call to the hypervisor.
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4171
IoMmuDxe once was designed to support DMA operation when SEV is enabled.
After TDX is enabled in IoMmuDxe, some files' name in IoMmuDxe need to
be more general. So this patch rename:
AmdSevIoMmu.h -> CcIoMmu.h
AmdSevIoMmu.c -> CcIoMmu.c
Accordingly there are some udates in IoMmuDxe.c and IoMmuDxe.inf.
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.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.
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
The IOMMU protocol driver provides capabilities to set a DMA access
attribute and methods to allocate, free, map and unmap the DMA memory
for the PCI Bus devices.
The current IoMmuDxe driver supports DMA operations inside SEV guest.
To support DMA operation in TDX guest,
CC_GUEST_IS_XXX (PcdConfidentialComputingGuestAttr) is used to determine
if it is SEV guest or TDX guest.
Due to security reasons all DMA operations inside the SEV/TDX guest must
be performed on shared pages. The IOMMU protocol driver for the SEV/TDX
guest uses a bounce buffer to map guest DMA buffer to shared pages in
order to provide the support for DMA operations inside SEV/TDX guest.
The call of SEV or TDX specific function to set/clear EncMask/SharedBit
is determined by CC_GUEST_IS_XXX (PcdConfidentialComputingGuestAttr).
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
|
|
|
|
|
|
|
|
|
|
|
|
| |
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3737
Apply uncrustify changes to .c/.h files in the OvmfPkg package
Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Andrew Fish <afish@apple.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
The Flush parameter is used to provide a hint whether the specified range
is Mmio address. Now that we have a dedicated helper to clear the
memory encryption mask for the Mmio address range, its safe to remove the
Flush parameter from MemEncryptSev{Set,Clear}PageEncMask().
Since the address specified in the MemEncryptSev{Set,Clear}PageEncMask()
points to a system RAM, thus a cache flush is required during the
encryption mask update.
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Message-Id: <20210519181949.6574-14-brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
https://bugzilla.tianocore.org/show_bug.cgi?id=1373
Replace BSD 2-Clause License with BSD+Patent License. This change is
based on the following emails:
https://lists.01.org/pipermail/edk2-devel/2019-February/036260.html
https://lists.01.org/pipermail/edk2-devel/2018-October/030385.html
RFCs with detailed process for the license change:
V3: https://lists.01.org/pipermail/edk2-devel/2019-March/038116.html
V2: https://lists.01.org/pipermail/edk2-devel/2019-March/037669.html
V1: https://lists.01.org/pipermail/edk2-devel/2019-March/037500.html
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The header file declares the AmdSevInstallIoMmuProtocol() function, which
is implemented in "AmdSevIoMmu.c" and called from "IoMmuDxe.c".
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Suggested-by: Michael Kinney <michael.d.kinney@intel.com>
Ref: http://mid.mail-archive.com/E92EE9817A31E24EB0585FDF735412F56327F7D3@ORSMSX113.amr.corp.intel.com
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Register an ExitBootServices() callback that tears down all IOMMU
mappings, without modifying the UEFI memory map.
The trick is that in the ExitBootServices() callback, we don't immediately
do the work; instead we signal another (private) event.
Normally the dispatch order of ExitBootServices() callbacks is unspecified
(within the same task priority level anyway). By queueing another
function, we delay the unmapping until after all PciIo and Virtio drivers
abort -- in their own ExitBootServices() callbacks -- the pending DMA
operations of their respective controllers.
Furthermore, the fact that IoMmuUnmapWorker() rewrites client-owned memory
when it unmaps a Write or CommonBuffer bus master operation, is safe even
in this context. The existence of any given "MapInfo" in "mMapInfos"
implies that the client buffer pointed-to by "MapInfo->CryptedAddress" was
live when ExitBootServices() was entered. And, after entering
ExitBootServices(), nothing must have changed the UEFI memory map, hence
the client buffer at "MapInfo->CryptedAddress" still exists.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
IoMmuUnmapWorker() is identical to IoMmuUnmap(), it just takes an
additional BOOLEAN parameter called "MemoryMapLocked". If the memory map
is locked, IoMmuUnmapWorker() does its usual job, but it purposely leaks
memory rather than freeing it. This makes it callable from
ExitBootServices() context.
Turn IoMmuUnmap() into a thin wrapper around IoMmuUnmapWorker() that
passes constant FALSE for "MemoryMapLocked".
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The "mRecycledMapInfos" list implements an internal pool of unused
MAP_INFO structures between the IoMmuUnmap() and IoMmuMap() functions. The
original goal was to allow IoMmuUnmap() to tear down CommonBuffer mappings
without releasing any memory: IoMmuUnmap() would recycle the MAP_INFO
structure to the list, and IoMmuMap() would always check the list first,
before allocating a brand new MAP_INFO structure.
In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it
unmaps all existent bus master operations (CommonBuffer, Read, Write) at
ExitBootServices(), strictly after the individual device drivers abort
pending DMA on the devices they manage, in their own ExitBootServices()
notification functions.
For this, rename and repurpose the list to track all live mappings.
This means that IoMmuUnmap() will always release a MAP_INFO structure
(even when cleaning up a CommonBuffer operation). That's fine (for now),
because device drivers are no longer expected to call Unmap() in their
ExitBootServices() notification functions.
In theory, we could also move the allocation and freeing of the stash
buffer from IoMmuAllocateBuffer() and IoMmuFreeBuffer(), respectively, to
IoMmuMap() and IoMmuUnmap(). However, this would require allocating and
freeing a stash buffer in *both* IoMmuMap() and IoMmuUnmap(), as
IoMmuMap() performs in-place decryption for CommonBuffer operations, and
IoMmuUnmap() performs in-place encryption for the same.
By keeping the stash buffer allocation as-is, not only do we keep the code
almost fully undisturbed, but
- we also continue to guarantee that IoMmuUnmap() succeeds: allocating a
stash buffer in IoMmuUnmap(), for in-place encryption after a
CommonBuffer operation, could fail;
- we also keep IoMmuUnmap() largely reusable for ExitBootServices()
callback context: allocating a stash buffer in IoMmuUnmap() would simply
be forbidden in that context.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
| |
Log all relevant IN parameters on entry. (There are only IN parameters.)
Beautify the format string.
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Log all relevant IN and IN OUT parameters on entry.
(Note that the HostAddress parameter is IN OUT rather than OUT due to
historical reasons. The "IN EFI_ALLOCATE_TYPE Type" parameter is now to be
ignored, but historically it could be set to AllocateMaxAddress for
example, and for that HostAddress had to be IN OUT.)
When exiting with success, log all relevant OUT parameters (i.e.,
HostAddress). Also log the new (internal) StashBuffer address, on which
IoMmuMap() and IoMmuUnmap() rely on, for BusMasterCommonBuffer operations
(in-place decryption and encryption, respectively).
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The only important external information for this function, and for the
human looking at the log, is the Mapping input parameter. Log it on entry.
Stop logging the contents of the MAP_INFO structure pointed-to by Mapping.
Thanks to the previous patch, we can now associate IoMmuUnmap() messages
with IoMmuMap() messages -- and thereby with MAP_INFO contents -- purely
via Mapping.
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Log all relevant IN and IN OUT parameters on entry.
When exiting with success, log all relevant OUT and IN OUT parameters.
Don't log OUT and IN OUT parameters that are never set or changed after
entering the function (i.e., *NumberOfBytes).
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In order for Unmap() to be callable from ExitBootServices() event handler
context (for cleaning up a BusMasterCommonBuffer[64] operation), we have
to completely liberate the affected path in Unmap() from dynamic memory
management.
The last remaining piece is the release of the MAP_INFO structure. Rather
than freeing it with FreePool(), recycle it to an internal list. Elements
of this "free list" can be reused for any kind of Map() operation, and can
be freed later, or recycled again.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Upon a MemEncryptSevClearPageEncMask() failure in Map(), it wouldn't be
difficult to release the bounce buffer that was implicitly allocated for
BusMasterRead[64] and BusMasterWrite[64] operations. However, undoing any
partial memory encryption mask changes -- partial page splitting and PTE
modifications -- is practically impossible. (For example, restoring the
encryption mask on the entire range has no reason to fare any better than
the MemEncryptSevClearPageEncMask() call itself.)
For this reason, keep ASSERT_EFI_ERROR(), but hang in RELEASE builds too,
if MemEncryptSevClearPageEncMask() or MemEncryptSevSetPageEncMask() fails.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
At the moment, we have the following distribution of actions between the
IOMMU protocol member functions:
- AllocateBuffer() allocates pages and clears the memory encryption mask.
- FreeBuffer() re-sets the memory encryption mask, and deallocates pages.
- Map() does nothing at all when BusMasterCommonBuffer[64] is requested
(and AllocateBuffer() was called previously). Otherwise, Map() allocates
pages, and clears the memory encryption mask.
- Unmap() does nothing when cleaning up a BusMasterCommonBuffer[64]
operation. Otherwise, Unmap() clears the encryption mask, and frees the
pages.
This is wrong: the AllocateBuffer() protocol member is not expected to
produce a buffer that is immediately usable, and client code is required
to call Map() unconditionally, even if BusMasterCommonBuffer[64] is the
desired operation. Implement the right distribution of actions as follows:
- AllocateBuffer() allocates pages and does not touch the encryption mask.
- FreeBuffer() deallocates pages and does not touch the encryption mask.
- Map() does not allocate pages when BusMasterCommonBuffer[64] is
requested, and it allocates pages (bounce buffer) otherwise. Regardless
of the BusMaster operation, Map() (and Map() only) clears the memory
encryption mask.
- Unmap() restores the encryption mask unconditionally. If the operation
was BusMasterCommonBuffer[64], then Unmap() does not release the pages.
Otherwise, the pages (bounce buffer) are released.
This approach also ensures that Unmap() can be called from
ExitBootServices() event handlers, for cleaning up
BusMasterCommonBuffer[64] operations. (More specifically, for restoring
the SEV encryption mask on any in-flight buffers, after resetting any
referring devices.) ExitBootServices() event handlers must not change the
UEFI memory map, thus any memory allocation or freeing in Unmap() would
disqualify Unmap() from being called in such a context.
Map()-ing and Unmap()-ing memory for a BusMasterCommonBuffer[64] operation
effectively means in-place decryption and encryption in a SEV context. As
an additional hurdle, section "7.10.8 Encrypt-in-Place" of AMD publication
Nr.24593 implies that we need a separate temporary buffer for decryption
and encryption that will eventually land in-place. Allocating said
temporary buffer in the straightforward way would violate the above
allocation/freeing restrictions on Map()/Unmap(), therefore pre-allocate
this "stash buffer" too in AllocateBuffer(), and free it in FreeBuffer().
To completely rid Unmap() of dynamic memory impact, for
BusMasterCommonBuffer[64] operations, we're going to rework the lifecycle of
the MAP_INFO structures in a later patch.
(The MemEncryptSevSetPageEncMask() call in Unmap() could theoretically
allocate memory internally for page splitting, however this won't happen
in practice: in Unmap() we only restore the memory encryption mask, and
don't genuinely set it. Any page splitting will have occurred in Map()'s
MemEncryptSevClearPageEncMask() call first.)
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There are three issues with the current calculations:
- The initial logic that sets up "DmaMemoryTop" and "AllocateType" checks
for the BusMasterCommonBuffer64 operation in two places. The inner check
for BusMasterCommonBuffer64 will never evaluate to TRUE however, because
the outer check excludes BusMasterCommonBuffer64.
- In order to lower "DmaMemoryTop" to (SIZE_4GB - 1), the outer check
requires that the encrypted (original) buffer cross the 4GB mark. This
is wrong: for BusMasterRead[64] and BusMasterWrite[64] operations, we
unconditionally need a bounce buffer (a decrypted memory area), and for
the 32-bit variants, "DmaMemoryTop" should be lowered regardless of the
location of the original (encrypted) buffer.
- The current logic would be hard to extend for the in-place decryption
that we'll implement in the next patch.
Therefore rework the "MapInfo->PlainTextAddress" setup. No functional
changes beyond said bugfixes.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Whenever we release the plaintext bounce buffer pages that were allocated
implicitly in Map() for BusMasterRead[64] and BusMasterWrite[64], we
restore the encryption mask on them. However, we should also rewrite the
area (fill it with zeros) so that the hypervisor is not left with a
plaintext view of the earlier data.
Similarly, whenever we release the plaintext common buffer pages that were
allocated explicitly in AllocateBuffer() for BusMasterCommonBuffer[64], we
restore the encryption mask on them. However, we should also rewrite the
area (fill it with zeros) so that the hypervisor is not left with a
plaintext view of the earlier data.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The following library classes are not used by this module, so remove them
from the INF file's [LibraryClasses] section:
- DxeServicesTableLib
- UefiLib
The following library classes are used by this module, so add them to the
INF file's [LibraryClasses] section:
- BaseMemoryLib (e.g. via CopyMem())
- MemoryAllocationLib (e.g. via AllocatePool())
Sort the list of library classes (in both "IoMmuDxe.inf" and
"AmdSevIoMmu.h").
Remove all non-local #include directives from "IoMmuDxe.c"; both C files
of this module include "AmdSevIoMmu.h", and "AmdSevIoMmu.h" includes all
non-local headers already.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If we cannot install the IOMMU protocol for whatever reason, exit the
driver with an error. The same is already done for the IOMMU Absent
protocol.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The edk2 coding style requires separate assignments.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The portable way to print UINTN values is to use the %Lx format specifier,
and to convert the values to UINT64. The second step is currently missing,
add it.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As a continuation of the last patch, clarify that the area pointed-to by
"HostAddress" is encrypted and hidden from the hypervisor.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In this particular IOMMU driver, "DeviceAddress" is just as accessible to
the CPU as "HostAddress", the difference is that the area pointed-to by
the former is plain-text and accessible to the hypervisor. Rename
"DeviceAddress" to "PlainTextAddress" in MAP_INFO.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
No functional changes.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
|
|
|
|
|
|
|
|
| |
Correct the header guard macro
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Thomas Palmer <thomas.palmer@hpe.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
|
|
The IOMMU protocol driver provides capabilities to set a DMA access
attribute and methods to allocate, free, map and unmap the DMA memory
for the PCI Bus devices.
Due to security reasons all DMA operations inside the SEV guest must
be performed on shared (i.e unencrypted) pages. The IOMMU protocol
driver for the SEV guest uses a bounce buffer to map guest DMA buffer
to shared pages inorder to provide the support for DMA operations inside
SEV guest.
IoMmuDxe driver looks for SEV capabilities, if present then it installs
the real IOMMU protocol otherwise it installs placeholder protocol.
Currently, PciHostBridgeDxe and QemuFWCfgLib need to know the existance
of IOMMU protocol. The modules needing to know the existance of IOMMU
support should add
gEdkiiIoMmuProtocolGuid OR gIoMmuAbsentProtocolGuid
in their depex to ensure that platform IOMMU detection has been performed.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leo Duran <leo.duran@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Suggested-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Jordan Justen <jordan.l.justen@intel.com>
|