Commit:24e4ad7 (OvmfPkg: Add AmdSevDxe driver) added a driver which runs
early in DXE phase and clears the C-bit from NonExistent entry -- which
is later split and accommodate the flash MMIO. When SMM is enabled, we
build two sets of page tables; first page table is used when executing
code in non SMM mode (SMM-less-pgtable) and second page table is used
when we are executing code in SMM mode (SMM-pgtable).
During boot time, AmdSevDxe driver clears the C-bit from the
SMM-less-pgtable. But when SMM is enabled, Qemu Flash services are used
from SMM mode.
In this patch we explicitly clear the C-bit from Qemu flash MMIO range
before we probe the flash. When OVMF is built with SMM_REQUIRE then
call to initialize the flash services happen after the SMM-pgtable is
created and processor has served the first SMI. At this time we will
have access to the SMM-pgtable.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[lersek@redhat.com: trivial coding style improvements]
Reviewed-by: Laszlo Ersek <lersek@redhat.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>
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>
When the user requires "security" by passing -D SMM_REQUIRE, and
consequently by setting PcdSmmSmramRequire, enforce flash-based variables.
Furthermore, add two ASSERT()s to catch if the wrong module were pulled
into the build.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@19063 6f19259b-4bc3-4df7-8a09-765794883524
Before introducing the SMM driver interface, clean up #include directives
and [LibraryClasses] by:
- removing what's not directly used (HobLib and UefiLib),
- adding what's used but not spelled out (DevicePathLib),
- sorting the result.
This helps with seeing each source file's dependencies and with
determining the library classes for the SMM driver.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18672 6f19259b-4bc3-4df7-8a09-765794883524
In preparation for introducing an SMM interface to this driver, move the
following traits to separate files, so that we can replace them in the new
SMM INF file:
- Protocol installations. The SMM driver will install protocol interfaces
in the SMM protocol database, using SMM services.
- Virtual address change handler and pointer conversions. SMM drivers run
with physical mappings and pointers must not be converted.
There are further restrictions and changes for an SMM driver, but the rest
of the code either complies with those already, or will handle the changes
transparently. For example:
- SMM drivers have access to both UEFI and SMM protocols in their entry
points (see the PI spec 1.4, "1.7 SMM Driver Initialization"),
- MemoryAllocationLib has an SMM instance that serves allocation requests
with the gSmst->SmmAllocatePool() service transparently, allocating
runtime-marked SMRAM.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18671 6f19259b-4bc3-4df7-8a09-765794883524
Currently the EFI_FW_VOL_INSTANCE and ESAL_FWB_GLOBAL structures declare
the following entries as arrays, with two entries each:
- EFI_FW_VOL_INSTANCE.FvBase[2]
- ESAL_FWB_GLOBAL.FvInstance[2]
In every case, the entry at subscript zero is meant as "physical address",
while the entry at subscript one is meant as "virtual address" -- a
pointer to the same object. The virtual address entry is originally
initialized to the physical address, and then it is converted to the
virtual mapping in FvbVirtualddressChangeEvent().
Functions that (a) read the listed fields and (b) run both before and
after the virtual address change event -- since this is a runtime DXE
driver -- derive the correct array subscript by calling the
EfiGoneVirtual() function from UefiRuntimeLib.
The problem with the above infrastructure is that it's entirely
superfluous.
EfiGoneVirtual() "knows" whether EFI has gone virtual only because the
UefiRuntimeLib constructor registers the exact same kind of virtual
address change callback, and the callback flips a static variabe to TRUE,
and EfiGoneVirtual() queries that static variable.
In effect this means for QemuFlashFvbServicesRuntimeDxe: "when there is a
virtual address change, convert the entries with subscript one from
physical to virtual, and from then on use the entries with subscript one".
This would only make sense if QemuFlashFvbServicesRuntimeDxe ever needed
the original (physical) addresses (ie. the entries with subscript zero)
after the virtual address change, but that is not the case.
Replace the arrays with single elements. The subscript zero elements
simply disappear, and the single elements take the role of the prior
subscript one elements.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18670 6f19259b-4bc3-4df7-8a09-765794883524
The ESAL_FWB_GLOBAL.FvbScratchSpace array is never initialized (it
contains garbage from AllocateRuntimePool()). Its element at subscript one
(=FVB_VIRTUAL), containing garbage as well, is converted to virtual
mapping. Then the array is never used again.
Remove it.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18669 6f19259b-4bc3-4df7-8a09-765794883524
The EFI_FW_VOL_INSTANCE.FvbDevLock member is initialized and then never
used. Remove it.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18668 6f19259b-4bc3-4df7-8a09-765794883524
We build this driver for X64 as well -- the comment isn't overly
important, but it shouldn't be misleading.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18667 6f19259b-4bc3-4df7-8a09-765794883524
Some of the line lengths in this driver are atrocious. While we have to
put up with the status quo outside of OvmfPkg, we can at least rewrap this
driver before refactoring it.
In the FvbInitialize() function there's no way around introducing two
local variables, just for the sake of sensibly rewrapping the code.
Furthermore, in "FwBlockService.c" the function comment blocks are now
indented; their original position causes diff to print bogus function
names at the top of hunks.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18666 6f19259b-4bc3-4df7-8a09-765794883524
The current types of subexpressions used in QemuFlashPtr() are as follows.
(We also show the types of "larger" subexpressions, according to operator
binding.)
mFlashBase + (Lba * mFdBlockSize) + Offset
^ ^ ^ ^
| | | |
(UINT8*) EFI_LBA UINTN UINTN
(UINT64)
--------------------------------- ------
(UINT8*) UINTN
------------------------------------------
(UINT8*)
When building with VS2010 for Ia32 / NOOPT, the 64-by-32 bit
multiplication is translated to an intrinsic, which is not allowed in
edk2.
Recognize that "Lba" is always bounded by "mFdBlockCount" (an UINTN) here
-- all callers of QemuFlashPtr() ensure that. In addition, the flash chip
in question is always under 4GB, which is why we can address it at all on
Ia32. Narrow "Lba" to UINTN, without any loss of range.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan <scott@notabs.org>
[commit message by lersek@redhat.com]
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Build-tested-by: Scott Duplichan <scott@notabs.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16384 6f19259b-4bc3-4df7-8a09-765794883524
In the InitializeVariableFvHeader() function, all three of "Offset",
"Start" and "BlockSize" have type UINTN. Therefore the (Offset /
BlockSize) and (Start / BlockSize) divisions can be compiled on all
platforms without intrinsics.
In the current expressions
(EFI_LBA) Offset / BlockSize
(EFI_LBA) Start / BlockSize
"Offset" and "Start" are cast to UINT64 (== EFI_LBA), which leads to
64-by-32 bit divisions on Ia32, breaking the VS2010 / NOOPT / Ia32 build.
The simplest way to fix them is to realize we don't need casts at all.
(The prototypes of QemuFlashEraseBlock() and QemuFlashWrite() are visible
via "QemuFlash.h", and they will easily take our UINTN quotients as
UINT64.)
Suggested-by: Scott Duplichan <scott@notabs.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Build-tested-by: Scott Duplichan <scott@notabs.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16383 6f19259b-4bc3-4df7-8a09-765794883524
The MarkMemoryRangeForRuntimeAccess() function passes the Length parameter
(of type UINT64) to the macro EFI_SIZE_TO_PAGES(). When building for the
Ia32 platform, this violates the interface contract of the macro:
[...] Passing in a parameter that is larger than UINTN may produce
unexpected results.
In addition, it trips up compilation by VS2010 for the Ia32 platform and
the NOOPT target -- it generates calls to intrinsics, which are not
allowed in edk2.
Fix both issues with the following steps:
(1) Demote the Length parameter of MarkMemoryRangeForRuntimeAccess() to
UINTN. Even a UINT32 value is plenty for representing the size of the
flash chip holding the variable store. Length parameter is used in the
following contexts:
- passed to gDS->RemoveMemorySpace() -- takes an UINT64
- passed to gDS->AddMemorySpace() -- ditto
- passed to EFI_SIZE_TO_PAGES() -- requires an UINTN. This also guarantees
that the return type of EFI_SIZE_TO_PAGES() will be UINTN, hence we can
drop the outer cast.
(2) The only caller of MarkMemoryRangeForRuntimeAccess() is
FvbInitialize(). The latter function populates the local Length variable
(passed to MarkMemoryRangeForRuntimeAccess()) from
PcdGet32(PcdOvmfFirmwareFdSize). Therefore we can simply demote the local
variable to UINTN in this function as well.
- There's only one other use of Length in FvbInitialize(): it is passed to
GetFvbInfo(). GetFvbInfo() takes an UINT64, so passing an UINTN is fine.
Suggested-by: Scott Duplichan <scott@notabs.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Build-tested-by: Scott Duplichan <scott@notabs.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16382 6f19259b-4bc3-4df7-8a09-765794883524
Lack of these braces causes build errors when -Wno-missing-braces is
absent. Spelling out more braces also helps understanding the code.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@15586 6f19259b-4bc3-4df7-8a09-765794883524
When QemuFlashWrite() is asked to write a range that includes the last
byte of the LBA, then the byte that the function uses to switch the flash
device back to read mode (ROMD mode in KVM speak) actually falls out of
the LBA.
Normally this doesn't cause visible problems. However, if the variable
store and the firmware code are backed by separate flash devices, as
implemented by
[Qemu-devel] [PATCH v2] hw/i386/pc_sysfw: support two flash drives
http://thread.gmane.org/gmane.comp.emulators.qemu/243678
plus
[edk2] [edk2 PATCH] OvmfPkg: split the variable store to a separate file
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/5045/focus=5046
then the READ_ARRAY_CMD not only reaches a different LBA, it reaches a
different qemu device. This results in a guest reboot soon after.
Fix this by ensuring that we always stay within the LBA just written when
issuing READ_ARRAY_CMD.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@14996 6f19259b-4bc3-4df7-8a09-765794883524
PcdFlashNvStorageVariableBase64 is used to arbitrate between
QemuFlashFvbServicesRuntimeDxe and EmuVariableFvbRuntimeDxe, but even the
latter driver sets it if we fall back to it.
Allow code running later than the startup of these drivers to know about
the availability of flash variables, through a dedicated PCD.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@14843 6f19259b-4bc3-4df7-8a09-765794883524
If QEMU flash is detected, this module will install
FirmwareVolumeBlock support for the QEMU flash device.
It will also set PCDs with the results that:
1. OvmfPkg/EmuVariableFvbRuntimeDxe will be disabled
2. MdeModulePkg variable services will read/write flash directly
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@14839 6f19259b-4bc3-4df7-8a09-765794883524