diff mbox series

[v3,1/2] schemas: Add a schema for memory map

Message ID 20230822203446.4111742-1-sjg@chromium.org
State New
Headers show
Series [v3,1/2] schemas: Add a schema for memory map | expand

Commit Message

Simon Glass Aug. 22, 2023, 8:34 p.m. UTC
The Devicetree specification skips over handling of a logical view of
the memory map, pointing users to the UEFI specification.

It is common to split firmware into 'Platform Init', which does the
initial hardware setup and a "Payload" which selects the OS to be booted.
Thus an handover interface is required between these two pieces.

Where UEFI boot-time services are not available, but UEFI firmware is
present on either side of this interface, information about memory usage
and attributes must be presented to the "Payload" in some form.

This aims to provide an initial schema for this mapping.

Note that this is separate from the existing /memory and /reserved-memory
nodes, since it is mostly concerned with what the memory is used for. It
may cover only a small fraction of available memory.

For now, no attempt is made to create an exhaustive binding, so there are
some example types listed. This can be completed once this has passed
initial review.

This binding does not include a binding for the memory 'attribute'
property, defined by EFI_BOOT_SERVICES.GetMemoryMap(). It may be useful
to have that as well, but perhaps not as a bit mask.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Reword commit message again
- cc a lot more people, from the FFI patch
- Split out the attributes into the /memory nodes

Changes in v2:
- Reword commit message

 dtschema/schemas/memory-map.yaml | 61 ++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 dtschema/schemas/memory-map.yaml

Comments

Mark Rutland Aug. 23, 2023, 8:58 a.m. UTC | #1
On Tue, Aug 22, 2023 at 02:34:42PM -0600, Simon Glass wrote:
> The Devicetree specification skips over handling of a logical view of
> the memory map, pointing users to the UEFI specification.
> 
> It is common to split firmware into 'Platform Init', which does the
> initial hardware setup and a "Payload" which selects the OS to be booted.
> Thus an handover interface is required between these two pieces.
> 
> Where UEFI boot-time services are not available, but UEFI firmware is
> present on either side of this interface, information about memory usage
> and attributes must be presented to the "Payload" in some form.

Today Linux does that by passing:

  /chosen/linux,uefi-mmap-start
  /chosen/linux,uefi-mmap-size
  /chosen/linux,uefi-mmap-desc-size
  /chosen/linux,uefi-mmap-desc-ver

... or /chosen/xen,* variants of those.

Can't we document / genericise that?

Pointing to that rather than re-encoding it in DT means that it stays in-sync
with the EFI spec and we won't back ourselves into a corner where we cannot
encode something due to a structural difference. I don't think it's a good idea
to try to re-encode it, or we're just setting ourselves up for futher pain.

Thanks,
Mark.

> 
> This aims to provide an initial schema for this mapping.
> 
> Note that this is separate from the existing /memory and /reserved-memory
> nodes, since it is mostly concerned with what the memory is used for. It
> may cover only a small fraction of available memory.
> 
> For now, no attempt is made to create an exhaustive binding, so there are
> some example types listed. This can be completed once this has passed
> initial review.
> 
> This binding does not include a binding for the memory 'attribute'
> property, defined by EFI_BOOT_SERVICES.GetMemoryMap(). It may be useful
> to have that as well, but perhaps not as a bit mask.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v3:
> - Reword commit message again
> - cc a lot more people, from the FFI patch
> - Split out the attributes into the /memory nodes
> 
> Changes in v2:
> - Reword commit message
> 
>  dtschema/schemas/memory-map.yaml | 61 ++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
>  create mode 100644 dtschema/schemas/memory-map.yaml
> 
> diff --git a/dtschema/schemas/memory-map.yaml b/dtschema/schemas/memory-map.yaml
> new file mode 100644
> index 0000000..4b06583
> --- /dev/null
> +++ b/dtschema/schemas/memory-map.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: BSD-2-Clause
> +# Copyright 2023 Google LLC
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-map.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: /memory-map nodes
> +description: |
> +  Common properties always required in /memory-map nodes. These nodes are
> +  intended to resolve the nonchalant clause 3.4.1 ("/memory node and UEFI")
> +  in the Devicetree Specification.
> +
> +maintainers:
> +  - Simon Glass <sjg@chromium.org>
> +
> +properties:
> +  $nodename:
> +    const: 'memory-map'
> +
> +patternProperties:
> +  "^([a-z][a-z0-9\\-]+@[0-9a-f]+)?$":
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      reg:
> +        minItems: 1
> +        maxItems: 1024
> +
> +      usage:
> +        $ref: /schemas/types.yaml#/definitions/string
> +        description: |
> +          Describes the usage of the memory region, e.g.:
> +
> +            "acpi-reclaim", "acpi-nvs", "bootcode", "bootdata", "bootdata",
> +            "runtime-code", "runtime-data".
> +
> +            See enum EFI_MEMORY_TYPE in "Unified Extensible Firmware Interface
> +            (UEFI) Specification" for all the types. For now there are not
> +            listed here.
> +
> +    required:
> +      - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    memory-map {
> +        acpi@f0000 {
> +            reg = <0xf0000 0x4000>;
> +            usage = "acpi-reclaim";
> +        };
> +
> +        runtime@12300000 {
> +            reg = <0x12300000 0x28000>;
> +            usage = "runtime-code";
> +        };
> +    };
> +...
> -- 
> 2.42.0.rc1.204.g551eb34607-goog
>
Ard Biesheuvel Aug. 24, 2023, 9:10 a.m. UTC | #2
On Wed, 23 Aug 2023 at 22:04, Simon Glass <sjg@chromium.org> wrote:
>
> Hi,
>
> On Wed, 23 Aug 2023 at 08:24, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Wed, 23 Aug 2023 at 10:59, Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > On Tue, Aug 22, 2023 at 02:34:42PM -0600, Simon Glass wrote:
> > > > The Devicetree specification skips over handling of a logical view of
> > > > the memory map, pointing users to the UEFI specification.
> > > >
> > > > It is common to split firmware into 'Platform Init', which does the
> > > > initial hardware setup and a "Payload" which selects the OS to be booted.
> > > > Thus an handover interface is required between these two pieces.
> > > >
> > > > Where UEFI boot-time services are not available, but UEFI firmware is
> > > > present on either side of this interface, information about memory usage
> > > > and attributes must be presented to the "Payload" in some form.
> >
> > Not quite.
> >
> > This seems to be intended for consumption by Linux booting in ACPI
> > mode, but not via UEFI, right?
>
> Actually, this is for consumption by firmware. The goal is to allow
> edk2 to boot into U-Boot and vice versa, i.e. provide some
> interoperability between firmware projects. I will use the "Platform
> Init" and "Payload" terminology here too.
>

OK. It was the cc to linux-acpi@ and the authors of the
ACPI/SMBIOS-without-UEFI patches that threw me off here.

If we are talking about an internal interface for firmware components,
I'd be inclined to treat this as an implementation detail, as long as
the OS is not expected to consume these DT nodes.

However, I struggle to see the point of framing this information as a
'UEFI memory map'. Neither EDK2 nor u-boot consume this information
natively, and there is already prior art in both projects to consume
nodes following the existing bindings for device_type=memory and the
/reserved-memory node. UEFI runtime memory is generally useless
without UEFI runtime services, and UEFI boot services memory is just
free memory.

There is also an overlap with the handover between secure and
non-secure firmware on arm64, which is also DT based, and communicates
available memory as well as RAM regions that are reserved for firmware
use.

In summary, I don't see why a non-UEFI payload would care about UEFI
semantics for pre-existing memory reservations, or vice versa. Note
that EDK2 will manage its own memory map, and expose it via UEFI boot
services and not via DT.

...
>
> There is no intent to implement the UEFI spec, here. It is simply that
> some payloads (EDK2) are used to having this information.
>
> Imagine splitting EDK2 into two parts, one of which does platform init
> and the other which (the payload) boots the OS. The payload wants
> information from Platform Init and it needs to be in a devicetree,
> since that is what we have chosen for this interface. So to some
> extent this is unrelated to whether you have EFI boot services. We
> just need to be able to pass the information across the interface.
> Note that the user can (without recompilation, etc.) replace the
> second part with U-Boot (for example) and it must still work.
>

OK, so device tree makes sense for this. This is how I implemented the
EDK2 port that targets QEMU/mach-virt - it consumes the DT to discover
the UART, RTC,, memory, PCI host bridge, etc.

But I don't see a use case for a UEFI memory map here.


> >
> > >
> > > Today Linux does that by passing:
> > >
> > >   /chosen/linux,uefi-mmap-start
> > >   /chosen/linux,uefi-mmap-size
> > >   /chosen/linux,uefi-mmap-desc-size
> > >   /chosen/linux,uefi-mmap-desc-ver
> > >
> > > ... or /chosen/xen,* variants of those.
> > >
> > > Can't we document / genericise that?
>
> That seems to me to be the fields from the EFI memory-map call, but
> where is the actual content? I looked in the kernel but it seems to be
> an internal interface (between the stub and the kernel)?
>
> > >
> >
> > Given the ACPI angle, promoting this to external ABI would introduce a
> > DT dependency to ACPI boot. So we'll at least have to be very clear
> > about which takes precedence, or maybe disregard everything except the
> > /chosen node when doing ACPI boot?
> >
> > This also argues for not creating an ordinary binding for this (i.e.,
> > describing it as part of the platform topology), but putting it under
> > /chosen as a Linux-only boot tweak.
> >
> > > Pointing to that rather than re-encoding it in DT means that it stays in-sync
> > > with the EFI spec and we won't back ourselves into a corner where we cannot
> > > encode something due to a structural difference. I don't think it's a good idea
> > > to try to re-encode it, or we're just setting ourselves up for futher pain.
> > >
> >
> > What I would prefer is to formalize pseudo-EFI boot and define the
> > bare required minimum (system table + memory map + config tables) in
> > an arch-agnostic manner. That way, the only thing that needs to be
> > passed via DT/boot_params/etc is the (pseudo-)EFI system table
> > address, and everything else (SMBIOS, ACPI as well as the EFI memory
> > map and even the initrd) can be passed via config tables as usual, all
> > of which is already supported in (mostly) generic kernel code.
> >

<snip some lines>

>
> Here I believe you are talking about booting the kernel in EFI mode,
> but that is not the intent of this patch. This is all about things
> happening in firmware. Now, if the payload (second) part of the
> firmware decides it wants to offer EFI boot services and boot the
> kernel via the EFI stub, then it may very well pack this information
> (with a few changes) into a system table and make it available to the
> kernel stub. But by then this FDT binding is irrelevant, since it has
> served its purpose (which, to reiterate, is to facilitate information
> passage from platform init to 'payload').
>

