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

[Rob Herring]

On Tue, Aug 22, 2023 at 3:34 PM Simon Glass <sjg@xxxxxxxxxxxx> wrote:
>
> Hi Rob,
>
> On Tue, 22 Aug 2023 at 12:53, Rob Herring <robh@xxxxxxxxxx> wrote:
> >
> > On Mon, Aug 21, 2023 at 2:48 PM Simon Glass <sjg@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> > > +
> > > +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