BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3223
In the current design, memory protection is not available till CpuDxe
is loaded. To resolve this, introduce CpuArchLib to move the
CPU Architectural initialization to DxeCore.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3704
build -a X64 -t CLANG38 -b RELEASE -p OvmfPkg/OvmfPkgX64.dsc
results in
UDK/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c:1284:31:
error: variable 'Status' set but not used
[-Werror,-Wunused-but-set-variable]
Signed-off-by: Mikhail Krichanov <krichanov@ispras.ru>
MdePkg/DebugLib(PerformanceLib): Fix "unused-but-set-variable" warning
The current definitions of DEBUG_CODE_BEGIN() and DEBUG_CODE_END() use
the local
variable __DebugCodeLocal as an attempt to track parity. If
DEBUG_CODE_END() is
used without a preceding DEBUG_CODE_BEGIN(), __DebugCodeLocal will not
have been
declared and a compilation error will be issued. The mutations of the
variable
are not used to track nesting or such. As the value of this variable is
never
actually used, recent Clang versions issue a "unused-but-set-variable"
warning
for it.
To solve this, re-define __DebugCodeLocal as a BOOLEAN that is always
FALSE and
use it in a do-while loop condition as done explicitly in many places.
Like the
previous solution, DEBUG_CODE_END() cannot be used without
DEBUG_CODE_BEGIN(),
as __DebugCodeLocal will be not have been defined.
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
ArmVirtQemu may execute at EL2, in which case monitor calls are
generally made using SMC instructions instead of HVC instructions.
Whether or not this is the case can only be decided at runtime, and so
the associated PCD needs to be settable at runtime, if the platform
definition chooses so. This implies a boolean PCD, given that a feature
PCD is build-time configurable only.
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Committed-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
The DebugPeCoffExtraActionLib implemention in ArmPkg contains some cruft
that dates back to the original RVCT based ARM port, and support for
RVCT was dropped a while ago.
Also drop the handling of Cygwin specific paths, which is highly
unlikely to be still depended upon by anyone.
Tweak the logic so that only two versions of the DEBUG() invocations
remain: one for __GNUC__ when PdbPointer is set, and the fallback that
just prints the image address and the address of the entrypoint.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
mReplaceLiveEntryFunc is a function pointer but assigned as a VOID*
pointer:
mReplaceLiveEntryFunc = *(VOID **)GET_GUID_HOB_DATA (Hob);
This leads to the Visual Studio warning:
nonstandard extension, function/data pointer conversion in
expression
This change updates the assignment to avoid using a data pointer and
defines a type for the function pointer to succinctly and accurately
refer to the type when it is used in the library code.
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Create a helper function to query whether ID_AA64MFR1_EL1 indicates
presence of the Embedded Trace Extension (ETE). This feature is only
visible in AARCH64 state.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Pierre Gondois <pierre.gondois@arm.com>
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
Create a helper function to query whether ID_AA64MFR1_EL1 indicates
presence of the Trace Buffer Extension (TRBE). This feature is only
visible in AARCH64 state.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Pierre Gondois <pierre.gondois@arm.com>
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
Create a helper function to query whether ID_AA64MFR1_EL1 indicates
presence of the Virtualization Host Extensions. This feature is only
visible in AARCH64 state.
Signed-off-by: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Tested-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
Jake reports that the IS_ARM_MEMORY_REGION_ATTRIBUTES_SECURE() macro is
no longer accurate since commit 852227a9d5 ("ArmPkg/Mmu: Remove
handling of NONSECURE memory regions").
Fortunately, it only affects the NS bit in level 1 short descriptors,
which is ignored when executing in non-secure mode. And given that
running UEFI in the secure world is not a use case we aim to support,
let's just drop this logic altogether.
Reported-by: Jake Garver <jake@nvidia.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
If gST->ConOut is available when Arm's DefaultExceptionHandler is
running, AsciiPrint will get called to attempt to print to ConOut, in
addition to the serial output.
AsciiPrint calls AsciiInternalPrint in UefiLibPrint.c which in turn
calls AllocatePool to allocate a buffer to convert the Ascii input
string to a Unicode string to pass to ConOut->OutputString.
Per the comment on DefaultExceptionHandler, we should not be allocating
memory in the exception handler, as this can cause the exception handler
to fail if we had a memory exception or the system state is such that we
cannot allocate memory.
It has been observed on ArmVirtQemu that exceptions generated in the
memory handling code will fail to output the stack dump and CPU state
that is critical to debugging because the AllocatePool will fail.
This patch fixes the Arm and AARCH64 DefaultExceptionHandlers to not
allocate memory when ConOut is available and instead use stack memory to
convert the Ascii string needed for SerialPortWrite to the Unicode
string needed for ConOut->OutputString. Correspondingly, ArmVirtQemu can
now output the stack dump and CPU state when hitting an exception in
memory code.
Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
The OP-TEE secure OS exposes a non-secure memory region for
communication between the secure OS itself and any clients in the
non-secure firmware. This memory is writable by non-secure and is not
used for code only data, and so it should be mapped non-executable.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Now that we have a sane API to set and clear memory permissions that
works the same on ARM and AArch64, we no longer have a need for the
individual set/clear no-access/read-only/no-exec helpers so let's drop
them.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Currently, ArmSetMemoryAttributes () takes a combination of
EFI_MEMORY_xx constants describing the memory type and permission
attributes that should be set on a region of memory. In cases where the
memory type is omitted, we assume that the memory permissions being set
are final, and that existing memory permissions can be discarded.
This is problematic, because we aim to map memory non-executable
(EFI_MEMORY_XP) by default, and only relax this requirement for code
regions that are mapped read-only (EFI_MEMORY_RO). Currently, setting
one permission clears the other, and so code managing these permissions
has to be aware of the existing permissions in order to be able to
preserve them, and this is not always tractable (e.g., the UEFI memory
attribute protocol implements an abstraction that promises to preserve
memory permissions that it is not operating on explicitly).
So let's add an AttributeMask parameter to ArmSetMemoryAttributes(),
which is permitted to be non-zero if no memory type is being provided,
in which case only memory permission attributes covered in the mask will
be affected by the update.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
The PCD gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable
indicates if a variable driver will emulate the variable NV mode.
This PCD is defined as [PcdsFixedAtBuild, PcdsPatchableInModule,
PcdsDynamic, PcdsDynamicEx].
Some firmware builds may define this PCD as a dynamic PCD and
initialise the value at runtime. Therefore, move the PCD declaration
from the [FixedPcd] section to the [Pcd] section in the platform
boot manager library file PlatformBootManagerLib.inf. Without this
change the build would not succeed.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
The helper that updates live page table entries writes a zero entry,
invalidates the covered address range from the TLBs, and finally writes
the actual entry. This ensures that no TLB conflicts can occur.
Writing the final entry needs to complete before any translations can be
performed, as otherwise, the zero entry, which describes an invalid
translation, may be observed by the page table walker, resulting in a
translation fault. For this reason, the final write is followed by a DSB
barrier instruction.
However, this barrier will not stall the pipeline, and instruction
fetches may still hit this invalid translation, as has been observed and
reported by Oliver. To ensure that the new translation is fully active
before returning from this helper, we have to insert an ISB barrier as
well.
Reported-by: Oliver Steffen <osteffen@redhat.com>
Tested-by: Oliver Steffen <osteffen@redhat.com>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
The UEFI Shell is a non-active boot option, at the opposite of UiApp.
If no valid boot option is found, UiApp is selected. UiApp requires a
human interaction. When installing a new EDKII image in CIs or when
scripting is required, this is problematic.
If no valid boot option is discovered, add a path to directly go to
the UEFI Shell where the startup.nsh script is automatically executed.
The UEFI Shell is launched after connecting possible devices, but
before the reset that is meant to automatically make them visible.
The new PcdUefiShellDefaultBootEnable must be set to TRUE to enable
this behaviour. The Pcd is set to false by default.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Tested-by: Patrik Berglund <patrik.berglund@arm.com>
ArmCpuInfo uses those to read system registers and other parts of EDK2
may find them useful.
Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
As the ASM_FUNC() macro performs a section switch, the preceding
.balign directive applies the alignment constraint to the current
location in the previous section. As the linker may not merge the
sections in-order, ArmReplaceLiveTranslationEntry() may be left
unaligned.
Replace the explicit invocation of .balign with the ASM_FUNC_ALIGN()
macro, which guarantees the alignment constraint is applied correctly.
To make sure related issues are reliably caught in the future, align the
end of the function before checking the total occupied size. This
ensures crossing a 0x200 boundary will cause a compilation error.
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
__FUNCTION__ is a pre-standard extension that gcc and Visual C++ among
others support, while __func__ was standardized in C99.
Since it's more standard, replace __FUNCTION__ with __func__ throughout
ArmPkg.
Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Mikhail Krichanov
Marvin Häuser
Ard Biesheuvel
Michael Kubacki
Sami Mujawar
Leif Lindholm
Oliver Smith-Denny
Pierre Gondois
Marcin Juszkiewicz
Marvin Häuser
Rebecca Cran