Indeed. As long as this binding is never consumed by the OS, I don't
have any objections to it - I just fail to see the point.
Ard Biesheuvel Aug. 29, 2023, 9:32 p.m. UTC | #3
On Tue, 29 Aug 2023 at 21:18, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ard,
>
> On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Wed, 23 Aug 2023 at 22:04, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi,
> > >
> > > On Wed, 23 Aug 2023 at 08:24, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Wed, 23 Aug 2023 at 10:59, Mark Rutland <mark.rutland@arm.com> wrote:
> > > > >
> > > > > On Tue, Aug 22, 2023 at 02:34:42PM -0600, Simon Glass wrote:
> > > > > > The Devicetree specification skips over handling of a logical view of
> > > > > > the memory map, pointing users to the UEFI specification.
> > > > > >
> > > > > > It is common to split firmware into 'Platform Init', which does the
> > > > > > initial hardware setup and a "Payload" which selects the OS to be booted.
> > > > > > Thus an handover interface is required between these two pieces.
> > > > > >
> > > > > > Where UEFI boot-time services are not available, but UEFI firmware is
> > > > > > present on either side of this interface, information about memory usage
> > > > > > and attributes must be presented to the "Payload" in some form.
> > > >
> > > > Not quite.
> > > >
> > > > This seems to be intended for consumption by Linux booting in ACPI
> > > > mode, but not via UEFI, right?
> > >
> > > Actually, this is for consumption by firmware. The goal is to allow
> > > edk2 to boot into U-Boot and vice versa, i.e. provide some
> > > interoperability between firmware projects. I will use the "Platform
> > > Init" and "Payload" terminology here too.
> > >
> >
> > OK. It was the cc to linux-acpi@ and the authors of the
> > ACPI/SMBIOS-without-UEFI patches that threw me off here.
> >
> > If we are talking about an internal interface for firmware components,
> > I'd be inclined to treat this as an implementation detail, as long as
> > the OS is not expected to consume these DT nodes.
> >
> > However, I struggle to see the point of framing this information as a
> > 'UEFI memory map'. Neither EDK2 nor u-boot consume this information
> > natively, and there is already prior art in both projects to consume
> > nodes following the existing bindings for device_type=memory and the
> > /reserved-memory node. UEFI runtime memory is generally useless
> > without UEFI runtime services, and UEFI boot services memory is just
> > free memory.
> >
> > There is also an overlap with the handover between secure and
> > non-secure firmware on arm64, which is also DT based, and communicates
> > available memory as well as RAM regions that are reserved for firmware
> > use.
> >
> > In summary, I don't see why a non-UEFI payload would care about UEFI
> > semantics for pre-existing memory reservations, or vice versa. Note
> > that EDK2 will manage its own memory map, and expose it via UEFI boot
> > services and not via DT.
>
> Bear in mind that one or both sides of this interface may be UEFI.
> There is no boot-services link between the two parts that I have
> outlined.
>

I don't understand what this means.

UEFI specifies how one component invokes another, and it is not based
on a DT binding. If the second component calls UEFI boot or runtime
services, it should be invoked in this manner. If it doesn't, then it
doesn't care about these memory reservations (and the OS will not be
booted via UEFI either)

So I feel I am missing something here. Perhaps a practical example
would be helpful?


> >
> > ...
> > >
> > > There is no intent to implement the UEFI spec, here. It is simply that
> > > some payloads (EDK2) are used to having this information.
> > >
> > > Imagine splitting EDK2 into two parts, one of which does platform init
> > > and the other which (the payload) boots the OS. The payload wants
> > > information from Platform Init and it needs to be in a devicetree,
> > > since that is what we have chosen for this interface. So to some
> > > extent this is unrelated to whether you have EFI boot services. We
> > > just need to be able to pass the information across the interface.
> > > Note that the user can (without recompilation, etc.) replace the
> > > second part with U-Boot (for example) and it must still work.
> > >
> >
> > OK, so device tree makes sense for this. This is how I implemented the
> > EDK2 port that targets QEMU/mach-virt - it consumes the DT to discover
> > the UART, RTC,, memory, PCI host bridge, etc.
> >
> > But I don't see a use case for a UEFI memory map here.
> >
> >
...
> > >
> > > Here I believe you are talking about booting the kernel in EFI mode,
> > > but that is not the intent of this patch. This is all about things
> > > happening in firmware. Now, if the payload (second) part of the
> > > firmware decides it wants to offer EFI boot services and boot the
> > > kernel via the EFI stub, then it may very well pack this information
> > > (with a few changes) into a system table and make it available to the
> > > kernel stub. But by then this FDT binding is irrelevant, since it has
> > > served its purpose (which, to reiterate, is to facilitate information
> > > passage from platform init to 'payload').
> > >
> >
> > Indeed. As long as this binding is never consumed by the OS, I don't
> > have any objections to it - I just fail to see the point.
>
> OK thanks.
>
> The point is that Platform Init and the payload need to agree about
> where certain things are in memory. It is true that this is coming
> from an EFI context, but that is just an example. Platform Init
> doesn't necessarily know whether its payload is EFI, but may set this
> up for use by the payload, just in case.
>

Platform init can communicate memory reservations to a UEFI payload if
needed, and there is prior art for this.

Platform init *cannot* communicate UEFI specific boot or runtime
reservations in this manner, as this doesn't make sense: either
Platform Init is UEFI and invokes a UEFI payload, in which case the
UEFI spec applies. In other cases, the UEFI memory regions either
don't exist or are irrelevant. The only EFI-agnostic aspect here is
RAM reservation for use by firmware in general, and this does not have
these UEFI semantics and does not need to be framed as such.
Simon Glass Aug. 30, 2023, 9:10 p.m. UTC | #4
Hi Ard,

On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 29 Aug 2023 at 21:18, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ard,
> >
> > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Wed, 23 Aug 2023 at 22:04, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Wed, 23 Aug 2023 at 08:24, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > On Wed, 23 Aug 2023 at 10:59, Mark Rutland <mark.rutland@arm.com> wrote:
> > > > > >
> > > > > > On Tue, Aug 22, 2023 at 02:34:42PM -0600, Simon Glass wrote:
> > > > > > > The Devicetree specification skips over handling of a logical view of
> > > > > > > the memory map, pointing users to the UEFI specification.
> > > > > > >
> > > > > > > It is common to split firmware into 'Platform Init', which does the
> > > > > > > initial hardware setup and a "Payload" which selects the OS to be booted.
> > > > > > > Thus an handover interface is required between these two pieces.
> > > > > > >
> > > > > > > Where UEFI boot-time services are not available, but UEFI firmware is
> > > > > > > present on either side of this interface, information about memory usage
> > > > > > > and attributes must be presented to the "Payload" in some form.
> > > > >
> > > > > Not quite.
> > > > >
> > > > > This seems to be intended for consumption by Linux booting in ACPI
> > > > > mode, but not via UEFI, right?
> > > >
> > > > Actually, this is for consumption by firmware. The goal is to allow
> > > > edk2 to boot into U-Boot and vice versa, i.e. provide some
> > > > interoperability between firmware projects. I will use the "Platform
> > > > Init" and "Payload" terminology here too.
> > > >
> > >
> > > OK. It was the cc to linux-acpi@ and the authors of the
> > > ACPI/SMBIOS-without-UEFI patches that threw me off here.
> > >
> > > If we are talking about an internal interface for firmware components,
> > > I'd be inclined to treat this as an implementation detail, as long as
> > > the OS is not expected to consume these DT nodes.
> > >
> > > However, I struggle to see the point of framing this information as a
> > > 'UEFI memory map'. Neither EDK2 nor u-boot consume this information
> > > natively, and there is already prior art in both projects to consume
> > > nodes following the existing bindings for device_type=memory and the
> > > /reserved-memory node. UEFI runtime memory is generally useless
> > > without UEFI runtime services, and UEFI boot services memory is just
> > > free memory.
> > >
> > > There is also an overlap with the handover between secure and
> > > non-secure firmware on arm64, which is also DT based, and communicates
> > > available memory as well as RAM regions that are reserved for firmware
> > > use.
> > >
> > > In summary, I don't see why a non-UEFI payload would care about UEFI
> > > semantics for pre-existing memory reservations, or vice versa. Note
> > > that EDK2 will manage its own memory map, and expose it via UEFI boot
> > > services and not via DT.
> >
> > Bear in mind that one or both sides of this interface may be UEFI.
> > There is no boot-services link between the two parts that I have
> > outlined.
> >
>
> I don't understand what this means.
>
> UEFI specifies how one component invokes another, and it is not based
> on a DT binding. If the second component calls UEFI boot or runtime
> services, it should be invoked in this manner. If it doesn't, then it
> doesn't care about these memory reservations (and the OS will not be
> booted via UEFI either)
>
> So I feel I am missing something here. Perhaps a practical example
> would be helpful?

Let's say we want to support these combinations:

Platform Init -> Payload
--------------------------------
U-Boot -> Tianocore
coreboot -> U-Boot
Tianocore -> U-Boot
Tianocore -> Tianocore
U-Boot -> U-Boot

Some of the above things have UEFI interfaces, some don't. But in the
case of Tianocore -> Tianocore we want things to work as if it were
Tianocore -> (its own handoff mechanism) Tiancore.

Some Platform Init may create runtime code which needs to accessible later.

The way I think of it is that we need to generalise the memory map a
bit. Saying that you must use UEFI boot services to discover it is too
UEFI-specific.

