Hi Ard, On Wed, Sep 6, 2023, 10:09 Ard Biesheuvel <a...@kernel.org> wrote:
> On Wed, 6 Sept 2023 at 16:54, Simon Glass <s...@chromium.org> wrote: > > > > Hi Rob, Ard, > > > > On Wed, 6 Sept 2023 at 08:34, Rob Herring <r...@kernel.org> wrote: > > > > > > On Tue, Sep 5, 2023 at 4:44 PM Ard Biesheuvel <a...@kernel.org> wrote: > > > > > > > > On Thu, 31 Aug 2023 at 01:18, Simon Glass <s...@chromium.org> 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. > > > > > > > > > > > > > I don't think the UEFI references are needed or helpful here. > > > > > > > > > This aims to provide an small schema addition for this mapping. > > > > > > > > > > For now, no attempt is made to create an exhaustive binding, so > there are > > > > > some example types listed. More can be added later. > > > > > > > > > > The compatible string is not included, since the node name is > enough to > > > > > indicate the purpose of a node, as per the existing reserved-memory > > > > > schema. > > > > > > Node names reflect the 'class', but not what's specifically in the > > > node. So really, all reserved-memory nodes should have the same name, > > > but that ship already sailed for existing users. 'compatible' is the > > > right thing here. As to what the node name should be, well, we haven't > > > defined that. I think we just used 'memory' on some platforms. > > > > OK > > > > > > > > > > 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 <s...@chromium.org> > > > > > --- > > > > > > > > > > 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 | 53 > +++++++++++++++++++ > > > > > 1 file changed, 53 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..d1b466b > > > > > --- /dev/null > > > > > +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml > > > > > @@ -0,0 +1,53 @@ > > > > > +# 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 node name. > > > > > + > > > > > + Clients may reuse this reserved memory if they understand what > it is for. > > > > > + > > > > > +maintainers: > > > > > + - Simon Glass <s...@chromium.org> > > > > > + > > > > > +allOf: > > > > > + - $ref: reserved-memory.yaml > > > > > + > > > > > +properties: > > > > > + $nodename: > > > > > + enum: > > > > > + - acpi-reclaim > > > > > + - acpi-nvs > > > > > + - boot-code > > > > > + - boot-data > > > > > + - runtime-code > > > > > + - runtime-data > > > > > + > > > > > > > > These types are used by firmware to describe the nature of certain > > > > memory regions to the OS. Boot code and data can be discarded, as > well > > > > as ACPI reclaim after its contents have been consumed. Runtime code > > > > and data need to be mapped for runtime features to work. > > > > > > > > When one firmware phase communicates the purpose of a certain memory > > > > reservation to another, it is typically not limited to whether its > > > > needs to be preserved and when it needs to be mapped (and with which > > > > attributes). I'd expect a memory reservation appearing under this > node > > > > to have a clearly defined purpose, and the subsequent phases need to > > > > be able to discover this information. > > > > > > > > For example, a communication buffer for secure<->non-secure > > > > communication or a page with spin tables used by PSCI. None of the > > > > proposed labels are appropriate for this, and I'd much rather have a > > > > compatible string or some other property that clarifies the nature in > > > > a more suitable way. Note that 'no-map' already exists to indicate > > > > that the CPU should not map this memory unless it does so for the > > > > specific purpose that the reservation was made for. > > > > > > I agree. I think compatible is the better approach. Some property like > > > 'discard' may not be sufficient information if the OS needs to consume > > > the region first and then discard it. Better to state exactly what's > > > there and then the OS can imply the rest. > > > > OK, so what sort of compatible strings? > > > > How about: > > "acpi-reclaim" - holds ACPI tables; memory can be reclaimed once the > > tables are read and no-longer needed > > ACPI reclaim is a policy, not a purpose. This memory could contain > many different things. > > > "boot-code" - holds boot code; memory can be reclaimed once the boot > > phase is complete > > "runtime-code" - holds runtime code; memory can be reclaimed only if > > this code will not be used from that point > > > > These are also policies. They can be inferred from the purpose. > > > etc. We can then have more specific compatibles, like: > > > > "psci-spin-table" - holds PSCI spin tables > > > > so you could do: > > > > compatible = "runtime-code", "psci-spin-table"; > > > > I understand that this binding targets firmware<->firmware rather than > firmware<->OS, which makes it much more difficult to keep it both > generic and sufficiently descriptive. > > However, I still feel that all the overlap with UEFI memory types is > not what we want here. UEFI knows how to manage its own memory map, > what it needs to know is what memory is already in use and for which > exact purpose. Whether or not that implies that the memory can be > freed at some point or can be mapped or not should follow from that. > Can you please make a suggestion? I am unsure what you are looking for. Regards, Simon