Problem has been identified with Ip6ProcessRouterAdvertise() when
Router Advertise packet contains options with malicious/invalid
'Length' field. This can lead to platform entering infinite loop
when processing options from that packet.
Cc: Jiaxin Wu <jiaxin.wu@intel.com> Cc: Siyuan Fu <siyuan.fu@intel.com> Signed-off-by: Maciej Rabeda <maciej.rabeda@linux.intel.com> Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
Building OVMF for X64 with secure boot enabled on VS2019 results in
the following error:
d:\a\1\s\OvmfPkg\Library\GenericQemuLoadImageLib\GenericQemuLoadImageLib.c(154):
error C2220: the following warning is treated as an error
d:\a\1\s\OvmfPkg\Library\GenericQemuLoadImageLib\GenericQemuLoadImageLib.c(154):
warning C4244: '=': conversion from 'UINTN' to 'UINT32', possible loss of data
On enabling the /analyse option the VS2017 compiler
reports: warning C6001: Using uninitialized memory.
This warning is reported as some variables that were
being logged were uninitialised. To fix this, moved
the logging code after the variables being logged are
initialised.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
On enabling the /analyse option the VS2017 compiler
reports: warning C6001: Using uninitialized memory.
This warning is reported as some variables that were
being logged were uninitialised. To fix this, moved
the logging code after the variables being logged are
initialised.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Sami Mujawar [Tue, 9 Jul 2019 15:50:42 +0000 (16:50 +0100)]
DynamicTablesPkg: Fix IORT node length assignment
The VS2017 compiler reports 'warning C4267: 'return': conversion
from 'size_t' to 'UINT32', possible loss of data' for a number of
functions that compute the IORT node length. Similarly, it reports
warnings for IORT node length field assignments as the length
field is 16-bit wide.
This patch adds type casts at appropriate places and also implements
validations to ensure that the max width of the respective fields
is not exceeded.
This patch also fixes a typo in one of the local variable names.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Sami Mujawar [Tue, 9 Jul 2019 15:18:27 +0000 (16:18 +0100)]
DynamicTablesPkg: Serial debug port initialisation
The ARM DCC serial port subtype is an option that is
supported by the DBG2 generator. However, the serial
port initialisation should only be done for PL011/SBSA
compatible UARTs.
Add check to conditionally initialise the serial port.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Sami Mujawar [Tue, 9 Jul 2019 14:58:37 +0000 (15:58 +0100)]
DynamicTablesPkg: Fix unaligned pointers usage
The VS2017 compiler reports 'warning C4366: The result of
the unary '&' operator may be unaligned' if an address of
an unaligned structure member is passed as an argument to
a function.
Fix this warning by using local variables in place of
unaligned structure members.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Sami Mujawar [Tue, 9 Jul 2019 14:11:49 +0000 (15:11 +0100)]
DynamicTablesPkg: Fix ACPI table rev field width
The VS2017 compiler reports 'warning C4244: '=': conversion from
'const UINT32' to 'UINT8', possible loss of data' when the ACPI
table revision field is being updated.
The width of the revision field in the EFI_ACPI_DESCRIPTION_HEADER
struct is 8-bit wide. Therefore, to fix the above warning make the
ACPI Table revision field usage 8-bit wide across Dynamic Tables
Framework.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Sami Mujawar [Tue, 9 Jul 2019 14:04:45 +0000 (15:04 +0100)]
DynamicTablesPkg: Fix Boot arch flag width
The ArmBootArch field of the FADT table is 16-bit wide. The
VS2017 compiler reports 'warning C4244: '=': conversion from
'UINT32' to 'UINT16', possible loss of data' when assigning the
CM_ARM_BOOT_ARCH_INFO.BootArchFlags value as the width of this
field in CM_ARM_BOOT_ARCH_INFO is 32-bit wide.
To fix this warning, update the CM_ARM_BOOT_ARCH_INFO struct
to make the BootArchFlags field 16-bit wide. This also makes
it compatible with the ACPI FADT specification.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Sami Mujawar [Tue, 9 Jul 2019 12:54:04 +0000 (13:54 +0100)]
DynamicTablesPkg: Fix GT Block length assignment
The VS2017 compiler reports 'warning C4267: '=': conversion from
'size_t' to 'UINT16', possible loss of data'.
The sizeof() operator is used to calculate the size of the
GT Block structure. The length field in the GT Block structure
is 16-bit wide. Since the return type of sizeof() operator
is size_t the VS2017 compiler reports the above warning.
To fix the warning, an explicit type cast is added. An additional
check is also performed to ensure that the calculated GT Block
length does not exceed MAX_UINT16.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Sami Mujawar [Tue, 9 Jul 2019 12:35:02 +0000 (13:35 +0100)]
DynamicTablesPkg: Fix Proc node length assignment
The length field for the Processor Hierarchy node structure is
8-bit wide while the number of private resource field is 32-bit
wide. Therefore, the GetProcHierarchyNodeSize() returns the size
as a 32-bit value.
The VS2017 compiler reports 'warning C4244: '=': conversion from
'UINT32' to 'UINT8', possible loss of data' while assigning the
length field of the Processor Hierarchy node structure.
To fix this, a type cast is added. In addition, there is a check
to ensure that the Processor Hierarchy node size does not exceed
MAX_UINT8.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Sami Mujawar [Tue, 9 Jul 2019 10:56:14 +0000 (11:56 +0100)]
DynamicTablesPkg: Fix serial port subtype warning
The VS2017 compiler reports 'warning C4244: '=': conversion
from 'UINT16' to 'UINT8', possible loss of data' for the
SPCR InterfaceType field assignment.
The SPCR InterfaceType field uses the same encoding as that
of the DBG2 table Port Subtype field. However SPCR.InterfaceType
is 8-bit while the Port Subtype field in DBG2 table is 16-bit.
Since the Configuration Manager represents the Serial port
information using the struct CM_ARM_SERIAL_PORT_INFO, the
PortSubtype member in this struct is 16-bit.
To fix the warning an explicit type case is added. A validation
is also added to ensure that the Serial Port Subtype value
provided by the Configuration Manager is within the 8-bit
range (less than 256).
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Sami Mujawar [Tue, 9 Jul 2019 11:14:24 +0000 (12:14 +0100)]
DynamicTablesPkg: Fix entry point param definition
VS2017 reports 'warning C4028: formal parameter 2 different
from declaration' for the library constructor and destructor
interfaces for the Generator modules. VS2017 compiler also
reports similar warnings for the DXE entry points.
Remove the CONST qualifier for the SystemTable pointer (the
second parameter to the constructor/destructor/DXE Entry
point) to make it compatible with the formal declaration.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
RpmcLib.h and VariableKeyLib.h are header files required to access RPMC
device and Key generator from platform. They will be used to ensure the
integrity and confidentiality of NV variables.
Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Cc: Nishant C Mistry <nishant.c.mistry@intel.com> Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
The UNIT_TEST_STATUS and FAILURE_TYPE have used 0 as status, so use 0 as
unknown is confused, remove it from array enumeration but keep it
location in the array.
Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Signed-off-by: Guomin Jiang <guomin.jiang@intel.com> Reviewed-by: Shenglei Zhang <shenglei.zhang@intel.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> Reviewed-by: Bret Barkelew <bret.barkelew@microsoft.com>
Token pointer may be NULL, it should be checked before use it.
Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Ray Ni <ray.ni@intel.com> Signed-off-by: Guomin Jiang <guomin.jiang@intel.com> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
The commit will introduce a static PCD to specify the periodic interval
for checking the AP status when MP services StartupAllAPs() and
StartupThisAP() are being executed in a non-blocking manner. Or in other
words, specifies the interval for callback function CheckApsStatus().
The purpose is to provide the platform owners with the ability to choose
the proper interval value to trigger CheckApsStatus() according to:
A) The number of processors in the system;
B) How MP services (StartupAllAPs & StartupThisAP) being used.
Setting the PCD to a small value means the AP status check callback will
be triggered more frequently, it can benefit the performance for the case
when the BSP uses WaitForEvent() or uses CheckEvent() in a loop to wait
for AP(s) to complete the task, especially when the task can be finished
considerably fast on AP(s).
An example is within function CpuFeaturesInitialize() under
UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c,
where BSP will perform the same task with APs and requires all the
processors to finish the task before BSP proceeds to its next task.
Setting the PCD to a big value, on the other hand, can reduce the impact
on BSP by the time being consumed in CheckApsStatus(), especially when the
number of processors is huge so that the time consumed in CheckApsStatus()
is not negligible.
The type of the PCD is UINT32, which means the maximum possible interval
value can be set to:
4,294,967,295 microseconds = 4,295 seconds = 71.58 minutes = 1.19 hours
which should be sufficient for usage.
For least impact, the default value of the new PCD will be the same with
the current interval value. It will be set to 100,000 microseconds, which
is 100 milliseconds.
Unitest done:
A) OS boot successfully;
B) Use debug message to confirm the 'TriggerTime' parameter for the
'SetTimer' service is the same before & after this patch.
Cc: Eric Dong <eric.dong@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Brian J. Johnson <brian.johnson@hpe.com> Signed-off-by: Hao A Wu <hao.a.wu@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Ard Biesheuvel [Sat, 7 Mar 2020 12:35:55 +0000 (13:35 +0100)]
ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table entry
Currently, depending on the size of the region being (re)mapped, the
page table manipulation code may replace a table entry with a block entry,
even if the existing table entry uses different mapping attributes to
describe different parts of the region it covers. This is undesirable, and
instead, we should avoid doing so unless we are disregarding the original
attributes anyway. And if we make such a replacement, we should free all
the page tables that have become orphaned in the process.
So let's implement this, by taking the table entry path through the code
for block sized regions if a table entry already exists, and the clear
mask is set (which means we are preserving attributes from the existing
mapping). And when we do replace a table entry with a block entry, free
all the pages that are no longer referenced.
Ard Biesheuvel [Sat, 7 Mar 2020 11:44:16 +0000 (12:44 +0100)]
ArmPkg/ArmMmuLib AARCH64: use helpers to determine table entry types
Given how the meaning of the attribute bits for page table entry types
is slightly awkward, and changes between levels, add some helpers to
abstract from this.
Ard Biesheuvel [Wed, 25 Mar 2020 14:54:09 +0000 (15:54 +0100)]
ArmPkg/ArmMmuLib AARCH64: limit recursion when freeing page tables
FreePageTablesRecursive () traverses the page table tree depth first
to free all pages that it finds, without taking into account the
level at which it is operating.
Since TT_TYPE_TABLE_ENTRY aliases TT_TYPE_BLOCK_ENTRY_LEVEL3, we cannot
distinguish table entries from block entries unless we take the level
into account, and so we may be dereferencing garbage if we happen to
try and free a hierarchy of page tables that has level 3 pages in it.
Let's fix this by passing the level into FreePageTablesRecursive (),
and limit the recursion to levels < 3.
Ard Biesheuvel [Wed, 25 Mar 2020 09:30:07 +0000 (10:30 +0100)]
ArmVirtPkg/PlatformPeiLib: add dummy assignment to work around older GCC
Older GCC (<= 4.9) fail to infer that Parent is never used unless it
has been assigned before, and may throw an error like
/work/git/edk2/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c:
In function ‘PlatformPeim’:
/work/git/edk2/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c:132:24:
error: ‘Parent’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
RangesProp = fdt_getprop (Base, Parent, "ranges", &RangesLen);
Set Parent to 0 at the start of the sequence to work around this.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2601 Fixes: 82662a3b5f56e974 ("ArmVirtPkg/PlatformPeiLib: discover the TPM base ...") Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Ard Biesheuvel [Wed, 25 Mar 2020 09:07:43 +0000 (10:07 +0100)]
OvmfPkg/X86QemuLoadImageLib: add dummy assignment to work around GCC
GCC 4.8 or 4.9 may throw the following error when building OVMF:
Edk2/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c:
In function ‘QemuLoadKernelImage’:
Edk2/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c:416:30:
error: ‘CommandLine’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
UnicodeSPrintAsciiFormat (
cc1: all warnings being treated as errors
This is due to the fact that older GCCs fail to infer that CommandLine is
never actually used unless it has been assigned. So add a redundant NULL
assignment to help these older GCCs understand this.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2630 Fixes: 7c47d89003a6f ("OvmfPkg: implement QEMU loader library for X86 with ...") Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
The following components are currently missing from the [Components]
section of ArmPlatformPkg.dsc:
* ArmPlatformPkg/Library/HdLcd/HdLcd.inf
* ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.inf
This commit includes the components in the package DSC build.
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
The following components are currently missing from the [Components]
section of ArmPkg.dsc:
* ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
* ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
* ArmPkg/Library/ArmMtlNullLib/ArmMtlNullLib.inf
* ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
This commit includes the components in the package DSC build.
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
We meet a case that the DEC file declaring the PCD isn't
included in the INF.it cause build tools report Traceback error.
Remove raise statements that generate Tracebacks that were only
intended for development/debug. With the raise statements removed
proper error messages are shown.
Cc: Bob Feng <bob.c.feng@intel.com> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com> Reviewed-by: Bob Feng <bob.c.feng@intel.com>
This patch is to fix a build tool regression issue which was introduced
by commit 8ddec24dea74.
compiler output message includes localized string.
So build failed when code decode the stdout/stderr byte arrays.
The cause of the build failed is that Commit 8ddec24dea74
removed "errors='ignore'".
The build tool does not need to deal with localized string,
so we need to add "errors='ignore'".
this function is only invoked for structure PCDs.
Build failed if structurePcd is used in platform dsc file.
The patch is going to fixed this issue
Cc: Liming Gao <liming.gao@intel.com> Cc: Bob Feng <bob.c.feng@intel.com> Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com> Reviewed-by: Bob Feng <bob.c.feng@intel.com>
Leif Lindholm [Wed, 11 Mar 2020 15:10:18 +0000 (15:10 +0000)]
ArmVirtPkg: fix ASSERT in ArmVirtGicArchLib with virtualization=on
ArmVirtGicArchLib was originally implemented before virtualization
emulation was implemented in QEMU, and the GICv2 model implemented only
the physical copy of control registers.
Enabling virtualization emulation to QEMU adds also the virtual copy,
doubling the RegSize returned by FindCompatibleNodeReg () in
ArmVirtGicArchLibConstructor (). This triggered an ASSERT when running
QEMU with -M virt,virtualization=on. Address this by testing for both
possible valid values of RegSize.
Liming Gao [Thu, 12 Mar 2020 04:30:08 +0000 (12:30 +0800)]
OvmfPkg: Fix build failure with VS2015 tool chain
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2582
warning C4244: '=': conversion from 'UINTN' to 'UINT32', possible loss of data
With this fix, OvmfIa32, OvmfX64 and OvmfIa32X64 can pass build.
Add a code comment that explains the nature of the NumberOfPages field
values. Including this kind of historical information was suggested by
Leif in <https://edk2.groups.io/g/devel/message/55797> (alternative link:
<http://mid.mail-archive.com/20200312104006.GB23627@bivouac.eciton.net>).
Right now, the most recent commit updating the page counts has been commit 991d95636264 ("[...] Update default memory type information to reduce EFI
Memory Map fragmentation.", 2010-07-16).
bullet#3 in section "Assumption and Recommendation", and bullet#4 in "Call
for action", recommend enabling the (adaptive) Memory Type Information
feature.
* In the Intel whitepaper:
--v--
A Tour Beyond BIOS -- Memory Map and Practices in UEFI BIOS
figure#6 describes the Memory Type Information feature in detail; namely
as a feedback loop between the Platform PEIM, the DXE IPL PEIM, the DXE
Core, and BDS.
Implement the missing PlatformPei functionality in OvmfPkg, for fulfilling
the Secure SMM Communication recommendation.
In the longer term, OVMF should install the WSMT ACPI table, and this
patch contributes to that.
Notes:
- the step in figure#6 where the UEFI variable is copied into the HOB is
covered by the DXE IPL PEIM, in the DxeLoadCore() function,
- "PcdResetOnMemoryTypeInformationChange" must be reverted to the DEC
default TRUE value, because both whitepapers indicate that BDS needs to
reset the system if the Memory Type Information changes.
Due to the previous patches in this series, the above PCDs are available
in the PEI phase, in the SMM_REQUIRE build.
FaultTolerantWritePei produces a GUID-ed HOB with
FAULT_TOLERANT_WRITE_LAST_WRITE_DATA as contents. It also installs a Null
PPI that carries the same gEdkiiFaultTolerantWriteGuid as the HOB.
VariablePei depends on the Null PPI mentioned above with a DEPEX, consumes
the HOB (which is safe due to the DEPEX), and produces
EFI_PEI_READ_ONLY_VARIABLE2_PPI.
This enables read-only access to non-volatile UEFI variables in the PEI
phase, in the SMM_REQUIRE build.
For now, the DxeLoadCore() function in
"MdeModulePkg/Core/DxeIplPeim/DxeLoad.c" will not access the
"MemoryTypeInformation" variable, because OVMF's PlatformPei always
produces the MemoryTypeInformation HOB.
(Note: when the boot mode is BOOT_ON_S3_RESUME, PlatformPei doesn't build
the HOB, but that's in sync with DxeLoadCore() also not looking for either
the HOB or the UEFI variable.)
are always set to constant (invariable) values in the "-D SMM_REQUIRE"
build of OVMF. (That's because in the SMM build, actual pflash is a hard
requirement, and the RAM-based emulation is never available.)
Set said PCDs statically, at build. This will allow us to depend on their
values in the PEI phase.
When SMM_REQUIRE is FALSE, this change has no effect (confirmed by report
file comparison).
When SMM_REQUIRE is TRUE, the report file shows the following changes:
- "PcdOvmfFlashNvStorageFtwSpareBase" and
"PcdOvmfFlashNvStorageFtwWorkingBase" are no longer consumed by any
module directly,
- for "PcdFlashNvStorageFtwSpareBase", "PcdFlashNvStorageFtwWorkingBase"
and "PcdFlashNvStorageVariableBase64", the access method changes from
DYN to FIXED,
- for the latter PCDs, the zero (dynamic default) values are replaced with
the desired constants.
Laszlo Ersek [Tue, 10 Mar 2020 22:27:36 +0000 (23:27 +0100)]
OvmfPkg/QemuFlashFvbServices: factor out SetPcdFlashNvStorageBaseAddresses
Extract the dynamic setting of the
- PcdFlashNvStorageVariableBase64
- PcdFlashNvStorageFtwWorkingBase
- PcdFlashNvStorageFtwSpareBase
addresses to a helper function.
For now, the helper function is identical (duplicated) between the SMM
flash driver and the runtime DXE flash driver. In subsequent patches, this
will change.
Laszlo Ersek [Tue, 10 Mar 2020 22:27:35 +0000 (23:27 +0100)]
OvmfPkg/QemuFlashFvbServicesRuntimeDxe: drop unused PCDs
The only two OvmfPkg references to "PcdFlashNvStorageVariableBase" are the
spurious ones in the runtime DXE driver and the SMM driver INF files of
the QEMU flash driver. Remove these references.
The flash driver does not access "PcdOvmfFlashNvStorageEventLogBase"
either, so remove that from the INF files too.
From the function description of GetIfrBinaryData(), FormSetGuid can be
NULL. However, FormSetGuid is passed to IsZeroGuid(). This causes exception
when FormSetGuid is NULL.
Signed-off-by: Nickle Wang <nickle.wang@hpe.com> Reviewed-by: Dandan Bi <dandan.bi@intel.com>
Ovmf build failed on Windows with VS2017 tool chain.
The error message like:
OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitr
dDynamicShellCommand.c(199): error C2220: warning treated as error -
no 'object' file generated
OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitrdDynamicShellCommand.c(199):
warning C4244: '=': conversion from 'UINT64' to 'UINTN',
possible loss of data
This patch is to cast UINT64 type to UINTN type
when doing the variable assignment.
PcdValueInit shares the same Edk2\BaseTools\Source\C\PcdValueCommon.c.
To avoid the conflict, it should copy this file to its output directory,
If so, PcdValueCommon.c file will be private for PcdValueInit
Cc: Liming Gao <liming.gao@intel.com> Cc: Bob Feng <bob.c.feng@intel.com> Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com> Reviewed-by: Liming Gao <liming.gao@intel.com> Reviewed-by: Bob Feng <bob.c.feng@intel.com>
Laszlo Ersek [Tue, 10 Mar 2020 17:50:25 +0000 (18:50 +0100)]
OvmfPkg: raise DXEFV size to 12 MB
Similarly to the "cadence" mentioned in commit d272449d9e1e ("OvmfPkg:
raise DXEFV size to 11 MB", 2018-05-29), it's been ~1.75 years, and we've
outgrown DXEFV again. Increase the DXEFV size to 12MB now.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Gary Lin <glin@suse.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Leif Lindholm <leif@nuviainc.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2585 Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200310175025.18849-1-lersek@redhat.com> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Ard Biesheuvel [Sat, 7 Mar 2020 09:10:08 +0000 (10:10 +0100)]
ArmPkg/ArmMmuLib AARCH64: cosmetic fixups
Some cosmetic fixups to the AArch64 MMU code:
- reflow overly long lines unless it hurts legibility
- add/remove whitespace according to the [de facto] coding style
- use camel case for goto labels
Ard Biesheuvel [Sat, 7 Mar 2020 09:10:07 +0000 (10:10 +0100)]
ArmPkg/ArmMmuLib AARCH64: drop pointless page table memory type check
This is the AARCH64 counterpart of commit 1f3b1eb3082206e4, to remove
a pointless check against the memory type of the allocations that the
page tables happened to land in. On ArmV8, we use writeback cacheable
exclusively for all memory.
Ard Biesheuvel [Sat, 7 Mar 2020 08:38:49 +0000 (09:38 +0100)]
ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating them
As it turns out, ARMv8 also permits accesses made with the MMU and
caches off to hit in the caches, so to ensure that any modifications
we make before enabling the MMU are visible afterwards as well, we
should invalidate page tables right after allocation like we do now on
ARM, if the MMU is still disabled at that point.
Ard Biesheuvel [Sat, 7 Mar 2020 08:38:48 +0000 (09:38 +0100)]
ArmPkg/ArmMmuLib AARCH64: rewrite page table code
Replace the slightly overcomplicated page table management code with
a simplified, recursive implementation that should be far easier to
reason about.
Note that, as a side effect, this extends the per-entry cache invalidation
that we do on page table entries to block and page entries, whereas the
previous change inadvertently only affected the creation of table entries.
Laszlo Ersek [Fri, 6 Mar 2020 23:04:42 +0000 (00:04 +0100)]
OvmfPkg/X86QemuLoadImageLib: fix "unused variable" error in X64 DXE builds
When the MDE_CPU_IA32 macro is not defined, there is no access to the
"KernelImageHandle" local variable in QemuStartKernelImage(). This breaks
the OvmfPkgIa32X64 and OvmfPkgX64 platform builds, at least with gcc-8.
Move the local variable to the inner scope, where declaration and usage
are inseparable.
(Note that such inner-scope declarations are frowned upon in the wider
edk2 codebase, but we use them liberally in ArmVirtPkg and OvmfPkg anyway,
because they help us reason about variable lifetime and visibility.)
Ard Biesheuvel [Fri, 6 Mar 2020 07:34:24 +0000 (08:34 +0100)]
OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition
Bob reports that VS2017 chokes on a tentative definition of the const
object 'mEfiFileProtocolTemplate', with the following error:
OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130):
error C2220: warning treated as error - no 'object' file generated
OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130):
warning C4132: 'mEfiFileProtocolTemplate': const object should be initialized
Let's turn the only function that relies on this tentative definition
into a forward declaration itself, and move its definition after the
external definition of the object. That allows us to drop the tentative
definition of the const object, and hopefully make VS2017 happy.
Ard Biesheuvel [Thu, 5 Mar 2020 21:21:15 +0000 (22:21 +0100)]
OvmfPkg/OvmfXen: fix build by providing QemuLoadImageLib resolution
Commit 859b55443a4253ba ("OvmfPkg/PlatformBootManagerLib: switch to
QemuLoadImageLib") replaced a dependency on LoadLinuxLib with one on
QemuLoadImageLib in the PlatformBootManagerLib implementation that is
shared between all OVMF builds, without taking into account that even
the Xen targeted builds incorporate this code, which is only used to
load kernels passed via the QEMU command line.
Since this is dead code on Xen, we can satisfy the dependency using
the generic version of QemuLoadImageLib, which does not rely on
LoadLinuxLib, which we can therefore drop from OvmfXen.dsc.
Ard Biesheuvel [Thu, 5 Mar 2020 12:49:32 +0000 (13:49 +0100)]
ArmPkg/ArmMmuLib ARM: drop memory type check for page tables
We already expect normal memory to be mapped writeback cacheable if
EDK2 itself is to make use of it, so doing an early sanity check on
the memory type of the allocation that the page tables happened to
land in isn't very useful. So let's drop it.
Ard Biesheuvel [Thu, 5 Mar 2020 12:36:14 +0000 (13:36 +0100)]
ArmPkg/ArmMmuLib ARM: simplify assignment of TTBR0 system register
The expression passed into ArmSetTTBR0 () in ArmConfigureMmu() is
sub-optimal at several levels:
- TranslationTable is already aligned, and if it wasn't, doing it
here wouldn't help
- TTBRAttributes is guaranteed not to have any bits set outside of
the 0x7f mask, so the mask operation is pointless as well,
- an additional (UINTN) cast for good measure is also not needed.
Ard Biesheuvel [Wed, 26 Feb 2020 13:07:32 +0000 (14:07 +0100)]
ArmPkg/ArmLib: ASSERT on set/way cache ops being used with MMU on
On ARMv7 and up, doing cache maintenance by set/way is only
permitted in the context of on/offlining a core, and any other
uses should be avoided. Add ASSERT()s in the right place to
ensure that any uses with the MMU enabled are caught in DEBUG
builds.
Ard Biesheuvel [Wed, 26 Feb 2020 12:49:03 +0000 (13:49 +0100)]
ArmPkg/ArmLib: clean up library includes
Suspiciously, ArmLib's INF does not contain a [LibraryClasses]
section at all, but it turns out that all the library includes
it contains (except for ArmLib.h itself) are actually bogus so
let's just drop all of them. While at it, replace <Uefi.h> with
the more accurate <Base.h> for a BASE type module, and put the
includes in a consistent order.
Ard Biesheuvel [Wed, 26 Feb 2020 09:07:37 +0000 (10:07 +0100)]
ArmPkg/ArmLib: move set/way helper functions into private header
The clean/invalidate helper functions that operate on a single cache
line identified by set, way and level in a special, architected format
are only used by the implementations of the clean/invalidate routines
that operate on the entire cache hierarchy, as exposed by ArmLib.
The latter routines will be deprecated soon, so move the helpers out
of ArmLib.h and into a private header so they are safe from abuse.
In the AARCH64 version of ArmMmuLib, we are currently relying on
set/way invalidation to ensure that the caches are in a consistent
state with respect to main memory once we turn the MMU on. Even if
set/way operations were the appropriate method to achieve this, doing
an invalidate-all first and then populating the page table entries
creates a window where page table entries could be loaded speculatively
into the caches before we modify them, and shadow the new values that
we write there.
So let's get rid of the blanket clean/invalidate operations, and
instead, update ArmUpdateTranslationTableEntry () to invalidate each
page table entry *after* it is written if the MMU is still disabled
at this point.
On ARMv8, it is guaranteed that memory accesses done by the page table
walker are cache coherent, and so we can ignore the case where the
MMU is on.
Since the MMU and D-cache are already off when we reach this point, we
can drop the MMU and D-cache disables as well. Maintenance of the I-cache
is unnecessary, since we are not modifying any code, and the installed
mapping is guaranteed to be 1:1. This means we can also leave it enabled
while the page table population code is running.
In the ARM version of ArmMmuLib, we are currently relying on set/way
invalidation to ensure that the caches are in a consistent state with
respect to main memory once we turn the MMU on. Even if set/way
operations were the appropriate method to achieve this, doing an
invalidate-all first and then populating the page table entries creates
a window where page table entries could be loaded speculatively into
the caches before we modify them, and shadow the new values that we
write there.
So let's get rid of the blanket clean/invalidate operations, and instead,
invalidate each page table right after allocating it, and each section
entry after it is updated (to address all the little corner cases that the
ARMv7 spec permits), and invalidate sets of level 2 entries in blocks,
using the generic invalidation routine from CacheMaintenanceLib
On ARMv7, cache maintenance may be required also when the MMU is
enabled, in case the page table walker is not cache coherent. However,
the code being updated here is guaranteed to run only when the MMU is
still off, and so we can disregard the case when the MMU and caches
are on.
Since the MMU and D-cache are already off when we reach this point, we
can drop the MMU and D-cache disables as well. Maintenance of the I-cache
is unnecessary, since we are not modifying any code, and the installed
mapping is guaranteed to be 1:1. This means we can also leave it enabled
while the page table population code is running.
Ard Biesheuvel [Thu, 5 Mar 2020 09:42:15 +0000 (10:42 +0100)]
ArmPkg/ArmMmuLib ARM: use AllocateAlignedPages() for alignment
Instead of overallocating memory and align the resulting base address
manually, use the AllocateAlignedPages () helper, which achieves the
same, and might even manage that without leaking a chunk of memory of
the same size as the allocation itself.
While at it, fix up a variable declaration in the same hunk, and drop
a comment whose contents add nothing to the following line of code.
Ard Biesheuvel [Wed, 26 Feb 2020 09:36:49 +0000 (10:36 +0100)]
ArmPkg/ArmMmuLib ARM: split ArmMmuLibCore.c into core and update code
Unlike the AArch64 implementation of ArmMmuLib, which combines the
initial page table population code with the code that runs at later
stages to manage permission attributes in the page tables, ARM uses
two completely separate sets of routines for this.
Since ArmMmuLib is a static library, we can prevent duplication of
this code between different users, which usually only need one or
the other. (Note that LTO should also achieve the same.)
This also makes it easier to reason about modifying the cache
maintenance handling, and replace the set/way ops with by-VA
ops, since the code that performs the set/way ops only executes
when the MMU is still off.
Ard Biesheuvel [Tue, 25 Feb 2020 18:28:34 +0000 (19:28 +0100)]
ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones
Cache maintenance operations by set/way are only intended to be used
in the context of on/offlining a core, while it has been taken out of
the coherency domain. Any use intended to ensure that the contents of
the cache have made it to main memory is unreliable, since cacheline
migration and non-architected system caches may cause these contents
to linger elsewhere, without being visible in main memory once the
MMU and caches are disabled.
In KVM on Linux, there are horrid hacks in place to ensure that such
set/way operations are trapped, and replaced with a single by-VA
clean/invalidate of the entire guest VA space once the MMU state
changes, which can be costly, and is unnecessary if we manage the
caches a bit more carefully, and perform maintenance by virtual
address only.
So let's get rid of the call to ArmInvalidateDataCache () in the
PrePeiCore startup code, and instead, invalidate the UEFI memory
region by virtual address, which is the only memory region we will
be touching with the caches and MMU both disabled and enabled.
(This will lead to data corruption if data written with the MMU off
is shadowed by clean, stale cachelines that stick around when the
MMU is enabled again.)
Ard Biesheuvel [Sat, 29 Feb 2020 09:42:43 +0000 (10:42 +0100)]
OvmfPkg: use generic QEMU image loader for secure boot enabled builds
The QemuLoadImageLib implementation we currently use for all OVMF
builds copies the behavior of the QEMU loader code that precedes it,
which is to disregard UEFI secure boot policies entirely when it comes
to loading kernel images that have been specified on the QEMU command
line. This behavior deviates from ArmVirtQemu based builds, which do
take UEFI secure boot policies into account, and refuse to load images
from the command line that cannot be authenticated.
The disparity was originally due to the fact that the QEMU command line
kernel loader did not use LoadImage and StartImage at all, but this
changed recently, and now, there are only a couple of reasons left to
stick with the legacy loader:
- it permits loading images that lack a valid PE/COFF header,
- it permits loading X64 kernels on IA32 firmware running on a X64
capable system.
Since every non-authentic PE/COFF image can trivially be converted into
an image that lacks a valid PE/COFF header, the former case can simply
not be supported in a UEFI secure boot context. The latter case is highly
theoretical, given that one could easily switch to native X64 firmware in
a VM scenario.
That leaves us with little justification to use the legacy loader at all
when UEFI secure boot policies are in effect, so let's switch to the
generic loader for UEFI secure boot enabled builds.
Ard Biesheuvel [Sat, 29 Feb 2020 12:59:52 +0000 (13:59 +0100)]
OvmfPkg/QemuKernelLoaderFsDxe: add support for new Linux initrd device path
Linux v5.7 will introduce a new method to load the initial ramdisk
(initrd) from the loader, using the LoadFile2 protocol installed on a
special vendor GUIDed media device path.
Add support for this to our QEMU command line kernel/initrd loader.
Ard Biesheuvel [Sat, 29 Feb 2020 09:33:21 +0000 (10:33 +0100)]
OvmfPkg/PlatformBootManagerLib: switch to QemuLoadImageLib
Replace the open coded sequence to load Linux on x86 with a short and
generic sequence invoking QemuLoadImageLib, which can be provided by
a generic version that only supports the LoadImage and StartImage boot
services, and one that incorporates the entire legacy loading sequence
as well.
Ard Biesheuvel [Sat, 29 Feb 2020 00:28:45 +0000 (01:28 +0100)]
OvmfPkg: implement QEMU loader library for X86 with legacy fallback
Implement another version of QemuLoadImageLib that uses LoadImage and
StartImage, but falls back to the legacy Linux loader code if that
fails. The logic in the legacy fallback routines is identical to the
current QEMU linux loader for X64 and IA32.
Note the use of the OVMF_LOADED_X86_LINUX_KERNEL protocol for the legacy
loaded image: this makes it possible to expose the LoadImage/StartImage
abstraction for the legacy loader, using the EFI paradigm of identifying
a loaded image solely by a handle.
Ard Biesheuvel [Thu, 5 Mar 2020 10:59:57 +0000 (11:59 +0100)]
OvmfPkg: create protocol and GUID header for loaded x86 Linux kernels
In preparation of moving the legacy x86 loading to an implementation
of the QEMU load image library class, introduce a protocol header
and GUID that we will use to identify legacy loaded x86 Linux kernels
in the protocol database.
Ard Biesheuvel [Fri, 28 Feb 2020 22:15:22 +0000 (23:15 +0100)]
OvmfPkg/QemuKernelLoaderFsDxe: add support for the kernel setup block
On x86, the kernel image consists of a setup block and the actual kernel,
and QEMU presents these as separate blobs, whereas on disk (and in terms
of PE/COFF image signing), they consist of a single image.
So add support to our FS loader driver to expose files via the abstract
file system that consist of up to two concatenated blobs, and redefine
the kernel file so it consists of the setup and kernel blobs, on every
architecture (on non-x86, the setup block is simply 0 bytes and is
therefore ignored implicitly)
Ard Biesheuvel [Fri, 28 Feb 2020 21:38:50 +0000 (22:38 +0100)]
OvmfPkg/QemuKernelLoaderFsDxe: don't expose kernel command line
We have no need for exposing the kernel command line as a file,
so remove support for that. Since the remaining blobs (kernel
and initrd) are typically much larger than a page, switch to
the page based allocator for blobs at the same time.
Ard Biesheuvel [Fri, 28 Feb 2020 16:32:35 +0000 (17:32 +0100)]
ArmVirtPkg/PlatformBootManagerLib: switch to separate QEMU loader
Drop the QEMU loader file system implementation inside this library,
and switch to the separate QemuLoadImageLib library and the associated
driver to expose the kernel and initrd passed via the QEMU command line.
Ard Biesheuvel [Fri, 28 Feb 2020 16:26:54 +0000 (17:26 +0100)]
OvmfPkg: provide a generic implementation of QemuLoadImageLib
Implement QemuLoadImageLib, and make it load the image provided by the
QEMU_EFI_LOADER_FS_MEDIA_GUID/kernel device path that we implemented
in a preceding patch in a separate DXE driver, using only the standard
LoadImage and StartImage boot services.
Ard Biesheuvel [Fri, 28 Feb 2020 16:04:01 +0000 (17:04 +0100)]
OvmfPkg: introduce QemuLoadImageLib library class
Introduce the QemuLoadImageLib library class that we will instantiate
to load the kernel image passed via the QEMU command line using the
standard LoadImage boot service.
Ard Biesheuvel [Fri, 28 Feb 2020 13:58:14 +0000 (14:58 +0100)]
OvmfPkg: export abstract QEMU blob filesystem in standalone driver
Expose the existing implementation of an abstract filesystem exposing
the blobs passed to QEMU via the command line via a standalone DXE
driver.
Notable difference with the original code is the switch to a new vendor
GUIDed media device path, as opposed to a vendor GUID hardware device
path, which is not entirely appropriate for pure software constructs.
Since we are using the GetTime() runtime service in a DXE_DRIVER type
module, we need to DEPEX explicitly on gEfiRealTimeClockArchProtocolGuid.
Ard Biesheuvel [Fri, 28 Feb 2020 13:13:03 +0000 (14:13 +0100)]
OvmfPkg: add GUID for the QEMU kernel loader fs media device path
In an upcoming patch, we will introduce a separate DXE driver that
exposes the virtual SimpleFileSystem implementation that carries the
kernel and initrd passed via the QEMU command line, and a separate
library that consumes it, to be incorporated into the boot manager.
Since the GUID used for the SimpleFileSystem implementation's device
path will no longer be for internal use only, create a well defined
GUID to identify the media device path.
Laszlo Ersek [Wed, 4 Mar 2020 09:44:13 +0000 (10:44 +0100)]
ArmVirtPkg/PlatformBootManagerLib: sync Timeout with PcdPlatformBootTimeOut
Set the Timeout global variable to the same value as
PcdPlatformBootTimeOut. This way the "setvar" command in the UEFI shell,
and the "efibootmgr" command in a Linux guest, can report the front page
timeout that was requested on the QEMU command line (see
GetFrontPageTimeoutFromQemu()).
A DEBUG_VERBOSE message is logged on success too, for our QE team's sake.
Laszlo Ersek [Wed, 4 Mar 2020 09:44:12 +0000 (10:44 +0100)]
OvmfPkg/PlatformBootManagerLib: sync Timeout with PcdPlatformBootTimeOut
Set the Timeout global variable to the same value as
PcdPlatformBootTimeOut. This way the "setvar" command in the UEFI shell,
and the "efibootmgr" command in a Linux guest, can report the front page
timeout that was requested on the QEMU command line (see
GetFrontPageTimeoutFromQemu()).
A DEBUG_VERBOSE message is logged on success too, for our QE team's sake.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200304094413.19462-2-lersek@redhat.com> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Gaurav Jain [Tue, 25 Feb 2020 12:00:41 +0000 (20:00 +0800)]
MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
ASSERT in PollMem_Conf, CopyMem_Conf, SetBarAttributes_Conf
Conformance Test.
SCT Test expect return as Invalid Parameter or Unsupported.
Added Checks for Function Parameters.
return Invalid or Unsupported if Check fails.
Added Checks in PciIoPollIo(), PciIoIoRead()
PciIoIoWrite()
Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Albecki, Mateusz [Thu, 27 Feb 2020 17:25:25 +0000 (01:25 +0800)]
MdeModulePkg/SdMmcPciHcDxe: Do not map memory for non DMA transfer
Driver code used to map memory for DMA transfer even if host doesn't
support DMA. This is causing memory corruption when driver transfers
data using PIO. This change refactors the code to skip call to
PciIo->Map for non DMA transfers.
Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Marcin Wojtas <mw@semihalf.com> Cc: Zhichao Gao <zhichao.gao@intel.com> Cc: Liming Gao <liming.gao@intel.com> Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com> Tested-by: Hao A Wu <hao.a.wu@intel.com> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
projects / mirror_edk2.git / log