>
>
> > >
> > > ...
> > > >
> > > > There is no intent to implement the UEFI spec, here. It is simply that
> > > > some payloads (EDK2) are used to having this information.
> > > >
> > > > Imagine splitting EDK2 into two parts, one of which does platform init
> > > > and the other which (the payload) boots the OS. The payload wants
> > > > information from Platform Init and it needs to be in a devicetree,
> > > > since that is what we have chosen for this interface. So to some
> > > > extent this is unrelated to whether you have EFI boot services. We
> > > > just need to be able to pass the information across the interface.
> > > > Note that the user can (without recompilation, etc.) replace the
> > > > second part with U-Boot (for example) and it must still work.
> > > >
> > >
> > > OK, so device tree makes sense for this. This is how I implemented the
> > > EDK2 port that targets QEMU/mach-virt - it consumes the DT to discover
> > > the UART, RTC,, memory, PCI host bridge, etc.
> > >
> > > But I don't see a use case for a UEFI memory map here.
> > >
> > >
> ...
> > > >
> > > > Here I believe you are talking about booting the kernel in EFI mode,
> > > > but that is not the intent of this patch. This is all about things
> > > > happening in firmware. Now, if the payload (second) part of the
> > > > firmware decides it wants to offer EFI boot services and boot the
> > > > kernel via the EFI stub, then it may very well pack this information
> > > > (with a few changes) into a system table and make it available to the
> > > > kernel stub. But by then this FDT binding is irrelevant, since it has
> > > > served its purpose (which, to reiterate, is to facilitate information
> > > > passage from platform init to 'payload').
> > > >
> > >
> > > Indeed. As long as this binding is never consumed by the OS, I don't
> > > have any objections to it - I just fail to see the point.
> >
> > OK thanks.
> >
> > The point is that Platform Init and the payload need to agree about
> > where certain things are in memory. It is true that this is coming
> > from an EFI context, but that is just an example. Platform Init
> > doesn't necessarily know whether its payload is EFI, but may set this
> > up for use by the payload, just in case.
> >
>
> Platform init can communicate memory reservations to a UEFI payload if
> needed, and there is prior art for this.
>
> Platform init *cannot* communicate UEFI specific boot or runtime
> reservations in this manner, as this doesn't make sense: either
> Platform Init is UEFI and invokes a UEFI payload, in which case the
> UEFI spec applies. In other cases, the UEFI memory regions either
> don't exist or are irrelevant. The only EFI-agnostic aspect here is
> RAM reservation for use by firmware in general, and this does not have
> these UEFI semantics and does not need to be framed as such.

How does one do the handoff if we don't know whether the payload
supports UEFI or not? I am coming from an interoperability POV here.

Regards,
Simon
Ard Biesheuvel Aug. 31, 2023, 12:28 p.m. UTC | #5
On Wed, 30 Aug 2023 at 23:11, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ard,
>
> On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Tue, 29 Aug 2023 at 21:18, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Ard,
> > >
> > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel <ardb@kernel.org> wrote:
...
> > > > In summary, I don't see why a non-UEFI payload would care about UEFI
> > > > semantics for pre-existing memory reservations, or vice versa. Note
> > > > that EDK2 will manage its own memory map, and expose it via UEFI boot
> > > > services and not via DT.
> > >
> > > Bear in mind that one or both sides of this interface may be UEFI.
> > > There is no boot-services link between the two parts that I have
> > > outlined.
> > >
> >
> > I don't understand what this means.
> >
> > UEFI specifies how one component invokes another, and it is not based
> > on a DT binding. If the second component calls UEFI boot or runtime
> > services, it should be invoked in this manner. If it doesn't, then it
> > doesn't care about these memory reservations (and the OS will not be
> > booted via UEFI either)
> >
> > So I feel I am missing something here. Perhaps a practical example
> > would be helpful?
>
> Let's say we want to support these combinations:
>
> Platform Init -> Payload
> --------------------------------
> U-Boot -> Tianocore
> coreboot -> U-Boot
> Tianocore -> U-Boot
> Tianocore -> Tianocore
> U-Boot -> U-Boot
>
> Some of the above things have UEFI interfaces, some don't. But in the
> case of Tianocore -> Tianocore we want things to work as if it were
> Tianocore -> (its own handoff mechanism) Tiancore.
>

If Tianocore is the payload, it is either implemented as a EFI app, in
which case it has access to EFI services, or it is not, in which case
it doesn't care about UEFI semantics of the existing reserved regions,
and it only needs to know which regions exist and which of those are
reserved.

And I think the same applies to all other rows in your table: either
the existence of UEFI needs to be carried forward, which needs to be
done via EFI services, or it doesn't, in which case the UEFI specific
reservations can be dropped, and only reserved and available memory is
relevant.

> Some Platform Init may create runtime code which needs to accessible later.
>

But not UEFI runtime code, right? If the payload is not UEFI based,
the OS would never be able to call that runtime code unless it is
described in a different, non-UEFI way. This is fine, but it is not
UEFI so we shouldn't call it UEFI runtime memory.

> The way I think of it is that we need to generalise the memory map a
> bit. Saying that you must use UEFI boot services to discover it is too
> UEFI-specific.
>

What I am questioning is why a memory map with UEFI semantics is even
relevant when those boot services do not exist.

Could you be more specific about why a payload would have to be aware
of the existence of UEFI boot/runtime service regions if it does not
consume the UEFI interfaces of the platform init? And if the payload
exposes UEFI services to the OS, why would it consume a memory map
with UEFI semantics rather than a simple list of memblocks and memory
reservations?

Again, I am inclined to treat this as a firmware implementation
detail, and the OS must never consume this binding. But I am still
puzzled about what exact purpose it is expected to serve.
Simon Glass Aug. 31, 2023, 7:02 p.m. UTC | #6
Hi Ard,

On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 30 Aug 2023 at 23:11, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ard,
> >
> > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Tue, 29 Aug 2023 at 21:18, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Ard,
> > > >
> > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel <ardb@kernel.org> wrote:
> ...
> > > > > In summary, I don't see why a non-UEFI payload would care about UEFI
> > > > > semantics for pre-existing memory reservations, or vice versa. Note
> > > > > that EDK2 will manage its own memory map, and expose it via UEFI boot
> > > > > services and not via DT.
> > > >
> > > > Bear in mind that one or both sides of this interface may be UEFI.
> > > > There is no boot-services link between the two parts that I have
> > > > outlined.
> > > >
> > >
> > > I don't understand what this means.
> > >
> > > UEFI specifies how one component invokes another, and it is not based
> > > on a DT binding. If the second component calls UEFI boot or runtime
> > > services, it should be invoked in this manner. If it doesn't, then it
> > > doesn't care about these memory reservations (and the OS will not be
> > > booted via UEFI either)
> > >
> > > So I feel I am missing something here. Perhaps a practical example
> > > would be helpful?
> >
> > Let's say we want to support these combinations:
> >
> > Platform Init -> Payload
> > --------------------------------
> > U-Boot -> Tianocore
> > coreboot -> U-Boot
> > Tianocore -> U-Boot
> > Tianocore -> Tianocore
> > U-Boot -> U-Boot
> >
> > Some of the above things have UEFI interfaces, some don't. But in the
> > case of Tianocore -> Tianocore we want things to work as if it were
> > Tianocore -> (its own handoff mechanism) Tiancore.
> >
>
> If Tianocore is the payload, it is either implemented as a EFI app, in
> which case it has access to EFI services, or it is not, in which case
> it doesn't care about UEFI semantics of the existing reserved regions,
> and it only needs to know which regions exist and which of those are
> reserved.
>
> And I think the same applies to all other rows in your table: either
> the existence of UEFI needs to be carried forward, which needs to be
> done via EFI services, or it doesn't, in which case the UEFI specific
> reservations can be dropped, and only reserved and available memory is
> relevant.
>
> > Some Platform Init may create runtime code which needs to accessible later.
> >
>
> But not UEFI runtime code, right? If the payload is not UEFI based,
> the OS would never be able to call that runtime code unless it is
> described in a different, non-UEFI way. This is fine, but it is not
> UEFI so we shouldn't call it UEFI runtime memory.
>
> > The way I think of it is that we need to generalise the memory map a
> > bit. Saying that you must use UEFI boot services to discover it is too
> > UEFI-specific.
> >
>
> What I am questioning is why a memory map with UEFI semantics is even
> relevant when those boot services do not exist.
>
> Could you be more specific about why a payload would have to be aware
> of the existence of UEFI boot/runtime service regions if it does not
> consume the UEFI interfaces of the platform init? And if the payload
> exposes UEFI services to the OS, why would it consume a memory map
> with UEFI semantics rather than a simple list of memblocks and memory
> reservations?

It seems like you are thinking of the Payload as grub, or something
like that? This is not about grub. If there are EFI boot services to
be provided, they are provided by the Payload, not Platform Init. I am
not that familiar with Tianocore, but if you are, perhaps think of it
as splitting Tianocore into two pieces, one of which inits the silicon
and the other which provides the EFI boot services.

Again, if you are familiar with Tianocore, it can be built either as a
monolithic whole, or as a coreboot Payload. The Payload part of the
code is (roughly) the same in each case. But the Platform Init is
different[1]

>
> Again, I am inclined to treat this as a firmware implementation
> detail, and the OS must never consume this binding. But I am still
> puzzled about what exact purpose it is expected to serve.

It really is purely so we can mix and match Platform Init (perhaps
silicon init is a more familiar term?) and the Payload.

Regards,
Simon

[1] Of course, coreboot uses blobs which are chunks of UEFI, but that
is a separate issue
Ard Biesheuvel Aug. 31, 2023, 9:47 p.m. UTC | #7
On Thu, 31 Aug 2023 at 21:03, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ard,
>
> On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Wed, 30 Aug 2023 at 23:11, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Ard,
> > >
> > > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Tue, 29 Aug 2023 at 21:18, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Ard,
> > > > >
> > > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel <ardb@kernel.org> wrote:
> > ...
> > > > > > In summary, I don't see why a non-UEFI payload would care about UEFI
> > > > > > semantics for pre-existing memory reservations, or vice versa. Note
> > > > > > that EDK2 will manage its own memory map, and expose it via UEFI boot
> > > > > > services and not via DT.
> > > > >
> > > > > Bear in mind that one or both sides of this interface may be UEFI.
> > > > > There is no boot-services link between the two parts that I have
> > > > > outlined.
> > > > >
> > > >
> > > > I don't understand what this means.
> > > >
> > > > UEFI specifies how one component invokes another, and it is not based
> > > > on a DT binding. If the second component calls UEFI boot or runtime
> > > > services, it should be invoked in this manner. If it doesn't, then it
> > > > doesn't care about these memory reservations (and the OS will not be
> > > > booted via UEFI either)
> > > >
> > > > So I feel I am missing something here. Perhaps a practical example
> > > > would be helpful?
> > >
> > > Let's say we want to support these combinations:
> > >
> > > Platform Init -> Payload
> > > --------------------------------
> > > U-Boot -> Tianocore
> > > coreboot -> U-Boot
> > > Tianocore -> U-Boot
> > > Tianocore -> Tianocore
> > > U-Boot -> U-Boot
> > >
> > > Some of the above things have UEFI interfaces, some don't. But in the
> > > case of Tianocore -> Tianocore we want things to work as if it were
> > > Tianocore -> (its own handoff mechanism) Tiancore.
> > >
> >
> > If Tianocore is the payload, it is either implemented as a EFI app, in
> > which case it has access to EFI services, or it is not, in which case
> > it doesn't care about UEFI semantics of the existing reserved regions,
> > and it only needs to know which regions exist and which of those are
> > reserved.
> >
> > And I think the same applies to all other rows in your table: either
> > the existence of UEFI needs to be carried forward, which needs to be
> > done via EFI services, or it doesn't, in which case the UEFI specific
> > reservations can be dropped, and only reserved and available memory is
> > relevant.
> >
> > > Some Platform Init may create runtime code which needs to accessible later.
> > >
> >
> > But not UEFI runtime code, right? If the payload is not UEFI based,
> > the OS would never be able to call that runtime code unless it is
> > described in a different, non-UEFI way. This is fine, but it is not
> > UEFI so we shouldn't call it UEFI runtime memory.
> >
> > > The way I think of it is that we need to generalise the memory map a
> > > bit. Saying that you must use UEFI boot services to discover it is too
> > > UEFI-specific.
> > >
> >
> > What I am questioning is why a memory map with UEFI semantics is even
> > relevant when those boot services do not exist.
> >
> > Could you be more specific about why a payload would have to be aware
> > of the existence of UEFI boot/runtime service regions if it does not
> > consume the UEFI interfaces of the platform init? And if the payload
> > exposes UEFI services to the OS, why would it consume a memory map
> > with UEFI semantics rather than a simple list of memblocks and memory
> > reservations?
>
> It seems like you are thinking of the Payload as grub, or something
> like that? This is not about grub. If there are EFI boot services to
> be provided, they are provided by the Payload, not Platform Init. I am
> not that familiar with Tianocore, but if you are, perhaps think of it
> as splitting Tianocore into two pieces, one of which inits the silicon
> and the other which provides the EFI boot services.
>
> Again, if you are familiar with Tianocore, it can be built either as a
> monolithic whole, or as a coreboot Payload. The Payload part of the
> code is (roughly) the same in each case. But the Platform Init is
> different[1]
>

