| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Add interrupt-enable recursion depth tracking using
mInterruptEnableNestDepth and a bounded assertion in
CoreSetInterruptState().
This provides early detection for unintended recursive
interrupt-enable loops.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
|
|
Refactor the CoreSetInterruptState(TRUE) flow so EnableInterrupt() is
invoked through a single call site.
Behavior is unchanged: interrupts remain disabled in SMM and are enabled
outside SMM.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
|
|
MemoryBinGoogleTest.PopulatesFromValidHob asserted fixed page counts for
each memory type. PopulateMemoryTypeInformation, however, rounds the
runtime memory types (EfiReservedMemoryType, EfiACPIMemoryNVS,
EfiRuntimeServicesCode, EfiRuntimeServicesData) up to
RUNTIME_PAGE_ALLOCATION_GRANULARITY. That granularity equals EFI_PAGE_SIZE
on IA32/X64, so the hard-coded values happened to match, but it is 64 KiB
on AArch64, where the same inputs round up to different page counts and the
test failed.
Compute the expected page counts with the same granularity rounding the
production code uses, so the test passes on all host architectures instead
of only x86.
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
|
|
Commit 43e306806e3c1ed3ad7e9913492732e893cafa0f added support
to DXE Core for EfiUnacceptedMemoryType. However, it incorrectly
added EFI_GCD_MEMORY_TYPE_UNACCEPTED to gMemoryTypeInformation,
which is the GCD memory type that is associated with
EfiUnacceptedMemoryType. All other changes from that PR
appear correct.
This is corrected to the EFI memory type.
The Memory Bin Google Test copied this incorrect definition,
so it is updated as well.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
This commit adds unit tests and documentation for the Memory Bin
feature.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
If memory bins are enabled for PEI, PEI will produce
Memory Allocation HOBs marked with gEfiMemoryTypeInformationGuid.
If these exist, DXE core will now process the stats from them
to have accurate numbers.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
This commits splits out logic currently contained in
Gcd.c and Page.c to a new file called
MemoryBin.c. This is set up in preparation to
add support to PEI for memory bins (an S4 resume stability
feature).
MemoryBin.c takes all global state in as parameters so that
DXE core can use globals and PEI core can use HOBs.
There is no logic change here, just consolidating the
functionality to share with PEI.
This was requested not to be a library.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
This commit adds a parameter to the memory bin allocation function
to tell it whether it should create the Resource Descriptor HOB
owned by gMemoryTypeInformationGuid. This will be used by PEI to
tell DXE where the memory bins are.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
This adds a helper function to initialize the
memory bin statistics as the same logic is
repeated in several places.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
Migrate CoreSetMemoryTypeInformationRange() to not use globals so
it can be used in PEI as well.
This temporarily moves the EFI_MEMORY_STATISTICS structure to
DxeMain.h so that it can be used in Gcd.c as well as Page.c.
This will migrate to a different private header once that is included.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
In preparation for sharing logic with PEI, create a helper
function to update the memory bin statistics.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
In preparation for sharing logic with PEI for memory bins,
add AllocateMemoryTypeInformationBins().
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
In preparation for sharing memory bin logic with PEI, create a
helper function that finds and validates a resource descriptor
HOB owned by gEfiMemoryTypeInformationGuid.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
In preparation for supporting shared memory bin logic in DXE
and PEI, create a PopulateMemoryTypeInformation() helper function.
This function searches for a Memory Type Information Hob and
populates an EFI_MEMORY_TYPE_INFORMATION struct with it.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
In preparation for sharing memory bin logic between
PEI and DXE, update CaclulateTotalMemoryBinSizeNeeded()
to take gMemoryTypeInformation by reference.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
Fix the GCD memory type selection logic in DXE GCD initialization so persistent memory correctly takes precedence over special-purpose memory when both attributes are present.
The existing code/comment said persistent should win, but the condition order allowed special-purpose to overwrite persistent. This change swaps the checks so behavior matches the intended precedence and comment.
Signed-off-by: Sureshkumar Ponnusamy <sponnusamy@microsoft.com>
|
|
Currently, the DXE dispatcher will skip extracting an FV file
when an FV2 HOB is found for it, as that indicates pre-DXE
extracted it.
However, the dispatcher does not check for FV3 HOBs, which also
can describe extracted FVs. That can result in extracting the
same FV in DXE that is already extracted, which can be a large
performance hit.
This updates the DXE dispatcher to check for the existence of
either an FV2 or FV3 HOB for this FV and skip extracting if either
is found.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
Currently, there is no guarantee that the memory bins will be
allocated contiguously. However, there are assumptions in the
code that bins are allocated contiguously, such as the GCD
init code requiring a free memory region be large enough for
a contiguous bin range on DXE Core init.
Ensuring contiguous bins also makes a cleaner model for the
bins and keeps the memory map in a more standard configuration
between different configurations, as platforms today can
pass a resource descriptor HOB to DXE core to describe the bin
range and this only supports a contiguous range.
This also sets up reusing the memory bin logic for PEI memory
bin support which will use the aforementioned resource descriptor
HOB to pass the bin range to DXE.
This commit updates CoreAddMemoryDescriptor() to
allocate a contiguous range.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
Update CoreLoadImageCommon to return EFI_NOT_FOUND instead of
EFI_INVALID_PARAMETER when both SourceBuffer and DevicePath are NULL.
UEFI 2.11 specification requires this change for LoadImage().
Signed-off-by: jie.fu <jie.fu@cixtech.com>
|
|
Replace manual alignment checks with IS_ALIGNED() and
ADDRESS_IS_ALIGNED().
Convert the following bitmask and modulo forms:
- ((E & ((PowOf2Expr) - ONE)) == ZERO)
- ((E & ((PowOf2Expr) - ONE)) != ZERO)
- ((E % (PowOf2Expr)) == ZERO)
- ((E % (PowOf2Expr)) != ZERO)
to the corresponding helper macro forms:
+ IS_ALIGNED (E, PowOf2Expr)
+ !IS_ALIGNED (E, PowOf2Expr)
PowOf2Expr is limited to known power-of-two expressions, including
SIZE_* and BASE_* macros, EFI_PAGE_SIZE, CPU_STACK_ALIGNMENT,
RUNTIME_PAGE_ALLOCATION_GRANULARITY, sizeof() of UEFI integer types
(e.g. BOOLEAN, CHAR16, UINT32, UINTN) and pointer types, and 1 << E1
expressions.
Address checks that cast the checked value to UINTN are written with
ADDRESS_IS_ALIGNED().
The change was generated with the Coccinelle semantic patch below.
```smpl
@power_of_2_expr@
expression PowOf2Expr;
expression E1;
typedef BOOLEAN, CHAR8, CHAR16, INT8, UINT8, INT16, UINT16, INT32, UINT32, INT64, UINT64, INTN, UINTN;
type ScalarType = { BOOLEAN, CHAR8, CHAR16, INT8, UINT8, INT16, UINT16, INT32, UINT32, INT64, UINT64, INTN, UINTN };
type AnyType;
type PointerType = AnyType *;
idexpression ScalarType ScalarValue;
idexpression PointerType PointerValue;
constant SizeBase =~ "^(SIZE|BASE)_(1|2|4|8|16|32|64|128|256|512)[KMGTPE]B$";
constant NamedPowerOf2 =~ "^(EFI_PAGE_SIZE|CPU_STACK_ALIGNMENT|RUNTIME_PAGE_ALLOCATION_GRANULARITY)$";
constant ONE = {1, 1U, 1u};
@@
(
(
SizeBase
|
NamedPowerOf2
|
ONE << E1
|
sizeof (ScalarType)
|
sizeof (PointerType)
|
sizeof (ScalarValue)
|
sizeof (PointerValue)
)
&
PowOf2Expr
)
@aligned depends on power_of_2_expr disable is_zero,isnt_zero@
expression E;
expression power_of_2_expr.PowOf2Expr;
constant ONE = {1, 1U, 1u};
constant ZERO = {0, 0U, 0u};
@@
(
((E & (E - ONE)) == ZERO)
|
- ((E & ((PowOf2Expr) - ONE)) == ZERO)
+ IS_ALIGNED (E, PowOf2Expr)
|
((E & (E - ONE)) != ZERO)
|
- ((E & ((PowOf2Expr) - ONE)) != ZERO)
+ !IS_ALIGNED (E, PowOf2Expr)
|
- ((E % (PowOf2Expr)) == ZERO)
+ IS_ALIGNED (E, PowOf2Expr)
|
- ((E % (PowOf2Expr)) != ZERO)
+ !IS_ALIGNED (E, PowOf2Expr)
)
@address_is_aligned@
typedef UINTN;
expression *Address;
expression Alignment;
@@
- IS_ALIGNED ((UINTN) Address, Alignment)
+ ADDRESS_IS_ALIGNED (Address, Alignment)
@normalize_aligned disable paren expression@
expression E, SZ;
@@
(
- (IS_ALIGNED (E, SZ))
+ IS_ALIGNED (E, SZ)
|
- (!IS_ALIGNED (E, SZ))
+ !IS_ALIGNED (E, SZ)
)
@normalize_macro_args disable paren expression@
expression E, SZ;
@@
(
- IS_ALIGNED ((E), SZ)
+ IS_ALIGNED (E, SZ)
|
- IS_ALIGNED (E, (SZ))
+ IS_ALIGNED (E, SZ)
)
```
Signed-off-by: Mingjie Shen <shen497@purdue.edu>
|
|
CoreSetMemoryTypeInformationRange() currently calculates the bin
size needed independently from the CalculateTotalMemoryBinSizeNeeded()
function. This commit updates to use that function and remove the
duplication.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
Currently CalculateTotalMemoryBinSizeNeeded() does not take
runtime alignment granularity considerations into account. This
means that the GCD initialization code can choose resource desc
HOBs to use for the bin region that are actually too small and
fail to initialize the bins.
This fixes CalculateTotalMemoryBinSizeNeeded() to take the
alignment requirements into consideration, both for size and for
alignment of the bin address range.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
Currently, the DXE page allocator does not ensure that guard
page allocations stay within the bin that it is attempting to
allocate within. As a result, S4 resume is jeopardized by
bins expanding due to guard pages, either into other bins or
out of bins. This is caught by a new assert in CoreGetMemoryMap()
to ensure the bins are correct.
This fixes this by changing the internal heap guard API to return
the adjusted size and start address of a proposed allocation. The
page allocator then can ensure that the adjusted allocation still
fits within the bin it is attempting to allocate within; if not,
it will search for another descriptor.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
fixes: #11112
Currently, function RegisterProtocolNotify() doesn't check the validity
of event and it accepts any event pointer as long as pointer isn't NULL.
However event could be closed and freed which could lead to use after
free.
Always check event signature before usage and return
EFI_INVALID_PARAMTER for events with invalid signature.
Signed-off-by: Khalid Ali <khaliidcaliy@gmail.com>
|
|
Updates BmBoot and dispatcher to use new perf macros.
Signed-off-by: Sherry Fan <sherryfan@microsoft.com>
|
|
Fixes: #11113
Currently, the global handle key and key inside handle structure is
incremented only when a new handle is allocated for protocol interface
to be installed.
However, when caller already supplies a handle gHandleDatabaseKey never
get incremented.
Move handle key incremental outside if block, just below the else
statement which allows gHandleDatabaseKey to always incremented whether
handle is supplied or not.
Signed-off-by: Khalid Ali <khaliidcaliy@gmail.com>
|
|
This change adds validation to CoreGetMemoryMap to ensure that special
memory bins are fully respected. Specifically, any memory map entry that
falls within a special bin must be entirely contained within that bin,
and its memory type must match the bin's designated type.
This check helps preventing unintended changes that could cause the
system memory map to cross bin boundaries unexpectedly.
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
|
|
Replace traditional `#ifndef`/`#define`/`#endif` include guards with
`#pragma` once.
`#pragma once` is a widely supported preprocessor directive that
prevents header files from being included multiple times. It is
supported by all toolchains used to build edk2: GCC, Clang/LLVM, and
MSVC.
Compared to macro-based include guards, `#pragma once`:
- Eliminates the risk of macro name collisions or copy/paste errors
where two headers inadvertently use the same guard macro.
- Eliminate inconsistency in the way include guard macros are named
(e.g., some files use `__FILE_H__`, others use `FILE_H_`, etc.).
- Reduces boilerplate (three lines replaced by one).
- Avoids polluting the macro namespace with guard symbols.
- Can improve build times as the preprocessor can skip re-opening the
file entirely, rather than re-reading it to find the matching
`#endif` ("multiple-include optimization").
- Note that some compilers may already optimize traditional include
guards, by recognzining the idiomatic pattern.
This change is made acknowledging that overall portability of the
code will technically be reduced, as `#pragma once` is not part of the
C/C++ standards.
However, this is considered acceptable given:
1. edk2 already defines a subset of supported compilers in
BaseTools/Conf/tools_def.template, all of which have supported
`#pragma once` for over two decades.
2. There have been concerns raised to the project about inconsistent
include guard naming and potential macro collisions.
Approximate compiler support dates:
- MSVC: Supported since Visual C++ 4.2 (1996)
- GCC: Supported since 3.4 (2004)
(http://gnu.ist.utl.pt/software/gcc/gcc-3.4/changes.html)
- Clang (LLVM based): Since initial release in 2007
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
|
|
Fix alignment issues in memory map entries returned by
GetMemoryMap() when RUNTIME_PAGE_ALLOCATION_GRANULARITY is
larger than DEFAULT_PAGE_ALLOCATION_GRANULARITY.
There are no issues in the Page/Pool Allocation/Free services.
Logic issues issues are addressed in the memory map returned
by GetMemoryMap() due to missing cases for memory map entries
of type EfiConventionalMemory that overlap special memory bins.
Add logic to handle all possible memory map splits required to
convert internal memory map entries into an EFI Memory Map with
EFI Memory Map entries the follow alignment requirements when
the EFI Memory Map entries cover memory bins.
The four cases that must be handled are:
* Memory map entry contained within a bin. [Already covered]
Convert memory map entry type
* Memory map entry overlaps beginning of bin. [Added]
Split memory map entry at beginning of bin.
* Memory map entry overlaps end of bin. [Added]
Split memory map entry at end of bin.
* Memory map entry overlaps entire bin. [Added]
Split memory map entry at both ends of bin.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
|
|
Fix alignment issues in memory map entries returned by
GetMemoryMap() when RUNTIME_PAGE_ALLOCATION_GRANULARITY is
larger than DEFAULT_PAGE_ALLOCATION_GRANULARITY.
Alignment issues are addressed in the initial memory map
layout when Memory Type Information is provided with memory
bins that use RUNTIME_PAGE_ALLOCATION_GRANULARITY.
There are no issues in the Page/Pool Allocation/Free services.
* CoreSetMemoryTypeInformationRange() make sure there is room
for all bins when accounting for alignment requirements.
Allocate space for bins with base and length following
alignment requirements.
* CoreSetMemoryTypeInformationRange() round up NumberOfPages in
Memory Type Information based on alignment requirements.
This is required so GetMemoryMap() will generate memory
map entries that always follow alignment requirements.
* CoreAddMemoryDescriptor() round up NumberOfPages in
Memory Type Information based on alignment requirements.
This is required so GetMemoryMap() will generate memory
map entries that always follow alignment requirements.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
|
|
Commit 2d69507a4dde02f1abf20c7eb3a43d1d3ef6b98f added an attribute check to
prevent freeing memory that is read-only, read-protected, or for which
attribute retrieval fails. In such cases the code returned EFI_SUCCESS and
leaked the memory.
This introduced a regression in the System Architecture Compliance Suite
(ACS) BS.FreePages – Not Existent Memory test.
Link: https://github.com/tianocore/edk2-test/blob/edk2-test-stable202509/uefi-sct/Doc/TestCaseSpec/03_Services_Boot_Services.md#freepages
Test number: 5.1.2.2.1
GetMemoryAttributes() returns EFI_UNSUPPORTED for memory regions outside
system memory. The previous change treated all errors as a reason to leak
memory, while only the EFI_NO_MAPPING error code should trigger that
behavior. As a result, freeing non-existent memory incorrectly returned
EFI_SUCCESS instead of EFI_NOT_FOUND.
To fix this, memory is now leaked only when:
- GetMemoryAttributes() returns EFI_NO_MAPPING (inconsistent attributes),
or
- GetMemoryAttributes() succeeds and the pages are marked RO or RP.
All other errors fall through to CoreInternalFreePages(), restoring the
previous and correct behavior.
Signed-off-by: Piotr Wejman <piotr.wejman@arm.com>
|
|
Replace UINTN casts with EFI_PHYSICAL_ADDRESS in places where the result
is negated, as otherwise, the top bits may remain 0 unexpectedly.
VS2022 started warning about this, and thus breaking the IA32 CI build.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
|
|
https://github.com/github/codeql/blob/codeql-cli-2.7.3/cpp/ql/src/Critical/MissingNullTest.qhelp
For items which allocate memory, or get a pointer from another
structure, it is important to validate that the pointers
are not null before they are dereferenced.
Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
|
|
During DXE core memory service initialization, the system would check
available resource descriptor hobs against the memory top from PHIT hob.
However, it is possible that a given resource descriptor hob will not be
larger than the cover the memory top, causing the Length calculation to
underflow.
This change adds a check for potential underflow before performing the
subtraction.
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
|
|
Current GCD logic uses plain addition calulation when iterating through
the resource descriptor hobs. However, if the resource descriptor is
incorrectly prepared, this could cause incorrect memory initialization
and other failures down the boot process.
This change adds an overflow check before using the value.
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
|
|
The current implementation from Dxe/Image/Image.c does not handle the
configuration where there might be multiple MemoryAllocationModules.
Given that the `ModuleName` is included in the hob data and used for
targetting the consumer, DXE core should specify the GUID when looking up
for its own MemoryAllocationModule.
This change adds a check to ensure the located hob is targetting DXE
core.
Signed-off-by: Kun Qin <kuqin12@gmail.com>
|
|
The DXE_SAL_DRIVER module type was introduced to support
Itanium (IPF) platforms. Since support for Itanium processors
has been dropped, the instances of DXE_SAL_DRIVER
have been removed.
Ref: [3cb0a311cb7e747d7be5c5076d0fff76ad256d2b]
Cc: Sachin Ganesh <sachinganesh@ami.com>
Signed-off-by: Sathya Ravichandran <sathyar@ami.com>
|
|
PR #11443 introduced a regression by adding a DEBUG() message
when the lock for events is acquired and that lock is at
TPL_HIGH_LEVEL. If DEBUG() messages are routed through Report
Status Code, and the Report Status Code Protocol has not been
located yet, then a call to gBS->LocateProtocol() is made and
that call raises TPL to TPL_NOTIFY which causes a TPL inversion.
The event lock is used to atomically update gEventSignalQueue.
There is no need for the DEBUG() message to within the event
lock scope.
The fix is to scope the event lock to only the InsertHeadList()
call to update gEventSignalQueue.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
|
|
Memory Attributes Table needs to be updated to contain executable
permissions for UEFI runtime drivers loaded after EndOfDxe.
Fixes a regression introduced by bb248a9.
Signed-off-by: Vitaly Cheptsov <vit9696@protonmail.com>
|
|
Following the APs now always initializing separate exception
stacks, this commit always initializes a separate exception
stack for the BSP as well. Previously, this was only enabled
when PcdCpuStackGuard was set.
However, even when a stack guard page is not present,
stack overflows can still occur and corrupt the stack; if an
exception is taken here, it is still valuable to have a separate
exception stack for sanity.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
Commit 43e306806e3c1ed3ad7e9913492732e893cafa0f added
EFIGcdMemoryTypeUnaccepted (as it was later renamed) to be
returned in the EFI_MEMORY_MAP. However, it did not add it
to the number of entries calculation, so if any
EfiGcdMemoryTypeUnaccepted entries exist in the GCD they will
overflow the EFI_MEMORY_MAP buffer provided by the bootloader.
This resolves that by accounting for unaccepted entries in
the number of entries calculation.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
edk2 commit 3bd5c994c879f78e8e3d5346dc3b627f199291aa added usage
of EFI_MEMORY_ATTRIBUTE_MASK to edk2. However, it applied it
incorrectly to some places that should instead use
EFI_MEMORY_ACCESS_MASK. EFI_MEMORY_ACCESS_MASK contains the actual
HW page table access attributes (read protect, read only, no-execute),
whereas EFI_MEMORY_ATTRIBUTE_MASK contains the access attributes in
addition to some virtual attributes (special purpose and cpu crypto).
The GCD has a behavior where if SetMemorySpaceAttributes() is called
with only virtual attributes set, it will not call into CpuDxe to
change the attributes; 0 is a valid page table attribute set (it means
RWX). However, after the above change, this behavior was altered so
that if EFI_MEMORY_SP or EFI_MEMORY_CPU_CRYPTO is applied, in attempt
to just update these virtual attributes, the GCD will call into CpuDxe
and apply RWX instead, which is not the intention of the caller.
One other place this was done incorrectly was in CoreGetMemoryMap,
but that was fixed in f1567720b13a578ffa54716119f826df622babcd.
SetUefiImageMemoryAttributes() is also updated here because that
logic was copied from the check the GCD has about whether to call
CpuDxe or not. Now that the GCD has been corrected, this also
needs to be corrected.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
edk2 is dropping support for the ARM32 architecture. This
commit removes ARM32 support from MdeModulePkg. This also
drops irrelevant VALID_ARCHITECTURE comments from infs that
are not arch specific.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
There are a lot of notify callback events for Event Groups. Usually they
are not reported unless there is a debug code in the callback itself.
The debug message helps to check which/when the callback is registered and
executed in POST. Also helps to notice the callback sequence. It depends
on DEBUG_EVENT flag enabled by PcdFixedDebugPrintErrorLevel PCD token.
Signed-off-by: Phil Noh <Phil.Noh@amd.com>
|
|
The various cores all attempt to print the EfiFileName when
loading/dispatching drivers, but they are not unified on
approach. This commit ensures they are using the same buffer
size and the loop parsing variables are unsigned, as we should
not have a negative index.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
Today, DXE/PEI/SMM Core's image loaders only print driver load messages
if debug code is enabled. However, these are some of the most
important prints in the codebase: on a given system even if you
have nothing else to debug with, you can see the last driver
executed.
Debug code blocks are used to skip logic that only exists for
debug purposes and wastes time on a release build. However, the
logic to print a line and determine the filename from the PDB
is not extensive and provides critical information, so it is
inappropriate to wrap in a debug code section.
Platforms can still choose to disable logging at DEBUG_INFO/DEBUG_LOAD
and will not see the error messages.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
Currently, if the DebugClearMemory bit is set in the
PcdDebugPropertyMask, CoreConvertPagesEx will attempt to write
a pattern to the pages being freed. However, it does not check
that the page is writeable, which will cause a page fault if not.
Furthermore, if NX protections are not enabled, the core does not
ensure that any freed pages are RW, which is the state expected
when they are allocated next. If they are not RW, the allocating
driver will crash trying to use them.
This patch updates the page freeing code to query the memory
attributes protocol, if present, for the attributes. If this call
fails or the attributes are not RW at a minimum, the core leaks
the memory (returning success to the caller). If the memory
attribute protocol is not present (either because a platform doesn't
produce it or it is before the protocol has been produced, the core
continues with freeing memory. This is either before the CPU Arch
protocol is available (so drivers can't change memory attributes) or
otherwise matches existing behavior. This was deemed the best
approach to let memory that can't be guaranteed to be RW leak
instead of letting a driver crash when allocating it. It was deemed
less brittle to simply leak the memory instead of attempting to
change the attributes.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
Currently DxeIpl attempts to set page 0 to all 0's and to
create a memory allocation HOB for it. However, DxeIpl will
also unmap the page when mapping page tables and if null
detection is not enabled, DxeCore will set the page to 0,
regardless of allocation status.
Because no consumers are using the memory allocation HOB for
page 0, drop it. Instead, ensure that PeiCore and DxeCore do
not allow allocating page 0; it should always be reserved for
null pointer detection. It also complicates the story for
platforms that are attempting to audit the system and ensure that
no modules are using page 0. With these memory allocation HOBs
in place, it is difficult to tell if it is simply DxeIpl who
has allocated the memory or another module.
This commit drops the memory allocation HOB publishing and ensures
that DxeCore and PeiCore do not allocate page 0. DxeCore already
will not allocate page 0 to callers of AllocatePages who call with
a type other than AllocateAddress, this just changes so that
AllocateAddress cannot allocate at page 0 (which if null detection
is enabled will cause a page fault). PeiCore does not have
AllocateAddress and so this ensures standard allocations do not
receive page 0.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
The DebugImageInfoTable contains an array of image info
structures. The current implementation removes an entry by
freeing the info structure and putting NULL in that entry of
the array. It then decrements the table size tracked in the table.
However, the array is invalid at this point, it contains a NULL
entry, which the UEFI spec does not envision and it contains a valid
entry past the end of the array as tracked in the spec defined config
table. If the table is consumed at this point it can lead to an
invalid assessment of the image state, which defeats the purpose of
the table.
When a new info structure is added, it then scans for the first NULL
entry adds a pointer to the new info structure there and increments
the table size to cover the entrythat was formerly past the end of
the array.
The current implementation requires that once an unload happens,
more loads happen than unloads and that the last operation is not
an unload (which won't be true in the shell, e.g.). This is
needlessly complex, as the order of the table doesn't matter
(and in fact this implementation doesn't preserve image loading
order either).
This patch updates the removal function to free the desired
info structure, move the last entry of the array to this freed
spot, mark the last entry as NULL, and decrement the table count.
The entry addition function then just always puts a new entry at
the end of the array, expanding it as necessary. This simplifies
the logic and covers the gaps that were present.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
|
Commit 5ccb5fff02a66b21898bd57f48bbd7c3cd6f4e8d updated the
image memory protection code to set the protection
attributes through the GCD instead of directly to the page
table. However, this code had an implicit assumption that
each base address passed to it was the beginning of a GCD
descriptor. On the virtual platforms tested, this was the case.
However, on a physical platform, a scenario was encountered
where the base address was not the beginning of a GCD
descriptor, thus causing memory attributes to be applied
incorrectly.
This assumption does not need to be made and this patch
updates the code to handle the case where the base address
is not the beginning of a GCD descriptor.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
|
