[PATCH v2] schemas: Add a schema for memory map

[PATCH v2] schemas: Add a schema for memory map

[Simon Glass]

The Devicespec 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, although it could be
used to signal which area of memory has ECC.

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

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

Changes in v2:
- Reword commit message

 dtschema/schemas/memory-map.yaml | 51 ++++++++++++++++++++++++++++++++
 1 file changed, 51 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..97e531e
--- /dev/null
+++ b/dtschema/schemas/memory-map.yaml
@@ -0,0 +1,51 @@
+# 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: '/'
+  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"
+  attr:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description: |
+      Attributes possessed by this memory region:
+
+        "single-bit-ecc" - supports single-bit ECC
+        "multi-bit-ecc" - supports multiple-bit ECC
+        "no-ecc" - non-ECC memory
+
+patternProperties:
+  "^([a-z][a-z0-9\\-]+@[0-9a-f]+)?$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      reg:
+        minItems: 1
+        maxItems: 1024
+
+    required:
+      - reg
+
+additionalProperties: true
+
+...
-- 
2.42.0.rc1.204.g551eb34607-goog

Re: [PATCH v2] schemas: Add a schema for memory map

[Rob Herring]

On Mon, Aug 21, 2023 at 2:48 PM Simon Glass <sjg@chromium.org> wrote:
>
> The Devicespec specification skips over handling of a logical view of
> the memory map, pointing users to the UEFI specification.

It's more that the DT spec defines what is not used with UEFI. If UEFI
covers more than the DT Spec defined, then we should look at that.

I would look some into (IBM) PowerPC for any prior art in this area.
Unfortunately, not publicly documented other than any users.

> 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, although it could be
> used to signal which area of memory has ECC.
>
> For now, no attempt is made to create an exhaustive binding, so there are
> some example types lists. This can be completed once this has passed
> initial review.

I don't have much interest in picking this up unless there's some
wider agreement. From the previously referenced discussion[1], it
didn't seem like there was. But none of those folk are Cc'ed here.

> ---
>
> Changes in v2:
> - Reword commit message
>
>  dtschema/schemas/memory-map.yaml | 51 ++++++++++++++++++++++++++++++++
>  1 file changed, 51 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..97e531e
> --- /dev/null
> +++ b/dtschema/schemas/memory-map.yaml
> @@ -0,0 +1,51 @@
> +# 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: '/'

This goes in the root node?

> +  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"

Can't these be covered by reserved-memory? The client is free to
reclaim any regions if it knows what they are.

> +  attr:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    description: |
> +      Attributes possessed by this memory region:
> +
> +        "single-bit-ecc" - supports single-bit ECC
> +        "multi-bit-ecc" - supports multiple-bit ECC
> +        "no-ecc" - non-ECC memory

Isn't this pretty much a property of a memory region as a whole. IOW,
couldn't it just go into /memory node(s)?

> +
> +patternProperties:
> +  "^([a-z][a-z0-9\\-]+@[0-9a-f]+)?$":
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      reg:
> +        minItems: 1
> +        maxItems: 1024
> +
> +    required:
> +      - reg
> +
> +additionalProperties: true
> +
> +...
> --
> 2.42.0.rc1.204.g551eb34607-goog

[1] https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/

Re: [PATCH v2] schemas: Add a schema for memory map

[Simon Glass]

Hi Rob,

On Tue, 22 Aug 2023 at 12:53, Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Aug 21, 2023 at 2:48 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > The Devicespec specification skips over handling of a logical view of
> > the memory map, pointing users to the UEFI specification.
>
> It's more that the DT spec defines what is not used with UEFI. If UEFI
> covers more than the DT Spec defined, then we should look at that.
>
> I would look some into (IBM) PowerPC for any prior art in this area.
> Unfortunately, not publicly documented other than any users.

OK, but I'm not sure what you are looking for here. The DT (as
currently specified) is an incomplete description of memory, for
EFI-type firmware. I recall the ePAPR thing, but not much else. Any
pointers?

>
> > 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, although it could be
> > used to signal which area of memory has ECC.
> >
> > For now, no attempt is made to create an exhaustive binding, so there are
> > some example types lists. This can be completed once this has passed
> > initial review.
>
> I don't have much interest in picking this up unless there's some
> wider agreement. From the previously referenced discussion[1], it
> didn't seem like there was. But none of those folk are Cc'ed here.

Yes, Ron pointed me to that...although it isn't quite the same thing.
That is implementing a way to pass SMBIOS and ACPI tables through to
Linux, right? But it does mention memory types, so I'll send a new
version with those people on cc, in case they don't look at linux-acpi
much.

But note, this is for *firmware* (on both sides of the interface).
Whether it is useful for Linux is another question. But in any case,
we should avoid having things in the DT which Linux cannot validate /
parse.

>
> > ---
> >
> > Changes in v2:
> > - Reword commit message
> >
> >  dtschema/schemas/memory-map.yaml | 51 ++++++++++++++++++++++++++++++++
> >  1 file changed, 51 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..97e531e
> > --- /dev/null
> > +++ b/dtschema/schemas/memory-map.yaml
> > @@ -0,0 +1,51 @@
> > +# 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: '/'
>
> This goes in the root node?

I suppose I'm just confused about how the schema is described. I think
it is better to have a /memory-map node with subnodes of that for each
region.