I co-maintain OVMF [including the ARM port that I created from
scratch] as well as the ARM architecture support in Tianocore, along
with a couple of platform ports for ARM boards, some of which could by
now be characterized as 'historical' (AMD Seattle, Socionext SynQuacer
and Raspberry Pi 3/4). So I think I have a pretty good handle on how
Tianocore based firmware is put together.

Tianocore as a payload will expose boot services to the OS, and will
provide the OS with a memory map using the UEFI APIs. But you still
haven't explained why the memory description this Tianocore inherits
from the Platform Init would include any UEFI boot or runtime service
regions, or any of the other memory regions with UEFI semantics.
TIanocore just needs to know a) where memory lives b) which parts of
it are already in use (as far as the memory map is concerned), and the
existing bindings suffice for this purpose.

In short, the memory regions with UEFI semantics are created by the
boot phase that actually exposes UEFI to the OS, in which case the
boot services can be used to obtain the memory map. If the consumer is
not UEFI based, there is no reason to bother it with descriptions of
memory regions that have no significance to it.

> >
> > Again, I am inclined to treat this as a firmware implementation
> > detail, and the OS must never consume this binding. But I am still
> > puzzled about what exact purpose it is expected to serve.
>
> It really is purely so we can mix and match Platform Init (perhaps
> silicon init is a more familiar term?) and the Payload.
>

That part is clear to me.

> [1] Of course, coreboot uses blobs which are chunks of UEFI, but that
> is a separate issue

I suppose you are referring to the proprietary FSP components? Those
are mostly made up of PEIMs (in Tianocore/EDK2 speak) for DDR training
and chipset initialization, and don't really take part in the
implementation of the UEFI APIs. Strictly, they are not 'chunks of
UEFI' but 'chunks of PI' (but even Tianocore itself, being the
reference implementation of both UEFI and PI, does a terrible job at
distinguishing between the two)
Simon Glass Aug. 31, 2023, 10:17 p.m. UTC | #8
Hi Ard,

On Thu, 31 Aug 2023 at 15:48, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 31 Aug 2023 at 21:03, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ard,
> >
> > On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Wed, 30 Aug 2023 at 23:11, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Ard,
> > > >
> > > > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > On Tue, 29 Aug 2023 at 21:18, Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Ard,
> > > > > >
> > > > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > ...
> > > > > > > In summary, I don't see why a non-UEFI payload would care about UEFI
> > > > > > > semantics for pre-existing memory reservations, or vice versa. Note
> > > > > > > that EDK2 will manage its own memory map, and expose it via UEFI boot
> > > > > > > services and not via DT.
> > > > > >
> > > > > > Bear in mind that one or both sides of this interface may be UEFI.
> > > > > > There is no boot-services link between the two parts that I have
> > > > > > outlined.
> > > > > >
> > > > >
> > > > > I don't understand what this means.
> > > > >
> > > > > UEFI specifies how one component invokes another, and it is not based
> > > > > on a DT binding. If the second component calls UEFI boot or runtime
> > > > > services, it should be invoked in this manner. If it doesn't, then it
> > > > > doesn't care about these memory reservations (and the OS will not be
> > > > > booted via UEFI either)
> > > > >
> > > > > So I feel I am missing something here. Perhaps a practical example
> > > > > would be helpful?
> > > >
> > > > Let's say we want to support these combinations:
> > > >
> > > > Platform Init -> Payload
> > > > --------------------------------
> > > > U-Boot -> Tianocore
> > > > coreboot -> U-Boot
> > > > Tianocore -> U-Boot
> > > > Tianocore -> Tianocore
> > > > U-Boot -> U-Boot
> > > >
> > > > Some of the above things have UEFI interfaces, some don't. But in the
> > > > case of Tianocore -> Tianocore we want things to work as if it were
> > > > Tianocore -> (its own handoff mechanism) Tiancore.
> > > >
> > >
> > > If Tianocore is the payload, it is either implemented as a EFI app, in
> > > which case it has access to EFI services, or it is not, in which case
> > > it doesn't care about UEFI semantics of the existing reserved regions,
> > > and it only needs to know which regions exist and which of those are
> > > reserved.
> > >
> > > And I think the same applies to all other rows in your table: either
> > > the existence of UEFI needs to be carried forward, which needs to be
> > > done via EFI services, or it doesn't, in which case the UEFI specific
> > > reservations can be dropped, and only reserved and available memory is
> > > relevant.
> > >
> > > > Some Platform Init may create runtime code which needs to accessible later.
> > > >
> > >
> > > But not UEFI runtime code, right? If the payload is not UEFI based,
> > > the OS would never be able to call that runtime code unless it is
> > > described in a different, non-UEFI way. This is fine, but it is not
> > > UEFI so we shouldn't call it UEFI runtime memory.
> > >
> > > > The way I think of it is that we need to generalise the memory map a
> > > > bit. Saying that you must use UEFI boot services to discover it is too
> > > > UEFI-specific.
> > > >
> > >
> > > What I am questioning is why a memory map with UEFI semantics is even
> > > relevant when those boot services do not exist.
> > >
> > > Could you be more specific about why a payload would have to be aware
> > > of the existence of UEFI boot/runtime service regions if it does not
> > > consume the UEFI interfaces of the platform init? And if the payload
> > > exposes UEFI services to the OS, why would it consume a memory map
> > > with UEFI semantics rather than a simple list of memblocks and memory
> > > reservations?
> >
> > It seems like you are thinking of the Payload as grub, or something
> > like that? This is not about grub. If there are EFI boot services to
> > be provided, they are provided by the Payload, not Platform Init. I am
> > not that familiar with Tianocore, but if you are, perhaps think of it
> > as splitting Tianocore into two pieces, one of which inits the silicon
> > and the other which provides the EFI boot services.
> >
> > Again, if you are familiar with Tianocore, it can be built either as a
> > monolithic whole, or as a coreboot Payload. The Payload part of the
> > code is (roughly) the same in each case. But the Platform Init is
> > different[1]
> >
>
> I co-maintain OVMF [including the ARM port that I created from
> scratch] as well as the ARM architecture support in Tianocore, along
> with a couple of platform ports for ARM boards, some of which could by
> now be characterized as 'historical' (AMD Seattle, Socionext SynQuacer
> and Raspberry Pi 3/4). So I think I have a pretty good handle on how
> Tianocore based firmware is put together.
>
> Tianocore as a payload will expose boot services to the OS, and will
> provide the OS with a memory map using the UEFI APIs. But you still
> haven't explained why the memory description this Tianocore inherits
> from the Platform Init would include any UEFI boot or runtime service
> regions, or any of the other memory regions with UEFI semantics.
> TIanocore just needs to know a) where memory lives b) which parts of
> it are already in use (as far as the memory map is concerned), and the
> existing bindings suffice for this purpose.
>
> In short, the memory regions with UEFI semantics are created by the
> boot phase that actually exposes UEFI to the OS, in which case the
> boot services can be used to obtain the memory map. If the consumer is
> not UEFI based, there is no reason to bother it with descriptions of
> memory regions that have no significance to it.

But aren't you assuming that the Payload knows how to handle the
hardware and can implement the runtime services? What if (for example)
powering off the device is hardware-specific and only Platform Init
knows how?

On another track, would it help if we just dropped all mention of
UEFI? The binding does not mention it.

On a third track, what if Platform Init wants to set aside some memory
for runtime code, e.g. in SRAM?

I'm just not sure that Platform Init and Payload are as completely
independent as you seem to be suggesting. Once we get into the
Payload, the only things we know are what Platform Init told us.

>
> > >
> > > Again, I am inclined to treat this as a firmware implementation
> > > detail, and the OS must never consume this binding. But I am still
> > > puzzled about what exact purpose it is expected to serve.
> >
> > It really is purely so we can mix and match Platform Init (perhaps
> > silicon init is a more familiar term?) and the Payload.
> >
>
> That part is clear to me.
>
> > [1] Of course, coreboot uses blobs which are chunks of UEFI, but that
> > is a separate issue
>
> I suppose you are referring to the proprietary FSP components? Those
> are mostly made up of PEIMs (in Tianocore/EDK2 speak) for DDR training
> and chipset initialization, and don't really take part in the
> implementation of the UEFI APIs. Strictly, they are not 'chunks of
> UEFI' but 'chunks of PI' (but even Tianocore itself, being the
> reference implementation of both UEFI and PI, does a terrible job at
> distinguishing between the two)

OK I see. From my POV they seem like binary blobs that are built from
UEFI, but I will try to remember about PI.

