-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GetMemoryMap(): reserved memory property no-map #52
Comments
Discussed at 2020.08.31 meeting. Heinrich to propose a patch. Need clarity on expected behaviour. Is the OS expected to parse reserved-memory even when UEFI booting, or is the UEFI implementation expected to scrub the provided memory map. |
Closes: ARM-software#52 The no-map property of the /reserved-memory DT node is used by Linux to signal that a memory region shall not be mapped and that speculative access shall not be permitted. The memory map returned by GetMemoryMap() does not have a flag corresponding to no-map. So the closest thing we can do is not to include no-map reserved memory into the map returned by GetMemoryMap(). Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Met with @xypron, @ardbiesheuvel, and @apalos over Zoom to discuss. Agreed that any static mapping within /reserved-memory should also be present in the UEFI memory map to protect against firmware allocating that same memory for a different purpose (e.g. type == EfiRuntimeServiceData). If a static /reserved-memory node doesn't appear in the UEFI memory map, the OS would be able to use it providing there is not a conflict between the two, but this is a fragile scenario. If there is a conflict, then the OS should complain. Loudly. Dynamic allocations in /reserved-memory do not need to be in the UEFI memory map because the OS is responsible for allocating them. @xypron is going to update his proposed patch to reflect the above. Some work need to be done to map the various types of /reserved-memory nodes to entries in the UEFI map. |
On Wed, 9 Sep 2020 at 19:34, Grant Likely ***@***.***> wrote:
Met with @xypron <https://github.com/xypron>, @ardbiesheuvel
<https://github.com/ardbiesheuvel>, and @apalos
<https://github.com/apalos> over Zoom to discuss. Agreed that any static
mapping within /reserved-memory should also be present in the UEFI memory
map to protect against firmware allocating that same memory for a different
purpose (e.g. type == EfiRuntimeServiceData).
If a static /reserved-memory node doesn't appear in the UEFI memory map,
the OS would be able to use it providing there is not a conflict between
the two, but this is a fragile scenario. If there is a conflict, then the
OS should complain. Loudly.
Dynamic allocations in /reserved-memory do not need to be in the UEFI
memory map because the OS is responsible for allocating them.
@xypron <https://github.com/xypron> is going to update his proposed patch
to reflect the above. Some work need to be done to map the various types of
/reserved-memory nodes to entries in the UEFI map.
@glikely <https://github.com/glikely> is going to look into putting the
/reserved-memory binding details w/ UEFI interaction into the DT spec.
I double checked the Linux implementations (both ARM and arm64): the EFI
memory map is loaded into memblock by efi_init(), and during
arm[64]_memblock_init(), which is called later on, the reserved-memory node
is parsed and reservations and allocations are added to memblock as well,
so this looks as expected.
Currently, there does not seem to be an appropriate spot to add any
validation code, i.e., to ensure that all /reserved-memory regions are
marked appropriately in the EFI memory map. However, since this is
essentially an internal firmware issue as well (given that EFI
AllocatePages() should not allocate from those regions either), perhaps it
makes more sense to add code for this to one of the validation test suites,
once the underying requirement has been recorded in any of the pertinent
specs.
Finally, after giving this a bit more thought, I think we should use the
following mapping for /reserved-memory nodes:
- dynamic: ignore
- static no-map: EfiReservedMemory
- other static: BootServicesData
The reasons are that
a) BootServicesData is covered by the linear region, so any code that
expects this will not be surprised if it isn't, which is what could happen
when we use EfiReservedMemory for regions that are not no-map
b) BootServicesData is given to memblock as memory, but memblock_reserve()d
at the same time, which means the 'reusable' property should work the same
between UEFI and non-UEFI boot (but we should probably document that
'reusable' and 'no-map' are mutually exclusive)
c) BootServicesData is sufficient to prevent any EFI programs to allocate
from the region
|
Dear Ard, A definition of "dynamic /reserved-memory node" in device-tree terms is needed. Here are some examples:
Should U-Boot treat the respective areas as static or as dynamic? The UEFI spec has this text: "The firmware does not return a range description for address space regions that are not backed by physical hardware. Regions that are backed by physical hardware, but are not supposed to be accessed by the OS, must be returned as EfiReservedMemoryType. The OS may use addresses of memory ranges that are not described in the memory map at its own discretion." So UEFI distinguishes between backed by physical hardware and not backed by physical hardware. Nice said but we would have to define how this is determined. Do you mean by dynamic a /reserved-memory node without a backing /memory node? Is this even allowable under the device-tree spec? If it is not allowable, we would not have to care about it. Passing areas marked as /reserved-memory nodes in the device tree as BootServicesData would be a change in U-Boot. Before we make this change we probably should check this not only against old and current Linux kernels but also against BSD. I just had a look at EDK II. On my MacchiatoBin I See the following line when running memmap in the UEFI shell:
The 4000000-53FFFFF region relates to the no-map /reserved-memory reg = <0x0 0x4000000 0x0 0x200000> in Linux. Best regards |
Hello Heinrich,
On Thu, 10 Sep 2020 at 16:58, Heinrich Schuchardt ***@***.***> wrote:
Dear Ard,
A definition of "dynamic /reserved-memory node" in device-tree terms is needed.
Is the definition in
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
in the Linux tree not sufficient?
Here are some examples:
U-Boot receives a device tree from OpenBSI or QEMU or TF-A with a /reserved-memory node marked as no-map.
U-Boot loads a device tree from Linux with a /reserved-memory node marked as no-map.
U-Boot uses its internal device-tree with a /reserved-memory node marked as no-map.
Should U-Boot treat the respective areas as static or as dynamic?
Whether a node is static or dynamic is based on how the node itself is
specified.
The UEFI spec has this text: "The firmware does not return a range description for address space regions that are not backed by physical hardware. Regions that are backed by physical hardware, but are not supposed to be accessed by the OS, must be returned as EfiReservedMemoryType. The OS may use addresses of memory ranges that are not described in the memory map at its own discretion."
So UEFI distinguishes between backed by physical hardware and not backed by physical hardware. Nice said but we would have to define how this is determined.
Do you mean by dynamic a /reserved-memory node without a backing /memory node? Is this even allowable under the device-tree spec? If it is not allowable, we would not have to care about it.
'Dynamic' means allocated by the OS, so the firmware does not know its address.
________________________________
Passing areas marked as /reserved-memory nodes in the device tree as BootServicesData would be a change in U-Boot.
So how does U-Boot currently ensure that regions covered by
/reserved-memory nodes without 'no-map' are not used by
efi_allocate_pages()? The OS will expect those to be available for
whatever purpose the DT has defined for them.
… Before we make this change we probably should check this not only against old and current Linux kernels but also against BSD.
________________________________
I just had a look at EDK II. On my MacchiatoBin I See the following line when running memmap in the UEFI shell:
Reserved 0000000004000000-00000000053FFFFF 0000000000001400 0000000000000000
The 4000000-53FFFFF region relates to the no-map /reserved-memory reg = <0x0 0x4000000 0x0 0x200000> in Linux.
Best regards
Heinrich
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
On 10.09.20 16:04, Ard Biesheuvel wrote:
Hello Heinrich,
On Thu, 10 Sep 2020 at 16:58, Heinrich Schuchardt
***@***.***> wrote:
>
> Dear Ard,
>
> A definition of "dynamic /reserved-memory node" in device-tree terms
is needed.
>
Is the definition in
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
in the Linux tree not sufficient?
> Here are some examples:
>
> U-Boot receives a device tree from OpenBSI or QEMU or TF-A with a
/reserved-memory node marked as no-map.
> U-Boot loads a device tree from Linux with a /reserved-memory node
marked as no-map.
> U-Boot uses its internal device-tree with a /reserved-memory node
marked as no-map.
>
> Should U-Boot treat the respective areas as static or as dynamic?
>
Whether a node is static or dynamic is based on how the node itself is
specified.
> The UEFI spec has this text: "The firmware does not return a range
description for address space regions that are not backed by physical
hardware. Regions that are backed by physical hardware, but are not
supposed to be accessed by the OS, must be returned as
EfiReservedMemoryType. The OS may use addresses of memory ranges that
are not described in the memory map at its own discretion."
>
> So UEFI distinguishes between backed by physical hardware and not
backed by physical hardware. Nice said but we would have to define how
this is determined.
>
> Do you mean by dynamic a /reserved-memory node without a backing
/memory node? Is this even allowable under the device-tree spec? If it
is not allowable, we would not have to care about it.
>
'Dynamic' means allocated by the OS, so the firmware does not know its
address.
A first memory map has to be built by the firmware *before* invoking the
UEFI payload. The rule-set for building the memory map cannot be forward
looking and therefore cannot relate to this sort of 'dynamic'.
If "allocated by the OS" means allocated by calling AllocatePages() or
AllocatePool(), than the allocated pages cannot be excluded from the
memory map. Instead they must be of the type specified in the call. The
call will fail if the memory has not been EfiConventionalMemory before
the call.
When invoking the payload the firmware passes the device-tree. So the
payload will have to derive the memory type it asks for in a call to
AllocatePages() or AllocatePool(). The firmware should not intervene.
If "allocated by the OS" means assigned by the OS after
ExitBootServices(), than this is nothing the firmware or EBBR would have
to care about.
So I am still clueless how to implement a rule "dynamic: ignore" to
build the memory map.
What I could check is if a /reserved-memory is backed by a /memory node
and if not ignore it.
> Passing areas marked as /reserved-memory nodes in the device tree as
BootServicesData would be a change in U-Boot.
>
So how does U-Boot currently ensure that regions covered by
/reserved-memory nodes without 'no-map' are not used by
efi_allocate_pages()? The OS will expect those to be available for
whatever purpose the DT has defined for them.
Currently all /reserved-memory nodes are translated to
EfiReservedMemoryType.
Best regards
Heinrich
|
On Thu, 10 Sep 2020 at 17:34, Heinrich Schuchardt
<notifications@github.com> wrote:
On 10.09.20 16:04, Ard Biesheuvel wrote:
> Hello Heinrich,
>
> On Thu, 10 Sep 2020 at 16:58, Heinrich Schuchardt
> ***@***.***> wrote:
>>
>> Dear Ard,
>>
>> A definition of "dynamic /reserved-memory node" in device-tree terms
> is needed.
>>
>
> Is the definition in
> Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> in the Linux tree not sufficient?
>
>> Here are some examples:
>>
>> U-Boot receives a device tree from OpenBSI or QEMU or TF-A with a
> /reserved-memory node marked as no-map.
>> U-Boot loads a device tree from Linux with a /reserved-memory node
> marked as no-map.
>> U-Boot uses its internal device-tree with a /reserved-memory node
> marked as no-map.
>>
>> Should U-Boot treat the respective areas as static or as dynamic?
>>
>
> Whether a node is static or dynamic is based on how the node itself is
> specified.
>
>> The UEFI spec has this text: "The firmware does not return a range
> description for address space regions that are not backed by physical
> hardware. Regions that are backed by physical hardware, but are not
> supposed to be accessed by the OS, must be returned as
> EfiReservedMemoryType. The OS may use addresses of memory ranges that
> are not described in the memory map at its own discretion."
>>
>> So UEFI distinguishes between backed by physical hardware and not
> backed by physical hardware. Nice said but we would have to define how
> this is determined.
>>
>> Do you mean by dynamic a /reserved-memory node without a backing
> /memory node? Is this even allowable under the device-tree spec? If it
> is not allowable, we would not have to care about it.
>>
>
> 'Dynamic' means allocated by the OS, so the firmware does not know its
> address.
A first memory map has to be built by the firmware *before* invoking the
UEFI payload. The rule-set for building the memory map cannot be forward
looking and therefore cannot relate to this sort of 'dynamic'.
If "allocated by the OS" means allocated by calling AllocatePages() or
AllocatePool(), than the allocated pages cannot be excluded from the
memory map. Instead they must be of the type specified in the call. The
call will fail if the memory has not been EfiConventionalMemory before
the call.
When invoking the payload the firmware passes the device-tree. So the
payload will have to derive the memory type it asks for in a call to
AllocatePages() or AllocatePool(). The firmware should not intervene.
If "allocated by the OS" means assigned by the OS after
ExitBootServices(), than this is nothing the firmware or EBBR would have
to care about.
So I am still clueless how to implement a rule "dynamic: ignore" to
build the memory map.
You simply don't do anything. A dynamic /reserved-memory node does not
exist yet at boot, only its size is known, and it will be allocated by
the OS.
What I could check is if a /reserved-memory is backed by a /memory node
and if not ignore it.
>> Passing areas marked as /reserved-memory nodes in the device tree as
> BootServicesData would be a change in U-Boot.
>>
>
> So how does U-Boot currently ensure that regions covered by
> /reserved-memory nodes without 'no-map' are not used by
> efi_allocate_pages()? The OS will expect those to be available for
> whatever purpose the DT has defined for them.
Currently all /reserved-memory nodes are translated to
EfiReservedMemoryType.
The problem is that this conflicts with the 'reusable' attribute, and
potentially other assumptions on the part of the OS regarding reserved
memory nodes that lack to 'no-map' attribute. 'reusable' means that
the reserved memory could be used as ordinary memory by the OS, but if
it is listed as EfiReservedMemoryType, it will not be covered by the
linear region.
|
You simply don't do anything. A dynamic /reserved-memory node does not
exist yet at boot, only its size is known, and it will be allocated by
the OS.
Do we have to check in the firmware if a /reserved-memory node is backed
by a /memory node? What shall happen if not? Or is this case simply
forbidden?
> >> Passing areas marked as /reserved-memory nodes in the device tree as
> > BootServicesData would be a change in U-Boot.
> >>
> >
> > So how does U-Boot currently ensure that regions covered by
> > /reserved-memory nodes without 'no-map' are not used by
> > efi_allocate_pages()? The OS will expect those to be available for
> > whatever purpose the DT has defined for them.
>
> Currently all /reserved-memory nodes are translated to
> EfiReservedMemoryType.
>
The problem is that this conflicts with the 'reusable' attribute, and
potentially other assumptions on the part of the OS regarding reserved
memory nodes that lack to 'no-map' attribute. 'reusable' means that
the reserved memory could be used as ordinary memory by the OS, but if
it is listed as EfiReservedMemoryType, it will not be covered by the
linear region.
For 'no-map' we already agreed on EfiReservedMemoryType.
For 'reusable' I understand that we should use a memory type that can be
reclaimed by the OS after ExitBootService, e.g. BootServiceData or
LoaderData (this excludes RuntimeDriver and RuntimeData). Is there any
reason why it should not be ConventionalMemory if the memory area is not
actually used by the loader of a firmware driver? It think the firmware
should select at its own discretion.
If a /reserved-memory node carries neither of the two flags
EfiReservedMemoryType should be fine.
If a /reserved-memory node carries both of the two flags, there is a
logical contradiction. This case should be forbidden by the EBBR. Using
EfiReservedMemoryType is on the safe side.
Best regards
Heinrich
|
On Thu, 10 Sep 2020 at 18:07, Heinrich Schuchardt ***@***.***> wrote:
> You simply don't do anything. A dynamic /reserved-memory node does not
> exist yet at boot, only its size is known, and it will be allocated by
> the OS.
>
Do we have to check in the firmware if a /reserved-memory node is backed
by a /memory node? What shall happen if not? Or is this case simply
forbidden?
I don't think it matters, really. The /reserved-memory node supersedes
the /memory node in any case, so whether a reserved region exists in
/memory or not does not really make a difference.
>> >> Passing areas marked as /reserved-memory nodes in the device tree as
>> > BootServicesData would be a change in U-Boot.
>> >>
>> >
>> > So how does U-Boot currently ensure that regions covered by
>> > /reserved-memory nodes without 'no-map' are not used by
>> > efi_allocate_pages()? The OS will expect those to be available for
>> > whatever purpose the DT has defined for them.
>>
>> Currently all /reserved-memory nodes are translated to
>> EfiReservedMemoryType.
>>
>
> The problem is that this conflicts with the 'reusable' attribute, and
> potentially other assumptions on the part of the OS regarding reserved
> memory nodes that lack to 'no-map' attribute. 'reusable' means that
> the reserved memory could be used as ordinary memory by the OS, but if
> it is listed as EfiReservedMemoryType, it will not be covered by the
> linear region.
>
For 'no-map' we already agreed on EfiReservedMemoryType.
OK.
For 'reusable' I understand that we should use a memory type that can be
reclaimed by the OS after ExitBootService, e.g. BootServiceData or
LoaderData (this excludes RuntimeDriver and RuntimeData). Is there any
reason why it should not be ConventionalMemory if the memory area is not
actually used by the loader of a firmware driver? It think the firmware
should select at its own discretion.
If it is ConventionalMemory, how are you going to prevent EFI from
allocating something from it that cannot be released when the driver
needs to take ownership of it?
If a /reserved-memory node carries neither of the two flags
EfiReservedMemoryType should be fine.
Not really. We don't know how existing drivers may use these regions:
they are covered by the linear map when doing a non-EFI boot, so I
don't think we should deviate from this when booting via EFI.
If a /reserved-memory node carries both of the two flags, there is a
logical contradiction. This case should be forbidden by the EBBR. Using
EfiReservedMemoryType is on the safe side.
Agreed that there is a conflict, but this should be clarified in DT or
in the binding documentation itself, as it is not only contradictory
in the EFI context, but in any context.
|
On 10.09.20 17:21, Ard Biesheuvel wrote:
> For 'reusable' I understand that we should use a memory type that can be
> reclaimed by the OS after ExitBootService, e.g. BootServiceData or
> LoaderData (this excludes RuntimeDriver and RuntimeData). Is there any
> reason why it should not be ConventionalMemory if the memory area is not
> actually used by the loader of a firmware driver? It think the firmware
> should select at its own discretion.
>
If it is ConventionalMemory, how are you going to prevent EFI from
allocating something from it that cannot be released when the driver
needs to take ownership of it?
Your point is especially valid when the UEFI firmware loads multiple
payloads one after the other.
I think we now have enough clarity for the different cases that I can
respin the patch. Thanks for all your input.
Best regards
Heinrich
|
Sounds well thought out to me. I agree and look forward to the patch! |
Closes: ARM-software#52 The no-map property of the /reserved-memory device tree nodes is used to signal that a memory region shall not be accessed by the operating system, even not via speculative access. /reserved-memory nodes without the no-map property describe memory that have special usage by the operating system. This difference has to be reflected in the memory map returned by GetMemoryMap(). Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Closes: ARM-software#52 The no-map property of the /reserved-memory device tree nodes is used to signal that a memory region shall not be accessed by the operating system, even not via speculative access. /reserved-memory nodes without the no-map property describe memory that have special usage by the operating system. This difference has to be reflected in the memory map returned by GetMemoryMap(). Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Output from discussion on EBBR issue: ARM-software/ebbr#52 When using the UEFI ABI for booting, the DT memory map is ignored and instead UEFI GetMemoryMap() is used to find system memory. Also, regions described in /reserved-memory need to also be added to the UEFI memory map to protect against overlapping allocations. The patch adds language to cover both of those cases. Signed-off-by: Grant Likely <grant.likely@arm.com> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de> Cc: Ard Biesheuvel <ardb@kernel.org> Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Output from discussion on EBBR issue: ARM-software/ebbr#52 When using the UEFI ABI for booting, the DT memory map is ignored and instead UEFI GetMemoryMap() is used to find system memory. Also, regions described in /reserved-memory need to also be added to the UEFI memory map to protect against overlapping allocations. The patch adds language to cover both of those cases. Signed-off-by: Grant Likely <grant.likely@arm.com> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de> Cc: Ard Biesheuvel <ardb@kernel.org> Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Closing because text is committed to Devicetree spec |
SBBR has been superseded by Arm BBR. Update the about section to reference BBR instead. However, none of the specification language changes because EBBR has a relaxed set of UEFI requirements compared to BBR. Fixes: ARM-software#52 Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
The reserved memory node of the device-tree may have the following properties according to https://www.kernel.org/doc/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt:
We need should define how these properties should be modeled in the memory map returned by GetMemoryMap().
The text was updated successfully, but these errors were encountered: