Since commit 19c6d9feaa ("MdePkg: Expand BaseIoLibIntrinsic (IoLib
class) library", 2017-01-14), IoWriteFifo8() has been widely available to
modules. Use it to print debug messages and assertion failures to the QEMU
debug port, rather than open-coded loops.
In the general case this speeds up logging, because debug messages will
now trap to QEMU once per message (as opposed to once per character), due
to "REP OUTSB" in "MdePkg/Library/BaseIoLibIntrinsic/*/IoFifoSev.nasm".
In SEV guests, there is no speedup (SEV doesn't support the REP prefix).
SEV is detected internally to BaseIoLibIntrinsic.
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>
Acked-by: Brijesh Singh <brijesh.singh@amd.com>
This patch enables UDF file system support by default.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@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>
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.
In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() call from VirtioScsiExitBoot(),
originally added in commit fc2168feb2 ("OvmfPkg/VirtioScsiDxe: map VRING
using VirtioRingMap()", 2017-08-31).
Add a DEBUG message so we can observe the ordering between
VirtioScsiExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.
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: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
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.
In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() call from VirtioRngExitBoot(),
originally added in commit 0a568ccbcb ("OvmfPkg/VirtioRngDxe: map host
address to device address", 2017-08-23).
Add a DEBUG message so we can observe the ordering between
VirtioRngExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.
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: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
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.
In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() calls from VirtioGpuExitBoot(),
originally added in commit 9bc5026c19 ("OvmfPkg/VirtioGpuDxe: map VRING
for bus master common buffer operation", 2017-08-26) and commit
f10ae92366 ("OvmfPkg/VirtioGpuDxe: map backing store to bus master
device address", 2017-08-26).
Add a DEBUG message so we can observe the ordering between
VirtioGpuExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.
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: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
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.
In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() call from VirtioBlkExitBoot(),
originally added in commit 1916513047 ("OvmfPkg/VirtioBlkDxe: map VRING
using VirtioRingMap()", 2017-08-27).
Add a DEBUG message so we can observe the ordering between
VirtioBlkExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.
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: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
In earlier PEI stage, temporary memory at PcdOvmfSecPeiTempRamBase is
employed as stack and heap. We move them to the new room and do some
relocation fixup when permanent memory becomes available.
TemporaryRamMigration() is responsible for switching the stack.
Before entering TemporaryRamMigration(), Ebp/Rbp is populated with the
content of Esp/Rsp and used as frame pointer.
After the execution of SetJump/LongJump, stack migrates to new position
while the context keeps unchanged.
But when TemporaryRamMigration() exits, Esp/Rsp is filled with
the content of Ebp/Rbp to destroy this stack frame.
The result is, stack switches back to previous temporary memory.
When permanent memory becomes available, modules that have registered
themselves for shadowing will be scheduled to execute. Some of them
need to consume more memory(heap/stack). Contrast to temporary stack,
permanent stack possesses larger space.
The potential risk is overflowing the stack if stack staying in
temporary memory. When it happens, system may crash during S3 resume.
More detailed information:
> (gdb) disassemble /r
> Dump of assembler code for function TemporaryRamMigration:
> 0x00000000fffcd29c <+0>: 55 push %rbp
> 0x00000000fffcd29d <+1>: 48 89 e5 mov %rsp,%rbp
> 0x00000000fffcd2a0 <+4>: 48 81 ec 70 01 00 00 sub
> $0x170,%rsp
> ...
> ...
> 0x00000000fffcd425 <+393>: e8 80 10 00 00 callq 0xfffce4aa
> <SaveAndSetDebugTimerInterrupt>
> => 0x00000000fffcd42a <+398>: b8 00 00 00 00 mov $0x0,%eax
> 0x00000000fffcd42f <+403>: c9 leaveq
> 0x00000000fffcd430 <+404>: c3 retq
> End of assembler dump.
See the description of leave(opcode: c9), from
Intel 64 and IA-32 Architectures Software Developer's Manual, Volume 2A
"Releases the stack frame set up by an earlier ENTER instruction. The
LEAVE instruction copies the frame pointer (in the EBP register) into
the stack pointer register (ESP), which releases the stack space
allocated to the stack frame. The old frame pointer (the frame pointer
for the calling procedure that was saved by the ENTER instruction) is
then popped from the stack into the EBP register, restoring the calling
procedure’s stack frame."
To solve this, update Ebp/Rbp too when Esp/Rsp is updated
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ge Song <ge.song@hxt-semitech.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
VirtioGpuDxe is now IOMMU-clean; it translates system memory addresses to
bus master device addresses. Negotiate VIRTIO_F_IOMMU_PLATFORM in parallel
with VIRTIO_F_VERSION_1. (Note: the VirtIo GPU device, and this driver,
are virtio-1.0 only (a.k.a. "modern-only").)
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.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
VirtioGpuDxe is a UEFI Bus driver (not a Device driver). This is because a
UEFI graphics driver is expected to produce its GraphicsOutput protocol
instance(s) on new child handle(s) of the video controller handle, one
child handle (plus GOP) per video output (or, one child handle plus GOP
per combination of multiple video outputs).
In VirtioGpuDxe, we support a single VirtIo GPU head (scanout), namely
head#0. This means that, with regard to a specific VirtIo GPU device, the
driver may be in one of three states, at any time:
[1] VirtioGpuDxe has not bound the device at all,
[2] VirtioGpuDxe has bound the device, but not produced the sole child
handle for head#0,
[3] VirtioGpuDxe has bound the device, and produced the sole child handle
for head#0, with a GOP instance on the child handle.
(Which state the driver is in wrt. a given VirtIo GPU device depends on
the VirtioGpuDriverBindingStart() / VirtioGpuDriverBindingStop()
invocations issued by the ConnectController() / DisconnectController()
boot services. In turn those come from BDS or e.g. the UEFI shell.)
The concept of "current video mode" is technically tied to the GOP (i.e.,
the child handle, state [3] only), not the VirtIo GPU controller handle.
This is why we manage the storage that backs the current video mode in our
EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode() member implementation.
GopSetMode() is first called *internally*, when we enter state [3] (that
is, when we produce the child handle + GOP on it):
VirtioGpuDriverBindingStart() [DriverBinding.c]
InitVgpuGop() [DriverBinding.c]
VgpuGop->Gop.SetMode() [Gop.c]
When this happens, we allocate the backing store *without* having a
preexistent backing store (due to no preexistent video mode and GOP).
Skipping VirtIo GPU details not relevant for this patch, we just note that
the backing store is exposed *permanently* to the VirtIo GPU device, with
the RESOURCE_ATTACH_BACKING command.
When external clients call the EFI_GRAPHICS_OUTPUT_PROTOCOL.Blt() member
function -- called GopBlt() in this driver --, in state [3], the function
operates on the backing store, and sends only small messages to the VirtIo
GPU device.
When external clients call GopSetMode() for switching between video modes
-- in state [3] --, then
- a new backing store is allocated and exposed to the device (attached to
a new host-side VirtIo GPU resource),
- head#0 is flipped to the new backing store,
- on success, the ReleaseGopResources() function both detaches the
previous backing store from the VirtIo GPU device, an releases it. The
new backing store address and size are saved in our GOP object. (In
other words, we "commit" to the new video mode.)
When the DisconnectController() boot service asks us to leave state [3] --
we can leave it directly only for state [2] --, then the
ReleaseGopResources() function is called on a different path:
VirtioGpuDriverBindingStop() [DriverBinding.c]
UninitVgpuGop() [DriverBinding.c]
ReleaseGopResources() [Gop.c]
In this case, the backing store being released is still in use (we're not
leaving it for a new mode -- head#0 has not been flipped "away" from it),
so in ReleaseGopResources() we disable head#0 first.
(The ReleaseGopResources() function is called the same way on the error
path in InitVgpuGop(), if the first -- internal -- VgpuGop->Gop.SetMode()
call succeeds, but the rest of InitVgpuGop() fails.)
Based on the above, for IOMMU-compatibility,
- in GopSetMode(), don't just allocate, but also map the backing store of
the nascent video mode to a device address, for bus master common buffer
operation,
- (the VirtioGpuAllocateZeroAndMapBackingStore() helper function
introduced in the last patch takes care of zeroing internally,)
- pass the device address to the VirtIo GPU device in the
RESOURCE_ATTACH_BACKING command,
- if GopSetMode() succeeds, save the mapping token,
- if GopSetMode() fails, don't just free but also unmap the still-born
backing store,
- in ReleaseGopResources(), don't just free but also unmap the backing
store -- which is the previous backing store if we're mode-switching,
and the current backing store if we're leaving state [3].
Finally, ExitBootServices() may be called when the driver is in either
state [1], [2] or [3], wrt. a given VirtIo GPU device. (Of course we are
only notified in states [2] and [3].) If we get the notification in state
[3], then the current video mode's backing store has to be unmapped, but
not released. (We must not change the UEFI memory map.)
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.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Introduce the VirtioGpuAllocateZeroAndMapBackingStore() and
VirtioGpuUnmapAndFreeBackingStore() helper functions. These functions tie
together the allocation, zeroing and mapping, and unmapping and
deallocation, respectively, of memory that the virtio GPU will permanently
reference after receiving the RESOURCE_ATTACH_BACKING command.
With these functions we can keep the next patch simpler -- the GOP
implementation in "Gop.c" retains its error handling structure, and
remains oblivious to VIRTIO_DEVICE_PROTOCOL and VirtioLib.
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.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
The RESOURCE_ATTACH_BACKING virtio GPU command assigns guest-side backing
pages to a host-side resource that was created earlier with the
RESOURCE_CREATE_2D command.
We compose the RESOURCE_ATTACH_BACKING command in the
VirtioGpuResourceAttachBacking() function. Currently this function takes
the parameter
IN VOID *FirstBackingPage
This is only appropriate as long as we pass a (guest-phys) system memory
address to the device. In preparation for a mapped bus master device
address, change the above parameter to
IN EFI_PHYSICAL_ADDRESS BackingStoreDeviceAddress
In order to keep the current call site functional, move the (VOID*) to
(UINTN) conversion out of the function, to the call site.
The "Request.Entry.Addr" field already has type UINT64.
This patch is similar to commit 4b725858de ("OvmfPkg/VirtioLib: change
the parameter of VirtioAppendDesc() to UINT64", 2017-08-23).
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.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Every virtio GPU command used by VirtioGpuDxe is synchronous and formatted
as a two-descriptor chain: request, response. The internal workhorse
function that all the command-specific functions call for such messaging
is VirtioGpuSendCommand().
In VirtioGpuSendCommand(), map the request from system memory to bus
master device address for BusMasterRead operation, and map the response
from system memory to bus master device address for BusMasterWrite
operation.
Pass the bus master device addresses to VirtioAppendDesc(). (See also
commit 4b725858de, "OvmfPkg/VirtioLib: change the parameter of
VirtioAppendDesc() to UINT64", 2017-08-23.)
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.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
VirtioGpuDxe uses one virtio ring, for VIRTIO_GPU_CONTROL_QUEUE.
Map it for bus master common buffer operation with VirtioRingMap(), so
that it can be accessed equally by both guest and hypervisor even if an
IOMMU is used. (VirtioRingInit() already allocates the ring suitably for
this, see commit b0338c5329, "OvmfPkg/VirtioLib: alloc VRING buffer with
AllocateSharedPages()", 2017-08-23).
Pass the resultant translation offset ("RingBaseShift"), from system
memory address to bus master device address, to VIRTIO_SET_QUEUE_ADDRESS.
Unmap the ring in all contexts where the ring becomes unused (these
contexts are mutually exclusive):
- in VirtioGpuInit(): the ring has been mapped, but we cannot complete the
virtio initialization for another reason,
- in VirtioGpuUninit(): the virtio initialization has succeeded, but
VirtioGpuDriverBindingStart() fails for another reason, or
VirtioGpuDriverBindingStop() unbinds the device after use,
- in VirtioGpuExitBoot(): ExitBootServices() is called after
VirtioGpuDriverBindingStart() has successfully bound the device.
(Unmapping the ring does not change the UEFI memory map.)
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.1
Signed-off-by: Laszlo Ersek <lersek@redhat.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>
Debug messages that start as natural (English) language phrases (after the
debug prefix) should uniformly begin with lower-case or upper-case. In
SetMemoryEncDec() we have a mixture now. Stick with lower-case.
(Upper-case is better for full sentences that also end with punctuation.)
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 SetMemoryEncDec(), we have four locations where we (a) log a message on
the DEBUG_WARN level that says "ERROR", (b) return the status code
RETURN_NO_MAPPING right after.
These messages clearly describe actual errors (bad PML4, PDPE, PDE, PTE).
Promote their debug levels to DEBUG_ERROR, and remove the word "ERROR"
from the messages.
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 the SetMemoryEncDec() function, the way we currently report
PhysicalAddress is not uniform:
- mostly we say "for %lx",
- in one spot we say "at %lx" (even though the 2MB page being split does
not live *at* PhysicalAddress, instead it maps PhysicalAddress),
- in another spot we don't log PhysicalAddress at all (when splitting a
1GB page).
Unify this, using the format string "for Physical=0x%Lx".
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>
None of the DEBUG macro invocations in SetMemoryEncDec() fit on a single
line. Break them to multiple lines, for (a) conforming to the coding style
spec, (b) easier modification in later patches.
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>
VirtioScsiDxe driver has been updated to use IOMMU-like member functions
from VIRTIO_DEVICE_PROTOCOL to translate the system physical address to
device address. We do not need to do anything special when
VIRTIO_F_IOMMU_PLATFORM bit is present hence treat it in parallel with
VIRTIO_F_VERSION_1.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
When device is behind the IOMMU, driver is require to pass the device
address of virtio request, response and any memory referenced by those
request/response to the bus master.
The patch uses IOMMU-like member functions from VIRTIO_DEVICE_PROTOCOL to
map request and response buffers system physical address to the device
address.
- If the buffer need to be accessed by both the processor and a bus
master then map with BusMasterCommonBuffer.
- If the buffer need to be accessed for a write operation by a bus master
then map with BusMasterWrite.
However, after a BusMasterWrite Unmap() failure, error reporting via
EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET would be very complex,
therefore we map such buffers too with BusMasterCommonBuffer.
- If the buffer need to be accessed for a read operation by a bus master
then map with BusMasterRead.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[lersek@redhat.com: restore lost sentence/paragraph in commit message]
[lersek@redhat.com: reindent/reflow "InDataBuffer" comment block]
[lersek@redhat.com: cast arg, not result, of EFI_SIZE_TO_PAGES() to UINTN]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
When virtio request fails we return EFI_DEVICE_ERROR, as per the spec
EFI_EXT_SCSI_PASS_THRU_PROTOCOL.PassThru() member function is required
to implement elaborated error reporting.
The patch refactors out entire block of the code that creates the host
adapter error into a separate helper function (ReportHostAdapterError).
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[lersek@redhat.com: fix style & typo in ReportHostAdapterError() comment]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
When device is behind the IOMMU then driver need to pass the device
address when programing the bus master. The patch uses VirtioRingMap() to
map the VRING system physical address to device address.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
when "RequestIsWrite" is FALSE -- i.e., the CPU wants data from
the device, we map "Buffer" for VirtioOperationBusMasterWrite. In
this case, checking the return status of
Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, BufferMapping);
is must. If the unmapping fails, then "Buffer" will not contain the
actual data from the device, and we must fail the request with
EFI_DEVICE_ERROR.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[lersek@redhat.com: fix typos in subject]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
There's a small window between
- AllocFwCfgDmaAccessBuffer() mapping the new FW_CFG_DMA_ACCESS object for
common buffer operation (i.e., decrypting it), and
- InternalQemuFwCfgDmaBytes() setting the fields of the object.
In this window, earlier garbage in the object is "leaked" to the
hypervisor. So zero the object before we decrypt it.
(This commit message references AMD SEV directly, because QemuFwCfgDxeLib
is not *generally* enabled for IOMMU operation just yet, unlike our goal
for the virtio infrastructure. Instead, QemuFwCfgDxeLib uses
MemEncryptSevLib explicitly to detect SEV, and then relies on IOMMU
protocol behavior that is specific to SEV. At this point, this is by
design.)
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.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
VirtioBlkDxe driver has been updated to use IOMMU-like member functions
from VIRTIO_DEVICE_PROTOCOL to translate the system physical address to
device address. We do not need to do anything special when
VIRTIO_F_IOMMU_PLATFORM bit is present hence treat it in parallel with
VIRTIO_F_VERSION_1.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
When device is behind the IOMMU, driver is require to pass the device
address of virtio request, response and any memory referenced by those
request/response to the bus master.
The patch uses IOMMU-like member functions from VIRTIO_DEVICE_PROTOCOL to
map request and response buffers system physical address to the device
address.
- If the buffer need to be accessed by both the processor and a bus
master then map with BusMasterCommonBuffer.
- If the buffer need to be accessed for a write operation by a bus master
then map with BusMasterWrite.
- If the buffer need to be accessed for a read operation by a bus master
then map with BusMasterRead.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
When device is behind the IOMMU then driver need to pass the device
address when programing the bus master. The patch uses VirtioRingMap() to
map the VRING system physical address to device address.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
This reverts commit ca56256d5e0b7e63325b049e90a6bd03f90e3598:
TianoCore BZ#671 <https://bugzilla.tianocore.org/show_bug.cgi?id=671> has
been fixed in commit 2f7f1e73c1 ("BaseTools: Add the missing -pie link
option in GCC tool chain", 2017-08-23), so we can return to the GCC5
toolchain with gcc-7.*.
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: Jordan Justen <jordan.l.justen@intel.com>
VirtioRngDxe driver has been updated to use IOMMU-like member functions
from VIRTIO_DEVICE_PROTOCOL to translate the system physical address to
device address. We do not need to do anything special when
VIRTIO_F_IOMMU_PLATFORM bit is present hence treat it in parallel with
VIRTIO_F_VERSION_1.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
This feature indicates that the device is behind an IOMMU that translates
bus addresses from the device into physical addresses in memory. If this
feature bit is set to 0, then the device emits physical addresses which
are not translated further, even though an IOMMU may be present.
see [1] for more infromation
[1] https://lists.oasis-open.org/archives/virtio-dev/201610/msg00121.html
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
patch maps the host address to a device address for buffers (including
rings, device specifc request and response pointed by vring descriptor,
and any further memory reference by those request and response).
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[lersek@redhat.com: return EFI_DEVICE_ERROR if mapping fails in GetRNG]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
The patch change the "BufferPhysAddr" parameter of VirtioAppendDesc()
from type UINTN to UINT64.
UINTN is appropriate as long as we pass system memory references. After
the introduction of bus master device addresses, that's no longer the case
in general. Should we implement "real" IOMMU support at some point, UINTN
could break in 32-bit builds of OVMF.
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[lersek@redhat.com: clarify commit message]
[lersek@redhat.com: balance parens in VirtioAppendDesc() comment blocks]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
The VRING buffer is a communication area between guest and hypervisor.
Allocate it using VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages() so that
it can be mapped later with VirtioRingMap() for bi-directional access.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[lersek@redhat.com: correct typo in VirtioRingInit() comment blocks]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Add a function to map the ring buffer with BusMasterCommonBuffer so that
ring can be accessed by both guest and hypervisor.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[lersek@redhat.com: fix typo in commit message]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
virtio drivers use VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer() to map the
ring buffer host address to a device address. If an IOMMU is present then
RingBaseShift contains the offset from the host address.
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
For the case when an IOMMU is used for translating system physical
addresses to DMA bus master addresses, the transport-independent
virtio device drivers will be required to map their VRING areas to
bus addresses with VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer() calls.
- MMIO and legacy virtio transport do not support IOMMU to translate the
addresses hence RingBaseShift will always be set to zero.
- modern virtio transport supports IOMMU to translate the address, in
next patch we will update the Virtio10Dxe to use RingBaseShift offset.
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[lersek@redhat.com: remove commit msg paragraph with VirtioLib reference]
[lersek@redhat.com: fix typo in VIRTIO_SET_QUEUE_ADDRESS comment block]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
The function can be used for mapping the system physical address to virtio
device address using VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer (). The
function helps with centralizing error handling, and it allows the caller
to pass in constant or other evaluated expressions for NumberOfBytes.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[lersek@redhat.com: s/This/VirtIo/ in the new function's comment blocks]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
The patch extends VIRTIO_DEVICE_PROTOCOL to provide the following new
member functions:
- AllocateSharedPages : allocate a memory region suitable for sharing
between guest and hypervisor (e.g ring buffer).
- FreeSharedPages: free the memory allocated using AllocateSharedPages ().
- MapSharedBuffer: map a host address to device address suitable to share
with device for bus master operations.
- UnmapSharedBuffer: unmap the device address obtained through the
MapSharedBuffer().
We're free to extend the protocol structure without changing the protocol
GUID, or bumping any protocol version fields (of which we currently have
none), because VIRTIO_DEVICE_PROTOCOL is internal to edk2 by design --
see the disclaimers in "VirtioDevice.h".
The patch implements Laszlo's recommendation [1].
[1] http://mid.mail-archive.com/841bec5f-6f6e-8b1f-25ba-0fd37a915b72@redhat.com
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Now that we have dropped QemuVideoDxe from all QEMU targeted builds
under ArmVirtPkg, we can revert the ARM specific changes to it.
This partially reverts commits 84a75f70e9 (SVN 16890) and
05a5379458.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
When UefiCpuPkg/MpInitLib is built for X64 with gcc-7, using the DEBUG
build target and the GCC5 toolchain settings, a C-language assignment is
miscompiled such that the initial AP startup hangs in CpuMpPei (X64) or
CpuDxe (Ia32X64). See <https://bugzilla.tianocore.org/show_bug.cgi?id=671>
for a detailed analysis of the symptoms, and for mailing list links.
This issue has been reported several times (one example is
<https://bugzilla.tianocore.org/show_bug.cgi?id=657>). Until we (or the
upstream gcc developers) figure out how to dissuade gcc-7 from the
miscompilation, pick the GCC49 toolchain in "build.sh" for gcc-7.*.
Cc: Alex Williamson <alex.williamson@redhat.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: Jordan Justen <jordan.l.justen@intel.com>
In commit 8057622527 ("OvmfPkg/AcpiPlatformDxe: save fw_cfg boot script
with QemuFwCfgS3Lib", 2017-02-23), we replaced the explicit S3 boot script
manipulation in TransferS3ContextToBootScript() with a call to
QemuFwCfgS3CallWhenBootScriptReady(). (Passing AppendFwCfgBootScript() as
callback.)
QemuFwCfgS3CallWhenBootScriptReady() checks for fw_cfg DMA up-front, and
bails with RETURN_NOT_FOUND if fw_cfg DMA is missing.
(This is justified as the goal of QemuFwCfgS3Lib is to "enable[] driver
modules [...] to produce fw_cfg DMA operations that are to be replayed at
S3 resume time".)
In turn, if QemuFwCfgS3CallWhenBootScriptReady() fails, then
OvmfPkg/AcpiPlatformDxe rolls back any earlier linker/loader script
processing, and falls back to the built-in ACPI tables.
(This is also justified because failure to save WRITE_POINTER commands for
replaying at S3 resume implies failure to process the linker/loader script
comprehensively.)
Calling QemuFwCfgS3CallWhenBootScriptReady() from
TransferS3ContextToBootScript() *unconditionally* is wrong however. For
the case when the linker/loader script contains no WRITE_POINTER commands,
the call perpetuated an earlier side effect, and introduced another one:
(1) On machine types that provide fw_cfg DMA (i.e., 2.5+),
QemuFwCfgS3CallWhenBootScriptReady() would succeed, and allocate
workspace for the boot script opcodes in reserved memory. However, no
opcodes would actually be produced in the AppendFwCfgBootScript()
callback, due to lack of any WRITE_POINTER commands.
This waste of reserved memory had been introduced in earlier commit
df73df138d ("OvmfPkg/AcpiPlatformDxe: replay
QEMU_LOADER_WRITE_POINTER commands at S3", 2017-02-09).
(2) On machine types that lack fw_cfg DMA (i.e., 2.4 and earlier),
TransferS3ContextToBootScript() would now fail the linker/loader
script for no reason.
(Note that QEMU itself prevents adding devices that depend on
WRITE_POINTER if the machine type lacks fw_cfg DMA:
$ qemu-system-x86_64 -M pc-q35-2.4 -device vmgenid
qemu-system-x86_64: -device vmgenid: vmgenid requires DMA write
support in fw_cfg, which this machine type does not provide)
Short-circuit an empty S3_CONTEXT in TransferS3ContextToBootScript() by
dropping S3_CONTEXT on the floor. This is compatible with the current
contract of the function as it constitutes a transfer of ownership.
Regression (2) was found and reported by Dhiru Kholia as an OSX guest boot
failure on the "pc-q35-2.4" machine type:
http://mid.mail-archive.com/CANO7a6x6EaWNZ8y=MvLU=w_LjRLXserO3NmsgHvaYE0aUCCWzg@mail.gmail.com
Dhiru bisected the issue to commit 8057622527.
Cc: Dhiru Kholia <dhiru.kholia@gmail.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Fixes: df73df138d
Fixes: 8057622527
Reported-by: Dhiru Kholia <dhiru.kholia@gmail.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Dhiru Kholia <dhiru.kholia@gmail.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
In OVMF we currently get the upper (>=4GB) memory size with the
GetSystemMemorySizeAbove4gb() function.
The GetSystemMemorySizeAbove4gb() function is used in two places:
(1) It is the starting point of the calculations in GetFirstNonAddress().
GetFirstNonAddress() in turn
- determines the placement of the 64-bit PCI MMIO aperture,
- provides input for the GCD memory space map's sizing (see
AddressWidthInitialization(), and the CPU HOB in
MiscInitialization()),
- influences the permanent PEI RAM cap (the DXE core's page tables,
built in permanent PEI RAM, grow as the RAM to map grows).
(2) In QemuInitializeRam(), GetSystemMemorySizeAbove4gb() determines the
single memory descriptor HOB that we produce for the upper memory.
Respectively, there are two problems with GetSystemMemorySizeAbove4gb():
(1) It reads a 24-bit count of 64KB RAM chunks from the CMOS, and
therefore cannot return a larger value than one terabyte.
(2) It cannot express discontiguous high RAM.
Starting with version 1.7.0, QEMU has provided the fw_cfg file called
"etc/e820". Refer to the following QEMU commits:
- 0624c7f916b4 ("e820: pass high memory too.", 2013-10-10),
- 7d67110f2d9a ("pc: add etc/e820 fw_cfg file", 2013-10-18)
- 7db16f2480db ("pc: register e820 entries for ram", 2013-10-10)
Ever since these commits in v1.7.0 -- with the last QEMU release being
v2.9.0, and v2.10.0 under development --, the only two RAM entries added
to this E820 map correspond to the below-4GB RAM range, and the above-4GB
RAM range. And, the above-4GB range exactly matches the CMOS registers in
question; see the use of "pcms->above_4g_mem_size":
pc_q35_init() | pc_init1()
pc_memory_init()
e820_add_entry(0x100000000ULL, pcms->above_4g_mem_size, E820_RAM);
pc_cmos_init()
val = pcms->above_4g_mem_size / 65536;
rtc_set_memory(s, 0x5b, val);
rtc_set_memory(s, 0x5c, val >> 8);
rtc_set_memory(s, 0x5d, val >> 16);
Therefore, remedy the above OVMF limitations as follows:
(1) Start off GetFirstNonAddress() by scanning the E820 map for the
highest exclusive >=4GB RAM address. Fall back to the CMOS if the E820
map is unavailable. Base all further calculations (such as 64-bit PCI
MMIO aperture placement, GCD sizing etc) on this value.
At the moment, the only difference this change makes is that we can
have more than 1TB above 4GB -- given that the sole "high RAM" entry
in the E820 map matches the CMOS exactly, modulo the most significant
bits (see above).
However, Igor plans to add discontiguous (cold-plugged) high RAM to
the fw_cfg E820 RAM map later on, and then this scanning will adapt
automatically.
(2) In QemuInitializeRam(), describe the high RAM regions from the E820
map one by one with memory HOBs. Fall back to the CMOS only if the
E820 map is missing.
Again, right now this change only makes a difference if there is at
least 1TB high RAM. Later on it will adapt to discontiguous high RAM
(regardless of its size) automatically.
-*-
Implementation details: introduce the ScanOrAdd64BitE820Ram() function,
which reads the E820 entries from fw_cfg, and finds the highest exclusive
>=4GB RAM address, or produces memory resource descriptor HOBs for RAM
entries that start at or above 4GB. The RAM map is not read in a single
go, because its size can vary, and in PlatformPei we should stay away from
dynamic memory allocation, for the following reasons:
- "Pool" allocations are limited to ~64KB, are served from HOBs, and
cannot be released ever.
- "Page" allocations are seriously limited before PlatformPei installs the
permanent PEI RAM. Furthermore, page allocations can only be released in
DXE, with dedicated code (so the address would have to be passed on with
a HOB or PCD).
- Raw memory allocation HOBs would require the same freeing in DXE.
Therefore we process each E820 entry as soon as it is read from fw_cfg.
-*-
Considering the impact of high RAM on the DXE core:
A few years ago, installing high RAM as *tested* would cause the DXE core
to inhabit such ranges rather than carving out its home from the permanent
PEI RAM. Fortunately, this was fixed in the following edk2 commit:
3a05b13106, "MdeModulePkg DxeCore: Take the range in resource HOB for
PHIT as higher priority", 2015-09-18
which I regression-tested at the time:
http://mid.mail-archive.com/55FC27B0.4070807@redhat.com
Later on, OVMF was changed to install its high RAM as tested (effectively
"arming" the earlier DXE core change for OVMF), in the following edk2
commit:
035ce3b37c, "OvmfPkg/PlatformPei: Add memory above 4GB as tested",
2016-04-21
which I also regression-tested at the time:
http://mid.mail-archive.com/571E8B90.1020102@redhat.com
Therefore adding more "tested memory" HOBs is safe.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1468526
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Commit 09719a01b1 (OvmfPkg/QemuFwCfgLib: Implement SEV internal function
for Dxe phase) uses IOMMU protocol to allocate and free FW_CFG_DMA_ACCESS
buffer when SEV is active. During initial commits we made assumption that
IOMMU.AllocateBuffer() will provide PlainTextAddress (i.e C-bit cleared).
This assumption was wrong, the AllocateBuffer() protocol member is not
expected to produce a buffer that is immediatly usable, and client is
required to call Map() uncondtionally with BusMasterCommonBuffer[64] to
get a mapping which is accessable by both host and device.
The patch refactors code a bit and add the support to Map()
FW_CFG_DMA_ACCESS buffer using BusMasterCommonBuffer operation after
allocation and Unamp() before free.
The complete discussion about this and recommendation from Laszlo can be
found here [1]
[1] https://lists.01.org/pipermail/edk2-devel/2017-July/012652.html
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[lersek@redhat.com: convert pointers to UINTN before converting to UINT64]
[lersek@redhat.com: fix argument indentation in multi-line function call]
[lersek@redhat.com: explicitly compare pointers to NULL]
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.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 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>
Undo removal of OvmfPkg/License.txt in commit
2a98de0344
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Andrew Fish <afish@apple.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
https://bugzilla.tianocore.org/show_bug.cgi?id=642
Add top level License.txt file with the BSD 2-Clause
License that is used by the majority of the EKD II open
source project content. Merge copyright statements
from the BSD 2-Clause License files in each package
directory and remove the duplication License.txt
file from package directories.
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Andrew Fish <afish@apple.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
https://bugzilla.tianocore.org/show_bug.cgi?id=629
Move Contributions.txt that contains the TianoCore
Contribution Agreement 1.0 to the root of the edk2
repository and remove the duplicate Contributions.txt
files from all packages.
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Andrew Fish <afish@apple.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
NumPages variable was introduced in commit 66c548be50. In this commit
we allocate an intermediate buffer when SEV is enabled. The 'BounceBuffer'
variable points to the intermediate buffer pointer and NumPages variables
stores the number of pages. Later in the code, 'BounceBuffer' variable is
checked to see if we need to free the intermediate buffers. The code looks
correct, suppress the warning.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reported-by: Gerd Hoffmann <kraxel@redhat.com>
Reported-by: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: s/warnigns/warnings/ in the code comment]
[lersek@redhat.com: add Gerd's Reported-by]
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
This patch enables PciHostBridgeDxe driver to use Platform IoMMU detection
library to ensure that PciHostBridgeDxe is run after platform IoMmuDxe
driver has checked whether platform need to install IOMMU protocol provider.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
When SEV is enabled, use a bounce buffer to perform the DMA operation.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Update InternalQemuFwCfgDmaBytes() to work with DMA Access pointer.
The change provides the flexibility to dynamically allocate the "Access"
when SEV is enabled.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
When SEV is enabled, the DMA must be performed on unencrypted pages.
So when get asked to perfom FWCFG DMA read or write, we allocate a
intermediate (bounce buffer) unencrypted buffer and use this buffer
for DMA read or write.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Add SEV specific internal functions which will be used while intergrating
the SEV support into QemuFwCfgLib.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Current QemuFwCfgLib.inf is used in both Pei and Dxe phases. Add Pei
and Dxe inf file to provide a seperate QemuFwCfgLib instances for Pei
and Dxe phases.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.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>
Add the shorter-term library instance outlined in the previous patch to
OvmfPkg, so that we can imbue PciHostBridgeDxe with a protocol dependency
on gEdkiiIoMmuProtocolGuid OR gIoMmuAbsentProtocolGuid.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Platforms that optionally provide an IOMMU protocol should do so by
including a DXE driver (usually called IoMmuDxe) that produces either
the IOMMU protocol -- if the underlying capabilities are available --,
or gIoMmuAbsentProtocolGuid, to signal that the IOMMU capability
detection completed with negative result (i.e., no IOMMU will be
available in the system).
In turn, DXE drivers (and library instances) that are supposed to use
the IOMMU protocol if it is available should add the following to
their DEPEX:
gEdkiiIoMmuProtocolGuid OR gIoMmuAbsentProtocolGuid
This ensures these client modules will only be dispatched after IOMMU
detection completes (with positive or negative result).
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leo Duran <leo.duran@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Suggested-by: Jordan Justen <jordan.l.justen@intel.com>
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
When SEV is enabled, the MMIO memory range must be mapped as unencrypted
(i.e C-bit cleared).
We need to clear the C-bit for MMIO GCD entries in order to cover the
ranges that were added during the PEI phase (through memory resource
descriptor HOBs). Additionally, the NonExistent ranges are processed
in order to cover, in advance, MMIO ranges added later in the DXE phase
by various device drivers, via the appropriate DXE memory space services.
The approach is not transparent for later addition of system memory ranges
to the GCD memory space map. (Such ranges should be encrypted.) OVMF does
not do such a thing at the moment, so this approach should be OK.
The driver is being added to the APRIORI DXE file so that, we clear the
C-bit from MMIO regions before any driver accesses it.
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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Jordan Justen <jordan.l.justen@intel.com>
Secure Encrypted Virtualization (SEV) guest VMs have the concept of
private and shared memory. Private memory is encrypted with the
guest-specific key, while shared memory may be encrypted with hypervisor
key. Certain types of memory (namely instruction pages and guest page
tables) are always treated as private memory by the hardware.
For data memory, SEV guest VMs can choose which pages they would like
to be private. The choice is done using the standard CPU page tables
using the C-bit. When building the initial page table we mark all the
memory as private.
The patch sets the memory encryption PCD. The PCD is consumed by the
following edk2 modules, which manipulate page tables:
- PEI phase modules: CapsulePei, DxeIplPeim, S3Resume2Pei.
CapsulePei is not used by OVMF. DxeIplPeim consumes the PCD at the
end of the PEI phase, when it builds the initial page tables for the
DXE core / DXE phase. S3Resume2Pei does not consume the PCD in its
entry point function, only when DxeIplPeim branches to the S3 resume
path at the end of the PEI phase, and calls S3Resume2Pei's
EFI_PEI_S3_RESUME2_PPI.S3RestoreConfig2() member function.
Therefore it is safe to set the PCD for these modules in PlatformPei.
- DXE phase modules: BootScriptExecutorDxe, CpuDxe, PiSmmCpuDxeSmm.
They are all dispatched after the PEI phase, so setting the PCD for
them in PlatformPei is safe. (BootScriptExecutorDxe is launched "for
real" in the PEI phase during S3 resume, but it caches the PCD into a
static variable when its entry point is originally invoked in DXE.)
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Add Secure Encrypted Virtualization (SEV) helper library.
The library provides the routines to:
- set or clear memory encryption bit for a given memory region.
- query whether SEV is enabled.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Jordan Justen <jordan.l.justen@intel.com>
When SEV is enabled then we must unroll the rep String I/O instructions.
The patch updates dsc file to use SEV version of IoLib inf. The main
difference between BaseIoLibIntrinsic.inf and BaseIoLibIntrinsicSev.inf
is, SEV version checks if its running under SEV enabled guest, If so
then it unroll the String I/O (REP INS/OUTS) otherwise fallbacks to
rep ins/outs.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
SEV guest VMs have the concept of private and shared memory. Private
memory is encrypted with the guest-specific key, while shared memory
may be encrypted with hypervisor key. Certain types of memory (namely
instruction pages and guest page tables) are always treated as private
memory by the hardware. The C-bit in PTE indicate whether the page is
private or shared. The C-bit position for the PTE can be obtained from
CPUID Fn8000_001F[EBX].
When SEV is active, the BIOS is encrypted by the Qemu launch sequence,
we must set the C-bit when building the page table.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
PlatformPei can now overwrite PcdQ35TsegMbytes; document this in
"OvmfPkg/OvmfPkg.dec".
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Recognize an extended TSEG when available in
Q35TsegMbytesInitialization(), and set both PcdQ35TsegMbytes (for
OvmfPkg/SmmAccess) and "mQ35TsegMbytes" (for PlatformPei's own use)
accordingly. The new logic interfaces with the QEMU feature added in QEMU
commit 2f295167e0c4 ("q35/mch: implement extended TSEG sizes",
2017-06-08).
At this point we have to explicitly restrict Q35TsegMbytesInitialization()
to the Q35 board, but that's OK, because Q35TsegMbytesInitialization() is
only called when PcdSmmSmramRequire is set, and for that Q35 is already an
enforced requirement.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Suggested-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
In SmmAccessPeiEntryPoint(), map TSEG megabyte counts different from 1, 2
and 8 to the MCH_ESMRAMC_TSEG_EXT bit pattern (introduced in the previous
patch), for the ESMRAMC.TSEG_SZ bit-field register. (Suggested by Jordan.)
In SmramAccessGetCapabilities() -- backing both
PEI_SMM_ACCESS_PPI.GetCapabilities() and
EFI_SMM_ACCESS2_PROTOCOL.GetCapabilities() --, map the
MCH_ESMRAMC_TSEG_EXT bit pattern found in the ESMRAMC.TSEG_SZ bit-field
register to a byte count of (mQ35TsegMbytes * SIZE_1MB).
(MCH_ESMRAMC_TSEG_EXT is the only possible pattern if none of
MCH_ESMRAMC_TSEG_1MB, MCH_ESMRAMC_TSEG_2MB, and MCH_ESMRAMC_TSEG_8MB
match.)
The new code paths are not exercised just yet; for that, PlatformPei is
going to have to set PcdQ35TsegMbytes (and consequently, SmramInternal's
"mQ35TsegMbytes") to a value different from 1, 2, and 8.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
We can now make PcdQ35TsegMbytes dynamic, in preparation for the extended
TSEG size feature. At the moment we only move the declaration in
OvmfPkg.dec from [PcdsFixedAtBuild] to [PcdsDynamic, PcdsDynamicEx], and
provide the dynamic defaults (with the same value, 8) in the DSC files if
SMM_REQUIRE is TRUE.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Suggested-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
In one of the next patches we'll turn PcdQ35TsegMbytes into a dynamic PCD,
to be set by PlatformPei.
Jordan suggested to use gEfiPeiMemoryDiscoveredPpiGuid as SmmAccessPei's
DEPEX for making sure that PlatformPei sets the PCD before SmmAccessPei
consumes it. (PlatformPei installs the permanent PEI RAM.) Such a DEPEX is
supposed to mirror physical firmware, where anything related to SMRAM
cannot run before said platform's physical RAM is discovered (signaled by
the presence of gEfiPeiMemoryDiscoveredPpiGuid).
Introduce the InitQ35TsegMbytes() function and the "mQ35TsegMbytes" extern
variable to "SmramInternal.h" and "SmramInternal.c":
- Both SmmAccess modules (PEIM and DXE driver) are supposed to call
InitQ35TsegMbytes() in their respective entry point functions, saving
PcdQ35TsegMbytes into "mQ35TsegMbytes". This way dynamic PCD fetches can
be kept out of PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member
functions later (when we add support for extended TSEG size).
- We can thus replace the current PcdQ35TsegMbytes fetches in
SmmAccessPei's entry point function as well, with reads from
"mQ35TsegMbytes".
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Laszlo Ersek
Paulo Alcantara
Ge Song
Brijesh Singh
Ard Biesheuvel
Thomas Palmer
Michael D Kinney