Regards,
Simon
Ard Biesheuvel Aug. 31, 2023, 10:39 p.m. UTC | #9
On Fri, 1 Sept 2023 at 00:17, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ard,
>
> On Thu, 31 Aug 2023 at 15:48, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Thu, 31 Aug 2023 at 21:03, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Ard,
> > >
> > > On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Wed, 30 Aug 2023 at 23:11, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Ard,
> > > > >
> > > > > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > >
> > > > > > On Tue, 29 Aug 2023 at 21:18, Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Ard,
> > > > > > >
> > > > > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > ...
> > > > > > > > In summary, I don't see why a non-UEFI payload would care about UEFI
> > > > > > > > semantics for pre-existing memory reservations, or vice versa. Note
> > > > > > > > that EDK2 will manage its own memory map, and expose it via UEFI boot
> > > > > > > > services and not via DT.
> > > > > > >
> > > > > > > Bear in mind that one or both sides of this interface may be UEFI.
> > > > > > > There is no boot-services link between the two parts that I have
> > > > > > > outlined.
> > > > > > >
> > > > > >
> > > > > > I don't understand what this means.
> > > > > >
> > > > > > UEFI specifies how one component invokes another, and it is not based
> > > > > > on a DT binding. If the second component calls UEFI boot or runtime
> > > > > > services, it should be invoked in this manner. If it doesn't, then it
> > > > > > doesn't care about these memory reservations (and the OS will not be
> > > > > > booted via UEFI either)
> > > > > >
> > > > > > So I feel I am missing something here. Perhaps a practical example
> > > > > > would be helpful?
> > > > >
> > > > > Let's say we want to support these combinations:
> > > > >
> > > > > Platform Init -> Payload
> > > > > --------------------------------
> > > > > U-Boot -> Tianocore
> > > > > coreboot -> U-Boot
> > > > > Tianocore -> U-Boot
> > > > > Tianocore -> Tianocore
> > > > > U-Boot -> U-Boot
> > > > >
> > > > > Some of the above things have UEFI interfaces, some don't. But in the
> > > > > case of Tianocore -> Tianocore we want things to work as if it were
> > > > > Tianocore -> (its own handoff mechanism) Tiancore.
> > > > >
> > > >
> > > > If Tianocore is the payload, it is either implemented as a EFI app, in
> > > > which case it has access to EFI services, or it is not, in which case
> > > > it doesn't care about UEFI semantics of the existing reserved regions,
> > > > and it only needs to know which regions exist and which of those are
> > > > reserved.
> > > >
> > > > And I think the same applies to all other rows in your table: either
> > > > the existence of UEFI needs to be carried forward, which needs to be
> > > > done via EFI services, or it doesn't, in which case the UEFI specific
> > > > reservations can be dropped, and only reserved and available memory is
> > > > relevant.
> > > >
> > > > > Some Platform Init may create runtime code which needs to accessible later.
> > > > >
> > > >
> > > > But not UEFI runtime code, right? If the payload is not UEFI based,
> > > > the OS would never be able to call that runtime code unless it is
> > > > described in a different, non-UEFI way. This is fine, but it is not
> > > > UEFI so we shouldn't call it UEFI runtime memory.
> > > >
> > > > > The way I think of it is that we need to generalise the memory map a
> > > > > bit. Saying that you must use UEFI boot services to discover it is too
> > > > > UEFI-specific.
> > > > >
> > > >
> > > > What I am questioning is why a memory map with UEFI semantics is even
> > > > relevant when those boot services do not exist.
> > > >
> > > > Could you be more specific about why a payload would have to be aware
> > > > of the existence of UEFI boot/runtime service regions if it does not
> > > > consume the UEFI interfaces of the platform init? And if the payload
> > > > exposes UEFI services to the OS, why would it consume a memory map
> > > > with UEFI semantics rather than a simple list of memblocks and memory
> > > > reservations?
> > >
> > > It seems like you are thinking of the Payload as grub, or something
> > > like that? This is not about grub. If there are EFI boot services to
> > > be provided, they are provided by the Payload, not Platform Init. I am
> > > not that familiar with Tianocore, but if you are, perhaps think of it
> > > as splitting Tianocore into two pieces, one of which inits the silicon
> > > and the other which provides the EFI boot services.
> > >
> > > Again, if you are familiar with Tianocore, it can be built either as a
> > > monolithic whole, or as a coreboot Payload. The Payload part of the
> > > code is (roughly) the same in each case. But the Platform Init is
> > > different[1]
> > >
> >
> > I co-maintain OVMF [including the ARM port that I created from
> > scratch] as well as the ARM architecture support in Tianocore, along
> > with a couple of platform ports for ARM boards, some of which could by
> > now be characterized as 'historical' (AMD Seattle, Socionext SynQuacer
> > and Raspberry Pi 3/4). So I think I have a pretty good handle on how
> > Tianocore based firmware is put together.
> >
> > Tianocore as a payload will expose boot services to the OS, and will
> > provide the OS with a memory map using the UEFI APIs. But you still
> > haven't explained why the memory description this Tianocore inherits
> > from the Platform Init would include any UEFI boot or runtime service
> > regions, or any of the other memory regions with UEFI semantics.
> > TIanocore just needs to know a) where memory lives b) which parts of
> > it are already in use (as far as the memory map is concerned), and the
> > existing bindings suffice for this purpose.
> >
> > In short, the memory regions with UEFI semantics are created by the
> > boot phase that actually exposes UEFI to the OS, in which case the
> > boot services can be used to obtain the memory map. If the consumer is
> > not UEFI based, there is no reason to bother it with descriptions of
> > memory regions that have no significance to it.
>
> But aren't you assuming that the Payload knows how to handle the
> hardware and can implement the runtime services? What if (for example)
> powering off the device is hardware-specific and only Platform Init
> knows how?
>

If the payload relies on the platform init for anything, it can use
whichever interface those components manage to agree on.

If this interface is UEFI, the payload can use UEFI to obtain the memory map.

If this interface is not UEFI, the UEFI memory map is irrelevant, and
existing DT bindings are available that can describe this information.

> On another track, would it help if we just dropped all mention of
> UEFI? The binding does not mention it.
>

Your binding has

+      usage:
+        $ref: /schemas/types.yaml#/definitions/string
+        description: |
+          Describes the usage of the memory region, e.g.:
+
+            "acpi-reclaim", "acpi-nvs", "bootcode", "bootdata", "bootdata",
+            "runtime-code", "runtime-data".
+
+            See enum EFI_MEMORY_TYPE in "Unified Extensible Firmware Interface
+            (UEFI) Specification" for all the types. For now there are not
+            listed here.

Every type listed is derived from a definition in the UEFI spec, which
is specifically mentioned as the source.

> On a third track, what if Platform Init wants to set aside some memory
> for runtime code, e.g. in SRAM?
>

SRAM is not the best example here, as it is typically not described by
DT using device_type=memory, and never treated as ordinary system RAM
that needs to be reserved in order to prevent the OS or other boot
stages from stepping on it. In UEFI, such a region would not appear in
the UEFI memory map at all unless the OS needed to add it to the
runtime memory map (i.e., when UEFI runtime service implementations
themselves need to access such a region, similar to how the RTC or NOR
flash may be described as MemoryMappedIo regions so that GetTime() or
GetVariable() can access the underlying peripherals)

But if the Platform Init wants to reserve some system RAM for runtime
code (e.g., for its PSCI implementation on ARM), it can add it to the
/reserved-memory node, where both the payload and the OS will be able
to find it if needed.

> I'm just not sure that Platform Init and Payload are as completely
> independent as you seem to be suggesting. Once we get into the
> Payload, the only things we know are what Platform Init told us.
>

They are not independent, and that is not at all what I am claiming.

What I am objecting to is framing the platform init<->payload memory
handover in terms of UEFI memory types, which may conflict with well
established DT bindings that already serve the same purpose. The only
difference between /reserved-memory and this binding seems to be the
collection of UEFI memory types, which don't belong there in the first
place.
Simon Glass Sept. 1, 2023, 1:12 a.m. UTC | #10
Hi Ard,

