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 small schema addition for the memory mapping
needed to keep these two pieces working together well.
Signed-off-by: Simon Glass <[email protected]>
---
Changes in v7:
- Rename acpi-reclaim to acpi
- Drop individual mention of when memory can be reclaimed
- Rewrite the item descriptions
- Add back the UEFI text (with trepidation)
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..f7fbdfd
--- /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 <[email protected]>
+
+allOf:
+ - $ref: reserved-memory.yaml
+
+properties:
+ compatible:
+ description: |
+ This describes some common memory reservations, with the compatible
+ string indicating what it is used for:
+
+ acpi: Advanced Configuration and Power Interface (ACPI) tables
+ acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
+ the firmware for its use and is required to be saved and restored
+ across an NVS sleep
+ boot-code: Contains code used for booting which is not needed by the OS
+ boot-code: Contains data used for booting which is not needed by the OS
+ runtime-code: Contains code used for interacting with the system when
+ running the OS
+ runtime-data: Contains data used for interacting with the system when
+ running the OS
+
+ enum:
+ - acpi
+ - 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>;
+ };
+ };
--
2.42.0.515.g380fc7ccd1-goog
Hi Rob,
On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
>
> 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 small schema addition for the memory mapping
> needed to keep these two pieces working together well.
>
> Signed-off-by: Simon Glass <[email protected]>
> ---
>
> Changes in v7:
> - Rename acpi-reclaim to acpi
> - Drop individual mention of when memory can be reclaimed
> - Rewrite the item descriptions
> - Add back the UEFI text (with trepidation)
I am again checking on this series. Can it be applied, please?
>
> 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..f7fbdfd
> --- /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 <[email protected]>
> +
> +allOf:
> + - $ref: reserved-memory.yaml
> +
> +properties:
> + compatible:
> + description: |
> + This describes some common memory reservations, with the compatible
> + string indicating what it is used for:
> +
> + acpi: Advanced Configuration and Power Interface (ACPI) tables
> + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> + the firmware for its use and is required to be saved and restored
> + across an NVS sleep
> + boot-code: Contains code used for booting which is not needed by the OS
> + boot-code: Contains data used for booting which is not needed by the OS
> + runtime-code: Contains code used for interacting with the system when
> + running the OS
> + runtime-data: Contains data used for interacting with the system when
> + running the OS
> +
> + enum:
> + - acpi
> + - 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>;
> + };
> + };
> --
> 2.42.0.515.g380fc7ccd1-goog
>
Regards,
Simon
On Mon, 2 Oct 2023 at 19:54, Simon Glass <[email protected]> wrote:
>
> Hi Rob,
>
> On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
> >
> > 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 small schema addition for the memory mapping
> > needed to keep these two pieces working together well.
> >
> > Signed-off-by: Simon Glass <[email protected]>
> > ---
> >
> > Changes in v7:
> > - Rename acpi-reclaim to acpi
> > - Drop individual mention of when memory can be reclaimed
> > - Rewrite the item descriptions
> > - Add back the UEFI text (with trepidation)
>
> I am again checking on this series. Can it be applied, please?
>
Apologies for the delay in response. I have been away.
>
> >
> > 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..f7fbdfd
> > --- /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 <[email protected]>
> > +
> > +allOf:
> > + - $ref: reserved-memory.yaml
> > +
> > +properties:
> > + compatible:
> > + description: |
> > + This describes some common memory reservations, with the compatible
> > + string indicating what it is used for:
> > +
> > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > + the firmware for its use and is required to be saved and restored
> > + across an NVS sleep
> > + boot-code: Contains code used for booting which is not needed by the OS
> > + boot-code: Contains data used for booting which is not needed by the OS
> > + runtime-code: Contains code used for interacting with the system when
> > + running the OS
> > + runtime-data: Contains data used for interacting with the system when
> > + running the OS
> > +
> > + enum:
> > + - acpi
> > + - acpi-nvs
> > + - boot-code
> > + - boot-data
> > + - runtime-code
> > + - runtime-data
> > +
As I mentioned a few times already, I don't think these compatibles
should be introduced here.
A reserved region has a specific purpose, and the compatible should be
more descriptive than the enum above. If the consumer does not
understand this purpose, it should simply treat the memory as reserved
and not touch it. Alternatively, these regions can be referenced from
other DT nodes using phandles if needed.
> > + 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>;
> > + };
> > + };
> > --
> > 2.42.0.515.g380fc7ccd1-goog
> >
>
> Regards,
> Simon
Hi Ard,
On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel <[email protected]> wrote:
>
> On Mon, 2 Oct 2023 at 19:54, Simon Glass <[email protected]> wrote:
> >
> > Hi Rob,
> >
> > On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
> > >
> > > 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 small schema addition for the memory mapping
> > > needed to keep these two pieces working together well.
> > >
> > > Signed-off-by: Simon Glass <[email protected]>
> > > ---
> > >
> > > Changes in v7:
> > > - Rename acpi-reclaim to acpi
> > > - Drop individual mention of when memory can be reclaimed
> > > - Rewrite the item descriptions
> > > - Add back the UEFI text (with trepidation)
> >
> > I am again checking on this series. Can it be applied, please?
> >
>
> Apologies for the delay in response. I have been away.
OK, I hope you had a nice trip.
>
> >
> > >
> > > 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..f7fbdfd
> > > --- /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 <[email protected]>
> > > +
> > > +allOf:
> > > + - $ref: reserved-memory.yaml
> > > +
> > > +properties:
> > > + compatible:
> > > + description: |
> > > + This describes some common memory reservations, with the compatible
> > > + string indicating what it is used for:
> > > +
> > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > > + the firmware for its use and is required to be saved and restored
> > > + across an NVS sleep
> > > + boot-code: Contains code used for booting which is not needed by the OS
> > > + boot-code: Contains data used for booting which is not needed by the OS
> > > + runtime-code: Contains code used for interacting with the system when
> > > + running the OS
> > > + runtime-data: Contains data used for interacting with the system when
> > > + running the OS
> > > +
> > > + enum:
> > > + - acpi
> > > + - acpi-nvs
> > > + - boot-code
> > > + - boot-data
> > > + - runtime-code
> > > + - runtime-data
> > > +
>
> As I mentioned a few times already, I don't think these compatibles
> should be introduced here.
>
> A reserved region has a specific purpose, and the compatible should be
> more descriptive than the enum above. If the consumer does not
> understand this purpose, it should simply treat the memory as reserved
> and not touch it. Alternatively, these regions can be referenced from
> other DT nodes using phandles if needed.
We still need some description of what these regions are used for, so
that the payload can use the correct regions. I do not have any other
solution to this problem. We are in v7 at present. At least explain
where you want the compatible strings to be introduced.
What sort of extra detail are you looking for? Please be specific and
preferably add some suggestions so I can close this out ASAP.
>
>
> > > + 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>;
> > > + };
> > > + };
> > > --
> > > 2.42.0.515.g380fc7ccd1-goog
Regards,
Simon
On Fri, 6 Oct 2023 at 20:17, Simon Glass <[email protected]> wrote:
>
> Hi Ard,
>
> On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel <[email protected]> wrote:
> >
> > On Mon, 2 Oct 2023 at 19:54, Simon Glass <[email protected]> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
> > > >
> > > > 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 small schema addition for the memory mapping
> > > > needed to keep these two pieces working together well.
> > > >
> > > > Signed-off-by: Simon Glass <[email protected]>
> > > > ---
> > > >
> > > > Changes in v7:
> > > > - Rename acpi-reclaim to acpi
> > > > - Drop individual mention of when memory can be reclaimed
> > > > - Rewrite the item descriptions
> > > > - Add back the UEFI text (with trepidation)
> > >
> > > I am again checking on this series. Can it be applied, please?
> > >
> >
> > Apologies for the delay in response. I have been away.
>
> OK, I hope you had a nice trip.
>
Thanks, it was wonderful!
> >
> > >
> > > >
> > > > 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..f7fbdfd
> > > > --- /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 <[email protected]>
> > > > +
> > > > +allOf:
> > > > + - $ref: reserved-memory.yaml
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + description: |
> > > > + This describes some common memory reservations, with the compatible
> > > > + string indicating what it is used for:
> > > > +
> > > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > > > + the firmware for its use and is required to be saved and restored
> > > > + across an NVS sleep
> > > > + boot-code: Contains code used for booting which is not needed by the OS
> > > > + boot-code: Contains data used for booting which is not needed by the OS
> > > > + runtime-code: Contains code used for interacting with the system when
> > > > + running the OS
> > > > + runtime-data: Contains data used for interacting with the system when
> > > > + running the OS
> > > > +
> > > > + enum:
> > > > + - acpi
> > > > + - acpi-nvs
> > > > + - boot-code
> > > > + - boot-data
> > > > + - runtime-code
> > > > + - runtime-data
> > > > +
> >
> > As I mentioned a few times already, I don't think these compatibles
> > should be introduced here.
> >
> > A reserved region has a specific purpose, and the compatible should be
> > more descriptive than the enum above. If the consumer does not
> > understand this purpose, it should simply treat the memory as reserved
> > and not touch it. Alternatively, these regions can be referenced from
> > other DT nodes using phandles if needed.
>
> We still need some description of what these regions are used for, so
> that the payload can use the correct regions. I do not have any other
> solution to this problem. We are in v7 at present. At least explain
> where you want the compatible strings to be introduced.
>
My point is really that by themselves, these regions are not usable by
either a payload or an OS that consumes this information. Unless there
is some other information being provided (via DT I imagine) that
describes how these things are supposed to be used, they are nothing
more than memory reservations that should be honored, and providing
this arbitrary set of labels is unnecessary.
> What sort of extra detail are you looking for? Please be specific and
> preferably add some suggestions so I can close this out ASAP.
>
A payload or OS can do nothing with a memory reservation called
'runtime-code' it it doesn't know what is inside. So there is another
DT node somewhere that describes this, and that can simply point to
this region (via a phandle) if it needs to describe the
correspondence. This is more idiomatic for DT afaik (but I am not the
expert). But more importantly, it avoids overloading some vague
labels with behavior (e.g., executable permissions for code regions)
that should only be displayed for regions with a particular use,
rather than for a ill defined class of reservations the purpose of
which is not clear.
What I am trying to avoid is the OS ending up being forced to consume
this information in parallel to the EFI memory map, and having to
reconcile them. I'd be much happier if this gets contributed to a spec
that only covers firmware-to-firmware, and is prevented from leaking
into the OS facing interface.
> >
> >
> > > > + 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>;
> > > > + };
> > > > + };
> > > > --
> > > > 2.42.0.515.g380fc7ccd1-goog
>
> Regards,
> Simon
Hi Ard,
On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel <[email protected]> wrote:
>
> On Fri, 6 Oct 2023 at 20:17, Simon Glass <[email protected]> wrote:
> >
> > Hi Ard,
> >
> > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel <[email protected]> wrote:
> > >
> > > On Mon, 2 Oct 2023 at 19:54, Simon Glass <[email protected]> wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
> > > > >
> > > > > 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 small schema addition for the memory mapping
> > > > > needed to keep these two pieces working together well.
> > > > >
> > > > > Signed-off-by: Simon Glass <[email protected]>
> > > > > ---
> > > > >
> > > > > Changes in v7:
> > > > > - Rename acpi-reclaim to acpi
> > > > > - Drop individual mention of when memory can be reclaimed
> > > > > - Rewrite the item descriptions
> > > > > - Add back the UEFI text (with trepidation)
> > > >
> > > > I am again checking on this series. Can it be applied, please?
> > > >
> > >
> > > Apologies for the delay in response. I have been away.
> >
> > OK, I hope you had a nice trip.
> >
>
> Thanks, it was wonderful!
>
> > >
> > > >
> > > > >
> > > > > 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..f7fbdfd
> > > > > --- /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 <[email protected]>
> > > > > +
> > > > > +allOf:
> > > > > + - $ref: reserved-memory.yaml
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + description: |
> > > > > + This describes some common memory reservations, with the compatible
> > > > > + string indicating what it is used for:
> > > > > +
> > > > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > > > > + the firmware for its use and is required to be saved and restored
> > > > > + across an NVS sleep
> > > > > + boot-code: Contains code used for booting which is not needed by the OS
> > > > > + boot-code: Contains data used for booting which is not needed by the OS
> > > > > + runtime-code: Contains code used for interacting with the system when
> > > > > + running the OS
> > > > > + runtime-data: Contains data used for interacting with the system when
> > > > > + running the OS
> > > > > +
> > > > > + enum:
> > > > > + - acpi
> > > > > + - acpi-nvs
> > > > > + - boot-code
> > > > > + - boot-data
> > > > > + - runtime-code
> > > > > + - runtime-data
> > > > > +
> > >
> > > As I mentioned a few times already, I don't think these compatibles
> > > should be introduced here.
> > >
> > > A reserved region has a specific purpose, and the compatible should be
> > > more descriptive than the enum above. If the consumer does not
> > > understand this purpose, it should simply treat the memory as reserved
> > > and not touch it. Alternatively, these regions can be referenced from
> > > other DT nodes using phandles if needed.
> >
> > We still need some description of what these regions are used for, so
> > that the payload can use the correct regions. I do not have any other
> > solution to this problem. We are in v7 at present. At least explain
> > where you want the compatible strings to be introduced.
> >
>
> My point is really that by themselves, these regions are not usable by
> either a payload or an OS that consumes this information. Unless there
> is some other information being provided (via DT I imagine) that
> describes how these things are supposed to be used, they are nothing
> more than memory reservations that should be honored, and providing
> this arbitrary set of labels is unnecessary.
>
> > What sort of extra detail are you looking for? Please be specific and
> > preferably add some suggestions so I can close this out ASAP.
> >
>
> A payload or OS can do nothing with a memory reservation called
> 'runtime-code' it it doesn't know what is inside. So there is another
> DT node somewhere that describes this, and that can simply point to
> this region (via a phandle) if it needs to describe the
> correspondence. This is more idiomatic for DT afaik (but I am not the
> expert). But more importantly, it avoids overloading some vague
> labels with behavior (e.g., executable permissions for code regions)
> that should only be displayed for regions with a particular use,
> rather than for a ill defined class of reservations the purpose of
> which is not clear.
>
> What I am trying to avoid is the OS ending up being forced to consume
> this information in parallel to the EFI memory map, and having to
> reconcile them. I'd be much happier if this gets contributed to a spec
> that only covers firmware-to-firmware, and is prevented from leaking
> into the OS facing interface.
I don't know about "another DT node". We don't have one at present.
There is already a note in the DT spec about this:
> 3.5.4 /reserved-memory and UEFI
> When booting via [UEFI], static /reserved-memory regions must also be listed in the system memory map obtained
> via the GetMemoryMap() UEFI boot time service as defined in [UEFI] § 7.2. The reserved memory regions need to be
> included in the UEFI memory map to protect against allocations by UEFI applications.
>
> Reserved regions with the no-map property must be listed in the memory map with type EfiReservedMemoryType. All
> other reserved regions must be listed with type EfiBootServicesData.
>
> Dynamic reserved memory regions must not be listed in the [UEFI] memory map because they are allocated by the OS
> after exiting firmware boot services.
I don't fully understand what all that means, but does it cover your concern?
Regards,
Simon
Hi all,
On Fri, 6 Oct 2023 at 18:03, Simon Glass <[email protected]> wrote:
>
> Hi Ard,
>
> On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel <[email protected]> wrote:
> >
> > On Fri, 6 Oct 2023 at 20:17, Simon Glass <[email protected]> wrote:
> > >
> > > Hi Ard,
> > >
> > > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel <[email protected]> wrote:
> > > >
> > > > On Mon, 2 Oct 2023 at 19:54, Simon Glass <[email protected]> wrote:
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
> > > > > >
> > > > > > 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 small schema addition for the memory mapping
> > > > > > needed to keep these two pieces working together well.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <[email protected]>
> > > > > > ---
> > > > > >
> > > > > > Changes in v7:
> > > > > > - Rename acpi-reclaim to acpi
> > > > > > - Drop individual mention of when memory can be reclaimed
> > > > > > - Rewrite the item descriptions
> > > > > > - Add back the UEFI text (with trepidation)
> > > > >
> > > > > I am again checking on this series. Can it be applied, please?
> > > > >
> > > >
> > > > Apologies for the delay in response. I have been away.
> > >
> > > OK, I hope you had a nice trip.
> > >
> >
> > Thanks, it was wonderful!
> >
> > > >
> > > > >
> > > > > >
> > > > > > 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..f7fbdfd
> > > > > > --- /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 <[email protected]>
> > > > > > +
> > > > > > +allOf:
> > > > > > + - $ref: reserved-memory.yaml
> > > > > > +
> > > > > > +properties:
> > > > > > + compatible:
> > > > > > + description: |
> > > > > > + This describes some common memory reservations, with the compatible
> > > > > > + string indicating what it is used for:
> > > > > > +
> > > > > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > > > > > + the firmware for its use and is required to be saved and restored
> > > > > > + across an NVS sleep
> > > > > > + boot-code: Contains code used for booting which is not needed by the OS
> > > > > > + boot-code: Contains data used for booting which is not needed by the OS
> > > > > > + runtime-code: Contains code used for interacting with the system when
> > > > > > + running the OS
> > > > > > + runtime-data: Contains data used for interacting with the system when
> > > > > > + running the OS
> > > > > > +
> > > > > > + enum:
> > > > > > + - acpi
> > > > > > + - acpi-nvs
> > > > > > + - boot-code
> > > > > > + - boot-data
> > > > > > + - runtime-code
> > > > > > + - runtime-data
> > > > > > +
> > > >
> > > > As I mentioned a few times already, I don't think these compatibles
> > > > should be introduced here.
> > > >
> > > > A reserved region has a specific purpose, and the compatible should be
> > > > more descriptive than the enum above. If the consumer does not
> > > > understand this purpose, it should simply treat the memory as reserved
> > > > and not touch it. Alternatively, these regions can be referenced from
> > > > other DT nodes using phandles if needed.
> > >
> > > We still need some description of what these regions are used for, so
> > > that the payload can use the correct regions. I do not have any other
> > > solution to this problem. We are in v7 at present. At least explain
> > > where you want the compatible strings to be introduced.
> > >
> >
> > My point is really that by themselves, these regions are not usable by
> > either a payload or an OS that consumes this information. Unless there
> > is some other information being provided (via DT I imagine) that
> > describes how these things are supposed to be used, they are nothing
> > more than memory reservations that should be honored, and providing
> > this arbitrary set of labels is unnecessary.
> >
> > > What sort of extra detail are you looking for? Please be specific and
> > > preferably add some suggestions so I can close this out ASAP.
> > >
> >
> > A payload or OS can do nothing with a memory reservation called
> > 'runtime-code' it it doesn't know what is inside. So there is another
> > DT node somewhere that describes this, and that can simply point to
> > this region (via a phandle) if it needs to describe the
> > correspondence. This is more idiomatic for DT afaik (but I am not the
> > expert). But more importantly, it avoids overloading some vague
> > labels with behavior (e.g., executable permissions for code regions)
> > that should only be displayed for regions with a particular use,
> > rather than for a ill defined class of reservations the purpose of
> > which is not clear.
> >
> > What I am trying to avoid is the OS ending up being forced to consume
> > this information in parallel to the EFI memory map, and having to
> > reconcile them. I'd be much happier if this gets contributed to a spec
> > that only covers firmware-to-firmware, and is prevented from leaking
> > into the OS facing interface.
>
> I don't know about "another DT node". We don't have one at present.
>
> There is already a note in the DT spec about this:
>
> > 3.5.4 /reserved-memory and UEFI
>
> > When booting via [UEFI], static /reserved-memory regions must also be listed in the system memory map obtained
> > via the GetMemoryMap() UEFI boot time service as defined in [UEFI] § 7.2. The reserved memory regions need to be
> > included in the UEFI memory map to protect against allocations by UEFI applications.
> >
> > Reserved regions with the no-map property must be listed in the memory map with type EfiReservedMemoryType. All
> > other reserved regions must be listed with type EfiBootServicesData.
> >
> > Dynamic reserved memory regions must not be listed in the [UEFI] memory map because they are allocated by the OS
> > after exiting firmware boot services.
>
> I don't fully understand what all that means, but does it cover your concern?
I have reread the discussion on this memory-reservation problem,
several dozen emails at this point. I believe we have covered
everything.
For now we will go with the binding in this patch. I hope it can be
applied soon, or if not, someone can send a different proposal to meet
the requirements.
Regards,
Simon
On Sat, 7 Oct 2023 at 02:03, Simon Glass <[email protected]> wrote:
>
> Hi Ard,
>
> On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel <[email protected]> wrote:
> >
> > On Fri, 6 Oct 2023 at 20:17, Simon Glass <[email protected]> wrote:
> > >
> > > Hi Ard,
> > >
> > > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel <[email protected]> wrote:
> > > >
> > > > On Mon, 2 Oct 2023 at 19:54, Simon Glass <[email protected]> wrote:
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
> > > > > >
> > > > > > 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 small schema addition for the memory mapping
> > > > > > needed to keep these two pieces working together well.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <[email protected]>
> > > > > > ---
> > > > > >
> > > > > > Changes in v7:
> > > > > > - Rename acpi-reclaim to acpi
> > > > > > - Drop individual mention of when memory can be reclaimed
> > > > > > - Rewrite the item descriptions
> > > > > > - Add back the UEFI text (with trepidation)
> > > > >
> > > > > I am again checking on this series. Can it be applied, please?
> > > > >
> > > >
> > > > Apologies for the delay in response. I have been away.
> > >
> > > OK, I hope you had a nice trip.
> > >
> >
> > Thanks, it was wonderful!
> >
> > > >
> > > > >
> > > > > >
> > > > > > 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..f7fbdfd
> > > > > > --- /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 <[email protected]>
> > > > > > +
> > > > > > +allOf:
> > > > > > + - $ref: reserved-memory.yaml
> > > > > > +
> > > > > > +properties:
> > > > > > + compatible:
> > > > > > + description: |
> > > > > > + This describes some common memory reservations, with the compatible
> > > > > > + string indicating what it is used for:
> > > > > > +
> > > > > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > > > > > + the firmware for its use and is required to be saved and restored
> > > > > > + across an NVS sleep
> > > > > > + boot-code: Contains code used for booting which is not needed by the OS
> > > > > > + boot-code: Contains data used for booting which is not needed by the OS
> > > > > > + runtime-code: Contains code used for interacting with the system when
> > > > > > + running the OS
> > > > > > + runtime-data: Contains data used for interacting with the system when
> > > > > > + running the OS
> > > > > > +
> > > > > > + enum:
> > > > > > + - acpi
> > > > > > + - acpi-nvs
> > > > > > + - boot-code
> > > > > > + - boot-data
> > > > > > + - runtime-code
> > > > > > + - runtime-data
> > > > > > +
> > > >
> > > > As I mentioned a few times already, I don't think these compatibles
> > > > should be introduced here.
> > > >
> > > > A reserved region has a specific purpose, and the compatible should be
> > > > more descriptive than the enum above. If the consumer does not
> > > > understand this purpose, it should simply treat the memory as reserved
> > > > and not touch it. Alternatively, these regions can be referenced from
> > > > other DT nodes using phandles if needed.
> > >
> > > We still need some description of what these regions are used for, so
> > > that the payload can use the correct regions. I do not have any other
> > > solution to this problem. We are in v7 at present. At least explain
> > > where you want the compatible strings to be introduced.
> > >
> >
> > My point is really that by themselves, these regions are not usable by
> > either a payload or an OS that consumes this information. Unless there
> > is some other information being provided (via DT I imagine) that
> > describes how these things are supposed to be used, they are nothing
> > more than memory reservations that should be honored, and providing
> > this arbitrary set of labels is unnecessary.
> >
> > > What sort of extra detail are you looking for? Please be specific and
> > > preferably add some suggestions so I can close this out ASAP.
> > >
> >
> > A payload or OS can do nothing with a memory reservation called
> > 'runtime-code' it it doesn't know what is inside. So there is another
> > DT node somewhere that describes this, and that can simply point to
> > this region (via a phandle) if it needs to describe the
> > correspondence. This is more idiomatic for DT afaik (but I am not the
> > expert). But more importantly, it avoids overloading some vague
> > labels with behavior (e.g., executable permissions for code regions)
> > that should only be displayed for regions with a particular use,
> > rather than for a ill defined class of reservations the purpose of
> > which is not clear.
> >
> > What I am trying to avoid is the OS ending up being forced to consume
> > this information in parallel to the EFI memory map, and having to
> > reconcile them. I'd be much happier if this gets contributed to a spec
> > that only covers firmware-to-firmware, and is prevented from leaking
> > into the OS facing interface.
>
> I don't know about "another DT node". We don't have one at present.
>
> There is already a note in the DT spec about this:
>
> > 3.5.4 /reserved-memory and UEFI
>
> > When booting via [UEFI], static /reserved-memory regions must also be listed in the system memory map obtained
> > via the GetMemoryMap() UEFI boot time service as defined in [UEFI] § 7.2. The reserved memory regions need to be
> > included in the UEFI memory map to protect against allocations by UEFI applications.
> >
> > Reserved regions with the no-map property must be listed in the memory map with type EfiReservedMemoryType. All
> > other reserved regions must be listed with type EfiBootServicesData.
> >
> > Dynamic reserved memory regions must not be listed in the [UEFI] memory map because they are allocated by the OS
> > after exiting firmware boot services.
>
> I don't fully understand what all that means, but does it cover your concern?
>
I don't fully agree with the wording here, but apparently there is
some awareness here of the concerns I raised.
Interestingly, 'when booting via UEFI' becomes a bit ambiguous now,
given that DT is passed from stage to stage, and not every handover is
booting via UEFI. In general, I think the introduction of bindings
that cover areas other than the handover from the final boot loader to
the OS may create some more confusion later on, but I'll leave it up
to the DT bindings maintainer to reason about that.
If you are dead set on introducing these items (and I note that you
still have not provided an actual example of how a PI -> payload
handover would make use of runtime-code or boot-code reservations), I
won't keep standing in your way.
Thanks for the discussion, and apologies for dragging this out.
--
Ard.
On Fri, Oct 6, 2023 at 7:03 PM Simon Glass <[email protected]> wrote:
>
> Hi Ard,
>
> On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel <[email protected]> wrote:
> >
> > On Fri, 6 Oct 2023 at 20:17, Simon Glass <[email protected]> wrote:
> > >
> > > Hi Ard,
> > >
> > > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel <[email protected]> wrote:
> > > >
> > > > On Mon, 2 Oct 2023 at 19:54, Simon Glass <[email protected]> wrote:
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
> > > > > >
> > > > > > 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 small schema addition for the memory mapping
> > > > > > needed to keep these two pieces working together well.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <[email protected]>
> > > > > > ---
> > > > > >
> > > > > > Changes in v7:
> > > > > > - Rename acpi-reclaim to acpi
> > > > > > - Drop individual mention of when memory can be reclaimed
> > > > > > - Rewrite the item descriptions
> > > > > > - Add back the UEFI text (with trepidation)
> > > > >
> > > > > I am again checking on this series. Can it be applied, please?
> > > > >
> > > >
> > > > Apologies for the delay in response. I have been away.
> > >
> > > OK, I hope you had a nice trip.
> > >
> >
> > Thanks, it was wonderful!
> >
> > > >
> > > > >
> > > > > >
> > > > > > 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..f7fbdfd
> > > > > > --- /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 <[email protected]>
> > > > > > +
> > > > > > +allOf:
> > > > > > + - $ref: reserved-memory.yaml
> > > > > > +
> > > > > > +properties:
> > > > > > + compatible:
> > > > > > + description: |
> > > > > > + This describes some common memory reservations, with the compatible
> > > > > > + string indicating what it is used for:
> > > > > > +
> > > > > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > > > > > + the firmware for its use and is required to be saved and restored
> > > > > > + across an NVS sleep
> > > > > > + boot-code: Contains code used for booting which is not needed by the OS
> > > > > > + boot-code: Contains data used for booting which is not needed by the OS
> > > > > > + runtime-code: Contains code used for interacting with the system when
> > > > > > + running the OS
> > > > > > + runtime-data: Contains data used for interacting with the system when
> > > > > > + running the OS
> > > > > > +
> > > > > > + enum:
> > > > > > + - acpi
> > > > > > + - acpi-nvs
> > > > > > + - boot-code
> > > > > > + - boot-data
> > > > > > + - runtime-code
> > > > > > + - runtime-data
> > > > > > +
> > > >
> > > > As I mentioned a few times already, I don't think these compatibles
> > > > should be introduced here.
> > > >
> > > > A reserved region has a specific purpose, and the compatible should be
> > > > more descriptive than the enum above. If the consumer does not
> > > > understand this purpose, it should simply treat the memory as reserved
> > > > and not touch it. Alternatively, these regions can be referenced from
> > > > other DT nodes using phandles if needed.
> > >
> > > We still need some description of what these regions are used for, so
> > > that the payload can use the correct regions. I do not have any other
> > > solution to this problem. We are in v7 at present. At least explain
> > > where you want the compatible strings to be introduced.
> > >
> >
> > My point is really that by themselves, these regions are not usable by
> > either a payload or an OS that consumes this information. Unless there
> > is some other information being provided (via DT I imagine) that
> > describes how these things are supposed to be used, they are nothing
> > more than memory reservations that should be honored, and providing
> > this arbitrary set of labels is unnecessary.
> >
> > > What sort of extra detail are you looking for? Please be specific and
> > > preferably add some suggestions so I can close this out ASAP.
> > >
> >
> > A payload or OS can do nothing with a memory reservation called
> > 'runtime-code' it it doesn't know what is inside.
Agreed. The question is WHAT runtime-code? The compatible needs to answer that.
For example, we have 'ramoops' as a compatible in reserved memory.
That tells us *exactly* what's there. We know how to parse it. If we
know ramoops is not supported, then we know we can toss it out and
reclaim the memory.
> > So there is another
> > DT node somewhere that describes this, and that can simply point to
> > this region (via a phandle) if it needs to describe the
> > correspondence. This is more idiomatic for DT afaik (but I am not the
> > expert).
I don't see why we need that indirection.
> > But more importantly, it avoids overloading some vague
> > labels with behavior (e.g., executable permissions for code regions)
> > that should only be displayed for regions with a particular use,
> > rather than for a ill defined class of reservations the purpose of
> > which is not clear.
> >
> > What I am trying to avoid is the OS ending up being forced to consume
> > this information in parallel to the EFI memory map, and having to
> > reconcile them. I'd be much happier if this gets contributed to a spec
> > that only covers firmware-to-firmware, and is prevented from leaking
> > into the OS facing interface.
>
> I don't know about "another DT node". We don't have one at present.
>
> There is already a note in the DT spec about this:
>
> > 3.5.4 /reserved-memory and UEFI
>
> > When booting via [UEFI], static /reserved-memory regions must also be listed in the system memory map obtained
> > via the GetMemoryMap() UEFI boot time service as defined in [UEFI] § 7.2. The reserved memory regions need to be
> > included in the UEFI memory map to protect against allocations by UEFI applications.
> >
> > Reserved regions with the no-map property must be listed in the memory map with type EfiReservedMemoryType. All
> > other reserved regions must be listed with type EfiBootServicesData.
> >
> > Dynamic reserved memory regions must not be listed in the [UEFI] memory map because they are allocated by the OS
> > after exiting firmware boot services.
>
> I don't fully understand what all that means, but does it cover your concern?
This section is purely about using UEFI mechanisms to load and boot
the OS. If we're not talking about this stage of booting, then none of
this applies.
Rob
Hi Rob,
On Fri, 13 Oct 2023 at 13:42, Rob Herring <[email protected]> wrote:
>
> On Fri, Oct 6, 2023 at 7:03 PM Simon Glass <[email protected]> wrote:
> >
> > Hi Ard,
> >
> > On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel <[email protected]> wrote:
> > >
> > > On Fri, 6 Oct 2023 at 20:17, Simon Glass <[email protected]> wrote:
> > > >
> > > > Hi Ard,
> > > >
> > > > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel <[email protected]> wrote:
> > > > >
> > > > > On Mon, 2 Oct 2023 at 19:54, Simon Glass <[email protected]> wrote:
> > > > > >
> > > > > > Hi Rob,
> > > > > >
> > > > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
> > > > > > >
> > > > > > > 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 small schema addition for the memory mapping
> > > > > > > needed to keep these two pieces working together well.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass <[email protected]>
> > > > > > > ---
> > > > > > >
> > > > > > > Changes in v7:
> > > > > > > - Rename acpi-reclaim to acpi
> > > > > > > - Drop individual mention of when memory can be reclaimed
> > > > > > > - Rewrite the item descriptions
> > > > > > > - Add back the UEFI text (with trepidation)
> > > > > >
> > > > > > I am again checking on this series. Can it be applied, please?
> > > > > >
> > > > >
> > > > > Apologies for the delay in response. I have been away.
> > > >
> > > > OK, I hope you had a nice trip.
> > > >
> > >
> > > Thanks, it was wonderful!
> > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > 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..f7fbdfd
> > > > > > > --- /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 <[email protected]>
> > > > > > > +
> > > > > > > +allOf:
> > > > > > > + - $ref: reserved-memory.yaml
> > > > > > > +
> > > > > > > +properties:
> > > > > > > + compatible:
> > > > > > > + description: |
> > > > > > > + This describes some common memory reservations, with the compatible
> > > > > > > + string indicating what it is used for:
> > > > > > > +
> > > > > > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > > > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > > > > > > + the firmware for its use and is required to be saved and restored
> > > > > > > + across an NVS sleep
> > > > > > > + boot-code: Contains code used for booting which is not needed by the OS
> > > > > > > + boot-code: Contains data used for booting which is not needed by the OS
> > > > > > > + runtime-code: Contains code used for interacting with the system when
> > > > > > > + running the OS
> > > > > > > + runtime-data: Contains data used for interacting with the system when
> > > > > > > + running the OS
> > > > > > > +
> > > > > > > + enum:
> > > > > > > + - acpi
> > > > > > > + - acpi-nvs
> > > > > > > + - boot-code
> > > > > > > + - boot-data
> > > > > > > + - runtime-code
> > > > > > > + - runtime-data
> > > > > > > +
> > > > >
> > > > > As I mentioned a few times already, I don't think these compatibles
> > > > > should be introduced here.
> > > > >
> > > > > A reserved region has a specific purpose, and the compatible should be
> > > > > more descriptive than the enum above. If the consumer does not
> > > > > understand this purpose, it should simply treat the memory as reserved
> > > > > and not touch it. Alternatively, these regions can be referenced from
> > > > > other DT nodes using phandles if needed.
> > > >
> > > > We still need some description of what these regions are used for, so
> > > > that the payload can use the correct regions. I do not have any other
> > > > solution to this problem. We are in v7 at present. At least explain
> > > > where you want the compatible strings to be introduced.
> > > >
> > >
> > > My point is really that by themselves, these regions are not usable by
> > > either a payload or an OS that consumes this information. Unless there
> > > is some other information being provided (via DT I imagine) that
> > > describes how these things are supposed to be used, they are nothing
> > > more than memory reservations that should be honored, and providing
> > > this arbitrary set of labels is unnecessary.
> > >
> > > > What sort of extra detail are you looking for? Please be specific and
> > > > preferably add some suggestions so I can close this out ASAP.
> > > >
> > >
> > > A payload or OS can do nothing with a memory reservation called
> > > 'runtime-code' it it doesn't know what is inside.
>
> Agreed. The question is WHAT runtime-code? The compatible needs to answer that.
>
> For example, we have 'ramoops' as a compatible in reserved memory.
> That tells us *exactly* what's there. We know how to parse it. If we
> know ramoops is not supported, then we know we can toss it out and
> reclaim the memory.
So if we said:
compatible = "runtime-code-efi"
would that be OK? We seem to be in catch 22 here, because if I don't
mention EFI unhappy, but if I do, Ard is unhappy.
What about the boottime code....you want to know which project it is from?
+ - acpi
+ - acpi-nvs
Those two should be enough info, right?
+ - boot-code
+ - boot-data
For these, they don't pertain to the OS, so perhaps they are OK? In
any case, using a generic term like this makes some sense to me. We
can always add a new compatible like "efi-boottime-services" later. It
may be that the boottime services would be handled by the payload, so
not needed in all cases.
+ - runtime-code
+ - runtime-data
See above.
>
>
> > > So there is another
> > > DT node somewhere that describes this, and that can simply point to
> > > this region (via a phandle) if it needs to describe the
> > > correspondence. This is more idiomatic for DT afaik (but I am not the
> > > expert).
>
> I don't see why we need that indirection.
>
> > > But more importantly, it avoids overloading some vague
> > > labels with behavior (e.g., executable permissions for code regions)
> > > that should only be displayed for regions with a particular use,
> > > rather than for a ill defined class of reservations the purpose of
> > > which is not clear.
> > >
> > > What I am trying to avoid is the OS ending up being forced to consume
> > > this information in parallel to the EFI memory map, and having to
> > > reconcile them. I'd be much happier if this gets contributed to a spec
> > > that only covers firmware-to-firmware, and is prevented from leaking
> > > into the OS facing interface.
> >
> > I don't know about "another DT node". We don't have one at present.
> >
> > There is already a note in the DT spec about this:
> >
> > > 3.5.4 /reserved-memory and UEFI
> >
> > > When booting via [UEFI], static /reserved-memory regions must also be listed in the system memory map obtained
> > > via the GetMemoryMap() UEFI boot time service as defined in [UEFI] § 7.2. The reserved memory regions need to be
> > > included in the UEFI memory map to protect against allocations by UEFI applications.
> > >
> > > Reserved regions with the no-map property must be listed in the memory map with type EfiReservedMemoryType. All
> > > other reserved regions must be listed with type EfiBootServicesData.
> > >
> > > Dynamic reserved memory regions must not be listed in the [UEFI] memory map because they are allocated by the OS
> > > after exiting firmware boot services.
> >
> > I don't fully understand what all that means, but does it cover your concern?
>
> This section is purely about using UEFI mechanisms to load and boot
> the OS. If we're not talking about this stage of booting, then none of
> this applies.
OK
Regards,
Simon
On Fri, Oct 13, 2023 at 4:09 PM Simon Glass <[email protected]> wrote:
>
> Hi Rob,
>
> On Fri, 13 Oct 2023 at 13:42, Rob Herring <[email protected]> wrote:
> >
> > On Fri, Oct 6, 2023 at 7:03 PM Simon Glass <[email protected]> wrote:
> > >
> > > Hi Ard,
> > >
> > > On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel <[email protected]> wrote:
> > > >
> > > > On Fri, 6 Oct 2023 at 20:17, Simon Glass <[email protected]> wrote:
> > > > >
> > > > > Hi Ard,
> > > > >
> > > > > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel <[email protected]> wrote:
> > > > > >
> > > > > > On Mon, 2 Oct 2023 at 19:54, Simon Glass <[email protected]> wrote:
> > > > > > >
> > > > > > > Hi Rob,
> > > > > > >
> > > > > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
> > > > > > > >
> > > > > > > > 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 small schema addition for the memory mapping
> > > > > > > > needed to keep these two pieces working together well.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass <[email protected]>
> > > > > > > > ---
> > > > > > > >
> > > > > > > > Changes in v7:
> > > > > > > > - Rename acpi-reclaim to acpi
> > > > > > > > - Drop individual mention of when memory can be reclaimed
> > > > > > > > - Rewrite the item descriptions
> > > > > > > > - Add back the UEFI text (with trepidation)
> > > > > > >
> > > > > > > I am again checking on this series. Can it be applied, please?
> > > > > > >
> > > > > >
> > > > > > Apologies for the delay in response. I have been away.
> > > > >
> > > > > OK, I hope you had a nice trip.
> > > > >
> > > >
> > > > Thanks, it was wonderful!
> > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > 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..f7fbdfd
> > > > > > > > --- /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 <[email protected]>
> > > > > > > > +
> > > > > > > > +allOf:
> > > > > > > > + - $ref: reserved-memory.yaml
> > > > > > > > +
> > > > > > > > +properties:
> > > > > > > > + compatible:
> > > > > > > > + description: |
> > > > > > > > + This describes some common memory reservations, with the compatible
> > > > > > > > + string indicating what it is used for:
> > > > > > > > +
> > > > > > > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > > > > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > > > > > > > + the firmware for its use and is required to be saved and restored
> > > > > > > > + across an NVS sleep
> > > > > > > > + boot-code: Contains code used for booting which is not needed by the OS
> > > > > > > > + boot-code: Contains data used for booting which is not needed by the OS
> > > > > > > > + runtime-code: Contains code used for interacting with the system when
> > > > > > > > + running the OS
> > > > > > > > + runtime-data: Contains data used for interacting with the system when
> > > > > > > > + running the OS
> > > > > > > > +
> > > > > > > > + enum:
> > > > > > > > + - acpi
> > > > > > > > + - acpi-nvs
> > > > > > > > + - boot-code
> > > > > > > > + - boot-data
> > > > > > > > + - runtime-code
> > > > > > > > + - runtime-data
> > > > > > > > +
> > > > > >
> > > > > > As I mentioned a few times already, I don't think these compatibles
> > > > > > should be introduced here.
> > > > > >
> > > > > > A reserved region has a specific purpose, and the compatible should be
> > > > > > more descriptive than the enum above. If the consumer does not
> > > > > > understand this purpose, it should simply treat the memory as reserved
> > > > > > and not touch it. Alternatively, these regions can be referenced from
> > > > > > other DT nodes using phandles if needed.
> > > > >
> > > > > We still need some description of what these regions are used for, so
> > > > > that the payload can use the correct regions. I do not have any other
> > > > > solution to this problem. We are in v7 at present. At least explain
> > > > > where you want the compatible strings to be introduced.
> > > > >
> > > >
> > > > My point is really that by themselves, these regions are not usable by
> > > > either a payload or an OS that consumes this information. Unless there
> > > > is some other information being provided (via DT I imagine) that
> > > > describes how these things are supposed to be used, they are nothing
> > > > more than memory reservations that should be honored, and providing
> > > > this arbitrary set of labels is unnecessary.
> > > >
> > > > > What sort of extra detail are you looking for? Please be specific and
> > > > > preferably add some suggestions so I can close this out ASAP.
> > > > >
> > > >
> > > > A payload or OS can do nothing with a memory reservation called
> > > > 'runtime-code' it it doesn't know what is inside.
> >
> > Agreed. The question is WHAT runtime-code? The compatible needs to answer that.
> >
> > For example, we have 'ramoops' as a compatible in reserved memory.
> > That tells us *exactly* what's there. We know how to parse it. If we
> > know ramoops is not supported, then we know we can toss it out and
> > reclaim the memory.
>
> So if we said:
>
> compatible = "runtime-code-efi"
>
> would that be OK? We seem to be in catch 22 here, because if I don't
> mention EFI unhappy, but if I do, Ard is unhappy.
Better yes, because then it is for something specific. However, AIUI,
that's setup for the OS and defining that region is already defined by
the EFI memory map. That's Ard's issue. If there's a need outside of
the EFI to OS handoff, then you need to define what that usecase looks
like. Describe the problem rather than present your solution.
If this is all specific to EDK2 then it should say that rather than
'efi'. I imagine Ard would be happier with something tied to EDK2 than
*all* UEFI. Though maybe the problem could be any implementation? IDK.
Maybe it's TF-A that needs to define where the EFI runtime services
region is and that needs to be passed all the way thru to the EFI
implementation? So again, define the problem.
> What about the boottime code....you want to know which project it is from?
I think it is the same.
>
> + - acpi
> + - acpi-nvs
>
> Those two should be enough info, right?
I think so. NVS is not a term I've heard in relation to ACPI, but that
may just be my limited ACPI knowledge.
> + - boot-code
> + - boot-data
>
> For these, they don't pertain to the OS, so perhaps they are OK?
Hard to tell that just from the name... 'boot' could be any component
involved in booting including the OS.
> In
> any case, using a generic term like this makes some sense to me. We
> can always add a new compatible like "efi-boottime-services" later. It
> may be that the boottime services would be handled by the payload, so
> not needed in all cases.
Why later? You have a specific use in mind and I imagine Ard has
thoughts on that.
Rob
Hi Rob,
On Mon, 16 Oct 2023 at 10:50, Rob Herring <[email protected]> wrote:
>
> On Fri, Oct 13, 2023 at 4:09 PM Simon Glass <[email protected]> wrote:
> >
> > Hi Rob,
> >
> > On Fri, 13 Oct 2023 at 13:42, Rob Herring <[email protected]> wrote:
> > >
> > > On Fri, Oct 6, 2023 at 7:03 PM Simon Glass <[email protected]> wrote:
> > > >
> > > > Hi Ard,
> > > >
> > > > On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel <[email protected]> wrote:
> > > > >
> > > > > On Fri, 6 Oct 2023 at 20:17, Simon Glass <[email protected]> wrote:
> > > > > >
> > > > > > Hi Ard,
> > > > > >
> > > > > > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel <[email protected]> wrote:
> > > > > > >
> > > > > > > On Mon, 2 Oct 2023 at 19:54, Simon Glass <[email protected]> wrote:
> > > > > > > >
> > > > > > > > Hi Rob,
> > > > > > > >
> > > > > > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
> > > > > > > > >
> > > > > > > > > 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 small schema addition for the memory mapping
> > > > > > > > > needed to keep these two pieces working together well.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass <[email protected]>
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > Changes in v7:
> > > > > > > > > - Rename acpi-reclaim to acpi
> > > > > > > > > - Drop individual mention of when memory can be reclaimed
> > > > > > > > > - Rewrite the item descriptions
> > > > > > > > > - Add back the UEFI text (with trepidation)
> > > > > > > >
> > > > > > > > I am again checking on this series. Can it be applied, please?
> > > > > > > >
> > > > > > >
> > > > > > > Apologies for the delay in response. I have been away.
> > > > > >
> > > > > > OK, I hope you had a nice trip.
> > > > > >
> > > > >
> > > > > Thanks, it was wonderful!
> > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > 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..f7fbdfd
> > > > > > > > > --- /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 <[email protected]>
> > > > > > > > > +
> > > > > > > > > +allOf:
> > > > > > > > > + - $ref: reserved-memory.yaml
> > > > > > > > > +
> > > > > > > > > +properties:
> > > > > > > > > + compatible:
> > > > > > > > > + description: |
> > > > > > > > > + This describes some common memory reservations, with the compatible
> > > > > > > > > + string indicating what it is used for:
> > > > > > > > > +
> > > > > > > > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > > > > > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > > > > > > > > + the firmware for its use and is required to be saved and restored
> > > > > > > > > + across an NVS sleep
> > > > > > > > > + boot-code: Contains code used for booting which is not needed by the OS
> > > > > > > > > + boot-code: Contains data used for booting which is not needed by the OS
> > > > > > > > > + runtime-code: Contains code used for interacting with the system when
> > > > > > > > > + running the OS
> > > > > > > > > + runtime-data: Contains data used for interacting with the system when
> > > > > > > > > + running the OS
> > > > > > > > > +
> > > > > > > > > + enum:
> > > > > > > > > + - acpi
> > > > > > > > > + - acpi-nvs
> > > > > > > > > + - boot-code
> > > > > > > > > + - boot-data
> > > > > > > > > + - runtime-code
> > > > > > > > > + - runtime-data
> > > > > > > > > +
> > > > > > >
> > > > > > > As I mentioned a few times already, I don't think these compatibles
> > > > > > > should be introduced here.
> > > > > > >
> > > > > > > A reserved region has a specific purpose, and the compatible should be
> > > > > > > more descriptive than the enum above. If the consumer does not
> > > > > > > understand this purpose, it should simply treat the memory as reserved
> > > > > > > and not touch it. Alternatively, these regions can be referenced from
> > > > > > > other DT nodes using phandles if needed.
> > > > > >
> > > > > > We still need some description of what these regions are used for, so
> > > > > > that the payload can use the correct regions. I do not have any other
> > > > > > solution to this problem. We are in v7 at present. At least explain
> > > > > > where you want the compatible strings to be introduced.
> > > > > >
> > > > >
> > > > > My point is really that by themselves, these regions are not usable by
> > > > > either a payload or an OS that consumes this information. Unless there
> > > > > is some other information being provided (via DT I imagine) that
> > > > > describes how these things are supposed to be used, they are nothing
> > > > > more than memory reservations that should be honored, and providing
> > > > > this arbitrary set of labels is unnecessary.
> > > > >
> > > > > > What sort of extra detail are you looking for? Please be specific and
> > > > > > preferably add some suggestions so I can close this out ASAP.
> > > > > >
> > > > >
> > > > > A payload or OS can do nothing with a memory reservation called
> > > > > 'runtime-code' it it doesn't know what is inside.
> > >
> > > Agreed. The question is WHAT runtime-code? The compatible needs to answer that.
> > >
> > > For example, we have 'ramoops' as a compatible in reserved memory.
> > > That tells us *exactly* what's there. We know how to parse it. If we
> > > know ramoops is not supported, then we know we can toss it out and
> > > reclaim the memory.
> >
> > So if we said:
> >
> > compatible = "runtime-code-efi"
> >
> > would that be OK? We seem to be in catch 22 here, because if I don't
> > mention EFI unhappy, but if I do, Ard is unhappy.
>
> Better yes, because then it is for something specific. However, AIUI,
> that's setup for the OS and defining that region is already defined by
> the EFI memory map. That's Ard's issue. If there's a need outside of
> the EFI to OS handoff,
There is a need. Here is part of the commit message again. If there is
something else you need to know, please ask.
>>>>
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 you need to define what that usecase looks
> like. Describe the problem rather than present your solution.
>
> If this is all specific to EDK2 then it should say that rather than
> 'efi'. I imagine Ard would be happier with something tied to EDK2 than
> *all* UEFI. Though maybe the problem could be any implementation? IDK.
> Maybe it's TF-A that needs to define where the EFI runtime services
> region is and that needs to be passed all the way thru to the EFI
> implementation? So again, define the problem.
It is not specific to EDK2. Imagine this boot sequence:
- Platform Init (U-Boot) starts up
- U-Boot uses its platform knowledge to sets some ACPI tables and put
various things in memory
- U-Boot sets up some runtime code and data for the OS
- U-Boot jumps to the Tianocore payload **
- Payload (Tianocore) wants to know where the ACPI tables are, for example
- Tianocore needs to provide boot services to the OS, so needs to know
the memory map, etc.
** At this point we want to use DT to pass the required information.
Of course, Platform Init could be coreboot or Tianocore or some
strange private binary. Payload could be U-Boot or something else.
That is the point of this effort, to build interoperability.
>
> > What about the boottime code....you want to know which project it is from?
>
> I think it is the same.
>
> >
> > + - acpi
> > + - acpi-nvs
> >
> > Those two should be enough info, right?
>
> I think so. NVS is not a term I've heard in relation to ACPI, but that
> may just be my limited ACPI knowledge.
Perhaps it is only an Intel thing. It stands for Non-Volatile-Sleeping
Memory and it has various platform settings in a binary format that is
normally SoC-specific.
>
> > + - boot-code
> > + - boot-data
> >
> > For these, they don't pertain to the OS, so perhaps they are OK?
>
> Hard to tell that just from the name... 'boot' could be any component
> involved in booting including the OS.
suggested that 'boot' should mean booting the OS. If the OS does lots
of fixup stuff at the start of it, I don't know what that is called.
So if boot-code is no good, what do you suggest?
Alternatively I could remove these for now, if it will help make progress.
>
> > In
> > any case, using a generic term like this makes some sense to me. We
> > can always add a new compatible like "efi-boottime-services" later. It
> > may be that the boottime services would be handled by the payload, so
> > not needed in all cases.
>
> Why later? You have a specific use in mind and I imagine Ard has
> thoughts on that.
Because we don't need it right away, and just want to make some progress.
Perhaps the problem here is that Linux has tied itself up in knots
with its EFI stuff and DT fixups and what-not. But this is not that.
It is a simple handoff between two pieces of firmware, Platform Init
and Payload. It has nothing to do with the OS. With Tianocore they are
typically combined, but with this usage they are split, and we can
swap out one project for another on either side of the DT interface.
I do have views on the 'EFI' opt-out with reserved-memory, if you are
interested, but that relates to the OS. If you are wanting to place
some constraints on /reserved-memory and /memory we could do that
e.g. that the DT and the map returned by EFI boot services must be
consistent. But as it is, the nodes are ignored by the OS when booting
with EFI, aren't they?
Regards,
Simon
Hi Rob,
On Mon, 16 Oct 2023 at 15:54, Simon Glass <[email protected]> wrote:
>
> Hi Rob,
>
> On Mon, 16 Oct 2023 at 10:50, Rob Herring <[email protected]> wrote:
> >
> > On Fri, Oct 13, 2023 at 4:09 PM Simon Glass <[email protected]> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Fri, 13 Oct 2023 at 13:42, Rob Herring <[email protected]> wrote:
> > > >
> > > > On Fri, Oct 6, 2023 at 7:03 PM Simon Glass <[email protected]> wrote:
> > > > >
> > > > > Hi Ard,
> > > > >
> > > > > On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel <[email protected]> wrote:
> > > > > >
> > > > > > On Fri, 6 Oct 2023 at 20:17, Simon Glass <[email protected]> wrote:
> > > > > > >
> > > > > > > Hi Ard,
> > > > > > >
> > > > > > > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel <[email protected]> wrote:
> > > > > > > >
> > > > > > > > On Mon, 2 Oct 2023 at 19:54, Simon Glass <[email protected]> wrote:
> > > > > > > > >
> > > > > > > > > Hi Rob,
> > > > > > > > >
> > > > > > > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass <[email protected]> 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.
> > > > > > > > > >
> > > > > > > > > > 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 small schema addition for the memory mapping
> > > > > > > > > > needed to keep these two pieces working together well.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Simon Glass <[email protected]>
> > > > > > > > > > ---
> > > > > > > > > >
> > > > > > > > > > Changes in v7:
> > > > > > > > > > - Rename acpi-reclaim to acpi
> > > > > > > > > > - Drop individual mention of when memory can be reclaimed
> > > > > > > > > > - Rewrite the item descriptions
> > > > > > > > > > - Add back the UEFI text (with trepidation)
> > > > > > > > >
> > > > > > > > > I am again checking on this series. Can it be applied, please?
> > > > > > > > >
> > > > > > > >
> > > > > > > > Apologies for the delay in response. I have been away.
> > > > > > >
> > > > > > > OK, I hope you had a nice trip.
> > > > > > >
> > > > > >
> > > > > > Thanks, it was wonderful!
> > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > 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..f7fbdfd
> > > > > > > > > > --- /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 <[email protected]>
> > > > > > > > > > +
> > > > > > > > > > +allOf:
> > > > > > > > > > + - $ref: reserved-memory.yaml
> > > > > > > > > > +
> > > > > > > > > > +properties:
> > > > > > > > > > + compatible:
> > > > > > > > > > + description: |
> > > > > > > > > > + This describes some common memory reservations, with the compatible
> > > > > > > > > > + string indicating what it is used for:
> > > > > > > > > > +
> > > > > > > > > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > > > > > > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > > > > > > > > > + the firmware for its use and is required to be saved and restored
> > > > > > > > > > + across an NVS sleep
> > > > > > > > > > + boot-code: Contains code used for booting which is not needed by the OS
> > > > > > > > > > + boot-code: Contains data used for booting which is not needed by the OS
> > > > > > > > > > + runtime-code: Contains code used for interacting with the system when
> > > > > > > > > > + running the OS
> > > > > > > > > > + runtime-data: Contains data used for interacting with the system when
> > > > > > > > > > + running the OS
> > > > > > > > > > +
> > > > > > > > > > + enum:
> > > > > > > > > > + - acpi
> > > > > > > > > > + - acpi-nvs
> > > > > > > > > > + - boot-code
> > > > > > > > > > + - boot-data
> > > > > > > > > > + - runtime-code
> > > > > > > > > > + - runtime-data
> > > > > > > > > > +
> > > > > > > >
> > > > > > > > As I mentioned a few times already, I don't think these compatibles
> > > > > > > > should be introduced here.
> > > > > > > >
> > > > > > > > A reserved region has a specific purpose, and the compatible should be
> > > > > > > > more descriptive than the enum above. If the consumer does not
> > > > > > > > understand this purpose, it should simply treat the memory as reserved
> > > > > > > > and not touch it. Alternatively, these regions can be referenced from
> > > > > > > > other DT nodes using phandles if needed.
> > > > > > >
> > > > > > > We still need some description of what these regions are used for, so
> > > > > > > that the payload can use the correct regions. I do not have any other
> > > > > > > solution to this problem. We are in v7 at present. At least explain
> > > > > > > where you want the compatible strings to be introduced.
> > > > > > >
> > > > > >
> > > > > > My point is really that by themselves, these regions are not usable by
> > > > > > either a payload or an OS that consumes this information. Unless there
> > > > > > is some other information being provided (via DT I imagine) that
> > > > > > describes how these things are supposed to be used, they are nothing
> > > > > > more than memory reservations that should be honored, and providing
> > > > > > this arbitrary set of labels is unnecessary.
> > > > > >
> > > > > > > What sort of extra detail are you looking for? Please be specific and
> > > > > > > preferably add some suggestions so I can close this out ASAP.
> > > > > > >
> > > > > >
> > > > > > A payload or OS can do nothing with a memory reservation called
> > > > > > 'runtime-code' it it doesn't know what is inside.
> > > >
> > > > Agreed. The question is WHAT runtime-code? The compatible needs to answer that.
> > > >
> > > > For example, we have 'ramoops' as a compatible in reserved memory.
> > > > That tells us *exactly* what's there. We know how to parse it. If we
> > > > know ramoops is not supported, then we know we can toss it out and
> > > > reclaim the memory.
> > >
> > > So if we said:
> > >
> > > compatible = "runtime-code-efi"
> > >
> > > would that be OK? We seem to be in catch 22 here, because if I don't
> > > mention EFI unhappy, but if I do, Ard is unhappy.
> >
> > Better yes, because then it is for something specific. However, AIUI,
> > that's setup for the OS and defining that region is already defined by
> > the EFI memory map. That's Ard's issue. If there's a need outside of
> > the EFI to OS handoff,
>
> There is a need. Here is part of the commit message again. If there is
> something else you need to know, please ask.
>
> >>>>
> 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 you need to define what that usecase looks
> > like. Describe the problem rather than present your solution.
> >
> > If this is all specific to EDK2 then it should say that rather than
> > 'efi'. I imagine Ard would be happier with something tied to EDK2 than
> > *all* UEFI. Though maybe the problem could be any implementation? IDK.
> > Maybe it's TF-A that needs to define where the EFI runtime services
> > region is and that needs to be passed all the way thru to the EFI
> > implementation? So again, define the problem.
>
> It is not specific to EDK2. Imagine this boot sequence:
>
> - Platform Init (U-Boot) starts up
> - U-Boot uses its platform knowledge to sets some ACPI tables and put
> various things in memory
> - U-Boot sets up some runtime code and data for the OS
> - U-Boot jumps to the Tianocore payload **
> - Payload (Tianocore) wants to know where the ACPI tables are, for example
> - Tianocore needs to provide boot services to the OS, so needs to know
> the memory map, etc.
>
> ** At this point we want to use DT to pass the required information.
>
> Of course, Platform Init could be coreboot or Tianocore or some
> strange private binary. Payload could be U-Boot or something else.
> That is the point of this effort, to build interoperability.
>
> >
> > > What about the boottime code....you want to know which project it is from?
> >
> > I think it is the same.
> >
> > >
> > > + - acpi
> > > + - acpi-nvs
> > >
> > > Those two should be enough info, right?
> >
> > I think so. NVS is not a term I've heard in relation to ACPI, but that
> > may just be my limited ACPI knowledge.
>
> Perhaps it is only an Intel thing. It stands for Non-Volatile-Sleeping
> Memory and it has various platform settings in a binary format that is
> normally SoC-specific.
>
> >
> > > + - boot-code
> > > + - boot-data
> > >
> > > For these, they don't pertain to the OS, so perhaps they are OK?
> >
> > Hard to tell that just from the name... 'boot' could be any component
> > involved in booting including the OS.
>
> suggested that 'boot' should mean booting the OS. If the OS does lots
> of fixup stuff at the start of it, I don't know what that is called.
>
> So if boot-code is no good, what do you suggest?
>
> Alternatively I could remove these for now, if it will help make progress.
>
> >
> > > In
> > > any case, using a generic term like this makes some sense to me. We
> > > can always add a new compatible like "efi-boottime-services" later. It
> > > may be that the boottime services would be handled by the payload, so
> > > not needed in all cases.
> >
> > Why later? You have a specific use in mind and I imagine Ard has
> > thoughts on that.
>
> Because we don't need it right away, and just want to make some progress.
>
> Perhaps the problem here is that Linux has tied itself up in knots
> with its EFI stuff and DT fixups and what-not. But this is not that.
> It is a simple handoff between two pieces of firmware, Platform Init
> and Payload. It has nothing to do with the OS. With Tianocore they are
> typically combined, but with this usage they are split, and we can
> swap out one project for another on either side of the DT interface.
>
> I do have views on the 'EFI' opt-out with reserved-memory, if you are
> interested, but that relates to the OS. If you are wanting to place
> some constraints on /reserved-memory and /memory we could do that
> e.g. that the DT and the map returned by EFI boot services must be
> consistent. But as it is, the nodes are ignored by the OS when booting
> with EFI, aren't they?
Can this be applied, please? If there are further comments, please let me know.
Regards,
Simon
Hi,
On Mon, 13 Nov 2023 at 11:09, Chiu, Chasel <[email protected]> wrote:
>
>
> Hi Ard,
>
> Please see my reply below inline.
>
> Thanks,
> Chasel
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel <[email protected]>
> > Sent: Saturday, November 11, 2023 3:04 AM
> > To: Chiu, Chasel <[email protected]>
> > Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> > <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> > <[email protected]>; lkml <[email protected]>; Dhaval
> > Sharma <[email protected]>; Brune, Maximilian
> > <[email protected]>; Yunhui Cui <[email protected]>;
> > Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > [email protected]; U-Boot Mailing List <[email protected]>
> > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > usages
> >
> > On Sat, 11 Nov 2023 at 04:20, Chiu, Chasel <[email protected]> wrote:
> > >
> > >
> > > Just sharing some usage examples from UEFI/EDK2 scenario.
> > > To support ACPI S4/Hibernation, memory map must be consistent before
> > > entering and after resuming from S4, in this case payload may need to
> > > know previous memory map from bootloader (currently generic payload
> > > cannot access platform/bootloader specific non-volatile data, thus
> > > could not save/restore memory map information)
> >
> > So how would EDK2 reconstruct the entire EFI memory map from just these
> > unannotated /reserved-memory nodes? The EFI memory map contains much
> > more information than that, and all of it has to match the pre-hibernate situation,
> > right? Can you given an example?
>
>
> Here we listed only typically memory types that may change cross different platforms.
> Reserved memory type already can be handled by reserved-memory node, and rest of the types usually no need to change cross platforms thus currently we could rely on default in generic payload.
> In the future if we see a need to add new memory types we will discuss and add it to FDT schema.
>
>
>
> >
> > > Another usage is to support binary model which generic payload is a prebuilt
> > binary compatible for all platforms/configurations, however the payload default
> > memory map might not always work for all the configurations and we want to
> > allow bootloader to override payload default memory map without recompiling.
> > >
> >
> > Agreed. But can you explain how a EDK2 payload might make meaningful use of
> > 'runtime-code' regions provided via DT by the non-EDK2 platform init? Can you
> > give an example?
>
>
> Runtime-code/data is used by UEFI payload for booting UEFI OS which required UEFI runtime services.
> Platform Init will select some regions from the usable memory and assign it to runtime-code/data for UPL to consume. Or assign same runtime-code/data from previous boot.
> If UEFI OS is not supported, PlatformInit may not need to provide runtime-code/data regions to payload. (always providing runtime-code/data should be supported too)
>
>
> >
> > > Under below assumption:
> > > FDT OS impact has been evaluated and taken care by relevant
> > experts/stakeholders.
> > > Reviewed-by: Chasel Chiu <[email protected]>
> > >
> >
> > I am sorry but I don't know what 'FDT OS impact' means. We are talking about a
> > firmware-to-firmware abstraction that has the potential to leak into the OS
> > visible interface.
> >
> > I am a maintainer in the Tianocore project myself, so it would help if you could
> > explain who these relevant experts and stakeholders are. Was this discussed on
> > the edk2-devel mailing list? If so, apologies for missing it but I may not have been
> > cc'ed perhaps?
>
>
>
>
> I'm not familiar with FDT OS, also I do not know if who from edk2-devel were supporting FDT OS, I think Simon might be able to connect FDT OS experts/stakeholders.
> We are mostly focusing on payload firmware phase implementation in edk2 (and other payloads too), however, since we have aligned the payload FDT and OS FDT months ago, I'm assuming FDT OS impact must be there and we need (or already done?) FDT OS experts to support it. (again, maybe Simon could share more information about FDT OS)
>
> In edk2 such FDT schema is UefiPayloadPkg internal usage only and payload entry will convert FDT into HOB thus we expected the most of the edk2 generic code are no-touch/no impact, that's why we only had small group (UefiPayloadPkg) discussion.
> Ard, if you are aware of any edk2 code that's for supporting FDT OS, please let us know and we can discuss if those code were impacted or not.
We discussed this and just to clarify, 'FDT OS' is not a special OS,
it is just Linux.
So, with the above, are we all on the same page? Can the patch be
applied, perhaps? If not, what other discussion is needed?
Regards,
Simon
>
>
>
>
> >
> >
> > >
> > > > -----Original Message-----
> > > > From: Simon Glass <[email protected]>
> > > > Sent: Tuesday, September 26, 2023 12:43 PM
> > > > To: [email protected]
> > > > Cc: Mark Rutland <[email protected]>; Rob Herring
> > > > <[email protected]>; Tan, Lean Sheng <[email protected]>; lkml
> > > > <linux- [email protected]>; Dhaval Sharma
> > > > <[email protected]>; Brune, Maximilian
> > > > <[email protected]>; Yunhui Cui
> > > > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > > > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > > > <[email protected]>; Chiu, Chasel <[email protected]>; linux-
> > > > [email protected]; U-Boot Mailing List <[email protected]>;
> > > > Ard Biesheuvel <[email protected]>; Simon Glass <[email protected]>
> > > > Subject: [PATCH v7 2/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.
> > > >
> > > > 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 small schema addition for the memory mapping
> > > > needed to keep these two pieces working together well.
> > > >
> > > > Signed-off-by: Simon Glass <[email protected]>
> > > > ---
> > > >
> > > > Changes in v7:
> > > > - Rename acpi-reclaim to acpi
> > > > - Drop individual mention of when memory can be reclaimed
> > > > - Rewrite the item descriptions
> > > > - Add back the UEFI text (with trepidation)
> > > >
> > > > 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..f7fbdfd
> > > > --- /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 <[email protected]>
> > > > +
> > > > +allOf:
> > > > + - $ref: reserved-memory.yaml
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + description: |
> > > > + This describes some common memory reservations, with the compatible
> > > > + string indicating what it is used for:
> > > > +
> > > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is reserved by
> > > > + the firmware for its use and is required to be saved and restored
> > > > + across an NVS sleep
> > > > + boot-code: Contains code used for booting which is not needed by the
> > OS
> > > > + boot-code: Contains data used for booting which is not needed by the
> > OS
> > > > + runtime-code: Contains code used for interacting with the system when
> > > > + running the OS
> > > > + runtime-data: Contains data used for interacting with the system when
> > > > + running the OS
> > > > +
> > > > + enum:
> > > > + - acpi
> > > > + - 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>;
> > > > + };
> > > > + };
> > > > --
> > > > 2.42.0.515.g380fc7ccd1-goog
> > >
On Mon, 20 Nov 2023 at 21:12, Simon Glass <[email protected]> wrote:
>
> Hi,
>
> On Mon, 13 Nov 2023 at 11:09, Chiu, Chasel <[email protected]> wrote:
> >
> >
> > Hi Ard,
> >
> > Please see my reply below inline.
> >
> > Thanks,
> > Chasel
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <[email protected]>
> > > Sent: Saturday, November 11, 2023 3:04 AM
> > > To: Chiu, Chasel <[email protected]>
> > > Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> > > <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> > > <[email protected]>; lkml <[email protected]>; Dhaval
> > > Sharma <[email protected]>; Brune, Maximilian
> > > <[email protected]>; Yunhui Cui <[email protected]>;
> > > Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> > > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > > [email protected]; U-Boot Mailing List <[email protected]>
> > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > usages
> > >
> > > On Sat, 11 Nov 2023 at 04:20, Chiu, Chasel <[email protected]> wrote:
> > > >
> > > >
> > > > Just sharing some usage examples from UEFI/EDK2 scenario.
> > > > To support ACPI S4/Hibernation, memory map must be consistent before
> > > > entering and after resuming from S4, in this case payload may need to
> > > > know previous memory map from bootloader (currently generic payload
> > > > cannot access platform/bootloader specific non-volatile data, thus
> > > > could not save/restore memory map information)
> > >
> > > So how would EDK2 reconstruct the entire EFI memory map from just these
> > > unannotated /reserved-memory nodes? The EFI memory map contains much
> > > more information than that, and all of it has to match the pre-hibernate situation,
> > > right? Can you given an example?
> >
> >
> > Here we listed only typically memory types that may change cross different platforms.
> > Reserved memory type already can be handled by reserved-memory node, and rest of the types usually no need to change cross platforms thus currently we could rely on default in generic payload.
> > In the future if we see a need to add new memory types we will discuss and add it to FDT schema.
> >
> >
> >
> > >
> > > > Another usage is to support binary model which generic payload is a prebuilt
> > > binary compatible for all platforms/configurations, however the payload default
> > > memory map might not always work for all the configurations and we want to
> > > allow bootloader to override payload default memory map without recompiling.
> > > >
> > >
> > > Agreed. But can you explain how a EDK2 payload might make meaningful use of
> > > 'runtime-code' regions provided via DT by the non-EDK2 platform init? Can you
> > > give an example?
> >
> >
> > Runtime-code/data is used by UEFI payload for booting UEFI OS which required UEFI runtime services.
> > Platform Init will select some regions from the usable memory and assign it to runtime-code/data for UPL to consume. Or assign same runtime-code/data from previous boot.
> > If UEFI OS is not supported, PlatformInit may not need to provide runtime-code/data regions to payload. (always providing runtime-code/data should be supported too)
> >
> >
> > >
> > > > Under below assumption:
> > > > FDT OS impact has been evaluated and taken care by relevant
> > > experts/stakeholders.
> > > > Reviewed-by: Chasel Chiu <[email protected]>
> > > >
> > >
> > > I am sorry but I don't know what 'FDT OS impact' means. We are talking about a
> > > firmware-to-firmware abstraction that has the potential to leak into the OS
> > > visible interface.
> > >
> > > I am a maintainer in the Tianocore project myself, so it would help if you could
> > > explain who these relevant experts and stakeholders are. Was this discussed on
> > > the edk2-devel mailing list? If so, apologies for missing it but I may not have been
> > > cc'ed perhaps?
> >
> >
> >
> >
> > I'm not familiar with FDT OS, also I do not know if who from edk2-devel were supporting FDT OS, I think Simon might be able to connect FDT OS experts/stakeholders.
> > We are mostly focusing on payload firmware phase implementation in edk2 (and other payloads too), however, since we have aligned the payload FDT and OS FDT months ago, I'm assuming FDT OS impact must be there and we need (or already done?) FDT OS experts to support it. (again, maybe Simon could share more information about FDT OS)
> >
> > In edk2 such FDT schema is UefiPayloadPkg internal usage only and payload entry will convert FDT into HOB thus we expected the most of the edk2 generic code are no-touch/no impact, that's why we only had small group (UefiPayloadPkg) discussion.
> > Ard, if you are aware of any edk2 code that's for supporting FDT OS, please let us know and we can discuss if those code were impacted or not.
>
> We discussed this and just to clarify, 'FDT OS' is not a special OS,
> it is just Linux.
>
> So, with the above, are we all on the same page? Can the patch be
> applied, perhaps? If not, what other discussion is needed?
>
An example of how a platform-init/payload combination would make
meaningful use of such runtime-code/data regions.
Hi Ard,
Here is the POC PR for your reference: https://github.com/tianocore/edk2/pull/4969/files#diff-ccebabae5274b21634723a2111ee0de11bed6cfe8cb206ef9e263d9c5f926a9cR268
Please note that this PR is still in early phase and expected to have significant changes.
The idea is that payload entry will create gEfiMemoryTypeInformationGuid HOB with payload default memory types and allow FDT to override if correspond node present.
Please let me know if you have questions or suggestions.
Thanks,
Chasel
> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Tuesday, November 21, 2023 8:42 AM
> To: Simon Glass <[email protected]>
> Cc: Chiu, Chasel <[email protected]>; [email protected]; Mark
> Rutland <[email protected]>; Rob Herring <[email protected]>; Tan, Lean
> Sheng <[email protected]>; lkml <[email protected]>;
> Dhaval Sharma <[email protected]>; Brune, Maximilian
> <[email protected]>; Yunhui Cui <[email protected]>;
> Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> <[email protected]>; Guo, Gua <[email protected]>; linux-
> [email protected]; U-Boot Mailing List <[email protected]>
> Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> usages
>
> On Mon, 20 Nov 2023 at 21:12, Simon Glass <[email protected]> wrote:
> >
> > Hi,
> >
> > On Mon, 13 Nov 2023 at 11:09, Chiu, Chasel <[email protected]> wrote:
> > >
> > >
> > > Hi Ard,
> > >
> > > Please see my reply below inline.
> > >
> > > Thanks,
> > > Chasel
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <[email protected]>
> > > > Sent: Saturday, November 11, 2023 3:04 AM
> > > > To: Chiu, Chasel <[email protected]>
> > > > Cc: Simon Glass <[email protected]>; [email protected];
> > > > Mark Rutland <[email protected]>; Rob Herring
> > > > <[email protected]>; Tan, Lean Sheng <[email protected]>; lkml
> > > > <[email protected]>; Dhaval Sharma
> > > > <[email protected]>; Brune, Maximilian
> > > > <[email protected]>; Yunhui Cui
> > > > <[email protected]>; Dong, Guo <[email protected]>; Tom
> > > > Rini <[email protected]>; ron minnich <[email protected]>; Guo,
> > > > Gua <[email protected]>; linux- [email protected]; U-Boot
> > > > Mailing List <[email protected]>
> > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common
> > > > reserved-memory usages
> > > >
> > > > On Sat, 11 Nov 2023 at 04:20, Chiu, Chasel <[email protected]> wrote:
> > > > >
> > > > >
> > > > > Just sharing some usage examples from UEFI/EDK2 scenario.
> > > > > To support ACPI S4/Hibernation, memory map must be consistent
> > > > > before entering and after resuming from S4, in this case payload
> > > > > may need to know previous memory map from bootloader (currently
> > > > > generic payload cannot access platform/bootloader specific
> > > > > non-volatile data, thus could not save/restore memory map
> > > > > information)
> > > >
> > > > So how would EDK2 reconstruct the entire EFI memory map from just
> > > > these unannotated /reserved-memory nodes? The EFI memory map
> > > > contains much more information than that, and all of it has to
> > > > match the pre-hibernate situation, right? Can you given an example?
> > >
> > >
> > > Here we listed only typically memory types that may change cross different
> platforms.
> > > Reserved memory type already can be handled by reserved-memory node,
> and rest of the types usually no need to change cross platforms thus currently we
> could rely on default in generic payload.
> > > In the future if we see a need to add new memory types we will discuss and
> add it to FDT schema.
> > >
> > >
> > >
> > > >
> > > > > Another usage is to support binary model which generic payload
> > > > > is a prebuilt
> > > > binary compatible for all platforms/configurations, however the
> > > > payload default memory map might not always work for all the
> > > > configurations and we want to allow bootloader to override payload default
> memory map without recompiling.
> > > > >
> > > >
> > > > Agreed. But can you explain how a EDK2 payload might make
> > > > meaningful use of 'runtime-code' regions provided via DT by the
> > > > non-EDK2 platform init? Can you give an example?
> > >
> > >
> > > Runtime-code/data is used by UEFI payload for booting UEFI OS which
> required UEFI runtime services.
> > > Platform Init will select some regions from the usable memory and assign it to
> runtime-code/data for UPL to consume. Or assign same runtime-code/data from
> previous boot.
> > > If UEFI OS is not supported, PlatformInit may not need to provide
> > > runtime-code/data regions to payload. (always providing
> > > runtime-code/data should be supported too)
> > >
> > >
> > > >
> > > > > Under below assumption:
> > > > > FDT OS impact has been evaluated and taken care by
> > > > > relevant
> > > > experts/stakeholders.
> > > > > Reviewed-by: Chasel Chiu <[email protected]>
> > > > >
> > > >
> > > > I am sorry but I don't know what 'FDT OS impact' means. We are
> > > > talking about a firmware-to-firmware abstraction that has the
> > > > potential to leak into the OS visible interface.
> > > >
> > > > I am a maintainer in the Tianocore project myself, so it would
> > > > help if you could explain who these relevant experts and
> > > > stakeholders are. Was this discussed on the edk2-devel mailing
> > > > list? If so, apologies for missing it but I may not have been cc'ed perhaps?
> > >
> > >
> > >
> > >
> > > I'm not familiar with FDT OS, also I do not know if who from edk2-devel were
> supporting FDT OS, I think Simon might be able to connect FDT OS
> experts/stakeholders.
> > > We are mostly focusing on payload firmware phase implementation in
> > > edk2 (and other payloads too), however, since we have aligned the
> > > payload FDT and OS FDT months ago, I'm assuming FDT OS impact must
> > > be there and we need (or already done?) FDT OS experts to support
> > > it. (again, maybe Simon could share more information about FDT OS)
> > >
> > > In edk2 such FDT schema is UefiPayloadPkg internal usage only and payload
> entry will convert FDT into HOB thus we expected the most of the edk2 generic
> code are no-touch/no impact, that's why we only had small group
> (UefiPayloadPkg) discussion.
> > > Ard, if you are aware of any edk2 code that's for supporting FDT OS, please let
> us know and we can discuss if those code were impacted or not.
> >
> > We discussed this and just to clarify, 'FDT OS' is not a special OS,
> > it is just Linux.
> >
> > So, with the above, are we all on the same page? Can the patch be
> > applied, perhaps? If not, what other discussion is needed?
> >
>
> An example of how a platform-init/payload combination would make meaningful
> use of such runtime-code/data regions.
In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line 268 is for related example code.
Thanks,
Chasel
> -----Original Message-----
> From: Chiu, Chasel
> Sent: Tuesday, November 21, 2023 10:34 AM
> To: Ard Biesheuvel <[email protected]>; Simon Glass <[email protected]>
> Cc: [email protected]; Mark Rutland <[email protected]>; Rob
> Herring <[email protected]>; Tan, Lean Sheng <[email protected]>; lkml
> <[email protected]>; Dhaval Sharma <[email protected]>; Brune,
> Maximilian <[email protected]>; Yunhui Cui
> <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> <[email protected]>; [email protected]; U-Boot Mailing List <u-
> [email protected]>; Chiu, Chasel <[email protected]>
> Subject: RE: [PATCH v7 2/2] schemas: Add some common reserved-memory
> usages
>
>
> Hi Ard,
>
> Here is the POC PR for your reference:
> https://github.com/tianocore/edk2/pull/4969/files#diff-
> ccebabae5274b21634723a2111ee0de11bed6cfe8cb206ef9e263d9c5f926a9cR26
> 8
> Please note that this PR is still in early phase and expected to have significant
> changes.
>
> The idea is that payload entry will create gEfiMemoryTypeInformationGuid HOB
> with payload default memory types and allow FDT to override if correspond node
> present.
> Please let me know if you have questions or suggestions.
>
> Thanks,
> Chasel
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel <[email protected]>
> > Sent: Tuesday, November 21, 2023 8:42 AM
> > To: Simon Glass <[email protected]>
> > Cc: Chiu, Chasel <[email protected]>; [email protected];
> > Mark Rutland <[email protected]>; Rob Herring <[email protected]>;
> > Tan, Lean Sheng <[email protected]>; lkml
> > <[email protected]>; Dhaval Sharma <[email protected]>;
> > Brune, Maximilian <[email protected]>; Yunhui Cui
> > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > <[email protected]>; linux- [email protected]; U-Boot Mailing List
> > <[email protected]>
> > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > usages
> >
> > On Mon, 20 Nov 2023 at 21:12, Simon Glass <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, 13 Nov 2023 at 11:09, Chiu, Chasel <[email protected]> wrote:
> > > >
> > > >
> > > > Hi Ard,
> > > >
> > > > Please see my reply below inline.
> > > >
> > > > Thanks,
> > > > Chasel
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel <[email protected]>
> > > > > Sent: Saturday, November 11, 2023 3:04 AM
> > > > > To: Chiu, Chasel <[email protected]>
> > > > > Cc: Simon Glass <[email protected]>; [email protected];
> > > > > Mark Rutland <[email protected]>; Rob Herring
> > > > > <[email protected]>; Tan, Lean Sheng <[email protected]>;
> > > > > lkml <[email protected]>; Dhaval Sharma
> > > > > <[email protected]>; Brune, Maximilian
> > > > > <[email protected]>; Yunhui Cui
> > > > > <[email protected]>; Dong, Guo <[email protected]>; Tom
> > > > > Rini <[email protected]>; ron minnich <[email protected]>;
> > > > > Guo, Gua <[email protected]>; linux- [email protected];
> > > > > U-Boot Mailing List <[email protected]>
> > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common
> > > > > reserved-memory usages
> > > > >
> > > > > On Sat, 11 Nov 2023 at 04:20, Chiu, Chasel <[email protected]>
> wrote:
> > > > > >
> > > > > >
> > > > > > Just sharing some usage examples from UEFI/EDK2 scenario.
> > > > > > To support ACPI S4/Hibernation, memory map must be consistent
> > > > > > before entering and after resuming from S4, in this case
> > > > > > payload may need to know previous memory map from bootloader
> > > > > > (currently generic payload cannot access platform/bootloader
> > > > > > specific non-volatile data, thus could not save/restore memory
> > > > > > map
> > > > > > information)
> > > > >
> > > > > So how would EDK2 reconstruct the entire EFI memory map from
> > > > > just these unannotated /reserved-memory nodes? The EFI memory
> > > > > map contains much more information than that, and all of it has
> > > > > to match the pre-hibernate situation, right? Can you given an example?
> > > >
> > > >
> > > > Here we listed only typically memory types that may change cross
> > > > different
> > platforms.
> > > > Reserved memory type already can be handled by reserved-memory
> > > > node,
> > and rest of the types usually no need to change cross platforms thus
> > currently we could rely on default in generic payload.
> > > > In the future if we see a need to add new memory types we will
> > > > discuss and
> > add it to FDT schema.
> > > >
> > > >
> > > >
> > > > >
> > > > > > Another usage is to support binary model which generic payload
> > > > > > is a prebuilt
> > > > > binary compatible for all platforms/configurations, however the
> > > > > payload default memory map might not always work for all the
> > > > > configurations and we want to allow bootloader to override
> > > > > payload default
> > memory map without recompiling.
> > > > > >
> > > > >
> > > > > Agreed. But can you explain how a EDK2 payload might make
> > > > > meaningful use of 'runtime-code' regions provided via DT by the
> > > > > non-EDK2 platform init? Can you give an example?
> > > >
> > > >
> > > > Runtime-code/data is used by UEFI payload for booting UEFI OS
> > > > which
> > required UEFI runtime services.
> > > > Platform Init will select some regions from the usable memory and
> > > > assign it to
> > runtime-code/data for UPL to consume. Or assign same runtime-code/data
> > from previous boot.
> > > > If UEFI OS is not supported, PlatformInit may not need to provide
> > > > runtime-code/data regions to payload. (always providing
> > > > runtime-code/data should be supported too)
> > > >
> > > >
> > > > >
> > > > > > Under below assumption:
> > > > > > FDT OS impact has been evaluated and taken care by
> > > > > > relevant
> > > > > experts/stakeholders.
> > > > > > Reviewed-by: Chasel Chiu <[email protected]>
> > > > > >
> > > > >
> > > > > I am sorry but I don't know what 'FDT OS impact' means. We are
> > > > > talking about a firmware-to-firmware abstraction that has the
> > > > > potential to leak into the OS visible interface.
> > > > >
> > > > > I am a maintainer in the Tianocore project myself, so it would
> > > > > help if you could explain who these relevant experts and
> > > > > stakeholders are. Was this discussed on the edk2-devel mailing
> > > > > list? If so, apologies for missing it but I may not have been cc'ed perhaps?
> > > >
> > > >
> > > >
> > > >
> > > > I'm not familiar with FDT OS, also I do not know if who from
> > > > edk2-devel were
> > supporting FDT OS, I think Simon might be able to connect FDT OS
> > experts/stakeholders.
> > > > We are mostly focusing on payload firmware phase implementation in
> > > > edk2 (and other payloads too), however, since we have aligned the
> > > > payload FDT and OS FDT months ago, I'm assuming FDT OS impact must
> > > > be there and we need (or already done?) FDT OS experts to support
> > > > it. (again, maybe Simon could share more information about FDT OS)
> > > >
> > > > In edk2 such FDT schema is UefiPayloadPkg internal usage only and
> > > > payload
> > entry will convert FDT into HOB thus we expected the most of the edk2
> > generic code are no-touch/no impact, that's why we only had small
> > group
> > (UefiPayloadPkg) discussion.
> > > > Ard, if you are aware of any edk2 code that's for supporting FDT
> > > > OS, please let
> > us know and we can discuss if those code were impacted or not.
> > >
> > > We discussed this and just to clarify, 'FDT OS' is not a special OS,
> > > it is just Linux.
> > >
> > > So, with the above, are we all on the same page? Can the patch be
> > > applied, perhaps? If not, what other discussion is needed?
> > >
> >
> > An example of how a platform-init/payload combination would make
> > meaningful use of such runtime-code/data regions.
You are referring to a 2000 line patch so it is not 100% clear where
to look tbh.
On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <[email protected]> wrote:
>
>
> In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line 268 is for related example code.
>
That refers to a 'memory-allocation' node, right? How does that relate
to the 'reserved-memory' node?
And crucially, how does this clarify in which way "runtime-code" and
"runtime-data" reservations are being used?
Since the very beginning of this discussion, I have been asking
repeatedly for examples that describe the wider context in which these
reservations are used. The "runtime" into runtime-code and
runtime-data means that these regions have a special significance to
the operating system, not just to the next bootloader stage. So I want
to understand exactly why it is necessary to describe these regions in
a way where the operating system might be expected to interpret this
information and act upon it.
>
> > -----Original Message-----
> > From: Chiu, Chasel
> > Sent: Tuesday, November 21, 2023 10:34 AM
> > To: Ard Biesheuvel <[email protected]>; Simon Glass <[email protected]>
> > Cc: [email protected]; Mark Rutland <[email protected]>; Rob
> > Herring <[email protected]>; Tan, Lean Sheng <[email protected]>; lkml
> > <[email protected]>; Dhaval Sharma <[email protected]>; Brune,
> > Maximilian <[email protected]>; Yunhui Cui
> > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > <[email protected]>; [email protected]; U-Boot Mailing List <u-
> > [email protected]>; Chiu, Chasel <[email protected]>
> > Subject: RE: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > usages
> >
> >
> > Hi Ard,
> >
> > Here is the POC PR for your reference:
> > https://github.com/tianocore/edk2/pull/4969/files#diff-
> > ccebabae5274b21634723a2111ee0de11bed6cfe8cb206ef9e263d9c5f926a9cR26
> > 8
> > Please note that this PR is still in early phase and expected to have significant
> > changes.
> >
> > The idea is that payload entry will create gEfiMemoryTypeInformationGuid HOB
> > with payload default memory types and allow FDT to override if correspond node
> > present.
> > Please let me know if you have questions or suggestions.
> >
> > Thanks,
> > Chasel
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <[email protected]>
> > > Sent: Tuesday, November 21, 2023 8:42 AM
> > > To: Simon Glass <[email protected]>
> > > Cc: Chiu, Chasel <[email protected]>; [email protected];
> > > Mark Rutland <[email protected]>; Rob Herring <[email protected]>;
> > > Tan, Lean Sheng <[email protected]>; lkml
> > > <[email protected]>; Dhaval Sharma <[email protected]>;
> > > Brune, Maximilian <[email protected]>; Yunhui Cui
> > > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > > <[email protected]>; linux- [email protected]; U-Boot Mailing List
> > > <[email protected]>
> > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > usages
> > >
> > > On Mon, 20 Nov 2023 at 21:12, Simon Glass <[email protected]> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Mon, 13 Nov 2023 at 11:09, Chiu, Chasel <[email protected]> wrote:
> > > > >
> > > > >
> > > > > Hi Ard,
> > > > >
> > > > > Please see my reply below inline.
> > > > >
> > > > > Thanks,
> > > > > Chasel
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ard Biesheuvel <[email protected]>
> > > > > > Sent: Saturday, November 11, 2023 3:04 AM
> > > > > > To: Chiu, Chasel <[email protected]>
> > > > > > Cc: Simon Glass <[email protected]>; [email protected];
> > > > > > Mark Rutland <[email protected]>; Rob Herring
> > > > > > <[email protected]>; Tan, Lean Sheng <[email protected]>;
> > > > > > lkml <[email protected]>; Dhaval Sharma
> > > > > > <[email protected]>; Brune, Maximilian
> > > > > > <[email protected]>; Yunhui Cui
> > > > > > <[email protected]>; Dong, Guo <[email protected]>; Tom
> > > > > > Rini <[email protected]>; ron minnich <[email protected]>;
> > > > > > Guo, Gua <[email protected]>; linux- [email protected];
> > > > > > U-Boot Mailing List <[email protected]>
> > > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common
> > > > > > reserved-memory usages
> > > > > >
> > > > > > On Sat, 11 Nov 2023 at 04:20, Chiu, Chasel <[email protected]>
> > wrote:
> > > > > > >
> > > > > > >
> > > > > > > Just sharing some usage examples from UEFI/EDK2 scenario.
> > > > > > > To support ACPI S4/Hibernation, memory map must be consistent
> > > > > > > before entering and after resuming from S4, in this case
> > > > > > > payload may need to know previous memory map from bootloader
> > > > > > > (currently generic payload cannot access platform/bootloader
> > > > > > > specific non-volatile data, thus could not save/restore memory
> > > > > > > map
> > > > > > > information)
> > > > > >
> > > > > > So how would EDK2 reconstruct the entire EFI memory map from
> > > > > > just these unannotated /reserved-memory nodes? The EFI memory
> > > > > > map contains much more information than that, and all of it has
> > > > > > to match the pre-hibernate situation, right? Can you given an example?
> > > > >
> > > > >
> > > > > Here we listed only typically memory types that may change cross
> > > > > different
> > > platforms.
> > > > > Reserved memory type already can be handled by reserved-memory
> > > > > node,
> > > and rest of the types usually no need to change cross platforms thus
> > > currently we could rely on default in generic payload.
> > > > > In the future if we see a need to add new memory types we will
> > > > > discuss and
> > > add it to FDT schema.
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > > > Another usage is to support binary model which generic payload
> > > > > > > is a prebuilt
> > > > > > binary compatible for all platforms/configurations, however the
> > > > > > payload default memory map might not always work for all the
> > > > > > configurations and we want to allow bootloader to override
> > > > > > payload default
> > > memory map without recompiling.
> > > > > > >
> > > > > >
> > > > > > Agreed. But can you explain how a EDK2 payload might make
> > > > > > meaningful use of 'runtime-code' regions provided via DT by the
> > > > > > non-EDK2 platform init? Can you give an example?
> > > > >
> > > > >
> > > > > Runtime-code/data is used by UEFI payload for booting UEFI OS
> > > > > which
> > > required UEFI runtime services.
> > > > > Platform Init will select some regions from the usable memory and
> > > > > assign it to
> > > runtime-code/data for UPL to consume. Or assign same runtime-code/data
> > > from previous boot.
> > > > > If UEFI OS is not supported, PlatformInit may not need to provide
> > > > > runtime-code/data regions to payload. (always providing
> > > > > runtime-code/data should be supported too)
> > > > >
> > > > >
> > > > > >
> > > > > > > Under below assumption:
> > > > > > > FDT OS impact has been evaluated and taken care by
> > > > > > > relevant
> > > > > > experts/stakeholders.
> > > > > > > Reviewed-by: Chasel Chiu <[email protected]>
> > > > > > >
> > > > > >
> > > > > > I am sorry but I don't know what 'FDT OS impact' means. We are
> > > > > > talking about a firmware-to-firmware abstraction that has the
> > > > > > potential to leak into the OS visible interface.
> > > > > >
> > > > > > I am a maintainer in the Tianocore project myself, so it would
> > > > > > help if you could explain who these relevant experts and
> > > > > > stakeholders are. Was this discussed on the edk2-devel mailing
> > > > > > list? If so, apologies for missing it but I may not have been cc'ed perhaps?
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > I'm not familiar with FDT OS, also I do not know if who from
> > > > > edk2-devel were
> > > supporting FDT OS, I think Simon might be able to connect FDT OS
> > > experts/stakeholders.
> > > > > We are mostly focusing on payload firmware phase implementation in
> > > > > edk2 (and other payloads too), however, since we have aligned the
> > > > > payload FDT and OS FDT months ago, I'm assuming FDT OS impact must
> > > > > be there and we need (or already done?) FDT OS experts to support
> > > > > it. (again, maybe Simon could share more information about FDT OS)
> > > > >
> > > > > In edk2 such FDT schema is UefiPayloadPkg internal usage only and
> > > > > payload
> > > entry will convert FDT into HOB thus we expected the most of the edk2
> > > generic code are no-touch/no impact, that's why we only had small
> > > group
> > > (UefiPayloadPkg) discussion.
> > > > > Ard, if you are aware of any edk2 code that's for supporting FDT
> > > > > OS, please let
> > > us know and we can discuss if those code were impacted or not.
> > > >
> > > > We discussed this and just to clarify, 'FDT OS' is not a special OS,
> > > > it is just Linux.
> > > >
> > > > So, with the above, are we all on the same page? Can the patch be
> > > > applied, perhaps? If not, what other discussion is needed?
> > > >
> > >
> > > An example of how a platform-init/payload combination would make
> > > meaningful use of such runtime-code/data regions.
> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Tuesday, November 28, 2023 10:08 AM
> To: Chiu, Chasel <[email protected]>
> Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> <[email protected]>; lkml <[email protected]>; Dhaval
> Sharma <[email protected]>; Brune, Maximilian
> <[email protected]>; Yunhui Cui <[email protected]>;
> Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> <[email protected]>; Guo, Gua <[email protected]>; linux-
> [email protected]; U-Boot Mailing List <[email protected]>
> Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> usages
>
> You are referring to a 2000 line patch so it is not 100% clear where to look tbh.
>
>
> On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <[email protected]> wrote:
> >
> >
> > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line 268 is for
> related example code.
> >
>
> That refers to a 'memory-allocation' node, right? How does that relate to the
> 'reserved-memory' node?
>
> And crucially, how does this clarify in which way "runtime-code" and "runtime-
> data" reservations are being used?
>
> Since the very beginning of this discussion, I have been asking repeatedly for
> examples that describe the wider context in which these reservations are used.
> The "runtime" into runtime-code and runtime-data means that these regions have
> a special significance to the operating system, not just to the next bootloader
> stage. So I want to understand exactly why it is necessary to describe these
> regions in a way where the operating system might be expected to interpret this
> information and act upon it.
>
I think runtime code and data today are mainly for supporting UEFI runtime services - some BIOS functions for OS to utilize, OS may follow below ACPI spec to treat them as reserved range:
https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.html#uefi-memory-types-and-mapping-to-acpi-address-range-types
Like I mentioned earlier, that PR is still in early phase and has not reflected all the required changes yet, but the idea is to build gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes.
UEFI generic Payload has DxeMain integrated, however Memory Types are platform-specific, for example, some platforms may need bigger runtime memory for their implementation, that's why we want such FDT reserved-memory node to tell DxeMain.
The Payload flow will be like this:
Payload creates built-in default MemoryTypes table ->
FDT reserved-memory node to override if required (this also ensures the same memory map cross boots so ACPI S4 works) ->
Build gEfiMemoryTypeInformationGuid HOB by "platfom specific" MemoryTypes Table ->
DxeMain/GCD to consume this MemoryTypes table and setup memory service ->
Install memory types table to UEFI system table.Configuration table...
Note: if Payload built-in default MemoryTypes table works fine for the platform, then FDT reserved-memory node does not need to provide such 'usage' compatible strings. (optional)
This FDT node could allow flexibility/compatibility without rebuilding Payload binary.
Not sure if I answered all your questions, please highlight which area you need more information.
Thanks,
Chasel
>
> >
> > > -----Original Message-----
> > > From: Chiu, Chasel
> > > Sent: Tuesday, November 21, 2023 10:34 AM
> > > To: Ard Biesheuvel <[email protected]>; Simon Glass <[email protected]>
> > > Cc: [email protected]; Mark Rutland <[email protected]>;
> > > Rob Herring <[email protected]>; Tan, Lean Sheng
> > > <[email protected]>; lkml <[email protected]>;
> > > Dhaval Sharma <[email protected]>; Brune, Maximilian
> > > <[email protected]>; Yunhui Cui
> > > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > > <[email protected]>; [email protected]; U-Boot Mailing List
> > > <u- [email protected]>; Chiu, Chasel <[email protected]>
> > > Subject: RE: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > usages
> > >
> > >
> > > Hi Ard,
> > >
> > > Here is the POC PR for your reference:
> > > https://github.com/tianocore/edk2/pull/4969/files#diff-
> > >
> ccebabae5274b21634723a2111ee0de11bed6cfe8cb206ef9e263d9c5f926a9cR26
> > > 8
> > > Please note that this PR is still in early phase and expected to
> > > have significant changes.
> > >
> > > The idea is that payload entry will create
> > > gEfiMemoryTypeInformationGuid HOB with payload default memory types
> > > and allow FDT to override if correspond node present.
> > > Please let me know if you have questions or suggestions.
> > >
> > > Thanks,
> > > Chasel
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <[email protected]>
> > > > Sent: Tuesday, November 21, 2023 8:42 AM
> > > > To: Simon Glass <[email protected]>
> > > > Cc: Chiu, Chasel <[email protected]>;
> > > > [email protected]; Mark Rutland <[email protected]>;
> > > > Rob Herring <[email protected]>; Tan, Lean Sheng
> > > > <[email protected]>; lkml <[email protected]>;
> > > > Dhaval Sharma <[email protected]>; Brune, Maximilian
> > > > <[email protected]>; Yunhui Cui
> > > > <[email protected]>; Dong, Guo <[email protected]>; Tom
> > > > Rini <[email protected]>; ron minnich <[email protected]>; Guo,
> > > > Gua <[email protected]>; linux- [email protected]; U-Boot
> > > > Mailing List <[email protected]>
> > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common
> > > > reserved-memory usages
> > > >
> > > > On Mon, 20 Nov 2023 at 21:12, Simon Glass <[email protected]> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Mon, 13 Nov 2023 at 11:09, Chiu, Chasel <[email protected]>
> wrote:
> > > > > >
> > > > > >
> > > > > > Hi Ard,
> > > > > >
> > > > > > Please see my reply below inline.
> > > > > >
> > > > > > Thanks,
> > > > > > Chasel
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Ard Biesheuvel <[email protected]>
> > > > > > > Sent: Saturday, November 11, 2023 3:04 AM
> > > > > > > To: Chiu, Chasel <[email protected]>
> > > > > > > Cc: Simon Glass <[email protected]>;
> > > > > > > [email protected]; Mark Rutland
> > > > > > > <[email protected]>; Rob Herring <[email protected]>; Tan,
> > > > > > > Lean Sheng <[email protected]>; lkml
> > > > > > > <[email protected]>; Dhaval Sharma
> > > > > > > <[email protected]>; Brune, Maximilian
> > > > > > > <[email protected]>; Yunhui Cui
> > > > > > > <[email protected]>; Dong, Guo <[email protected]>;
> > > > > > > Tom Rini <[email protected]>; ron minnich
> > > > > > > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > > > > > > [email protected]; U-Boot Mailing List
> > > > > > > <[email protected]>
> > > > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common
> > > > > > > reserved-memory usages
> > > > > > >
> > > > > > > On Sat, 11 Nov 2023 at 04:20, Chiu, Chasel
> > > > > > > <[email protected]>
> > > wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > Just sharing some usage examples from UEFI/EDK2 scenario.
> > > > > > > > To support ACPI S4/Hibernation, memory map must be
> > > > > > > > consistent before entering and after resuming from S4, in
> > > > > > > > this case payload may need to know previous memory map
> > > > > > > > from bootloader (currently generic payload cannot access
> > > > > > > > platform/bootloader specific non-volatile data, thus could
> > > > > > > > not save/restore memory map
> > > > > > > > information)
> > > > > > >
> > > > > > > So how would EDK2 reconstruct the entire EFI memory map from
> > > > > > > just these unannotated /reserved-memory nodes? The EFI
> > > > > > > memory map contains much more information than that, and all
> > > > > > > of it has to match the pre-hibernate situation, right? Can you given an
> example?
> > > > > >
> > > > > >
> > > > > > Here we listed only typically memory types that may change
> > > > > > cross different
> > > > platforms.
> > > > > > Reserved memory type already can be handled by reserved-memory
> > > > > > node,
> > > > and rest of the types usually no need to change cross platforms
> > > > thus currently we could rely on default in generic payload.
> > > > > > In the future if we see a need to add new memory types we will
> > > > > > discuss and
> > > > add it to FDT schema.
> > > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > > Another usage is to support binary model which generic
> > > > > > > > payload is a prebuilt
> > > > > > > binary compatible for all platforms/configurations, however
> > > > > > > the payload default memory map might not always work for all
> > > > > > > the configurations and we want to allow bootloader to
> > > > > > > override payload default
> > > > memory map without recompiling.
> > > > > > > >
> > > > > > >
> > > > > > > Agreed. But can you explain how a EDK2 payload might make
> > > > > > > meaningful use of 'runtime-code' regions provided via DT by
> > > > > > > the
> > > > > > > non-EDK2 platform init? Can you give an example?
> > > > > >
> > > > > >
> > > > > > Runtime-code/data is used by UEFI payload for booting UEFI OS
> > > > > > which
> > > > required UEFI runtime services.
> > > > > > Platform Init will select some regions from the usable memory
> > > > > > and assign it to
> > > > runtime-code/data for UPL to consume. Or assign same
> > > > runtime-code/data from previous boot.
> > > > > > If UEFI OS is not supported, PlatformInit may not need to
> > > > > > provide runtime-code/data regions to payload. (always
> > > > > > providing runtime-code/data should be supported too)
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > > Under below assumption:
> > > > > > > > FDT OS impact has been evaluated and taken care by
> > > > > > > > relevant
> > > > > > > experts/stakeholders.
> > > > > > > > Reviewed-by: Chasel Chiu <[email protected]>
> > > > > > > >
> > > > > > >
> > > > > > > I am sorry but I don't know what 'FDT OS impact' means. We
> > > > > > > are talking about a firmware-to-firmware abstraction that
> > > > > > > has the potential to leak into the OS visible interface.
> > > > > > >
> > > > > > > I am a maintainer in the Tianocore project myself, so it
> > > > > > > would help if you could explain who these relevant experts
> > > > > > > and stakeholders are. Was this discussed on the edk2-devel
> > > > > > > mailing list? If so, apologies for missing it but I may not have been cc'ed
> perhaps?
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > I'm not familiar with FDT OS, also I do not know if who from
> > > > > > edk2-devel were
> > > > supporting FDT OS, I think Simon might be able to connect FDT OS
> > > > experts/stakeholders.
> > > > > > We are mostly focusing on payload firmware phase
> > > > > > implementation in
> > > > > > edk2 (and other payloads too), however, since we have aligned
> > > > > > the payload FDT and OS FDT months ago, I'm assuming FDT OS
> > > > > > impact must be there and we need (or already done?) FDT OS
> > > > > > experts to support it. (again, maybe Simon could share more
> > > > > > information about FDT OS)
> > > > > >
> > > > > > In edk2 such FDT schema is UefiPayloadPkg internal usage only
> > > > > > and payload
> > > > entry will convert FDT into HOB thus we expected the most of the
> > > > edk2 generic code are no-touch/no impact, that's why we only had
> > > > small group
> > > > (UefiPayloadPkg) discussion.
> > > > > > Ard, if you are aware of any edk2 code that's for supporting
> > > > > > FDT OS, please let
> > > > us know and we can discuss if those code were impacted or not.
> > > > >
> > > > > We discussed this and just to clarify, 'FDT OS' is not a special
> > > > > OS, it is just Linux.
> > > > >
> > > > > So, with the above, are we all on the same page? Can the patch
> > > > > be applied, perhaps? If not, what other discussion is needed?
> > > > >
> > > >
> > > > An example of how a platform-init/payload combination would make
> > > > meaningful use of such runtime-code/data regions.
Hi,
On Tue, 28 Nov 2023 at 13:31, Chiu, Chasel <[email protected]> wrote:
>
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel <[email protected]>
> > Sent: Tuesday, November 28, 2023 10:08 AM
> > To: Chiu, Chasel <[email protected]>
> > Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> > <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> > <[email protected]>; lkml <[email protected]>; Dhaval
> > Sharma <[email protected]>; Brune, Maximilian
> > <[email protected]>; Yunhui Cui <[email protected]>;
> > Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > [email protected]; U-Boot Mailing List <[email protected]>
> > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > usages
> >
> > You are referring to a 2000 line patch so it is not 100% clear where to look tbh.
> >
> >
> > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <[email protected]> wrote:
> > >
> > >
> > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line 268 is for
> > related example code.
> > >
> >
> > That refers to a 'memory-allocation' node, right? How does that relate to the
> > 'reserved-memory' node?
> >
> > And crucially, how does this clarify in which way "runtime-code" and "runtime-
> > data" reservations are being used?
> >
> > Since the very beginning of this discussion, I have been asking repeatedly for
> > examples that describe the wider context in which these reservations are used.
> > The "runtime" into runtime-code and runtime-data means that these regions have
> > a special significance to the operating system, not just to the next bootloader
> > stage. So I want to understand exactly why it is necessary to describe these
> > regions in a way where the operating system might be expected to interpret this
> > information and act upon it.
> >
>
>
> I think runtime code and data today are mainly for supporting UEFI runtime services - some BIOS functions for OS to utilize, OS may follow below ACPI spec to treat them as reserved range:
> https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.html#uefi-memory-types-and-mapping-to-acpi-address-range-types
>
> Like I mentioned earlier, that PR is still in early phase and has not reflected all the required changes yet, but the idea is to build gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes.
> UEFI generic Payload has DxeMain integrated, however Memory Types are platform-specific, for example, some platforms may need bigger runtime memory for their implementation, that's why we want such FDT reserved-memory node to tell DxeMain.
>
> The Payload flow will be like this:
> Payload creates built-in default MemoryTypes table ->
> FDT reserved-memory node to override if required (this also ensures the same memory map cross boots so ACPI S4 works) ->
> Build gEfiMemoryTypeInformationGuid HOB by "platfom specific" MemoryTypes Table ->
> DxeMain/GCD to consume this MemoryTypes table and setup memory service ->
> Install memory types table to UEFI system table.Configuration table...
>
> Note: if Payload built-in default MemoryTypes table works fine for the platform, then FDT reserved-memory node does not need to provide such 'usage' compatible strings. (optional)
> This FDT node could allow flexibility/compatibility without rebuilding Payload binary.
>
> Not sure if I answered all your questions, please highlight which area you need more information.
Any more thoughts on this? If not, I would like to see this patch
applied, please.
Regards,
Simon
>
> Thanks,
> Chasel
>
>
> >
> > >
> > > > -----Original Message-----
> > > > From: Chiu, Chasel
> > > > Sent: Tuesday, November 21, 2023 10:34 AM
> > > > To: Ard Biesheuvel <[email protected]>; Simon Glass <[email protected]>
> > > > Cc: [email protected]; Mark Rutland <[email protected]>;
> > > > Rob Herring <[email protected]>; Tan, Lean Sheng
> > > > <[email protected]>; lkml <[email protected]>;
> > > > Dhaval Sharma <[email protected]>; Brune, Maximilian
> > > > <[email protected]>; Yunhui Cui
> > > > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > > > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > > > <[email protected]>; [email protected]; U-Boot Mailing List
> > > > <u- [email protected]>; Chiu, Chasel <[email protected]>
> > > > Subject: RE: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > > usages
> > > >
> > > >
> > > > Hi Ard,
> > > >
> > > > Here is the POC PR for your reference:
> > > > https://github.com/tianocore/edk2/pull/4969/files#diff-
> > > >
> > ccebabae5274b21634723a2111ee0de11bed6cfe8cb206ef9e263d9c5f926a9cR26
> > > > 8
> > > > Please note that this PR is still in early phase and expected to
> > > > have significant changes.
> > > >
> > > > The idea is that payload entry will create
> > > > gEfiMemoryTypeInformationGuid HOB with payload default memory types
> > > > and allow FDT to override if correspond node present.
> > > > Please let me know if you have questions or suggestions.
> > > >
> > > > Thanks,
> > > > Chasel
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel <[email protected]>
> > > > > Sent: Tuesday, November 21, 2023 8:42 AM
> > > > > To: Simon Glass <[email protected]>
> > > > > Cc: Chiu, Chasel <[email protected]>;
> > > > > [email protected]; Mark Rutland <[email protected]>;
> > > > > Rob Herring <[email protected]>; Tan, Lean Sheng
> > > > > <[email protected]>; lkml <[email protected]>;
> > > > > Dhaval Sharma <[email protected]>; Brune, Maximilian
> > > > > <[email protected]>; Yunhui Cui
> > > > > <[email protected]>; Dong, Guo <[email protected]>; Tom
> > > > > Rini <[email protected]>; ron minnich <[email protected]>; Guo,
> > > > > Gua <[email protected]>; linux- [email protected]; U-Boot
> > > > > Mailing List <[email protected]>
> > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common
> > > > > reserved-memory usages
> > > > >
> > > > > On Mon, 20 Nov 2023 at 21:12, Simon Glass <[email protected]> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > On Mon, 13 Nov 2023 at 11:09, Chiu, Chasel <[email protected]>
> > wrote:
> > > > > > >
> > > > > > >
> > > > > > > Hi Ard,
> > > > > > >
> > > > > > > Please see my reply below inline.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Chasel
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Ard Biesheuvel <[email protected]>
> > > > > > > > Sent: Saturday, November 11, 2023 3:04 AM
> > > > > > > > To: Chiu, Chasel <[email protected]>
> > > > > > > > Cc: Simon Glass <[email protected]>;
> > > > > > > > [email protected]; Mark Rutland
> > > > > > > > <[email protected]>; Rob Herring <[email protected]>; Tan,
> > > > > > > > Lean Sheng <[email protected]>; lkml
> > > > > > > > <[email protected]>; Dhaval Sharma
> > > > > > > > <[email protected]>; Brune, Maximilian
> > > > > > > > <[email protected]>; Yunhui Cui
> > > > > > > > <[email protected]>; Dong, Guo <[email protected]>;
> > > > > > > > Tom Rini <[email protected]>; ron minnich
> > > > > > > > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > > > > > > > [email protected]; U-Boot Mailing List
> > > > > > > > <[email protected]>
> > > > > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common
> > > > > > > > reserved-memory usages
> > > > > > > >
> > > > > > > > On Sat, 11 Nov 2023 at 04:20, Chiu, Chasel
> > > > > > > > <[email protected]>
> > > > wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Just sharing some usage examples from UEFI/EDK2 scenario.
> > > > > > > > > To support ACPI S4/Hibernation, memory map must be
> > > > > > > > > consistent before entering and after resuming from S4, in
> > > > > > > > > this case payload may need to know previous memory map
> > > > > > > > > from bootloader (currently generic payload cannot access
> > > > > > > > > platform/bootloader specific non-volatile data, thus could
> > > > > > > > > not save/restore memory map
> > > > > > > > > information)
> > > > > > > >
> > > > > > > > So how would EDK2 reconstruct the entire EFI memory map from
> > > > > > > > just these unannotated /reserved-memory nodes? The EFI
> > > > > > > > memory map contains much more information than that, and all
> > > > > > > > of it has to match the pre-hibernate situation, right? Can you given an
> > example?
> > > > > > >
> > > > > > >
> > > > > > > Here we listed only typically memory types that may change
> > > > > > > cross different
> > > > > platforms.
> > > > > > > Reserved memory type already can be handled by reserved-memory
> > > > > > > node,
> > > > > and rest of the types usually no need to change cross platforms
> > > > > thus currently we could rely on default in generic payload.
> > > > > > > In the future if we see a need to add new memory types we will
> > > > > > > discuss and
> > > > > add it to FDT schema.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > Another usage is to support binary model which generic
> > > > > > > > > payload is a prebuilt
> > > > > > > > binary compatible for all platforms/configurations, however
> > > > > > > > the payload default memory map might not always work for all
> > > > > > > > the configurations and we want to allow bootloader to
> > > > > > > > override payload default
> > > > > memory map without recompiling.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Agreed. But can you explain how a EDK2 payload might make
> > > > > > > > meaningful use of 'runtime-code' regions provided via DT by
> > > > > > > > the
> > > > > > > > non-EDK2 platform init? Can you give an example?
> > > > > > >
> > > > > > >
> > > > > > > Runtime-code/data is used by UEFI payload for booting UEFI OS
> > > > > > > which
> > > > > required UEFI runtime services.
> > > > > > > Platform Init will select some regions from the usable memory
> > > > > > > and assign it to
> > > > > runtime-code/data for UPL to consume. Or assign same
> > > > > runtime-code/data from previous boot.
> > > > > > > If UEFI OS is not supported, PlatformInit may not need to
> > > > > > > provide runtime-code/data regions to payload. (always
> > > > > > > providing runtime-code/data should be supported too)
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > Under below assumption:
> > > > > > > > > FDT OS impact has been evaluated and taken care by
> > > > > > > > > relevant
> > > > > > > > experts/stakeholders.
> > > > > > > > > Reviewed-by: Chasel Chiu <[email protected]>
> > > > > > > > >
> > > > > > > >
> > > > > > > > I am sorry but I don't know what 'FDT OS impact' means. We
> > > > > > > > are talking about a firmware-to-firmware abstraction that
> > > > > > > > has the potential to leak into the OS visible interface.
> > > > > > > >
> > > > > > > > I am a maintainer in the Tianocore project myself, so it
> > > > > > > > would help if you could explain who these relevant experts
> > > > > > > > and stakeholders are. Was this discussed on the edk2-devel
> > > > > > > > mailing list? If so, apologies for missing it but I may not have been cc'ed
> > perhaps?
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > I'm not familiar with FDT OS, also I do not know if who from
> > > > > > > edk2-devel were
> > > > > supporting FDT OS, I think Simon might be able to connect FDT OS
> > > > > experts/stakeholders.
> > > > > > > We are mostly focusing on payload firmware phase
> > > > > > > implementation in
> > > > > > > edk2 (and other payloads too), however, since we have aligned
> > > > > > > the payload FDT and OS FDT months ago, I'm assuming FDT OS
> > > > > > > impact must be there and we need (or already done?) FDT OS
> > > > > > > experts to support it. (again, maybe Simon could share more
> > > > > > > information about FDT OS)
> > > > > > >
> > > > > > > In edk2 such FDT schema is UefiPayloadPkg internal usage only
> > > > > > > and payload
> > > > > entry will convert FDT into HOB thus we expected the most of the
> > > > > edk2 generic code are no-touch/no impact, that's why we only had
> > > > > small group
> > > > > (UefiPayloadPkg) discussion.
> > > > > > > Ard, if you are aware of any edk2 code that's for supporting
> > > > > > > FDT OS, please let
> > > > > us know and we can discuss if those code were impacted or not.
> > > > > >
> > > > > > We discussed this and just to clarify, 'FDT OS' is not a special
> > > > > > OS, it is just Linux.
> > > > > >
> > > > > > So, with the above, are we all on the same page? Can the patch
> > > > > > be applied, perhaps? If not, what other discussion is needed?
> > > > > >
> > > > >
> > > > > An example of how a platform-init/payload combination would make
> > > > > meaningful use of such runtime-code/data regions.
Hi,
On Mon, 11 Dec 2023 at 10:52, Simon Glass <[email protected]> wrote:
>
> Hi,
>
> On Tue, 28 Nov 2023 at 13:31, Chiu, Chasel <[email protected]> wrote:
> >
> >
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <[email protected]>
> > > Sent: Tuesday, November 28, 2023 10:08 AM
> > > To: Chiu, Chasel <[email protected]>
> > > Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> > > <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> > > <[email protected]>; lkml <[email protected]>; Dhaval
> > > Sharma <[email protected]>; Brune, Maximilian
> > > <[email protected]>; Yunhui Cui <[email protected]>;
> > > Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> > > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > > [email protected]; U-Boot Mailing List <[email protected]>
> > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > usages
> > >
> > > You are referring to a 2000 line patch so it is not 100% clear where to look tbh.
> > >
> > >
> > > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <[email protected]> wrote:
> > > >
> > > >
> > > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line 268 is for
> > > related example code.
> > > >
> > >
> > > That refers to a 'memory-allocation' node, right? How does that relate to the
> > > 'reserved-memory' node?
> > >
> > > And crucially, how does this clarify in which way "runtime-code" and "runtime-
> > > data" reservations are being used?
> > >
> > > Since the very beginning of this discussion, I have been asking repeatedly for
> > > examples that describe the wider context in which these reservations are used.
> > > The "runtime" into runtime-code and runtime-data means that these regions have
> > > a special significance to the operating system, not just to the next bootloader
> > > stage. So I want to understand exactly why it is necessary to describe these
> > > regions in a way where the operating system might be expected to interpret this
> > > information and act upon it.
> > >
> >
> >
> > I think runtime code and data today are mainly for supporting UEFI runtime services - some BIOS functions for OS to utilize, OS may follow below ACPI spec to treat them as reserved range:
> > https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.html#uefi-memory-types-and-mapping-to-acpi-address-range-types
> >
> > Like I mentioned earlier, that PR is still in early phase and has not reflected all the required changes yet, but the idea is to build gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes.
> > UEFI generic Payload has DxeMain integrated, however Memory Types are platform-specific, for example, some platforms may need bigger runtime memory for their implementation, that's why we want such FDT reserved-memory node to tell DxeMain.
> >
> > The Payload flow will be like this:
> > Payload creates built-in default MemoryTypes table ->
> > FDT reserved-memory node to override if required (this also ensures the same memory map cross boots so ACPI S4 works) ->
> > Build gEfiMemoryTypeInformationGuid HOB by "platfom specific" MemoryTypes Table ->
> > DxeMain/GCD to consume this MemoryTypes table and setup memory service ->
> > Install memory types table to UEFI system table.Configuration table...
> >
> > Note: if Payload built-in default MemoryTypes table works fine for the platform, then FDT reserved-memory node does not need to provide such 'usage' compatible strings. (optional)
> > This FDT node could allow flexibility/compatibility without rebuilding Payload binary.
> >
> > Not sure if I answered all your questions, please highlight which area you need more information.
>
> Any more thoughts on this? If not, I would like to see this patch
> applied, please.
I am really not sure who or what is holding this up, so far.
Can we perhaps get this applied in time for Christmas? It would be a
nice end to the year.
Regards,
Simon
On Tue, 28 Nov 2023 at 21:31, Chiu, Chasel <[email protected]> wrote:
>
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel <[email protected]>
> > Sent: Tuesday, November 28, 2023 10:08 AM
> > To: Chiu, Chasel <[email protected]>
> > Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> > <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> > <[email protected]>; lkml <[email protected]>; Dhaval
> > Sharma <[email protected]>; Brune, Maximilian
> > <[email protected]>; Yunhui Cui <[email protected]>;
> > Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > [email protected]; U-Boot Mailing List <[email protected]>
> > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > usages
> >
> > You are referring to a 2000 line patch so it is not 100% clear where to look tbh.
> >
> >
> > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <[email protected]> wrote:
> > >
> > >
> > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line 268 is for
> > related example code.
> > >
> >
> > That refers to a 'memory-allocation' node, right? How does that relate to the
> > 'reserved-memory' node?
> >
> > And crucially, how does this clarify in which way "runtime-code" and "runtime-
> > data" reservations are being used?
> >
> > Since the very beginning of this discussion, I have been asking repeatedly for
> > examples that describe the wider context in which these reservations are used.
> > The "runtime" into runtime-code and runtime-data means that these regions have
> > a special significance to the operating system, not just to the next bootloader
> > stage. So I want to understand exactly why it is necessary to describe these
> > regions in a way where the operating system might be expected to interpret this
> > information and act upon it.
> >
>
>
> I think runtime code and data today are mainly for supporting UEFI runtime services - some BIOS functions for OS to utilize, OS may follow below ACPI spec to treat them as reserved range:
> https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.html#uefi-memory-types-and-mapping-to-acpi-address-range-types
>
> Like I mentioned earlier, that PR is still in early phase and has not reflected all the required changes yet, but the idea is to build gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes.
> UEFI generic Payload has DxeMain integrated, however Memory Types are platform-specific, for example, some platforms may need bigger runtime memory for their implementation, that's why we want such FDT reserved-memory node to tell DxeMain.
>
> The Payload flow will be like this:
> Payload creates built-in default MemoryTypes table ->
> FDT reserved-memory node to override if required (this also ensures the same memory map cross boots so ACPI S4 works) ->
> Build gEfiMemoryTypeInformationGuid HOB by "platfom specific" MemoryTypes Table ->
> DxeMain/GCD to consume this MemoryTypes table and setup memory service ->
> Install memory types table to UEFI system table.Configuration table...
>
> Note: if Payload built-in default MemoryTypes table works fine for the platform, then FDT reserved-memory node does not need to provide such 'usage' compatible strings. (optional)
> This FDT node could allow flexibility/compatibility without rebuilding Payload binary.
>
> Not sure if I answered all your questions, please highlight which area you need more information.
>
The gEfiMemoryTypeInformationGuid HOB typically carries platform
defaults, and the actual memory type information is kept in a
non-volatile EFI variable, which gets updated when the memory usage
changes. Is this different for UefiPayloadPkg?
(For those among the cc'ees less versed in EFI/EDK2: when you get the
'config changed -rebooting' message from the boot firmware, it
typically means that this memory type table has changed, and a reboot
is necessary.)
So the platform init needs to read this variable, or get the
information in a different way. I assume it is the payload, not the
platform init that updates the variable when necessary. This means the
information flows from payload(n) to platform init(n+1), where n is a
monotonic index tracking consecutive boots of the system.
Can you explain how the DT fits into this? How are the runtime-code
and runtime-data memory reservation nodes under /reserved-memory used
to implement this information exchange between platform init and
payload? And how do the HOB and the EFI variable fit into this
picture?
Hi Ard,
Please see my reply below inline and let me know your thoughts.
Thanks,
Chasel
> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Thursday, December 21, 2023 6:31 AM
> To: Chiu, Chasel <[email protected]>
> Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> <[email protected]>; lkml <[email protected]>; Dhaval
> Sharma <[email protected]>; Brune, Maximilian
> <[email protected]>; Yunhui Cui <[email protected]>;
> Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> <[email protected]>; Guo, Gua <[email protected]>; linux-
> [email protected]; U-Boot Mailing List <[email protected]>
> Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> usages
>
> On Tue, 28 Nov 2023 at 21:31, Chiu, Chasel <[email protected]> wrote:
> >
> >
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <[email protected]>
> > > Sent: Tuesday, November 28, 2023 10:08 AM
> > > To: Chiu, Chasel <[email protected]>
> > > Cc: Simon Glass <[email protected]>; [email protected]; Mark
> > > Rutland <[email protected]>; Rob Herring <[email protected]>; Tan,
> > > Lean Sheng <[email protected]>; lkml
> > > <[email protected]>; Dhaval Sharma <[email protected]>;
> > > Brune, Maximilian <[email protected]>; Yunhui Cui
> > > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > > <[email protected]>; linux- [email protected]; U-Boot Mailing
> > > List <[email protected]>
> > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > usages
> > >
> > > You are referring to a 2000 line patch so it is not 100% clear where to look tbh.
> > >
> > >
> > > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <[email protected]> wrote:
> > > >
> > > >
> > > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line
> > > > 268 is for
> > > related example code.
> > > >
> > >
> > > That refers to a 'memory-allocation' node, right? How does that
> > > relate to the 'reserved-memory' node?
> > >
> > > And crucially, how does this clarify in which way "runtime-code" and
> > > "runtime- data" reservations are being used?
> > >
> > > Since the very beginning of this discussion, I have been asking
> > > repeatedly for examples that describe the wider context in which these
> reservations are used.
> > > The "runtime" into runtime-code and runtime-data means that these
> > > regions have a special significance to the operating system, not
> > > just to the next bootloader stage. So I want to understand exactly
> > > why it is necessary to describe these regions in a way where the
> > > operating system might be expected to interpret this information and act
> upon it.
> > >
> >
> >
> > I think runtime code and data today are mainly for supporting UEFI runtime
> services - some BIOS functions for OS to utilize, OS may follow below ACPI spec to
> treat them as reserved range:
> > https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.html#
> > uefi-memory-types-and-mapping-to-acpi-address-range-types
> >
> > Like I mentioned earlier, that PR is still in early phase and has not reflected all
> the required changes yet, but the idea is to build
> gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes.
> > UEFI generic Payload has DxeMain integrated, however Memory Types are
> platform-specific, for example, some platforms may need bigger runtime memory
> for their implementation, that's why we want such FDT reserved-memory node to
> tell DxeMain.
> >
>
> > The Payload flow will be like this:
> > Payload creates built-in default MemoryTypes table ->
> > FDT reserved-memory node to override if required (this also ensures the
> same memory map cross boots so ACPI S4 works) ->
> > Build gEfiMemoryTypeInformationGuid HOB by "platfom specific"
> MemoryTypes Table ->
> > DxeMain/GCD to consume this MemoryTypes table and setup memory
> service ->
> > Install memory types table to UEFI system table.Configuration table...
> >
> > Note: if Payload built-in default MemoryTypes table works fine for the
> > platform, then FDT reserved-memory node does not need to provide such
> 'usage' compatible strings. (optional) This FDT node could allow
> flexibility/compatibility without rebuilding Payload binary.
> >
> > Not sure if I answered all your questions, please highlight which area you need
> more information.
> >
>
> The gEfiMemoryTypeInformationGuid HOB typically carries platform defaults, and
> the actual memory type information is kept in a non-volatile EFI variable, which
> gets updated when the memory usage changes. Is this different for
> UefiPayloadPkg?
>
> (For those among the cc'ees less versed in EFI/EDK2: when you get the 'config
> changed -rebooting' message from the boot firmware, it typically means that this
> memory type table has changed, and a reboot is necessary.)
>
> So the platform init needs to read this variable, or get the information in a
> different way. I assume it is the payload, not the platform init that updates the
> variable when necessary. This means the information flows from payload(n) to
> platform init(n+1), where n is a monotonic index tracking consecutive boots of the
> system.
>
> Can you explain how the DT fits into this? How are the runtime-code and
> runtime-data memory reservation nodes under /reserved-memory used to
> implement this information exchange between platform init and payload? And
> how do the HOB and the EFI variable fit into this picture?
1. With some offline discussion, we would move gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node. This is because it is edk2 implementation choice and non-edk2 PlatformInit or Payload may not have such memory optimization implementation. (not a generic usage/requirement for PlatformInit and Payload)
The edk2 example flow will be like below:
PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid and create Hob->
PlatformInit to initialize FDT->upl-custom node to report gEfiMemoryTypeInformationGuid HOB information ->
UefiPayload entry to re-create gEfiMemoryTypeInformationGuid HOB basing on FDT input (instead of the default MemoryType inside UefiPayload) ->
UefiPayload DxeMain/Gcd will consume gEfiMemoryTypeInformationGuid Hob for memory type information ->
UefiPayload to initialize UEFI environment (mainly DXE dispatcher) ->
(additional FV binary appended to common UefiPayload binary) PlatformPayload to provide VariableService which is platform specific ->
UefiPayload UefiBootManager will SetVariable if memory type change needed and request a warm reset ->
Back to PlatformInit ...
2. Now the proposed reserved-memory node usages will be for PlatformInit to provide data which may be used by Payload or OS. This is not edk2 specific and any PlatformInit/Payload could have same support.
Note: all of below are optional and PlatformInit may choose to implement some of them or not.
- acpi
If PlatformInit created some ACPI tables, this will report a memory region which contains all the tables to Payload and Payload may base on this to add some more tables if required.
- acpi-nvs
If PlatformInit has created some ACPI tables which having ACPI NVS memory dependency, this will be that nvs region.
- boot-code
When PlatformInit having some FW boot phase code that could be freed for OS to use when payload transferring control to UEFI OS
- boot-data
When PlatformInit having some FW boot phase data that could be freed for OS to use when payload transferring control to UEFI OS.
- runtime-code
PlatformInit may provide some services code that can be used for Payload to initialize UEFI Runtime Services for supporting UEFI OS.
- runtime-data
PlatformInit may provide some services data that can be used for Payload to Initialize UEFI Runtime Services for supporting UEFI OS.
On Thu, 21 Dec 2023 at 17:50, Chiu, Chasel <[email protected]> wrote:
>
>
> Hi Ard,
>
> Please see my reply below inline and let me know your thoughts.
>
> Thanks,
> Chasel
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel <[email protected]>
> > Sent: Thursday, December 21, 2023 6:31 AM
> > To: Chiu, Chasel <[email protected]>
> > Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> > <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> > <[email protected]>; lkml <[email protected]>; Dhaval
> > Sharma <[email protected]>; Brune, Maximilian
> > <[email protected]>; Yunhui Cui <[email protected]>;
> > Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > [email protected]; U-Boot Mailing List <[email protected]>
> > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > usages
> >
> > On Tue, 28 Nov 2023 at 21:31, Chiu, Chasel <[email protected]> wrote:
> > >
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <[email protected]>
> > > > Sent: Tuesday, November 28, 2023 10:08 AM
> > > > To: Chiu, Chasel <[email protected]>
> > > > Cc: Simon Glass <[email protected]>; [email protected]; Mark
> > > > Rutland <[email protected]>; Rob Herring <[email protected]>; Tan,
> > > > Lean Sheng <[email protected]>; lkml
> > > > <[email protected]>; Dhaval Sharma <[email protected]>;
> > > > Brune, Maximilian <[email protected]>; Yunhui Cui
> > > > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > > > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > > > <[email protected]>; linux- [email protected]; U-Boot Mailing
> > > > List <[email protected]>
> > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > > usages
> > > >
> > > > You are referring to a 2000 line patch so it is not 100% clear where to look tbh.
> > > >
> > > >
> > > > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <[email protected]> wrote:
> > > > >
> > > > >
> > > > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line
> > > > > 268 is for
> > > > related example code.
> > > > >
> > > >
> > > > That refers to a 'memory-allocation' node, right? How does that
> > > > relate to the 'reserved-memory' node?
> > > >
> > > > And crucially, how does this clarify in which way "runtime-code" and
> > > > "runtime- data" reservations are being used?
> > > >
> > > > Since the very beginning of this discussion, I have been asking
> > > > repeatedly for examples that describe the wider context in which these
> > reservations are used.
> > > > The "runtime" into runtime-code and runtime-data means that these
> > > > regions have a special significance to the operating system, not
> > > > just to the next bootloader stage. So I want to understand exactly
> > > > why it is necessary to describe these regions in a way where the
> > > > operating system might be expected to interpret this information and act
> > upon it.
> > > >
> > >
> > >
> > > I think runtime code and data today are mainly for supporting UEFI runtime
> > services - some BIOS functions for OS to utilize, OS may follow below ACPI spec to
> > treat them as reserved range:
> > > https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.html#
> > > uefi-memory-types-and-mapping-to-acpi-address-range-types
> > >
> > > Like I mentioned earlier, that PR is still in early phase and has not reflected all
> > the required changes yet, but the idea is to build
> > gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes.
> > > UEFI generic Payload has DxeMain integrated, however Memory Types are
> > platform-specific, for example, some platforms may need bigger runtime memory
> > for their implementation, that's why we want such FDT reserved-memory node to
> > tell DxeMain.
> > >
> >
> > > The Payload flow will be like this:
> > > Payload creates built-in default MemoryTypes table ->
> > > FDT reserved-memory node to override if required (this also ensures the
> > same memory map cross boots so ACPI S4 works) ->
> > > Build gEfiMemoryTypeInformationGuid HOB by "platfom specific"
> > MemoryTypes Table ->
> > > DxeMain/GCD to consume this MemoryTypes table and setup memory
> > service ->
> > > Install memory types table to UEFI system table.Configuration table...
> > >
> > > Note: if Payload built-in default MemoryTypes table works fine for the
> > > platform, then FDT reserved-memory node does not need to provide such
> > 'usage' compatible strings. (optional) This FDT node could allow
> > flexibility/compatibility without rebuilding Payload binary.
> > >
> > > Not sure if I answered all your questions, please highlight which area you need
> > more information.
> > >
> >
> > The gEfiMemoryTypeInformationGuid HOB typically carries platform defaults, and
> > the actual memory type information is kept in a non-volatile EFI variable, which
> > gets updated when the memory usage changes. Is this different for
> > UefiPayloadPkg?
> >
> > (For those among the cc'ees less versed in EFI/EDK2: when you get the 'config
> > changed -rebooting' message from the boot firmware, it typically means that this
> > memory type table has changed, and a reboot is necessary.)
> >
> > So the platform init needs to read this variable, or get the information in a
> > different way. I assume it is the payload, not the platform init that updates the
> > variable when necessary. This means the information flows from payload(n) to
> > platform init(n+1), where n is a monotonic index tracking consecutive boots of the
> > system.
> >
> > Can you explain how the DT fits into this? How are the runtime-code and
> > runtime-data memory reservation nodes under /reserved-memory used to
> > implement this information exchange between platform init and payload? And
> > how do the HOB and the EFI variable fit into this picture?
>
>
> 1. With some offline discussion, we would move gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node. This is because it is edk2 implementation choice and non-edk2 PlatformInit or Payload may not have such memory optimization implementation. (not a generic usage/requirement for PlatformInit and Payload)
>
> The edk2 example flow will be like below:
>
> PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid and create Hob->
> PlatformInit to initialize FDT->upl-custom node to report gEfiMemoryTypeInformationGuid HOB information ->
> UefiPayload entry to re-create gEfiMemoryTypeInformationGuid HOB basing on FDT input (instead of the default MemoryType inside UefiPayload) ->
> UefiPayload DxeMain/Gcd will consume gEfiMemoryTypeInformationGuid Hob for memory type information ->
> UefiPayload to initialize UEFI environment (mainly DXE dispatcher) ->
> (additional FV binary appended to common UefiPayload binary) PlatformPayload to provide VariableService which is platform specific ->
> UefiPayload UefiBootManager will SetVariable if memory type change needed and request a warm reset ->
> Back to PlatformInit ...
>
OK so the upl-custom node can do whatever it needs to. I imagine these
will include the memory descriptor attribute field, and other parts
that may be missing from the /reserved-memory DT node specification?
>
> 2. Now the proposed reserved-memory node usages will be for PlatformInit to provide data which may be used by Payload or OS. This is not edk2 specific and any PlatformInit/Payload could have same support.
> Note: all of below are optional and PlatformInit may choose to implement some of them or not.
>
> - acpi
> If PlatformInit created some ACPI tables, this will report a memory region which contains all the tables to Payload and Payload may base on this to add some more tables if required.
>
> - acpi-nvs
> If PlatformInit has created some ACPI tables which having ACPI NVS memory dependency, this will be that nvs region.
>
These make sense.
> - boot-code
> When PlatformInit having some FW boot phase code that could be freed for OS to use when payload transferring control to UEFI OS
>
> - boot-data
> When PlatformInit having some FW boot phase data that could be freed for OS to use when payload transferring control to UEFI OS.
>
> - runtime-code
> PlatformInit may provide some services code that can be used for Payload to initialize UEFI Runtime Services for supporting UEFI OS.
>
> - runtime-data
> PlatformInit may provide some services data that can be used for Payload to Initialize UEFI Runtime Services for supporting UEFI OS.
>
A UEFI OS must consume this information from the UEFI memory map, not
from the /reserved-memory nodes. So these nodes must either not be
visible to the OS at all, or carry an annotation that the OS must
ignore them.
Would it be possible to include a restriction in the DT schema that
these are only valid in the firmware boot phase?
Please see my reply below inline.
Thanks,
Chasel
> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Friday, December 22, 2023 4:48 AM
> To: Chiu, Chasel <[email protected]>
> Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> <[email protected]>; lkml <[email protected]>; Dhaval
> Sharma <[email protected]>; Brune, Maximilian
> <[email protected]>; Yunhui Cui <[email protected]>;
> Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> <[email protected]>; Guo, Gua <[email protected]>; linux-
> [email protected]; U-Boot Mailing List <[email protected]>
> Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> usages
>
> On Thu, 21 Dec 2023 at 17:50, Chiu, Chasel <[email protected]> wrote:
> >
> >
> > Hi Ard,
> >
> > Please see my reply below inline and let me know your thoughts.
> >
> > Thanks,
> > Chasel
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <[email protected]>
> > > Sent: Thursday, December 21, 2023 6:31 AM
> > > To: Chiu, Chasel <[email protected]>
> > > Cc: Simon Glass <[email protected]>; [email protected]; Mark
> > > Rutland <[email protected]>; Rob Herring <[email protected]>; Tan,
> > > Lean Sheng <[email protected]>; lkml
> > > <[email protected]>; Dhaval Sharma <[email protected]>;
> > > Brune, Maximilian <[email protected]>; Yunhui Cui
> > > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > > <[email protected]>; linux- [email protected]; U-Boot Mailing
> > > List <[email protected]>
> > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > usages
> > >
> > > On Tue, 28 Nov 2023 at 21:31, Chiu, Chasel <[email protected]> wrote:
> > > >
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel <[email protected]>
> > > > > Sent: Tuesday, November 28, 2023 10:08 AM
> > > > > To: Chiu, Chasel <[email protected]>
> > > > > Cc: Simon Glass <[email protected]>; [email protected];
> > > > > Mark Rutland <[email protected]>; Rob Herring
> > > > > <[email protected]>; Tan, Lean Sheng <[email protected]>;
> > > > > lkml <[email protected]>; Dhaval Sharma
> > > > > <[email protected]>; Brune, Maximilian
> > > > > <[email protected]>; Yunhui Cui
> > > > > <[email protected]>; Dong, Guo <[email protected]>; Tom
> > > > > Rini <[email protected]>; ron minnich <[email protected]>;
> > > > > Guo, Gua <[email protected]>; linux- [email protected];
> > > > > U-Boot Mailing List <[email protected]>
> > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common
> > > > > reserved-memory usages
> > > > >
> > > > > You are referring to a 2000 line patch so it is not 100% clear where to look
> tbh.
> > > > >
> > > > >
> > > > > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <[email protected]>
> wrote:
> > > > > >
> > > > > >
> > > > > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c,
> > > > > > line
> > > > > > 268 is for
> > > > > related example code.
> > > > > >
> > > > >
> > > > > That refers to a 'memory-allocation' node, right? How does that
> > > > > relate to the 'reserved-memory' node?
> > > > >
> > > > > And crucially, how does this clarify in which way "runtime-code"
> > > > > and
> > > > > "runtime- data" reservations are being used?
> > > > >
> > > > > Since the very beginning of this discussion, I have been asking
> > > > > repeatedly for examples that describe the wider context in which
> > > > > these
> > > reservations are used.
> > > > > The "runtime" into runtime-code and runtime-data means that
> > > > > these regions have a special significance to the operating
> > > > > system, not just to the next bootloader stage. So I want to
> > > > > understand exactly why it is necessary to describe these regions
> > > > > in a way where the operating system might be expected to
> > > > > interpret this information and act
> > > upon it.
> > > > >
> > > >
> > > >
> > > > I think runtime code and data today are mainly for supporting UEFI
> > > > runtime
> > > services - some BIOS functions for OS to utilize, OS may follow
> > > below ACPI spec to treat them as reserved range:
> > > > https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.h
> > > > tml# uefi-memory-types-and-mapping-to-acpi-address-range-types
> > > >
> > > > Like I mentioned earlier, that PR is still in early phase and has
> > > > not reflected all
> > > the required changes yet, but the idea is to build
> > > gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes.
> > > > UEFI generic Payload has DxeMain integrated, however Memory Types
> > > > are
> > > platform-specific, for example, some platforms may need bigger
> > > runtime memory for their implementation, that's why we want such FDT
> > > reserved-memory node to tell DxeMain.
> > > >
> > >
> > > > The Payload flow will be like this:
> > > > Payload creates built-in default MemoryTypes table ->
> > > > FDT reserved-memory node to override if required (this also
> > > > ensures the
> > > same memory map cross boots so ACPI S4 works) ->
> > > > Build gEfiMemoryTypeInformationGuid HOB by "platfom specific"
> > > MemoryTypes Table ->
> > > > DxeMain/GCD to consume this MemoryTypes table and setup
> > > > memory
> > > service ->
> > > > Install memory types table to UEFI system table.Configuration table...
> > > >
> > > > Note: if Payload built-in default MemoryTypes table works fine for
> > > > the platform, then FDT reserved-memory node does not need to
> > > > provide such
> > > 'usage' compatible strings. (optional) This FDT node could allow
> > > flexibility/compatibility without rebuilding Payload binary.
> > > >
> > > > Not sure if I answered all your questions, please highlight which
> > > > area you need
> > > more information.
> > > >
> > >
> > > The gEfiMemoryTypeInformationGuid HOB typically carries platform
> > > defaults, and the actual memory type information is kept in a
> > > non-volatile EFI variable, which gets updated when the memory usage
> > > changes. Is this different for UefiPayloadPkg?
> > >
> > > (For those among the cc'ees less versed in EFI/EDK2: when you get
> > > the 'config changed -rebooting' message from the boot firmware, it
> > > typically means that this memory type table has changed, and a
> > > reboot is necessary.)
> > >
> > > So the platform init needs to read this variable, or get the
> > > information in a different way. I assume it is the payload, not the
> > > platform init that updates the variable when necessary. This means
> > > the information flows from payload(n) to platform init(n+1), where n
> > > is a monotonic index tracking consecutive boots of the system.
> > >
> > > Can you explain how the DT fits into this? How are the runtime-code
> > > and runtime-data memory reservation nodes under /reserved-memory
> > > used to implement this information exchange between platform init
> > > and payload? And how do the HOB and the EFI variable fit into this picture?
> >
> >
> > 1. With some offline discussion, we would move
> > gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node. This is
> > because it is edk2 implementation choice and non-edk2 PlatformInit or
> > Payload may not have such memory optimization implementation. (not a
> > generic usage/requirement for PlatformInit and Payload)
> >
> > The edk2 example flow will be like below:
> >
> > PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid and create Hob-
> >
> > PlatformInit to initialize FDT->upl-custom node to report
> gEfiMemoryTypeInformationGuid HOB information ->
> > UefiPayload entry to re-create gEfiMemoryTypeInformationGuid HOB basing
> on FDT input (instead of the default MemoryType inside UefiPayload) ->
> > UefiPayload DxeMain/Gcd will consume gEfiMemoryTypeInformationGuid
> Hob for memory type information ->
> > UefiPayload to initialize UEFI environment (mainly DXE dispatcher) ->
> > (additional FV binary appended to common UefiPayload binary)
> PlatformPayload to provide VariableService which is platform specific ->
> > UefiPayload UefiBootManager will SetVariable if memory type change
> needed and request a warm reset ->
> > Back to PlatformInit ...
> >
>
> OK so the upl-custom node can do whatever it needs to. I imagine these will
> include the memory descriptor attribute field, and other parts that may be missing
> from the /reserved-memory DT node specification?
Yes, if needed by edk2 specific implementation, not generic enough, we may consider to use upl-custom node to pass those data.
>
> >
> > 2. Now the proposed reserved-memory node usages will be for PlatformInit to
> provide data which may be used by Payload or OS. This is not edk2 specific and
> any PlatformInit/Payload could have same support.
> > Note: all of below are optional and PlatformInit may choose to implement some
> of them or not.
> >
> > - acpi
> > If PlatformInit created some ACPI tables, this will report a memory region which
> contains all the tables to Payload and Payload may base on this to add some more
> tables if required.
> >
> > - acpi-nvs
> > If PlatformInit has created some ACPI tables which having ACPI NVS memory
> dependency, this will be that nvs region.
> >
>
> These make sense.
>
> > - boot-code
> > When PlatformInit having some FW boot phase code that could be freed
> > for OS to use when payload transferring control to UEFI OS
> >
> > - boot-data
> > When PlatformInit having some FW boot phase data that could be freed for OS
> to use when payload transferring control to UEFI OS.
> >
> > - runtime-code
> > PlatformInit may provide some services code that can be used for Payload to
> initialize UEFI Runtime Services for supporting UEFI OS.
> >
> > - runtime-data
> > PlatformInit may provide some services data that can be used for Payload to
> Initialize UEFI Runtime Services for supporting UEFI OS.
> >
>
> A UEFI OS must consume this information from the UEFI memory map, not from
> the /reserved-memory nodes. So these nodes must either not be visible to the OS
> at all, or carry an annotation that the OS must ignore them.
>
> Would it be possible to include a restriction in the DT schema that these are only
> valid in the firmware boot phase?
https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#efi-boot-services-exitbootservices
Per UEFI specification, UEFI OS will always call UEFI GetMemoryMap function to retrieve memory map, so FDT node present or not does not matter to UEFI OS. We probably could have annotation in UPL specification to emphasize this.
I'm not familiar with Linux FDT boot, but if non-UEFI OS does not call UEFI GetMemoryMap() and does not know what is runtime-code/data, boot-code/data, it might just treat such reserved-memory nodes as 'regular' reserved memory nodes, and that's still ok because non-UEFI OS will not call to any runtime service or re-purpose boot-code/data memory regions.
Would you provide a real OS case which will be impacted by this reserved-memory schema so we can discuss basing on real case?
On Fri, 22 Dec 2023 at 20:52, Chiu, Chasel <[email protected]> wrote:
>
>
> Please see my reply below inline.
>
> Thanks,
> Chasel
>
...
> > > > The gEfiMemoryTypeInformationGuid HOB typically carries platform
> > > > defaults, and the actual memory type information is kept in a
> > > > non-volatile EFI variable, which gets updated when the memory usage
> > > > changes. Is this different for UefiPayloadPkg?
> > > >
> > > > (For those among the cc'ees less versed in EFI/EDK2: when you get
> > > > the 'config changed -rebooting' message from the boot firmware, it
> > > > typically means that this memory type table has changed, and a
> > > > reboot is necessary.)
> > > >
> > > > So the platform init needs to read this variable, or get the
> > > > information in a different way. I assume it is the payload, not the
> > > > platform init that updates the variable when necessary. This means
> > > > the information flows from payload(n) to platform init(n+1), where n
> > > > is a monotonic index tracking consecutive boots of the system.
> > > >
> > > > Can you explain how the DT fits into this? How are the runtime-code
> > > > and runtime-data memory reservation nodes under /reserved-memory
> > > > used to implement this information exchange between platform init
> > > > and payload? And how do the HOB and the EFI variable fit into this picture?
> > >
> > >
> > > 1. With some offline discussion, we would move
> > > gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node. This is
> > > because it is edk2 implementation choice and non-edk2 PlatformInit or
> > > Payload may not have such memory optimization implementation. (not a
> > > generic usage/requirement for PlatformInit and Payload)
> > >
> > > The edk2 example flow will be like below:
> > >
> > > PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid and create Hob-
> > >
> > > PlatformInit to initialize FDT->upl-custom node to report
> > gEfiMemoryTypeInformationGuid HOB information ->
> > > UefiPayload entry to re-create gEfiMemoryTypeInformationGuid HOB basing
> > on FDT input (instead of the default MemoryType inside UefiPayload) ->
> > > UefiPayload DxeMain/Gcd will consume gEfiMemoryTypeInformationGuid
> > Hob for memory type information ->
> > > UefiPayload to initialize UEFI environment (mainly DXE dispatcher) ->
> > > (additional FV binary appended to common UefiPayload binary)
> > PlatformPayload to provide VariableService which is platform specific ->
> > > UefiPayload UefiBootManager will SetVariable if memory type change
> > needed and request a warm reset ->
> > > Back to PlatformInit ...
> > >
> >
> > OK so the upl-custom node can do whatever it needs to. I imagine these will
> > include the memory descriptor attribute field, and other parts that may be missing
> > from the /reserved-memory DT node specification?
>
>
> Yes, if needed by edk2 specific implementation, not generic enough, we may consider to use upl-custom node to pass those data.
>
>
> >
> > >
> > > 2. Now the proposed reserved-memory node usages will be for PlatformInit to
> > provide data which may be used by Payload or OS. This is not edk2 specific and
> > any PlatformInit/Payload could have same support.
> > > Note: all of below are optional and PlatformInit may choose to implement some
> > of them or not.
> > >
> > > - acpi
> > > If PlatformInit created some ACPI tables, this will report a memory region which
> > contains all the tables to Payload and Payload may base on this to add some more
> > tables if required.
> > >
> > > - acpi-nvs
> > > If PlatformInit has created some ACPI tables which having ACPI NVS memory
> > dependency, this will be that nvs region.
> > >
> >
> > These make sense.
> >
> > > - boot-code
> > > When PlatformInit having some FW boot phase code that could be freed
> > > for OS to use when payload transferring control to UEFI OS
> > >
> > > - boot-data
> > > When PlatformInit having some FW boot phase data that could be freed for OS
> > to use when payload transferring control to UEFI OS.
> > >
> > > - runtime-code
> > > PlatformInit may provide some services code that can be used for Payload to
> > initialize UEFI Runtime Services for supporting UEFI OS.
> > >
> > > - runtime-data
> > > PlatformInit may provide some services data that can be used for Payload to
> > Initialize UEFI Runtime Services for supporting UEFI OS.
> > >
> >
> > A UEFI OS must consume this information from the UEFI memory map, not from
> > the /reserved-memory nodes. So these nodes must either not be visible to the OS
> > at all, or carry an annotation that the OS must ignore them.
> >
> > Would it be possible to include a restriction in the DT schema that these are only
> > valid in the firmware boot phase?
>
>
> https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#efi-boot-services-exitbootservices
> Per UEFI specification, UEFI OS will always call UEFI GetMemoryMap function to retrieve memory map, so FDT node present or not does not matter to UEFI OS. We probably could have annotation in UPL specification to emphasize this.
> I'm not familiar with Linux FDT boot, but if non-UEFI OS does not call UEFI GetMemoryMap() and does not know what is runtime-code/data, boot-code/data, it might just treat such reserved-memory nodes as 'regular' reserved memory nodes, and that's still ok because non-UEFI OS will not call to any runtime service or re-purpose boot-code/data memory regions.
>
You are saying the same thing but in a different way. A UEFI OS must
only rely on GetMemoryMap(), and not on the /reserved-memory node to
obtain this information. But this requirement needs to be stated
somewhere: the UEFI spec does not reason about other sources of EFI
memory information at all, and this DT schema does not mention any of
this either.
> Would you provide a real OS case which will be impacted by this reserved-memory schema so we can discuss basing on real case?
>
Funny, that is what I have been trying to get from you :-)
The problem I am anticipating here is that the information in
/reserved-memory may be out of sync with the EFI memory map. It needs
to be made clear that the EFI memory map is the only source of truth
when the OS is involved, and this /reserved-memory mechanism should
only be used by other firmware stages. But the schema does not mention
this at all. The schema also does not mention that the information in
/reserved-memory is not actually sufficient to reconstruct the EFI
memory map that the firmware payload expects (which is why the
upl-custom-node exists too)
On Fri, Dec 22, 2023 at 5:48 AM Ard Biesheuvel <[email protected]> wrote:
>
> On Thu, 21 Dec 2023 at 17:50, Chiu, Chasel <[email protected]> wrote:
> >
> >
> > Hi Ard,
> >
> > Please see my reply below inline and let me know your thoughts.
> >
> > Thanks,
> > Chasel
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <[email protected]>
> > > Sent: Thursday, December 21, 2023 6:31 AM
> > > To: Chiu, Chasel <[email protected]>
> > > Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> > > <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> > > <[email protected]>; lkml <[email protected]>; Dhaval
> > > Sharma <[email protected]>; Brune, Maximilian
> > > <[email protected]>; Yunhui Cui <[email protected]>;
> > > Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> > > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > > [email protected]; U-Boot Mailing List <[email protected]>
> > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > usages
> > >
> > > On Tue, 28 Nov 2023 at 21:31, Chiu, Chasel <[email protected]> wrote:
> > > >
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel <[email protected]>
> > > > > Sent: Tuesday, November 28, 2023 10:08 AM
> > > > > To: Chiu, Chasel <[email protected]>
> > > > > Cc: Simon Glass <[email protected]>; [email protected]; Mark
> > > > > Rutland <[email protected]>; Rob Herring <[email protected]>; Tan,
> > > > > Lean Sheng <[email protected]>; lkml
> > > > > <[email protected]>; Dhaval Sharma <[email protected]>;
> > > > > Brune, Maximilian <[email protected]>; Yunhui Cui
> > > > > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > > > > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > > > > <[email protected]>; linux- [email protected]; U-Boot Mailing
> > > > > List <[email protected]>
> > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > > > usages
> > > > >
> > > > > You are referring to a 2000 line patch so it is not 100% clear where to look tbh.
> > > > >
> > > > >
> > > > > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <[email protected]> wrote:
> > > > > >
> > > > > >
> > > > > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line
> > > > > > 268 is for
> > > > > related example code.
> > > > > >
> > > > >
> > > > > That refers to a 'memory-allocation' node, right? How does that
> > > > > relate to the 'reserved-memory' node?
> > > > >
> > > > > And crucially, how does this clarify in which way "runtime-code" and
> > > > > "runtime- data" reservations are being used?
> > > > >
> > > > > Since the very beginning of this discussion, I have been asking
> > > > > repeatedly for examples that describe the wider context in which these
> > > reservations are used.
> > > > > The "runtime" into runtime-code and runtime-data means that these
> > > > > regions have a special significance to the operating system, not
> > > > > just to the next bootloader stage. So I want to understand exactly
> > > > > why it is necessary to describe these regions in a way where the
> > > > > operating system might be expected to interpret this information and act
> > > upon it.
> > > > >
> > > >
> > > >
> > > > I think runtime code and data today are mainly for supporting UEFI runtime
> > > services - some BIOS functions for OS to utilize, OS may follow below ACPI spec to
> > > treat them as reserved range:
> > > > https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.html#
> > > > uefi-memory-types-and-mapping-to-acpi-address-range-types
> > > >
> > > > Like I mentioned earlier, that PR is still in early phase and has not reflected all
> > > the required changes yet, but the idea is to build
> > > gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes.
> > > > UEFI generic Payload has DxeMain integrated, however Memory Types are
> > > platform-specific, for example, some platforms may need bigger runtime memory
> > > for their implementation, that's why we want such FDT reserved-memory node to
> > > tell DxeMain.
> > > >
> > >
> > > > The Payload flow will be like this:
> > > > Payload creates built-in default MemoryTypes table ->
> > > > FDT reserved-memory node to override if required (this also ensures the
> > > same memory map cross boots so ACPI S4 works) ->
> > > > Build gEfiMemoryTypeInformationGuid HOB by "platfom specific"
> > > MemoryTypes Table ->
> > > > DxeMain/GCD to consume this MemoryTypes table and setup memory
> > > service ->
> > > > Install memory types table to UEFI system table.Configuration table...
> > > >
> > > > Note: if Payload built-in default MemoryTypes table works fine for the
> > > > platform, then FDT reserved-memory node does not need to provide such
> > > 'usage' compatible strings. (optional) This FDT node could allow
> > > flexibility/compatibility without rebuilding Payload binary.
> > > >
> > > > Not sure if I answered all your questions, please highlight which area you need
> > > more information.
> > > >
> > >
> > > The gEfiMemoryTypeInformationGuid HOB typically carries platform defaults, and
> > > the actual memory type information is kept in a non-volatile EFI variable, which
> > > gets updated when the memory usage changes. Is this different for
> > > UefiPayloadPkg?
> > >
> > > (For those among the cc'ees less versed in EFI/EDK2: when you get the 'config
> > > changed -rebooting' message from the boot firmware, it typically means that this
> > > memory type table has changed, and a reboot is necessary.)
> > >
> > > So the platform init needs to read this variable, or get the information in a
> > > different way. I assume it is the payload, not the platform init that updates the
> > > variable when necessary. This means the information flows from payload(n) to
> > > platform init(n+1), where n is a monotonic index tracking consecutive boots of the
> > > system.
> > >
> > > Can you explain how the DT fits into this? How are the runtime-code and
> > > runtime-data memory reservation nodes under /reserved-memory used to
> > > implement this information exchange between platform init and payload? And
> > > how do the HOB and the EFI variable fit into this picture?
> >
> >
> > 1. With some offline discussion, we would move gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node. This is because it is edk2 implementation choice and non-edk2 PlatformInit or Payload may not have such memory optimization implementation. (not a generic usage/requirement for PlatformInit and Payload)
> >
> > The edk2 example flow will be like below:
> >
> > PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid and create Hob->
> > PlatformInit to initialize FDT->upl-custom node to report gEfiMemoryTypeInformationGuid HOB information ->
> > UefiPayload entry to re-create gEfiMemoryTypeInformationGuid HOB basing on FDT input (instead of the default MemoryType inside UefiPayload) ->
> > UefiPayload DxeMain/Gcd will consume gEfiMemoryTypeInformationGuid Hob for memory type information ->
> > UefiPayload to initialize UEFI environment (mainly DXE dispatcher) ->
> > (additional FV binary appended to common UefiPayload binary) PlatformPayload to provide VariableService which is platform specific ->
> > UefiPayload UefiBootManager will SetVariable if memory type change needed and request a warm reset ->
> > Back to PlatformInit ...
> >
>
> OK so the upl-custom node can do whatever it needs to. I imagine these
> will include the memory descriptor attribute field, and other parts
> that may be missing from the /reserved-memory DT node specification?
>
> >
> > 2. Now the proposed reserved-memory node usages will be for PlatformInit to provide data which may be used by Payload or OS. This is not edk2 specific and any PlatformInit/Payload could have same support.
> > Note: all of below are optional and PlatformInit may choose to implement some of them or not.
> >
> > - acpi
> > If PlatformInit created some ACPI tables, this will report a memory region which contains all the tables to Payload and Payload may base on this to add some more tables if required.
> >
> > - acpi-nvs
> > If PlatformInit has created some ACPI tables which having ACPI NVS memory dependency, this will be that nvs region.
> >
>
> These make sense.
>
> > - boot-code
> > When PlatformInit having some FW boot phase code that could be freed for OS to use when payload transferring control to UEFI OS
> >
> > - boot-data
> > When PlatformInit having some FW boot phase data that could be freed for OS to use when payload transferring control to UEFI OS.
> >
> > - runtime-code
> > PlatformInit may provide some services code that can be used for Payload to initialize UEFI Runtime Services for supporting UEFI OS.
> >
> > - runtime-data
> > PlatformInit may provide some services data that can be used for Payload to Initialize UEFI Runtime Services for supporting UEFI OS.
> >
I'll say it again. "boot" and "runtime" on their own could mean about
anything, but the usage here is clearly tied to UEFI (or the EDK2
implementation) and its meaning of boot and runtime. So the naming
needs to reflect that.
> A UEFI OS must consume this information from the UEFI memory map, not
> from the /reserved-memory nodes. So these nodes must either not be
> visible to the OS at all, or carry an annotation that the OS must
> ignore them.
The kernel will process /reserved-memory for UEFI boot, so the
expectation is anything in the EFI memory map is not present there. An
annotation to ignore some nodes would require going back in time or
accepting 2 sources of truth on existing OS.
> Would it be possible to include a restriction in the DT schema that
> these are only valid in the firmware boot phase?
The only way ATM is including a schema or not when running validation
on a DT for a particular boot phase. Include the schema in the project
that wants to use these nodes and don't include it in cases that don't
use it. I don't see a reason why this needs to be in dtschema.
Rob
> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: Wednesday, January 3, 2024 8:01 AM
> To: Ard Biesheuvel <[email protected]>
> Cc: Chiu, Chasel <[email protected]>; Simon Glass <[email protected]>;
> [email protected]; Mark Rutland <[email protected]>; Tan, Lean
> Sheng <[email protected]>; lkml <[email protected]>;
> Dhaval Sharma <[email protected]>; Brune, Maximilian
> <[email protected]>; Yunhui Cui <[email protected]>;
> Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> <[email protected]>; Guo, Gua <[email protected]>; linux-
> [email protected]; U-Boot Mailing List <[email protected]>
> Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> usages
>
> On Fri, Dec 22, 2023 at 5:48 AM Ard Biesheuvel <[email protected]> wrote:
> >
> > On Thu, 21 Dec 2023 at 17:50, Chiu, Chasel <[email protected]> wrote:
> > >
> > >
> > > Hi Ard,
> > >
> > > Please see my reply below inline and let me know your thoughts.
> > >
> > > Thanks,
> > > Chasel
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <[email protected]>
> > > > Sent: Thursday, December 21, 2023 6:31 AM
> > > > To: Chiu, Chasel <[email protected]>
> > > > Cc: Simon Glass <[email protected]>; [email protected];
> > > > Mark Rutland <[email protected]>; Rob Herring
> > > > <[email protected]>; Tan, Lean Sheng <[email protected]>; lkml
> > > > <[email protected]>; Dhaval Sharma
> > > > <[email protected]>; Brune, Maximilian
> > > > <[email protected]>; Yunhui Cui
> > > > <[email protected]>; Dong, Guo <[email protected]>; Tom
> > > > Rini <[email protected]>; ron minnich <[email protected]>; Guo,
> > > > Gua <[email protected]>; linux- [email protected]; U-Boot
> > > > Mailing List <[email protected]>
> > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common
> > > > reserved-memory usages
> > > >
> > > > On Tue, 28 Nov 2023 at 21:31, Chiu, Chasel <[email protected]> wrote:
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ard Biesheuvel <[email protected]>
> > > > > > Sent: Tuesday, November 28, 2023 10:08 AM
> > > > > > To: Chiu, Chasel <[email protected]>
> > > > > > Cc: Simon Glass <[email protected]>;
> > > > > > [email protected]; Mark Rutland
> > > > > > <[email protected]>; Rob Herring <[email protected]>; Tan,
> > > > > > Lean Sheng <[email protected]>; lkml
> > > > > > <[email protected]>; Dhaval Sharma
> > > > > > <[email protected]>; Brune, Maximilian
> > > > > > <[email protected]>; Yunhui Cui
> > > > > > <[email protected]>; Dong, Guo <[email protected]>; Tom
> > > > > > Rini <[email protected]>; ron minnich <[email protected]>;
> > > > > > Guo, Gua <[email protected]>; linux- [email protected];
> > > > > > U-Boot Mailing List <[email protected]>
> > > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common
> > > > > > reserved-memory usages
> > > > > >
> > > > > > You are referring to a 2000 line patch so it is not 100% clear where to
> look tbh.
> > > > > >
> > > > > >
> > > > > > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <[email protected]>
> wrote:
> > > > > > >
> > > > > > >
> > > > > > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c,
> > > > > > > line
> > > > > > > 268 is for
> > > > > > related example code.
> > > > > > >
> > > > > >
> > > > > > That refers to a 'memory-allocation' node, right? How does
> > > > > > that relate to the 'reserved-memory' node?
> > > > > >
> > > > > > And crucially, how does this clarify in which way
> > > > > > "runtime-code" and
> > > > > > "runtime- data" reservations are being used?
> > > > > >
> > > > > > Since the very beginning of this discussion, I have been
> > > > > > asking repeatedly for examples that describe the wider context
> > > > > > in which these
> > > > reservations are used.
> > > > > > The "runtime" into runtime-code and runtime-data means that
> > > > > > these regions have a special significance to the operating
> > > > > > system, not just to the next bootloader stage. So I want to
> > > > > > understand exactly why it is necessary to describe these
> > > > > > regions in a way where the operating system might be expected
> > > > > > to interpret this information and act
> > > > upon it.
> > > > > >
> > > > >
> > > > >
> > > > > I think runtime code and data today are mainly for supporting
> > > > > UEFI runtime
> > > > services - some BIOS functions for OS to utilize, OS may follow
> > > > below ACPI spec to treat them as reserved range:
> > > > > https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces
> > > > > .html# uefi-memory-types-and-mapping-to-acpi-address-range-types
> > > > >
> > > > > Like I mentioned earlier, that PR is still in early phase and
> > > > > has not reflected all
> > > > the required changes yet, but the idea is to build
> > > > gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes.
> > > > > UEFI generic Payload has DxeMain integrated, however Memory
> > > > > Types are
> > > > platform-specific, for example, some platforms may need bigger
> > > > runtime memory for their implementation, that's why we want such
> > > > FDT reserved-memory node to tell DxeMain.
> > > > >
> > > >
> > > > > The Payload flow will be like this:
> > > > > Payload creates built-in default MemoryTypes table ->
> > > > > FDT reserved-memory node to override if required (this also
> > > > > ensures the
> > > > same memory map cross boots so ACPI S4 works) ->
> > > > > Build gEfiMemoryTypeInformationGuid HOB by "platfom specific"
> > > > MemoryTypes Table ->
> > > > > DxeMain/GCD to consume this MemoryTypes table and setup
> > > > > memory
> > > > service ->
> > > > > Install memory types table to UEFI system table.Configuration
> table...
> > > > >
> > > > > Note: if Payload built-in default MemoryTypes table works fine
> > > > > for the platform, then FDT reserved-memory node does not need to
> > > > > provide such
> > > > 'usage' compatible strings. (optional) This FDT node could allow
> > > > flexibility/compatibility without rebuilding Payload binary.
> > > > >
> > > > > Not sure if I answered all your questions, please highlight
> > > > > which area you need
> > > > more information.
> > > > >
> > > >
> > > > The gEfiMemoryTypeInformationGuid HOB typically carries platform
> > > > defaults, and the actual memory type information is kept in a
> > > > non-volatile EFI variable, which gets updated when the memory
> > > > usage changes. Is this different for UefiPayloadPkg?
> > > >
> > > > (For those among the cc'ees less versed in EFI/EDK2: when you get
> > > > the 'config changed -rebooting' message from the boot firmware, it
> > > > typically means that this memory type table has changed, and a
> > > > reboot is necessary.)
> > > >
> > > > So the platform init needs to read this variable, or get the
> > > > information in a different way. I assume it is the payload, not
> > > > the platform init that updates the variable when necessary. This
> > > > means the information flows from payload(n) to platform init(n+1),
> > > > where n is a monotonic index tracking consecutive boots of the system.
> > > >
> > > > Can you explain how the DT fits into this? How are the
> > > > runtime-code and runtime-data memory reservation nodes under
> > > > /reserved-memory used to implement this information exchange
> > > > between platform init and payload? And how do the HOB and the EFI
> variable fit into this picture?
> > >
> > >
> > > 1. With some offline discussion, we would move
> > > gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node. This is
> > > because it is edk2 implementation choice and non-edk2 PlatformInit
> > > or Payload may not have such memory optimization implementation.
> > > (not a generic usage/requirement for PlatformInit and Payload)
> > >
> > > The edk2 example flow will be like below:
> > >
> > > PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid and create
> Hob->
> > > PlatformInit to initialize FDT->upl-custom node to report
> gEfiMemoryTypeInformationGuid HOB information ->
> > > UefiPayload entry to re-create gEfiMemoryTypeInformationGuid HOB
> basing on FDT input (instead of the default MemoryType inside UefiPayload) ->
> > > UefiPayload DxeMain/Gcd will consume gEfiMemoryTypeInformationGuid
> Hob for memory type information ->
> > > UefiPayload to initialize UEFI environment (mainly DXE dispatcher) ->
> > > (additional FV binary appended to common UefiPayload binary)
> PlatformPayload to provide VariableService which is platform specific ->
> > > UefiPayload UefiBootManager will SetVariable if memory type change
> needed and request a warm reset ->
> > > Back to PlatformInit ...
> > >
> >
> > OK so the upl-custom node can do whatever it needs to. I imagine these
> > will include the memory descriptor attribute field, and other parts
> > that may be missing from the /reserved-memory DT node specification?
> >
> > >
> > > 2. Now the proposed reserved-memory node usages will be for PlatformInit to
> provide data which may be used by Payload or OS. This is not edk2 specific and
> any PlatformInit/Payload could have same support.
> > > Note: all of below are optional and PlatformInit may choose to implement
> some of them or not.
> > >
> > > - acpi
> > > If PlatformInit created some ACPI tables, this will report a memory region
> which contains all the tables to Payload and Payload may base on this to add
> some more tables if required.
> > >
> > > - acpi-nvs
> > > If PlatformInit has created some ACPI tables which having ACPI NVS memory
> dependency, this will be that nvs region.
> > >
> >
> > These make sense.
> >
> > > - boot-code
> > > When PlatformInit having some FW boot phase code that could be freed
> > > for OS to use when payload transferring control to UEFI OS
> > >
> > > - boot-data
> > > When PlatformInit having some FW boot phase data that could be freed for OS
> to use when payload transferring control to UEFI OS.
> > >
> > > - runtime-code
> > > PlatformInit may provide some services code that can be used for Payload to
> initialize UEFI Runtime Services for supporting UEFI OS.
> > >
> > > - runtime-data
> > > PlatformInit may provide some services data that can be used for Payload to
> Initialize UEFI Runtime Services for supporting UEFI OS.
> > >
>
> I'll say it again. "boot" and "runtime" on their own could mean about anything,
> but the usage here is clearly tied to UEFI (or the EDK2
> implementation) and its meaning of boot and runtime. So the naming needs to
> reflect that.
>
How about renaming to below?
uefiboot-code
uefiboot-data
uefiruntime-code
uefiruntime-data
>
> > A UEFI OS must consume this information from the UEFI memory map, not
> > from the /reserved-memory nodes. So these nodes must either not be
> > visible to the OS at all, or carry an annotation that the OS must
> > ignore them.
>
> The kernel will process /reserved-memory for UEFI boot, so the expectation is
> anything in the EFI memory map is not present there. An annotation to ignore
> some nodes would require going back in time or accepting 2 sources of truth on
> existing OS.
>
I thought UEFI boot will rely on UEFI GetMemoryMap() for knowing current memory map, and the DT memory/reserved-memory nodes should have been included/converted by GetMemoryMap() function.
How kernel handling DT nodes vs GetMemoryMap() in UEFI boot case?
> > Would it be possible to include a restriction in the DT schema that
> > these are only valid in the firmware boot phase?
>
> The only way ATM is including a schema or not when running validation on a DT
> for a particular boot phase. Include the schema in the project that wants to use
> these nodes and don't include it in cases that don't use it. I don't see a reason
> why this needs to be in dtschema.
>
> Rob
Hi Rob, Ard,
On Wed, Jan 3, 2024 at 9:01 AM Rob Herring <[email protected]> wrote:
>
> On Fri, Dec 22, 2023 at 5:48 AM Ard Biesheuvel <[email protected]> wrote:
> >
> > On Thu, 21 Dec 2023 at 17:50, Chiu, Chasel <[email protected]> wrote:
> > >
> > >
> > > Hi Ard,
> > >
> > > Please see my reply below inline and let me know your thoughts.
> > >
> > > Thanks,
> > > Chasel
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <[email protected]>
> > > > Sent: Thursday, December 21, 2023 6:31 AM
> > > > To: Chiu, Chasel <[email protected]>
> > > > Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> > > > <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> > > > <[email protected]>; lkml <[email protected]>; Dhaval
> > > > Sharma <[email protected]>; Brune, Maximilian
> > > > <[email protected]>; Yunhui Cui <[email protected]>;
> > > > Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> > > > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > > > [email protected]; U-Boot Mailing List <[email protected]>
> > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > > usages
> > > >
> > > > On Tue, 28 Nov 2023 at 21:31, Chiu, Chasel <[email protected]> wrote:
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ard Biesheuvel <[email protected]>
> > > > > > Sent: Tuesday, November 28, 2023 10:08 AM
> > > > > > To: Chiu, Chasel <[email protected]>
> > > > > > Cc: Simon Glass <[email protected]>; [email protected]; Mark
> > > > > > Rutland <[email protected]>; Rob Herring <[email protected]>; Tan,
> > > > > > Lean Sheng <[email protected]>; lkml
> > > > > > <[email protected]>; Dhaval Sharma <[email protected]>;
> > > > > > Brune, Maximilian <[email protected]>; Yunhui Cui
> > > > > > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > > > > > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > > > > > <[email protected]>; linux- [email protected]; U-Boot Mailing
> > > > > > List <[email protected]>
> > > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > > > > usages
> > > > > >
> > > > > > You are referring to a 2000 line patch so it is not 100% clear where to look tbh.
> > > > > >
> > > > > >
> > > > > > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <[email protected]> wrote:
> > > > > > >
> > > > > > >
> > > > > > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line
> > > > > > > 268 is for
> > > > > > related example code.
> > > > > > >
> > > > > >
> > > > > > That refers to a 'memory-allocation' node, right? How does that
> > > > > > relate to the 'reserved-memory' node?
> > > > > >
> > > > > > And crucially, how does this clarify in which way "runtime-code" and
> > > > > > "runtime- data" reservations are being used?
> > > > > >
> > > > > > Since the very beginning of this discussion, I have been asking
> > > > > > repeatedly for examples that describe the wider context in which these
> > > > reservations are used.
> > > > > > The "runtime" into runtime-code and runtime-data means that these
> > > > > > regions have a special significance to the operating system, not
> > > > > > just to the next bootloader stage. So I want to understand exactly
> > > > > > why it is necessary to describe these regions in a way where the
> > > > > > operating system might be expected to interpret this information and act
> > > > upon it.
> > > > > >
> > > > >
> > > > >
> > > > > I think runtime code and data today are mainly for supporting UEFI runtime
> > > > services - some BIOS functions for OS to utilize, OS may follow below ACPI spec to
> > > > treat them as reserved range:
> > > > > https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.html#
> > > > > uefi-memory-types-and-mapping-to-acpi-address-range-types
> > > > >
> > > > > Like I mentioned earlier, that PR is still in early phase and has not reflected all
> > > > the required changes yet, but the idea is to build
> > > > gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes.
> > > > > UEFI generic Payload has DxeMain integrated, however Memory Types are
> > > > platform-specific, for example, some platforms may need bigger runtime memory
> > > > for their implementation, that's why we want such FDT reserved-memory node to
> > > > tell DxeMain.
> > > > >
> > > >
> > > > > The Payload flow will be like this:
> > > > > Payload creates built-in default MemoryTypes table ->
> > > > > FDT reserved-memory node to override if required (this also ensures the
> > > > same memory map cross boots so ACPI S4 works) ->
> > > > > Build gEfiMemoryTypeInformationGuid HOB by "platfom specific"
> > > > MemoryTypes Table ->
> > > > > DxeMain/GCD to consume this MemoryTypes table and setup memory
> > > > service ->
> > > > > Install memory types table to UEFI system table.Configuration table...
> > > > >
> > > > > Note: if Payload built-in default MemoryTypes table works fine for the
> > > > > platform, then FDT reserved-memory node does not need to provide such
> > > > 'usage' compatible strings. (optional) This FDT node could allow
> > > > flexibility/compatibility without rebuilding Payload binary.
> > > > >
> > > > > Not sure if I answered all your questions, please highlight which area you need
> > > > more information.
> > > > >
> > > >
> > > > The gEfiMemoryTypeInformationGuid HOB typically carries platform defaults, and
> > > > the actual memory type information is kept in a non-volatile EFI variable, which
> > > > gets updated when the memory usage changes. Is this different for
> > > > UefiPayloadPkg?
> > > >
> > > > (For those among the cc'ees less versed in EFI/EDK2: when you get the 'config
> > > > changed -rebooting' message from the boot firmware, it typically means that this
> > > > memory type table has changed, and a reboot is necessary.)
> > > >
> > > > So the platform init needs to read this variable, or get the information in a
> > > > different way. I assume it is the payload, not the platform init that updates the
> > > > variable when necessary. This means the information flows from payload(n) to
> > > > platform init(n+1), where n is a monotonic index tracking consecutive boots of the
> > > > system.
> > > >
> > > > Can you explain how the DT fits into this? How are the runtime-code and
> > > > runtime-data memory reservation nodes under /reserved-memory used to
> > > > implement this information exchange between platform init and payload? And
> > > > how do the HOB and the EFI variable fit into this picture?
> > >
> > >
> > > 1. With some offline discussion, we would move gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node. This is because it is edk2 implementation choice and non-edk2 PlatformInit or Payload may not have such memory optimization implementation. (not a generic usage/requirement for PlatformInit and Payload)
> > >
> > > The edk2 example flow will be like below:
> > >
> > > PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid and create Hob->
> > > PlatformInit to initialize FDT->upl-custom node to report gEfiMemoryTypeInformationGuid HOB information ->
> > > UefiPayload entry to re-create gEfiMemoryTypeInformationGuid HOB basing on FDT input (instead of the default MemoryType inside UefiPayload) ->
> > > UefiPayload DxeMain/Gcd will consume gEfiMemoryTypeInformationGuid Hob for memory type information ->
> > > UefiPayload to initialize UEFI environment (mainly DXE dispatcher) ->
> > > (additional FV binary appended to common UefiPayload binary) PlatformPayload to provide VariableService which is platform specific ->
> > > UefiPayload UefiBootManager will SetVariable if memory type change needed and request a warm reset ->
> > > Back to PlatformInit ...
> > >
> >
> > OK so the upl-custom node can do whatever it needs to. I imagine these
> > will include the memory descriptor attribute field, and other parts
> > that may be missing from the /reserved-memory DT node specification?
From my side, ideally this will be empty, but it may be needed to make
things work.
> >
> > >
> > > 2. Now the proposed reserved-memory node usages will be for PlatformInit to provide data which may be used by Payload or OS. This is not edk2 specific and any PlatformInit/Payload could have same support.
> > > Note: all of below are optional and PlatformInit may choose to implement some of them or not.
> > >
> > > - acpi
> > > If PlatformInit created some ACPI tables, this will report a memory region which contains all the tables to Payload and Payload may base on this to add some more tables if required.
> > >
> > > - acpi-nvs
> > > If PlatformInit has created some ACPI tables which having ACPI NVS memory dependency, this will be that nvs region.
> > >
> >
> > These make sense.
> >
> > > - boot-code
> > > When PlatformInit having some FW boot phase code that could be freed for OS to use when payload transferring control to UEFI OS
> > >
> > > - boot-data
> > > When PlatformInit having some FW boot phase data that could be freed for OS to use when payload transferring control to UEFI OS.
> > >
> > > - runtime-code
> > > PlatformInit may provide some services code that can be used for Payload to initialize UEFI Runtime Services for supporting UEFI OS.
> > >
> > > - runtime-data
> > > PlatformInit may provide some services data that can be used for Payload to Initialize UEFI Runtime Services for supporting UEFI OS.
> > >
>
> I'll say it again. "boot" and "runtime" on their own could mean about
> anything, but the usage here is clearly tied to UEFI (or the EDK2
> implementation) and its meaning of boot and runtime. So the naming
> needs to reflect that.
>
Chasel has made a suggestion.
>
> > A UEFI OS must consume this information from the UEFI memory map, not
> > from the /reserved-memory nodes. So these nodes must either not be
> > visible to the OS at all, or carry an annotation that the OS must
> > ignore them.
>
> The kernel will process /reserved-memory for UEFI boot, so the
> expectation is anything in the EFI memory map is not present there. An
> annotation to ignore some nodes would require going back in time or
> accepting 2 sources of truth on existing OS.
The DT spec says:
>>>>>
3.5.4 /reserved-memory and UEFI
When booting via [UEFI], static /reserved-memory regions must also be
listed in the system memory map obtained
via the GetMemoryMap() UEFI boot time service as defined in [UEFI] §
7.2. The reserved memory regions need to be
included in the UEFI memory map to protect against allocations by UEFI
applications.
<<<<<
To my reading, that means that the requirement is that the
/reserved-memory regions must be returned from the GetMemoryMap() call
too. I don't see it saying that the reserved-memory regions must be
empty, though.
>
> > Would it be possible to include a restriction in the DT schema that
> > these are only valid in the firmware boot phase?
>
> The only way ATM is including a schema or not when running validation
> on a DT for a particular boot phase. Include the schema in the project
> that wants to use these nodes and don't include it in cases that don't
> use it. I don't see a reason why this needs to be in dtschema.
We would like to use a single schema, which is dtschema. UPL
validation will eventually check this, so it needs to be correct.
Regards,
Simon
> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Wednesday, January 3, 2024 7:22 AM
> To: Chiu, Chasel <[email protected]>
> Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> <[email protected]>; lkml <[email protected]>; Dhaval
> Sharma <[email protected]>; Brune, Maximilian
> <[email protected]>; Yunhui Cui <[email protected]>;
> Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> <[email protected]>; Guo, Gua <[email protected]>; linux-
> [email protected]; U-Boot Mailing List <[email protected]>
> Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> usages
>
> On Fri, 22 Dec 2023 at 20:52, Chiu, Chasel <[email protected]> wrote:
> >
> >
> > Please see my reply below inline.
> >
> > Thanks,
> > Chasel
> >
> ...
> > > > > The gEfiMemoryTypeInformationGuid HOB typically carries platform
> > > > > defaults, and the actual memory type information is kept in a
> > > > > non-volatile EFI variable, which gets updated when the memory
> > > > > usage changes. Is this different for UefiPayloadPkg?
> > > > >
> > > > > (For those among the cc'ees less versed in EFI/EDK2: when you
> > > > > get the 'config changed -rebooting' message from the boot
> > > > > firmware, it typically means that this memory type table has
> > > > > changed, and a reboot is necessary.)
> > > > >
> > > > > So the platform init needs to read this variable, or get the
> > > > > information in a different way. I assume it is the payload, not
> > > > > the platform init that updates the variable when necessary. This
> > > > > means the information flows from payload(n) to platform
> > > > > init(n+1), where n is a monotonic index tracking consecutive boots of the
> system.
> > > > >
> > > > > Can you explain how the DT fits into this? How are the
> > > > > runtime-code and runtime-data memory reservation nodes under
> > > > > /reserved-memory used to implement this information exchange
> > > > > between platform init and payload? And how do the HOB and the EFI
> variable fit into this picture?
> > > >
> > > >
> > > > 1. With some offline discussion, we would move
> > > > gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node. This
> > > > is because it is edk2 implementation choice and non-edk2
> > > > PlatformInit or Payload may not have such memory optimization
> > > > implementation. (not a generic usage/requirement for PlatformInit
> > > > and Payload)
> > > >
> > > > The edk2 example flow will be like below:
> > > >
> > > > PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid and
> > > > create Hob-
> > > >
> > > > PlatformInit to initialize FDT->upl-custom node to report
> > > gEfiMemoryTypeInformationGuid HOB information ->
> > > > UefiPayload entry to re-create gEfiMemoryTypeInformationGuid
> > > > HOB basing
> > > on FDT input (instead of the default MemoryType inside UefiPayload)
> > > ->
> > > > UefiPayload DxeMain/Gcd will consume
> > > > gEfiMemoryTypeInformationGuid
> > > Hob for memory type information ->
> > > > UefiPayload to initialize UEFI environment (mainly DXE dispatcher) ->
> > > > (additional FV binary appended to common UefiPayload
> > > > binary)
> > > PlatformPayload to provide VariableService which is platform
> > > specific ->
> > > > UefiPayload UefiBootManager will SetVariable if memory
> > > > type change
> > > needed and request a warm reset ->
> > > > Back to PlatformInit ...
> > > >
> > >
> > > OK so the upl-custom node can do whatever it needs to. I imagine
> > > these will include the memory descriptor attribute field, and other
> > > parts that may be missing from the /reserved-memory DT node specification?
> >
> >
> > Yes, if needed by edk2 specific implementation, not generic enough, we may
> consider to use upl-custom node to pass those data.
> >
> >
> > >
> > > >
> > > > 2. Now the proposed reserved-memory node usages will be for
> > > > PlatformInit to
> > > provide data which may be used by Payload or OS. This is not edk2
> > > specific and any PlatformInit/Payload could have same support.
> > > > Note: all of below are optional and PlatformInit may choose to
> > > > implement some
> > > of them or not.
> > > >
> > > > - acpi
> > > > If PlatformInit created some ACPI tables, this will report a
> > > > memory region which
> > > contains all the tables to Payload and Payload may base on this to
> > > add some more tables if required.
> > > >
> > > > - acpi-nvs
> > > > If PlatformInit has created some ACPI tables which having ACPI NVS
> > > > memory
> > > dependency, this will be that nvs region.
> > > >
> > >
> > > These make sense.
> > >
> > > > - boot-code
> > > > When PlatformInit having some FW boot phase code that could be
> > > > freed for OS to use when payload transferring control to UEFI OS
> > > >
> > > > - boot-data
> > > > When PlatformInit having some FW boot phase data that could be
> > > > freed for OS
> > > to use when payload transferring control to UEFI OS.
> > > >
> > > > - runtime-code
> > > > PlatformInit may provide some services code that can be used for
> > > > Payload to
> > > initialize UEFI Runtime Services for supporting UEFI OS.
> > > >
> > > > - runtime-data
> > > > PlatformInit may provide some services data that can be used for
> > > > Payload to
> > > Initialize UEFI Runtime Services for supporting UEFI OS.
> > > >
> > >
> > > A UEFI OS must consume this information from the UEFI memory map,
> > > not from the /reserved-memory nodes. So these nodes must either not
> > > be visible to the OS at all, or carry an annotation that the OS must ignore
> them.
> > >
> > > Would it be possible to include a restriction in the DT schema that
> > > these are only valid in the firmware boot phase?
> >
> >
> > https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#efi-bo
> > ot-services-exitbootservices Per UEFI specification, UEFI OS will
> > always call UEFI GetMemoryMap function to retrieve memory map, so FDT
> node present or not does not matter to UEFI OS. We probably could have
> annotation in UPL specification to emphasize this.
> > I'm not familiar with Linux FDT boot, but if non-UEFI OS does not call UEFI
> GetMemoryMap() and does not know what is runtime-code/data, boot-
> code/data, it might just treat such reserved-memory nodes as 'regular' reserved
> memory nodes, and that's still ok because non-UEFI OS will not call to any
> runtime service or re-purpose boot-code/data memory regions.
> >
>
> You are saying the same thing but in a different way. A UEFI OS must only rely on
> GetMemoryMap(), and not on the /reserved-memory node to obtain this
> information. But this requirement needs to be stated
> somewhere: the UEFI spec does not reason about other sources of EFI memory
> information at all, and this DT schema does not mention any of this either.
>
> > Would you provide a real OS case which will be impacted by this reserved-
> memory schema so we can discuss basing on real case?
> >
>
> Funny, that is what I have been trying to get from you :-)
>
> The problem I am anticipating here is that the information in /reserved-memory
> may be out of sync with the EFI memory map. It needs to be made clear that the
> EFI memory map is the only source of truth when the OS is involved, and this
> /reserved-memory mechanism should only be used by other firmware stages. But
> the schema does not mention this at all. The schema also does not mention that
> the information in /reserved-memory is not actually sufficient to reconstruct the
> EFI memory map that the firmware payload expects (which is why the upl-
> custom-node exists too)
Does below solve your concerns if we mention those in schema description? (please feel free to add more if you have)
. boot-code/boot-data and runtime-code/runtime-data usages are following UEFI specification
. before ExitBootServices: https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#memory-type-usage-before-exitbootservices
. after ExitBootServices: https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#memory-type-usage-after-exitbootservices
. These usages do not intend to construct full UEFI memory map, it is only for PlatformInit to pass pre-installed tables or services to Payload for supporting UEFI OS boot.
. These usages are optional
. Typically UEFI OS boot will always call GetMemoryMap() to retrieve memory map following UEFI spec, no matter DT nodes present or not (https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#efi-boot-services-exitbootservices)
. Typically Non-UEFI OS boot will treat those boot* or runtime* reserved-memory as 'regular' reserved memory if present.
On Thu, 4 Jan 2024 at 01:25, Chiu, Chasel <[email protected]> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel <[email protected]>
> > Sent: Wednesday, January 3, 2024 7:22 AM
> > To: Chiu, Chasel <[email protected]>
> > Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> > <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> > <[email protected]>; lkml <[email protected]>; Dhaval
> > Sharma <[email protected]>; Brune, Maximilian
> > <[email protected]>; Yunhui Cui <[email protected]>;
> > Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > [email protected]; U-Boot Mailing List <[email protected]>
> > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > usages
> >
> > On Fri, 22 Dec 2023 at 20:52, Chiu, Chasel <[email protected]> wrote:
> > >
> > >
> > > Please see my reply below inline.
> > >
> > > Thanks,
> > > Chasel
> > >
> > ...
> > > > > > The gEfiMemoryTypeInformationGuid HOB typically carries platform
> > > > > > defaults, and the actual memory type information is kept in a
> > > > > > non-volatile EFI variable, which gets updated when the memory
> > > > > > usage changes. Is this different for UefiPayloadPkg?
> > > > > >
> > > > > > (For those among the cc'ees less versed in EFI/EDK2: when you
> > > > > > get the 'config changed -rebooting' message from the boot
> > > > > > firmware, it typically means that this memory type table has
> > > > > > changed, and a reboot is necessary.)
> > > > > >
> > > > > > So the platform init needs to read this variable, or get the
> > > > > > information in a different way. I assume it is the payload, not
> > > > > > the platform init that updates the variable when necessary. This
> > > > > > means the information flows from payload(n) to platform
> > > > > > init(n+1), where n is a monotonic index tracking consecutive boots of the
> > system.
> > > > > >
> > > > > > Can you explain how the DT fits into this? How are the
> > > > > > runtime-code and runtime-data memory reservation nodes under
> > > > > > /reserved-memory used to implement this information exchange
> > > > > > between platform init and payload? And how do the HOB and the EFI
> > variable fit into this picture?
> > > > >
> > > > >
> > > > > 1. With some offline discussion, we would move
> > > > > gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node. This
> > > > > is because it is edk2 implementation choice and non-edk2
> > > > > PlatformInit or Payload may not have such memory optimization
> > > > > implementation. (not a generic usage/requirement for PlatformInit
> > > > > and Payload)
> > > > >
> > > > > The edk2 example flow will be like below:
> > > > >
> > > > > PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid and
> > > > > create Hob-
> > > > >
> > > > > PlatformInit to initialize FDT->upl-custom node to report
> > > > gEfiMemoryTypeInformationGuid HOB information ->
> > > > > UefiPayload entry to re-create gEfiMemoryTypeInformationGuid
> > > > > HOB basing
> > > > on FDT input (instead of the default MemoryType inside UefiPayload)
> > > > ->
> > > > > UefiPayload DxeMain/Gcd will consume
> > > > > gEfiMemoryTypeInformationGuid
> > > > Hob for memory type information ->
> > > > > UefiPayload to initialize UEFI environment (mainly DXE dispatcher) ->
> > > > > (additional FV binary appended to common UefiPayload
> > > > > binary)
> > > > PlatformPayload to provide VariableService which is platform
> > > > specific ->
> > > > > UefiPayload UefiBootManager will SetVariable if memory
> > > > > type change
> > > > needed and request a warm reset ->
> > > > > Back to PlatformInit ...
> > > > >
> > > >
> > > > OK so the upl-custom node can do whatever it needs to. I imagine
> > > > these will include the memory descriptor attribute field, and other
> > > > parts that may be missing from the /reserved-memory DT node specification?
> > >
> > >
> > > Yes, if needed by edk2 specific implementation, not generic enough, we may
> > consider to use upl-custom node to pass those data.
> > >
> > >
> > > >
> > > > >
> > > > > 2. Now the proposed reserved-memory node usages will be for
> > > > > PlatformInit to
> > > > provide data which may be used by Payload or OS. This is not edk2
> > > > specific and any PlatformInit/Payload could have same support.
> > > > > Note: all of below are optional and PlatformInit may choose to
> > > > > implement some
> > > > of them or not.
> > > > >
> > > > > - acpi
> > > > > If PlatformInit created some ACPI tables, this will report a
> > > > > memory region which
> > > > contains all the tables to Payload and Payload may base on this to
> > > > add some more tables if required.
> > > > >
> > > > > - acpi-nvs
> > > > > If PlatformInit has created some ACPI tables which having ACPI NVS
> > > > > memory
> > > > dependency, this will be that nvs region.
> > > > >
> > > >
> > > > These make sense.
> > > >
> > > > > - boot-code
> > > > > When PlatformInit having some FW boot phase code that could be
> > > > > freed for OS to use when payload transferring control to UEFI OS
> > > > >
> > > > > - boot-data
> > > > > When PlatformInit having some FW boot phase data that could be
> > > > > freed for OS
> > > > to use when payload transferring control to UEFI OS.
> > > > >
> > > > > - runtime-code
> > > > > PlatformInit may provide some services code that can be used for
> > > > > Payload to
> > > > initialize UEFI Runtime Services for supporting UEFI OS.
> > > > >
> > > > > - runtime-data
> > > > > PlatformInit may provide some services data that can be used for
> > > > > Payload to
> > > > Initialize UEFI Runtime Services for supporting UEFI OS.
> > > > >
> > > >
> > > > A UEFI OS must consume this information from the UEFI memory map,
> > > > not from the /reserved-memory nodes. So these nodes must either not
> > > > be visible to the OS at all, or carry an annotation that the OS must ignore
> > them.
> > > >
> > > > Would it be possible to include a restriction in the DT schema that
> > > > these are only valid in the firmware boot phase?
> > >
> > >
> > > https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#efi-bo
> > > ot-services-exitbootservices Per UEFI specification, UEFI OS will
> > > always call UEFI GetMemoryMap function to retrieve memory map, so FDT
> > node present or not does not matter to UEFI OS. We probably could have
> > annotation in UPL specification to emphasize this.
> > > I'm not familiar with Linux FDT boot, but if non-UEFI OS does not call UEFI
> > GetMemoryMap() and does not know what is runtime-code/data, boot-
> > code/data, it might just treat such reserved-memory nodes as 'regular' reserved
> > memory nodes, and that's still ok because non-UEFI OS will not call to any
> > runtime service or re-purpose boot-code/data memory regions.
> > >
> >
> > You are saying the same thing but in a different way. A UEFI OS must only rely on
> > GetMemoryMap(), and not on the /reserved-memory node to obtain this
> > information. But this requirement needs to be stated
> > somewhere: the UEFI spec does not reason about other sources of EFI memory
> > information at all, and this DT schema does not mention any of this either.
> >
> > > Would you provide a real OS case which will be impacted by this reserved-
> > memory schema so we can discuss basing on real case?
> > >
> >
> > Funny, that is what I have been trying to get from you :-)
> >
> > The problem I am anticipating here is that the information in /reserved-memory
> > may be out of sync with the EFI memory map. It needs to be made clear that the
> > EFI memory map is the only source of truth when the OS is involved, and this
> > /reserved-memory mechanism should only be used by other firmware stages. But
> > the schema does not mention this at all. The schema also does not mention that
> > the information in /reserved-memory is not actually sufficient to reconstruct the
> > EFI memory map that the firmware payload expects (which is why the upl-
> > custom-node exists too)
>
>
>
> Does below solve your concerns if we mention those in schema description? (please feel free to add more if you have)
> . boot-code/boot-data and runtime-code/runtime-data usages are following UEFI specification
> . before ExitBootServices: https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#memory-type-usage-before-exitbootservices
> . after ExitBootServices: https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#memory-type-usage-after-exitbootservices
> . These usages do not intend to construct full UEFI memory map, it is only for PlatformInit to pass pre-installed tables or services to Payload for supporting UEFI OS boot.
> . These usages are optional
> . Typically UEFI OS boot will always call GetMemoryMap() to retrieve memory map following UEFI spec, no matter DT nodes present or not (https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#efi-boot-services-exitbootservices)
> . Typically Non-UEFI OS boot will treat those boot* or runtime* reserved-memory as 'regular' reserved memory if present.
>
This already helps quite a lot, thanks.
But why should a non-UEFI OS be required to keep boot* or runtime*
regions reserved? The firmware stage that boots the OS knows whether
it is performing an UEFI boot or a non-UEFI boot, and it should only
present the information that goes along with that. The OS should never
have to worry about reconciling two sources of truth.
And to Rob's point about boot / runtime being ill-defined: I would
argue that 'runtime' quite clearly implies 'under the OS', and so UEFI
runtime* reservations are assumed to always be relevant to UEFI OSes.
I think there is a fundamental difference of opinion here, where the
position of the firmware developers is that the DT should be the same
across all boot stages, while my position reasoning from the OS side
is that the OS should be able to observe only the abstractions that
are part of the contract between firmware and OS.
> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Thursday, January 4, 2024 12:43 AM
> To: Chiu, Chasel <[email protected]>
> Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> <[email protected]>; lkml <[email protected]>; Dhaval
> Sharma <[email protected]>; Brune, Maximilian
> <[email protected]>; Yunhui Cui <[email protected]>;
> Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> <[email protected]>; Guo, Gua <[email protected]>; linux-
> [email protected]; U-Boot Mailing List <[email protected]>
> Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> usages
>
> On Thu, 4 Jan 2024 at 01:25, Chiu, Chasel <[email protected]> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <[email protected]>
> > > Sent: Wednesday, January 3, 2024 7:22 AM
> > > To: Chiu, Chasel <[email protected]>
> > > Cc: Simon Glass <[email protected]>; [email protected]; Mark
> > > Rutland <[email protected]>; Rob Herring <[email protected]>; Tan,
> > > Lean Sheng <[email protected]>; lkml
> > > <[email protected]>; Dhaval Sharma <[email protected]>;
> > > Brune, Maximilian <[email protected]>; Yunhui Cui
> > > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > > <[email protected]>; linux- [email protected]; U-Boot Mailing
> > > List <[email protected]>
> > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > usages
> > >
> > > On Fri, 22 Dec 2023 at 20:52, Chiu, Chasel <[email protected]> wrote:
> > > >
> > > >
> > > > Please see my reply below inline.
> > > >
> > > > Thanks,
> > > > Chasel
> > > >
> > > ...
> > > > > > > The gEfiMemoryTypeInformationGuid HOB typically carries
> > > > > > > platform defaults, and the actual memory type information is
> > > > > > > kept in a non-volatile EFI variable, which gets updated when
> > > > > > > the memory usage changes. Is this different for UefiPayloadPkg?
> > > > > > >
> > > > > > > (For those among the cc'ees less versed in EFI/EDK2: when
> > > > > > > you get the 'config changed -rebooting' message from the
> > > > > > > boot firmware, it typically means that this memory type
> > > > > > > table has changed, and a reboot is necessary.)
> > > > > > >
> > > > > > > So the platform init needs to read this variable, or get the
> > > > > > > information in a different way. I assume it is the payload,
> > > > > > > not the platform init that updates the variable when
> > > > > > > necessary. This means the information flows from payload(n)
> > > > > > > to platform init(n+1), where n is a monotonic index tracking
> > > > > > > consecutive boots of the
> > > system.
> > > > > > >
> > > > > > > Can you explain how the DT fits into this? How are the
> > > > > > > runtime-code and runtime-data memory reservation nodes under
> > > > > > > /reserved-memory used to implement this information exchange
> > > > > > > between platform init and payload? And how do the HOB and
> > > > > > > the EFI
> > > variable fit into this picture?
> > > > > >
> > > > > >
> > > > > > 1. With some offline discussion, we would move
> > > > > > gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node.
> > > > > > This is because it is edk2 implementation choice and non-edk2
> > > > > > PlatformInit or Payload may not have such memory optimization
> > > > > > implementation. (not a generic usage/requirement for
> > > > > > PlatformInit and Payload)
> > > > > >
> > > > > > The edk2 example flow will be like below:
> > > > > >
> > > > > > PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid
> > > > > > and create Hob-
> > > > > >
> > > > > > PlatformInit to initialize FDT->upl-custom node to report
> > > > > gEfiMemoryTypeInformationGuid HOB information ->
> > > > > > UefiPayload entry to re-create
> > > > > > gEfiMemoryTypeInformationGuid HOB basing
> > > > > on FDT input (instead of the default MemoryType inside
> > > > > UefiPayload)
> > > > > ->
> > > > > > UefiPayload DxeMain/Gcd will consume
> > > > > > gEfiMemoryTypeInformationGuid
> > > > > Hob for memory type information ->
> > > > > > UefiPayload to initialize UEFI environment (mainly DXE dispatcher) -
> >
> > > > > > (additional FV binary appended to common UefiPayload
> > > > > > binary)
> > > > > PlatformPayload to provide VariableService which is platform
> > > > > specific ->
> > > > > > UefiPayload UefiBootManager will SetVariable if
> > > > > > memory type change
> > > > > needed and request a warm reset ->
> > > > > > Back to PlatformInit ...
> > > > > >
> > > > >
> > > > > OK so the upl-custom node can do whatever it needs to. I imagine
> > > > > these will include the memory descriptor attribute field, and
> > > > > other parts that may be missing from the /reserved-memory DT node
> specification?
> > > >
> > > >
> > > > Yes, if needed by edk2 specific implementation, not generic
> > > > enough, we may
> > > consider to use upl-custom node to pass those data.
> > > >
> > > >
> > > > >
> > > > > >
> > > > > > 2. Now the proposed reserved-memory node usages will be for
> > > > > > PlatformInit to
> > > > > provide data which may be used by Payload or OS. This is not
> > > > > edk2 specific and any PlatformInit/Payload could have same support.
> > > > > > Note: all of below are optional and PlatformInit may choose to
> > > > > > implement some
> > > > > of them or not.
> > > > > >
> > > > > > - acpi
> > > > > > If PlatformInit created some ACPI tables, this will report a
> > > > > > memory region which
> > > > > contains all the tables to Payload and Payload may base on this
> > > > > to add some more tables if required.
> > > > > >
> > > > > > - acpi-nvs
> > > > > > If PlatformInit has created some ACPI tables which having ACPI
> > > > > > NVS memory
> > > > > dependency, this will be that nvs region.
> > > > > >
> > > > >
> > > > > These make sense.
> > > > >
> > > > > > - boot-code
> > > > > > When PlatformInit having some FW boot phase code that could be
> > > > > > freed for OS to use when payload transferring control to UEFI
> > > > > > OS
> > > > > >
> > > > > > - boot-data
> > > > > > When PlatformInit having some FW boot phase data that could be
> > > > > > freed for OS
> > > > > to use when payload transferring control to UEFI OS.
> > > > > >
> > > > > > - runtime-code
> > > > > > PlatformInit may provide some services code that can be used
> > > > > > for Payload to
> > > > > initialize UEFI Runtime Services for supporting UEFI OS.
> > > > > >
> > > > > > - runtime-data
> > > > > > PlatformInit may provide some services data that can be used
> > > > > > for Payload to
> > > > > Initialize UEFI Runtime Services for supporting UEFI OS.
> > > > > >
> > > > >
> > > > > A UEFI OS must consume this information from the UEFI memory
> > > > > map, not from the /reserved-memory nodes. So these nodes must
> > > > > either not be visible to the OS at all, or carry an annotation
> > > > > that the OS must ignore
> > > them.
> > > > >
> > > > > Would it be possible to include a restriction in the DT schema
> > > > > that these are only valid in the firmware boot phase?
> > > >
> > > >
> > > > https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#ef
> > > > i-bo ot-services-exitbootservices Per UEFI specification, UEFI OS
> > > > will always call UEFI GetMemoryMap function to retrieve memory
> > > > map, so FDT
> > > node present or not does not matter to UEFI OS. We probably could
> > > have annotation in UPL specification to emphasize this.
> > > > I'm not familiar with Linux FDT boot, but if non-UEFI OS does not
> > > > call UEFI
> > > GetMemoryMap() and does not know what is runtime-code/data, boot-
> > > code/data, it might just treat such reserved-memory nodes as
> > > 'regular' reserved memory nodes, and that's still ok because
> > > non-UEFI OS will not call to any runtime service or re-purpose boot-code/data
> memory regions.
> > > >
> > >
> > > You are saying the same thing but in a different way. A UEFI OS must
> > > only rely on GetMemoryMap(), and not on the /reserved-memory node to
> > > obtain this information. But this requirement needs to be stated
> > > somewhere: the UEFI spec does not reason about other sources of EFI
> > > memory information at all, and this DT schema does not mention any of this
> either.
> > >
> > > > Would you provide a real OS case which will be impacted by this
> > > > reserved-
> > > memory schema so we can discuss basing on real case?
> > > >
> > >
> > > Funny, that is what I have been trying to get from you :-)
> > >
> > > The problem I am anticipating here is that the information in
> > > /reserved-memory may be out of sync with the EFI memory map. It
> > > needs to be made clear that the EFI memory map is the only source of
> > > truth when the OS is involved, and this /reserved-memory mechanism
> > > should only be used by other firmware stages. But the schema does
> > > not mention this at all. The schema also does not mention that the
> > > information in /reserved-memory is not actually sufficient to
> > > reconstruct the EFI memory map that the firmware payload expects
> > > (which is why the upl- custom-node exists too)
> >
> >
> >
> > Does below solve your concerns if we mention those in schema
> > description? (please feel free to add more if you have) . boot-code/boot-data
> and runtime-code/runtime-data usages are following UEFI specification
> > . before ExitBootServices:
> https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#memory-
> type-usage-before-exitbootservices
> > . after ExitBootServices:
> > https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#memory
> > -type-usage-after-exitbootservices
> > . These usages do not intend to construct full UEFI memory map, it is only for
> PlatformInit to pass pre-installed tables or services to Payload for supporting UEFI
> OS boot.
> > . These usages are optional
> > . Typically UEFI OS boot will always call GetMemoryMap() to retrieve
> > memory map following UEFI spec, no matter DT nodes present or not
> > (https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#efi-b
> > oot-services-exitbootservices) . Typically Non-UEFI OS boot will treat
> > those boot* or runtime* reserved-memory as 'regular' reserved memory if
> present.
> >
>
> This already helps quite a lot, thanks.
>
> But why should a non-UEFI OS be required to keep boot* or runtime* regions
> reserved? The firmware stage that boots the OS knows whether it is performing
> an UEFI boot or a non-UEFI boot, and it should only present the information that
> goes along with that. The OS should never have to worry about reconciling two
> sources of truth.
>
> And to Rob's point about boot / runtime being ill-defined: I would argue that
> 'runtime' quite clearly implies 'under the OS', and so UEFI
> runtime* reservations are assumed to always be relevant to UEFI OSes.
>
> I think there is a fundamental difference of opinion here, where the position of
> the firmware developers is that the DT should be the same across all boot stages,
> while my position reasoning from the OS side is that the OS should be able to
> observe only the abstractions that are part of the contract between firmware and
> OS.
I agree that boot* and runtime* can be utilized by non-UEFI OS too, we are just reusing existing definitions from UEFI spec.
. boot-code/boot-data: firmware stage code/data that can be freed after firmware stage ending so OS will have more usable memory.
. runtime-code/runtime-data: firmware stage code/data that are intended to be utilized by OS stage.
Non-UEFI OS still can implement/support boot* or runtime* memory if they want, and the runtime service can be 'non-UEFI' runtime service too as long as OS/FW aligning each other.
Or non-UEFI OS can simply treat them as "usable memory" if they do not call to any runtime services from those memory regions. (in this case runtime* memory can be repurposed just like boot* memory)
That will be OS choices and we may add some example OS handling to schema description too.
While we are working on UPL specific DT, we got agreement that 2 separate DT are unnecessary, we better align/merge with existing OS DT and OS could utilize those additional UPL DT information too if they want.
This also simplifies/unifies PlatformInit as same DT could support different OS loader Payloads.
On Thu, 4 Jan 2024 at 18:53, Chiu, Chasel <[email protected]> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel <[email protected]>
> > Sent: Thursday, January 4, 2024 12:43 AM
> > To: Chiu, Chasel <[email protected]>
> > Cc: Simon Glass <[email protected]>; [email protected]; Mark Rutland
> > <[email protected]>; Rob Herring <[email protected]>; Tan, Lean Sheng
> > <[email protected]>; lkml <[email protected]>; Dhaval
> > Sharma <[email protected]>; Brune, Maximilian
> > <[email protected]>; Yunhui Cui <[email protected]>;
> > Dong, Guo <[email protected]>; Tom Rini <[email protected]>; ron minnich
> > <[email protected]>; Guo, Gua <[email protected]>; linux-
> > [email protected]; U-Boot Mailing List <[email protected]>
> > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > usages
> >
> > On Thu, 4 Jan 2024 at 01:25, Chiu, Chasel <[email protected]> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <[email protected]>
> > > > Sent: Wednesday, January 3, 2024 7:22 AM
> > > > To: Chiu, Chasel <[email protected]>
> > > > Cc: Simon Glass <[email protected]>; [email protected]; Mark
> > > > Rutland <[email protected]>; Rob Herring <[email protected]>; Tan,
> > > > Lean Sheng <[email protected]>; lkml
> > > > <[email protected]>; Dhaval Sharma <[email protected]>;
> > > > Brune, Maximilian <[email protected]>; Yunhui Cui
> > > > <[email protected]>; Dong, Guo <[email protected]>; Tom Rini
> > > > <[email protected]>; ron minnich <[email protected]>; Guo, Gua
> > > > <[email protected]>; linux- [email protected]; U-Boot Mailing
> > > > List <[email protected]>
> > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > > usages
> > > >
> > > > On Fri, 22 Dec 2023 at 20:52, Chiu, Chasel <[email protected]> wrote:
> > > > >
> > > > >
> > > > > Please see my reply below inline.
> > > > >
> > > > > Thanks,
> > > > > Chasel
> > > > >
> > > > ...
> > > > > > > > The gEfiMemoryTypeInformationGuid HOB typically carries
> > > > > > > > platform defaults, and the actual memory type information is
> > > > > > > > kept in a non-volatile EFI variable, which gets updated when
> > > > > > > > the memory usage changes. Is this different for UefiPayloadPkg?
> > > > > > > >
> > > > > > > > (For those among the cc'ees less versed in EFI/EDK2: when
> > > > > > > > you get the 'config changed -rebooting' message from the
> > > > > > > > boot firmware, it typically means that this memory type
> > > > > > > > table has changed, and a reboot is necessary.)
> > > > > > > >
> > > > > > > > So the platform init needs to read this variable, or get the
> > > > > > > > information in a different way. I assume it is the payload,
> > > > > > > > not the platform init that updates the variable when
> > > > > > > > necessary. This means the information flows from payload(n)
> > > > > > > > to platform init(n+1), where n is a monotonic index tracking
> > > > > > > > consecutive boots of the
> > > > system.
> > > > > > > >
> > > > > > > > Can you explain how the DT fits into this? How are the
> > > > > > > > runtime-code and runtime-data memory reservation nodes under
> > > > > > > > /reserved-memory used to implement this information exchange
> > > > > > > > between platform init and payload? And how do the HOB and
> > > > > > > > the EFI
> > > > variable fit into this picture?
> > > > > > >
> > > > > > >
> > > > > > > 1. With some offline discussion, we would move
> > > > > > > gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node.
> > > > > > > This is because it is edk2 implementation choice and non-edk2
> > > > > > > PlatformInit or Payload may not have such memory optimization
> > > > > > > implementation. (not a generic usage/requirement for
> > > > > > > PlatformInit and Payload)
> > > > > > >
> > > > > > > The edk2 example flow will be like below:
> > > > > > >
> > > > > > > PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid
> > > > > > > and create Hob-
> > > > > > >
> > > > > > > PlatformInit to initialize FDT->upl-custom node to report
> > > > > > gEfiMemoryTypeInformationGuid HOB information ->
> > > > > > > UefiPayload entry to re-create
> > > > > > > gEfiMemoryTypeInformationGuid HOB basing
> > > > > > on FDT input (instead of the default MemoryType inside
> > > > > > UefiPayload)
> > > > > > ->
> > > > > > > UefiPayload DxeMain/Gcd will consume
> > > > > > > gEfiMemoryTypeInformationGuid
> > > > > > Hob for memory type information ->
> > > > > > > UefiPayload to initialize UEFI environment (mainly DXE dispatcher) -
> > >
> > > > > > > (additional FV binary appended to common UefiPayload
> > > > > > > binary)
> > > > > > PlatformPayload to provide VariableService which is platform
> > > > > > specific ->
> > > > > > > UefiPayload UefiBootManager will SetVariable if
> > > > > > > memory type change
> > > > > > needed and request a warm reset ->
> > > > > > > Back to PlatformInit ...
> > > > > > >
> > > > > >
> > > > > > OK so the upl-custom node can do whatever it needs to. I imagine
> > > > > > these will include the memory descriptor attribute field, and
> > > > > > other parts that may be missing from the /reserved-memory DT node
> > specification?
> > > > >
> > > > >
> > > > > Yes, if needed by edk2 specific implementation, not generic
> > > > > enough, we may
> > > > consider to use upl-custom node to pass those data.
> > > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > 2. Now the proposed reserved-memory node usages will be for
> > > > > > > PlatformInit to
> > > > > > provide data which may be used by Payload or OS. This is not
> > > > > > edk2 specific and any PlatformInit/Payload could have same support.
> > > > > > > Note: all of below are optional and PlatformInit may choose to
> > > > > > > implement some
> > > > > > of them or not.
> > > > > > >
> > > > > > > - acpi
> > > > > > > If PlatformInit created some ACPI tables, this will report a
> > > > > > > memory region which
> > > > > > contains all the tables to Payload and Payload may base on this
> > > > > > to add some more tables if required.
> > > > > > >
> > > > > > > - acpi-nvs
> > > > > > > If PlatformInit has created some ACPI tables which having ACPI
> > > > > > > NVS memory
> > > > > > dependency, this will be that nvs region.
> > > > > > >
> > > > > >
> > > > > > These make sense.
> > > > > >
> > > > > > > - boot-code
> > > > > > > When PlatformInit having some FW boot phase code that could be
> > > > > > > freed for OS to use when payload transferring control to UEFI
> > > > > > > OS
> > > > > > >
> > > > > > > - boot-data
> > > > > > > When PlatformInit having some FW boot phase data that could be
> > > > > > > freed for OS
> > > > > > to use when payload transferring control to UEFI OS.
> > > > > > >
> > > > > > > - runtime-code
> > > > > > > PlatformInit may provide some services code that can be used
> > > > > > > for Payload to
> > > > > > initialize UEFI Runtime Services for supporting UEFI OS.
> > > > > > >
> > > > > > > - runtime-data
> > > > > > > PlatformInit may provide some services data that can be used
> > > > > > > for Payload to
> > > > > > Initialize UEFI Runtime Services for supporting UEFI OS.
> > > > > > >
> > > > > >
> > > > > > A UEFI OS must consume this information from the UEFI memory
> > > > > > map, not from the /reserved-memory nodes. So these nodes must
> > > > > > either not be visible to the OS at all, or carry an annotation
> > > > > > that the OS must ignore
> > > > them.
> > > > > >
> > > > > > Would it be possible to include a restriction in the DT schema
> > > > > > that these are only valid in the firmware boot phase?
> > > > >
> > > > >
> > > > > https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#ef
> > > > > i-bo ot-services-exitbootservices Per UEFI specification, UEFI OS
> > > > > will always call UEFI GetMemoryMap function to retrieve memory
> > > > > map, so FDT
> > > > node present or not does not matter to UEFI OS. We probably could
> > > > have annotation in UPL specification to emphasize this.
> > > > > I'm not familiar with Linux FDT boot, but if non-UEFI OS does not
> > > > > call UEFI
> > > > GetMemoryMap() and does not know what is runtime-code/data, boot-
> > > > code/data, it might just treat such reserved-memory nodes as
> > > > 'regular' reserved memory nodes, and that's still ok because
> > > > non-UEFI OS will not call to any runtime service or re-purpose boot-code/data
> > memory regions.
> > > > >
> > > >
> > > > You are saying the same thing but in a different way. A UEFI OS must
> > > > only rely on GetMemoryMap(), and not on the /reserved-memory node to
> > > > obtain this information. But this requirement needs to be stated
> > > > somewhere: the UEFI spec does not reason about other sources of EFI
> > > > memory information at all, and this DT schema does not mention any of this
> > either.
> > > >
> > > > > Would you provide a real OS case which will be impacted by this
> > > > > reserved-
> > > > memory schema so we can discuss basing on real case?
> > > > >
> > > >
> > > > Funny, that is what I have been trying to get from you :-)
> > > >
> > > > The problem I am anticipating here is that the information in
> > > > /reserved-memory may be out of sync with the EFI memory map. It
> > > > needs to be made clear that the EFI memory map is the only source of
> > > > truth when the OS is involved, and this /reserved-memory mechanism
> > > > should only be used by other firmware stages. But the schema does
> > > > not mention this at all. The schema also does not mention that the
> > > > information in /reserved-memory is not actually sufficient to
> > > > reconstruct the EFI memory map that the firmware payload expects
> > > > (which is why the upl- custom-node exists too)
> > >
> > >
> > >
> > > Does below solve your concerns if we mention those in schema
> > > description? (please feel free to add more if you have) . boot-code/boot-data
> > and runtime-code/runtime-data usages are following UEFI specification
> > > . before ExitBootServices:
> > https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#memory-
> > type-usage-before-exitbootservices
> > > . after ExitBootServices:
> > > https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#memory
> > > -type-usage-after-exitbootservices
> > > . These usages do not intend to construct full UEFI memory map, it is only for
> > PlatformInit to pass pre-installed tables or services to Payload for supporting UEFI
> > OS boot.
> > > . These usages are optional
> > > . Typically UEFI OS boot will always call GetMemoryMap() to retrieve
> > > memory map following UEFI spec, no matter DT nodes present or not
> > > (https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#efi-b
> > > oot-services-exitbootservices) . Typically Non-UEFI OS boot will treat
> > > those boot* or runtime* reserved-memory as 'regular' reserved memory if
> > present.
> > >
> >
> > This already helps quite a lot, thanks.
> >
> > But why should a non-UEFI OS be required to keep boot* or runtime* regions
> > reserved? The firmware stage that boots the OS knows whether it is performing
> > an UEFI boot or a non-UEFI boot, and it should only present the information that
> > goes along with that. The OS should never have to worry about reconciling two
> > sources of truth.
> >
> > And to Rob's point about boot / runtime being ill-defined: I would argue that
> > 'runtime' quite clearly implies 'under the OS', and so UEFI
> > runtime* reservations are assumed to always be relevant to UEFI OSes.
> >
> > I think there is a fundamental difference of opinion here, where the position of
> > the firmware developers is that the DT should be the same across all boot stages,
> > while my position reasoning from the OS side is that the OS should be able to
> > observe only the abstractions that are part of the contract between firmware and
> > OS.
>
> I agree that boot* and runtime* can be utilized by non-UEFI OS too, we are just reusing existing definitions from UEFI spec.
> . boot-code/boot-data: firmware stage code/data that can be freed after firmware stage ending so OS will have more usable memory.
> . runtime-code/runtime-data: firmware stage code/data that are intended to be utilized by OS stage.
> Non-UEFI OS still can implement/support boot* or runtime* memory if they want, and the runtime service can be 'non-UEFI' runtime service too as long as OS/FW aligning each other.
No, I disagree here, and this is the core of the issue IMO. Boot data
and runtime data are tied to UEFI boot. A non-UEFI OS must either keep
these regions reserved and not use them at all, or disregard the
reservation and use them as ordinary memory.
What I don't want is a frankenstein construction where UEFI boot is
avoided for religious reasons, but the regions in question are still
parsed/accessed/etc to reimplement things like SMBIOS etc in a way
that violates the specification.
> Or non-UEFI OS can simply treat them as "usable memory" if they do not call to any runtime services from those memory regions. (in this case runtime* memory can be repurposed just like boot* memory)
> That will be OS choices and we may add some example OS handling to schema description too.
>
The OS should not have to care about this at all. The reservations are
intended for firmware stages, not the OS. The OS will either find
these reservations in the UEFI memory map, or it should not care about
them at all.
> While we are working on UPL specific DT, we got agreement that 2 separate DT are unnecessary, we better align/merge with existing OS DT and OS could utilize those additional UPL DT information too if they want.
> This also simplifies/unifies PlatformInit as same DT could support different OS loader Payloads.
>
I was not part of any agreement that 2 separate DTs are unnecessary,
and I think this discussion proves that exposing firmware
implementation details to the OS via a unified DT can be problematic.