[PATCH v6 1/2] schemas: Add some common reserved-memory usages
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 + acpi-nvs: Contains ACPI Non-volatile-storage data; memory may be + reclaimed when the tables are no-longer needed + 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 + the OS when it is running + 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 + enum: + - acpi-reclaim + - acpi-nvs + - boot-code + - boot-data + - runtime-code + - runtime-data + + reg: + description: region of memory that is reserved for the purpose indicated + by the compatible string. + +required: + - reg + +unevaluatedProperties: false + +examples: + - | + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + + reserved@12340000 { + compatible = "boot-code"; + reg = <0x12340000 0x00800000>; + }; + + reserved@43210000 { + compatible = "boot-data"; + reg = <0x43210000 0x00800000>; + }; + };
Some memories provide ECC detection and/or correction. For software which wants to check memory, it is helpful to see which regions provide this feature.
Add this as a property of the /memory nodes, since it presumably follows the hardware-level memory system.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v6: - Use a number of bits instead of a string property - Fix inidcates typo
Changes in v5: - Redo to make this property specific to ECC - Provide properties both for detection and correction
Changes in v3: - Add new patch to update the /memory nodes
dtschema/schemas/memory.yaml | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/dtschema/schemas/memory.yaml b/dtschema/schemas/memory.yaml index 1d74410..1a48b1c 100644 --- a/dtschema/schemas/memory.yaml +++ b/dtschema/schemas/memory.yaml @@ -31,10 +31,21 @@ patternProperties:
numa-node-id: $ref: types.yaml#/definitions/uint32 - description: + description: | For the purpose of identification, each NUMA node is associated with a unique token known as a node id. - + ecc-detection-bits: + default: 0 + description: | + If present, this indicates the number of bits of memory error which + can be detected and reported by the Error-Correction Code (ECC) memory + subsystem (typically 0, 1 or 2). + ecc-correction-bits: + default: 0 + description: | + If present, this indicates the number of bits of memory error which + can be corrected by the Error-Correction Code (ECC) memory subsystem + (typically 0, 1 or 2).
required: - device_type
On Thu, Sep 7, 2023 at 11:57 PM Simon Glass sjg@chromium.org wrote:
Some memories provide ECC detection and/or correction. For software which wants to check memory, it is helpful to see which regions provide this feature.
Add this as a property of the /memory nodes, since it presumably follows the hardware-level memory system.
Have these been accepted upstream? A grep of bindings in the upstream kernel I don't see them.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v6:
- Use a number of bits instead of a string property
- Fix inidcates typo
Changes in v5:
- Redo to make this property specific to ECC
- Provide properties both for detection and correction
Changes in v3:
- Add new patch to update the /memory nodes
dtschema/schemas/memory.yaml | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/dtschema/schemas/memory.yaml b/dtschema/schemas/memory.yaml index 1d74410..1a48b1c 100644 --- a/dtschema/schemas/memory.yaml +++ b/dtschema/schemas/memory.yaml @@ -31,10 +31,21 @@ patternProperties:
numa-node-id: $ref: types.yaml#/definitions/uint32
description:
description: | For the purpose of identification, each NUMA node is associated with a unique token known as a node id.
ecc-detection-bits:default: 0description: |If present, this indicates the number of bits of memory error whichcan be detected and reported by the Error-Correction Code (ECC) memorysubsystem (typically 0, 1 or 2).ecc-correction-bits:default: 0description: |If present, this indicates the number of bits of memory error whichcan be corrected by the Error-Correction Code (ECC) memory subsystem(typically 0, 1 or 2).required: - device_type
-- 2.42.0.283.g2d96d420d3-goog
On Thu, Sep 7, 2023 at 4:40 PM Simon Glass sjg@chromium.org wrote:
Some memories provide ECC detection and/or correction. For software which wants to check memory, it is helpful to see which regions provide this feature.
Add this as a property of the /memory nodes, since it presumably follows the hardware-level memory system.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v6:
- Use a number of bits instead of a string property
- Fix inidcates typo
Changes in v5:
- Redo to make this property specific to ECC
- Provide properties both for detection and correction
Changes in v3:
- Add new patch to update the /memory nodes
dtschema/schemas/memory.yaml | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/dtschema/schemas/memory.yaml b/dtschema/schemas/memory.yaml index 1d74410..1a48b1c 100644 --- a/dtschema/schemas/memory.yaml +++ b/dtschema/schemas/memory.yaml @@ -31,10 +31,21 @@ patternProperties:
numa-node-id: $ref: types.yaml#/definitions/uint32
description:
description: |
Why? '|' is not needed for any of these.
For the purpose of identification, each NUMA node is associated with a unique token known as a node id.
blank line between properties.
I can fix these up when applying.
ecc-detection-bits:default: 0description: |If present, this indicates the number of bits of memory error whichcan be detected and reported by the Error-Correction Code (ECC) memorysubsystem (typically 0, 1 or 2).ecc-correction-bits:default: 0description: |If present, this indicates the number of bits of memory error whichcan be corrected by the Error-Correction Code (ECC) memory subsystem(typically 0, 1 or 2).required: - device_type
-- 2.42.0.283.g2d96d420d3-goog
Hi Rob,
On Fri, 8 Sept 2023 at 05:46, Rob Herring robh@kernel.org wrote:
On Thu, Sep 7, 2023 at 4:40 PM Simon Glass sjg@chromium.org wrote:
Some memories provide ECC detection and/or correction. For software which wants to check memory, it is helpful to see which regions provide this feature.
Add this as a property of the /memory nodes, since it presumably follows the hardware-level memory system.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v6:
- Use a number of bits instead of a string property
- Fix inidcates typo
Changes in v5:
- Redo to make this property specific to ECC
- Provide properties both for detection and correction
Changes in v3:
- Add new patch to update the /memory nodes
dtschema/schemas/memory.yaml | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/dtschema/schemas/memory.yaml b/dtschema/schemas/memory.yaml index 1d74410..1a48b1c 100644 --- a/dtschema/schemas/memory.yaml +++ b/dtschema/schemas/memory.yaml @@ -31,10 +31,21 @@ patternProperties:
numa-node-id: $ref: types.yaml#/definitions/uint32
description:
description: |Why? '|' is not needed for any of these.
For the purpose of identification, each NUMA node is associated with a unique token known as a node id.
blank line between properties.
I can fix these up when applying.
OK thank you.
ecc-detection-bits:default: 0description: |If present, this indicates the number of bits of memory error whichcan be detected and reported by the Error-Correction Code (ECC) memorysubsystem (typically 0, 1 or 2).ecc-correction-bits:default: 0description: |If present, this indicates the number of bits of memory error whichcan be corrected by the Error-Correction Code (ECC) memory subsystem(typically 0, 1 or 2).required: - device_type
-- 2.42.0.283.g2d96d420d3-goog
Regards, Simon
Hi,
On Thu, 7 Sept 2023 at 15:40, 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
If there are no more comments could these two patches be applied, please?
Regards, Simon
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 thetables 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 bereclaimed 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 bythe OS when it is runningboot-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 whenrunning; memory may be reclaimed if this code is not calledruntime-data: Contains data used for interacting with the system whenrunning; 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
Hi Rob,
On Thu, 21 Sept 2023 at 08:25, Rob Herring robh@kernel.org wrote:
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 thetables are no-longer neededI 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 bereclaimed when the tables are no-longer neededNo 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?
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 bythe OS when it is runningboot-code: Contains data used for booting; memory may be reclaimed byboot-data?
Yes
the OS when it is runningI 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 whenrunning; memory may be reclaimed if this code is not calledruntime-data: Contains data used for interacting with the system whenrunning; 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
?
Regards, Simon
On Thu, Sep 21, 2023 at 9:38 AM Simon Glass sjg@chromium.org wrote:
Hi Rob,
On Thu, 21 Sept 2023 at 08:25, Rob Herring robh@kernel.org wrote:
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 thetables are no-longer neededI 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 bereclaimed when the tables are no-longer neededNo 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.
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 bythe OS when it is runningboot-code: Contains data used for booting; memory may be reclaimed byboot-data?
Yes
the OS when it is runningI 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 whenrunning; memory may be reclaimed if this code is not calledruntime-data: Contains data used for interacting with the system whenrunning; 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 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.
Rob
Hi Rob,
On Thu, 21 Sept 2023 at 09:20, Rob Herring robh@kernel.org wrote:
On Thu, Sep 21, 2023 at 9:38 AM Simon Glass sjg@chromium.org wrote:
Hi Rob,
On Thu, 21 Sept 2023 at 08:25, Rob Herring robh@kernel.org wrote:
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 thetables are no-longer neededI 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 bereclaimed when the tables are no-longer neededNo 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 bythe OS when it is runningboot-code: Contains data used for booting; memory may be reclaimed byboot-data?
Yes
the OS when it is runningI 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 whenrunning; memory may be reclaimed if this code is not calledruntime-data: Contains data used for interacting with the system whenrunning; 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.
Regards, Simon
[1] https://lore.kernel.org/lkml/CAMj1kXHGpCt8qkd6XYQF8mMdivQkTnEWjv6NzsFK=+N72L...
Hi Rob,
On Thu, 21 Sept 2023 at 10:45, Simon Glass sjg@chromium.org wrote:
Hi Rob,
On Thu, 21 Sept 2023 at 09:20, Rob Herring robh@kernel.org wrote:
On Thu, Sep 21, 2023 at 9:38 AM Simon Glass sjg@chromium.org wrote:
Hi Rob,
On Thu, 21 Sept 2023 at 08:25, Rob Herring robh@kernel.org wrote:
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 thetables are no-longer neededI 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 bereclaimed when the tables are no-longer neededNo 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 bythe OS when it is runningboot-code: Contains data used for booting; memory may be reclaimed byboot-data?
Yes
the OS when it is runningI 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 whenrunning; memory may be reclaimed if this code is not calledruntime-data: Contains data used for interacting with the system whenrunning; 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=+N72L...
participants (3)
-
Peter Robinson -
Rob Herring -
Simon Glass