On Thu, 31 Aug 2023 at 16:39, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 1 Sept 2023 at 00:17, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ard,
> >
> > On Thu, 31 Aug 2023 at 15:48, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Thu, 31 Aug 2023 at 21:03, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Ard,
> > > >
> > > > On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > On Wed, 30 Aug 2023 at 23:11, Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Ard,
> > > > > >
> > > > > > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > >
> > > > > > > On Tue, 29 Aug 2023 at 21:18, Simon Glass <sjg@chromium.org> wrote:
> > > > > > > >
> > > > > > > > Hi Ard,
> > > > > > > >
> > > > > > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > ...
> > > > > > > > > In summary, I don't see why a non-UEFI payload would care about UEFI
> > > > > > > > > semantics for pre-existing memory reservations, or vice versa. Note
> > > > > > > > > that EDK2 will manage its own memory map, and expose it via UEFI boot
> > > > > > > > > services and not via DT.
> > > > > > > >
> > > > > > > > Bear in mind that one or both sides of this interface may be UEFI.
> > > > > > > > There is no boot-services link between the two parts that I have
> > > > > > > > outlined.
> > > > > > > >
> > > > > > >
> > > > > > > I don't understand what this means.
> > > > > > >
> > > > > > > UEFI specifies how one component invokes another, and it is not based
> > > > > > > on a DT binding. If the second component calls UEFI boot or runtime
> > > > > > > services, it should be invoked in this manner. If it doesn't, then it
> > > > > > > doesn't care about these memory reservations (and the OS will not be
> > > > > > > booted via UEFI either)
> > > > > > >
> > > > > > > So I feel I am missing something here. Perhaps a practical example
> > > > > > > would be helpful?
> > > > > >
> > > > > > Let's say we want to support these combinations:
> > > > > >
> > > > > > Platform Init -> Payload
> > > > > > --------------------------------
> > > > > > U-Boot -> Tianocore
> > > > > > coreboot -> U-Boot
> > > > > > Tianocore -> U-Boot
> > > > > > Tianocore -> Tianocore
> > > > > > U-Boot -> U-Boot
> > > > > >
> > > > > > Some of the above things have UEFI interfaces, some don't. But in the
> > > > > > case of Tianocore -> Tianocore we want things to work as if it were
> > > > > > Tianocore -> (its own handoff mechanism) Tiancore.
> > > > > >
> > > > >
> > > > > If Tianocore is the payload, it is either implemented as a EFI app, in
> > > > > which case it has access to EFI services, or it is not, in which case
> > > > > it doesn't care about UEFI semantics of the existing reserved regions,
> > > > > and it only needs to know which regions exist and which of those are
> > > > > reserved.
> > > > >
> > > > > And I think the same applies to all other rows in your table: either
> > > > > the existence of UEFI needs to be carried forward, which needs to be
> > > > > done via EFI services, or it doesn't, in which case the UEFI specific
> > > > > reservations can be dropped, and only reserved and available memory is
> > > > > relevant.
> > > > >
> > > > > > Some Platform Init may create runtime code which needs to accessible later.
> > > > > >
> > > > >
> > > > > But not UEFI runtime code, right? If the payload is not UEFI based,
> > > > > the OS would never be able to call that runtime code unless it is
> > > > > described in a different, non-UEFI way. This is fine, but it is not
> > > > > UEFI so we shouldn't call it UEFI runtime memory.
> > > > >
> > > > > > The way I think of it is that we need to generalise the memory map a
> > > > > > bit. Saying that you must use UEFI boot services to discover it is too
> > > > > > UEFI-specific.
> > > > > >
> > > > >
> > > > > What I am questioning is why a memory map with UEFI semantics is even
> > > > > relevant when those boot services do not exist.
> > > > >
> > > > > Could you be more specific about why a payload would have to be aware
> > > > > of the existence of UEFI boot/runtime service regions if it does not
> > > > > consume the UEFI interfaces of the platform init? And if the payload
> > > > > exposes UEFI services to the OS, why would it consume a memory map
> > > > > with UEFI semantics rather than a simple list of memblocks and memory
> > > > > reservations?
> > > >
> > > > It seems like you are thinking of the Payload as grub, or something
> > > > like that? This is not about grub. If there are EFI boot services to
> > > > be provided, they are provided by the Payload, not Platform Init. I am
> > > > not that familiar with Tianocore, but if you are, perhaps think of it
> > > > as splitting Tianocore into two pieces, one of which inits the silicon
> > > > and the other which provides the EFI boot services.
> > > >
> > > > Again, if you are familiar with Tianocore, it can be built either as a
> > > > monolithic whole, or as a coreboot Payload. The Payload part of the
> > > > code is (roughly) the same in each case. But the Platform Init is
> > > > different[1]
> > > >
> > >
> > > I co-maintain OVMF [including the ARM port that I created from
> > > scratch] as well as the ARM architecture support in Tianocore, along
> > > with a couple of platform ports for ARM boards, some of which could by
> > > now be characterized as 'historical' (AMD Seattle, Socionext SynQuacer
> > > and Raspberry Pi 3/4). So I think I have a pretty good handle on how
> > > Tianocore based firmware is put together.
> > >
> > > Tianocore as a payload will expose boot services to the OS, and will
> > > provide the OS with a memory map using the UEFI APIs. But you still
> > > haven't explained why the memory description this Tianocore inherits
> > > from the Platform Init would include any UEFI boot or runtime service
> > > regions, or any of the other memory regions with UEFI semantics.
> > > TIanocore just needs to know a) where memory lives b) which parts of
> > > it are already in use (as far as the memory map is concerned), and the
> > > existing bindings suffice for this purpose.
> > >
> > > In short, the memory regions with UEFI semantics are created by the
> > > boot phase that actually exposes UEFI to the OS, in which case the
> > > boot services can be used to obtain the memory map. If the consumer is
> > > not UEFI based, there is no reason to bother it with descriptions of
> > > memory regions that have no significance to it.
> >
> > But aren't you assuming that the Payload knows how to handle the
> > hardware and can implement the runtime services? What if (for example)
> > powering off the device is hardware-specific and only Platform Init
> > knows how?
> >
>
> If the payload relies on the platform init for anything, it can use
> whichever interface those components manage to agree on.
>
> If this interface is UEFI, the payload can use UEFI to obtain the memory map.

I think you are still getting mixed up with grub. Platform Init does
not necessarily provide EFI boot services, even for Tianocore. It is
the Payload which provides those services. In other words, the second
half of Tianocore does not use EFI boot services to talk to the first
half.

>
> If this interface is not UEFI, the UEFI memory map is irrelevant, and
> existing DT bindings are available that can describe this information.

Can you please point me to those?

>
> > On another track, would it help if we just dropped all mention of
> > UEFI? The binding does not mention it.
> >
>
> Your binding has
>
> +      usage:
> +        $ref: /schemas/types.yaml#/definitions/string
> +        description: |
> +          Describes the usage of the memory region, e.g.:
> +
> +            "acpi-reclaim", "acpi-nvs", "bootcode", "bootdata", "bootdata",
> +            "runtime-code", "runtime-data".
> +
> +            See enum EFI_MEMORY_TYPE in "Unified Extensible Firmware Interface
> +            (UEFI) Specification" for all the types. For now there are not
> +            listed here.
>
> Every type listed is derived from a definition in the UEFI spec, which
> is specifically mentioned as the source.

Yes, but please see the v4 or v5 patch version, where that has
changed. I sent v4 two days ago. I am worried that you are still
responding to something that I revised in response to your original
comments?

>
> > On a third track, what if Platform Init wants to set aside some memory
> > for runtime code, e.g. in SRAM?
> >
>
> SRAM is not the best example here, as it is typically not described by
> DT using device_type=memory, and never treated as ordinary system RAM
> that needs to be reserved in order to prevent the OS or other boot
> stages from stepping on it. In UEFI, such a region would not appear in
> the UEFI memory map at all unless the OS needed to add it to the
> runtime memory map (i.e., when UEFI runtime service implementations
> themselves need to access such a region, similar to how the RTC or NOR
> flash may be described as MemoryMappedIo regions so that GetTime() or
> GetVariable() can access the underlying peripherals)

OK I stand corrected.

>
> But if the Platform Init wants to reserve some system RAM for runtime
> code (e.g., for its PSCI implementation on ARM), it can add it to the
> /reserved-memory node, where both the payload and the OS will be able
> to find it if needed.

OK good. So with my binding that would be 'runtime-code@...'. I am
still not sure what the problem is here.

>
> > I'm just not sure that Platform Init and Payload are as completely
> > independent as you seem to be suggesting. Once we get into the
> > Payload, the only things we know are what Platform Init told us.
> >
>
> They are not independent, and that is not at all what I am claiming.
>
> What I am objecting to is framing the platform init<->payload memory
> handover in terms of UEFI memory types, which may conflict with well
> established DT bindings that already serve the same purpose. The only
> difference between /reserved-memory and this binding seems to be the
> collection of UEFI memory types, which don't belong there in the first
> place.

OK, so please help me get this resolved.

Regards,
Simon
Ard Biesheuvel Sept. 1, 2023, 10:48 a.m. UTC | #11
On Fri, 1 Sept 2023 at 03:12, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ard,
>
> On Thu, 31 Aug 2023 at 16:39, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 1 Sept 2023 at 00:17, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Ard,
> > >
> > > On Thu, 31 Aug 2023 at 15:48, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Thu, 31 Aug 2023 at 21:03, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Ard,
> > > > >
> > > > > On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > >
> > > > > > On Wed, 30 Aug 2023 at 23:11, Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Ard,
> > > > > > >
> > > > > > > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > > >
> > > > > > > > On Tue, 29 Aug 2023 at 21:18, Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > >
> > > > > > > > > Hi Ard,
> > > > > > > > >
> > > > > > > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > ...
> > > > > > > > > > In summary, I don't see why a non-UEFI payload would care about UEFI
> > > > > > > > > > semantics for pre-existing memory reservations, or vice versa. Note
> > > > > > > > > > that EDK2 will manage its own memory map, and expose it via UEFI boot
> > > > > > > > > > services and not via DT.
> > > > > > > > >
> > > > > > > > > Bear in mind that one or both sides of this interface may be UEFI.
> > > > > > > > > There is no boot-services link between the two parts that I have
> > > > > > > > > outlined.
> > > > > > > > >
> > > > > > > >
> > > > > > > > I don't understand what this means.
> > > > > > > >
> > > > > > > > UEFI specifies how one component invokes another, and it is not based
> > > > > > > > on a DT binding. If the second component calls UEFI boot or runtime
> > > > > > > > services, it should be invoked in this manner. If it doesn't, then it
> > > > > > > > doesn't care about these memory reservations (and the OS will not be
> > > > > > > > booted via UEFI either)
> > > > > > > >
> > > > > > > > So I feel I am missing something here. Perhaps a practical example
> > > > > > > > would be helpful?
> > > > > > >
> > > > > > > Let's say we want to support these combinations:
> > > > > > >
> > > > > > > Platform Init -> Payload
> > > > > > > --------------------------------
> > > > > > > U-Boot -> Tianocore
> > > > > > > coreboot -> U-Boot
> > > > > > > Tianocore -> U-Boot
> > > > > > > Tianocore -> Tianocore
> > > > > > > U-Boot -> U-Boot
> > > > > > >
> > > > > > > Some of the above things have UEFI interfaces, some don't. But in the
> > > > > > > case of Tianocore -> Tianocore we want things to work as if it were
> > > > > > > Tianocore -> (its own handoff mechanism) Tiancore.
> > > > > > >
> > > > > >
> > > > > > If Tianocore is the payload, it is either implemented as a EFI app, in
> > > > > > which case it has access to EFI services, or it is not, in which case
> > > > > > it doesn't care about UEFI semantics of the existing reserved regions,
> > > > > > and it only needs to know which regions exist and which of those are
> > > > > > reserved.
> > > > > >
> > > > > > And I think the same applies to all other rows in your table: either
> > > > > > the existence of UEFI needs to be carried forward, which needs to be
> > > > > > done via EFI services, or it doesn't, in which case the UEFI specific
> > > > > > reservations can be dropped, and only reserved and available memory is
> > > > > > relevant.
> > > > > >
> > > > > > > Some Platform Init may create runtime code which needs to accessible later.
> > > > > > >
> > > > > >
> > > > > > But not UEFI runtime code, right? If the payload is not UEFI based,
> > > > > > the OS would never be able to call that runtime code unless it is
> > > > > > described in a different, non-UEFI way. This is fine, but it is not
> > > > > > UEFI so we shouldn't call it UEFI runtime memory.
> > > > > >
> > > > > > > The way I think of it is that we need to generalise the memory map a
> > > > > > > bit. Saying that you must use UEFI boot services to discover it is too
> > > > > > > UEFI-specific.
> > > > > > >
> > > > > >
> > > > > > What I am questioning is why a memory map with UEFI semantics is even
> > > > > > relevant when those boot services do not exist.
> > > > > >
> > > > > > Could you be more specific about why a payload would have to be aware
> > > > > > of the existence of UEFI boot/runtime service regions if it does not
> > > > > > consume the UEFI interfaces of the platform init? And if the payload
> > > > > > exposes UEFI services to the OS, why would it consume a memory map
> > > > > > with UEFI semantics rather than a simple list of memblocks and memory
> > > > > > reservations?
> > > > >
> > > > > It seems like you are thinking of the Payload as grub, or something
> > > > > like that? This is not about grub. If there are EFI boot services to
> > > > > be provided, they are provided by the Payload, not Platform Init. I am
> > > > > not that familiar with Tianocore, but if you are, perhaps think of it
> > > > > as splitting Tianocore into two pieces, one of which inits the silicon
> > > > > and the other which provides the EFI boot services.
> > > > >
> > > > > Again, if you are familiar with Tianocore, it can be built either as a
> > > > > monolithic whole, or as a coreboot Payload. The Payload part of the
> > > > > code is (roughly) the same in each case. But the Platform Init is
> > > > > different[1]
> > > > >
> > > >
> > > > I co-maintain OVMF [including the ARM port that I created from
> > > > scratch] as well as the ARM architecture support in Tianocore, along
> > > > with a couple of platform ports for ARM boards, some of which could by
> > > > now be characterized as 'historical' (AMD Seattle, Socionext SynQuacer
> > > > and Raspberry Pi 3/4). So I think I have a pretty good handle on how
> > > > Tianocore based firmware is put together.
> > > >
> > > > Tianocore as a payload will expose boot services to the OS, and will
> > > > provide the OS with a memory map using the UEFI APIs. But you still
> > > > haven't explained why the memory description this Tianocore inherits
> > > > from the Platform Init would include any UEFI boot or runtime service
> > > > regions, or any of the other memory regions with UEFI semantics.
> > > > TIanocore just needs to know a) where memory lives b) which parts of
> > > > it are already in use (as far as the memory map is concerned), and the
> > > > existing bindings suffice for this purpose.
> > > >
> > > > In short, the memory regions with UEFI semantics are created by the
> > > > boot phase that actually exposes UEFI to the OS, in which case the
> > > > boot services can be used to obtain the memory map. If the consumer is
> > > > not UEFI based, there is no reason to bother it with descriptions of
> > > > memory regions that have no significance to it.
> > >
> > > But aren't you assuming that the Payload knows how to handle the
> > > hardware and can implement the runtime services? What if (for example)
> > > powering off the device is hardware-specific and only Platform Init
> > > knows how?
> > >
> >
> > If the payload relies on the platform init for anything, it can use
> > whichever interface those components manage to agree on.
> >
> > If this interface is UEFI, the payload can use UEFI to obtain the memory map.
>
> I think you are still getting mixed up with grub. Platform Init does
> not necessarily provide EFI boot services, even for Tianocore. It is
> the Payload which provides those services. In other words, the second
> half of Tianocore does not use EFI boot services to talk to the first
> half.
>

