[RFC PATCH 0/5] Allow for removal of DT nodes and properties
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
Add a function which is registered to spy for a EVT_FT_FIXUP event, and removes the non upstreamed nodes and properties from the devicetree before it gets passed to the OS.
This allows removing entire nodes, or specific properties under nodes from the devicetree. The required nodes and properties can be registered for removal through the DT_NON_COMPLIANT_PURGE and DT_NON_COMPLIANT_PURGE_LIST macros.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- include/dt-structs.h | 11 +++++++ lib/Makefile | 1 + lib/dt_purge.c | 73 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 lib/dt_purge.c
diff --git a/include/dt-structs.h b/include/dt-structs.h index fa1622cb1d..f535c60471 100644 --- a/include/dt-structs.h +++ b/include/dt-structs.h @@ -57,3 +57,14 @@ struct phandle_2_arg { #endif
#endif + +struct dt_non_compliant_purge { + const char *node_path; + const char *prop; +}; + +#define DT_NON_COMPLIANT_PURGE(__name) \ + ll_entry_declare(struct dt_non_compliant_purge, __name, dt_purge) + +#define DT_NON_COMPLIANT_PURGE_LIST(__name) \ + ll_entry_declare_list(struct dt_non_compliant_purge, __name, dt_purge) diff --git a/lib/Makefile b/lib/Makefile index 8d8ccc8bbc..82a906daa0 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -37,6 +37,7 @@ endif obj-y += crc8.o obj-y += crc16.o obj-y += crc16-ccitt.o +obj-y += dt_purge.o obj-$(CONFIG_ERRNO_STR) += errno_str.o obj-$(CONFIG_FIT) += fdtdec_common.o obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o diff --git a/lib/dt_purge.c b/lib/dt_purge.c new file mode 100644 index 0000000000..f893ba9796 --- /dev/null +++ b/lib/dt_purge.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2023, Linaro Limited + */ + +#include <dt-structs.h> +#include <event.h> +#include <linker_lists.h> + +#include <linux/libfdt.h> + +/** + * dt_non_compliant_purge() - Remove non-upstreamed nodes and properties + * from the DT + * @ctx: Context for event + * @event: Event to process + * + * Iterate through an array of DT nodes and properties, and remove them + * from the device-tree before the DT gets handed over to the kernel. + * These are nodes and properties which do not have upstream bindings + * and need to be purged before being handed over to the kernel. + * + * If both the node and property are specified, delete the property. If + * only the node is specified, delete the entire node, including it's + * subnodes, if any. + * + * Return: 0 if OK, -ve on error + */ +static int dt_non_compliant_purge(void *ctx, struct event *event) +{ + int nodeoff = 0; + int err = 0; + void *fdt; + const struct event_ft_fixup *fixup = &event->data.ft_fixup; + struct dt_non_compliant_purge *purge_entry; + struct dt_non_compliant_purge *purge_start = + ll_entry_start(struct dt_non_compliant_purge, dt_purge); + int nentries = ll_entry_count(struct dt_non_compliant_purge, dt_purge); + + if (fixup->images) + return 0; + + fdt = fixup->tree.fdt; + for (purge_entry = purge_start; purge_entry != purge_start + nentries; + purge_entry++) { + nodeoff = fdt_path_offset(fdt, purge_entry->node_path); + if (nodeoff < 0) { + log_debug("Error (%d) getting node offset for %s\n", + nodeoff, purge_entry->node_path); + continue; + } + + if (purge_entry->prop) { + err = fdt_delprop(fdt, nodeoff, purge_entry->prop); + if (err < 0 && err != -FDT_ERR_NOTFOUND) { + log_debug("Error (%d) deleting %s\n", + err, purge_entry->prop); + goto out; + } + } else { + err = fdt_del_node(fdt, nodeoff); + if (err) { + log_debug("Error (%d) trying to delete node %s\n", + err, purge_entry->node_path); + goto out; + } + } + } + +out: + return err; +} +EVENT_SPY(EVT_FT_FIXUP, dt_non_compliant_purge);
On 8/26/23 11:06, Sughosh Ganu wrote:
Add a function which is registered to spy for a EVT_FT_FIXUP event, and removes the non upstreamed nodes and properties from the devicetree before it gets passed to the OS.
This allows removing entire nodes, or specific properties under nodes from the devicetree. The required nodes and properties can be registered for removal through the DT_NON_COMPLIANT_PURGE and DT_NON_COMPLIANT_PURGE_LIST macros.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
include/dt-structs.h | 11 +++++++ lib/Makefile | 1 + lib/dt_purge.c | 73 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 lib/dt_purge.c
diff --git a/include/dt-structs.h b/include/dt-structs.h index fa1622cb1d..f535c60471 100644 --- a/include/dt-structs.h +++ b/include/dt-structs.h @@ -57,3 +57,14 @@ struct phandle_2_arg { #endif
#endif
+struct dt_non_compliant_purge {
- const char *node_path;
- const char *prop;
+};
+#define DT_NON_COMPLIANT_PURGE(__name) \
- ll_entry_declare(struct dt_non_compliant_purge, __name, dt_purge)
+#define DT_NON_COMPLIANT_PURGE_LIST(__name) \
- ll_entry_declare_list(struct dt_non_compliant_purge, __name, dt_purge)
diff --git a/lib/Makefile b/lib/Makefile index 8d8ccc8bbc..82a906daa0 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -37,6 +37,7 @@ endif obj-y += crc8.o obj-y += crc16.o obj-y += crc16-ccitt.o +obj-y += dt_purge.o obj-$(CONFIG_ERRNO_STR) += errno_str.o obj-$(CONFIG_FIT) += fdtdec_common.o obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o diff --git a/lib/dt_purge.c b/lib/dt_purge.c new file mode 100644 index 0000000000..f893ba9796 --- /dev/null +++ b/lib/dt_purge.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) 2023, Linaro Limited
- */
+#include <dt-structs.h> +#include <event.h> +#include <linker_lists.h>
+#include <linux/libfdt.h>
+/**
- dt_non_compliant_purge() - Remove non-upstreamed nodes and properties
from the DT
- @ctx: Context for event
- @event: Event to process
- Iterate through an array of DT nodes and properties, and remove them
- from the device-tree before the DT gets handed over to the kernel.
- These are nodes and properties which do not have upstream bindings
- and need to be purged before being handed over to the kernel.
- If both the node and property are specified, delete the property. If
- only the node is specified, delete the entire node, including it's
- subnodes, if any.
- Return: 0 if OK, -ve on error
- */
+static int dt_non_compliant_purge(void *ctx, struct event *event) +{
- int nodeoff = 0;
- int err = 0;
- void *fdt;
- const struct event_ft_fixup *fixup = &event->data.ft_fixup;
- struct dt_non_compliant_purge *purge_entry;
- struct dt_non_compliant_purge *purge_start =
ll_entry_start(struct dt_non_compliant_purge, dt_purge);- int nentries = ll_entry_count(struct dt_non_compliant_purge, dt_purge);
- if (fixup->images)
return 0;- fdt = fixup->tree.fdt;
- for (purge_entry = purge_start; purge_entry != purge_start + nentries;
purge_entry++) {nodeoff = fdt_path_offset(fdt, purge_entry->node_path);if (nodeoff < 0) {log_debug("Error (%d) getting node offset for %s\n",nodeoff, purge_entry->node_path);continue;}if (purge_entry->prop) {err = fdt_delprop(fdt, nodeoff, purge_entry->prop);if (err < 0 && err != -FDT_ERR_NOTFOUND) {log_debug("Error (%d) deleting %s\n",err, purge_entry->prop);goto out;}} else {err = fdt_del_node(fdt, nodeoff);if (err) {log_debug("Error (%d) trying to delete node %s\n",err, purge_entry->node_path);goto out;}}- }
+out:
- return err;
+} +EVENT_SPY(EVT_FT_FIXUP, dt_non_compliant_purge);
This may interfere with other fixup code requiring U-Boot specific properties like bootmeth_vbe_ft_fixup().
We should ensure that call dt_non_compliant_purge() is the last fixup action. Please, consider a separate event.
This code does not check if the property or node complies to the device tree scheme or not. So why not call the function dt_purge()?
Best regards
Heinrich
On Sat, 26 Aug 2023 at 15:51, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/26/23 11:06, Sughosh Ganu wrote:
Add a function which is registered to spy for a EVT_FT_FIXUP event, and removes the non upstreamed nodes and properties from the devicetree before it gets passed to the OS.
This allows removing entire nodes, or specific properties under nodes from the devicetree. The required nodes and properties can be registered for removal through the DT_NON_COMPLIANT_PURGE and DT_NON_COMPLIANT_PURGE_LIST macros.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
include/dt-structs.h | 11 +++++++ lib/Makefile | 1 + lib/dt_purge.c | 73 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 lib/dt_purge.c
diff --git a/include/dt-structs.h b/include/dt-structs.h index fa1622cb1d..f535c60471 100644 --- a/include/dt-structs.h +++ b/include/dt-structs.h @@ -57,3 +57,14 @@ struct phandle_2_arg { #endif
#endif
+struct dt_non_compliant_purge {
const char *node_path;const char *prop;+};
+#define DT_NON_COMPLIANT_PURGE(__name) \
ll_entry_declare(struct dt_non_compliant_purge, __name, dt_purge)+#define DT_NON_COMPLIANT_PURGE_LIST(__name) \
ll_entry_declare_list(struct dt_non_compliant_purge, __name, dt_purge)diff --git a/lib/Makefile b/lib/Makefile index 8d8ccc8bbc..82a906daa0 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -37,6 +37,7 @@ endif obj-y += crc8.o obj-y += crc16.o obj-y += crc16-ccitt.o +obj-y += dt_purge.o obj-$(CONFIG_ERRNO_STR) += errno_str.o obj-$(CONFIG_FIT) += fdtdec_common.o obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o diff --git a/lib/dt_purge.c b/lib/dt_purge.c new file mode 100644 index 0000000000..f893ba9796 --- /dev/null +++ b/lib/dt_purge.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) 2023, Linaro Limited
- */
+#include <dt-structs.h> +#include <event.h> +#include <linker_lists.h>
+#include <linux/libfdt.h>
+/**
- dt_non_compliant_purge() - Remove non-upstreamed nodes and properties
from the DT
- @ctx: Context for event
- @event: Event to process
- Iterate through an array of DT nodes and properties, and remove them
- from the device-tree before the DT gets handed over to the kernel.
- These are nodes and properties which do not have upstream bindings
- and need to be purged before being handed over to the kernel.
- If both the node and property are specified, delete the property. If
- only the node is specified, delete the entire node, including it's
- subnodes, if any.
- Return: 0 if OK, -ve on error
- */
+static int dt_non_compliant_purge(void *ctx, struct event *event) +{
int nodeoff = 0;int err = 0;void *fdt;const struct event_ft_fixup *fixup = &event->data.ft_fixup;struct dt_non_compliant_purge *purge_entry;struct dt_non_compliant_purge *purge_start =ll_entry_start(struct dt_non_compliant_purge, dt_purge);int nentries = ll_entry_count(struct dt_non_compliant_purge, dt_purge);if (fixup->images)return 0;fdt = fixup->tree.fdt;for (purge_entry = purge_start; purge_entry != purge_start + nentries;purge_entry++) {nodeoff = fdt_path_offset(fdt, purge_entry->node_path);if (nodeoff < 0) {log_debug("Error (%d) getting node offset for %s\n",nodeoff, purge_entry->node_path);continue;}if (purge_entry->prop) {err = fdt_delprop(fdt, nodeoff, purge_entry->prop);if (err < 0 && err != -FDT_ERR_NOTFOUND) {log_debug("Error (%d) deleting %s\n",err, purge_entry->prop);goto out;}} else {err = fdt_del_node(fdt, nodeoff);if (err) {log_debug("Error (%d) trying to delete node %s\n",err, purge_entry->node_path);goto out;}}}+out:
return err;+} +EVENT_SPY(EVT_FT_FIXUP, dt_non_compliant_purge);
This may interfere with other fixup code requiring U-Boot specific properties like bootmeth_vbe_ft_fixup().
Okay, I was thinking if we can have a flag to indicate which event handler is to be run. I will check this, and add a separate event if this solution turns to be overly complex.
We should ensure that call dt_non_compliant_purge() is the last fixup action. Please, consider a separate event.
Will check.
This code does not check if the property or node complies to the device tree scheme or not. So why not call the function dt_purge()?
Okay
-sughosh
On 8/26/23 11:06, Sughosh Ganu wrote:
Add a function which is registered to spy for a EVT_FT_FIXUP event, and removes the non upstreamed nodes and properties from the devicetree before it gets passed to the OS.
This allows removing entire nodes, or specific properties under nodes from the devicetree. The required nodes and properties can be registered for removal through the DT_NON_COMPLIANT_PURGE and DT_NON_COMPLIANT_PURGE_LIST macros.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
include/dt-structs.h | 11 +++++++ lib/Makefile | 1 + lib/dt_purge.c | 73 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 lib/dt_purge.c
diff --git a/include/dt-structs.h b/include/dt-structs.h index fa1622cb1d..f535c60471 100644 --- a/include/dt-structs.h +++ b/include/dt-structs.h @@ -57,3 +57,14 @@ struct phandle_2_arg { #endif
#endif
+struct dt_non_compliant_purge {
- const char *node_path;
- const char *prop;
+};
+#define DT_NON_COMPLIANT_PURGE(__name) \
- ll_entry_declare(struct dt_non_compliant_purge, __name, dt_purge)
+#define DT_NON_COMPLIANT_PURGE_LIST(__name) \
- ll_entry_declare_list(struct dt_non_compliant_purge, __name, dt_purge)
diff --git a/lib/Makefile b/lib/Makefile index 8d8ccc8bbc..82a906daa0 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -37,6 +37,7 @@ endif obj-y += crc8.o obj-y += crc16.o obj-y += crc16-ccitt.o +obj-y += dt_purge.o
SPL can be the last boot stage (e.g. for Falcon Mode). So placing this under 'ifndef CONFIG_SPL_BUILD' is not correct.
You need some logic that identifies into which boot stage this code belongs, e.g. use obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT).
Best regards
Heinrich
obj-$(CONFIG_ERRNO_STR) += errno_str.o obj-$(CONFIG_FIT) += fdtdec_common.o obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o diff --git a/lib/dt_purge.c b/lib/dt_purge.c new file mode 100644 index 0000000000..f893ba9796 --- /dev/null +++ b/lib/dt_purge.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) 2023, Linaro Limited
- */
+#include <dt-structs.h> +#include <event.h> +#include <linker_lists.h>
+#include <linux/libfdt.h>
+/**
- dt_non_compliant_purge() - Remove non-upstreamed nodes and properties
from the DT
- @ctx: Context for event
- @event: Event to process
- Iterate through an array of DT nodes and properties, and remove them
- from the device-tree before the DT gets handed over to the kernel.
- These are nodes and properties which do not have upstream bindings
- and need to be purged before being handed over to the kernel.
- If both the node and property are specified, delete the property. If
- only the node is specified, delete the entire node, including it's
- subnodes, if any.
- Return: 0 if OK, -ve on error
- */
+static int dt_non_compliant_purge(void *ctx, struct event *event) +{
- int nodeoff = 0;
- int err = 0;
- void *fdt;
- const struct event_ft_fixup *fixup = &event->data.ft_fixup;
- struct dt_non_compliant_purge *purge_entry;
- struct dt_non_compliant_purge *purge_start =
ll_entry_start(struct dt_non_compliant_purge, dt_purge);- int nentries = ll_entry_count(struct dt_non_compliant_purge, dt_purge);
- if (fixup->images)
return 0;- fdt = fixup->tree.fdt;
- for (purge_entry = purge_start; purge_entry != purge_start + nentries;
purge_entry++) {nodeoff = fdt_path_offset(fdt, purge_entry->node_path);if (nodeoff < 0) {log_debug("Error (%d) getting node offset for %s\n",nodeoff, purge_entry->node_path);continue;}if (purge_entry->prop) {err = fdt_delprop(fdt, nodeoff, purge_entry->prop);if (err < 0 && err != -FDT_ERR_NOTFOUND) {log_debug("Error (%d) deleting %s\n",err, purge_entry->prop);goto out;}} else {err = fdt_del_node(fdt, nodeoff);if (err) {log_debug("Error (%d) trying to delete node %s\n",err, purge_entry->node_path);goto out;}}- }
+out:
- return err;
+} +EVENT_SPY(EVT_FT_FIXUP, dt_non_compliant_purge);
On Sat, 26 Aug 2023 at 16:09, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/26/23 11:06, Sughosh Ganu wrote:
Add a function which is registered to spy for a EVT_FT_FIXUP event, and removes the non upstreamed nodes and properties from the devicetree before it gets passed to the OS.
This allows removing entire nodes, or specific properties under nodes from the devicetree. The required nodes and properties can be registered for removal through the DT_NON_COMPLIANT_PURGE and DT_NON_COMPLIANT_PURGE_LIST macros.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
include/dt-structs.h | 11 +++++++ lib/Makefile | 1 + lib/dt_purge.c | 73 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 lib/dt_purge.c
diff --git a/include/dt-structs.h b/include/dt-structs.h index fa1622cb1d..f535c60471 100644 --- a/include/dt-structs.h +++ b/include/dt-structs.h @@ -57,3 +57,14 @@ struct phandle_2_arg { #endif
#endif
+struct dt_non_compliant_purge {
const char *node_path;const char *prop;+};
+#define DT_NON_COMPLIANT_PURGE(__name) \
ll_entry_declare(struct dt_non_compliant_purge, __name, dt_purge)+#define DT_NON_COMPLIANT_PURGE_LIST(__name) \
ll_entry_declare_list(struct dt_non_compliant_purge, __name, dt_purge)diff --git a/lib/Makefile b/lib/Makefile index 8d8ccc8bbc..82a906daa0 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -37,6 +37,7 @@ endif obj-y += crc8.o obj-y += crc16.o obj-y += crc16-ccitt.o +obj-y += dt_purge.o
SPL can be the last boot stage (e.g. for Falcon Mode). So placing this under 'ifndef CONFIG_SPL_BUILD' is not correct.
You need some logic that identifies into which boot stage this code belongs, e.g. use obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT).
Okay. Will check and add this under the suggested config symbol.
-sughosh
Best regards
Heinrich
obj-$(CONFIG_ERRNO_STR) += errno_str.o obj-$(CONFIG_FIT) += fdtdec_common.o obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o diff --git a/lib/dt_purge.c b/lib/dt_purge.c new file mode 100644 index 0000000000..f893ba9796 --- /dev/null +++ b/lib/dt_purge.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) 2023, Linaro Limited
- */
+#include <dt-structs.h> +#include <event.h> +#include <linker_lists.h>
+#include <linux/libfdt.h>
+/**
- dt_non_compliant_purge() - Remove non-upstreamed nodes and properties
from the DT
- @ctx: Context for event
- @event: Event to process
- Iterate through an array of DT nodes and properties, and remove them
- from the device-tree before the DT gets handed over to the kernel.
- These are nodes and properties which do not have upstream bindings
- and need to be purged before being handed over to the kernel.
- If both the node and property are specified, delete the property. If
- only the node is specified, delete the entire node, including it's
- subnodes, if any.
- Return: 0 if OK, -ve on error
- */
+static int dt_non_compliant_purge(void *ctx, struct event *event) +{
int nodeoff = 0;int err = 0;void *fdt;const struct event_ft_fixup *fixup = &event->data.ft_fixup;struct dt_non_compliant_purge *purge_entry;struct dt_non_compliant_purge *purge_start =ll_entry_start(struct dt_non_compliant_purge, dt_purge);int nentries = ll_entry_count(struct dt_non_compliant_purge, dt_purge);if (fixup->images)return 0;fdt = fixup->tree.fdt;for (purge_entry = purge_start; purge_entry != purge_start + nentries;purge_entry++) {nodeoff = fdt_path_offset(fdt, purge_entry->node_path);if (nodeoff < 0) {log_debug("Error (%d) getting node offset for %s\n",nodeoff, purge_entry->node_path);continue;}if (purge_entry->prop) {err = fdt_delprop(fdt, nodeoff, purge_entry->prop);if (err < 0 && err != -FDT_ERR_NOTFOUND) {log_debug("Error (%d) deleting %s\n",err, purge_entry->prop);goto out;}} else {err = fdt_del_node(fdt, nodeoff);if (err) {log_debug("Error (%d) trying to delete node %s\n",err, purge_entry->node_path);goto out;}}}+out:
return err;+} +EVENT_SPY(EVT_FT_FIXUP, dt_non_compliant_purge);
On Sat, Aug 26, 2023 at 02:36:29PM +0530, Sughosh Ganu wrote:
Add a function which is registered to spy for a EVT_FT_FIXUP event, and removes the non upstreamed nodes and properties from the devicetree before it gets passed to the OS.
This allows removing entire nodes, or specific properties under nodes from the devicetree. The required nodes and properties can be registered for removal through the DT_NON_COMPLIANT_PURGE and DT_NON_COMPLIANT_PURGE_LIST macros.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
So, the conceptual problem here is that while we do need to have a way to purge nodes and properties that have been rejected by upstream, we also need to ensure an upstream attempt was made. To that end, how I envision things is: - When we build the docs, something under doc/develop/ has a page / section for each binding we're purging, which links to and summarizes why it's not appropriate for upstream. - It's non-default to purge said nodes. - We at least make checkpatch / et al complain about new purges being added, and CI failing on the number of purges increasing without also increasing the number of allowed purges.
The FWU metadata devicetree node points to the device which stores the metadata structure. This node is relevant only in U-Boot, and is not to be passed to the OS. Register for purging this node from the devicetree, before passing it to the OS.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- drivers/fwu-mdata/fwu-mdata-uclass.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c b/drivers/fwu-mdata/fwu-mdata-uclass.c index 0a8edaaa41..71411d9c19 100644 --- a/drivers/fwu-mdata/fwu-mdata-uclass.c +++ b/drivers/fwu-mdata/fwu-mdata-uclass.c @@ -7,6 +7,7 @@
#include <common.h> #include <dm.h> +#include <dt-structs.h> #include <efi_loader.h> #include <fwu.h> #include <fwu_mdata.h> @@ -53,3 +54,7 @@ UCLASS_DRIVER(fwu_mdata) = { .id = UCLASS_FWU_MDATA, .name = "fwu-mdata", }; + +DT_NON_COMPLIANT_PURGE(fwu_mdata) = { + .node_path = "/fwu-mdata", +};
The capsule-key property contains the public key in the form of an EFI Signature List(ESL) structure. This property is relevant only in U-Boot, and is not to be passed to the OS. Register for purging this property from the devicetree, before passing it to the OS.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- lib/efi_loader/efi_capsule.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index af8a2ee940..5c6c87458f 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -9,6 +9,7 @@ #define LOG_CATEGORY LOGC_EFI
#include <common.h> +#include <dt-structs.h> #include <efi_loader.h> #include <efi_variable.h> #include <env.h> @@ -403,6 +404,12 @@ out:
return status; } + +DT_NON_COMPLIANT_PURGE(capsule_key) = { + .node_path = "/signature", + .prop = "capsule-key", +}; + #endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
static __maybe_unused bool fwu_empty_capsule(struct efi_capsule_header *capsule)
The bootefi command passes the devicetree to the kernel through the EFI config table. Call the event handlers for fixing the devicetree before jumping into the kernel. This removes any devicetree nodes and/or properties that are specific only to U-Boot, and are not to be passed to the OS.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- cmd/bootefi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index f73d6eb0e2..c359a46ec4 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -237,6 +237,23 @@ static void *get_config_table(const efi_guid_t *guid) return NULL; }
+/** + * event_notify_dt_fixup() - call ft_fixup event + * + * @fdt: address of the device tree to be passed to the kernel + * through the configuration table + * Return: None + */ +static void event_notify_dt_fixup(void *fdt) +{ + int ret; + struct event_ft_fixup fixup = {0}; + + fixup.tree.fdt = fdt; + ret = event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup)); + if (ret) + printf("Error: %d: FDT Fixup event failed\n", ret); +} #endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
/** @@ -318,6 +335,7 @@ efi_status_t efi_install_fdt(void *fdt) efi_carve_out_dt_rsv(fdt);
efi_try_purge_kaslr_seed(fdt); + event_notify_dt_fixup(fdt);
if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) { ret = efi_tcg2_measure_dtb(fdt);
On 8/26/23 11:06, Sughosh Ganu wrote:
The bootefi command passes the devicetree to the kernel through the EFI config table. Call the event handlers for fixing the devicetree before jumping into the kernel. This removes any devicetree nodes and/or properties that are specific only to U-Boot, and are not to be passed to the OS.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
cmd/bootefi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index f73d6eb0e2..c359a46ec4 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -237,6 +237,23 @@ static void *get_config_table(const efi_guid_t *guid) return NULL; }
+/**
- event_notify_dt_fixup() - call ft_fixup event
- @fdt: address of the device tree to be passed to the kernel
through the configuration table
- Return: None
- */
+static void event_notify_dt_fixup(void *fdt) +{
- int ret;
- struct event_ft_fixup fixup = {0};
- fixup.tree.fdt = fdt;
- ret = event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup));
- if (ret)
printf("Error: %d: FDT Fixup event failed\n", ret);+} #endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
/** @@ -318,6 +335,7 @@ efi_status_t efi_install_fdt(void *fdt) efi_carve_out_dt_rsv(fdt);
efi_try_purge_kaslr_seed(fdt);
- event_notify_dt_fixup(fdt);
The event is already triggered in image_setup_libfdt(). Don't trigger it twice.
Best regards
Heinrich
if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) { ret = efi_tcg2_measure_dtb(fdt);
On Sat, 26 Aug 2023 at 15:57, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/26/23 11:06, Sughosh Ganu wrote:
The bootefi command passes the devicetree to the kernel through the EFI config table. Call the event handlers for fixing the devicetree before jumping into the kernel. This removes any devicetree nodes and/or properties that are specific only to U-Boot, and are not to be passed to the OS.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
cmd/bootefi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index f73d6eb0e2..c359a46ec4 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -237,6 +237,23 @@ static void *get_config_table(const efi_guid_t *guid) return NULL; }
+/**
- event_notify_dt_fixup() - call ft_fixup event
- @fdt: address of the device tree to be passed to the kernel
through the configuration table
- Return: None
- */
+static void event_notify_dt_fixup(void *fdt) +{
int ret;struct event_ft_fixup fixup = {0};fixup.tree.fdt = fdt;ret = event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup));if (ret)printf("Error: %d: FDT Fixup event failed\n", ret);+} #endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
/** @@ -318,6 +335,7 @@ efi_status_t efi_install_fdt(void *fdt) efi_carve_out_dt_rsv(fdt);
efi_try_purge_kaslr_seed(fdt);
event_notify_dt_fixup(fdt);The event is already triggered in image_setup_libfdt(). Don't trigger it twice.
The reason I put an explicit event_notify call is because the image_setup_libfdt() call only calls the ft_fixup handlers if the livetree is not active. So the fixup handlers would not be called on platforms that enable livetree. Although I'm not sure if livetree should be disabled before the ft fixup has to happen, or platforms that need ft fixup should not enable OF_LIVE. Disabling the livetree is not happening now, so I am not sure how the fixup event should work on platforms which have OF_LIVE enabled.
-sughosh
Best regards
Heinrich
if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) { ret = efi_tcg2_measure_dtb(fdt);
On 28.08.23 11:32, Sughosh Ganu wrote:
On Sat, 26 Aug 2023 at 15:57, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/26/23 11:06, Sughosh Ganu wrote:
The bootefi command passes the devicetree to the kernel through the EFI config table. Call the event handlers for fixing the devicetree before jumping into the kernel. This removes any devicetree nodes and/or properties that are specific only to U-Boot, and are not to be passed to the OS.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
cmd/bootefi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index f73d6eb0e2..c359a46ec4 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -237,6 +237,23 @@ static void *get_config_table(const efi_guid_t *guid) return NULL; }
+/**
- event_notify_dt_fixup() - call ft_fixup event
- @fdt: address of the device tree to be passed to the kernel
through the configuration table
- Return: None
- */
+static void event_notify_dt_fixup(void *fdt) +{
int ret;struct event_ft_fixup fixup = {0};fixup.tree.fdt = fdt;ret = event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup));if (ret)printf("Error: %d: FDT Fixup event failed\n", ret);+} #endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
/** @@ -318,6 +335,7 @@ efi_status_t efi_install_fdt(void *fdt) efi_carve_out_dt_rsv(fdt);
efi_try_purge_kaslr_seed(fdt);
event_notify_dt_fixup(fdt);The event is already triggered in image_setup_libfdt(). Don't trigger it twice.
The reason I put an explicit event_notify call is because the image_setup_libfdt() call only calls the ft_fixup handlers if the livetree is not active. So the fixup handlers would not be called on platforms that enable livetree. Although I'm not sure if livetree should be disabled before the ft fixup has to happen, or platforms that need ft fixup should not enable OF_LIVE. Disabling the livetree is not happening now, so I am not sure how the fixup event should work on platforms which have OF_LIVE enabled.
We still must not call the same event twice. We further must ensure that our fix-up is called last (e.g. by calling it zzz_something) and not before adding VBE nodes.
Is there any need to use the event mechanism here?
@Simon: None of the other function calls in image_setup_libfdt() depend on of_live_active(). I think it would be preferable to move that check into the listeners. If instead of of_live_active() you could use a configuration setting for the decision, gcc could eliminate a lot of code.
We definitely need better descriptions in include/event.h and a man-page generated from that include:
EVT_NONE undescribed EVT_TEST undescribed EVT_DM_POST_INIT_F undescribed EVT_DM_POST_INIT_R undescribed EVT_DM_PRE_PROBE Device is about to be probed EVT_DM_POST_PROBE undescribed EVT_DM_PRE_REMOVE undescribed EVT_DM_POST_REMOVE undescribed EVT_MISC_INIT_F undescribed EVT_FPGA_LOAD undescribed EVT_FT_FIXUP undescribed EVT_MAIN_LOOP undescribed EVT_COUNT undescribed
Best regards
Heinrich
Hi Heinrich,
On Mon, 28 Aug 2023 at 05:02, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 28.08.23 11:32, Sughosh Ganu wrote:
On Sat, 26 Aug 2023 at 15:57, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/26/23 11:06, Sughosh Ganu wrote:
The bootefi command passes the devicetree to the kernel through the EFI config table. Call the event handlers for fixing the devicetree before jumping into the kernel. This removes any devicetree nodes and/or properties that are specific only to U-Boot, and are not to be passed to the OS.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
cmd/bootefi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index f73d6eb0e2..c359a46ec4 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -237,6 +237,23 @@ static void *get_config_table(const efi_guid_t *guid) return NULL; }
+/**
- event_notify_dt_fixup() - call ft_fixup event
- @fdt: address of the device tree to be passed to the kernel
through the configuration table
- Return: None
- */
+static void event_notify_dt_fixup(void *fdt) +{
int ret;struct event_ft_fixup fixup = {0};fixup.tree.fdt = fdt;ret = event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup));if (ret)printf("Error: %d: FDT Fixup event failed\n", ret);+} #endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
/** @@ -318,6 +335,7 @@ efi_status_t efi_install_fdt(void *fdt) efi_carve_out_dt_rsv(fdt);
efi_try_purge_kaslr_seed(fdt);
event_notify_dt_fixup(fdt);The event is already triggered in image_setup_libfdt(). Don't trigger it twice.
The reason I put an explicit event_notify call is because the image_setup_libfdt() call only calls the ft_fixup handlers if the livetree is not active. So the fixup handlers would not be called on platforms that enable livetree. Although I'm not sure if livetree should be disabled before the ft fixup has to happen, or platforms that need ft fixup should not enable OF_LIVE. Disabling the livetree is not happening now, so I am not sure how the fixup event should work on platforms which have OF_LIVE enabled.
We still must not call the same event twice. We further must ensure that our fix-up is called last (e.g. by calling it zzz_something) and not before adding VBE nodes.
Is there any need to use the event mechanism here?
@Simon: None of the other function calls in image_setup_libfdt() depend on of_live_active(). I think it would be preferable to move that check into the listeners. If instead of of_live_active() you could use a configuration setting for the decision, gcc could eliminate a lot of code.
Firstly, of_live_active() should check CONFIG_IS_ENABLED(OF_LIVE) first. I noticed this over the weekend when looking at something else.
The original code was due to a limitation with U-Boot's ofnode implementation, in that it did not support multiple devicetrees without OF_LIVE. We need to support at least two, since we have the 'control' DTB and the one being set up to boot the OS.
Anyway, that limitation can be removed, so long as CONFIG_MULTI_DTB_FIT is set. In other words, we should enable MULTI_DTB_FIT if !OF_LIVE and all will be well.
We definitely need better descriptions in include/event.h and a man-page generated from that include:
EVT_NONE undescribed EVT_TEST undescribed EVT_DM_POST_INIT_F undescribed EVT_DM_POST_INIT_R undescribed EVT_DM_PRE_PROBE Device is about to be probed EVT_DM_POST_PROBE undescribed EVT_DM_PRE_REMOVE undescribed EVT_DM_POST_REMOVE undescribed EVT_MISC_INIT_F undescribed EVT_FPGA_LOAD undescribed EVT_FT_FIXUP undescribed EVT_MAIN_LOOP undescribed EVT_COUNT undescribed
Yes indeed. I added some for the latest patch in this area.
https://patchwork.ozlabs.org/project/uboot/patch/20230822031705.1340941-15-s...
Regards, Simon
Add a document explaining the need for removal of non-compliant devicetree nodes and properties. Also describe in brief, the macros that can be used for this removal.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- .../devicetree/dt_non_compliant_purge.rst | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst new file mode 100644 index 0000000000..c3a8feab5b --- /dev/null +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst @@ -0,0 +1,64 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Removal of non-compliant nodes and properties +============================================= + +The devicetree used in U-Boot might contain nodes and properties which +are specific only to U-Boot, and are not necessarily being used to +describe hardware but to pass information to U-Boot. An example of +such a property would be the public key being passed to U-Boot for +verification. + +This devicetree can then be passed to the OS. Since certain nodes and +properties are not really describing hardware, and more importantly, +these are only relevant to U-Boot, bindings for these cannot be +upstreamed into the devicetree repository. There have been instances +of attempts being made to upstream such bindings, and these deemed not +fit for upstreaming. Not having a binding for these nodes and +properties means that the devicetree fails the schema compliance tests +[1]. This also means that the platform cannot get certifications like +SystemReady [2] which, among other things require a devicetree which +passes the schema compliance tests. + +For such nodes and properties, it has been suggested by the devicetree +maintainers that the right thing to do is to remove them from the +devicetree before it gets passed on to the OS [3]. + +Removing nodes/properties +------------------------- + +In U-Boot, this is been done through adding information on such nodes +and properties in a list. The entire node can be deleted, or a +specific property under a node can be deleted. The list of such nodes +and properties is generated at compile time, and the function to purge +these can be invoked through a EVT_FT_FIXUP event notify call. + +For deleting a node, this can be done by declaring a macro:: + + DT_NON_COMPLIANT_PURGE(fwu_mdata) = { + .node_path = "/fwu-mdata", + }; + +Similarly, for deleting a property under a node, that can be done by +specifying the property name:: + + DT_NON_COMPLIANT_PURGE(capsule_key) = { + .node_path = "/signature", + .prop = "capsule-key", + }; + +In the first example, the entire node with path /fwu-mdata will be +removed. In the second example, the property capsule-key +under /signature node will be removed. + +Similarly, a list of nodes and properties can be specified using the +following macro:: + + DT_NON_COMPLIANT_PURGE_LIST(foo) = { + { .node_path = "/some_node", .prop = "some_bar" }, + { .node_path = "/some_node" }, + }; + +[1] - https://github.com/devicetree-org/dt-schema +[2] - https://www.arm.com/architecture/system-architectures/systemready-certificat... +[3] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
On 8/26/23 11:06, Sughosh Ganu wrote:
Add a document explaining the need for removal of non-compliant devicetree nodes and properties. Also describe in brief, the macros that can be used for this removal.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Thanks for properly documenting the change.
Warning, treated as error: doc/develop/devicetree/dt_non_compliant_purge.rst: document isn't included in any toctree
Please, add the document to doc/develop/devicetree/index.rst
Please, run make htmldocs before resubmitting.
.../devicetree/dt_non_compliant_purge.rst | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst new file mode 100644 index 0000000000..c3a8feab5b --- /dev/null +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst @@ -0,0 +1,64 @@ +.. SPDX-License-Identifier: GPL-2.0+
+Removal of non-compliant nodes and properties +=============================================
+The devicetree used in U-Boot might contain nodes and properties which +are specific only to U-Boot, and are not necessarily being used to +describe hardware but to pass information to U-Boot. An example of +such a property would be the public key being passed to U-Boot for +verification.
+This devicetree can then be passed to the OS. Since certain nodes and +properties are not really describing hardware, and more importantly, +these are only relevant to U-Boot, bindings for these cannot be +upstreamed into the devicetree repository. There have been instances +of attempts being made to upstream such bindings, and these deemed not
but these were not deemed fit
+fit for upstreaming. Not having a binding for these nodes and +properties means that the devicetree fails the schema compliance tests +[1]. This also means that the platform cannot get certifications like +SystemReady [2] which, among other things require a devicetree which
%s/require/requires/
+passes the schema compliance tests.
+For such nodes and properties, it has been suggested by the devicetree +maintainers that the right thing to do is to remove them from the
%s/that the right thing to do is//
+devicetree before it gets passed on to the OS [3].
%s/on to/to/
+Removing nodes/properties +-------------------------
+In U-Boot, this is been done through adding information on such nodes
%s/is been done through/is done by/
+and properties in a list. The entire node can be deleted, or a +specific property under a node can be deleted. The list of such nodes +and properties is generated at compile time, and the function to purge +these can be invoked through a EVT_FT_FIXUP event notify call.
+For deleting a node, this can be done by declaring a macro::
- DT_NON_COMPLIANT_PURGE(fwu_mdata) = {
.node_path = "/fwu-mdata",- };
Where should such a macro be placed?
+Similarly, for deleting a property under a node, that can be done by +specifying the property name::
- DT_NON_COMPLIANT_PURGE(capsule_key) = {
.node_path = "/signature",.prop = "capsule-key",- };
Why is capsule_key needed twice here? What would be the effect of:
DT_NON_COMPLIANT_PURGE(voodoo) = { .node_path = "/signature", .prop = "capsule-key", };
+In the first example, the entire node with path /fwu-mdata will be +removed. In the second example, the property capsule-key +under /signature node will be removed.
+Similarly, a list of nodes and properties can be specified using the +following macro::
- DT_NON_COMPLIANT_PURGE_LIST(foo) = {
What does foo refer to here? Can this parameter be eliminated from the syntax?
Will the application of the removals be sorted alphabetically by this argument?
What will happen if a property or node does not exist at time of removal? Hopefully that does not stop the boot process but is silently ignored.
{ .node_path = "/some_node", .prop = "some_bar" },{ .node_path = "/some_node" },- };
Why do you need the first element in your example? Wouldn't deleting /some/node imply that all its children are deleted?
Why do we need separate properties .node_path and .prop. It would be less effort to write:
DT_NON_COMPLIANT_PURGE_LIST() = { "/some_node_1/some_property_1", "/some_node_1/some_property_2", "/some_node_2", }
I assume the macro does its job irrespective of non-compliance. Why not call it DT_PURGE()?
Best regards
Heinrich
+[1] - https://github.com/devicetree-org/dt-schema +[2] - https://www.arm.com/architecture/system-architectures/systemready-certificat... +[3] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
On Sat, 26 Aug 2023 at 15:35, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/26/23 11:06, Sughosh Ganu wrote:
Add a document explaining the need for removal of non-compliant devicetree nodes and properties. Also describe in brief, the macros that can be used for this removal.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Thanks for properly documenting the change.
Warning, treated as error: doc/develop/devicetree/dt_non_compliant_purge.rst: document isn't included in any toctree
Please, add the document to doc/develop/devicetree/index.rst
Please, run make htmldocs before resubmitting.
.../devicetree/dt_non_compliant_purge.rst | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst new file mode 100644 index 0000000000..c3a8feab5b --- /dev/null +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst @@ -0,0 +1,64 @@ +.. SPDX-License-Identifier: GPL-2.0+
+Removal of non-compliant nodes and properties +=============================================
+The devicetree used in U-Boot might contain nodes and properties which +are specific only to U-Boot, and are not necessarily being used to +describe hardware but to pass information to U-Boot. An example of +such a property would be the public key being passed to U-Boot for +verification.
+This devicetree can then be passed to the OS. Since certain nodes and +properties are not really describing hardware, and more importantly, +these are only relevant to U-Boot, bindings for these cannot be +upstreamed into the devicetree repository. There have been instances +of attempts being made to upstream such bindings, and these deemed not
but these were not deemed fit
+fit for upstreaming. Not having a binding for these nodes and +properties means that the devicetree fails the schema compliance tests +[1]. This also means that the platform cannot get certifications like +SystemReady [2] which, among other things require a devicetree which
%s/require/requires/
+passes the schema compliance tests.
+For such nodes and properties, it has been suggested by the devicetree +maintainers that the right thing to do is to remove them from the
%s/that the right thing to do is//
+devicetree before it gets passed on to the OS [3].
%s/on to/to/
+Removing nodes/properties +-------------------------
+In U-Boot, this is been done through adding information on such nodes
%s/is been done through/is done by/
+and properties in a list. The entire node can be deleted, or a +specific property under a node can be deleted. The list of such nodes +and properties is generated at compile time, and the function to purge +these can be invoked through a EVT_FT_FIXUP event notify call.
+For deleting a node, this can be done by declaring a macro::
DT_NON_COMPLIANT_PURGE(fwu_mdata) = {.node_path = "/fwu-mdata",};Where should such a macro be placed?
It should be placed in any file that will get compiled.
+Similarly, for deleting a property under a node, that can be done by +specifying the property name::
DT_NON_COMPLIANT_PURGE(capsule_key) = {.node_path = "/signature",.prop = "capsule-key",};Why is capsule_key needed twice here? What would be the effect of:
DT_NON_COMPLIANT_PURGE(voodoo) = { .node_path = "/signature", .prop = "capsule-key", };
In your above example, voodoo just happens to be the name of the entry that will be created. But the property that will be searched in the devicetree for removal is "capsule-key".
+In the first example, the entire node with path /fwu-mdata will be +removed. In the second example, the property capsule-key +under /signature node will be removed.
+Similarly, a list of nodes and properties can be specified using the +following macro::
DT_NON_COMPLIANT_PURGE_LIST(foo) = {What does foo refer to here? Can this parameter be eliminated from the syntax?
foo is the name of the entry being generated through the ll_entry_declare macro. We do need this parameter for ll_entry_declare to generate a unique entry. Using a common name results in a linker error.
Will the application of the removals be sorted alphabetically by this argument?
I am not sure how the linker arranges the entries in the list. Why does it matter?
What will happen if a property or node does not exist at time of removal? Hopefully that does not stop the boot process but is silently ignored.
Yes, it will not cause the boot to fail.
{ .node_path = "/some_node", .prop = "some_bar" },{ .node_path = "/some_node" },};Why do you need the first element in your example? Wouldn't deleting /some/node imply that all its children are deleted?
Ah, this is a bad example. I should be using a different name for the second entry here. Will fix.
Why do we need separate properties .node_path and .prop. It would be less effort to write:
DT_NON_COMPLIANT_PURGE_LIST() = { "/some_node_1/some_property_1", "/some_node_1/some_property_2", "/some_node_2", }
I assume the macro does its job irrespective of non-compliance. Why not call it DT_PURGE()?
Yes, it can be called DT_PURGE. I was using the term non_compliant simply to highlight the fact that we would need to use this for removal of non-compliant nodes or properties.
-sughosh
Best regards
Heinrich
+[1] - https://github.com/devicetree-org/dt-schema +[2] - https://www.arm.com/architecture/system-architectures/systemready-certificat... +[3] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
Hi Sughosh,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add a document explaining the need for removal of non-compliant devicetree nodes and properties. Also describe in brief, the macros that can be used for this removal.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
.../devicetree/dt_non_compliant_purge.rst | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst new file mode 100644 index 0000000000..c3a8feab5b --- /dev/null +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst @@ -0,0 +1,64 @@ +.. SPDX-License-Identifier: GPL-2.0+
+Removal of non-compliant nodes and properties +=============================================
+The devicetree used in U-Boot might contain nodes and properties which +are specific only to U-Boot, and are not necessarily being used to +describe hardware but to pass information to U-Boot. An example of +such a property would be the public key being passed to U-Boot for +verification.
It has nothing to do with describing hardware. The DT can describe other things too. See the /options node, for example.
Please don't bring this highly misleading language into U-Boot.
+This devicetree can then be passed to the OS. Since certain nodes and +properties are not really describing hardware, and more importantly, +these are only relevant to U-Boot, bindings for these cannot be +upstreamed into the devicetree repository. There have been instances +of attempts being made to upstream such bindings, and these deemed not +fit for upstreaming.
Then either they should not be in U-Boot, or there is a problem with the process.
Not having a binding for these nodes and +properties means that the devicetree fails the schema compliance tests +[1]. This also means that the platform cannot get certifications like +SystemReady [2] which, among other things require a devicetree which +passes the schema compliance tests.
+For such nodes and properties, it has been suggested by the devicetree +maintainers that the right thing to do is to remove them from the +devicetree before it gets passed on to the OS [3].
Hard NAK. If we go this way, then no one will ever have an incentive to do the right thing.
Please send bindings for Linaro's work, instead. If something is entirely U-Boot-specific, then it can go in /options/u-boot but it still must be in the dt-schema.
+Removing nodes/properties +-------------------------
+In U-Boot, this is been done through adding information on such nodes +and properties in a list. The entire node can be deleted, or a +specific property under a node can be deleted. The list of such nodes +and properties is generated at compile time, and the function to purge +these can be invoked through a EVT_FT_FIXUP event notify call.
+For deleting a node, this can be done by declaring a macro::
DT_NON_COMPLIANT_PURGE(fwu_mdata) = {.node_path = "/fwu-mdata",};+Similarly, for deleting a property under a node, that can be done by +specifying the property name::
DT_NON_COMPLIANT_PURGE(capsule_key) = {.node_path = "/signature",.prop = "capsule-key",};+In the first example, the entire node with path /fwu-mdata will be +removed. In the second example, the property capsule-key +under /signature node will be removed.
+Similarly, a list of nodes and properties can be specified using the +following macro::
DT_NON_COMPLIANT_PURGE_LIST(foo) = {{ .node_path = "/some_node", .prop = "some_bar" },{ .node_path = "/some_node" },};+[1] - https://github.com/devicetree-org/dt-schema +[2] - https://www.arm.com/architecture/system-architectures/systemready-certificat...
+[3] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
2.34.1
Regards, Simon
hi Simon,
On Mon, 28 Aug 2023 at 23:25, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add a document explaining the need for removal of non-compliant devicetree nodes and properties. Also describe in brief, the macros that can be used for this removal.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
.../devicetree/dt_non_compliant_purge.rst | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst new file mode 100644 index 0000000000..c3a8feab5b --- /dev/null +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst @@ -0,0 +1,64 @@ +.. SPDX-License-Identifier: GPL-2.0+
+Removal of non-compliant nodes and properties +=============================================
+The devicetree used in U-Boot might contain nodes and properties which +are specific only to U-Boot, and are not necessarily being used to +describe hardware but to pass information to U-Boot. An example of +such a property would be the public key being passed to U-Boot for +verification.
It has nothing to do with describing hardware. The DT can describe other things too. See the /options node, for example.
Please don't bring this highly misleading language into U-Boot.
Please point out what is misleading in the above paragraph. What is being emphasised in the above paragraph is that certain nodes and properties in the devicetree are relevant only in u-boot, and not the kernel. And this is precisely what the devicetree maintainers are saying [1].
+This devicetree can then be passed to the OS. Since certain nodes and +properties are not really describing hardware, and more importantly, +these are only relevant to U-Boot, bindings for these cannot be +upstreamed into the devicetree repository. There have been instances +of attempts being made to upstream such bindings, and these deemed not +fit for upstreaming.
Then either they should not be in U-Boot, or there is a problem with the process.
Not having a binding for these nodes and +properties means that the devicetree fails the schema compliance tests +[1]. This also means that the platform cannot get certifications like +SystemReady [2] which, among other things require a devicetree which +passes the schema compliance tests.
+For such nodes and properties, it has been suggested by the devicetree +maintainers that the right thing to do is to remove them from the +devicetree before it gets passed on to the OS [3].
Hard NAK. If we go this way, then no one will ever have an incentive to do the right thing.
Please send bindings for Linaro's work, instead. If something is entirely U-Boot-specific, then it can go in /options/u-boot but it still must be in the dt-schema.
Please re-read the document including the last link [1]. If you go through that entire thread, you will notice that this is precisely what Linaro was trying to do here -- upstream the binding for the fwu-mdata node. It is only based on the feedback of the devicetree maintainers that this patchset was required.
-sughosh
[1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
+Removing nodes/properties +-------------------------
+In U-Boot, this is been done through adding information on such nodes +and properties in a list. The entire node can be deleted, or a +specific property under a node can be deleted. The list of such nodes +and properties is generated at compile time, and the function to purge +these can be invoked through a EVT_FT_FIXUP event notify call.
+For deleting a node, this can be done by declaring a macro::
DT_NON_COMPLIANT_PURGE(fwu_mdata) = {.node_path = "/fwu-mdata",};+Similarly, for deleting a property under a node, that can be done by +specifying the property name::
DT_NON_COMPLIANT_PURGE(capsule_key) = {.node_path = "/signature",.prop = "capsule-key",};+In the first example, the entire node with path /fwu-mdata will be +removed. In the second example, the property capsule-key +under /signature node will be removed.
+Similarly, a list of nodes and properties can be specified using the +following macro::
DT_NON_COMPLIANT_PURGE_LIST(foo) = {{ .node_path = "/some_node", .prop = "some_bar" },{ .node_path = "/some_node" },};+[1] - https://github.com/devicetree-org/dt-schema +[2] - https://www.arm.com/architecture/system-architectures/systemready-certificat...
+[3] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
2.34.1
Regards, Simon
On Tue, Aug 29, 2023 at 12:04:53AM +0530, Sughosh Ganu wrote:
hi Simon,
On Mon, 28 Aug 2023 at 23:25, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add a document explaining the need for removal of non-compliant devicetree nodes and properties. Also describe in brief, the macros that can be used for this removal.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
.../devicetree/dt_non_compliant_purge.rst | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst new file mode 100644 index 0000000000..c3a8feab5b --- /dev/null +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst @@ -0,0 +1,64 @@ +.. SPDX-License-Identifier: GPL-2.0+
+Removal of non-compliant nodes and properties +=============================================
+The devicetree used in U-Boot might contain nodes and properties which +are specific only to U-Boot, and are not necessarily being used to +describe hardware but to pass information to U-Boot. An example of +such a property would be the public key being passed to U-Boot for +verification.
It has nothing to do with describing hardware. The DT can describe other things too. See the /options node, for example.
Please don't bring this highly misleading language into U-Boot.
Please point out what is misleading in the above paragraph. What is being emphasised in the above paragraph is that certain nodes and properties in the devicetree are relevant only in u-boot, and not the kernel. And this is precisely what the devicetree maintainers are saying [1].
+This devicetree can then be passed to the OS. Since certain nodes and +properties are not really describing hardware, and more importantly, +these are only relevant to U-Boot, bindings for these cannot be +upstreamed into the devicetree repository. There have been instances +of attempts being made to upstream such bindings, and these deemed not +fit for upstreaming.
Then either they should not be in U-Boot, or there is a problem with the process.
Not having a binding for these nodes and +properties means that the devicetree fails the schema compliance tests +[1]. This also means that the platform cannot get certifications like +SystemReady [2] which, among other things require a devicetree which +passes the schema compliance tests.
+For such nodes and properties, it has been suggested by the devicetree +maintainers that the right thing to do is to remove them from the +devicetree before it gets passed on to the OS [3].
Hard NAK. If we go this way, then no one will ever have an incentive to do the right thing.
Please send bindings for Linaro's work, instead. If something is entirely U-Boot-specific, then it can go in /options/u-boot but it still must be in the dt-schema.
Please re-read the document including the last link [1]. If you go through that entire thread, you will notice that this is precisely what Linaro was trying to do here -- upstream the binding for the fwu-mdata node. It is only based on the feedback of the devicetree maintainers that this patchset was required.
-sughosh
[1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
Please note that this right here, that the explanation of why we need to delete this node, not being a bright shiny visible object is one of the big problems with this patchset and implementation. It cannot be footnotes in email threads as to why such-and-such node/property isn't upstream, it needs to be documented and visible in the code base / documentation and an obvious you must do this for future cases.
Hi Tom
On Mon, 28 Aug 2023 at 21:39, Tom Rini trini@konsulko.com wrote:
On Tue, Aug 29, 2023 at 12:04:53AM +0530, Sughosh Ganu wrote:
hi Simon,
On Mon, 28 Aug 2023 at 23:25, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add a document explaining the need for removal of non-compliant devicetree nodes and properties. Also describe in brief, the macros that can be used for this removal.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
.../devicetree/dt_non_compliant_purge.rst | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst new file mode 100644 index 0000000000..c3a8feab5b --- /dev/null +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst @@ -0,0 +1,64 @@ +.. SPDX-License-Identifier: GPL-2.0+
+Removal of non-compliant nodes and properties +=============================================
+The devicetree used in U-Boot might contain nodes and properties which +are specific only to U-Boot, and are not necessarily being used to +describe hardware but to pass information to U-Boot. An example of +such a property would be the public key being passed to U-Boot for +verification.
It has nothing to do with describing hardware. The DT can describe other things too. See the /options node, for example.
Please don't bring this highly misleading language into U-Boot.
Please point out what is misleading in the above paragraph. What is being emphasised in the above paragraph is that certain nodes and properties in the devicetree are relevant only in u-boot, and not the kernel. And this is precisely what the devicetree maintainers are saying [1].
+This devicetree can then be passed to the OS. Since certain nodes and +properties are not really describing hardware, and more importantly, +these are only relevant to U-Boot, bindings for these cannot be +upstreamed into the devicetree repository. There have been instances +of attempts being made to upstream such bindings, and these deemed not +fit for upstreaming.
Then either they should not be in U-Boot, or there is a problem with the process.
Not having a binding for these nodes and +properties means that the devicetree fails the schema compliance tests +[1]. This also means that the platform cannot get certifications like +SystemReady [2] which, among other things require a devicetree which +passes the schema compliance tests.
+For such nodes and properties, it has been suggested by the devicetree +maintainers that the right thing to do is to remove them from the +devicetree before it gets passed on to the OS [3].
Hard NAK. If we go this way, then no one will ever have an incentive to do the right thing.
Please send bindings for Linaro's work, instead. If something is entirely U-Boot-specific, then it can go in /options/u-boot but it still must be in the dt-schema.
Please re-read the document including the last link [1]. If you go through that entire thread, you will notice that this is precisely what Linaro was trying to do here -- upstream the binding for the fwu-mdata node. It is only based on the feedback of the devicetree maintainers that this patchset was required.
-sughosh
[1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
Please note that this right here, that the explanation of why we need to delete this node, not being a bright shiny visible object is one of the big problems with this patchset and implementation. It cannot be footnotes in email threads as to why such-and-such node/property isn't upstream, it needs to be documented and visible in the code base / documentation and an obvious you must do this for future cases.
I thought we agreed that deleting nodes that won't be accepted upstream is the right approach since that would break the systemready 2.0 compatibility.
Yes, it can't be footnotes or hidden links, but this is totally different than what I am reading on this thread.
Thanks /Ilias
-- Tom
On Wed, Aug 30, 2023 at 10:24:39AM +0300, Ilias Apalodimas wrote:
Hi Tom
On Mon, 28 Aug 2023 at 21:39, Tom Rini trini@konsulko.com wrote:
On Tue, Aug 29, 2023 at 12:04:53AM +0530, Sughosh Ganu wrote:
hi Simon,
On Mon, 28 Aug 2023 at 23:25, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add a document explaining the need for removal of non-compliant devicetree nodes and properties. Also describe in brief, the macros that can be used for this removal.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
.../devicetree/dt_non_compliant_purge.rst | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst new file mode 100644 index 0000000000..c3a8feab5b --- /dev/null +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst @@ -0,0 +1,64 @@ +.. SPDX-License-Identifier: GPL-2.0+
+Removal of non-compliant nodes and properties +=============================================
+The devicetree used in U-Boot might contain nodes and properties which +are specific only to U-Boot, and are not necessarily being used to +describe hardware but to pass information to U-Boot. An example of +such a property would be the public key being passed to U-Boot for +verification.
It has nothing to do with describing hardware. The DT can describe other things too. See the /options node, for example.
Please don't bring this highly misleading language into U-Boot.
Please point out what is misleading in the above paragraph. What is being emphasised in the above paragraph is that certain nodes and properties in the devicetree are relevant only in u-boot, and not the kernel. And this is precisely what the devicetree maintainers are saying [1].
+This devicetree can then be passed to the OS. Since certain nodes and +properties are not really describing hardware, and more importantly, +these are only relevant to U-Boot, bindings for these cannot be +upstreamed into the devicetree repository. There have been instances +of attempts being made to upstream such bindings, and these deemed not +fit for upstreaming.
Then either they should not be in U-Boot, or there is a problem with the process.
Not having a binding for these nodes and +properties means that the devicetree fails the schema compliance tests +[1]. This also means that the platform cannot get certifications like +SystemReady [2] which, among other things require a devicetree which +passes the schema compliance tests.
+For such nodes and properties, it has been suggested by the devicetree +maintainers that the right thing to do is to remove them from the +devicetree before it gets passed on to the OS [3].
Hard NAK. If we go this way, then no one will ever have an incentive to do the right thing.
Please send bindings for Linaro's work, instead. If something is entirely U-Boot-specific, then it can go in /options/u-boot but it still must be in the dt-schema.
Please re-read the document including the last link [1]. If you go through that entire thread, you will notice that this is precisely what Linaro was trying to do here -- upstream the binding for the fwu-mdata node. It is only based on the feedback of the devicetree maintainers that this patchset was required.
-sughosh
[1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
Please note that this right here, that the explanation of why we need to delete this node, not being a bright shiny visible object is one of the big problems with this patchset and implementation. It cannot be footnotes in email threads as to why such-and-such node/property isn't upstream, it needs to be documented and visible in the code base / documentation and an obvious you must do this for future cases.
I thought we agreed that deleting nodes that won't be accepted upstream is the right approach since that would break the systemready 2.0 compatibility.
Yes, it can't be footnotes or hidden links, but this is totally different than what I am reading on this thread.
An issue is that the functionality got posted without clear links as to why the initial nodes to be deleted had been rejected, in the patchset itself (and so not preserved long term). Being able to show that yes, really, it was attempted to upstream the nodes, and not "delete first upstream later (never)" is critical.
Hi Ilias,
On Wed, 30 Aug 2023 at 01:25, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Tom
On Mon, 28 Aug 2023 at 21:39, Tom Rini trini@konsulko.com wrote:
On Tue, Aug 29, 2023 at 12:04:53AM +0530, Sughosh Ganu wrote:
hi Simon,
On Mon, 28 Aug 2023 at 23:25, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add a document explaining the need for removal of non-compliant devicetree nodes and properties. Also describe in brief, the macros that can be used for this removal.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
.../devicetree/dt_non_compliant_purge.rst | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst new file mode 100644 index 0000000000..c3a8feab5b --- /dev/null +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst @@ -0,0 +1,64 @@ +.. SPDX-License-Identifier: GPL-2.0+
+Removal of non-compliant nodes and properties +=============================================
+The devicetree used in U-Boot might contain nodes and properties which +are specific only to U-Boot, and are not necessarily being used to +describe hardware but to pass information to U-Boot. An example of +such a property would be the public key being passed to U-Boot for +verification.
It has nothing to do with describing hardware. The DT can describe other things too. See the /options node, for example.
Please don't bring this highly misleading language into U-Boot.
Please point out what is misleading in the above paragraph. What is being emphasised in the above paragraph is that certain nodes and properties in the devicetree are relevant only in u-boot, and not the kernel. And this is precisely what the devicetree maintainers are saying [1].
+This devicetree can then be passed to the OS. Since certain nodes and +properties are not really describing hardware, and more importantly, +these are only relevant to U-Boot, bindings for these cannot be +upstreamed into the devicetree repository. There have been instances +of attempts being made to upstream such bindings, and these deemed not +fit for upstreaming.
Then either they should not be in U-Boot, or there is a problem with the process.
Not having a binding for these nodes and +properties means that the devicetree fails the schema compliance tests +[1]. This also means that the platform cannot get certifications like +SystemReady [2] which, among other things require a devicetree which +passes the schema compliance tests.
+For such nodes and properties, it has been suggested by the devicetree +maintainers that the right thing to do is to remove them from the +devicetree before it gets passed on to the OS [3].
Hard NAK. If we go this way, then no one will ever have an incentive to do the right thing.
Please send bindings for Linaro's work, instead. If something is entirely U-Boot-specific, then it can go in /options/u-boot but it still must be in the dt-schema.
Please re-read the document including the last link [1]. If you go through that entire thread, you will notice that this is precisely what Linaro was trying to do here -- upstream the binding for the fwu-mdata node. It is only based on the feedback of the devicetree maintainers that this patchset was required.
-sughosh
[1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
Please note that this right here, that the explanation of why we need to delete this node, not being a bright shiny visible object is one of the big problems with this patchset and implementation. It cannot be footnotes in email threads as to why such-and-such node/property isn't upstream, it needs to be documented and visible in the code base / documentation and an obvious you must do this for future cases.
I thought we agreed that deleting nodes that won't be accepted upstream is the right approach since that would break the systemready 2.0 compatibility.
Isn't that controlled by ARM/Linaros, as are the devicetree bindings? What am I missing? Let's just fix the bindings so they can be accepted. What we decide here will have enormous repercussions in the future. SoC vendors are watching to see whether they should bother to upstream bindings or not.
Yes, it can't be footnotes or hidden links, but this is totally different than what I am reading on this thread.
Regards, Simon
Hi Simon,
On Thu, 31 Aug 2023 at 05:50, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Wed, 30 Aug 2023 at 01:25, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Tom
On Mon, 28 Aug 2023 at 21:39, Tom Rini trini@konsulko.com wrote:
On Tue, Aug 29, 2023 at 12:04:53AM +0530, Sughosh Ganu wrote:
[...]
Please re-read the document including the last link [1]. If you go through that entire thread, you will notice that this is precisely what Linaro was trying to do here -- upstream the binding for the fwu-mdata node. It is only based on the feedback of the devicetree maintainers that this patchset was required.
-sughosh
[1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
Please note that this right here, that the explanation of why we need to delete this node, not being a bright shiny visible object is one of the big problems with this patchset and implementation. It cannot be footnotes in email threads as to why such-and-such node/property isn't upstream, it needs to be documented and visible in the code base / documentation and an obvious you must do this for future cases.
I thought we agreed that deleting nodes that won't be accepted upstream is the right approach since that would break the systemready 2.0 compatibility.
Isn't that controlled by ARM/Linaros, as are the devicetree bindings?
The device tree validation happens through the dt schema validation. You can read a bit more here [0]
What am I missing? Let's just fix the bindings so they can be accepted.
I can go through the mailing lists, but I am pretty sure you've been trying to do this for a number of years and got pushback for many of those bindings. Has that changed? Can you guarantee to all the vendors that all the bindings will get merged and they won't have a problem getting their boards certified?
What we decide here will have enormous repercussions in the future. SoC vendors are watching to see whether they should bother to upstream bindings or not.
How is that remotely relevant to the discussion we have here? If it's hardware that the OS has to use they *will* have to upstream it, otherwise their device won't work with upstream kernels (not that they care, I am just pointing it out). If it's an internal thing they need to perform an X action in U-Boot and the OS has no interest in that property (e.g. the driver model nodes), they can just get rid of it.
Yes, it can't be footnotes or hidden links, but this is totally different than what I am reading on this thread.
Regards, Simon
[0] https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-source...
Thanks /Ilias
On Thu, Aug 31, 2023 at 10:52:04AM +0300, Ilias Apalodimas wrote:
Hi Simon,
On Thu, 31 Aug 2023 at 05:50, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Wed, 30 Aug 2023 at 01:25, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Tom
On Mon, 28 Aug 2023 at 21:39, Tom Rini trini@konsulko.com wrote:
On Tue, Aug 29, 2023 at 12:04:53AM +0530, Sughosh Ganu wrote:
[...]
Please re-read the document including the last link [1]. If you go through that entire thread, you will notice that this is precisely what Linaro was trying to do here -- upstream the binding for the fwu-mdata node. It is only based on the feedback of the devicetree maintainers that this patchset was required.
-sughosh
[1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
Please note that this right here, that the explanation of why we need to delete this node, not being a bright shiny visible object is one of the big problems with this patchset and implementation. It cannot be footnotes in email threads as to why such-and-such node/property isn't upstream, it needs to be documented and visible in the code base / documentation and an obvious you must do this for future cases.
I thought we agreed that deleting nodes that won't be accepted upstream is the right approach since that would break the systemready 2.0 compatibility.
Isn't that controlled by ARM/Linaros, as are the devicetree bindings?
The device tree validation happens through the dt schema validation. You can read a bit more here [0]
What am I missing? Let's just fix the bindings so they can be accepted.
I can go through the mailing lists, but I am pretty sure you've been trying to do this for a number of years and got pushback for many of those bindings. Has that changed?
Yes, yes it has changed. It has changed for the better, and we're all working together to get things accepted. Hence Simon's big concern that adding the framework to delete things from the tree easily will cause people to stop trying.
Can you guarantee to all the vendors that all the bindings will get merged and they won't have a problem getting their boards certified?
It certainly seems that, ahem, no reasonable binding will be refused.
Hi Sughosh,
On Mon, 28 Aug 2023 at 12:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Mon, 28 Aug 2023 at 23:25, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add a document explaining the need for removal of non-compliant devicetree nodes and properties. Also describe in brief, the macros that can be used for this removal.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
.../devicetree/dt_non_compliant_purge.rst | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst new file mode 100644 index 0000000000..c3a8feab5b --- /dev/null +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst @@ -0,0 +1,64 @@ +.. SPDX-License-Identifier: GPL-2.0+
+Removal of non-compliant nodes and properties +=============================================
+The devicetree used in U-Boot might contain nodes and properties which +are specific only to U-Boot, and are not necessarily being used to +describe hardware but to pass information to U-Boot. An example of +such a property would be the public key being passed to U-Boot for +verification.
It has nothing to do with describing hardware. The DT can describe other things too. See the /options node, for example.
Please don't bring this highly misleading language into U-Boot.
Please point out what is misleading in the above paragraph. What is being emphasised in the above paragraph is that certain nodes and properties in the devicetree are relevant only in u-boot, and not the kernel. And this is precisely what the devicetree maintainers are saying [1].
That is not relevant though...we need to make sure all the nodes are in the dt schema.
It is misleading because you imply that DT should only describe hardware. That is not true.
+This devicetree can then be passed to the OS. Since certain nodes and +properties are not really describing hardware, and more importantly, +these are only relevant to U-Boot, bindings for these cannot be +upstreamed into the devicetree repository. There have been instances +of attempts being made to upstream such bindings, and these deemed not +fit for upstreaming.
Then either they should not be in U-Boot, or there is a problem with the process.
Not having a binding for these nodes and +properties means that the devicetree fails the schema compliance tests +[1]. This also means that the platform cannot get certifications like +SystemReady [2] which, among other things require a devicetree which +passes the schema compliance tests.
+For such nodes and properties, it has been suggested by the devicetree +maintainers that the right thing to do is to remove them from the +devicetree before it gets passed on to the OS [3].
Hard NAK. If we go this way, then no one will ever have an incentive to do the right thing.
Please send bindings for Linaro's work, instead. If something is entirely U-Boot-specific, then it can go in /options/u-boot but it still must be in the dt-schema.
Please re-read the document including the last link [1]. If you go through that entire thread, you will notice that this is precisely what Linaro was trying to do here -- upstream the binding for the fwu-mdata node. It is only based on the feedback of the devicetree maintainers that this patchset was required.
It looks like it should go in /options/u-boot ? Can you resubmit it there?
The stripping is a no-go for me, sorry. It is absolutely going to destroy any chance of tidying up DT in U-Boot.
Please also figure out how to add DT validation to U-Boot, so we can see what the gap is. Once we have that in, I will be happier to do workarounds.
Regards, Simon
-sughosh
[1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
+Removing nodes/properties +-------------------------
+In U-Boot, this is been done through adding information on such nodes +and properties in a list. The entire node can be deleted, or a +specific property under a node can be deleted. The list of such nodes +and properties is generated at compile time, and the function to purge +these can be invoked through a EVT_FT_FIXUP event notify call.
+For deleting a node, this can be done by declaring a macro::
DT_NON_COMPLIANT_PURGE(fwu_mdata) = {.node_path = "/fwu-mdata",};+Similarly, for deleting a property under a node, that can be done by +specifying the property name::
DT_NON_COMPLIANT_PURGE(capsule_key) = {.node_path = "/signature",.prop = "capsule-key",};+In the first example, the entire node with path /fwu-mdata will be +removed. In the second example, the property capsule-key +under /signature node will be removed.
+Similarly, a list of nodes and properties can be specified using the +following macro::
DT_NON_COMPLIANT_PURGE_LIST(foo) = {{ .node_path = "/some_node", .prop = "some_bar" },{ .node_path = "/some_node" },};+[1] - https://github.com/devicetree-org/dt-schema +[2] - https://www.arm.com/architecture/system-architectures/systemready-certificat...
+[3] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
2.34.1
Regards, Simon
hi Simon,
On Tue, 29 Aug 2023 at 22:55, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Mon, 28 Aug 2023 at 12:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Mon, 28 Aug 2023 at 23:25, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add a document explaining the need for removal of non-compliant devicetree nodes and properties. Also describe in brief, the macros that can be used for this removal.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
.../devicetree/dt_non_compliant_purge.rst | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst new file mode 100644 index 0000000000..c3a8feab5b --- /dev/null +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst @@ -0,0 +1,64 @@ +.. SPDX-License-Identifier: GPL-2.0+
+Removal of non-compliant nodes and properties +=============================================
+The devicetree used in U-Boot might contain nodes and properties which +are specific only to U-Boot, and are not necessarily being used to +describe hardware but to pass information to U-Boot. An example of +such a property would be the public key being passed to U-Boot for +verification.
It has nothing to do with describing hardware. The DT can describe other things too. See the /options node, for example.
Please don't bring this highly misleading language into U-Boot.
Please point out what is misleading in the above paragraph. What is being emphasised in the above paragraph is that certain nodes and properties in the devicetree are relevant only in u-boot, and not the kernel. And this is precisely what the devicetree maintainers are saying [1].
That is not relevant though...we need to make sure all the nodes are in the dt schema.
It is misleading because you imply that DT should only describe hardware. That is not true.
+This devicetree can then be passed to the OS. Since certain nodes and +properties are not really describing hardware, and more importantly, +these are only relevant to U-Boot, bindings for these cannot be +upstreamed into the devicetree repository. There have been instances +of attempts being made to upstream such bindings, and these deemed not +fit for upstreaming.
Then either they should not be in U-Boot, or there is a problem with the process.
Not having a binding for these nodes and +properties means that the devicetree fails the schema compliance tests +[1]. This also means that the platform cannot get certifications like +SystemReady [2] which, among other things require a devicetree which +passes the schema compliance tests.
+For such nodes and properties, it has been suggested by the devicetree +maintainers that the right thing to do is to remove them from the +devicetree before it gets passed on to the OS [3].
Hard NAK. If we go this way, then no one will ever have an incentive to do the right thing.
Please send bindings for Linaro's work, instead. If something is entirely U-Boot-specific, then it can go in /options/u-boot but it still must be in the dt-schema.
Please re-read the document including the last link [1]. If you go through that entire thread, you will notice that this is precisely what Linaro was trying to do here -- upstream the binding for the fwu-mdata node. It is only based on the feedback of the devicetree maintainers that this patchset was required.
It looks like it should go in /options/u-boot ? Can you resubmit it there?
Okay. I will try this approach. If I face any hurdles, or the devicetree maintainers have a different take on this, I will let you know.
-sughosh
The stripping is a no-go for me, sorry. It is absolutely going to destroy any chance of tidying up DT in U-Boot.
Please also figure out how to add DT validation to U-Boot, so we can see what the gap is. Once we have that in, I will be happier to do workarounds.
Regards, Simon
-sughosh
[1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
+Removing nodes/properties +-------------------------
+In U-Boot, this is been done through adding information on such nodes +and properties in a list. The entire node can be deleted, or a +specific property under a node can be deleted. The list of such nodes +and properties is generated at compile time, and the function to purge +these can be invoked through a EVT_FT_FIXUP event notify call.
+For deleting a node, this can be done by declaring a macro::
DT_NON_COMPLIANT_PURGE(fwu_mdata) = {.node_path = "/fwu-mdata",};+Similarly, for deleting a property under a node, that can be done by +specifying the property name::
DT_NON_COMPLIANT_PURGE(capsule_key) = {.node_path = "/signature",.prop = "capsule-key",};+In the first example, the entire node with path /fwu-mdata will be +removed. In the second example, the property capsule-key +under /signature node will be removed.
+Similarly, a list of nodes and properties can be specified using the +following macro::
DT_NON_COMPLIANT_PURGE_LIST(foo) = {{ .node_path = "/some_node", .prop = "some_bar" },{ .node_path = "/some_node" },};+[1] - https://github.com/devicetree-org/dt-schema +[2] - https://www.arm.com/architecture/system-architectures/systemready-certificat...
+[3] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6...
2.34.1
Regards, Simon
On 8/26/23 11:06, Sughosh Ganu wrote:
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
We should have a test for the new functionality. E.g. add some superfluous properties and nodes to arch/sandbox/dts/test.dts, delete them via the DT_PURGE macro, and check that the device-tree passed to an EFI binary does not contain these properties and nodes.
Best regards
Heinrich
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
On Sat, 26 Aug 2023 at 15:36, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/26/23 11:06, Sughosh Ganu wrote:
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
We should have a test for the new functionality. E.g. add some superfluous properties and nodes to arch/sandbox/dts/test.dts, delete them via the DT_PURGE macro, and check that the device-tree passed to an EFI binary does not contain these properties and nodes.
Yes, I plan to have a test in the non-RFC version. I was thinking of adding a command and then using that to test the functionality.
-sughosh
Best regards
Heinrich
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
Hi,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Instead of this, how about working on bringing the DT validation into U-Boot so we can see what state things are in?
Please send the bindings for Linaro's recent series (which I suspect are motivating this series) so we can start the process.
Regards, Simon
On Mon, Aug 28, 2023 at 5:20 PM Simon Glass sjg@chromium.org wrote:
Hi,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
Instead of this, how about working on bringing the DT validation into U-Boot so we can see what state things are in?
Please send the bindings for Linaro's recent series (which I suspect are motivating this series) so we can start the process.
Regards, Simon
On Mon, Aug 28, 2023 at 05:37:45PM +0100, Peter Robinson wrote:
On Mon, Aug 28, 2023 at 5:20 PM Simon Glass sjg@chromium.org wrote:
Hi,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
It's about validation and Simon is literally in the process of having the binman bindings upstreamed.
Hi Peter,
On Mon, 28 Aug 2023 at 10:37, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 5:20 PM Simon Glass sjg@chromium.org wrote:
Hi,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
I have started the process to upstream the binman bindings.
Perhaps we should use the issue tracker[1] to follow progress on all of this. We need to clean it up.
Instead of this, how about working on bringing the DT validation into U-Boot so we can see what state things are in?
Please send the bindings for Linaro's recent series (which I suspect are motivating this series) so we can start the process.
Regards, Simon
On Mon, Aug 28, 2023 at 6:54 PM Simon Glass sjg@chromium.org wrote:
Hi Peter,
On Mon, 28 Aug 2023 at 10:37, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 5:20 PM Simon Glass sjg@chromium.org wrote:
Hi,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
Perhaps we should use the issue tracker[1] to follow progress on all of this. We need to clean it up.
Instead of this, how about working on bringing the DT validation into U-Boot so we can see what state things are in?
Please send the bindings for Linaro's recent series (which I suspect are motivating this series) so we can start the process.
Regards, Simon
Hi Peter,
On Mon, 28 Aug 2023 at 14:29, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 6:54 PM Simon Glass sjg@chromium.org wrote:
Hi Peter,
On Mon, 28 Aug 2023 at 10:37, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 5:20 PM Simon Glass sjg@chromium.org wrote:
Hi,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Perhaps we should use the issue tracker[1] to follow progress on all of this. We need to clean it up.
Instead of this, how about working on bringing the DT validation into U-Boot so we can see what state things are in?
Please send the bindings for Linaro's recent series (which I suspect are motivating this series) so we can start the process.
Regards, Simon
Regards, Simon
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
Can you give me an example of "options" as grep doesn't give me a single one in the kernel tree? I think we can just agree to disagree here.
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Can you provide an example as to how that is used during runtime? Do you mean runtime during the build process or runtime on the device?
Perhaps we should use the issue tracker[1] to follow progress on all of this. We need to clean it up.
Instead of this, how about working on bringing the DT validation into U-Boot so we can see what state things are in?
Please send the bindings for Linaro's recent series (which I suspect are motivating this series) so we can start the process.
Regards, Simon
Regards, Simon
Hi Peter,
On Tue, 29 Aug 2023 at 04:33, Peter Robinson pbrobinson@gmail.com wrote:
> Provide a way for removing certain devicetree nodes and/or properties > from the devicetree. This is needed to purge certain nodes and > properties which may be relevant only in U-Boot. Such nodes and > properties are then removed from the devicetree before it is passed to > the kernel. This ensures that the devicetree passed to the OS does not > contain any non-compliant nodes and properties. > > The removal of the nodes and properties is being done through an > EVT_FT_FIXUP handler. I am not sure if the removal code needs to be > behind any Kconfig symbol. > > I have only build tested this on sandbox, and tested on qemu arm64 > virt platform. This being a RFC, I have not put this through a CI run. > > Sughosh Ganu (5): > dt: Provide a way to remove non-compliant nodes and properties > fwu: Add the fwu-mdata node for removal from devicetree > capsule: Add the capsule-key property for removal from devicetree > bootefi: Call the EVT_FT_FIXUP event handler > doc: Add a document for non-compliant DT node/property removal > > cmd/bootefi.c | 18 +++++ > .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ > drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ > include/dt-structs.h | 11 +++ > lib/Makefile | 1 + > lib/dt_purge.c | 73 +++++++++++++++++++ > lib/efi_loader/efi_capsule.c | 7 ++ > 7 files changed, 179 insertions(+) > create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst > create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
Can you give me an example of "options" as grep doesn't give me a single one in the kernel tree? I think we can just agree to disagree here.
See here: https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/optio...
I don't mind disagreeing, so long as it doesn't result in any restrictions on using devicetree in firmware. But it is very important to the success of firmware that we can make full use of the devicetree.
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Can you provide an example as to how that is used during runtime? Do you mean runtime during the build process or runtime on the device?
I mean runtime on the device. An example is that we might want to control whether the serial UART is enabled, without having to rebuild each program in the firmware stack.
Perhaps we should use the issue tracker[1] to follow progress on all of this. We need to clean it up.
Instead of this, how about working on bringing the DT validation into U-Boot so we can see what state things are in?
Please send the bindings for Linaro's recent series (which I suspect are motivating this series) so we can start the process.
Regards, Simon
Regards, Simon
Regards, Simon
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
Can you give me an example of "options" as grep doesn't give me a single one in the kernel tree? I think we can just agree to disagree here.
See here: https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/optio...
All of those options look to me like they would work just fine in a .env file like you've added board/raspberrypi/rpi/rpi.env
I don't mind disagreeing, so long as it doesn't result in any restrictions on using devicetree in firmware. But it is very important to the success of firmware that we can make full use of the devicetree.
That statement doesn't make sense, of course the firmware makes full use of the DT because it needs it to instantiate the HW. You don't actually answer my question though.
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Can you provide an example as to how that is used during runtime? Do you mean runtime during the build process or runtime on the device?
I mean runtime on the device. An example is that we might want to control whether the serial UART is enabled, without having to rebuild each program in the firmware stack.
That is describing the HW though, such as whether a serial port is enabled or not, it's not how the binman pieces you're adding to DT are used during runtime which is the question I was asking.
Perhaps we should use the issue tracker[1] to follow progress on all of this. We need to clean it up.
> Instead of this, how about working on bringing the DT validation into > U-Boot so we can see what state things are in? > > Please send the bindings for Linaro's recent series (which I suspect > are motivating this series) so we can start the process.
Regards, Simon
Regards, Simon
Regards, Simon
Hi Peter,
On Wed, 30 Aug 2023 at 02:19, Peter Robinson pbrobinson@gmail.com wrote:
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
Can you give me an example of "options" as grep doesn't give me a single one in the kernel tree? I think we can just agree to disagree here.
See here: https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/optio...
All of those options look to me like they would work just fine in a .env file like you've added board/raspberrypi/rpi/rpi.env
But that is built into U-Boot. How can it be controlled by another previous progress in the firmware stack?
I don't mind disagreeing, so long as it doesn't result in any restrictions on using devicetree in firmware. But it is very important to the success of firmware that we can make full use of the devicetree.
That statement doesn't make sense, of course the firmware makes full use of the DT because it needs it to instantiate the HW. You don't actually answer my question though.
I think I tried to answer your question but we are not on the same page. Please don't say that DT is just for hardware, since that is a sore point with me. It helf back U-Boot for many years, to no useful purpose.
Perhaps you could restate the question?
Also, why are you trying to keep things out of the DT?
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Can you provide an example as to how that is used during runtime? Do you mean runtime during the build process or runtime on the device?
I mean runtime on the device. An example is that we might want to control whether the serial UART is enabled, without having to rebuild each program in the firmware stack.
That is describing the HW though, such as whether a serial port is enabled or not, it's not how the binman pieces you're adding to DT are used during runtime which is the question I was asking.
There is:
1. whether the serial device is enabled in the DT 2. whether it emits character or not
Often we always want 1, in case we need to emit something. But for 2 we want to control it with a global setting that applies to all programs in the firmware flow.
Another example would be a framebuffer address, where we want to provide it in the DT so that all programs can use the same one.
Perhaps we should use the issue tracker[1] to follow progress on all of this. We need to clean it up.
> > > Instead of this, how about working on bringing the DT validation into > > U-Boot so we can see what state things are in? > > > > Please send the bindings for Linaro's recent series (which I suspect > > are motivating this series) so we can start the process.
Regards, Simon
Regards, Simon
On Mon, Aug 28, 2023 at 04:09:29PM -0600, Simon Glass wrote:
Hi Peter,
On Mon, 28 Aug 2023 at 14:29, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 6:54 PM Simon Glass sjg@chromium.org wrote:
Hi Peter,
On Mon, 28 Aug 2023 at 10:37, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 5:20 PM Simon Glass sjg@chromium.org wrote:
Hi,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
For the record, I tend to agree.
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Really, I don't want to have to care about the binman binding. If it is u-boot specific, then it should stay in u-boot. I took /options/u-boot/, but now I'm starting to have second thoughts on that being in dtschema if it is going to be continually and frequently extended. Validating it in SR does little. If a vendor is abusing /options/u-boot/ in some way they could just as easily remove the node in their u-boot fork to pass. SR is certainly not going to require the node be there.
On A/B updates, that really doesn't seem like a u-boot specific problem to me. No one wants A/B updates in EDK2 or anything else?
Rob
Hi Rob,
On Wed, 6 Sept 2023 at 08:21, Rob Herring robh@kernel.org wrote:
On Mon, Aug 28, 2023 at 04:09:29PM -0600, Simon Glass wrote:
Hi Peter,
On Mon, 28 Aug 2023 at 14:29, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 6:54 PM Simon Glass sjg@chromium.org wrote:
Hi Peter,
On Mon, 28 Aug 2023 at 10:37, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 5:20 PM Simon Glass sjg@chromium.org wrote:
Hi,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote: > > > Provide a way for removing certain devicetree nodes and/or properties > from the devicetree. This is needed to purge certain nodes and > properties which may be relevant only in U-Boot. Such nodes and > properties are then removed from the devicetree before it is passed to > the kernel. This ensures that the devicetree passed to the OS does not > contain any non-compliant nodes and properties. > > The removal of the nodes and properties is being done through an > EVT_FT_FIXUP handler. I am not sure if the removal code needs to be > behind any Kconfig symbol. > > I have only build tested this on sandbox, and tested on qemu arm64 > virt platform. This being a RFC, I have not put this through a CI run. > > Sughosh Ganu (5): > dt: Provide a way to remove non-compliant nodes and properties > fwu: Add the fwu-mdata node for removal from devicetree > capsule: Add the capsule-key property for removal from devicetree > bootefi: Call the EVT_FT_FIXUP event handler > doc: Add a document for non-compliant DT node/property removal > > cmd/bootefi.c | 18 +++++ > .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ > drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ > include/dt-structs.h | 11 +++ > lib/Makefile | 1 + > lib/dt_purge.c | 73 +++++++++++++++++++ > lib/efi_loader/efi_capsule.c | 7 ++ > 7 files changed, 179 insertions(+) > create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst > create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
For the record, I tend to agree.
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
I wish we could stop having this discussion entirely...to me the devicetree is all that firmware has to handle its configuration, both within projects and across project boundaries. There is no user space, no filesystem, etc.
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Really, I don't want to have to care about the binman binding. If it is u-boot specific, then it should stay in u-boot.
I don't believe it is U-Boot-specific since it describes the system firmware as a whole. It collects things from multiple projects and we want to know where they ended up, so we can do firmware update, etc.
I took /options/u-boot/, but now I'm starting to have second thoughts on that being in dtschema if it is going to be continually and frequently extended. Validating it in SR does little. If a vendor is abusing /options/u-boot/ in some way they could just as easily remove the node in their u-boot fork to pass. SR is certainly not going to require the node be there.
The challenge here is that we are trying to validate that the U-Boot DT matches the schema. We don't want to have people adding things willy nilly, even in /options/u-boot.
On A/B updates, that really doesn't seem like a u-boot specific problem to me. No one wants A/B updates in EDK2 or anything else?
I agree we should specify it correctly and in a general manner.
Regards, Simon
On Wed, Sep 06, 2023 at 09:21:39AM -0500, Rob Herring wrote:
[snip]
On A/B updates, that really doesn't seem like a u-boot specific problem to me. No one wants A/B updates in EDK2 or anything else?
This specific case goes back to: https://lore.kernel.org/u-boot/20230410232112.72778-1-jaswinder.singh@linaro...
But I believe Sughosh and Ilias are basically the people who are in charge of the code at this point? So perhaps a step here is some advice on whatever they need to redo/rethink to get something that is acceptable.
On Wed, Sep 06, 2023 at 09:21:39AM -0500, Rob Herring wrote:
On Mon, Aug 28, 2023 at 04:09:29PM -0600, Simon Glass wrote:
Hi Peter,
On Mon, 28 Aug 2023 at 14:29, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 6:54 PM Simon Glass sjg@chromium.org wrote:
Hi Peter,
On Mon, 28 Aug 2023 at 10:37, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 5:20 PM Simon Glass sjg@chromium.org wrote:
Hi,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote: > > > Provide a way for removing certain devicetree nodes and/or properties > from the devicetree. This is needed to purge certain nodes and > properties which may be relevant only in U-Boot. Such nodes and > properties are then removed from the devicetree before it is passed to > the kernel. This ensures that the devicetree passed to the OS does not > contain any non-compliant nodes and properties. > > The removal of the nodes and properties is being done through an > EVT_FT_FIXUP handler. I am not sure if the removal code needs to be > behind any Kconfig symbol. > > I have only build tested this on sandbox, and tested on qemu arm64 > virt platform. This being a RFC, I have not put this through a CI run. > > Sughosh Ganu (5): > dt: Provide a way to remove non-compliant nodes and properties > fwu: Add the fwu-mdata node for removal from devicetree > capsule: Add the capsule-key property for removal from devicetree > bootefi: Call the EVT_FT_FIXUP event handler > doc: Add a document for non-compliant DT node/property removal > > cmd/bootefi.c | 18 +++++ > .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ > drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ > include/dt-structs.h | 11 +++ > lib/Makefile | 1 + > lib/dt_purge.c | 73 +++++++++++++++++++ > lib/efi_loader/efi_capsule.c | 7 ++ > 7 files changed, 179 insertions(+) > create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst > create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
For the record, I tend to agree.
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Really, I don't want to have to care about the binman binding. If it is u-boot specific, then it should stay in u-boot. I took /options/u-boot/, but now I'm starting to have second thoughts on that being in dtschema if it is going to be continually and frequently extended. Validating it in SR does little. If a vendor is abusing /options/u-boot/ in some way they could just as easily remove the node in their u-boot fork to pass. SR is certainly not going to require the node be there.
It's going to continue to be a "fun" tight-rope to walk. No one wants an easy way for vendors to cheat the requirements which is why in some other parts of this thread (you may or may not have been on, I don't recall, sorry) I've been trying to make it clear that the removal mechanism should be both a slight pain to add to, and at least wrt upstream, clear that effort was made to upstream things. Cheaters are going to cheat and they could just chain a bunch of "fdt rm" together to do it today.
On 9/7/23 00:04, Tom Rini wrote:
On Wed, Sep 06, 2023 at 09:21:39AM -0500, Rob Herring wrote:
On Mon, Aug 28, 2023 at 04:09:29PM -0600, Simon Glass wrote:
Hi Peter,
On Mon, 28 Aug 2023 at 14:29, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 6:54 PM Simon Glass sjg@chromium.org wrote:
Hi Peter,
On Mon, 28 Aug 2023 at 10:37, Peter Robinson pbrobinson@gmail.com wrote:
On Mon, Aug 28, 2023 at 5:20 PM Simon Glass sjg@chromium.org wrote: > > Hi, > > On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote: >> >> >> Provide a way for removing certain devicetree nodes and/or properties >> from the devicetree. This is needed to purge certain nodes and >> properties which may be relevant only in U-Boot. Such nodes and >> properties are then removed from the devicetree before it is passed to >> the kernel. This ensures that the devicetree passed to the OS does not >> contain any non-compliant nodes and properties. >> >> The removal of the nodes and properties is being done through an >> EVT_FT_FIXUP handler. I am not sure if the removal code needs to be >> behind any Kconfig symbol. >> >> I have only build tested this on sandbox, and tested on qemu arm64 >> virt platform. This being a RFC, I have not put this through a CI run. >> >> Sughosh Ganu (5): >> dt: Provide a way to remove non-compliant nodes and properties >> fwu: Add the fwu-mdata node for removal from devicetree >> capsule: Add the capsule-key property for removal from devicetree >> bootefi: Call the EVT_FT_FIXUP event handler >> doc: Add a document for non-compliant DT node/property removal >> >> cmd/bootefi.c | 18 +++++ >> .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ >> drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ >> include/dt-structs.h | 11 +++ >> lib/Makefile | 1 + >> lib/dt_purge.c | 73 +++++++++++++++++++ >> lib/efi_loader/efi_capsule.c | 7 ++ >> 7 files changed, 179 insertions(+) >> create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst >> create mode 100644 lib/dt_purge.c > > What is the point of removing them? Instead, we should make sure that > we upstream the bindings and encourage SoC vendors to sync them. If we > remove them, no one will bother and U-Boot just becomes a dumping > ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
For the record, I tend to agree.
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Really, I don't want to have to care about the binman binding. If it is u-boot specific, then it should stay in u-boot. I took /options/u-boot/, but now I'm starting to have second thoughts on that being in dtschema if it is going to be continually and frequently extended. Validating it in SR does little. If a vendor is abusing /options/u-boot/ in some way they could just as easily remove the node in their u-boot fork to pass. SR is certainly not going to require the node be there.
The missing piece is validation of the U-Boot internal device-trees against a schema in the U-Boot CI. This should be possible even if some of the schema yaml files are maintained inside the U-Boot repo.
Best regards
Heinrich
It's going to continue to be a "fun" tight-rope to walk. No one wants an easy way for vendors to cheat the requirements which is why in some other parts of this thread (you may or may not have been on, I don't recall, sorry) I've been trying to make it clear that the removal mechanism should be both a slight pain to add to, and at least wrt upstream, clear that effort was made to upstream things. Cheaters are going to cheat and they could just chain a bunch of "fdt rm" together to do it today.
On Thu, Sep 07, 2023 at 01:30:01AM +0200, Heinrich Schuchardt wrote:
[snip]
The missing piece is validation of the U-Boot internal device-trees against a schema in the U-Boot CI. This should be possible even if some of the schema yaml files are maintained inside the U-Boot repo.
Dropping Rob since I don't think he cares about this part. Yes, what we need is some help in re-syncing our Kbuild (and Kconfig, but..) logic with the kernel as it's stuck at v4.20 still and all of the parts that make running the validation targets would be much easier (free) if we were in sync or much closer at least. It would be much appreciated if Linaro or Canonical or one of the other companies doing U-Boot work could get someone to focus on this task. It would help with a number of things, this included.
Hi Rob,
[...]
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
For the record, I tend to agree.
+1
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Really, I don't want to have to care about the binman binding. If it is u-boot specific, then it should stay in u-boot. I took /options/u-boot/, but now I'm starting to have second thoughts on that being in dtschema if it is going to be continually and frequently extended. Validating it in SR does little. If a vendor is abusing /options/u-boot/ in some way they could just as easily remove the node in their u-boot fork to pass. SR is certainly not going to require the node be there.
On A/B updates, that really doesn't seem like a u-boot specific problem to me. No one wants A/B updates in EDK2 or anything else?
A/B updates might be implemented in EDK2 or any other bootloader that chooses to implement it. The metadata information a bootloader needs to implement it are documented here [0]. Those metadata are not part of the DT. If they were it would make sense to add them on the schema. The DT entry we are using though serves a different purpose. It tells the bootloader the location of the metadata (iow where can I read them from the disk). Since bootloaders have a different way of storing their configuration, I don't think this needs to be in the spec. EDK2 for example doesn't always use a DT and I don't think they'll ever use it to store configuration information.
[0] https://developer.arm.com/documentation/den0118/b/?lang=en
Thanks /Ilias
Rob
Hi Ilias,
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Rob,
[...]
> > What is the point of removing them? Instead, we should make sure that > we upstream the bindings and encourage SoC vendors to sync them. If we > remove them, no one will bother and U-Boot just becomes a dumping > ground.
Well things like the binman entries in DT are U-Boot specific and not useful for HW related descriptions or for Linux or another OS being able to deal with HW so arguably we're already a dumping ground to some degree for not defining hardware.
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
For the record, I tend to agree.
+1
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
We cannot have it both ways, i.e. refusing to accept non-hardware bindings and then complaining that U-Boot does not pass schema validation. Devicetree should be a shared resource, not just for the use of Linux. We already have reserved-memory, flash layout and other things which don't relate to hardware. I would love to somehome get past this fundamental discussion which seems to come up every time we get close to making progress.
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Really, I don't want to have to care about the binman binding. If it is u-boot specific, then it should stay in u-boot. I took /options/u-boot/, but now I'm starting to have second thoughts on that being in dtschema if it is going to be continually and frequently extended. Validating it in SR does little. If a vendor is abusing /options/u-boot/ in some way they could just as easily remove the node in their u-boot fork to pass. SR is certainly not going to require the node be there.
On A/B updates, that really doesn't seem like a u-boot specific problem to me. No one wants A/B updates in EDK2 or anything else?
A/B updates might be implemented in EDK2 or any other bootloader that chooses to implement it. The metadata information a bootloader needs to implement it are documented here [0]. Those metadata are not part of the DT. If they were it would make sense to add them on the schema. The DT entry we are using though serves a different purpose. It tells the bootloader the location of the metadata (iow where can I read them from the disk). Since bootloaders have a different way of storing their configuration, I don't think this needs to be in the spec. EDK2 for example doesn't always use a DT and I don't think they'll ever use it to store configuration information.
In another thread with Rob involved, we are trying to provide bindings for EDK2 to use.
Whatever the efforts of those trying to fragment the firmware industry, there are efforts to bring it together with a consistent configuration story. Ultimately I believe these will have to prevail, as the complexity increases.
[0] https://developer.arm.com/documentation/den0118/b/?lang=en
Thanks /Ilias
Rob
Regards, Simon
Hi Simon,
On Thu, 7 Sept 2023 at 15:23, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Rob,
[...]
> > > > What is the point of removing them? Instead, we should make sure that > > we upstream the bindings and encourage SoC vendors to sync them. If we > > remove them, no one will bother and U-Boot just becomes a dumping > > ground. > > Well things like the binman entries in DT are U-Boot specific and not > useful for HW related descriptions or for Linux or another OS being > able to deal with HW so arguably we're already a dumping ground to > some degree for not defining hardware.
I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
For the record, I tend to agree.
+1
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
And that's what the firmware handoff was all about. I get what you are trying to do here. I am just aware of any other project apart from U-Boot which uses DT for it's own configuration. So trying to standardize some bindings that are useful to all projects that use DT is fine. Trying to *enforce* them to use it for config isn't IMHO.
We cannot have it both ways, i.e. refusing to accept non-hardware bindings and then complaining that U-Boot does not pass schema validation. Devicetree should be a shared resource, not just for the use of Linux.
It's not for the use of Linux, I've wasted enough time repeating that and so has Rob. Please go back to previous emails and read the arguments.
We already have reserved-memory, flash layout and other things which don't relate to hardware. I would love to somehome get past this fundamental discussion which seems to come up every time we get close to making progress
Most of the nodes we already have were used across projects and made sense to all of them. U-Boot might need to reserve some memory and so does linux etc etc. Some other nodes make nosense at all to and they just serve internal ABI implementation details. I can't possibly fathom how these would be justifiable. On top of all that, there's a huge danger here. How are you planning on separating arbitrary entries from various projects?
What I am afraid is going to happen here is simple. If a project doesn't use DT to configure itself and wants to provide a DT to U-Boot, then are you going to say "Can you please inject various DT nodes in the tree because U-Boot *needs* them and they are now part of the spec"? Anyway, it's not up to me to decide here, I am just saying what makes sense to me.
We need run-time configuration here, since we cannot know at build time what we will be asked to do by a previous firmware phase.
Really, I don't want to have to care about the binman binding. If it is u-boot specific, then it should stay in u-boot. I took /options/u-boot/, but now I'm starting to have second thoughts on that being in dtschema if it is going to be continually and frequently extended. Validating it in SR does little. If a vendor is abusing /options/u-boot/ in some way they could just as easily remove the node in their u-boot fork to pass. SR is certainly not going to require the node be there.
On A/B updates, that really doesn't seem like a u-boot specific problem to me. No one wants A/B updates in EDK2 or anything else?
A/B updates might be implemented in EDK2 or any other bootloader that chooses to implement it. The metadata information a bootloader needs to implement it are documented here [0]. Those metadata are not part of the DT. If they were it would make sense to add them on the schema. The DT entry we are using though serves a different purpose. It tells the bootloader the location of the metadata (iow where can I read them from the disk). Since bootloaders have a different way of storing their configuration, I don't think this needs to be in the spec. EDK2 for example doesn't always use a DT and I don't think they'll ever use it to store configuration information.
In another thread with Rob involved, we are trying to provide bindings for EDK2 to use.
Whatever the efforts of those trying to fragment the firmware industry, there are efforts to bring it together with a consistent configuration story. Ultimately I believe these will have to prevail, as the complexity increases.
Regards /Ilias
On Fri, Sep 08, 2023 at 01:13:42PM +0300, Ilias Apalodimas wrote:
Hi Simon,
On Thu, 7 Sept 2023 at 15:23, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Rob,
[...]
> > > > > > What is the point of removing them? Instead, we should make sure that > > > we upstream the bindings and encourage SoC vendors to sync them. If we > > > remove them, no one will bother and U-Boot just becomes a dumping > > > ground. > > > > Well things like the binman entries in DT are U-Boot specific and not > > useful for HW related descriptions or for Linux or another OS being > > able to deal with HW so arguably we're already a dumping ground to > > some degree for not defining hardware. > > I have started the process to upstream the binman bindings.
I don't think they should be in DT at all, they don't describe anything to do with hardware, or generally even the runtime of a device, they don't even describe the boot/runtime state of the firmware, they describe build time, so I don't see what that has to do with device tree? Can you explain that? To me those sorts of things should live in a build time style config file.
For the record, I tend to agree.
+1
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
And that's what the firmware handoff was all about. I get what you are trying to do here. I am just aware of any other
"just not aware" did you mean?
project apart from U-Boot which uses DT for it's own configuration. So trying to standardize some bindings that are useful to all projects that use DT is fine. Trying to *enforce* them to use it for config isn't IMHO.
We cannot have it both ways, i.e. refusing to accept non-hardware bindings and then complaining that U-Boot does not pass schema validation. Devicetree should be a shared resource, not just for the use of Linux.
It's not for the use of Linux, I've wasted enough time repeating that and so has Rob. Please go back to previous emails and read the arguments.
Right, it's not just for Linux, but Linux is where most of the exceptions to the "ONLY HARDWARE" rule got in, because they also make sense. And the overarching point Simon keeps trying to make I believe can be boiled down to that too. There are things that one does not have a (reasonable) choice about how to do things with when interacting with the hunk of melted sand on your desk and that information needs to go somewhere.
We already have reserved-memory, flash layout and other things which don't relate to hardware. I would love to somehome get past this fundamental discussion which seems to come up every time we get close to making progress
Most of the nodes we already have were used across projects and made sense to all of them. U-Boot might need to reserve some memory and so does linux etc etc. Some other nodes make nosense at all to and they just serve internal ABI implementation details. I can't possibly fathom how these would be justifiable. On top of all that, there's a huge danger here. How are you planning on separating arbitrary entries from various projects?
I think in some ways this is the whole point of at least what I'm asking for. It's fine to say "Here is the mechanism to remove nodes / properties from the device tree". BUT adding entries to that list MUST document where someone tried to upstream and explain that this is something that belongs in the device tree because it is useful to everyone.
What I am afraid is going to happen here is simple. If a project doesn't use DT to configure itself and wants to provide a DT to U-Boot, then are you going to say "Can you please inject various DT nodes in the tree because U-Boot *needs* them and they are now part of the spec"? Anyway, it's not up to me to decide here, I am just saying what makes sense to me.
What's the difference between that and "If a project doesn't use DT to configure itself and wants to provide a DT to Linux, ..." ?
Hi Tom,
On Fri, 8 Sept 2023 at 17:54, Tom Rini trini@konsulko.com wrote:
On Fri, Sep 08, 2023 at 01:13:42PM +0300, Ilias Apalodimas wrote:
Hi Simon,
On Thu, 7 Sept 2023 at 15:23, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Rob,
[...]
> > > > > > > > What is the point of removing them? Instead, we should make sure that > > > > we upstream the bindings and encourage SoC vendors to sync them. If we > > > > remove them, no one will bother and U-Boot just becomes a dumping > > > > ground. > > > > > > Well things like the binman entries in DT are U-Boot specific and not > > > useful for HW related descriptions or for Linux or another OS being > > > able to deal with HW so arguably we're already a dumping ground to > > > some degree for not defining hardware. > > > > I have started the process to upstream the binman bindings. > > I don't think they should be in DT at all, they don't describe > anything to do with hardware, or generally even the runtime of a > device, they don't even describe the boot/runtime state of the > firmware, they describe build time, so I don't see what that has to do > with device tree? Can you explain that? To me those sorts of things > should live in a build time style config file.
For the record, I tend to agree.
+1
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
And that's what the firmware handoff was all about. I get what you are trying to do here. I am just aware of any other
"just not aware" did you mean?
Yep, sorry!
project apart from U-Boot which uses DT for it's own configuration. So trying to standardize some bindings that are useful to all projects that use DT is fine. Trying to *enforce* them to use it for config isn't IMHO.
We cannot have it both ways, i.e. refusing to accept non-hardware bindings and then complaining that U-Boot does not pass schema validation. Devicetree should be a shared resource, not just for the use of Linux.
It's not for the use of Linux, I've wasted enough time repeating that and so has Rob. Please go back to previous emails and read the arguments.
Right, it's not just for Linux, but Linux is where most of the exceptions to the "ONLY HARDWARE" rule got in, because they also make sense.
Exactly.
And the overarching point Simon keeps trying to make I believe can be boiled down to that too. There are things that one does not have a (reasonable) choice about how to do things with when interacting with the hunk of melted sand on your desk and that information needs to go somewhere.
DT is a big hammer indeed, but that doesn't mean we always need to use it. I never disagreed with adding nodes that make sense and will be useful for others. For example, the internal Driver model configuration options used to enable a device early etc etc are probably useful to more projects. On the other hand, if U-Boot is indeed the only project using DT for its internal configuration why should we care?
For example, let's imagine you build TF-A, and TF-A is configured and bundled with a device tree that gets passed along to U-Boot (using OF_BOARD). Why on earth should TF-A be aware of internal DM implementation details and build a device tree containing u-boot,dm-pre-reloc, u-boot,dm-spl, dm-tpl, and every other non-upstreamed nodes we have? Another example would be the public key that we shoehorn on the DT. In commit ddf67daac39d ("efi_capsule: Move signature from DTB to .rodata") I tried to get rid of that because since I was aware of the dt-schema conformance and honestly having the capsule public portion of the key there makes little sense. Unfortunately, that got reverted in commit 47a25e81d35c8 with a bogus commit message as well. So again imagine building TF-A, which is a first-stage bootloader and has no understanding of UEFI whatsoever, asking the TF-A project to start injecting public keys around that has no idea why or how they will be used.
Can you imagine how the device tree would look like in a couple of years from now if we allow *every* project to add its own special config needs in there? So perhaps we should take a step back, agree that some level of config is needed, identify the common options, and add that to the spec instead of dumping everything that doesn't fit somewhere else in there.
We already have reserved-memory, flash layout and other things which don't relate to hardware. I would love to somehome get past this fundamental discussion which seems to come up every time we get close to making progress
Most of the nodes we already have were used across projects and made sense to all of them. U-Boot might need to reserve some memory and so does linux etc etc. Some other nodes make nosense at all to and they just serve internal ABI implementation details. I can't possibly fathom how these would be justifiable. On top of all that, there's a huge danger here. How are you planning on separating arbitrary entries from various projects?
I think in some ways this is the whole point of at least what I'm asking for. It's fine to say "Here is the mechanism to remove nodes / properties from the device tree". BUT adding entries to that list MUST document where someone tried to upstream and explain that this is something that belongs in the device tree because it is useful to everyone.
And we don't disagree on that either. That's why the link to the FWU discussion was there (although it should have been in a doc and not in a mail). I am not arguing against adding nodes, I am arguing that we shouldn't rush them and that there's zero chance that we manage to upstream everything and keep some level of sanity on the spec. So, since U-Boot is currently using the DT for its own configuration needs, not having the ability to provide a DT that conforms to the spec and hope that we can upstream everything will just delay all of SystemReady 2.0 conformance efforts (and is unrealistic IMHO).
What I am afraid is going to happen here is simple. If a project doesn't use DT to configure itself and wants to provide a DT to U-Boot, then are you going to say "Can you please inject various DT nodes in the tree because U-Boot *needs* them and they are now part of the spec"? Anyway, it's not up to me to decide here, I am just saying what makes sense to me.
What's the difference between that and "If a project doesn't use DT to configure itself and wants to provide a DT to Linux, ..." ?
See the example above with TF-A, I hope that clears it up.
Thanks /Ilias
-- Tom
On Fri, Sep 08, 2023 at 06:28:14PM +0300, Ilias Apalodimas wrote:
Hi Tom,
On Fri, 8 Sept 2023 at 17:54, Tom Rini trini@konsulko.com wrote:
On Fri, Sep 08, 2023 at 01:13:42PM +0300, Ilias Apalodimas wrote:
Hi Simon,
On Thu, 7 Sept 2023 at 15:23, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Rob,
[...]
> > > > > > > > > > What is the point of removing them? Instead, we should make sure that > > > > > we upstream the bindings and encourage SoC vendors to sync them. If we > > > > > remove them, no one will bother and U-Boot just becomes a dumping > > > > > ground. > > > > > > > > Well things like the binman entries in DT are U-Boot specific and not > > > > useful for HW related descriptions or for Linux or another OS being > > > > able to deal with HW so arguably we're already a dumping ground to > > > > some degree for not defining hardware. > > > > > > I have started the process to upstream the binman bindings. > > > > I don't think they should be in DT at all, they don't describe > > anything to do with hardware, or generally even the runtime of a > > device, they don't even describe the boot/runtime state of the > > firmware, they describe build time, so I don't see what that has to do > > with device tree? Can you explain that? To me those sorts of things > > should live in a build time style config file.
For the record, I tend to agree.
+1
> I beg to differ. Devicetree is more than just hardware and always has > been. See, for example the /chosen and /options nodes.
There are exceptions...
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
And that's what the firmware handoff was all about. I get what you are trying to do here. I am just aware of any other
"just not aware" did you mean?
Yep, sorry!
project apart from U-Boot which uses DT for it's own configuration. So trying to standardize some bindings that are useful to all projects that use DT is fine. Trying to *enforce* them to use it for config isn't IMHO.
We cannot have it both ways, i.e. refusing to accept non-hardware bindings and then complaining that U-Boot does not pass schema validation. Devicetree should be a shared resource, not just for the use of Linux.
It's not for the use of Linux, I've wasted enough time repeating that and so has Rob. Please go back to previous emails and read the arguments.
Right, it's not just for Linux, but Linux is where most of the exceptions to the "ONLY HARDWARE" rule got in, because they also make sense.
Exactly.
And the overarching point Simon keeps trying to make I believe can be boiled down to that too. There are things that one does not have a (reasonable) choice about how to do things with when interacting with the hunk of melted sand on your desk and that information needs to go somewhere.
DT is a big hammer indeed, but that doesn't mean we always need to use it. I never disagreed with adding nodes that make sense and will be useful for others. For example, the internal Driver model configuration options used to enable a device early etc etc are probably useful to more projects. On the other hand, if U-Boot is indeed the only project using DT for its internal configuration why should we care?
For example, let's imagine you build TF-A, and TF-A is configured and bundled with a device tree that gets passed along to U-Boot (using OF_BOARD). Why on earth should TF-A be aware of internal DM implementation details and build a device tree containing u-boot,dm-pre-reloc, u-boot,dm-spl, dm-tpl, and every other non-upstreamed nodes we have?
I don't think this is a clear example, sorry. "dm-pre-reloc" etc are the bootph things now that you say could be useful. So they're an example of how (now that things are more receptive) we need to look at what U-Boot has that doesn't pass validation and see "does this make sense, today" or not.
I guess I'm confused as to why it's a theoretical problem for TF-A to pass along /binman/ but not a problem to pass along /soc/.../snvs/.../linux,snvs_pwrkey on i.MX8. _Sometimes_ internals just need to be there. That also does not mean every single should be there.
Another example would be the public key that we shoehorn on the DT. In commit ddf67daac39d ("efi_capsule: Move signature from DTB to .rodata") I tried to get rid of that because since I was aware of the dt-schema conformance and honestly having the capsule public portion of the key there makes little sense. Unfortunately, that got reverted in commit 47a25e81d35c8 with a bogus commit message as well. So again imagine building TF-A, which is a first-stage bootloader and has no understanding of UEFI whatsoever, asking the TF-A project to start injecting public keys around that has no idea why or how they will be used.
Can you imagine how the device tree would look like in a couple of years from now if we allow *every* project to add its own special config needs in there? So perhaps we should take a step back, agree that some level of config is needed, identify the common options, and add that to the spec instead of dumping everything that doesn't fit somewhere else in there.
Part of the problem here and now with capsule update stuff seems to be that, well, I don't know what the heck we should do. It's a "lovely" specification defined feature and so I honestly don't know how much leeway we have for how we can and can't represent and implement the portions that are left up to the implementation or board specific. I don't see why TF-A would inject something that should have been present already? And/or ...
We already have reserved-memory, flash layout and other things which don't relate to hardware. I would love to somehome get past this fundamental discussion which seems to come up every time we get close to making progress
Most of the nodes we already have were used across projects and made sense to all of them. U-Boot might need to reserve some memory and so does linux etc etc. Some other nodes make nosense at all to and they just serve internal ABI implementation details. I can't possibly fathom how these would be justifiable. On top of all that, there's a huge danger here. How are you planning on separating arbitrary entries from various projects?
I think in some ways this is the whole point of at least what I'm asking for. It's fine to say "Here is the mechanism to remove nodes / properties from the device tree". BUT adding entries to that list MUST document where someone tried to upstream and explain that this is something that belongs in the device tree because it is useful to everyone.
And we don't disagree on that either. That's why the link to the FWU discussion was there (although it should have been in a doc and not in a mail). I am not arguing against adding nodes, I am arguing that we shouldn't rush them and that there's zero chance that we manage to upstream everything and keep some level of sanity on the spec. So, since U-Boot is currently using the DT for its own configuration needs, not having the ability to provide a DT that conforms to the spec and hope that we can upstream everything will just delay all of SystemReady 2.0 conformance efforts (and is unrealistic IMHO).
The first problem is how does the capsule update specification specify handling the stuff that we put in the FWU nodes that we then need to delete?
The second problem is that I don't want the discussion link to just be in the cover letter, I want it in the tree, in documentation and heck, an unused-by-the-compiler parameter in the macro that adds a node to delete that is the rST file that documents the "we tried, it was rejected, this still makes sense" or whatever is appropriate as to why we're deleting the node. Cheaters shall cheat here, yes, but upstream will have a record of trying.
Hi Tom,
[...]
> > > I don't think they should be in DT at all, they don't describe > > > anything to do with hardware, or generally even the runtime of a > > > device, they don't even describe the boot/runtime state of the > > > firmware, they describe build time, so I don't see what that has to do > > > with device tree? Can you explain that? To me those sorts of things > > > should live in a build time style config file. > > For the record, I tend to agree. >
+1
> > I beg to differ. Devicetree is more than just hardware and always has > > been. See, for example the /chosen and /options nodes. > > There are exceptions... >
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
And that's what the firmware handoff was all about. I get what you are trying to do here. I am just aware of any other
"just not aware" did you mean?
Yep, sorry!
project apart from U-Boot which uses DT for it's own configuration. So trying to standardize some bindings that are useful to all projects that use DT is fine. Trying to *enforce* them to use it for config isn't IMHO.
We cannot have it both ways, i.e. refusing to accept non-hardware bindings and then complaining that U-Boot does not pass schema validation. Devicetree should be a shared resource, not just for the use of Linux.
It's not for the use of Linux, I've wasted enough time repeating that and so has Rob. Please go back to previous emails and read the arguments.
Right, it's not just for Linux, but Linux is where most of the exceptions to the "ONLY HARDWARE" rule got in, because they also make sense.
Exactly.
And the overarching point Simon keeps trying to make I believe can be boiled down to that too. There are things that one does not have a (reasonable) choice about how to do things with when interacting with the hunk of melted sand on your desk and that information needs to go somewhere.
DT is a big hammer indeed, but that doesn't mean we always need to use it. I never disagreed with adding nodes that make sense and will be useful for others. For example, the internal Driver model configuration options used to enable a device early etc etc are probably useful to more projects. On the other hand, if U-Boot is indeed the only project using DT for its internal configuration why should we care?
For example, let's imagine you build TF-A, and TF-A is configured and bundled with a device tree that gets passed along to U-Boot (using OF_BOARD). Why on earth should TF-A be aware of internal DM implementation details and build a device tree containing u-boot,dm-pre-reloc, u-boot,dm-spl, dm-tpl, and every other non-upstreamed nodes we have?
I don't think this is a clear example, sorry. "dm-pre-reloc" etc are the bootph things now that you say could be useful. So they're an example of how (now that things are more receptive) we need to look at what U-Boot has that doesn't pass validation and see "does this make sense, today" or not.
The point here is a bit different though. We need this in U-Boot *because* we use the DT to configure things. They are useful information, but unless another bootloader uses the same config method, U-Boot is the only consumer.
If we could split those nodes in an internal u-boot .dtsi file that would be much much cleaner. But IIRC we'll have problems with patching DTs in TPL/SPL with limited memory.
I guess I'm confused as to why it's a theoretical problem for TF-A to pass along /binman/ but not a problem to pass along /soc/.../snvs/.../linux,snvs_pwrkey on i.MX8. _Sometimes_ internals
It's the same problem and I don't think it's ok for TF-A to pass those as either.
just need to be there. That also does not mean every single should be there.
Another example would be the public key that we shoehorn on the DT. In commit ddf67daac39d ("efi_capsule: Move signature from DTB to .rodata") I tried to get rid of that because since I was aware of the dt-schema conformance and honestly having the capsule public portion of the key there makes little sense. Unfortunately, that got reverted in commit 47a25e81d35c8 with a bogus commit message as well. So again imagine building TF-A, which is a first-stage bootloader and has no understanding of UEFI whatsoever, asking the TF-A project to start injecting public keys around that has no idea why or how they will be used.
Can you imagine how the device tree would look like in a couple of years from now if we allow *every* project to add its own special config needs in there? So perhaps we should take a step back, agree that some level of config is needed, identify the common options, and add that to the spec instead of dumping everything that doesn't fit somewhere else in there.
Part of the problem here and now with capsule update stuff seems to be that, well, I don't know what the heck we should do. It's a "lovely" specification defined feature and so I honestly don't know how much leeway we have for how we can and can't represent and implement the portions that are left up to the implementation or board specific.
Heinrich and I can help on that. In short, the capsule update chapter doesn't define where the key should be stored. It should obviously be on a tamper resistant medium and it has a specific format, but that's as far as the spec would go.
I don't see why TF-A would inject something that should have been present already? And/or ...
I am not following you here. The public key is unique per class of devices. If someone builds TF-A and decides to provide a DTB though that, you then need to, unpack the TF-A binary when you build U-Boot, amend it and repack it via binman. On top of that, those binaries will probably be signed, so the repacking exercise becomes pretty painful.
We already have reserved-memory, flash layout and other things which don't relate to hardware. I would love to somehome get past this fundamental discussion which seems to come up every time we get close to making progress
Most of the nodes we already have were used across projects and made sense to all of them. U-Boot might need to reserve some memory and so does linux etc etc. Some other nodes make nosense at all to and they just serve internal ABI implementation details. I can't possibly fathom how these would be justifiable. On top of all that, there's a huge danger here. How are you planning on separating arbitrary entries from various projects?
I think in some ways this is the whole point of at least what I'm asking for. It's fine to say "Here is the mechanism to remove nodes / properties from the device tree". BUT adding entries to that list MUST document where someone tried to upstream and explain that this is something that belongs in the device tree because it is useful to everyone.
We never disagreed on that. I already said that the FWU link Sughosh sent in the cover letter should be added on a doc. But that's irrelevant to 'hard NAKing' patches [0]. It's also the complete opposite of what we are discussing here, since AFAICT you are fine with the removal mechanism as long as the nodes-to-be-removed are documented properly and there has been an upstream effort of those beforehand.
And we don't disagree on that either. That's why the link to the FWU discussion was there (although it should have been in a doc and not in a mail). I am not arguing against adding nodes, I am arguing that we shouldn't rush them and that there's zero chance that we manage to upstream everything and keep some level of sanity on the spec. So, since U-Boot is currently using the DT for its own configuration needs, not having the ability to provide a DT that conforms to the spec and hope that we can upstream everything will just delay all of SystemReady 2.0 conformance efforts (and is unrealistic IMHO).
The first problem is how does the capsule update specification specify handling the stuff that we put in the FWU nodes that we then need to delete?
It doesn't. The A/B update support is not part of the spec. FWU and the A/B updates are designed on top of the EFI spec to provide an easy way to do firmware updates without bricking the board while at the same time provide rollback protection.
The second problem is that I don't want the discussion link to just be in the cover letter, I want it in the tree, in documentation and heck, an unused-by-the-compiler parameter in the macro that adds a node to delete that is the rST file that documents the "we tried, it was rejected, this still makes sense" or whatever is appropriate as to why we're deleting the node. Cheaters shall cheat here, yes, but upstream will have a record of trying.
Again, we never disagreed on that
[0] https://lore.kernel.org/u-boot/CAPnjgZ3AexW4vfO-A8WYGE0OD5EZnOUA7tA1QP71Bcw5...
Thanks /Ilias
-- Tom
Hi Simon,
On Thu, Sep 7, 2023 at 3:08 PM Simon Glass sjg@chromium.org wrote:
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
Sure, programmatically we can pass any data/info via DT, however it is only meant to map hardware features onto data structures.
devicetree.org landing page "The devicetree is a data structure for describing hardware."
devicetree-specification-v0.3.pdf Chapter-2 Line-1 "DTSpec specifies a construct called a devicetree to describe system hardware."
If we want to digress from the spec, we need the majority of developers to switch sides :) which is unlikely to happen and rightly so, imho.
regards.
On Fri, Sep 08, 2023 at 09:37:12AM -0500, Jassi Brar wrote:
Hi Simon,
On Thu, Sep 7, 2023 at 3:08 PM Simon Glass sjg@chromium.org wrote:
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
Sure, programmatically we can pass any data/info via DT, however it is only meant to map hardware features onto data structures.
devicetree.org landing page "The devicetree is a data structure for describing hardware."
devicetree-specification-v0.3.pdf Chapter-2 Line-1 "DTSpec specifies a construct called a devicetree to describe system hardware."
If we want to digress from the spec, we need the majority of developers to switch sides :) which is unlikely to happen and rightly so, imho.
It's the same language-lawyering that's been going on since device trees moved from the neat thing Apple-based PowerPC platforms provided (and extended), and then got ad-hoc'd to support other PowerPC platforms, and then was used to solve "Linus is sick of the patch conflicts on ARM" problems. And everyone is tired of talking about the exceptions to "it must be hardware" because if your software isn't where the software-burned-into-the-hardware wants it to be, the hardware won't work and so on.
From: Jassi Brar jassisinghbrar@gmail.com Date: Fri, 8 Sep 2023 09:37:12 -0500
Hi Simon,
On Thu, Sep 7, 2023 at 3:08 PM Simon Glass sjg@chromium.org wrote:
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
Sure, programmatically we can pass any data/info via DT, however it is only meant to map hardware features onto data structures.
devicetree.org landing page "The devicetree is a data structure for describing hardware."
devicetree-specification-v0.3.pdf Chapter-2 Line-1 "DTSpec specifies a construct called a devicetree to describe system hardware."
But it doesn't say that it describes *only* hardware. And it does describe more than just hardware. There is /chosen to specify firmware configuration and there are several examples of device tree bindings that describe non-discoverable firmware interfaces in the Linux tree. And it has been that way since the days of IEEE 1275. There are also bindings describing things like the firmware partition layout.
If we want to digress from the spec, we need the majority of developers to switch sides :) which is unlikely to happen and rightly so, imho.
It isn't even clear what the spec is. There is the document you reference above, there are the yaml files at https://github.com/devicetree-org/dt-schema and then there are additional yaml files in the Linux tree. As far as I know it isn't written down anywhere that those are the only places where device tree bindings can live.
Anyway, let's face it, there is some consensus among developers that what Simon has done in U-Boot is pushing the use of devicetree beyond the point where a significant fraction of developers thinks it makes sense. And I think I agree with that. But you can't beat him with the spec to make your point.
Now the devicetree is cleverly constructed such that it is possible to define additional bindings without the risk of conflicting with bindings developed by other parties. In particular if U-Boot is augmenting a device tree with properties that are prefixed with "u-boot," this isn't going to hurt an operating system that uses such an augmented device tree.
The real problem is that some folks developed a certification program that allegedly requires schema verification and now propose adding code to U-Boot that doesn't really solve any problem. My proposed solution would be to change said certification program to allow firmware to augment the device tree with properties and nodes with compatibles that are in the namespace controlled by the firmware.
Cheers,
Mark
On Fri, Sep 8, 2023 at 1:06 PM Mark Kettenis mark.kettenis@xs4all.nl wrote:
From: Jassi Brar jassisinghbrar@gmail.com Date: Fri, 8 Sep 2023 09:37:12 -0500
Hi Simon,
On Thu, Sep 7, 2023 at 3:08 PM Simon Glass sjg@chromium.org wrote:
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas
I beg to differ. Devicetree is more than just hardware and always has been. See, for example the /chosen and /options nodes.
There are exceptions...
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
Sure, programmatically we can pass any data/info via DT, however it is only meant to map hardware features onto data structures.
devicetree.org landing page "The devicetree is a data structure for describing hardware."
devicetree-specification-v0.3.pdf Chapter-2 Line-1 "DTSpec specifies a construct called a devicetree to describe system hardware."
But it doesn't say that it describes *only* hardware. And it does describe more than just hardware. There is /chosen to specify firmware configuration and there are several examples of device tree bindings that describe non-discoverable firmware interfaces in the Linux tree. And it has been that way since the days of IEEE 1275. There are also bindings describing things like the firmware partition layout.
Yes. The exact title for 1275[1] is: IEEE Standard for Boot (Initialization Configuration) Firmware: Core Requirements and Practices
I see "configuration" in there. However, in the OF case, it's generally how firmware configured the hardware and what it discovered. That's a little different than telling the OS how to configure the hardware which is what we do with FDT. Then there's stuff that's how to configure Linux which we try to reject.
Once we get into configuration of the OS/client (including u-boot), making that an ABI is a lot harder and if we use DT for that, I don't think it should be an ABI.
If we want to digress from the spec, we need the majority of developers to switch sides :) which is unlikely to happen and rightly so, imho.
It isn't even clear what the spec is. There is the document you reference above, there are the yaml files at https://github.com/devicetree-org/dt-schema and then there are additional yaml files in the Linux tree. As far as I know it isn't written down anywhere that those are the only places where device tree bindings can live.
There's not any restriction.
My intention with dtschema schemas is to only have "spec level" schemas. (Stuff we'd add to DTSpec (but don't because no one wants to write specs).) Hardware specific stuff lives somewhere else. That happens to be the Linux tree because that is where all the h/w support is (nothing else is close (by far)). Last I checked, we've got ~3700 schemas and ~1500 unconverted bindings.
Anyway, let's face it, there is some consensus among developers that what Simon has done in U-Boot is pushing the use of devicetree beyond the point where a significant fraction of developers thinks it makes sense. And I think I agree with that. But you can't beat him with the spec to make your point.
Now the devicetree is cleverly constructed such that it is possible to define additional bindings without the risk of conflicting with bindings developed by other parties. In particular if U-Boot is augmenting a device tree with properties that are prefixed with "u-boot," this isn't going to hurt an operating system that uses such an augmented device tree.
Until someone has some great idea to start using them. If the OS doesn't need something, then why pass it in the first place? What purpose does it serve? It's an invitation for someone somewhere to start using them.
The downside of keeping the nodes is it creates an ABI where there doesn't need to be one necessarily.
The real problem is that some folks developed a certification program that allegedly requires schema verification and now propose adding code to U-Boot that doesn't really solve any problem. My proposed solution would be to change said certification program to allow firmware to augment the device tree with properties and nodes with compatibles that are in the namespace controlled by the firmware.
I don't think we should decide what to do here based on said certification program. It can adapt to what's decided. Probably, the /option nodes will be stripped out or ignored for certification.
I accepted u-boot options node schema into dtschema, but now have second thoughts on that. Now I'm getting more u-boot specific (perhaps, not clear) stuff and widevine stuff internal to a TEE.
Rob
Hi Rob,
On Wed, 13 Sept 2023 at 16:39, Rob Herring robh@kernel.org wrote:
On Fri, Sep 8, 2023 at 1:06 PM Mark Kettenis mark.kettenis@xs4all.nl wrote:
From: Jassi Brar jassisinghbrar@gmail.com Date: Fri, 8 Sep 2023 09:37:12 -0500
Hi Simon,
On Thu, Sep 7, 2023 at 3:08 PM Simon Glass sjg@chromium.org wrote:
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas
> I beg to differ. Devicetree is more than just hardware and always has > been. See, for example the /chosen and /options nodes.
There are exceptions...
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
Sure, programmatically we can pass any data/info via DT, however it is only meant to map hardware features onto data structures.
devicetree.org landing page "The devicetree is a data structure for describing hardware."
devicetree-specification-v0.3.pdf Chapter-2 Line-1 "DTSpec specifies a construct called a devicetree to describe system hardware."
But it doesn't say that it describes *only* hardware. And it does describe more than just hardware. There is /chosen to specify firmware configuration and there are several examples of device tree bindings that describe non-discoverable firmware interfaces in the Linux tree. And it has been that way since the days of IEEE 1275. There are also bindings describing things like the firmware partition layout.
Yes. The exact title for 1275[1] is: IEEE Standard for Boot (Initialization Configuration) Firmware: Core Requirements and Practices
I see "configuration" in there. However, in the OF case, it's generally how firmware configured the hardware and what it discovered. That's a little different than telling the OS how to configure the hardware which is what we do with FDT.
For the /options node it says "The properties of this node are the system’s configuration variables."
Then there is section 7.4.4 which has a large list of options which don't seem to be so narrowly defined.
In any case, this is not quite the point, which IMO is that we need DT to support firmware use cases, whether or not a 29-year-old spec thought of it or not. In fact it is amazing to me how forward-looking Open Firmware was.
Then there's stuff that's how to configure Linux which we try to reject.
Fair enough: Linux has user-space for that.
Once we get into configuration of the OS/client (including u-boot), making that an ABI is a lot harder and if we use DT for that, I don't think it should be an ABI.
If we want to digress from the spec, we need the majority of developers to switch sides :) which is unlikely to happen and rightly so, imho.
It isn't even clear what the spec is. There is the document you reference above, there are the yaml files at https://github.com/devicetree-org/dt-schema and then there are additional yaml files in the Linux tree. As far as I know it isn't written down anywhere that those are the only places where device tree bindings can live.
There's not any restriction.
My intention with dtschema schemas is to only have "spec level" schemas. (Stuff we'd add to DTSpec (but don't because no one wants to write specs).) Hardware specific stuff lives somewhere else. That happens to be the Linux tree because that is where all the h/w support is (nothing else is close (by far)). Last I checked, we've got ~3700 schemas and ~1500 unconverted bindings.
That scope is quite a bit narrower than I understood it to be. It seems to leave a significant gap between the Linux repo and yours.
Anyway, let's face it, there is some consensus among developers that what Simon has done in U-Boot is pushing the use of devicetree beyond the point where a significant fraction of developers thinks it makes sense. And I think I agree with that. But you can't beat him with the spec to make your point.
Now the devicetree is cleverly constructed such that it is possible to define additional bindings without the risk of conflicting with bindings developed by other parties. In particular if U-Boot is augmenting a device tree with properties that are prefixed with "u-boot," this isn't going to hurt an operating system that uses such an augmented device tree.
Until someone has some great idea to start using them. If the OS doesn't need something, then why pass it in the first place? What purpose does it serve? It's an invitation for someone somewhere to start using them.
The downside of keeping the nodes is it creates an ABI where there doesn't need to be one necessarily.
I'd love to get away from the idea that DT is just for the OS. We are trying to use it for firmware-to-firmware communication, too. The programs on each side of that interface may be different projects. For that, we do need to have a binding, otherwise we end up with series like this one.
The real problem is that some folks developed a certification program that allegedly requires schema verification and now propose adding code to U-Boot that doesn't really solve any problem. My proposed solution would be to change said certification program to allow firmware to augment the device tree with properties and nodes with compatibles that are in the namespace controlled by the firmware.
I don't think we should decide what to do here based on said certification program. It can adapt to what's decided. Probably, the /option nodes will be stripped out or ignored for certification.
I accepted u-boot options node schema into dtschema, but now have second thoughts on that. Now I'm getting more u-boot specific (perhaps, not clear) stuff and widevine stuff internal to a TEE.
Where should these bindings go such that ARM / Linaro are not trying to remove them? I would be OK with moving them out somewhere else, but how are people supposed to deal with such fragmentation? My understanding was that dt-schema was an attempt to set up a neutral area where bindings could be accepted that were not just for Linux...did that change?
U-Boot is certainly the firmware project that is trying hardest to clean up things in this area...but it isn't the only consumer of these bindings. Given that we invent a new bootloader about every 6 months, I'm pretty sure the number of consumers will grow.
Regards, Simon
On Thu, Sep 14, 2023 at 04:41:43PM -0600, Simon Glass wrote:
Hi Rob,
On Wed, 13 Sept 2023 at 16:39, Rob Herring robh@kernel.org wrote:
[snip]
I don't think we should decide what to do here based on said certification program. It can adapt to what's decided. Probably, the /option nodes will be stripped out or ignored for certification.
I accepted u-boot options node schema into dtschema, but now have second thoughts on that. Now I'm getting more u-boot specific (perhaps, not clear) stuff and widevine stuff internal to a TEE.
Where should these bindings go such that ARM / Linaro are not trying to remove them? I would be OK with moving them out somewhere else, but how are people supposed to deal with such fragmentation? My understanding was that dt-schema was an attempt to set up a neutral area where bindings could be accepted that were not just for Linux...did that change?
Well, part of the problem here is that I've been talking with Ilias more about what's intended here and the fwu-* stuff that Rob rejected is indeed not right. We should drop it and replace it with something that really addresses the underlying problem (which is how do you know how/where to find some GUIDs) and I think we think it's something that can be shared between projects too and so be easier to convince Rob that the next form of it is right (or the right direction).
+CC Jose who's maintaining the metadata spec from Arm side.
On Fri, 15 Sept 2023 at 02:38, Tom Rini trini@konsulko.com wrote:
On Thu, Sep 14, 2023 at 04:41:43PM -0600, Simon Glass wrote:
Hi Rob,
On Wed, 13 Sept 2023 at 16:39, Rob Herring robh@kernel.org wrote:
[snip]
I don't think we should decide what to do here based on said certification program. It can adapt to what's decided. Probably, the /option nodes will be stripped out or ignored for certification.
I accepted u-boot options node schema into dtschema, but now have second thoughts on that. Now I'm getting more u-boot specific (perhaps, not clear) stuff and widevine stuff internal to a TEE.
Where should these bindings go such that ARM / Linaro are not trying to remove them? I would be OK with moving them out somewhere else, but how are people supposed to deal with such fragmentation? My understanding was that dt-schema was an attempt to set up a neutral area where bindings could be accepted that were not just for Linux...did that change?
Well, part of the problem here is that I've been talking with Ilias more about what's intended here and the fwu-* stuff that Rob rejected is indeed not right. We should drop it and replace it with something that really addresses the underlying problem (which is how do you know how/where to find some GUIDs) and I think we think it's something that can be shared between projects too and so be easier to convince Rob that the next form of it is right (or the right direction).
Tom, I gave the discussion we had last night a bit of thought (thanks for that) and talked with Jose. The spec as is written right now recommends a GPT-based partition. It then has various GPT UUIDs that indicate where to find the metadata as well as the various firmware banks that participate in the whole scheme. If you have that GPT, then the only thing you really need is to find the metadata UUID and handle the various banks.
However, the synquacer which boots from a NOR has no GPT. What that fwu node entry does, in order to abstract the rest of the implementation, is map UUIIDs <-> offset + length. That way the core code still thinks it's trying to discover UUIDs but the eventual reads/writes end up on a NOR. That box boots via SCP [0] , which has no understanding of device trees, instead they hardcode the UUIDS, offsets etc for the Synquacer. But if they ever wanted to switch over having that information would come in handy.
Jose and I discussed this a bit. I'll backpaddle and have someone look into upstreaming the fwu part. There are a few changes required, since we already have some MTD information in the DT and those entries would make more sense there instead of inventing a new node, but we can discuss this when the patches are sent. Once those are sent, we can add a recommendation on the spec, pointing to that DT entry for any future early stage boot loaders that want to implement it, pointing to the DT entry.
That only solves a fraction of the problem though. Other nodes, like the EFI public key etc. still need to be removed.
[0] https://github.com/ARM-software/SCP-firmware
Thanks /Ilias
-- Tom
On Thu, Sep 14, 2023 at 5:42 PM Simon Glass sjg@chromium.org wrote:
Hi Rob,
On Wed, 13 Sept 2023 at 16:39, Rob Herring robh@kernel.org wrote:
On Fri, Sep 8, 2023 at 1:06 PM Mark Kettenis mark.kettenis@xs4all.nl wrote:
From: Jassi Brar jassisinghbrar@gmail.com Date: Fri, 8 Sep 2023 09:37:12 -0500
Hi Simon,
On Thu, Sep 7, 2023 at 3:08 PM Simon Glass sjg@chromium.org wrote:
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas
> > I beg to differ. Devicetree is more than just hardware and always has > > been. See, for example the /chosen and /options nodes. > > There are exceptions... >
We've been this over and over again and frankly it gets a bit annoying. It's called *DEVICE* tree for a reason. As Rob pointed out there are exceptions, but those made a lot of sense. Having arbitrary internal ABI stuff of various projects in the schema just defeats the definition of a spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
Sure, programmatically we can pass any data/info via DT, however it is only meant to map hardware features onto data structures.
devicetree.org landing page "The devicetree is a data structure for describing hardware."
devicetree-specification-v0.3.pdf Chapter-2 Line-1 "DTSpec specifies a construct called a devicetree to describe system hardware."
But it doesn't say that it describes *only* hardware. And it does describe more than just hardware. There is /chosen to specify firmware configuration and there are several examples of device tree bindings that describe non-discoverable firmware interfaces in the Linux tree. And it has been that way since the days of IEEE 1275. There are also bindings describing things like the firmware partition layout.
Yes. The exact title for 1275[1] is: IEEE Standard for Boot (Initialization Configuration) Firmware: Core Requirements and Practices
I see "configuration" in there. However, in the OF case, it's generally how firmware configured the hardware and what it discovered. That's a little different than telling the OS how to configure the hardware which is what we do with FDT.
For the /options node it says "The properties of this node are the system’s configuration variables."
Then there is section 7.4.4 which has a large list of options which don't seem to be so narrowly defined.
In any case, this is not quite the point, which IMO is that we need DT to support firmware use cases, whether or not a 29-year-old spec thought of it or not. In fact it is amazing to me how forward-looking Open Firmware was.
Then there's stuff that's how to configure Linux which we try to reject.
Fair enough: Linux has user-space for that.
Yep, the question I usually ask is who needs to configure something and what determines the config. Changing things with sysfs is much easier than changing the DT provided by firmware.
Once we get into configuration of the OS/client (including u-boot), making that an ABI is a lot harder and if we use DT for that, I don't think it should be an ABI.
If we want to digress from the spec, we need the majority of developers to switch sides :) which is unlikely to happen and rightly so, imho.
It isn't even clear what the spec is. There is the document you reference above, there are the yaml files at https://github.com/devicetree-org/dt-schema and then there are additional yaml files in the Linux tree. As far as I know it isn't written down anywhere that those are the only places where device tree bindings can live.
There's not any restriction.
My intention with dtschema schemas is to only have "spec level" schemas. (Stuff we'd add to DTSpec (but don't because no one wants to write specs).) Hardware specific stuff lives somewhere else. That happens to be the Linux tree because that is where all the h/w support is (nothing else is close (by far)). Last I checked, we've got ~3700 schemas and ~1500 unconverted bindings.
That scope is quite a bit narrower than I understood it to be. It seems to leave a significant gap between the Linux repo and yours.
Anyway, let's face it, there is some consensus among developers that what Simon has done in U-Boot is pushing the use of devicetree beyond the point where a significant fraction of developers thinks it makes sense. And I think I agree with that. But you can't beat him with the spec to make your point.
Now the devicetree is cleverly constructed such that it is possible to define additional bindings without the risk of conflicting with bindings developed by other parties. In particular if U-Boot is augmenting a device tree with properties that are prefixed with "u-boot," this isn't going to hurt an operating system that uses such an augmented device tree.
Until someone has some great idea to start using them. If the OS doesn't need something, then why pass it in the first place? What purpose does it serve? It's an invitation for someone somewhere to start using them.
The downside of keeping the nodes is it creates an ABI where there doesn't need to be one necessarily.
I'd love to get away from the idea that DT is just for the OS. We are trying to use it for firmware-to-firmware communication, too. The programs on each side of that interface may be different projects. For that, we do need to have a binding, otherwise we end up with series like this one.
I don't think DT is just for the OS, but DT for the OS is an ABI and other cases may not be. Or they just don't need to follow all the same rules. Zephyr is doing their own thing for example.
I don't think /options/u-boot is an ABI. AIUI, u-boot populates the node and consumes it. No ABI there (or limited to SPL to u-boot proper). I suppose a prior firmware stage could populate it, but that doesn't scale as then the prior stage has to know details of the next stage.
The real problem is that some folks developed a certification program that allegedly requires schema verification and now propose adding code to U-Boot that doesn't really solve any problem. My proposed solution would be to change said certification program to allow firmware to augment the device tree with properties and nodes with compatibles that are in the namespace controlled by the firmware.
I don't think we should decide what to do here based on said certification program. It can adapt to what's decided. Probably, the /option nodes will be stripped out or ignored for certification.
I accepted u-boot options node schema into dtschema, but now have second thoughts on that. Now I'm getting more u-boot specific (perhaps, not clear) stuff and widevine stuff internal to a TEE.
Where should these bindings go such that ARM / Linaro are not trying to remove them? I would be OK with moving them out somewhere else, but how are people supposed to deal with such fragmentation? My understanding was that dt-schema was an attempt to set up a neutral area where bindings could be accepted that were not just for Linux...did that change?
No change, just not communicated I guess. And again, bindings are not "just for Linux". They are hosted there because that is where *all* the hardware support is (by far). But we'll probably never get past the "Linux binding" perception no matter what we do or how many times I say it.
To rephrase things a bit, I'm happy to host anything that's multi-project, not a large number of bindings, and not a device/hardware specific binding. The device specific bindings live in the kernel tree primarily. For any new binding (foos/#foo-cell type ones), I want to see multiple users. Really for these, probably best to start with them in Linux repo (or elsewhere) and then promote them to dtschema. That gives a little flexibility in changing/abandoning them.
Removing nodes and/or properties and where things live are mostly independent discussions. SystemReady can adapt to whatever is decided for the former. In general, IMO, when passing DT from stage N to N+1, the N+1 stage should remove things which only apply to N+2 stage. For example, kexec removes /chosen and recreates it for the next kernel.
Rob
Hi Rob,
On Mon, 18 Sept 2023 at 11:00, Rob Herring robh@kernel.org wrote:
On Thu, Sep 14, 2023 at 5:42 PM Simon Glass sjg@chromium.org wrote:
Hi Rob,
On Wed, 13 Sept 2023 at 16:39, Rob Herring robh@kernel.org wrote:
On Fri, Sep 8, 2023 at 1:06 PM Mark Kettenis mark.kettenis@xs4all.nl wrote:
From: Jassi Brar jassisinghbrar@gmail.com Date: Fri, 8 Sep 2023 09:37:12 -0500
Hi Simon,
On Thu, Sep 7, 2023 at 3:08 PM Simon Glass sjg@chromium.org wrote:
On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas > > > > I beg to differ. Devicetree is more than just hardware and always has > > > been. See, for example the /chosen and /options nodes. > > > > There are exceptions... > > > > We've been this over and over again and frankly it gets a bit annoying. > It's called *DEVICE* tree for a reason. As Rob pointed out there are > exceptions, but those made a lot of sense. Having arbitrary internal ABI > stuff of various projects in the schema just defeats the definition of a > spec.
Our efforts should not just be about internal ABI, but working to provide a consistent configuration system for all firmware elements.
Sure, programmatically we can pass any data/info via DT, however it is only meant to map hardware features onto data structures.
devicetree.org landing page "The devicetree is a data structure for describing hardware."
devicetree-specification-v0.3.pdf Chapter-2 Line-1 "DTSpec specifies a construct called a devicetree to describe system hardware."
But it doesn't say that it describes *only* hardware. And it does describe more than just hardware. There is /chosen to specify firmware configuration and there are several examples of device tree bindings that describe non-discoverable firmware interfaces in the Linux tree. And it has been that way since the days of IEEE 1275. There are also bindings describing things like the firmware partition layout.
Yes. The exact title for 1275[1] is: IEEE Standard for Boot (Initialization Configuration) Firmware: Core Requirements and Practices
I see "configuration" in there. However, in the OF case, it's generally how firmware configured the hardware and what it discovered. That's a little different than telling the OS how to configure the hardware which is what we do with FDT.
For the /options node it says "The properties of this node are the system’s configuration variables."
Then there is section 7.4.4 which has a large list of options which don't seem to be so narrowly defined.
In any case, this is not quite the point, which IMO is that we need DT to support firmware use cases, whether or not a 29-year-old spec thought of it or not. In fact it is amazing to me how forward-looking Open Firmware was.
Then there's stuff that's how to configure Linux which we try to reject.
Fair enough: Linux has user-space for that.
Yep, the question I usually ask is who needs to configure something and what determines the config. Changing things with sysfs is much easier than changing the DT provided by firmware.
Once we get into configuration of the OS/client (including u-boot), making that an ABI is a lot harder and if we use DT for that, I don't think it should be an ABI.
If we want to digress from the spec, we need the majority of developers to switch sides :) which is unlikely to happen and rightly so, imho.
It isn't even clear what the spec is. There is the document you reference above, there are the yaml files at https://github.com/devicetree-org/dt-schema and then there are additional yaml files in the Linux tree. As far as I know it isn't written down anywhere that those are the only places where device tree bindings can live.
There's not any restriction.
My intention with dtschema schemas is to only have "spec level" schemas. (Stuff we'd add to DTSpec (but don't because no one wants to write specs).) Hardware specific stuff lives somewhere else. That happens to be the Linux tree because that is where all the h/w support is (nothing else is close (by far)). Last I checked, we've got ~3700 schemas and ~1500 unconverted bindings.
That scope is quite a bit narrower than I understood it to be. It seems to leave a significant gap between the Linux repo and yours.
Anyway, let's face it, there is some consensus among developers that what Simon has done in U-Boot is pushing the use of devicetree beyond the point where a significant fraction of developers thinks it makes sense. And I think I agree with that. But you can't beat him with the spec to make your point.
Now the devicetree is cleverly constructed such that it is possible to define additional bindings without the risk of conflicting with bindings developed by other parties. In particular if U-Boot is augmenting a device tree with properties that are prefixed with "u-boot," this isn't going to hurt an operating system that uses such an augmented device tree.
Until someone has some great idea to start using them. If the OS doesn't need something, then why pass it in the first place? What purpose does it serve? It's an invitation for someone somewhere to start using them.
The downside of keeping the nodes is it creates an ABI where there doesn't need to be one necessarily.
I'd love to get away from the idea that DT is just for the OS. We are trying to use it for firmware-to-firmware communication, too. The programs on each side of that interface may be different projects. For that, we do need to have a binding, otherwise we end up with series like this one.
I don't think DT is just for the OS, but DT for the OS is an ABI and other cases may not be. Or they just don't need to follow all the same rules. Zephyr is doing their own thing for example.
I don't think /options/u-boot is an ABI. AIUI, u-boot populates the node and consumes it. No ABI there (or limited to SPL to u-boot proper). I suppose a prior firmware stage could populate it, but that doesn't scale as then the prior stage has to know details of the next stage.
I still don't understand the distinction, or really why U-Boot (or u-boot) is different. The main reason for the /options/u-boot node is so that prior phase firmware can tell U-Boot what to do. It is similar to the /chosen node for the OS. If it is not an ABI, how can any project rely on it? It is absolutely not just U-Boot generating something for its own consumption. Where is that idea even coming from? We even have an OF_HAS_PRIOR_STAGE Kconfig option for it in U-Boot, specifically for TF-A, etc.
As to the scaling, I agree. That suggests we try to make things generic, i.e. have an /options node with these sorts of generic settings. Candidates for that are console and logging controls, for example. I would prefer that to /options/u-boot, as you know.
The real problem is that some folks developed a certification program that allegedly requires schema verification and now propose adding code to U-Boot that doesn't really solve any problem. My proposed solution would be to change said certification program to allow firmware to augment the device tree with properties and nodes with compatibles that are in the namespace controlled by the firmware.
I don't think we should decide what to do here based on said certification program. It can adapt to what's decided. Probably, the /option nodes will be stripped out or ignored for certification.
I accepted u-boot options node schema into dtschema, but now have second thoughts on that. Now I'm getting more u-boot specific (perhaps, not clear) stuff and widevine stuff internal to a TEE.
Where should these bindings go such that ARM / Linaro are not trying to remove them? I would be OK with moving them out somewhere else, but how are people supposed to deal with such fragmentation? My understanding was that dt-schema was an attempt to set up a neutral area where bindings could be accepted that were not just for Linux...did that change?
No change, just not communicated I guess. And again, bindings are not "just for Linux". They are hosted there because that is where *all* the hardware support is (by far). But we'll probably never get past the "Linux binding" perception no matter what we do or how many times I say it.
To rephrase things a bit, I'm happy to host anything that's multi-project, not a large number of bindings, and not a device/hardware specific binding. The device specific bindings live in the kernel tree primarily. For any new binding (foos/#foo-cell type ones), I want to see multiple users. Really for these, probably best to start with them in Linux repo (or elsewhere) and then promote them to dtschema. That gives a little flexibility in changing/abandoning them.
So now I am not sure what you are suggesting. Are you wanting bindings in many places (yours, Linux, U-Boot, TF-A, ...)? I am sure you are aware that if we put bindings in U-Boot they will not be considered as bindings at all, including by Linaro.
To restate the problem, we need (and until your previous email I thought we had) a unified repo where cross-project, firmware-targeted bindings can be accepted and agreed by interested projects.
Removing nodes and/or properties and where things live are mostly independent discussions. SystemReady can adapt to whatever is decided for the former. In general, IMO, when passing DT from stage N to N+1, the N+1 stage should remove things which only apply to N+2 stage. For example, kexec removes /chosen and recreates it for the next kernel.
Sounds good. But note that the reason for that is a conflict, since the node is used for different things - i.e. the two kernels need different settings. That is quite different from the case I am talking about, where the settings either apply globally (to interested projects) or to a single project (for all boot phases of that project). In my case, I believe there is no need to remove anything.
Regards, Simon
PS Zephyr is absolutely doing its own thing. Apart from the fact that it doesn't even have a runtime DT, the bindings are entirely within the project. and bear little relation to Linux bindings. I was not around for that decision, but I suspect part of the rationale was that many of the MCUs which Zephyr targets are not supported by Linux. Of course, that is not fully true and I believe it will become less true with time. Then, perhaps, in 5 years it will be Zephyr's turn to think about bindings more deeply.
On Tue, Sep 19, 2023 at 02:25:49PM -0600, Simon Glass wrote:
Hi Rob,
On Mon, 18 Sept 2023 at 11:00, Rob Herring robh@kernel.org wrote:
On Thu, Sep 14, 2023 at 5:42 PM Simon Glass sjg@chromium.org wrote:
Hi Rob,
On Wed, 13 Sept 2023 at 16:39, Rob Herring robh@kernel.org wrote:
On Fri, Sep 8, 2023 at 1:06 PM Mark Kettenis mark.kettenis@xs4all.nl wrote:
From: Jassi Brar jassisinghbrar@gmail.com Date: Fri, 8 Sep 2023 09:37:12 -0500
Hi Simon,
On Thu, Sep 7, 2023 at 3:08 PM Simon Glass sjg@chromium.org wrote: > On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas > > > > > > I beg to differ. Devicetree is more than just hardware and always has > > > > been. See, for example the /chosen and /options nodes. > > > > > > There are exceptions... > > > > > > > We've been this over and over again and frankly it gets a bit annoying. > > It's called *DEVICE* tree for a reason. As Rob pointed out there are > > exceptions, but those made a lot of sense. Having arbitrary internal ABI > > stuff of various projects in the schema just defeats the definition of a > > spec. > > Our efforts should not just be about internal ABI, but working to > provide a consistent configuration system for all firmware elements. > Sure, programmatically we can pass any data/info via DT, however it is only meant to map hardware features onto data structures.
devicetree.org landing page "The devicetree is a data structure for describing hardware."
devicetree-specification-v0.3.pdf Chapter-2 Line-1 "DTSpec specifies a construct called a devicetree to describe system hardware."
But it doesn't say that it describes *only* hardware. And it does describe more than just hardware. There is /chosen to specify firmware configuration and there are several examples of device tree bindings that describe non-discoverable firmware interfaces in the Linux tree. And it has been that way since the days of IEEE 1275. There are also bindings describing things like the firmware partition layout.
Yes. The exact title for 1275[1] is: IEEE Standard for Boot (Initialization Configuration) Firmware: Core Requirements and Practices
I see "configuration" in there. However, in the OF case, it's generally how firmware configured the hardware and what it discovered. That's a little different than telling the OS how to configure the hardware which is what we do with FDT.
For the /options node it says "The properties of this node are the system’s configuration variables."
Then there is section 7.4.4 which has a large list of options which don't seem to be so narrowly defined.
In any case, this is not quite the point, which IMO is that we need DT to support firmware use cases, whether or not a 29-year-old spec thought of it or not. In fact it is amazing to me how forward-looking Open Firmware was.
Then there's stuff that's how to configure Linux which we try to reject.
Fair enough: Linux has user-space for that.
Yep, the question I usually ask is who needs to configure something and what determines the config. Changing things with sysfs is much easier than changing the DT provided by firmware.
Once we get into configuration of the OS/client (including u-boot), making that an ABI is a lot harder and if we use DT for that, I don't think it should be an ABI.
If we want to digress from the spec, we need the majority of developers to switch sides :) which is unlikely to happen and rightly so, imho.
It isn't even clear what the spec is. There is the document you reference above, there are the yaml files at https://github.com/devicetree-org/dt-schema and then there are additional yaml files in the Linux tree. As far as I know it isn't written down anywhere that those are the only places where device tree bindings can live.
There's not any restriction.
My intention with dtschema schemas is to only have "spec level" schemas. (Stuff we'd add to DTSpec (but don't because no one wants to write specs).) Hardware specific stuff lives somewhere else. That happens to be the Linux tree because that is where all the h/w support is (nothing else is close (by far)). Last I checked, we've got ~3700 schemas and ~1500 unconverted bindings.
That scope is quite a bit narrower than I understood it to be. It seems to leave a significant gap between the Linux repo and yours.
Anyway, let's face it, there is some consensus among developers that what Simon has done in U-Boot is pushing the use of devicetree beyond the point where a significant fraction of developers thinks it makes sense. And I think I agree with that. But you can't beat him with the spec to make your point.
Now the devicetree is cleverly constructed such that it is possible to define additional bindings without the risk of conflicting with bindings developed by other parties. In particular if U-Boot is augmenting a device tree with properties that are prefixed with "u-boot," this isn't going to hurt an operating system that uses such an augmented device tree.
Until someone has some great idea to start using them. If the OS doesn't need something, then why pass it in the first place? What purpose does it serve? It's an invitation for someone somewhere to start using them.
The downside of keeping the nodes is it creates an ABI where there doesn't need to be one necessarily.
I'd love to get away from the idea that DT is just for the OS. We are trying to use it for firmware-to-firmware communication, too. The programs on each side of that interface may be different projects. For that, we do need to have a binding, otherwise we end up with series like this one.
I don't think DT is just for the OS, but DT for the OS is an ABI and other cases may not be. Or they just don't need to follow all the same rules. Zephyr is doing their own thing for example.
I don't think /options/u-boot is an ABI. AIUI, u-boot populates the node and consumes it. No ABI there (or limited to SPL to u-boot proper). I suppose a prior firmware stage could populate it, but that doesn't scale as then the prior stage has to know details of the next stage.
I still don't understand the distinction, or really why U-Boot (or u-boot) is different. The main reason for the /options/u-boot node is so that prior phase firmware can tell U-Boot what to do. It is similar to the /chosen node for the OS. If it is not an ABI, how can any project rely on it? It is absolutely not just U-Boot generating something for its own consumption. Where is that idea even coming from? We even have an OF_HAS_PRIOR_STAGE Kconfig option for it in U-Boot, specifically for TF-A, etc.
Well, maybe we aren't different, we just need to figure out for ourselves what is useful to have in DT, and what isn't because no one else will use it and it's not the best mechanism for our use either. Or there might be cases where it is useful to us to do it that way and we just delete that part of the tree prior to passing on to the OS.
On Tue, Sep 19, 2023 at 3:26 PM Simon Glass sjg@chromium.org wrote:
Hi Rob,
On Mon, 18 Sept 2023 at 11:00, Rob Herring robh@kernel.org wrote:
On Thu, Sep 14, 2023 at 5:42 PM Simon Glass sjg@chromium.org wrote:
Hi Rob,
On Wed, 13 Sept 2023 at 16:39, Rob Herring robh@kernel.org wrote:
On Fri, Sep 8, 2023 at 1:06 PM Mark Kettenis mark.kettenis@xs4all.nl wrote:
From: Jassi Brar jassisinghbrar@gmail.com Date: Fri, 8 Sep 2023 09:37:12 -0500
Hi Simon,
On Thu, Sep 7, 2023 at 3:08 PM Simon Glass sjg@chromium.org wrote: > On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas > > > > > > I beg to differ. Devicetree is more than just hardware and always has > > > > been. See, for example the /chosen and /options nodes. > > > > > > There are exceptions... > > > > > > > We've been this over and over again and frankly it gets a bit annoying. > > It's called *DEVICE* tree for a reason. As Rob pointed out there are > > exceptions, but those made a lot of sense. Having arbitrary internal ABI > > stuff of various projects in the schema just defeats the definition of a > > spec. > > Our efforts should not just be about internal ABI, but working to > provide a consistent configuration system for all firmware elements. > Sure, programmatically we can pass any data/info via DT, however it is only meant to map hardware features onto data structures.
devicetree.org landing page "The devicetree is a data structure for describing hardware."
devicetree-specification-v0.3.pdf Chapter-2 Line-1 "DTSpec specifies a construct called a devicetree to describe system hardware."
But it doesn't say that it describes *only* hardware. And it does describe more than just hardware. There is /chosen to specify firmware configuration and there are several examples of device tree bindings that describe non-discoverable firmware interfaces in the Linux tree. And it has been that way since the days of IEEE 1275. There are also bindings describing things like the firmware partition layout.
Yes. The exact title for 1275[1] is: IEEE Standard for Boot (Initialization Configuration) Firmware: Core Requirements and Practices
I see "configuration" in there. However, in the OF case, it's generally how firmware configured the hardware and what it discovered. That's a little different than telling the OS how to configure the hardware which is what we do with FDT.
For the /options node it says "The properties of this node are the system’s configuration variables."
Then there is section 7.4.4 which has a large list of options which don't seem to be so narrowly defined.
In any case, this is not quite the point, which IMO is that we need DT to support firmware use cases, whether or not a 29-year-old spec thought of it or not. In fact it is amazing to me how forward-looking Open Firmware was.
Then there's stuff that's how to configure Linux which we try to reject.
Fair enough: Linux has user-space for that.
Yep, the question I usually ask is who needs to configure something and what determines the config. Changing things with sysfs is much easier than changing the DT provided by firmware.
Once we get into configuration of the OS/client (including u-boot), making that an ABI is a lot harder and if we use DT for that, I don't think it should be an ABI.
If we want to digress from the spec, we need the majority of developers to switch sides :) which is unlikely to happen and rightly so, imho.
It isn't even clear what the spec is. There is the document you reference above, there are the yaml files at https://github.com/devicetree-org/dt-schema and then there are additional yaml files in the Linux tree. As far as I know it isn't written down anywhere that those are the only places where device tree bindings can live.
There's not any restriction.
My intention with dtschema schemas is to only have "spec level" schemas. (Stuff we'd add to DTSpec (but don't because no one wants to write specs).) Hardware specific stuff lives somewhere else. That happens to be the Linux tree because that is where all the h/w support is (nothing else is close (by far)). Last I checked, we've got ~3700 schemas and ~1500 unconverted bindings.
That scope is quite a bit narrower than I understood it to be. It seems to leave a significant gap between the Linux repo and yours.
Anyway, let's face it, there is some consensus among developers that what Simon has done in U-Boot is pushing the use of devicetree beyond the point where a significant fraction of developers thinks it makes sense. And I think I agree with that. But you can't beat him with the spec to make your point.
Now the devicetree is cleverly constructed such that it is possible to define additional bindings without the risk of conflicting with bindings developed by other parties. In particular if U-Boot is augmenting a device tree with properties that are prefixed with "u-boot," this isn't going to hurt an operating system that uses such an augmented device tree.
Until someone has some great idea to start using them. If the OS doesn't need something, then why pass it in the first place? What purpose does it serve? It's an invitation for someone somewhere to start using them.
The downside of keeping the nodes is it creates an ABI where there doesn't need to be one necessarily.
I'd love to get away from the idea that DT is just for the OS. We are trying to use it for firmware-to-firmware communication, too. The programs on each side of that interface may be different projects. For that, we do need to have a binding, otherwise we end up with series like this one.
I don't think DT is just for the OS, but DT for the OS is an ABI and other cases may not be. Or they just don't need to follow all the same rules. Zephyr is doing their own thing for example.
I don't think /options/u-boot is an ABI. AIUI, u-boot populates the node and consumes it. No ABI there (or limited to SPL to u-boot proper). I suppose a prior firmware stage could populate it, but that doesn't scale as then the prior stage has to know details of the next stage.
I still don't understand the distinction, or really why U-Boot (or u-boot) is different. The main reason for the /options/u-boot node is so that prior phase firmware can tell U-Boot what to do. It is similar to the /chosen node for the OS. If it is not an ABI, how can any project rely on it? It is absolutely not just U-Boot generating something for its own consumption. Where is that idea even coming from? We even have an OF_HAS_PRIOR_STAGE Kconfig option for it in U-Boot, specifically for TF-A, etc.
Okay, I misunderstood the intent since I thought u-boot gets its DT from the one in the u-boot tree (at least usually).
Though I just looked at TF-A source and see no evidence of it doing anything with /options or /options/u-boot. I'm really not that interested in what is possible and only used by 1 board and fork somewhere. I'm interested in supporting what the community defines as best practice (e.g. DT (for OS) comes from firmware), but so far that hasn't been defined below the firmware-OS level. Yes, there's efforts addressing aspects of it, but it's not really clear to me what the high-level goals are and what projects are onboard.
Note that I did find "/secure-chosen" though. And if they want to use that, then fine, but I don't want to see it.
As to the scaling, I agree. That suggests we try to make things generic, i.e. have an /options node with these sorts of generic settings. Candidates for that are console and logging controls, for example. I would prefer that to /options/u-boot, as you know.
The real problem is that some folks developed a certification program that allegedly requires schema verification and now propose adding code to U-Boot that doesn't really solve any problem. My proposed solution would be to change said certification program to allow firmware to augment the device tree with properties and nodes with compatibles that are in the namespace controlled by the firmware.
I don't think we should decide what to do here based on said certification program. It can adapt to what's decided. Probably, the /option nodes will be stripped out or ignored for certification.
I accepted u-boot options node schema into dtschema, but now have second thoughts on that. Now I'm getting more u-boot specific (perhaps, not clear) stuff and widevine stuff internal to a TEE.
Where should these bindings go such that ARM / Linaro are not trying to remove them? I would be OK with moving them out somewhere else, but how are people supposed to deal with such fragmentation? My understanding was that dt-schema was an attempt to set up a neutral area where bindings could be accepted that were not just for Linux...did that change?
No change, just not communicated I guess. And again, bindings are not "just for Linux". They are hosted there because that is where *all* the hardware support is (by far). But we'll probably never get past the "Linux binding" perception no matter what we do or how many times I say it.
To rephrase things a bit, I'm happy to host anything that's multi-project, not a large number of bindings, and not a device/hardware specific binding. The device specific bindings live in the kernel tree primarily. For any new binding (foos/#foo-cell type ones), I want to see multiple users. Really for these, probably best to start with them in Linux repo (or elsewhere) and then promote them to dtschema. That gives a little flexibility in changing/abandoning them.
So now I am not sure what you are suggesting. Are you wanting bindings in many places (yours, Linux, U-Boot, TF-A, ...)? I am sure you are aware that if we put bindings in U-Boot they will not be considered as bindings at all, including by Linaro.
If the instances of a binding are only produced and consumed in $project, then it is perfectly fine to host the bindings in $project. It's also not something fixed. It can start out in $project and get moved out if it proves useful elsewhere.
To restate the problem, we need (and until your previous email I thought we had) a unified repo where cross-project, firmware-targeted bindings can be accepted and agreed by interested projects.
I'm happy to take those (assuming I'm not bombarded with 100s). So far, nothing I've seen is clearly cross project. Send me stuff that has acks from multiple projects.
Removing nodes and/or properties and where things live are mostly independent discussions. SystemReady can adapt to whatever is decided for the former. In general, IMO, when passing DT from stage N to N+1, the N+1 stage should remove things which only apply to N+2 stage. For example, kexec removes /chosen and recreates it for the next kernel.
Sounds good. But note that the reason for that is a conflict, since the node is used for different things - i.e. the two kernels need different settings. That is quite different from the case I am talking about, where the settings either apply globally (to interested projects) or to a single project (for all boot phases of that project). In my case, I believe there is no need to remove anything.
There is also no need to keep it. We can always decide to keep it later if there is a need. We can't decide later to remove it because someone may be relying on it.
Regards, Simon
PS Zephyr is absolutely doing its own thing. Apart from the fact that it doesn't even have a runtime DT, the bindings are entirely within the project. and bear little relation to Linux bindings. I was not around for that decision, but I suspect part of the rationale was that many of the MCUs which Zephyr targets are not supported by Linux. Of course, that is not fully true and I believe it will become less true with time. Then, perhaps, in 5 years it will be Zephyr's turn to think about bindings more deeply.
Little overlap is part of it, but a large part of the reason is Zephyr needed something machine parsable and it was before we came up with dtschema. There's desire to adopt dtschema, but at this point I imagine with the first reason, there is little pressing need. They've got something that works for them.
Rob
Hi Rob,
On Thu, 21 Sept 2023 at 07:59, Rob Herring robh@kernel.org wrote:
On Tue, Sep 19, 2023 at 3:26 PM Simon Glass sjg@chromium.org wrote:
Hi Rob,
On Mon, 18 Sept 2023 at 11:00, Rob Herring robh@kernel.org wrote:
On Thu, Sep 14, 2023 at 5:42 PM Simon Glass sjg@chromium.org wrote:
Hi Rob,
On Wed, 13 Sept 2023 at 16:39, Rob Herring robh@kernel.org wrote:
On Fri, Sep 8, 2023 at 1:06 PM Mark Kettenis mark.kettenis@xs4all.nl wrote:
> From: Jassi Brar jassisinghbrar@gmail.com > Date: Fri, 8 Sep 2023 09:37:12 -0500 > > Hi Simon, > > On Thu, Sep 7, 2023 at 3:08 PM Simon Glass sjg@chromium.org wrote: > > On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas > > > > > > > > I beg to differ. Devicetree is more than just hardware and always has > > > > > been. See, for example the /chosen and /options nodes. > > > > > > > > There are exceptions... > > > > > > > > > > We've been this over and over again and frankly it gets a bit annoying. > > > It's called *DEVICE* tree for a reason. As Rob pointed out there are > > > exceptions, but those made a lot of sense. Having arbitrary internal ABI > > > stuff of various projects in the schema just defeats the definition of a > > > spec. > > > > Our efforts should not just be about internal ABI, but working to > > provide a consistent configuration system for all firmware elements. > > > Sure, programmatically we can pass any data/info via DT, however it is > only meant to map hardware features onto data structures. > > devicetree.org landing page > "The devicetree is a data structure for describing hardware." > > devicetree-specification-v0.3.pdf Chapter-2 Line-1 > "DTSpec specifies a construct called a devicetree to describe > system hardware."
But it doesn't say that it describes *only* hardware. And it does describe more than just hardware. There is /chosen to specify firmware configuration and there are several examples of device tree bindings that describe non-discoverable firmware interfaces in the Linux tree. And it has been that way since the days of IEEE 1275. There are also bindings describing things like the firmware partition layout.
Yes. The exact title for 1275[1] is: IEEE Standard for Boot (Initialization Configuration) Firmware: Core Requirements and Practices
I see "configuration" in there. However, in the OF case, it's generally how firmware configured the hardware and what it discovered. That's a little different than telling the OS how to configure the hardware which is what we do with FDT.
For the /options node it says "The properties of this node are the system’s configuration variables."
Then there is section 7.4.4 which has a large list of options which don't seem to be so narrowly defined.
In any case, this is not quite the point, which IMO is that we need DT to support firmware use cases, whether or not a 29-year-old spec thought of it or not. In fact it is amazing to me how forward-looking Open Firmware was.
Then there's stuff that's how to configure Linux which we try to reject.
Fair enough: Linux has user-space for that.
Yep, the question I usually ask is who needs to configure something and what determines the config. Changing things with sysfs is much easier than changing the DT provided by firmware.
Once we get into configuration of the OS/client (including u-boot), making that an ABI is a lot harder and if we use DT for that, I don't think it should be an ABI.
> If we want to digress from the spec, we need the majority of > developers to switch sides :) which is unlikely to happen and rightly > so, imho.
It isn't even clear what the spec is. There is the document you reference above, there are the yaml files at https://github.com/devicetree-org/dt-schema and then there are additional yaml files in the Linux tree. As far as I know it isn't written down anywhere that those are the only places where device tree bindings can live.
There's not any restriction.
My intention with dtschema schemas is to only have "spec level" schemas. (Stuff we'd add to DTSpec (but don't because no one wants to write specs).) Hardware specific stuff lives somewhere else. That happens to be the Linux tree because that is where all the h/w support is (nothing else is close (by far)). Last I checked, we've got ~3700 schemas and ~1500 unconverted bindings.
That scope is quite a bit narrower than I understood it to be. It seems to leave a significant gap between the Linux repo and yours.
Anyway, let's face it, there is some consensus among developers that what Simon has done in U-Boot is pushing the use of devicetree beyond the point where a significant fraction of developers thinks it makes sense. And I think I agree with that. But you can't beat him with the spec to make your point.
Now the devicetree is cleverly constructed such that it is possible to define additional bindings without the risk of conflicting with bindings developed by other parties. In particular if U-Boot is augmenting a device tree with properties that are prefixed with "u-boot," this isn't going to hurt an operating system that uses such an augmented device tree.
Until someone has some great idea to start using them. If the OS doesn't need something, then why pass it in the first place? What purpose does it serve? It's an invitation for someone somewhere to start using them.
The downside of keeping the nodes is it creates an ABI where there doesn't need to be one necessarily.
I'd love to get away from the idea that DT is just for the OS. We are trying to use it for firmware-to-firmware communication, too. The programs on each side of that interface may be different projects. For that, we do need to have a binding, otherwise we end up with series like this one.
I don't think DT is just for the OS, but DT for the OS is an ABI and other cases may not be. Or they just don't need to follow all the same rules. Zephyr is doing their own thing for example.
I don't think /options/u-boot is an ABI. AIUI, u-boot populates the node and consumes it. No ABI there (or limited to SPL to u-boot proper). I suppose a prior firmware stage could populate it, but that doesn't scale as then the prior stage has to know details of the next stage.
I still don't understand the distinction, or really why U-Boot (or u-boot) is different. The main reason for the /options/u-boot node is so that prior phase firmware can tell U-Boot what to do. It is similar to the /chosen node for the OS. If it is not an ABI, how can any project rely on it? It is absolutely not just U-Boot generating something for its own consumption. Where is that idea even coming from? We even have an OF_HAS_PRIOR_STAGE Kconfig option for it in U-Boot, specifically for TF-A, etc.
Okay, I misunderstood the intent since I thought u-boot gets its DT from the one in the u-boot tree (at least usually).
That sounds right, although it isn't the intended destination. But in any case, we want to use the same DT as is in Linux, so Linux needs to accept DTs with these properties in them.
Though I just looked at TF-A source and see no evidence of it doing anything with /options or /options/u-boot. I'm really not that interested in what is possible and only used by 1 board and fork somewhere. I'm interested in supporting what the community defines as best practice (e.g. DT (for OS) comes from firmware), but so far that hasn't been defined below the firmware-OS level. Yes, there's efforts addressing aspects of it, but it's not really clear to me what the high-level goals are and what projects are onboard.
So, chicken and egg? U-Boot cannot define /options until other projects add them, but they won't until U-Boot defines them?
Note that I did find "/secure-chosen" though. And if they want to use that, then fine, but I don't want to see it.
I wonder what that is? Is it in the bindings anywhere?
As to the scaling, I agree. That suggests we try to make things generic, i.e. have an /options node with these sorts of generic settings. Candidates for that are console and logging controls, for example. I would prefer that to /options/u-boot, as you know.
The real problem is that some folks developed a certification program that allegedly requires schema verification and now propose adding code to U-Boot that doesn't really solve any problem. My proposed solution would be to change said certification program to allow firmware to augment the device tree with properties and nodes with compatibles that are in the namespace controlled by the firmware.
I don't think we should decide what to do here based on said certification program. It can adapt to what's decided. Probably, the /option nodes will be stripped out or ignored for certification.
I accepted u-boot options node schema into dtschema, but now have second thoughts on that. Now I'm getting more u-boot specific (perhaps, not clear) stuff and widevine stuff internal to a TEE.
Where should these bindings go such that ARM / Linaro are not trying to remove them? I would be OK with moving them out somewhere else, but how are people supposed to deal with such fragmentation? My understanding was that dt-schema was an attempt to set up a neutral area where bindings could be accepted that were not just for Linux...did that change?
No change, just not communicated I guess. And again, bindings are not "just for Linux". They are hosted there because that is where *all* the hardware support is (by far). But we'll probably never get past the "Linux binding" perception no matter what we do or how many times I say it.
To rephrase things a bit, I'm happy to host anything that's multi-project, not a large number of bindings, and not a device/hardware specific binding. The device specific bindings live in the kernel tree primarily. For any new binding (foos/#foo-cell type ones), I want to see multiple users. Really for these, probably best to start with them in Linux repo (or elsewhere) and then promote them to dtschema. That gives a little flexibility in changing/abandoning them.
So now I am not sure what you are suggesting. Are you wanting bindings in many places (yours, Linux, U-Boot, TF-A, ...)? I am sure you are aware that if we put bindings in U-Boot they will not be considered as bindings at all, including by Linaro.
If the instances of a binding are only produced and consumed in $project, then it is perfectly fine to host the bindings in $project. It's also not something fixed. It can start out in $project and get moved out if it proves useful elsewhere.
Does Linaro agree with that, though? We have this thread which suggests not.
I really think we need to resolve this somehow. It is just causing so much churn and confusion.
How about we agree that firmware can have bindings and that the DTs with those bindings can be in Linux, with no conditions other than binding review?
To restate the problem, we need (and until your previous email I thought we had) a unified repo where cross-project, firmware-targeted bindings can be accepted and agreed by interested projects.
I'm happy to take those (assuming I'm not bombarded with 100s). So far, nothing I've seen is clearly cross project. Send me stuff that has acks from multiple projects.
Part of the disconnect here is that you seem to be assuming that each project has its own DTs and hacks them up as much as it likes, but that is not the world I advocate. I would like to have one DT which can support both firmware and OS.
Removing nodes and/or properties and where things live are mostly independent discussions. SystemReady can adapt to whatever is decided for the former. In general, IMO, when passing DT from stage N to N+1, the N+1 stage should remove things which only apply to N+2 stage. For example, kexec removes /chosen and recreates it for the next kernel.
Sounds good. But note that the reason for that is a conflict, since the node is used for different things - i.e. the two kernels need different settings. That is quite different from the case I am talking about, where the settings either apply globally (to interested projects) or to a single project (for all boot phases of that project). In my case, I believe there is no need to remove anything.
There is also no need to keep it. We can always decide to keep it later if there is a need. We can't decide later to remove it because someone may be relying on it.
Is this an open source project relying on it, or a closed-source one? Bear in mind that if someone removes a firmware feature and the OS doesn't boot, they will fix the firmware.
Anyway, I don't mind what people do here. I was just pointing out that using the same node for two different things seems awkward, to say the least.
Regards, Simon
PS Zephyr is absolutely doing its own thing. Apart from the fact that it doesn't even have a runtime DT, the bindings are entirely within the project. and bear little relation to Linux bindings. I was not around for that decision, but I suspect part of the rationale was that many of the MCUs which Zephyr targets are not supported by Linux. Of course, that is not fully true and I believe it will become less true with time. Then, perhaps, in 5 years it will be Zephyr's turn to think about bindings more deeply.
Little overlap is part of it, but a large part of the reason is Zephyr needed something machine parsable and it was before we came up with dtschema. There's desire to adopt dtschema, but at this point I imagine with the first reason, there is little pressing need. They've got something that works for them.
Right, which sounds like where U-Boot started and was for years forced to stay. That is what I am trying to change.
Regards, Simon
On Mon, Aug 28, 2023 at 10:19:55AM -0600, Simon Glass wrote:
Hi,
On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Provide a way for removing certain devicetree nodes and/or properties from the devicetree. This is needed to purge certain nodes and properties which may be relevant only in U-Boot. Such nodes and properties are then removed from the devicetree before it is passed to the kernel. This ensures that the devicetree passed to the OS does not contain any non-compliant nodes and properties.
The removal of the nodes and properties is being done through an EVT_FT_FIXUP handler. I am not sure if the removal code needs to be behind any Kconfig symbol.
I have only build tested this on sandbox, and tested on qemu arm64 virt platform. This being a RFC, I have not put this through a CI run.
Sughosh Ganu (5): dt: Provide a way to remove non-compliant nodes and properties fwu: Add the fwu-mdata node for removal from devicetree capsule: Add the capsule-key property for removal from devicetree bootefi: Call the EVT_FT_FIXUP event handler doc: Add a document for non-compliant DT node/property removal
cmd/bootefi.c | 18 +++++ .../devicetree/dt_non_compliant_purge.rst | 64 ++++++++++++++++ drivers/fwu-mdata/fwu-mdata-uclass.c | 5 ++ include/dt-structs.h | 11 +++ lib/Makefile | 1 + lib/dt_purge.c | 73 +++++++++++++++++++ lib/efi_loader/efi_capsule.c | 7 ++ 7 files changed, 179 insertions(+) create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst create mode 100644 lib/dt_purge.c
What is the point of removing them? Instead, we should make sure that we upstream the bindings and encourage SoC vendors to sync them. If we remove them, no one will bother and U-Boot just becomes a dumping ground.
It's about having a defined process to remove them, rather than an ad-hoc process like one can do today to remove them. And it's about having control over the situation rather than dismissing it, as vendor can already say they used $this version of the software for validation, so patches-on-top aren't out of the question.
participants (9)
-
Heinrich Schuchardt -
Ilias Apalodimas -
Jassi Brar -
Mark Kettenis -
Peter Robinson -
Rob Herring -
Simon Glass -
Sughosh Ganu -
Tom Rini