>
> > +  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"
>
> Can't these be covered by reserved-memory? The client is free to
> reclaim any regions if it knows what they are.

I don't see that in the schema, but given what you say, it is
definitely an option.

If the reserved-memory node hiding somewhere?

>
> > +  attr:
> > +    $ref: /schemas/types.yaml#/definitions/string-array
> > +    description: |
> > +      Attributes possessed by this memory region:
> > +
> > +        "single-bit-ecc" - supports single-bit ECC
> > +        "multi-bit-ecc" - supports multiple-bit ECC
> > +        "no-ecc" - non-ECC memory
>
> Isn't this pretty much a property of a memory region as a whole. IOW,
> couldn't it just go into /memory node(s)?

Yes I think so. I wasn't sure if adding it there would break things,
but it seems not.

>
> > +
> > +patternProperties:
> > +  "^([a-z][a-z0-9\\-]+@[0-9a-f]+)?$":
> > +    type: object
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      reg:
> > +        minItems: 1
> > +        maxItems: 1024
> > +
> > +    required:
> > +      - reg
> > +
> > +additionalProperties: true
> > +
> > +...
> > --
> > 2.42.0.rc1.204.g551eb34607-goog
>
> [1] https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/

I collected email addresses from that thread as best I could, and will
cc them on v3.

Regards,
Simon

Re: [PATCH v2] schemas: Add a schema for memory map

[Rob Herring]

On Tue, Aug 22, 2023 at 3:34 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Rob,
>
> On Tue, 22 Aug 2023 at 12:53, Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Aug 21, 2023 at 2:48 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > The Devicespec specification skips over handling of a logical view of
> > > the memory map, pointing users to the UEFI specification.
> >
> > It's more that the DT spec defines what is not used with UEFI. If UEFI
> > covers more than the DT Spec defined, then we should look at that.
> >
> > I would look some into (IBM) PowerPC for any prior art in this area.
> > Unfortunately, not publicly documented other than any users.
>
> OK, but I'm not sure what you are looking for here. The DT (as
> currently specified) is an incomplete description of memory, for
> EFI-type firmware.

I thought this was for non-EFI based systems. Confused.

> I recall the ePAPR thing, but not much else. Any
> pointers?

ePAPR is the source of DT Spec. That was mainly FSL PPC, not IBM PPC.
There's something called SPAPR, but no public spec. Otherwise, it's
looking at arch/powerpc in the kernel.

> > > 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, although it could be
> > > used to signal which area of memory has ECC.
> > >
> > > For now, no attempt is made to create an exhaustive binding, so there are
> > > some example types lists. This can be completed once this has passed
> > > initial review.
> >
> > I don't have much interest in picking this up unless there's some
> > wider agreement. From the previously referenced discussion[1], it
> > didn't seem like there was. But none of those folk are Cc'ed here.
>
> Yes, Ron pointed me to that...although it isn't quite the same thing.
> That is implementing a way to pass SMBIOS and ACPI tables through to
> Linux, right? But it does mention memory types, so I'll send a new
> version with those people on cc, in case they don't look at linux-acpi
> much.

Both are defining regions of memory to pass from one stage to the
next. Isn't that the same thing?

> But note, this is for *firmware* (on both sides of the interface).
> Whether it is useful for Linux is another question. But in any case,
> we should avoid having things in the DT which Linux cannot validate /
> parse.

Perhaps it is easiest if firmware removes its private stuff. You can
put whatever you want into a DT and I don't care if it's not an ABI
between the components. You may still want to document things and have
a schema for other reasons.

> > > ---
> > >
> > > Changes in v2:
> > > - Reword commit message
> > >
> > >  dtschema/schemas/memory-map.yaml | 51 ++++++++++++++++++++++++++++++++
> > >  1 file changed, 51 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..97e531e
> > > --- /dev/null
> > > +++ b/dtschema/schemas/memory-map.yaml
> > > @@ -0,0 +1,51 @@
> > > +# 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: '/'
> >
> > This goes in the root node?
>
> I suppose I'm just confused about how the schema is described. I think
> it is better to have a /memory-map node with subnodes of that for each
> region.

What you need is $nodename should be "memory-map", not "/". There is
not a way to say it has to be under the root node other than adding it
to root-node.yaml.

> > > +  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"
> >
> > Can't these be covered by reserved-memory? The client is free to
> > reclaim any regions if it knows what they are.
>
> I don't see that in the schema, but given what you say, it is
> definitely an option.
>
> If the reserved-memory node hiding somewhere?

The DT Spec. :)

The schema is in the kernel currently in
Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml.
I need to move it out.

> > > +  attr:
> > > +    $ref: /schemas/types.yaml#/definitions/string-array
> > > +    description: |
> > > +      Attributes possessed by this memory region:
> > > +
> > > +        "single-bit-ecc" - supports single-bit ECC
> > > +        "multi-bit-ecc" - supports multiple-bit ECC
> > > +        "no-ecc" - non-ECC memory
> >
> > Isn't this pretty much a property of a memory region as a whole. IOW,
> > couldn't it just go into /memory node(s)?
>
> Yes I think so. I wasn't sure if adding it there would break things,
> but it seems not.

There's some precedence for other things already. The spec defines the
"hotpluggable" property. lshw will parse some DIMM properties which I
think date back to PowerMacs.

Rob