I might be misunderstanding your examples, as they are somewhat vague
and hypothetical.

Drawing from my experience working on Tianocore, allow me to give a
few examples myself:
- ArmVirtQemu (ARM port of OVMF) receives information about system RAM
via memory nodes in the device tree, using device_type=memory, from
QEMU, which fulfills the role of platform init in this case.
ArmVirtQemu currently doesn't consume the /reserved-memory node as
QEMU does not populate it, but that would be the appropriate place to
document RAM regions that Tianocore must treat as reserved;
- DeveloperBox [0] (based on Socionext SynQuacer) receives a platform
specific struct with memory regions and reservations in a patch of
SRAM that the early Tianocore code uses for stack and heap. Note that
system RAM is not available yet at this point (as it is being
discovered via this mechanism) and SRAM is quite tight, so DT is not
an option here;
- The Tianocore port for Raspberry Pi 4 [1] receives RAM information
from the VideoCore firmware by calling the mailbox interface. This
covers both available memory and reserved memory (for the GPU). The
statically allocated TF-A code and data regions that reside in
non-secure DRAM on this platform are reserved implicitly due to the
fact that they are part of the same firmware image, which knows not to
step on itself.

In none of these cases, I see a need for the binding that you propose.
Platfom Init -> Payload handover is highly platform specific, so
adding another generic DT binding for an as yet unidentified use case
seems seems premature at the very least.

[0] https://github.com/tianocore/edk2-platforms/tree/master/Platform/Socionext/DeveloperBox
[1] https://github.com/tianocore/edk2-platforms/tree/master/Platform/RaspberryPi/RPi4

> >
> > If this interface is not UEFI, the UEFI memory map is irrelevant, and
> > existing DT bindings are available that can describe this information.
>
> Can you please point me to those?
>

The /memory node is documented in the DT specification. The
/reserved-memory node is documented here:

Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml

> >
> > > On another track, would it help if we just dropped all mention of
> > > UEFI? The binding does not mention it.
> > >
> >
> > Your binding has
> >
> > +      usage:
> > +        $ref: /schemas/types.yaml#/definitions/string
> > +        description: |
> > +          Describes the usage of the memory region, e.g.:
> > +
> > +            "acpi-reclaim", "acpi-nvs", "bootcode", "bootdata", "bootdata",
> > +            "runtime-code", "runtime-data".
> > +
> > +            See enum EFI_MEMORY_TYPE in "Unified Extensible Firmware Interface
> > +            (UEFI) Specification" for all the types. For now there are not
> > +            listed here.
> >
> > Every type listed is derived from a definition in the UEFI spec, which
> > is specifically mentioned as the source.
>
> Yes, but please see the v4 or v5 patch version, where that has
> changed. I sent v4 two days ago. I am worried that you are still
> responding to something that I revised in response to your original
> comments?
>

No, I haven't looked at those yet. Maybe the uboot community is
different, but in the Linux community, we tend to finish discussing vN
of a series before sendindg out vN+1. This prevents the kind of
parallel track discussions that seem to be taking place here. Also, it
is highly appreciated when an author takes all feedback into account
(or at least acknowledges it) in a vN+1 submission, which is difficult
to do before the discussion around vN has concluded.

...
> >
> > But if the Platform Init wants to reserve some system RAM for runtime
> > code (e.g., for its PSCI implementation on ARM), it can add it to the
> > /reserved-memory node, where both the payload and the OS will be able
> > to find it if needed.
>
> OK good. So with my binding that would be 'runtime-code@...'. I am
> still not sure what the problem is here.
>

The problem is that you are not using /reserved-memory to describe a
memory reservation but something new that all DT consumers need to be
taught about, or they might step on memory that turns out to be
reserved. The DT ecosystem is large and heterogeneous, and any tool or
boot stage in existence is at the risk of stepping on this memory
inadvertently, whereas /reserved-memory already provides the means to
reserve it and document its nature.

> >
> > > I'm just not sure that Platform Init and Payload are as completely
> > > independent as you seem to be suggesting. Once we get into the
> > > Payload, the only things we know are what Platform Init told us.
> > >
> >
> > They are not independent, and that is not at all what I am claiming.
> >
> > What I am objecting to is framing the platform init<->payload memory
> > handover in terms of UEFI memory types, which may conflict with well
> > established DT bindings that already serve the same purpose. The only
> > difference between /reserved-memory and this binding seems to be the
> > collection of UEFI memory types, which don't belong there in the first
> > place.
>
> OK, so please help me get this resolved.
>

I have already indicated how this should be resolved in my opinion,
which is by using the /reserved-memory node to describe memory
reservations, and not this binding.
Simon Glass Sept. 1, 2023, 11:54 a.m. UTC | #12
Hi Ard,

