Re: [PATCH v6 1/2] schemas: Add some common reserved-memory usages

[Rob Herring <robh@kernel.org>]

On Thu, Sep 7, 2023 at 4:40 PM Simon Glass <sjg@chromium.org> wrote:
>
> 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.
>
> This aims to provide an small schema addition for the memory mapping
> needed to keep these two pieces working together well.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v6:
> - Drop mention of UEFI
> - Use compatible strings instead of node names
>
> Changes in v5:
> - Drop the memory-map node (should have done that in v4)
> - Tidy up schema a bit
>
> Changes in v4:
> - Make use of the reserved-memory node instead of creating a new one
>
> 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
>
>  .../reserved-memory/common-reserved.yaml      | 71 +++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 dtschema/schemas/reserved-memory/common-reserved.yaml
>
> diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml b/dtschema/schemas/reserved-memory/common-reserved.yaml
> new file mode 100644
> index 0000000..4889f59
> --- /dev/null
> +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/common-reserved.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common memory reservations
> +
> +description: |
> +  Specifies that the reserved memory region can be used for the purpose
> +  indicated by its compatible string.
> +
> +  Clients may reuse this reserved memory if they understand what it is for,
> +  subject to the notes below.
> +
> +maintainers:
> +  - Simon Glass <sjg@chromium.org>
> +
> +allOf:
> +  - $ref: reserved-memory.yaml
> +
> +properties:
> +  compatible:
> +    description: |
> +      This describes some common memory reservations:
> +
> +         acpi-reclaim: Contains ACPI tables; memory may be reclaimed when the
> +           tables are no-longer needed

I think you are mixing 2 things with the name here. What the memory
contains and what to do with it. You don't need the latter. The
consumer of the region will know what to do with it if anything based
on knowing what is in the region. For example, The DTB passed to the
OS is typically in a reserved region (probably still /mem-reserve/
though). The DTB may remain there forever or the OS could copy it
somewhere else and free the reserved region. The Linux kernel does
both depending on the arch. (Of course there is no "dtb" compatible
because we have to pass the location of the dtb to even find the
reserved regions in the first place.)

So the question here is whether just "acpi" (or "acpi-tables"?) would
be explicit enough?

> +         acpi-nvs: Contains ACPI Non-volatile-storage data; memory may be
> +           reclaimed when the tables are no-longer needed

No need to say anything about reclaiming.

I know some ACPIisms (e.g. DSDT), but I don't know what NVS or
"Non-volatile-storage data" is in an ACPI context.

> +         boot-code: Contains code used for booting; memory may be reclaimed by
> +           the OS when it is running
> +         boot-code: Contains data used for booting; memory may be reclaimed by

boot-data?

> +           the OS when it is running

I thought these were for stages before we get to OS?

> +         runtime-code: Contains code used for interacting with the system when
> +           running; memory may be reclaimed if this code is not called
> +         runtime-data: Contains data used for interacting with the system when
> +           running; memory may be reclaimed if the runtime code is not used

"boot" vs. "runtime" seem too vague. However, if these mean EFI boot
time services and runtime services, then I understand exactly what
they are. In that case dropping 'uefi,' was a mistake. But EFI has its
own way to define these regions, right?

Rob