Re: [edk2-devel][PATCH v1 1/1] MdeModulePkg: DxeCore: Don't Guard Large Runtime Granularity Allocations

["Ard Biesheuvel" <ardb@kernel.org>]

On Thu, 15 Feb 2024 at 20:55, Oliver Smith-Denny
<osde@linux.microsoft.com> wrote:
>
> On 2/14/2024 11:50 PM, Ard Biesheuvel wrote:
> > I looked at the EFI spec again, and EfiACPIReclaimMemory is not
> > actually listed as a memory type that has this 64k alignment
> > requirement. This makes sense, given that this memory type has no
> > significance to the firmware itself, only to the OS. OTOH, reserved
> > memory does appear there.
> >
> > So I suggest we fix that first, and then drop any mention of
> > EfiACPIReclaimMemory from this patch. At least we'll have heap guard
> > coverage for ACPI table allocations on arm64 going forward.
> >
> > The logic in question was added in 2007 in commit 28a00297189c, so
> > this was probably the rule on Itanium, but that support is long gone.
> >
>
> I wanted to understand this a little bit further. I do see that the spec
> does not call out EfiACPIReclaimMemory as needing 64k alignment.
> However, your statement that the memory type does not have have
> significance to the FW, only to the OS, so we don't need the 64k
> alignment is confusing to me. Isn't the 64k alignment needed only for
> regions that the OS does care about? In order to read this information
> at their 64k page granularity, doesn't this need to be aligned to 64k?
>

It is not about accessing the information in the pages. Most consumers
don't care about this at all, and even code that does will rarely be
materially different between 4k and 64k OSes.

When running under the firmware, we run with 4k pages and programming
the memory attributes is entirely under the control of the firmware
itself. So in this phase, the alignment truly does not matter,
especially because all of RAM is 1:1 mapped anyway.

When running under the OS, all memory that is part of the OS's memory
pool is entirely under its control, and this includes ACPI reclaim
memory, given that the firmware itself should not be accessing this:
the purpose of this memory type is specifically to expose information
to the OS in memory that it can use as it sees fit after it is done
processing the information.

The alignment issues we need to avoid are:
- runtime DXEs where we want to map code RO and data NX, which implies
that the boundary between the two must be 64k aligned;
- other EFI_MEMORY_RUNTIME regions that may be described with
EFI_MEMORY_WC rather than EFI_MEMORY_WB; this could be related to
implementations of the varstore or other runtime services, and the OS
has no idea what the region is for and how the runtime code is using
it;
- reserved regions that are mapped, e.g., by AML code, as a
SystemMemory OpRegion; there are examples in RAS code where the memory
needs to be mapped uncached so that stores are immediately visible to
the BMC or other agents that sit on the interconnect. The existence of
a cacheable alias of the same mapping will interfere with this, i.e.,
in many cases it will simply make both mappings cacheable.


> Or are you saying the OS can just read the 64k page(s) containing these
> 4k aligned EfiACPIReclaimMemory pages and then start reading at the
> calculated offset from within the larger page, since these pages don't
> have any pointers to outside memory, unlike image sections?
>

In all these examples, the alignment *itself* is not the goal. The 64k
alignment is way to ensure that the OS is never in a situation where a
64k page frame is shared between a cacheable and an uncacheable
mapping, or between a ROX and a W+NX mapping. Note that, in the
runtime DXE case, the only way to map the 64k page that has both code
and data is to map it RWX, given that the contents need to appear
contiguous in virtual memory (we had great fun with that when we
designed the MAT)

For the other cases, there may be mappings that are completely
disjoint in the virtual address space. But if two mappings exist of
the same physical page, one cacheable via one alias, and one
non-cacheable via another, the result is unpredictable and things like
RAS or BMC comms from runtime firmware may be completely broken.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115534): https://edk2.groups.io/g/devel/message/115534
Mute This Topic: https://groups.io/mt/104364784/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-