On Fri, 1 Sept 2023 at 04:48, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 1 Sept 2023 at 03:12, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ard,
> >
> > On Thu, 31 Aug 2023 at 16:39, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Fri, 1 Sept 2023 at 00:17, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Ard,
> > > >
> > > > On Thu, 31 Aug 2023 at 15:48, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > On Thu, 31 Aug 2023 at 21:03, Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Ard,
> > > > > >
> > > > > > On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > >
> > > > > > > On Wed, 30 Aug 2023 at 23:11, Simon Glass <sjg@chromium.org> wrote:
> > > > > > > >
> > > > > > > > Hi Ard,
> > > > > > > >
> > > > > > > > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, 29 Aug 2023 at 21:18, Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Ard,
> > > > > > > > > >
> > > > > > > > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > > ...
> > > > > > > > > > > In summary, I don't see why a non-UEFI payload would care about UEFI
> > > > > > > > > > > semantics for pre-existing memory reservations, or vice versa. Note
> > > > > > > > > > > that EDK2 will manage its own memory map, and expose it via UEFI boot
> > > > > > > > > > > services and not via DT.
> > > > > > > > > >
> > > > > > > > > > Bear in mind that one or both sides of this interface may be UEFI.
> > > > > > > > > > There is no boot-services link between the two parts that I have
> > > > > > > > > > outlined.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > I don't understand what this means.
> > > > > > > > >
> > > > > > > > > UEFI specifies how one component invokes another, and it is not based
> > > > > > > > > on a DT binding. If the second component calls UEFI boot or runtime
> > > > > > > > > services, it should be invoked in this manner. If it doesn't, then it
> > > > > > > > > doesn't care about these memory reservations (and the OS will not be
> > > > > > > > > booted via UEFI either)
> > > > > > > > >
> > > > > > > > > So I feel I am missing something here. Perhaps a practical example
> > > > > > > > > would be helpful?
> > > > > > > >
> > > > > > > > Let's say we want to support these combinations:
> > > > > > > >
> > > > > > > > Platform Init -> Payload
> > > > > > > > --------------------------------
> > > > > > > > U-Boot -> Tianocore
> > > > > > > > coreboot -> U-Boot
> > > > > > > > Tianocore -> U-Boot
> > > > > > > > Tianocore -> Tianocore
> > > > > > > > U-Boot -> U-Boot
> > > > > > > >
> > > > > > > > Some of the above things have UEFI interfaces, some don't. But in the
> > > > > > > > case of Tianocore -> Tianocore we want things to work as if it were
> > > > > > > > Tianocore -> (its own handoff mechanism) Tiancore.
> > > > > > > >
> > > > > > >
> > > > > > > If Tianocore is the payload, it is either implemented as a EFI app, in
> > > > > > > which case it has access to EFI services, or it is not, in which case
> > > > > > > it doesn't care about UEFI semantics of the existing reserved regions,
> > > > > > > and it only needs to know which regions exist and which of those are
> > > > > > > reserved.
> > > > > > >
> > > > > > > And I think the same applies to all other rows in your table: either
> > > > > > > the existence of UEFI needs to be carried forward, which needs to be
> > > > > > > done via EFI services, or it doesn't, in which case the UEFI specific
> > > > > > > reservations can be dropped, and only reserved and available memory is
> > > > > > > relevant.
> > > > > > >
> > > > > > > > Some Platform Init may create runtime code which needs to accessible later.
> > > > > > > >
> > > > > > >
> > > > > > > But not UEFI runtime code, right? If the payload is not UEFI based,
> > > > > > > the OS would never be able to call that runtime code unless it is
> > > > > > > described in a different, non-UEFI way. This is fine, but it is not
> > > > > > > UEFI so we shouldn't call it UEFI runtime memory.
> > > > > > >
> > > > > > > > The way I think of it is that we need to generalise the memory map a
> > > > > > > > bit. Saying that you must use UEFI boot services to discover it is too
> > > > > > > > UEFI-specific.
> > > > > > > >
> > > > > > >
> > > > > > > What I am questioning is why a memory map with UEFI semantics is even
> > > > > > > relevant when those boot services do not exist.
> > > > > > >
> > > > > > > Could you be more specific about why a payload would have to be aware
> > > > > > > of the existence of UEFI boot/runtime service regions if it does not
> > > > > > > consume the UEFI interfaces of the platform init? And if the payload
> > > > > > > exposes UEFI services to the OS, why would it consume a memory map
> > > > > > > with UEFI semantics rather than a simple list of memblocks and memory
> > > > > > > reservations?
> > > > > >
> > > > > > It seems like you are thinking of the Payload as grub, or something
> > > > > > like that? This is not about grub. If there are EFI boot services to
> > > > > > be provided, they are provided by the Payload, not Platform Init. I am
> > > > > > not that familiar with Tianocore, but if you are, perhaps think of it
> > > > > > as splitting Tianocore into two pieces, one of which inits the silicon
> > > > > > and the other which provides the EFI boot services.
> > > > > >
> > > > > > Again, if you are familiar with Tianocore, it can be built either as a
> > > > > > monolithic whole, or as a coreboot Payload. The Payload part of the
> > > > > > code is (roughly) the same in each case. But the Platform Init is
> > > > > > different[1]
> > > > > >
> > > > >
> > > > > I co-maintain OVMF [including the ARM port that I created from
> > > > > scratch] as well as the ARM architecture support in Tianocore, along
> > > > > with a couple of platform ports for ARM boards, some of which could by
> > > > > now be characterized as 'historical' (AMD Seattle, Socionext SynQuacer
> > > > > and Raspberry Pi 3/4). So I think I have a pretty good handle on how
> > > > > Tianocore based firmware is put together.
> > > > >
> > > > > Tianocore as a payload will expose boot services to the OS, and will
> > > > > provide the OS with a memory map using the UEFI APIs. But you still
> > > > > haven't explained why the memory description this Tianocore inherits
> > > > > from the Platform Init would include any UEFI boot or runtime service
> > > > > regions, or any of the other memory regions with UEFI semantics.
> > > > > TIanocore just needs to know a) where memory lives b) which parts of
> > > > > it are already in use (as far as the memory map is concerned), and the
> > > > > existing bindings suffice for this purpose.
> > > > >
> > > > > In short, the memory regions with UEFI semantics are created by the
> > > > > boot phase that actually exposes UEFI to the OS, in which case the
> > > > > boot services can be used to obtain the memory map. If the consumer is
> > > > > not UEFI based, there is no reason to bother it with descriptions of
> > > > > memory regions that have no significance to it.
> > > >
> > > > But aren't you assuming that the Payload knows how to handle the
> > > > hardware and can implement the runtime services? What if (for example)
> > > > powering off the device is hardware-specific and only Platform Init
> > > > knows how?
> > > >
> > >
> > > If the payload relies on the platform init for anything, it can use
> > > whichever interface those components manage to agree on.
> > >
> > > If this interface is UEFI, the payload can use UEFI to obtain the memory map.
> >
> > I think you are still getting mixed up with grub. Platform Init does
> > not necessarily provide EFI boot services, even for Tianocore. It is
> > the Payload which provides those services. In other words, the second
> > half of Tianocore does not use EFI boot services to talk to the first
> > half.
> >
>
> I might be misunderstanding your examples, as they are somewhat vague
> and hypothetical.
>
> Drawing from my experience working on Tianocore, allow me to give a
> few examples myself:
> - ArmVirtQemu (ARM port of OVMF) receives information about system RAM
> via memory nodes in the device tree, using device_type=memory, from
> QEMU, which fulfills the role of platform init in this case.
> ArmVirtQemu currently doesn't consume the /reserved-memory node as
> QEMU does not populate it, but that would be the appropriate place to
> document RAM regions that Tianocore must treat as reserved;
> - DeveloperBox [0] (based on Socionext SynQuacer) receives a platform
> specific struct with memory regions and reservations in a patch of
> SRAM that the early Tianocore code uses for stack and heap. Note that
> system RAM is not available yet at this point (as it is being
> discovered via this mechanism) and SRAM is quite tight, so DT is not
> an option here;
> - The Tianocore port for Raspberry Pi 4 [1] receives RAM information
> from the VideoCore firmware by calling the mailbox interface. This
> covers both available memory and reserved memory (for the GPU). The
> statically allocated TF-A code and data regions that reside in
> non-secure DRAM on this platform are reserved implicitly due to the
> fact that they are part of the same firmware image, which knows not to
> step on itself.
>
> In none of these cases, I see a need for the binding that you propose.
> Platfom Init -> Payload handover is highly platform specific, so
> adding another generic DT binding for an as yet unidentified use case
> seems seems premature at the very least.

I am working with the Tiancocore people and volunteered to submit this
binding on their behalf. I did try suggesting it was not needed, but
it seems that it is.

>
> [0] https://github.com/tianocore/edk2-platforms/tree/master/Platform/Socionext/DeveloperBox
> [1] https://github.com/tianocore/edk2-platforms/tree/master/Platform/RaspberryPi/RPi4
>
> > >
> > > If this interface is not UEFI, the UEFI memory map is irrelevant, and
> > > existing DT bindings are available that can describe this information.
> >
> > Can you please point me to those?
> >
>
> The /memory node is documented in the DT specification. The
> /reserved-memory node is documented here:
>
> Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml

OK, so I think we have reached a conclusion here.

>
> > >
> > > > On another track, would it help if we just dropped all mention of
> > > > UEFI? The binding does not mention it.
> > > >
> > >
> > > Your binding has
> > >
> > > +      usage:
> > > +        $ref: /schemas/types.yaml#/definitions/string
> > > +        description: |
> > > +          Describes the usage of the memory region, e.g.:
> > > +
> > > +            "acpi-reclaim", "acpi-nvs", "bootcode", "bootdata", "bootdata",
> > > +            "runtime-code", "runtime-data".
> > > +
> > > +            See enum EFI_MEMORY_TYPE in "Unified Extensible Firmware Interface
> > > +            (UEFI) Specification" for all the types. For now there are not
> > > +            listed here.
> > >
> > > Every type listed is derived from a definition in the UEFI spec, which
> > > is specifically mentioned as the source.
> >
> > Yes, but please see the v4 or v5 patch version, where that has
> > changed. I sent v4 two days ago. I am worried that you are still
> > responding to something that I revised in response to your original
> > comments?
> >
>
> No, I haven't looked at those yet. Maybe the uboot community is
> different, but in the Linux community, we tend to finish discussing vN
> of a series before sendindg out vN+1. This prevents the kind of
> parallel track discussions that seem to be taking place here. Also, it
> is highly appreciated when an author takes all feedback into account
> (or at least acknowledges it) in a vN+1 submission, which is difficult
> to do before the discussion around vN has concluded.

OK. I tend to react to feedback and try to rework my patches accordingly.

>
> ...
> > >
> > > But if the Platform Init wants to reserve some system RAM for runtime
> > > code (e.g., for its PSCI implementation on ARM), it can add it to the
> > > /reserved-memory node, where both the payload and the OS will be able
> > > to find it if needed.
> >
> > OK good. So with my binding that would be 'runtime-code@...'. I am
> > still not sure what the problem is here.
> >
>
> The problem is that you are not using /reserved-memory to describe a
> memory reservation but something new that all DT consumers need to be
> taught about, or they might step on memory that turns out to be
> reserved. The DT ecosystem is large and heterogeneous, and any tool or
> boot stage in existence is at the risk of stepping on this memory
> inadvertently, whereas /reserved-memory already provides the means to
> reserve it and document its nature.
>
> > >
> > > > I'm just not sure that Platform Init and Payload are as completely
> > > > independent as you seem to be suggesting. Once we get into the
> > > > Payload, the only things we know are what Platform Init told us.
> > > >
> > >
> > > They are not independent, and that is not at all what I am claiming.
> > >
> > > What I am objecting to is framing the platform init<->payload memory
> > > handover in terms of UEFI memory types, which may conflict with well
> > > established DT bindings that already serve the same purpose. The only
> > > difference between /reserved-memory and this binding seems to be the
> > > collection of UEFI memory types, which don't belong there in the first
> > > place.
> >
> > OK, so please help me get this resolved.
> >
>
> I have already indicated how this should be resolved in my opinion,
> which is by using the /reserved-memory node to describe memory
> reservations, and not this binding.

OK, that is the direction I took from Rob's email a week ago. Let's
continue any discussion on the v5 series.

Regards,
Simon
diff mbox series

Patch

diff --git a/dtschema/schemas/memory-map.yaml b/dtschema/schemas/memory-map.yaml
new file mode 100644
index 0000000..4b06583
--- /dev/null
+++ b/dtschema/schemas/memory-map.yaml
@@ -0,0 +1,61 @@ 
+# SPDX-License-Identifier: BSD-2-Clause
+# Copyright 2023 Google LLC
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-map.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: /memory-map nodes
+description: |
+  Common properties always required in /memory-map nodes. These nodes are
+  intended to resolve the nonchalant clause 3.4.1 ("/memory node and UEFI")
+  in the Devicetree Specification.
+
+maintainers:
+  - Simon Glass <sjg@chromium.org>
+
+properties:
+  $nodename:
+    const: 'memory-map'
+
+patternProperties:
+  "^([a-z][a-z0-9\\-]+@[0-9a-f]+)?$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      reg:
+        minItems: 1
+        maxItems: 1024
+
+      usage:
+        $ref: /schemas/types.yaml#/definitions/string
+        description: |
+          Describes the usage of the memory region, e.g.:
+
+            "acpi-reclaim", "acpi-nvs", "bootcode", "bootdata", "bootdata",
+            "runtime-code", "runtime-data".
+
+            See enum EFI_MEMORY_TYPE in "Unified Extensible Firmware Interface
+            (UEFI) Specification" for all the types. For now there are not
+            listed here.
+
+    required:
+      - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    memory-map {
+        acpi@f0000 {
+            reg = <0xf0000 0x4000>;
+            usage = "acpi-reclaim";
+        };
+
+        runtime@12300000 {
+            reg = <0x12300000 0x28000>;
+            usage = "runtime-code";
+        };
+    };
+...