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>
In one of the next patches we'll turn PcdQ35TsegMbytes into a dynamic PCD,
to be set by PlatformPei. Introduce the Q35TsegMbytesInitialization()
function and the "mQ35TsegMbytes" global variable to support this.
Q35TsegMbytesInitialization() manages the PCD and caches its final value
into "mQ35TsegMbytes". Call Q35TsegMbytesInitialization() from
InitializePlatform() just in time for the current PCD consumers,
PublishPeiMemory(), InitializeRamRegions() and QemuInitializeRam() --
which is called from InitializeRamRegions() -- to be rebased on top of
"mQ35TsegMbytes".
Call Q35TsegMbytesInitialization() only when PcdSmmSmramRequire is TRUE,
given that PcdQ35TsegMbytes is consumed in that case only.
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>
Widen PcdQ35TsegMbytes to UINT16, in preparation for setting it
dynamically to the QEMU-advertized extended TSEG size (which is 16-bits
wide).
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>
Jiaxin reports that the OvmfPkg/README instructions for downloading the
Intel PROEFI drivers, and the filenames in OvmfPkg/OvmfPkg*.fdf for
incorporating the same in the OVMF binaries, are no longer up to date; the
download link has stopped working.
Additionally, the IA32 driver binary is no more distributed by Intel.
Update OvmfPkg/README with new download instructions, and adapt the OVMF
FDF files.
With this driver in use for QEMU's e1000 NIC, the DH shell command prints,
as Controller Name, "Intel(R) PRO/1000 MT Network Connection". I
successfully tested DHCP and ping from the UEFI shell.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Reported-by: Jiaxin Wu <jiaxin.wu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=613
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
Acked-by: Jordan Justen <jordan.l.justen@intel.com>
When the GenFv utility from BaseTools composes a firmware volume, it
checks whether modules in the firmware volume are subject to build-time
relocation. The primary indication for relocation is whether the firmware
volume has a nonzero base address, according to the [FD] section(s) in the
FDF file that refer to the firmware volume.
The idea behind build-time relocation is that XIP (execute in place)
modules will not be relocated at boot-time:
- Pre-DXE phase modules generally execute in place.
(OVMF is no exception, despite the fact that we have writeable memory
even in SEC: PEI_CORE and PEIMs run in-place from PEIFV, after SEC
decompresses PEIFV and DXEFV from FVMAIN_COMPACT (flash) to RAM.
PEI_CORE and the PEIMs are relocated at boot-time only after PlatformPei
installs the permanent PEI RAM, and the RAM migration occurs.)
- Modules dispatched by the DXE Core are generally relocated at boot-time.
However, this is not necessarily so. Quoting Liming from
<https://lists.01.org/pipermail/edk2-devel/2017-July/012053.html>:
> PI spec has no limitation that XIP is for PEIM only. DXE driver may be
> built as XIP for other purpose. For example, if DXE driver image address
> is not zero, DxeCore will try allocating the preferred address and load
> it. In another case, once DXE driver is relocated at build time, DxeCore
> will dispatch it and start it directly without loading, it may save boot
> performance.
Therefore GenFv relocates even DXE and UEFI driver modules if the
containing firmware volume has a nonzero base address.
In OVMF, this is the case for both PEIV and DXEFV:
> [FD.MEMFD]
> BaseAddress = $(MEMFD_BASE_ADDRESS)
> Size = 0xB00000
> ErasePolarity = 1
> BlockSize = 0x10000
> NumBlocks = 0xB0
> ...
> 0x020000|0x0E0000
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
> FV = PEIFV
>
> 0x100000|0xA00000
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
> FV = DXEFV
While the build-time relocation certainly makes sense for PEIFV (see
above), the reasons for which we specify DXEFV under [FD.MEMFD] are
weaker:
- we set the PcdOvmfDxeMemFvBase and PcdOvmfDxeMemFvSize PCDs here,
- and we ascertain that DXEFV, when decompressed by SEC from
FVMAIN_COMPACT, will fit into the area allotted here, at build time.
In other words, the build-time relocation of the modules in DXEFV is a
waste of resources. But, it gets worse:
Build-time relocation of an executable is only possible if the on-disk and
in-memory layouts are identical, i.e., if the sections of the PE/COFF
image adhere to the same alignment on disk and in memory. Put differently,
the FileAlignment and SectionAlignment headers must be equal.
For boot-time modules that we build as part of edk2, both alignment values
are 0x20 bytes. For runtime modules that we build as part of edk2, both
alignment values are 0x1000 bytes. This is why the DXEFV relocation,
albeit wasteful, is also successful every time.
Unfortunately, if we try to include a PE/COFF binary in DXEFV that
originates from outside of edk2, the DXEFV relocation can fail due to the
binary having unmatched FileAlignment and SectionAlignment headers. This
is precisely the case with the E3522X2.EFI network driver for the e1000
NIC, from Intel's BootUtil / PREBOOT.EXE distribution.
The solution is to use the FvForceRebase=FALSE override under [FV.DXEFV].
This tells GenFv not to perform build-time relocation on the firmware
volume, despite the FV having a nonzero base address.
In DXEFV we also have SMM drivers. Those are relocated at boot-time (into
SMRAM) unconditionally; SMRAM is always discovered at boot-time.
Kudos to Ard and Liming for the PE/COFF sections & relocations
explanation, and for the FvForceRebase=FALSE tip.
I regression-tested this change in the following configurations (all with
normal boot and S3 suspend/resume):
IA32, q35, SMM, Linux
IA32X64, q35, SMM, Linux
IA32X64, q35, SMM, Windows-8.1
X64, i440fx, no-SMM, Linux
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=613
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=615
Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Suggested-by: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Acked-by: Jordan Justen <jordan.l.justen@intel.com>
Commit 4275f38507 ("OvmfPkg/AcpiPlatformDxe: alloc blobs from 64-bit
space unless restricted") introduced a variable which is [incorrectly]
identified by GCC as being potentially uninitialized. So let's just set
it to NULL before use.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
... by narrower than 8-byte ADD_POINTER references.
Introduce the CollectAllocationsRestrictedTo32Bit() function, which
iterates over the linker/loader script, and collects the names of the
fw_cfg blobs that are referenced by QEMU_LOADER_ADD_POINTER.PointeeFile
fields, such that QEMU_LOADER_ADD_POINTER.PointerSize is less than 8. This
means that the pointee blob's address will have to be patched into a
narrower-than-8 byte pointer field, hence the pointee blob must not be
allocated from 64-bit address space.
In ProcessCmdAllocate(), consult these restrictions when setting the
maximum address for gBS->AllocatePages(). The default is now MAX_UINT64,
unless restricted like described above to the pre-patch MAX_UINT32 limit.
In combination with Ard's QEMU commit cb51ac2ffe36 ("hw/arm/virt: generate
64-bit addressable ACPI objects", 2017-04-10), this patch enables
OvmfPkg/AcpiPlatformDxe to work entirely above the 4GB mark.
(An upcoming / planned aarch64 QEMU machine type will have no RAM under
4GB at all. Plus, moving the allocations higher is beneficial to the
current "virt" machine type as well; in Ard's words: "having all firmware
allocations inside the same 1 GB (or 512 MB for 64k pages) frame reduces
the TLB footprint".)
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Suggested-by: Gerd Hoffmann <kraxel@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Xen gained support for the 4MB flash image in Xen commit 0d6968635ce5
("hvmloader: avoid tests when they would clobber used memory",
2017-05-19), which is part of Xen 4.9.0-rc6.
The previously default 2MB can be explicitly selected with
-D FD_SIZE_2MB
or
-D FD_SIZE_IN_KB=2048
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>
(cherry picked from commit bba8dfbec3)
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
[lersek@redhat.com: reference Xen commit in commit message]
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
https://bugzilla.tianocore.org/show_bug.cgi?id=559
The XCODE5 tool chain has a FAMILY of GCC. The
GCC statements in the [BuildOptions] section add
flags that are not compatible with XCODE5. Add
empty XCODE5 statements in [BuildOptions] sections
to prevent the use of the GCC flags in XCODE5
builds.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Andrew Fish <afish@apple.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
EmuVariableFvbRuntimeDxe now uses a 4KB (EFI_PAGE_SIZE) block size.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Gary Lin <glin@suse.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
EmuVariableFvbRuntimeDxe currently produces a Firmware Volume Block
protocol that is based on a block map of two blocks, each block having
PcdFlashNvStorageFtwSpareSize for size.
(The total size is 2 * PcdFlashNvStorageFtwSpareSize.)
FaultTolerantWriteDxe in turn expects the block size to be a power of two.
In the 4MB build of OVMF, PcdFlashNvStorageFtwSpareSize is 264KB, which is
not a power of two. In order to equip EmuVariableFvbRuntimeDxe for this
build, shrink the block size to 4KB (EFI_PAGE_SIZE), and grow the block
count from 2 to EFI_SIZE_TO_PAGES(2 * PcdFlashNvStorageFtwSpareSize). The
total size remains
2 * PcdFlashNvStorageFtwSpareSize
--------------------------------- * EFI_PAGE_SIZE
EFI_PAGE_SIZE
Right now EmuVariableFvbRuntimeDxe open-codes the block count of 2 in
various limit checks, so introduce a few new macros:
- EMU_FVB_NUM_TOTAL_BLOCKS, for the LHS of the above product,
- EMU_FVB_NUM_SPARE_BLOCKS for the half of that.
Also rework the FVB protocol members to support an arbitrary count of
blocks.
Keep the invariant intact that the first half of the firmware volume hosts
the variable store and the FTW working block, and that the second half
maps the FTW spare area.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=527
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Gary Lin <glin@suse.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
According to the PI spec, Volume 3,
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> The variable argument list is a list of tuples. Each tuple describes a
> range of LBAs to erase and consists of the following:
> * An EFI_LBA that indicates the starting LBA
> * A UINTN that indicates the number of blocks to erase
(NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
In this driver, the NumOfLba local variable is defined with type UINTN,
but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Reported-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>
According to the PI spec, Volume 3,
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> The variable argument list is a list of tuples. Each tuple describes a
> range of LBAs to erase and consists of the following:
> * An EFI_LBA that indicates the starting LBA
> * A UINTN that indicates the number of blocks to erase
(NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
In this driver, the NumOfLba local variable is defined with type UINTN,
but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
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>
In the previous patch we had to add two explicit Null resolutions, but
here we can remove five PeiPcdLib ones, after setting the default to it.
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>
Currently the default (module type independent) PcdLib resolution is to
BasePcdLibNull.inf, which is inherited by all PEIMs. In the next patch,
we'll flip the PEIM default resolution to PeiPcdLib.inf, but in order to
keep that patch both correct and simple to review, we should spell out the
Null resolution for those two PEIMs (ReportStatusCodeRouterPei and
StatusCodeHandlerPei) that are now the only ones that don't specify an
explicit resolution.
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>
For the emulated variable store, PlatformPei allocates reserved memory (as
early as possible, so that the address remains the same during reboot),
and PcdEmuVariableNvStoreReserved carries the address to
EmuVariableFvbRuntimeDxe.
However, EmuVariableFvbRuntimeDxe is excluded from the SMM_REQUIRE build,
and then noone consumes PcdEmuVariableNvStoreReserved. Don't waste
reserved memory whenever that's the case.
(Even a dynamic default for PcdEmuVariableNvStoreReserved would be
unnecessary; but that way the PcdSet64S() call in the
ReserveEmuVariableNvStore() function doesn't compile.)
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
Brijesh Singh
Ard Biesheuvel
Thomas Palmer
Michael D Kinney