In the expression
(RemainderByMediaBlockSize != 0 ||
Media->BlockSize > UDF_LOGICAL_SECTOR_SIZE)
the second expression is only evaluated if the first expression is false.
If the first expression is false, i.e.,
RemainderByMediaBlockSize == 0
then UDF_LOGICAL_SECTOR_SIZE is a whole multiple of "Media->BlockSize",
which implies
UDF_LOGICAL_SECTOR_SIZE >= Media->BlockSize.
Therefore whenever
Media->BlockSize > UDF_LOGICAL_SECTOR_SIZE
is evaluated, it is false.
The expression
((expression) || FALSE)
is equivalent to
(expression).
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
In edk2, the division and shifting of 64-bit values are forbidden with
C-language operators, because the compiler may generate intrinsic calls
for them.
For example, clang-3.8 emits a call to "__umoddi3" for
UDF_LOGICAL_SECTOR_SIZE % Media->BlockSize
in PartitionInstallUdfChildHandles(), if PartitionDxe is built for IA32,
which then fails to link.
UDF_LOGICAL_SECTOR_SIZE has type UINT64, while
EFI_BLOCK_IO_MEDIA.BlockSize has type UINT32(). Replace the % operator
with a DivU64x32Remainder() call.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
In edk2, initialization of local variables is forbidden, both for
stylistic reasons and because such initialization may generate calls to
compiler intrinsics.
For the following initialization in UdfRead():
CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
clang-3.8 generates a memset() call, when building UdfDxe for IA32, which
then fails to link.
Replace the initialization with ZeroMem().
Do the same to "FilePath" in UdfOpen().
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Ard reports that clang-3.8 correctly flags the following issue in the
ReadFile() function:
If "RecordingFlags" is INLINE_DATA, then there are three paths through the
code where we mean to return success, but forget to set Status
accordingly:
(1) when "ReadFileInfo->Flags" is READ_FILE_GET_FILESIZE, or
(2) when "ReadFileInfo->Flags" is READ_FILE_ALLOCATE_AND_READ and
AllocatePool() succeeds, or
(3) when "ReadFileInfo->Flags" is READ_FILE_SEEK_AND_READ.
Set "Status" to EFI_SUCCESS when we are done processing the INLINE_DATA
request, i.e., when we reach the corresponding "break" statament under the
INLINE_DATA case label.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
In the ReadFile() function, if "RecordingFlags" is INLINE_DATA, then we
cover the following values of "ReadFileInfo->Flags":
- READ_FILE_GET_FILESIZE
- READ_FILE_ALLOCATE_AND_READ
- READ_FILE_SEEK_AND_READ
We don't do anything (just proceed to the end of the function) if
"ReadFileInfo->Flags" is anything else.
In reality the above three values cover the domain of the
UDF_READ_FILE_FLAGS enum type fully, and "ReadFileInfo->Flags" is only
ever set internally to UdfDxe. Therefore any other flag value would be a
bug in UdfDxe.
ASSERT() specifically that "ReadFileInfo->Flags" has been set correctly,
so that the reader is not left wondering what happens if none of the enum
constants match.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Initialize the array DescriptorLBAs[] after declaration to fix
non-constant aggregate initializer warning in VS tool chains.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Initialize the array DescriptorLBAs[] after declaration to fix
non-constant aggregate initializer warning in VS tool chains.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
V3: Remove one unnecessay type cast in patch 1.
Codes:
if (FilePosition + ExtentLength > ReadFileInfo->FilePosition) {
Offset = ReadFileInfo->FilePosition - FilePosition;
if (Offset < 0) {
Offset = -(Offset)
}
...
}
Offset is UINT64 can not < 0, so the code logic may have some issue.
and Offset = -(Offset) may build failure in some circumstance.
Previously type cast Offset to INT64 to fix build break. Now remove
the type cast. Then can to check the code logic later.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
If DAD failed, we print "It failed to set", print the IPv6
address and exit. We need a better error string
and also, after printing the IPv6 address, we need a new line
else the Shell> prompt will overwrite the IPv6 address. Fixed
these issues in this patch.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>
Reviewed-by: Sriram Subramanian <sriram-s@hpe.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
When creating the ISCSI string OpCode for IP_ADDRESS, the max length
should be IP(4)_STR_MAX_SIZE instead of IP(4)_MAX_SIZE.
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
When a UEFI_DRIVER attempts to open a protocol interface with BY_DRIVER
attribute that it already has open with BY_DRIVER attribute,
OpenProtocol() returns EFI_ALREADY_STARTED. This is not an error. The
UEFI-2.7 spec currently says,
> EFI_ALREADY_STARTED -- Attributes is BY_DRIVER and there is an item on
> the open list with an attribute of BY_DRIVER
> whose agent handle is the same as AgentHandle.
(In fact it is so much an expected condition that recent USWG Mantis
ticket <https://mantis.uefi.org/mantis/view.php?id=1815> will codify its
additional edk2-specific behavior, namely to output the protocol interface
at once.)
Downgrade the log mask for this one condition to DEBUG_INFO, in
SataControllerStart(). This will match the log mask of the other two
informative messages in this function, "SataControllerStart START", and
"SataControllerStart END status = %r" (at which point Status can only be
EFI_SUCCESS).
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
"Boot Mode:%x" is an informative message, not an error report. Set its
debug mask to DEBUG_INFO.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
"Platform PEIM Loaded" is an informative message, not an error report. Set
its debug mask to DEBUG_INFO.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
"Detect CPU count: %d\n" is an informative message, not an error report.
Set its debug mask to DEBUG_INFO.
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
"UsbSelectConfig: failed to connect driver %r, ignored" is an error
message, but it states at once that the error condition will not affect
the control flow. Degrade the report to DEBUG_WARN.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Since commit 19c6d9feaa ("MdePkg: Expand BaseIoLibIntrinsic (IoLib
class) library", 2017-01-14), IoWriteFifo8() has been widely available to
modules. Use it to print debug messages and assertion failures to the QEMU
debug port, rather than open-coded loops.
In the general case this speeds up logging, because debug messages will
now trap to QEMU once per message (as opposed to once per character), due
to "REP OUTSB" in "MdePkg/Library/BaseIoLibIntrinsic/*/IoFifoSev.nasm".
In SEV guests, there is no speedup (SEV doesn't support the REP prefix).
SEV is detected internally to BaseIoLibIntrinsic.
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Brijesh Singh <brijesh.singh@amd.com>
In commit b6d11d7c46 ("MdePkg: BaseIoLibIntrinsic (IoLib class)
library", 2017-04-12), the MOV instructions in the write loops were
probably copied from the read loops. However, the operand order was not
adjusted.
As a result, the IoWriteFifoXX() routines, when invoked in SEV guests, now
overwrite the source buffer with value 0x01 / 0x0001 / 0x00000001 -- the
SevNoRepIo() function returns value 1 in EAX, in SEV guests --, and write
the same value to the target IO port.
Fix this by putting the target operand (AL / AX / EAX) first, and the
source operand (BYTE / WORD / DWORD [ESI/RSI]) second.
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Fixes: b6d11d7c46
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
the case is that override the mixed pcd value in DSC [Components]
section, the value display in the report is incorrect.
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=653
Correct description of Timeout param in XhciReg.h to be matched with
XhciReg.c.
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Per build spec, If the value obtained from either a build option, the
DSC or FDF is the same as the value in the DEC, then *B , *P or *F
will not be shown in the report.
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
This patch fixed the bug that same region print twice in the build log.
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Not generic to use 'Common' arch, but use current build arch.
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
TianoCore BZ#700 [1]
Set the '-Wno-unused-const-variables' in RELEASE builds with the
GGC49 and GCC5 toolchain.
This fixes the RELEASE build of OVMF with GCC in version 6 or newer.
GCC 6 added the '-Wunused-const-variable' warning, which gets
activated by '-Wunused-variable' and has the following behavior:
"Warn whenever a constant static variable is unused aside from its
declaration" [2]
Commit 2ad6ba80a1 introduced a case
where exactly this happens on a RELEASE build. All uses of the static
const variable are located in debug code only, which gets thrown out
by the compiler on RELEASE builds and thus triggers the
unused-const-variable warning.
There is currently no GCC 6 toolchain target defined and doing so
would add a lot of boilerplate code. Instead, use the fact that GCC
ignores unknown '-Wno-*' options:
"[...] if the -Wno- form is used [...] no diagnostic is produced for
-Wno-unknown-warning unless other diagnostics are being produced"
This behavior is available in GCC 4.9 [3] (and also earlier, for that
matter), so add the flag to the GCC49 and GCC5 toolchain, even if
both GCC versions do not supports it.
GCC49 doesn't enables LTO whereas GCC5 does. As GCC 6.0 through 6.2
had bugs relating to LTO there can be desire to use the GCC49 target
even if compiling with GCC 6, see 432f1d83f7.
Orient the changes on 20d00edf21 which moved the
'-Wno-unused-but-set-variable' flag to RELEASE builds only, as there
it ensure that it does not gets raised if the only usage of a
variable is in (then collapsed) debug code.
[1] https://bugzilla.tianocore.org/show_bug.cgi?id=700
[2] https://gcc.gnu.org/onlinedocs/gcc-6.4.0/gcc/Warning-Options.html#index-Wunused-const-variable
[3] https://gcc.gnu.org/onlinedocs/gcc-4.9.0/gcc/Warning-Options.html
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Liming Gao <liming.gao@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: Thomas Lamprecht <t.lamprecht@proxmox.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Liming Gao <liming.gao@intel.com>
[lersek@redhat.com: fix typo in subject]
This patch enables UDF file system support by default.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
This patch enables UDF file system support by default.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
This patch introduces UDF file system support in EDK2. All block devices
that support BlockIo and DiskIo protocols and contain a valid UDF file
system - as specified by OSTA Universal Disk Format (revisions 1.02
through 2.60) - will be installed EFI_SIMPLE_FILE_SYSTEM_PROTOCOL to
provide access to underlying file system.
File system operations on regular, directory and symlink files are
supported.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Scan for UDF file systems on all block devices, as specified by OSTA
Universal Disk Format Specification 2.60, and install a Vendor-Defined
Media Device Path for each file system found.
The Vendor-Defined Media Device Path for the UDF file system is then
checked by UdfDxe to decide whether or not start the driver.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
This patch adds basic volume structure definitions necessary to identify
a valid UDF file system on a block device, as specified by OSTA
Universal Disk Format Specification 2.60.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Register an ExitBootServices() callback that tears down all IOMMU
mappings, without modifying the UEFI memory map.
The trick is that in the ExitBootServices() callback, we don't immediately
do the work; instead we signal another (private) event.
Normally the dispatch order of ExitBootServices() callbacks is unspecified
(within the same task priority level anyway). By queueing another
function, we delay the unmapping until after all PciIo and Virtio drivers
abort -- in their own ExitBootServices() callbacks -- the pending DMA
operations of their respective controllers.
Furthermore, the fact that IoMmuUnmapWorker() rewrites client-owned memory
when it unmaps a Write or CommonBuffer bus master operation, is safe even
in this context. The existence of any given "MapInfo" in "mMapInfos"
implies that the client buffer pointed-to by "MapInfo->CryptedAddress" was
live when ExitBootServices() was entered. And, after entering
ExitBootServices(), nothing must have changed the UEFI memory map, hence
the client buffer at "MapInfo->CryptedAddress" still exists.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
IoMmuUnmapWorker() is identical to IoMmuUnmap(), it just takes an
additional BOOLEAN parameter called "MemoryMapLocked". If the memory map
is locked, IoMmuUnmapWorker() does its usual job, but it purposely leaks
memory rather than freeing it. This makes it callable from
ExitBootServices() context.
Turn IoMmuUnmap() into a thin wrapper around IoMmuUnmapWorker() that
passes constant FALSE for "MemoryMapLocked".
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
The "mRecycledMapInfos" list implements an internal pool of unused
MAP_INFO structures between the IoMmuUnmap() and IoMmuMap() functions. The
original goal was to allow IoMmuUnmap() to tear down CommonBuffer mappings
without releasing any memory: IoMmuUnmap() would recycle the MAP_INFO
structure to the list, and IoMmuMap() would always check the list first,
before allocating a brand new MAP_INFO structure.
In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it
unmaps all existent bus master operations (CommonBuffer, Read, Write) at
ExitBootServices(), strictly after the individual device drivers abort
pending DMA on the devices they manage, in their own ExitBootServices()
notification functions.
For this, rename and repurpose the list to track all live mappings.
This means that IoMmuUnmap() will always release a MAP_INFO structure
(even when cleaning up a CommonBuffer operation). That's fine (for now),
because device drivers are no longer expected to call Unmap() in their
ExitBootServices() notification functions.
In theory, we could also move the allocation and freeing of the stash
buffer from IoMmuAllocateBuffer() and IoMmuFreeBuffer(), respectively, to
IoMmuMap() and IoMmuUnmap(). However, this would require allocating and
freeing a stash buffer in *both* IoMmuMap() and IoMmuUnmap(), as
IoMmuMap() performs in-place decryption for CommonBuffer operations, and
IoMmuUnmap() performs in-place encryption for the same.
By keeping the stash buffer allocation as-is, not only do we keep the code
almost fully undisturbed, but
- we also continue to guarantee that IoMmuUnmap() succeeds: allocating a
stash buffer in IoMmuUnmap(), for in-place encryption after a
CommonBuffer operation, could fail;
- we also keep IoMmuUnmap() largely reusable for ExitBootServices()
callback context: allocating a stash buffer in IoMmuUnmap() would simply
be forbidden in that context.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it
unmaps all existent bus master operations (CommonBuffer, Read, Write) at
ExitBootServices(), strictly after the individual device drivers abort
pending DMA on the devices they manage, in their own ExitBootServices()
notification functions.
In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() call from VirtioScsiExitBoot(),
originally added in commit fc2168feb2 ("OvmfPkg/VirtioScsiDxe: map VRING
using VirtioRingMap()", 2017-08-31).
Add a DEBUG message so we can observe the ordering between
VirtioScsiExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it
unmaps all existent bus master operations (CommonBuffer, Read, Write) at
ExitBootServices(), strictly after the individual device drivers abort
pending DMA on the devices they manage, in their own ExitBootServices()
notification functions.
In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() call from VirtioRngExitBoot(),
originally added in commit 0a568ccbcb ("OvmfPkg/VirtioRngDxe: map host
address to device address", 2017-08-23).
Add a DEBUG message so we can observe the ordering between
VirtioRngExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it
unmaps all existent bus master operations (CommonBuffer, Read, Write) at
ExitBootServices(), strictly after the individual device drivers abort
pending DMA on the devices they manage, in their own ExitBootServices()
notification functions.
In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() calls from VirtioGpuExitBoot(),
originally added in commit 9bc5026c19 ("OvmfPkg/VirtioGpuDxe: map VRING
for bus master common buffer operation", 2017-08-26) and commit
f10ae92366 ("OvmfPkg/VirtioGpuDxe: map backing store to bus master
device address", 2017-08-26).
Add a DEBUG message so we can observe the ordering between
VirtioGpuExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it
unmaps all existent bus master operations (CommonBuffer, Read, Write) at
ExitBootServices(), strictly after the individual device drivers abort
pending DMA on the devices they manage, in their own ExitBootServices()
notification functions.
In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() call from VirtioBlkExitBoot(),
originally added in commit 1916513047 ("OvmfPkg/VirtioBlkDxe: map VRING
using VirtioRingMap()", 2017-08-27).
Add a DEBUG message so we can observe the ordering between
VirtioBlkExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
The AtaAtapiPassThru driver maps three system memory regions for Bus
Master Common Buffer operation on the following call path, if the
controller has PCI_CLASS_MASS_STORAGE_SATADPA class code:
AtaAtapiPassThruStart()
EnumerateAttachedDevice()
AhciModeInitialization()
AhciCreateTransferDescriptor()
The device is disabled (including Bus Master DMA) when the controller is
unbound, in AtaAtapiPassThruStop(). Then the regions are unmapped.
The former step should also be done when we exit the boot services, and
the OS gains ownership of system memory.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
In AtaAtapiPassThruStop(), if the device has been operating in AHCI mode,
we unmap the DMA buffers and then disable the device (including bus master
DMA). The order of these actions is wrong; we shouldn't unmap DMA buffers
until bus master DMA is turned off. Reverse the steps.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Both AtaAtapiPassThruStart() and AtaAtapiPassThruStop() fetch the
supported attributes of the device, just so they can toggle the
IO+MMIO+BusMaster subset.
After we compute this bitmask in AtaAtapiPassThruStart(), we can cache it
for later, and save the fetch in AtaAtapiPassThruStop().
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
For setting high speed in SD card,
First CMD 6 (Switch) is send to check if card supports High Speed and
Second command is send to switch card to high speed mode.
In current inplementation, CMD 6 was sent only once to switch the
card into HS mode without checking if card supports HS or not, which is
not as per specification and also we are not setting the HS i.e. 50000000
but directly asking the card to switch to 26000000 which is incorrect as
SD card supports either 25000000 or 50000000.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
Reviewed-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
We want to print name of the function resulted in error,
but __FUNCTION__ macro was missing.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
BIT0: This is to control if a platform wants to enable VTd
based protection during boot.
BIT1: This is to control if a platform wants to keep VTd
enabled at ExitBootService.
The default configuration is BIT0:1, BIT1:0.
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
In earlier PEI stage, temporary memory at PcdOvmfSecPeiTempRamBase is
employed as stack and heap. We move them to the new room and do some
relocation fixup when permanent memory becomes available.
TemporaryRamMigration() is responsible for switching the stack.
Before entering TemporaryRamMigration(), Ebp/Rbp is populated with the
content of Esp/Rsp and used as frame pointer.
After the execution of SetJump/LongJump, stack migrates to new position
while the context keeps unchanged.
But when TemporaryRamMigration() exits, Esp/Rsp is filled with
the content of Ebp/Rbp to destroy this stack frame.
The result is, stack switches back to previous temporary memory.
When permanent memory becomes available, modules that have registered
themselves for shadowing will be scheduled to execute. Some of them
need to consume more memory(heap/stack). Contrast to temporary stack,
permanent stack possesses larger space.
The potential risk is overflowing the stack if stack staying in
temporary memory. When it happens, system may crash during S3 resume.
More detailed information:
> (gdb) disassemble /r
> Dump of assembler code for function TemporaryRamMigration:
> 0x00000000fffcd29c <+0>: 55 push %rbp
> 0x00000000fffcd29d <+1>: 48 89 e5 mov %rsp,%rbp
> 0x00000000fffcd2a0 <+4>: 48 81 ec 70 01 00 00 sub
> $0x170,%rsp
> ...
> ...
> 0x00000000fffcd425 <+393>: e8 80 10 00 00 callq 0xfffce4aa
> <SaveAndSetDebugTimerInterrupt>
> => 0x00000000fffcd42a <+398>: b8 00 00 00 00 mov $0x0,%eax
> 0x00000000fffcd42f <+403>: c9 leaveq
> 0x00000000fffcd430 <+404>: c3 retq
> End of assembler dump.
See the description of leave(opcode: c9), from
Intel 64 and IA-32 Architectures Software Developer's Manual, Volume 2A
"Releases the stack frame set up by an earlier ENTER instruction. The
LEAVE instruction copies the frame pointer (in the EBP register) into
the stack pointer register (ESP), which releases the stack space
allocated to the stack frame. The old frame pointer (the frame pointer
for the calling procedure that was saved by the ENTER instruction) is
then popped from the stack into the EBP register, restoring the calling
procedure’s stack frame."
To solve this, update Ebp/Rbp too when Esp/Rsp is updated
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ge Song <ge.song@hxt-semitech.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Even though RELEASE builds produce some diagnostics when a crash
occurs, they can be rather unhelpful:
Synchronous Exception at 0x0000000000000000
and sometimes, it would be useful to get a full register dump from
a production machine without having to modify the firmware.
This can be achieved very easily by incorporating a DEBUG build of
ARM's DefaultExceptionHandlerLib into a DXE driver, and registering
its DefaultExceptionHandler entry point as the synchronous exception
handler, overriding the default one. If we then build this driver
using the UefiDebugLibConOut DebugLib implementation, we end up
with a module than can simply be loaded via the Shell on any system.
Shell> load fs0:ArmCrashDumpDxe.efi
As a bonus, the crash dump will also appear on the graphical display,
not only on the serial port.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
The IP driver uses EfiCreateProtocolNotifyEvent() to register notify callback
function for IpSec protocol, but it didn't notice that the callback will always
be executed at least once, even the protocol wasn't in handle database.
As a result, the Ip6IpSecProcessPacket() will still always call LocateProtocol()
even the IpSec protocol is not installed, which will impact the network
performance.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
The IP driver uses EfiCreateProtocolNotifyEvent() to register notify callback
function for IpSec protocol, but it didn't notice that the callback will always
be executed at least once, even the protocol wasn't in handle database.
As a result, the Ip4IpSecProcessPacket() will still always call LocateProtocol()
even the IpSec protocol is not installed, which will impact the network
performance.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
Laszlo Ersek
Dandan Bi
hegdenag
Jiaxin Wu
Yonghong Zhu
Star Zeng
Thomas Lamprecht
Paulo Alcantara
Liming Gao
Meenakshi Aggarwal
Jiewen Yao
Ge Song
Ard Biesheuvel
Fu Siyuan