Hi Rob, On Thu, 21 Sept 2023 at 10:45, Simon Glass <sjg@xxxxxxxxxxxx> wrote: > > Hi Rob, > > On Thu, 21 Sept 2023 at 09:20, Rob Herring <robh@xxxxxxxxxx> wrote: > > > > On Thu, Sep 21, 2023 at 9:38 AM Simon Glass <sjg@xxxxxxxxxxxx> wrote: > > > > > > Hi Rob, > > > > > > On Thu, 21 Sept 2023 at 08:25, Rob Herring <robh@xxxxxxxxxx> wrote: > > > > > > > > On Thu, Sep 7, 2023 at 4:40 PM Simon Glass <sjg@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> > > > > > --- > > > > > > > > > > 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@xxxxxxxxxxxx> > > > > > + > > > > > +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? > > > > > > Yes acpi-tables would be 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. > > > > > > OK...so what about all that discussion about being able to reclaim the > > > memory if you know what it is for? Where should that be written? Or is > > > it somewhere else already? > > > > Reclaiming is a policy of the consumer of the data. It belongs in the > > documentation of the consumer if you are going to document it. > > > > The global policy is you can't use reserved regions and you can't > > reclaim them if you don't know what they are. If you want to add > > something to that effect in reserved-memory.yaml or the spec, that's > > fine, but that's not a per compatible thing. > > OK I will do that. > > > > > > > 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? > > > > > > Yes > > > > > > > > > > > > + the OS when it is running > > > > > > > > I thought these were for stages before we get to OS? > > > > > > Yes...but of course these will be passed on to the OS in some form. > > > See above re reclaiming. > > > > > > > > > > > > + 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? > > > > > > I really don't want to do another round of that circle. I was asked to > > > drop mention of EFI which I did. If these are too vague, what should I > > > do instead? Perhaps: > > > > > > boot-code / data > > > os-code / data > > > > The kernel is boot code (and os code and runtime code). Can I use > > What do you mean by that? The kernel is an OS. It might have an EFI > stub or other stuff, but it is not a boot loader. We have to have some > generally accepted terms for what is the OS and what is the firmware. > > > these for the kernel image? Certainly not. But they are too vague to > > understand when to use them and when not to. Are they specific to EDK2 > > implementation? Then perhaps they need an 'edk2' prefix. > > > > Either these are related to EFI boot time services and runtime > > services or they aren't. Which is it? If they are related, then EFI > > should certainly be mentioned. Review comments are wrong all the time > > when the reasoning is missing or misunderstood. Please back up and > > explain why these are needed. Maybe it's buried in prior threads > > somewhere, but I'm not going back to re-read all that. This patch has > > to stand on its own. > > For my email client it is buried in this thread, see the ~20 messages > above, or [1]. I took all the EFI stuff out of here because it was > going nowhere. > > The text I removed in this version is: > > >>> > 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. > <<< > > Then I tried to explain it in terms of Tianocore being split into two > parts, one of which does Platform Init (e.g. silicon setup) and the > other which boots the OS. But that got lost is discussion about DXEs, > etc. There is no other communication mechanism between these two > pieces, nor does either piece know (at runtime or build time) whether > the other is there, or whether it is something else (e.g. coreboot or > U-Boot). This allows supporting Platform Init / Payload combinations > like coreboot->Tianocore, Tianocore->Tianocore. coreboot->U-Boot, > etc., i.e. universal compatibility. > > IMhO this binding doesn't need to be about EFI. The concept of code > which is used during boot and code which is used by the OS doesn't > seem specific to EFI. After all, ARM has PSCI code which presumably > lives somewhere. > > So do you really want it to mention EFI? > > I have volunteered to take this on on behalf of the EDK2 people, who > are copied on this thread and hoping / relying on something being > resolved. Do you have any thoughts on the above? I'd really like to understand what needs to be done here. We have a meeting tomorrow on the handoff format and I don't have a lot to show for my efforts of the last 5 weeks. Regards, SImon > [1] https://lore.kernel.org/lkml/CAMj1kXHGpCt8qkd6XYQF8mMdivQkTnEWjv6NzsFK=+N72LAn=Q@xxxxxxxxxxxxxx/
