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>
"MdeModulePkg/MdeModulePkg.dec" declares PcdVariableStoreSize like this:
> The size of volatile buffer. This buffer is used to store VOLATILE
> attribute variables.
There is no inherent reason why the size of the volatile variable store
should match the same of the non-volatile variable store. Indeed flash
variables in the 4MB build work fine without this equality.
However, OvmfPkg/EmuVariableFvbRuntimeDxe uses PcdVariableStoreSize to
initialize the non-volatile VARIABLE_STORE_HEADER too. (Presumably based
on the fact that ultimately that storage will not be permanent.) When
using EmuVariableFvbRuntimeDxe in the 4MB build, the mismatch between the
two mentioned PCDs (which is apparent through EmuVariableFvbRuntimeDxe's
VARIABLE_STORE_HEADER) triggers an assertion in the variable driver:
> ASSERT MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c(3772):
> mNvVariableCache->Size == VariableStoreLength
Bringing PcdVariableStoreSize in sync with PcdFlashNvStorageVariableSize
fixes this. It also happens to ensure a volatile store size in the 4MB
build that equals the non-volatile store size, which likely doesn't hurt
for symmetry.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Fixes: b24fca0575
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
This PCD is no longer used.
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 commit b24fca0575 ("OvmfPkg: introduce 4MB flash image (mainly) for
Windows HCK", 2017-04-29), I changed PcdFlashNvStorageFtwSpareSize to
264KB, in the then-new default 4MB build.
While PcdFlashNvStorageFtwSpareSize remains exactly half of the entire
non-volatile store (which is 528KB), 264KB isn't itself a power of two.
This triggers an assertion failure in AllocateAlignedRuntimePages() when
PlatformPei calls it from the ReserveEmuVariableNvStore() function,
passing PcdFlashNvStorageFtwSpareSize as the Alignment parameter:
> ASSERT MdePkg/Library/PeiMemoryAllocationLib/MemoryAllocationLib.c(196):
> (Alignment & (Alignment - 1)) == 0
Round up the alignment to the next power of two if necessary.
Fixes: b24fca0575
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
This reverts commit bba8dfbec3.
The 264KB size introduced for the NV spare area in commit b24fca0575
("OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK",
2017-04-29) breaks the "-bios" (emulated varstore) use case. Until we sort
that out, revert the default build to the 2MB image.
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>
The previously default 2MB can be explicitly selected with
-D FD_SIZE_2MB
or
-D FD_SIZE_IN_KB=2048
Cc: Gary Ching-Pang Lin <glin@suse.com>
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>
The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
Certification Kit sets a 32 KB large non-authenticated variable.
In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
for the variable name and attributes as well.
Cc: Gary Ching-Pang Lin <glin@suse.com>
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>
The "Confirm64KilobytesOfUnauthenticatedVariableStorage" test case of the
Secure Boot Logo Test ("Microsoft.UefiSecureBootLogo.Tests") suite in the
Microsoft Hardware Certification Kit expects to be able to populate the
variable store up to roughly 64 KB, with a series of 1 KB sized,
unauthenticated variables. OVMF's current live varstore area is too small
for this: 56 KB.
Introduce the FD_SIZE_4MB build macro (equivalently, FD_SIZE_IN_KB=4096),
which
- enlarges the full flash image to 4MB -- QEMU supports up to 8MB, see
FLASH_MAP_BASE_MIN in "hw/i386/pc_sysfw.c" --,
- inside that, grows the varstore area / pflash chip to 528 KB, and within
it, the live area from 56 KB to 256 KB.
Importantly, a firmware binary built with -D FD_SIZE_4MB will *not* be
compatible with a variable store that originates from a variable store
template built *without* -D FD_SIZE_4MB. This is the reason for the large
increase, as every such change breaks compatibility between a new firmware
binary and old varstore files.
Enlarging the varstore does not impact the performance of normal
operations, as we keep the varstore block size 4KB. The performance of
reclaim is affected, but that is expected (since reclaim has to rework the
full live area). And, reclaim occurs proportionally less frequently.
While at it, the FVMAIN_COMPACT volume (with the compressed FFS file in
it) is also enlarged significantly, so that we have plenty of room for
future DXEFV (and perhaps PEIFV) increments -- DXEFV has been growing
steadily, and that increase shows through compression too. Right now the
PEIFV and DXEFV volumes need no resizing.
Here's a summary:
Description Compression type Size [KB]
------------------------- ----------------- ----------------------
Non-volatile data storage open-coded binary 128 -> 528 ( +400)
data
Variable store 56 -> 256 ( +200)
Event log 4 -> 4 ( +0)
Working block 4 -> 4 ( +0)
Spare area 64 -> 264 ( +200)
FVMAIN_COMPACT uncompressed 1712 -> 3360 (+1648)
FV FFS file LZMA compressed
PEIFV uncompressed 896 -> 896 ( +0)
individual PEI uncompressed
modules
DXEFV uncompressed 10240 -> 10240 ( +0)
individual DXE uncompressed
modules
SECFV uncompressed 208 -> 208 ( +0)
SEC driver
reset vector code
For now, the 2MB flash image remains the default.
Cc: Gary Ching-Pang Lin <glin@suse.com>
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>
FD_SIZE_xMB defines have existed for flash size selection. They can be
passed as "-D FD_SIZE_xMB" on the command line. Passing multiple of them
at the same time has never been supported; earlier settings on the command
line cannot be overridden.
Introduce the integer valued FD_SIZE_IN_KB macro, which provides the
following improvements:
- several instances of it are permitted on the command line, with the last
one taking effect,
- conditional statements in the DSC and FDF files need only check a single
macro, and multiple values can be checked in a single !if with the ||
operator,
- nested !ifdef / !else ladders can be replaced with flat equality tests,
- in the future, flash sizes can be expressed with a finer than MB
granularity, if necessary.
For now, we're going to preserve the FD_SIZE_xMB defines as convenience
wrappers for FD_SIZE_IN_KB.
FD_SIZE_IN_KB is being added to the DSC files because this way we can
depend on it in both the DSC and FDF files.
Cc: Gary Ching-Pang Lin <glin@suse.com>
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 addition to the QXL, Cirrus, etc. VGA adapters, Qemu also implements
a basic version of VMWare's SVGA display device. Drivers for this
device exist for some guest OSes which do not support Qemu's other
display adapters, so supporting it in OVMF is useful in conjunction
with those OSes.
This change adds support for the SVGA device's framebuffer to
QemuVideoDxe's graphics output protocol implementation, based on
VMWare's documentation. The most basic initialisation, framebuffer
layout query, and mode setting operations are implemented.
The device relies on port-based 32-bit I/O, unfortunately on misaligned
addresses. This limits the driver's support to the x86 family of
platforms.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
The VMWare SVGA display device implemented by Qemu (-vga vmware) uses
an I/O-type BAR which is laid out such that some register offsets are
not aligned to the read/write width with which they are expected to be
accessed. (The register value port has an offset of 1 and requires
32 bit wide read/write access.)
The EFI_PCI_IO_PROTOCOL's Io.Read/Io.Write functions do not support
such unaligned I/O.
Before a driver for this device can be added to QemuVideoDxe, helper
functions for unaligned I/O are therefore required. This adds the
functions UnalignedIoWrite32 and UnalignedIoRead32, based on IoLib's
IoWrite32 and IoRead32, for the Ia32 and X64 architectures. Port I/O
requires inline assembly, so implementations are provided for the GCC,
ICC, and Microsoft compiler families. Such I/O is not possible on other
architectures, a dummy (ASSERT()ing) implementation is therefore
provided to satisfy the linker.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Suggested-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
This adds a header file defining symbolic constants for the VMWare SVGA
virtual display device in preparation for supporting it in
QemuVideoDxe.
It is mostly an extract of the file lib/vmware/svga_reg.h from commit
329dd537456f93a806841ec8a8213aed11395def of VMWare's vmware-svga
repository at git://git.code.sf.net/p/vmware-svga/git (See also
http://vmware-svga.sourceforge.net/ )
Only the bare essentials necessary for initialisation, modesetting and
framebuffer access have been kept from the original file; macro names
have been prefixed with VMWARE_SVGA_ instead of SVGA2_, and the enum
definition has been adapted to comply with EDK2 naming conventions.
The original file was released by VMWare under the MIT license, this
has been retained.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
ACPI tables may contain multiple fields which point to the same
destination table. For example, in some revisions, the FADT contains
both DSDT and X_DSDT fields, and they may both point to the DSDT.
Previously, if Qemu created QEMU_LOADER_ADD_POINTER linker commands for
such instances, the linking process would attempt to install the same
pointed-to table repeatedly. For tables of which there must only be one
instance, the call to AcpiProtocol->InstallAcpiTable() would fail during
the second linker command pointing to the same table, thus entirely
aborting the ACPI table linking process. In the case of tables of which
there may be multiple instances, the table would end up duplicated.
This change adds a memoisation data structure which tracks the table
pointers that have already been processed; even if the same pointer is
encountered multiple times, it is only processed once.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: DSDT<->XSDT typo, debug msg, and coding style fixups]
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=368
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Since qemu v2.7.0, the pkgversion appears to have a bug:
$ ./configure --target-list=x86_64-softmmu --with-pkgversion=foo
Results in this output:
$ x86_64-softmmu/qemu-system-x86_64 -version
QEMU emulator version 2.8.90(foo)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
This appears to have been introduced in:
67a1de0d19 Makefile: Derive "PKGVERSION" from "git describe" by default
The previous commit (077de81a4c) produces this output:
$ x86_64-softmmu/qemu-system-x86_64 -version
QEMU emulator version 2.6.50 (foo), Copyright (c) 2003-2008 Fabrice Bellard
Now the OvmfPkg/build.sh script uses grep with '-o' to return only the
matched text.
grep -E is also used with a simple regex to extract only the digits of
the version.
qemu-bug: https://bugs.launchpad.net/bugs/1673373
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Drop the explicit S3SaveState protocol and opcode management; instead,
create ACPI S3 Boot Script opcodes for the WRITE_POINTER commands with
QemuFwCfgS3Lib functions.
In this case, we have a dynamically allocated Context structure, hence the
patch demonstrates how the FW_CFG_BOOT_SCRIPT_CALLBACK_FUNCTION takes
ownership of Context.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
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 cannot entirely eliminate the manual boot script building in this
driver, as it also programs lower-level chipset registers (SMI_EN,
GEN_PMCON_1) at S3 resume, not just registers exposed via fw_cfg.
We can nonetheless replace the manually built opcodes for the latter class
of registers with QemuFwCfgS3Lib function calls. We preserve the ordering
between the two sets of registers (low-level chipset first, fw_cfg
second).
This patch demonstrates that manual handling of S3SaveState protocol
installation can be combined with QemuFwCfgS3Lib, even without upsetting
the original order between boot script fragments. An S3SaveState notify
function running at TPL_CALLBACK can safely queue another S3SaveState
notify function at TPL_CALLBACK with QemuFwCfgS3CallWhenBootScriptReady().
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
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 DXE fw_cfg instance:
- QemuFwCfgS3Enabled() queries S3 enablement via fw_cfg. This behavior is
shared with the PEI fw_cfg instance, and the DXE fw_cfg instance already
pulls in the function from "QemuFwCfgS3PeiDxe.c".
- If QemuFwCfgS3Enabled() returns TRUE, the client module is permitted to
call QemuFwCfgS3CallWhenBootScriptReady().
We provide a fully functional implementation for
QemuFwCfgS3CallWhenBootScriptReady(). A protocol notify is installed at
TPL_CALLBACK for EFI_S3_SAVE_STATE_PROTOCOL. If / once the protocol is
available, the client module's Callback() function is called, which is
expected to produce ACPI S3 Boot Script opcodes using the helper
functions listed below. In QemuFwCfgS3CallWhenBootScriptReady(), we also
allocate a reserved memory buffer, sized & typed by the client module,
for the opcodes and (internally) the fw_cfg DMA operations to work upon,
during S3 resume.
This behavior is unique to the DXE fw_cfg instance. Thus, add the
function to "QemuFwCfgS3Dxe.c".
- The QemuFwCfgS3ScriptWriteBytes(), QemuFwCfgS3ScriptReadBytes(),
QemuFwCfgS3ScriptSkipBytes(), and QemuFwCfgS3ScriptCheckValue()
functions are also implemented usefully, since the client module's
Callback() function is expected to invoke them.
Each of the first three functions produces MEM_WRITE, IO_WRITE, and
MEM_POLL opcodes, to set up the DMA command in reserved memory, to start
the DMA transfer, and to check the DMA result, respectively.
The QemuFwCfgS3ScriptCheckValue() function produces a MEM_POLL opcode to
validate an unsigned integer field in data that was read via
QemuFwCfgS3ScriptReadBytes().
This behavior is again unique to the DXE fw_cfg instance, so add the
functions to "QemuFwCfgS3Dxe.c".
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
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 PEI fw_cfg instance:
- QemuFwCfgS3Enabled() queries S3 enablement via fw_cfg. This behavior is
shared with the DXE fw_cfg instance, and the PEI fw_cfg instance already
pulls in the function from "QemuFwCfgS3PeiDxe.c".
- If QemuFwCfgS3Enabled() returns TRUE, the client module is permitted to
call QemuFwCfgS3CallWhenBootScriptReady(). However, in the PEI phase we
have no support for capturing ACPI S3 Boot Script opcodes, hence we
return RETURN_UNSUPPORTED unconditionally. This behavior is unique to
the PEI fw_cfg instance, so add the function to "QemuFwCfgS3Pei.c".
- Consequently, the QemuFwCfgS3ScriptWriteBytes(),
QemuFwCfgS3ScriptReadBytes(), QemuFwCfgS3ScriptSkipBytes(), and
QemuFwCfgS3ScriptCheckValue() functions must never be called. (They
could only be called from the client module's callback, but
QemuFwCfgS3CallWhenBootScriptReady() will never install such callback in
the PEI fw_cfg instance -- see above.)
This behavior is not unique to the PEI fw_cfg instance (it is shared
with the Base Null instance), so pull in these functions from
"QemuFwCfgS3BasePei.c".
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
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 Base Null instance:
- QemuFwCfgS3Enabled() returns constant FALSE. This is unique to the Base
Null instance, and the function is already present in
"QemuFwCfgS3Base.c".
- The QemuFwCfgS3CallWhenBootScriptReady() function must never be called
(according to the documentation, given the above). This is also unique
to the Base Null instance, so implement the function in
"QemuFwCfgS3Base.c".
- Consequently, the QemuFwCfgS3ScriptWriteBytes(),
QemuFwCfgS3ScriptReadBytes(), QemuFwCfgS3ScriptSkipBytes(), and
QemuFwCfgS3ScriptCheckValue() functions must never be called either.
This behavior is not unique to the Base Null instance (it will be shared
with the PEI fw_cfg instance), so add these functions to
"QemuFwCfgS3BasePei.c".
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Introduce the following APIs:
- QemuFwCfgS3CallWhenBootScriptReady(): central function that registers a
callback function, with a context parameter, for when ACPI S3 Boot
Script opcodes can be produced. This function also allocates reserved
memory for the opcodes to operate upon.
The client module is supposed to produce the boot script fragment in the
callback function.
- QemuFwCfgS3ScriptWriteBytes(), QemuFwCfgS3ScriptReadBytes(),
QemuFwCfgS3ScriptSkipBytes(), QemuFwCfgS3ScriptCheckValue(): helper
functions, available only to the above callback function, for composing
the boot script fragment. QemuFwCfgS3ScriptSkipBytes() can double as a
plain "select" whenever necessary.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
At this point we're ready to retire QemuFwCfgS3Enabled() from the
QemuFwCfgLib class, together with its implementations in:
- ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
- OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
Extend all modules that call the function with a new QemuFwCfgS3Lib class
dependency. Thanks to the previously added library class, instances, and
class resolutions, we can do this switch now as tightly as possible.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
QemuFwCfgS3Enabled() in "OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c"
queries the "etc/system-states" fw_cfg file.
The same implementation is now available factored-out in
"OvmfPkg/Library/QemuFwCfgS3Lib/QemuFwCfgS3PeiDxe.c". It is available to
PEIMs through the PeiQemuFwCfgS3LibFwCfg instance, and to DXE_DRIVER and
DXE_RUNTIME_DRIVER modules through the DxeQemuFwCfgS3LibFwCfg instance.
Resolve QemuFwCfgS3Lib accordingly.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
This patch introduces PeiQemuFwCfgS3LibFwCfg, a limited functionality
QemuFwCfgS3Lib instance, for PEI phase modules.
The patch also introduces DxeQemuFwCfgS3LibFwCfg, a full functionality
QemuFwCfgS3Lib instance, for DXE_DRIVER and DXE_RUNTIME_DRIVER modules.
These library instances share the QemuFwCfgS3Enabled() function. The
function actually uses fw_cfg; the implementation is copied from
"OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c".
The library instances will diverge in the following patches.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
This library instance returns constant FALSE from QemuFwCfgS3Enabled(),
and all other library functions trigger assertion failures. It is suitable
for QEMU targets and machine types that never enable S3.
The QemuFwCfgS3Enabled() implementation is copied from
"ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c". Stubs for further
QemuFwCfgS3Lib APIs (with assertion failures, see above) will be added
later.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
This library class will enable driver modules (a) to query whether S3
support was enabled on the QEMU command line, (b) to produce fw_cfg DMA
operations that are to be replayed at S3 resume time.
Declare the library class in OvmfPkg/OvmfPkg.dec, and add the library
class header under OvmfPkg/Include/Library/. At the moment, the only API
we expose is QemuFwCfgS3Enabled(), which we'll first migrate from
QemuFwCfgLib. Further interfaces will be added in later patches.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=394
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>
The OpensslLibCrypto library instance (which does not contain libssl
functions) is sufficient for the Secure Boot feature.
Ease security analysis by excluding libssl functionality from the
OpensslLib instance we use with TLS_ENABLE=FALSE.
Cc: Gary Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tomas Hoger <thoger@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Gary Lin <glin@suse.com>
... because this function use VA_COPY.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Introduce the FW_CFG_IO_DMA_ADDRESS macro for IO Ports 0x514 and 0x518
(most significant and least significant halves of the DMA Address
Register, respectively), and update all references in OvmfPkg.
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>
Introduce the FW_CFG_IO_DATA macro for IO Port 0x511 (the Data Register),
and update all references in OvmfPkg.
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>
Introduce the FW_CFG_IO_SELECTOR macro for IO Port 0x510 (the Selector
Register), and update all references in OvmfPkg.
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>
Commit df73df138d ("OvmfPkg/AcpiPlatformDxe: replay
QEMU_LOADER_WRITE_POINTER commands at S3", 2017-02-09) added
"BootScript.c" with such comments on the PointerValue field of
CONDENSED_WRITE_POINTER, and on the corresponding PointerValue parameter
of SaveCondensedWritePointerToS3Context(), that did not consider the
then-latest update of the QEMU_LOADER_WRITE_POINTER structure. (Namely,
the introduction of the PointeeOffset field.)
The code is fine as-is -- ProcessCmdWritePointer() already calls
SaveCondensedWritePointerToS3Context() correctly, and "BootScript.c"
itself is indifferent to the exact values --, but the comments in
"BootScript.c" should match reality too. Update them.
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>
The Count parameter of RShiftU64() must be strictly smaller than 64.
ProcessCmdAddPointer() and ProcessCmdWritePointer() currently ensure this
by "cleverly" breaking the last bit of a potentially 8-byte right shift
out to a separate operation.
Instead, exclude the Count==64 case explicitly (in which case the
preexistent outer RShiftU64() would return 0), and keep only the inner
RShiftU64(), with the direct Count however.
This is not a functional change, just style improvement.
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>
Ultimately, each QEMU_LOADER_WRITE_POINTER command creates a guest memory
reference in some QEMU device. When the virtual machine is reset, the
device willfully forgets the guest address, since the guest memory is
wholly invalidated during platform reset.
... Unless the reset is part of S3 resume. Then the guest memory is
preserved intact, and the firmware must reprogram those devices with the
original guest memory allocation addresses.
This patch accumulates the fw_cfg select, skip and write operations of
ProcessCmdWritePointer() in a validated / condensed form, and turns them
into an ACPI S3 Boot Script fragment at the very end of
InstallQemuFwCfgTables().
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=359
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
The QEMU_LOADER_WRITE_POINTER command instructs the firmware to write the
address of a field within a previously allocated/downloaded fw_cfg blob
into another (writeable) fw_cfg file at a specific offset.
Put differently, QEMU_LOADER_WRITE_POINTER propagates, to QEMU, the
address that QEMU_LOADER_ALLOCATE placed the designated fw_cfg blob at, as
adjusted for the given field inside the allocated blob.
The implementation is similar to that of QEMU_LOADER_ADD_POINTER. Since
here we "patch" a pointer object in "fw_cfg file space", not guest memory
space, we utilize the QemuFwCfgSkipBytes() and QemuFwCfgWriteBytes() APIs
completed in commit range 465663e9f128..7fcb73541299.
An interesting aspect is that QEMU_LOADER_WRITE_POINTER creates a
host-level reference to a guest memory location. Therefore, if we fail to
process the linker/loader script for any reason, we have to clear out
those references first, before we release the guest memory allocations in
response to the error.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=359
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
The longest line is currently 84 characters long.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=359
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
No functional changes in this patch, just prepare the grounds with some
reformatting (trailing comma after the last enumeration constant,
horizontal whitespace insertion) so that the next patch can be cleaner.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=359
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
When writing to IO port 0xB2 (ICH9_APM_CNT), QEMU by default injects an
SMI only on the VCPU that is writing the port. This has exposed corner
cases and strange behavior with edk2 code, which generally expects a
software SMI to affect all CPUs at once. We've experienced instability
despite the fact that OVMF sets PcdCpuSmmApSyncTimeout and
PcdCpuSmmSyncMode differently from the UefiCpuPkg defaults, such that they
match QEMU's unicast SMIs better. (Refer to edk2 commits 9b1e378811 and
bb0f18b0bce6.)
Using the new fw_cfg-based SMI feature negotiation in QEMU (see commits
50de920b372b "hw/isa/lpc_ich9: add SMI feature negotiation via fw_cfg" and
5ce45c7a2b15 "hw/isa/lpc_ich9: add broadcast SMI feature"), we can ask
QEMU to broadcast SMIs. Extensive testing from earlier proves that
broadcast SMIs are only reliable if we use the UefiCpuPkg defaults for the
above PCDs. With those settings however, the broadcast is very reliable --
the most reliable configuration encountered thus far.
Therefore negotiate broadcast SMIs with QEMU, and if the negotiation is
successful, dynamically revert the PCDs to the UefiCpuPkg defaults.
Setting the PCDs in this module is safe:
- only PiSmmCpuDxeSmm consumes them,
- PiSmmCpuDxeSmm is a DXE_SMM_DRIVER, launched by the SMM_CORE
(MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf),
- the SMM_CORE is launched by the SMM IPL runtime DXE driver
(MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf),
- the SMM IPL has a DEPEX on EFI_SMM_CONTROL2_PROTOCOL,
- OvmfPkg/SmmControl2Dxe produces that protocol.
The end result is that PiSmmCpuDxeSmm cannot be dispatched before
SmmControl2Dxe installs EFI_SMM_CONTROL2_PROTOCOL and returns from its
entry point. Hence we can set the PCD's consumed by PiSmmCpuDxeSmm in
SmmControl2Dxe.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=230
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Move the platform-specific default values for these PCDs from the
[PcdsFixedAtBuild] / [PcdsFixedAtBuild.X64] sections to the
[PcdsDynamicDefault] section.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=230
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Introduce the new public API QemuFwCfgSkipBytes(), for advancing over
bytes in the selected firmware configuration item without transferring
data between the item and the caller.
When the DMA interface is available (the common case), the operation is
instantaneous. As a fallback, provide a loop of chunked reads into a small
stack-allocated scratch buffer.
This patch enables OvmfPkg/QemuFwCfgLib to overwrite part of a writeable
fw_cfg file, which will be particularly useful for the upcoming
QEMU_LOADER_WRITE_POINTER command in OvmfPkg/AcpiPlatformDxe.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=359
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
The fw_cfg DMA interface provides a simple method to skip over bytes in an
fw_cfg blob before reading or writing more bytes.
InternalQemuFwCfgDmaBytes() can support it easily, we just have to expose
the Control parameter more flexibly than the current "Write" BOOLEAN.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=359
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
https://bugzilla.tianocore.org/show_bug.cgi?id=339
The patch removes the assumption in QemuVideoDxe driver that it
wrongly assumes the frame buffer configure size is the same in
different video modes.
The assumption is true in old FrameBufferBltLib but is false in
new implementation.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
v2
* Move the setting above the "!ifndef $(USE_OLD_SHELL)" part.
* Un-indent the setting to column zero.
(Comments from Laszlo)
Overwrite the value of PcdAllowHttpConnections to allow HTTP
connections if HTTP Boot enabled (-D HTTP_BOOT_ENABLE).
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Justen Jordan L <jordan.l.justen@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Kinney Michael D <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Gary Lin <glin@suse.com>
Tested-by: Gary Lin <glin@suse.com>
If the code eventually returns "Status" anyway, it does not make
sense to explicitely return "Status" in case of an error, too.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
This commit introduces a new build option, TLS_ENABLE, to pull in the
TLS-related modules. If HTTP_BOOT_ENABLE and TLS_ENABLE are enabled at
the same time, the HTTP driver locates the TLS protocols automatically
and thus HTTPS is enabled.
To build OVMF with HTTP Boot:
$ ./build.sh -D HTTP_BOOT_ENABLE
To build OVMF with HTTPS Boot:
$ ./build.sh -D HTTP_BOOT_ENABLE -D TLS_ENABLE
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Justen Jordan L <jordan.l.justen@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Cc: Long Qin <qin.long@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin <glin@suse.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Always use IScsiDxe from NetworkPkg when IPv6 is enabled since it provides
the complete ISCSI support.
NOTE: This makes OpenSSL a hard requirement when NETWORK_IP6_ENABLE is
true.
(Based on Jiaxin's suggestion)
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Justen Jordan L <jordan.l.justen@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Cc: Long Qin <qin.long@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin <glin@suse.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: update subject line]
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
This commit provides unconditional library resolutions for the OpenSslLib,
IntrinsicLib and BaseCryptLib classes, regardless of whether those classes
are actually used by any module.
Although those libraries depends on OpenSSL, they won't be built unless
a module really uses them. Thus, missing OpenSSL from the tree won't
cause any build failure as long as SECURE_BOOT_ENABLE is false.
(Based on Jiaxin's patch and Laszlo's suggestion)
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Justen Jordan L <jordan.l.justen@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Cc: Long Qin <qin.long@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin <glin@suse.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
The Fifo routines from the QuemuFwCfgLib library have been ported
to the new BaseIoLibIntrinsic (IoLib class) library.
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>
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
The Debug Agent in the SourceLevelDebugPkg can multiplex
both source level debug messages and console messages on
the same UART. When this is done, the Debug Agent owns
the UART device and an additional device handle with a
Serial I/O Protocol is produced with a VenHw device path
node.
In order for a platform to provide a UART based console
when the Debug Agent is using the same UART device, the
PlatformBootManagerLib must consider the SerialI/O
Protocol produces by the Debug Agent as one of the
supported consoles.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney <michael.d.kinney@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
EFI_BOOT_SCRIPT_PCI_CONFIG_READ_WRITE_OPCODE expects the PCI address to
access in UEFI encoding, not in edk2/PciLib encoding.
Introduce the POWER_MGMT_REGISTER_Q35_EFI_PCI_ADDRESS() macro, and with
it, store the ICH9_GEN_PMCON_1 register's address to the boot script in
UEFI representation.
Cc: Jiewen Yao <jiewen.yao@intel.com>
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>
While debugging OS for ACPI BGRT support (especially on VMs),
it is very useful to have the EFI firmware (OVMF in most cases
which use Tianocore) to export the ACPI BGRT table.
This patch tries to add this support in OvmfPkg.
Tested this patch in the following environments:
1. On both RHEL7.3 and Fedora-25 VM guests running on a Fedora-24 Host:
- Ensured that the BGRT logo is properly prepared and
can be viewed with user-space tools (like 'Gwenview' on KDE,
for example):
$ file /sys/firmware/acpi/bgrt/image
/sys/firmware/acpi/bgrt/image: PC bitmap, Windows 3.x format, 193 x
58 x 24
2. On a Windows-10 VM Guest running on a Fedora-24 Host:
- Ensured that the BGRT ACPI table is properly prepared and can be
read with freeware tool like FirmwareTablesView:
==================================================
Signature : BGRT
Firmware Provider : ACPI
Length : 56
Revision : 1
Checksum : 129
OEM ID : INTEL
OEM Table ID : EDK2
OEM Revision : 0x00000002
Creator ID : 0x20202020
Creator Revision : 0x01000013
Description :
==================================================
Note from Laszlo Ersek <lersek@redhat.com>: without the BGRT ACPI table,
Windows 8 and Windows 10 first clear the screen, then display a blue,
slanted Windows picture above the rotating white boot animation. With the
BGRT ACPI table, Windows 8 and Windows 10 don't clear the screen, the blue
Windows image is not displayed, and the rotating white boot animation is
shown between the firmware's original TianoCore boot splash and (optional)
"Start boot option" progress bar.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: cover effect on Windows 8/10 boot anim. in commit msg]
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
The goal of the patch is to avoid using -flto with GCC 6.0 to 6.2.
This is to workaround a GCC bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70955
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Because EFIAPI is necessary for functions declared in library class header
files.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Build-tested-by: Laszlo Ersek <lersek@redhat.com>
The benefits of the DMA-like access method are (a) speed, (b) write
support in QEMU 2.9+.
(IOPort-based write support was discontinued in QEMU 2.4, and the
DMA-based one is being added to QEMU 2.9. Write support needs no separate
feature detection because writeability is governed on the level of
individual fw_cfg files -- if a file meant to be written by the firmware
exists in the directory, then it is writeable with the DMA method.)
We don't enable this feature for the SEC library instance, because:
- the SEC instance remains without clients (I've checked that it builds
though),
- in SEC, any possible fw_cfg use is expected to be small and read-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>
The last patch consists purely of code movement; going forward, we should
use a few more symbolic constants.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
InternalQemuFwCfgIsAvailable() is an API that is incorrectly exposed by
the "OvmfPkg/Include/Library/QemuFwCfgLib.h" library class header; the API
is meant to be used internally to library instances (if it's needed at
all).
In OvmfPkg, we have two lib instances (for SEC and PEI/DXE); they provide
different implementations of InternalQemuFwCfgIsAvailable(), for the
shared file "OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c". Move the API
declaration to a new internal header called "QemuFwCfgLibInternal.h", and
drop EFIAPI in the process.
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>
LzmaCustomDecompressLib and PeiDxeDebugLibReportStatusCode were copied
from IntelFrameworkModulePkg to MdeModulePkg, but the originals were
kept for compatibility.
Since the libraries are identical, move OvmfPkg to use the MdeModulePkg
versions instead.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
These settings will allow CpuMpPei and CpuDxe to wait for the initial AP
check-ins exactly as long as necessary.
It is safe to set PcdCpuMaxLogicalProcessorNumber and
PcdCpuApInitTimeOutInMicroSeconds in OvmfPkg/PlatformPei.
OvmfPkg/PlatformPei installs the permanent PEI RAM, producing
gEfiPeiMemoryDiscoveredPpiGuid, and UefiCpuPkg/CpuMpPei has a depex on
gEfiPeiMemoryDiscoveredPpiGuid.
It is safe to read the fw_cfg item QemuFwCfgItemSmpCpuCount (0x0005). It
was added to QEMU in 2008 as key FW_CFG_NB_CPUS, in commit 905fdcb5264c
("Add common keys to firmware configuration"). Even if the key is
unavailable (or if fw_cfg is entirely unavailable, for example on Xen),
QemuFwCfgRead16() will return 0, and then we stick with the current
behavior.
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@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 value of the page tables' address is hard-coded in the
ResetVector. This patch replaces these values with a PCD dependency.
A check for the size has been added to alert the developer to rewrite
the ASM according to the new size, if it has been changed.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Remove the ResetVector.asm file as it is no longer referenced since
the switch to ResetVector.nasmb.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Convert the remaining pieces to make the code shorter and more readable.
Cc: Justen Jordan <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin <glin@suse.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: tweak subject line]
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Rely on the central macro definition from "MdePkg/Include/Base.h" instead.
Cc: Gary Lin <glin@suse.com>
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: Gary Lin <glin@suse.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Rely on the central macro definition from "MdePkg/Include/Base.h" instead.
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 introduce ARRAY_SIZE in
"MdePkg/Include/Base.h". In order to proceed in small steps, make the
module-local definition of ARRAY_SIZE conditional. This way the
introduction of the macro under MdePkg will silently switch this module
over (after which we can remove the module-local definition completely).
Cc: Gary Lin <glin@suse.com>
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: Gary Lin <glin@suse.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
In one of the next patches, we'll introduce ARRAY_SIZE in
"MdePkg/Include/Base.h". In order to proceed in small steps, make the
module-local definition of ARRAY_SIZE conditional. This way the
introduction of the macro under MdePkg will silently switch this module
over (after which we can remove the module-local definition completely).
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>
XenConsoleSerialPortLib is a BASE type library instance, without being
restricted to UEFI client modules. (For example, the
"ArmVirtPkg/ArmVirtXen.dsc" platform builds this library instance into
"ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf", which is a SEC type
module.) For such library instances, including <Uefi/UefiBaseType.h> is
not right.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
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>
These are deprecated / disabled under the
DISABLE_NEW_DEPRECATED_INTERFACES feature test macro.
Introduce a variable called PcdStatus, and use it to assert the success of
these operations (there is no reason for them to fail here).
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=166
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
These are deprecated / disabled under the
DISABLE_NEW_DEPRECATED_INTERFACES feature test macro.
Introduce a variable called PcdStatus, and use it to assert the success of
these operations (there is no reason for them to fail here).
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Gary Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=166
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Gary Lin <glin@suse.com>
Tested-by: Gary Lin <glin@suse.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
These are deprecated / disabled under the
DISABLE_NEW_DEPRECATED_INTERFACES feature test macro.
Introduce a variable called PcdStatus, and use it to assert the success of
these operations (there is no reason for them to fail here).
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=166
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # RVCT
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
These are deprecated / disabled under the
DISABLE_NEW_DEPRECATED_INTERFACES feature test macro.
Introduce a variable called PcdStatus, and use it to assert the success of
these operations (there is no reason for them to fail here).
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=166
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # RVCT
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
These are deprecated / disabled under the
DISABLE_NEW_DEPRECATED_INTERFACES feature test macro.
Introduce a variable called PcdStatus, and use it to assert the success of
these operations (there is no reason for them to fail here).
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Gary Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=166
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Gary Lin <glin@suse.com>
Tested-by: Gary Lin <glin@suse.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
These are deprecated / disabled under the
DISABLE_NEW_DEPRECATED_INTERFACES feature test macro.
Introduce a variable called PcdStatus, and use it to assert the success of
these operations (there is no reason for them to fail here).
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Gary Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=166
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Gary Lin <glin@suse.com>
Tested-by: Gary Lin <glin@suse.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
AsciiStrCat() is deprecated / disabled under the
DISABLE_NEW_DEPRECATED_INTERFACES feature test macro.
Replace AsciiStrCat() with AsciiSPrint(). Spell out the (already existent)
PrintLib dependency in the INF file. Add an explicit ASSERT() to document
that XenStoreJoin() assumes that the pool allocation always succeeds.
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Gary Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=166
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Gary Lin <glin@suse.com>
Tested-by: Gary Lin <glin@suse.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
AsciiStrCpy() is deprecated / disabled under the
DISABLE_NEW_DEPRECATED_INTERFACES feature test macro.
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Gary Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=166
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Gary Lin <glin@suse.com>
Tested-by: Gary Lin <glin@suse.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
In commit 5b2291f956 ("OvmfPkg: QemuVideoDxe uses
MdeModulePkg/FrameBufferLib"), QemuVideoDxe was rebased to
FrameBufferBltLib.
The FrameBufferBltLib instance added in commit b1ca386074
("MdeModulePkg: Add FrameBufferBltLib library instance") logs many
messages on the VERBOSE level; for example, a normal boot with OVMF can
produce 500+ "VideoFill" messages, dependent on the progress bar, when the
VERBOSE bit is set in PcdDebugPrintErrorLevel. While FrameBufferBltLib is
certainly allowed to log such messages on the VERBOSE level, we should
separate those frequent messages from the (infrequent) ones produced by
QemuVideoDxe itself.
QemuVideoDxe logs VERBOSE messages in three locations (in two functions)
at the moment. All of them are infrequent: both QemuVideoBochsModeSetup()
and InstallVbeShim() are called from QemuVideoControllerDriverStart(),
that is, when a device is bound. Upgrade these messages to INFO level, so
that VERBOSE can be disabled in PcdDebugPrintErrorLevel -- perhaps
selectively for OvmfPkg/QemuVideoDxe -- without hiding these infrequent
messages.
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>
This field is (re)allocated in QemuVideoGraphicsOutputSetMode(), released
in QemuVideoGraphicsOutputDestructor(), and used for nothing else. Remove
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>
This field is never used.
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>
Thanks to the previous patch, this field is also unnecessary now. Remove
it.
The patch is best reviewed with "git show --word-diff".
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>
Brijesh Singh
Laszlo Ersek
Thomas Palmer
Michael D Kinney
Ard Biesheuvel
Phil Dennis-Jordan
Jordan Justen
Dandan Bi
Anthony PERARD
Ruiyu Ni
Jiaxin Wu
Thomas Huth
Gary Lin
Leo Duran
Bhupesh Sharma
Leif Lindholm
Konrad Rzeszutek Wilk
Jiewen Yao
Liming Gao
Marvin Häuser