[RFC PATCH v3 0/4] dtc: Add a plugin interface

[RFC PATCH v3 0/4] dtc: Add a plugin interface

[Andrei Ziureaev]

I added a simple live tree API. There's no longer a need to move any
definitions. The live tree struct is hidden from plugins, so it's not an ABI.
The API doesn't allow plugins to modify the live tree yet, but that will
probably change later.

I also removed the EXPORT_FUNCTION macro. Plugins now have to implement
certain prototypes, defined in dtc-plugin.h. This allows plugins to be
generated by tools.
===========

This is one possible implementation of a plugin interface for DTC.

A plugin interface would allow us to add checks in languages other than
C. Specifically, it would allow DTC and DT Schema (which is written in
Python) to be integrated.

It would also allow for better debugging of dts files. For example, DT
Schema would be able to print dts source line information, without us
having to add source line annotations to yaml files.

In the future, plugins will be allowed to modify the live tree and pass
it back for further processing, but the API doesn't support that yet.

There's a question of whether we should relicense some headers to
dual BSD to be able call Python code.

Any thoughts would be much appreciated.

Thanks,
Andrei.

Changes in v3:
- live tree API (dt.h and dt.c)
- additional functionality in dtc.h
- plugins have to implement prototypes
- improved documentation

Changes in v2:
- improved documentation
- plugins must register with the build system
- the "validate_fn_t" hook can return a status
- define "struct reserve_info" in "dtc-plugin.h"
- specify that minor versions are compatible
- check if plugin_dir is NULL, just in case
- better variable names in register_plugin_info

Andrei Ziureaev (4):
  dtc: Add marker type functionality to dtc.h
  dtc: Add a live tree API
  dtc: Add plugin documentation and examples
  dtc: Add a plugin interface

 Documentation/manual.txt         | 146 ++++++++++++++++++++++++++++
 Makefile                         |  29 +++++-
 Makefile.dtc                     |   1 +
 dt.c                             | 158 +++++++++++++++++++++++++++++++
 dt.h                             |  63 ++++++++++++
 dtc-plugin.h                     |  76 +++++++++++++++
 dtc.c                            | 142 ++++++++++++++++++++++++++-
 dtc.h                            |  57 +++++++++++
 plugins/example/Makefile.example |  27 ++++++
 plugins/example/example.c        |  33 +++++++
 treesource.c                     |   8 +-
 11 files changed, 734 insertions(+), 6 deletions(-)
 create mode 100644 dt.c
 create mode 100644 dt.h
 create mode 100644 dtc-plugin.h
 create mode 100644 plugins/example/Makefile.example
 create mode 100644 plugins/example/example.c

-- 
2.17.1

[RFC PATCH v3 1/4] dtc: Add marker type functionality to dtc.h

[Andrei Ziureaev]

Add an inline helper function to dtc.h that checks if there's a marker
of a type at an offset.

Also, there are two useful static functions in treesource.c. Make them
public, add their prototypes to dtc.h and rename one of them.

Signed-off-by: Andrei Ziureaev <andrei.ziureaev-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Andrei Ziureaev <andreiziureaev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 dtc.h        | 11 +++++++++++
 treesource.c |  8 ++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/dtc.h b/dtc.h
index a08f415..286d999 100644
--- a/dtc.h
+++ b/dtc.h
@@ -125,6 +125,17 @@ struct data {
 	for_each_marker(m) \
 		if ((m)->type == (t))
 
+static inline bool markers_have_type_at_offset(struct marker *m, enum markertype type, int off)
+{
+	for_each_marker_of_type(m, type)
+		if (m->offset == off)
+			return true;
+
+	return false;
+}
+
+bool marker_has_data_type_information(struct marker *m);
+struct marker *next_type_marker(struct marker *m);
 size_t type_marker_length(struct marker *m);
 
 void data_free(struct data d);
diff --git a/treesource.c b/treesource.c
index 061ba8c..0785d83 100644
--- a/treesource.c
+++ b/treesource.c
@@ -124,14 +124,14 @@ static void write_propval_int(FILE *f, const char *p, size_t len, size_t width)
 	}
 }
 
-static bool has_data_type_information(struct marker *m)
+bool marker_has_data_type_information(struct marker *m)
 {
 	return m->type >= TYPE_UINT8;
 }
 
-static struct marker *next_type_marker(struct marker *m)
+struct marker *next_type_marker(struct marker *m)
 {
-	while (m && !has_data_type_information(m))
+	while (m && !marker_has_data_type_information(m))
 		m = m->next;
 	return m;
 }
@@ -230,7 +230,7 @@ static void write_propval(FILE *f, struct property *prop)
 		size_t data_len = type_marker_length(m) ? : len - m->offset;
 		const char *p = &prop->val.val[m->offset];
 
-		if (has_data_type_information(m)) {
+		if (marker_has_data_type_information(m)) {
 			emit_type = m->type;
 			fprintf(f, " %s", delim_start[emit_type]);
 		} else if (m->type == LABEL)
-- 
2.17.1

[RFC PATCH v3 2/4] dtc: Add a live tree API

[Andrei Ziureaev]

The purpose of this API is to completely abstract away the internal live
tree implementation.

This API can traverse the tree and read its values. This is enough to
validate the tree in Python, for example. We could add more
functionality, documentation and tests, and turn it into a separate
library.

Signed-off-by: Andrei Ziureaev <andrei.ziureaev-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Andrei Ziureaev <andreiziureaev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 Makefile.dtc |   1 +
 dt.c         | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++
 dt.h         |  63 ++++++++++++++++++++
 3 files changed, 222 insertions(+)
 create mode 100644 dt.c
 create mode 100644 dt.h

diff --git a/Makefile.dtc b/Makefile.dtc
index 9c467b0..09b3123 100644
--- a/Makefile.dtc
+++ b/Makefile.dtc
@@ -7,6 +7,7 @@
 DTC_SRCS = \
 	checks.c \
 	data.c \
+	dt.c \
 	dtc.c \
 	flattree.c \
 	fstree.c \
diff --git a/dt.c b/dt.c
new file mode 100644
index 0000000..5024e63
--- /dev/null
+++ b/dt.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * (C) Copyright Arm Holdings.  2020
+ */
+
+#include "dt.h"
+#include "dtc.h"
+
+struct node *dt_root(struct dt_info *dti)
+{
+	return dti->dt;
+}
+
+struct node *dt_first_child(struct node *node)
+{
+	return node->children;
+}
+
+struct node *dt_next_sibling(struct node *node)
+{
+	return node->next_sibling;
+}
+
+struct property *dt_first_property(struct node *node)
+{
+	return node->proplist;
+}
+
+struct property *dt_next_property(struct property *prop)
+{
+	return prop->next;
+}
+
+struct marker *dt_first_row(struct property *prop)
+{
+	return next_type_marker(prop->val.markers);
+}
+
+struct marker *dt_next_row(struct marker *row)
+{
+	return next_type_marker(row->next);
+}
+
+int dt_row_length(struct property *prop, struct marker *row)
+{
+	int length_bytes = type_marker_length(row) ? : prop->val.len - row->offset;
+	return length_bytes / dt_cell_width_bytes(row);
+}
+
+uint64_t dt_uint(struct property *prop, struct marker *row, int col)
+{
+	int width = dt_cell_width_bytes(row);
+	const char *p = &dt_string(prop, row)[col * width];
+
+	switch(width) {
+	case 2:
+		return dtb_ld16(p);
+	case 4:
+		return dtb_ld32(p);
+	case 8:
+		return dtb_ld64(p);
+	default:
+		return *(const uint8_t*)p;
+	}
+}
+
+bool dt_cell_is_phandle(struct property *prop, struct marker *row, int col)
+{
+	int width = dt_cell_width_bytes(row);
+	int off = row->offset + col * width;
+
+	if (width != 4)
+		return false;
+
+	return markers_have_type_at_offset(row, REF_PHANDLE, off);
+}
+
+const char *dt_string(struct property *prop, struct marker *row)
+{
+	return &prop->val.val[row->offset];
+}
+
+enum dt_type dt_row_type(struct marker *row)
+{
+	switch (row->type) {
+	/* fallthrough */
+	case TYPE_UINT8:
+	case TYPE_UINT16:
+	case TYPE_UINT32:
+	case TYPE_UINT64:
+		return DT_TYPE_UINT;
+	case TYPE_STRING:
+		return DT_TYPE_STRING;
+	default:
+		return DT_TYPE_NONE;
+	}
+}
+
+const char *dt_row_type_name(struct marker *row)
+{
+	switch (dt_row_type(row)) {
+	case DT_TYPE_UINT:
+		return "uint";
+	case DT_TYPE_STRING:
+		return "string";
+	default:
+		return "none";
+	}
+}
+
+int dt_cell_width_bytes(struct marker *row)
+{
+	switch (row->type) {
+	case TYPE_UINT16:
+		return 2;
+	case TYPE_UINT32:
+		return 4;
+	case TYPE_UINT64:
+		return 8;
+	default:
+		return 1;
+	}
+}
+
+const char *dt_node_name(struct node *node)
+{
+	return *node->name ? node->name : "/";
+}
+
+struct srcpos *dt_node_srcpos(struct node *node)
+{
+	return node->srcpos;
+}
+
+const char *dt_property_name(struct property *prop)
+{
+	return prop->name;
+}
+
+struct srcpos *dt_property_srcpos(struct property *prop)
+{
+	return prop->srcpos;
+}
+
+const char *dt_srcpos_dir(struct srcpos *srcpos)
+{
+	return srcpos->file->dir;
+}
+
+const char *dt_srcpos_file_name(struct srcpos *srcpos)
+{
+	return srcpos->file->name;
+}
+
+int dt_srcpos_first_line(struct srcpos *srcpos)
+{
+	return srcpos->first_line;
+}
diff --git a/dt.h b/dt.h
new file mode 100644
index 0000000..c6af112
--- /dev/null
+++ b/dt.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef DT_H
+#define DT_H
+
+/*
+ * (C) Copyright Arm Holdings.  2020
+ */
+
+#include <stdint.h>
+#include <stdbool.h>
+
+#include "srcpos.h"
+
+struct dt_info;
+struct node;
+struct property;
+struct marker;
+
+enum dt_type {
+	DT_TYPE_NONE,
+	DT_TYPE_UINT,
+	DT_TYPE_STRING,
+};
+
+/* Traversal */
+struct node *dt_root(struct dt_info *dti);
+
+struct node *dt_first_child(struct node *node);
+struct node *dt_next_sibling(struct node *node);
+
+struct property *dt_first_property(struct node *node);
+struct property *dt_next_property(struct property *prop);
+
+/* A property value resembles a matrix (or a table). A marker with data
+ * type information can represent a row.
+ *
+ * So, each property has a linked list of rows. */
+struct marker *dt_first_row(struct property *prop);
+struct marker *dt_next_row(struct marker *row);
+
+/* Accessing data. Each row is an array of cells. */
+int dt_row_length(struct property *prop, struct marker *row);
+
+uint64_t dt_uint(struct property *prop, struct marker *row, int col);
+bool dt_cell_is_phandle(struct property *prop, struct marker *row, int col);
+const char *dt_string(struct property *prop, struct marker *row);
+
+/* Accessing metadata */
+enum dt_type dt_row_type(struct marker *row);
+const char *dt_row_type_name(struct marker *row);
+int dt_cell_width_bytes(struct marker *row);
+
+const char *dt_node_name(struct node *node);
+struct srcpos *dt_node_srcpos(struct node *node);
+
+const char *dt_property_name(struct property *prop);
+struct srcpos *dt_property_srcpos(struct property *prop);
+
+const char *dt_srcpos_dir(struct srcpos *srcpos);
+const char *dt_srcpos_file_name(struct srcpos *srcpos);
+int dt_srcpos_first_line(struct srcpos *srcpos);
+
+#endif /* DT_H */
-- 
2.17.1

[RFC PATCH v3 3/4] dtc: Add plugin documentation and examples

[Andrei Ziureaev]

Document the plugin API in Documentation/manual.txt and provide an
example plugin.

Signed-off-by: Andrei Ziureaev <andrei.ziureaev-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Andrei Ziureaev <andreiziureaev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---

Changes in v3:
- plugins have to implement prototypes
- added license tags to the example
- copied some definitions from dtc-plugin.h to the manual
- better wording

Changes in v2:
- better structure
- information about the data model under "5.2) Exporting Functionality"
- plugins must register with the build system
- the "validate_fn_t" hook can return a status

 Documentation/manual.txt         | 146 +++++++++++++++++++++++++++++++
 plugins/example/Makefile.example |  27 ++++++
 plugins/example/example.c        |  33 +++++++
 3 files changed, 206 insertions(+)
 create mode 100644 plugins/example/Makefile.example
 create mode 100644 plugins/example/example.c

diff --git a/Documentation/manual.txt b/Documentation/manual.txt
index adf5ccb..c87d1f3 100644
--- a/Documentation/manual.txt
+++ b/Documentation/manual.txt
@@ -10,6 +10,9 @@ I - "dtc", the device tree compiler
     4.1) Overview
     4.2) Properties
     4.3) Labels and References
+    5) Plugins
+    5.1) Building and Installing
+    5.2) Exporting Functionality
 
 II - The DT block format
     1) Header
@@ -115,6 +118,16 @@ Options:
     -d <dependency_filename>
 	Generate a dependency file during compilation.
 
+    -l, --plugin <plugin name>[,key[,value]]
+	Load a plugin and, optionally, pass it an argument.
+		plugin name - the name of the shared object without the extension (can contain a path)
+		key         - the name of the argument
+		value       - the value of the argument (can be omitted)
+	Example: dtc -l some-plugin,o,out.dts -l some-plugin,help -l another-plugin [...]
+
+    -L, --plugin-dir <arg>
+	The directory in which to search for plugins
+
     -q
 	Quiet: -q suppress warnings, -qq errors, -qqq all
 
@@ -272,6 +285,139 @@ And used in properties, labels may appear before or after any value:
     };
 
 
+5) Plugins
+
+Plugins are shared libraries that DTC loads at runtime using
+
+	dlopen(path, RTLD_NOW | RTLD_GLOBAL | RTLD_DEEPBIND)
+
+RTLD_NOW means they are loaded immediately (right before the parsing
+stage).
+
+RTLD_GLOBAL means their symbols can be used by other plugins.
+
+RTLD_DEEPBIND means the dynamic linker will look for symbols within the
+plugin first, before searching in the main program and other plugins.
+
+All plugins must include the "dtc-plugin.h" header.
+
+
+5.1) Building and Installing
+
+Plugins are built and installed using the command "make plugins".
+
+Suppose your plugin is called "example" and its source code is in
+"plugins/example/example.c". To register it with the build system,
+create a file "plugins/example/Makefile.example" with the following
+line as its contents:
+
+PLUGIN_LIBS += $(PLUGIN_dir)/example/example.so
+
+This means "make plugins" will try to build
+
+	plugins/example/example.so
+
+from
+
+	plugins/example/example.c
+
+It will also make a symlink
+
+	plugins/example.so
+
+for convenience. You could then call DTC like this:
+
+    ./dtc -Onull some-file.dts -l plugins/example
+
+Please, see "plugins/example/Makefile.example" for how to override the
+default behavior, add GCC flags, etc.
+
+
+5.2) Exporting Functionality
+
+- From "dtc-plugin.h":
+
+```
+/*
+ * Plugins export functionality by implementing one or more of the
+ * functions below. DTC tries to call each function exactly once for
+ * each plugin.
+ *
+ * The typedefs are there for conveniently storing pointers to these
+ * functions.
+ */
+
+/**
+ * Initialize the plugin.
+ *
+ * Called right after the plugin is loaded.
+ *
+ * Every plugin must implement this. At the very least, it should
+ * perform a version check.
+ *
+ * @param dtc_ver       DTC's plugin API version
+ * @param argc          Length of argv
+ * @param argv          Array of plugin arguments
+ * @return 0 on success, or 1 on failure
+ */
+int dtc_init(struct plugin_version dtc_ver, int argc, struct plugin_arg *argv);
+typedef int (*dtc_init_fn_t)(struct plugin_version dtc_ver, int argc,
+			     struct plugin_arg *argv);
+
+/**
+ * Validate a device tree.
+ *
+ * Called after DTC's parsing stage, but before the output stage.
+ *
+ * @param dti   The unflattened device tree. Implementations can modify
+ *              it and "pass it back" to DTC and to subsequent plugins.
+ *              The header "dt.h" contains functionality for accessing
+ *              "struct dt_info".
+ * @return 1 on a fatal failure, otherwise 0
+ */
+int dtc_validate(struct dt_info *dti);
+typedef int (*dtc_validate_fn_t)(struct dt_info *dti);
+```
+
+- "struct plugin_version" is defined as:
+
+```
+struct plugin_version {
+	int major;	/* Incompatible changes */
+	int minor;	/* Compatible changes, such as adding a new field
+			 * to the end of a struct */
+};
+```
+
+- A version check can be performed by calling this function:
+
+```
+/**
+ * The strictest possible version check.
+ *
+ * @param dtc_ver       The version passed by DTC
+ * @return true on success, false on failure
+ */
+static inline bool dtc_plugin_default_version_check(struct plugin_version dtc_ver)
+{
+	struct plugin_version plugin_ver = DTC_PLUGIN_API_VERSION;
+	return dtc_ver.major == plugin_ver.major && dtc_ver.minor == plugin_ver.minor;
+}
+```
+
+- "struct plugin_arg" is defined as:
+
+```
+struct plugin_arg {
+	char *key;	/* A non-empty string */
+	char *value;	/* NULL or a non-empty string */
+};
+```
+
+Please, see an example of a "dtc_init" implementation in
+"plugins/example/example.c".
+
+
 
 II - The DT block format
 ========================
diff --git a/plugins/example/Makefile.example b/plugins/example/Makefile.example
new file mode 100644
index 0000000..9dc06e1
--- /dev/null
+++ b/plugins/example/Makefile.example
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Makefile.example
+#
+# This is not a complete Makefile of itself.  Instead, it is designed to
+# be easily embeddable into other systems of Makefiles.
+#
+
+# Allow "make plugins" to discover the plugin
+PLUGIN_LIBS += $(PLUGIN_dir)/example/example.$(SHAREDLIB_EXT)
+
+# Add GCC flags:
+# PLUGIN_CFLAGS_example = -some-flag
+
+# Add libraries:
+# PLUGIN_LDLIBS_example = -lsome-lib
+
+# Add files to clean:
+# PLUGIN_CLEANFILES += $(PLUGIN_dir)/example/*.tmp
+
+# Override the default rules:
+# $(PLUGIN_dir)/example/example.$(SHAREDLIB_EXT): $(PLUGIN_dir)/example/example.o
+#	@$(VECHO) LD $@
+#	...
+#
+# $(PLUGIN_dir)/example/example.o: $(PLUGIN_dir)/example/example.c
+#	@$(VECHO) CC $@
+#	...
diff --git a/plugins/example/example.c b/plugins/example/example.c
new file mode 100644
index 0000000..4d4e7f3
--- /dev/null
+++ b/plugins/example/example.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * (C) Copyright Arm Holdings.  2020
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include "dtc-plugin.h"
+
+#define NAME "example: "
+
+int dtc_init(struct plugin_version dtc_ver, int argc, struct plugin_arg *argv)
+{
+	if (!dtc_plugin_default_version_check(dtc_ver)) {
+		fprintf(stderr, NAME"Plugin is incompatible with this version of DTC\n");
+		return 1;
+	}
+
+	for (int i = 0; i < argc; i++) {
+		if (strcmp(argv[i].key, "h") == 0
+		 || strcmp(argv[i].key, "help") == 0) {
+			printf(NAME"This is an example plugin. It prints its arguments.\n");
+			return 1;
+		} else {
+			printf(NAME"%s: %s\n", argv[i].key,
+			       argv[i].value ? argv[i].value : "");
+		}
+	}
+
+	printf(NAME"Loaded plugin 'example'\n");
+	return 0;
+}
-- 
2.17.1

[RFC PATCH v3 4/4] dtc: Add a plugin interface

[Andrei Ziureaev]

Add support for building and loading plugins.

A plugin interface makes it possible to add checks in any language. It
also allows these checks to print dts source line information.

Signed-off-by: Andrei Ziureaev <andrei.ziureaev-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Andrei Ziureaev <andreiziureaev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---

Changes in v3:
- plugins have to implement prototypes
- better wording of comments and messages

Changes in v2:
- describe the data model in dtc-plugin.h
- plugins must register with the build system
- the "validate_fn_t" hook can return a status
- specify that minor versions are compatible
- check if plugin_dir is NULL, just in case
- better variable names in register_plugin_info

 Makefile     |  29 ++++++++++-
 dtc-plugin.h |  76 +++++++++++++++++++++++++++
 dtc.c        | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 dtc.h        |  46 +++++++++++++++++
 4 files changed, 291 insertions(+), 2 deletions(-)
 create mode 100644 dtc-plugin.h

diff --git a/Makefile b/Makefile
index c187d5f..e96bc6e 100644
--- a/Makefile
+++ b/Makefile
@@ -25,6 +25,8 @@ WARNINGS = -Wall -Wpointer-arith -Wcast-qual -Wnested-externs \
 	-Wstrict-prototypes -Wmissing-prototypes -Wredundant-decls -Wshadow
 CFLAGS = -g -Os $(SHAREDLIB_CFLAGS) -Werror $(WARNINGS) $(EXTRA_CFLAGS)
 
+LDLIBS_dtc += -ldl -export-dynamic
+
 BISON = bison
 LEX = flex
 SWIG = swig
@@ -66,14 +68,17 @@ ifeq ($(HOSTOS),darwin)
 SHAREDLIB_EXT     = dylib
 SHAREDLIB_CFLAGS  = -fPIC
 SHAREDLIB_LDFLAGS = -fPIC -dynamiclib -Wl,-install_name -Wl,
+PLUGIN_ldflags = -fPIC -dynamiclib
 else ifeq ($(HOSTOS),$(filter $(HOSTOS),msys cygwin))
 SHAREDLIB_EXT     = so
 SHAREDLIB_CFLAGS  =
 SHAREDLIB_LDFLAGS = -shared -Wl,--version-script=$(LIBFDT_version) -Wl,-soname,
+PLUGIN_ldflags = -shared
 else
 SHAREDLIB_EXT     = so
 SHAREDLIB_CFLAGS  = -fPIC
 SHAREDLIB_LDFLAGS = -fPIC -shared -Wl,--version-script=$(LIBFDT_version) -Wl,-soname,
+PLUGIN_ldflags = -fPIC -shared
 endif
 
 #
@@ -187,7 +192,29 @@ ifneq ($(MAKECMDGOALS),libfdt)
 endif
 endif
 
+#
+# Rules for plugins
+#
+PLUGIN_dir = plugins
+PLUGIN_CLEANFILES += $(PLUGIN_dir)/*.$(SHAREDLIB_EXT)
+PLUGIN_CLEANFILES += $(addprefix $(PLUGIN_dir)/*/,*.o *.$(SHAREDLIB_EXT))
+
+include $(wildcard $(PLUGIN_dir)/*/Makefile.*)
+
+plugins: $(PLUGIN_LIBS)
+
+$(PLUGIN_dir)/%.$(SHAREDLIB_EXT): $(PLUGIN_dir)/%.o
+	@$(VECHO) LD $@
+	$(LINK.c) $(PLUGIN_ldflags) -o $@ $< $(PLUGIN_LDLIBS_$(notdir $*))
+	ln -sf $(patsubst $(PLUGIN_dir)/%,%,$@) $(PLUGIN_dir)/$(notdir $@)
+
+$(PLUGIN_dir)/%.o: $(PLUGIN_dir)/%.c
+	@$(VECHO) CC $@
+	$(CC) $(CPPFLAGS) $(CFLAGS) $(PLUGIN_CFLAGS_$(notdir $*)) -o $@ -c $<
 
+plugins_clean:
+	@$(VECHO) CLEAN "(plugins)"
+	rm -f $(PLUGIN_CLEANFILES)
 
 #
 # Rules for libfdt
@@ -331,7 +358,7 @@ endif
 STD_CLEANFILES = *~ *.o *.$(SHAREDLIB_EXT) *.d *.a *.i *.s core a.out vgcore.* \
 	*.tab.[ch] *.lex.c *.output
 
-clean: libfdt_clean pylibfdt_clean tests_clean
+clean: libfdt_clean pylibfdt_clean tests_clean plugins_clean
 	@$(VECHO) CLEAN
 	rm -f $(STD_CLEANFILES)
 	rm -f $(VERSION_FILE)
diff --git a/dtc-plugin.h b/dtc-plugin.h
new file mode 100644
index 0000000..ea904bc
--- /dev/null
+++ b/dtc-plugin.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef DTC_PLUGIN_H
+#define DTC_PLUGIN_H
+
+/*
+ * (C) Copyright Arm Holdings.  2020
+ */
+
+#include "dt.h"
+
+struct plugin_arg {
+	char *key;	/* A non-empty string */
+	char *value;	/* NULL or a non-empty string */
+};
+
+struct plugin_version {
+	int major;	/* Incompatible changes */
+	int minor;	/* Compatible changes, such as adding a new field
+			 * to the end of a struct */
+};
+
+#define DTC_PLUGIN_API_VERSION ((struct plugin_version){ .major = 0, .minor = 0 })
+
+/**
+ * The strictest possible version check.
+ *
+ * @param dtc_ver       The version passed by DTC
+ * @return true on success, false on failure
+ */
+static inline bool dtc_plugin_default_version_check(struct plugin_version dtc_ver)
+{
+	struct plugin_version plugin_ver = DTC_PLUGIN_API_VERSION;
+	return dtc_ver.major == plugin_ver.major && dtc_ver.minor == plugin_ver.minor;
+}
+
+/*
+ * Plugins export functionality by implementing one or more of the
+ * functions below. DTC tries to call each function exactly once for
+ * each plugin.
+ *
+ * The typedefs are there for conveniently storing pointers to these
+ * functions.
+ */
+
+/**
+ * Initialize the plugin.
+ *
+ * Called right after the plugin is loaded.
+ *
+ * Every plugin must implement this. At the very least, it should
+ * perform a version check.
+ *
+ * @param dtc_ver       DTC's plugin API version
+ * @param argc          Length of argv
+ * @param argv          Array of plugin arguments
+ * @return 0 on success, or 1 on failure
+ */
+int dtc_init(struct plugin_version dtc_ver, int argc, struct plugin_arg *argv);
+typedef int (*dtc_init_fn_t)(struct plugin_version dtc_ver, int argc,
+			     struct plugin_arg *argv);
+
+/**
+ * Validate a device tree.
+ *
+ * Called after DTC's parsing stage, but before the output stage.
+ *
+ * @param dti   The unflattened device tree. Implementations can modify
+ *              it and "pass it back" to DTC and to subsequent plugins.
+ *              The header "dt.h" contains functionality for accessing
+ *              "struct dt_info".
+ * @return 1 on a fatal failure, otherwise 0
+ */
+int dtc_validate(struct dt_info *dti);
+typedef int (*dtc_validate_fn_t)(struct dt_info *dti);
+
+#endif /* DTC_PLUGIN_H */
diff --git a/dtc.c b/dtc.c
index bdb3f59..89f67aa 100644
--- a/dtc.c
+++ b/dtc.c
@@ -4,6 +4,7 @@
  */
 
 #include <sys/stat.h>
+#include <dlfcn.h>
 
 #include "dtc.h"
 #include "srcpos.h"
@@ -47,7 +48,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
 
 /* Usage related data. */
 static const char usage_synopsis[] = "dtc [options] <input file>";
-static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv";
+static const char usage_short_opts[] = "qI:O:o:V:d:l:L:R:S:p:a:fb:i:H:sW:E:@AThv";
 static struct option const usage_long_opts[] = {
 	{"quiet",            no_argument, NULL, 'q'},
 	{"in-format",         a_argument, NULL, 'I'},
@@ -55,6 +56,8 @@ static struct option const usage_long_opts[] = {
 	{"out-format",        a_argument, NULL, 'O'},
 	{"out-version",       a_argument, NULL, 'V'},
 	{"out-dependency",    a_argument, NULL, 'd'},
+	{"plugin",            a_argument, NULL, 'l'},
+	{"plugin-dir",        a_argument, NULL, 'L'},
 	{"reserve",           a_argument, NULL, 'R'},
 	{"space",             a_argument, NULL, 'S'},
 	{"pad",               a_argument, NULL, 'p'},
@@ -89,6 +92,13 @@ static const char * const usage_opts_help[] = {
 	 "\t\tasm - assembler source",
 	"\n\tBlob version to produce, defaults to "stringify(DEFAULT_FDT_VERSION)" (for dtb and asm output)",
 	"\n\tOutput dependency file",
+	"\n\tLoad a plugin and, optionally, pass it an argument.\n"
+	"\tUsage: -l <plugin name>[,key[,value]]\n"
+	 "\t\tplugin name - the name of the shared object without the extension (can contain a path)\n"
+	 "\t\tkey         - the name of the argument\n"
+	 "\t\tvalue       - the value of the argument (can be omitted)\n"
+	"\tExample: dtc -l some-plugin,o,out.dts -l some-plugin,help -l another-plugin [...]",
+	"\n\tThe directory in which to search for plugins",
 	"\n\tMake space for <number> reserve map entries (for dtb and asm output)",
 	"\n\tMake the blob at least <bytes> long (extra space)",
 	"\n\tAdd padding to the blob of <bytes> long (extra space)",
@@ -157,6 +167,114 @@ static const char *guess_input_format(const char *fname, const char *fallback)
 	return guess_type_by_name(fname, fallback);
 }
 
+static struct plugin *get_plugin_with_path(struct plugin_array *plugins,
+					   char *path)
+{
+	struct plugin *p;
+	array_for_each(plugins, p)
+		if (strcmp(p->path, path) == 0)
+			return p;
+
+	return NULL;
+}
+
+static void parse_plugin_info(char *arg, const char *plugin_dir, char **path,
+			      char **key, char **value)
+{
+	char *name;
+	size_t dirlen;
+	char *tmp;
+	size_t tmplen;
+
+	if (arg == NULL || *arg == '\0' || *arg == ',' || plugin_dir == NULL)
+		return;
+
+	name = strtok(arg, ",");
+
+	dirlen = strlen(plugin_dir);
+	tmplen = dirlen + strlen(name) + strlen(SHAREDLIB_EXT) + 2;
+	tmp = xmalloc(tmplen);
+
+	if (plugin_dir[0] == '\0' || plugin_dir[dirlen - 1] == DIR_SEPARATOR)
+		snprintf(tmp, tmplen, "%s%s%s", plugin_dir,
+			 name, SHAREDLIB_EXT);
+	else
+		snprintf(tmp, tmplen, "%s%c%s%s", plugin_dir,
+			 DIR_SEPARATOR, name, SHAREDLIB_EXT);
+
+	*path = realpath(tmp, NULL); /* malloc path */
+	if (*path == NULL)
+		die("Couldn't resolve plugin path %s: %s\n", tmp, strerror(errno));
+
+	*key = strtok(NULL, ",");
+	*value = strtok(NULL, "");
+	free(tmp);
+}
+
+static void register_plugin_info(struct plugin_array *plugins, char *arg,
+				 const char *plugin_dir)
+{
+	char *path = NULL;
+	char *key = NULL;
+	char *value = NULL;
+	struct plugin *old_plugin;
+	struct plugin new_plugin;
+	struct plugin_arg p_arg;
+
+	parse_plugin_info(arg, plugin_dir, &path, &key, &value);
+
+	if (path == NULL)
+		return;
+
+	p_arg = (struct plugin_arg){ .key = key, .value = value };
+	old_plugin = get_plugin_with_path(plugins, path);
+
+	if (old_plugin == NULL) {
+		new_plugin = (struct plugin){ .path = path, .args = empty_array };
+
+		if (key != NULL)
+			array_add(&new_plugin.args, p_arg);
+
+		array_add(plugins, new_plugin);
+		return;
+	}
+
+	if (key != NULL)
+		array_add(&old_plugin->args, p_arg);
+
+	free(path);
+}
+
+static void load_plugins(struct plugin_array *plugins,
+			 const struct string_array *plugin_args,
+			 const char *plugin_dir)
+{
+	dtc_init_fn_t init;
+	struct plugin *p;
+	char **arg;
+
+	array_for_each(plugin_args, arg) {
+		register_plugin_info(plugins, *arg, plugin_dir);
+	}
+
+	array_for_each(plugins, p) {
+		p->handle = dlopen(p->path, RTLD_NOW | RTLD_GLOBAL | RTLD_DEEPBIND);
+		if (p->handle == NULL)
+			die("Couldn't load plugin %s: %s\n", p->path, dlerror());
+
+		init = dlsym(p->handle, "dtc_init");
+		if (init == NULL)
+			die("Plugin %s needs to implement dtc_init\n",
+			    p->path);
+
+		if (init(DTC_PLUGIN_API_VERSION, p->args.len, p->args.data)) {
+			fprintf(stderr, "DTC: Plugin %s wasn't initialized. Exiting DTC.\n",
+				p->path);
+			exit(1);
+		}
+	}
+}
+
 int main(int argc, char *argv[])
 {
 	struct dt_info *dti;
@@ -170,6 +288,10 @@ int main(int argc, char *argv[])
 	FILE *outf = NULL;
 	int outversion = DEFAULT_FDT_VERSION;
 	long long cmdline_boot_cpuid = -1;
+	struct plugin_array plugins = empty_array;
+	struct plugin *plugin;
+	struct string_array plugin_args = empty_array;
+	const char *plugin_dir = "";
 
 	quiet      = 0;
 	reservenum = 0;
@@ -194,6 +316,12 @@ int main(int argc, char *argv[])
 		case 'd':
 			depname = optarg;
 			break;
+		case 'l':
+			array_add(&plugin_args, optarg);
+			break;
+		case 'L':
+			plugin_dir = optarg;
+			break;
 		case 'R':
 			reservenum = strtol(optarg, NULL, 0);
 			break;
@@ -283,6 +411,8 @@ int main(int argc, char *argv[])
 		fprintf(depfile, "%s:", outname);
 	}
 
+	load_plugins(&plugins, &plugin_args, plugin_dir);
+
 	if (inform == NULL)
 		inform = guess_input_format(arg, "dts");
 	if (outform == NULL) {
@@ -324,6 +454,15 @@ int main(int argc, char *argv[])
 
 	process_checks(force, dti);
 
+	array_for_each(&plugins, plugin) {
+		dtc_validate_fn_t val = dlsym(plugin->handle, "dtc_validate");
+		if (val && val(dti)) {
+			fprintf(stderr, "DTC: Plugin %s failed to validate the "
+				"device tree. Exiting DTC.\n", plugin->path);
+			exit(1);
+		}
+	}
+
 	if (auto_label_aliases)
 		generate_label_tree(dti, "aliases", false);
 
@@ -365,5 +504,6 @@ int main(int argc, char *argv[])
 		die("Unknown output format \"%s\"\n", outform);
 	}
 
+	/* Leak the plugins and the live tree */
 	exit(0);
 }
diff --git a/dtc.h b/dtc.h
index 286d999..e66e5d1 100644
--- a/dtc.h
+++ b/dtc.h
@@ -22,6 +22,7 @@
 #include <fdt.h>
 
 #include "util.h"
+#include "dtc-plugin.h"
 
 #ifdef DEBUG
 #define debug(...)	printf(__VA_ARGS__)
@@ -340,4 +341,49 @@ void dt_to_yaml(FILE *f, struct dt_info *dti);
 
 struct dt_info *dt_from_fs(const char *dirname);
 
+/* Plugins */
+
+struct plugin_arg_array {
+	size_t cap;
+	size_t len;
+	struct plugin_arg *data;
+};
+
+struct plugin {
+	const char *path;
+	struct plugin_arg_array args;
+	void *handle;
+};
+
+struct plugin_array {
+	size_t cap;
+	size_t len;
+	struct plugin *data;
+};
+
+struct string_array {
+	size_t cap;
+	size_t len;
+	char **data;
+};
+
+#define empty_array { 0 }
+
+/* Don't add elements to an array while iterating over it */
+#define array_for_each(a, p) \
+	for ((p) = (a)->data; (p) < (a)->data + (a)->len; (p)++)
+
+/* Invalidates all pointers to array members because of the realloc */
+#define array_add(a, p) (						\
+{									\
+	if ((a)->len == (a)->cap) {					\
+		(a)->cap = (a)->cap * 2 + 1;				\
+		(a)->data = xrealloc((a)->data, (a)->cap * sizeof(p));	\
+	}								\
+	(a)->data[(a)->len++] = (p);					\
+})
+
+#define DIR_SEPARATOR '/'
+#define SHAREDLIB_EXT ".so"
+
 #endif /* DTC_H */
-- 
2.17.1

Re: [RFC PATCH v3 2/4] dtc: Add a live tree API

[Simon Glass]

Hi Andrei,

On Sun, 6 Sep 2020 at 07:13, Andrei Ziureaev <andreiziureaev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> The purpose of this API is to completely abstract away the internal live
> tree implementation.
>
> This API can traverse the tree and read its values. This is enough to
> validate the tree in Python, for example. We could add more
> functionality, documentation and tests, and turn it into a separate
> library.
>
> Signed-off-by: Andrei Ziureaev <andrei.ziureaev-5wv7dgnIgG8@public.gmane.org>
> Signed-off-by: Andrei Ziureaev <andreiziureaev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  Makefile.dtc |   1 +
>  dt.c         | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  dt.h         |  63 ++++++++++++++++++++
>  3 files changed, 222 insertions(+)
>  create mode 100644 dt.c
>  create mode 100644 dt.h
>
> diff --git a/Makefile.dtc b/Makefile.dtc
> index 9c467b0..09b3123 100644
> --- a/Makefile.dtc
> +++ b/Makefile.dtc
> @@ -7,6 +7,7 @@
>  DTC_SRCS = \
>         checks.c \
>         data.c \
> +       dt.c \
>         dtc.c \
>         flattree.c \
>         fstree.c \
> diff --git a/dt.c b/dt.c
> new file mode 100644
> index 0000000..5024e63
> --- /dev/null
> +++ b/dt.c
> @@ -0,0 +1,158 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * (C) Copyright Arm Holdings.  2020
> + */
> +
> +#include "dt.h"
> +#include "dtc.h"
> +
> +struct node *dt_root(struct dt_info *dti)
> +{
> +       return dti->dt;
> +}
> +
> +struct node *dt_first_child(struct node *node)
> +{
> +       return node->children;
> +}
> +
> +struct node *dt_next_sibling(struct node *node)
> +{
> +       return node->next_sibling;
> +}
> +
> +struct property *dt_first_property(struct node *node)
> +{
> +       return node->proplist;
> +}
> +
> +struct property *dt_next_property(struct property *prop)
> +{
> +       return prop->next;
> +}
> +
> +struct marker *dt_first_row(struct property *prop)
> +{
> +       return next_type_marker(prop->val.markers);
> +}
> +
> +struct marker *dt_next_row(struct marker *row)
> +{
> +       return next_type_marker(row->next);
> +}
> +
> +int dt_row_length(struct property *prop, struct marker *row)
> +{
> +       int length_bytes = type_marker_length(row) ? : prop->val.len - row->offset;
> +       return length_bytes / dt_cell_width_bytes(row);
> +}
> +
> +uint64_t dt_uint(struct property *prop, struct marker *row, int col)
> +{
> +       int width = dt_cell_width_bytes(row);
> +       const char *p = &dt_string(prop, row)[col * width];
> +
> +       switch(width) {
> +       case 2:
> +               return dtb_ld16(p);
> +       case 4:
> +               return dtb_ld32(p);
> +       case 8:
> +               return dtb_ld64(p);
> +       default:
> +               return *(const uint8_t*)p;
> +       }
> +}
> +
> +bool dt_cell_is_phandle(struct property *prop, struct marker *row, int col)
> +{
> +       int width = dt_cell_width_bytes(row);
> +       int off = row->offset + col * width;
> +
> +       if (width != 4)
> +               return false;
> +
> +       return markers_have_type_at_offset(row, REF_PHANDLE, off);
> +}
> +
> +const char *dt_string(struct property *prop, struct marker *row)
> +{
> +       return &prop->val.val[row->offset];
> +}
> +
> +enum dt_type dt_row_type(struct marker *row)
> +{
> +       switch (row->type) {
> +       /* fallthrough */
> +       case TYPE_UINT8:
> +       case TYPE_UINT16:
> +       case TYPE_UINT32:
> +       case TYPE_UINT64:
> +               return DT_TYPE_UINT;
> +       case TYPE_STRING:
> +               return DT_TYPE_STRING;
> +       default:
> +               return DT_TYPE_NONE;
> +       }
> +}
> +
> +const char *dt_row_type_name(struct marker *row)
> +{
> +       switch (dt_row_type(row)) {
> +       case DT_TYPE_UINT:
> +               return "uint";
> +       case DT_TYPE_STRING:
> +               return "string";
> +       default:
> +               return "none";
> +       }
> +}
> +
> +int dt_cell_width_bytes(struct marker *row)
> +{
> +       switch (row->type) {
> +       case TYPE_UINT16:
> +               return 2;
> +       case TYPE_UINT32:
> +               return 4;
> +       case TYPE_UINT64:
> +               return 8;
> +       default:
> +               return 1;
> +       }
> +}
> +
> +const char *dt_node_name(struct node *node)
> +{
> +       return *node->name ? node->name : "/";
> +}
> +
> +struct srcpos *dt_node_srcpos(struct node *node)
> +{
> +       return node->srcpos;
> +}
> +
> +const char *dt_property_name(struct property *prop)
> +{
> +       return prop->name;
> +}
> +
> +struct srcpos *dt_property_srcpos(struct property *prop)
> +{
> +       return prop->srcpos;
> +}
> +
> +const char *dt_srcpos_dir(struct srcpos *srcpos)
> +{
> +       return srcpos->file->dir;
> +}
> +
> +const char *dt_srcpos_file_name(struct srcpos *srcpos)
> +{
> +       return srcpos->file->name;
> +}
> +
> +int dt_srcpos_first_line(struct srcpos *srcpos)
> +{
> +       return srcpos->first_line;
> +}
> diff --git a/dt.h b/dt.h
> new file mode 100644
> index 0000000..c6af112
> --- /dev/null
> +++ b/dt.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef DT_H
> +#define DT_H
> +
> +/*
> + * (C) Copyright Arm Holdings.  2020
> + */
> +
> +#include <stdint.h>
> +#include <stdbool.h>
> +
> +#include "srcpos.h"
> +
> +struct dt_info;
> +struct node;
> +struct property;
> +struct marker;
> +
> +enum dt_type {
> +       DT_TYPE_NONE,
> +       DT_TYPE_UINT,
> +       DT_TYPE_STRING,
> +};
> +
> +/* Traversal */
> +struct node *dt_root(struct dt_info *dti);
> +
> +struct node *dt_first_child(struct node *node);
> +struct node *dt_next_sibling(struct node *node);
> +
> +struct property *dt_first_property(struct node *node);
> +struct property *dt_next_property(struct property *prop);
> +
> +/* A property value resembles a matrix (or a table). A marker with data
> + * type information can represent a row.
> + *
> + * So, each property has a linked list of rows. */
> +struct marker *dt_first_row(struct property *prop);
> +struct marker *dt_next_row(struct marker *row);
> +
> +/* Accessing data. Each row is an array of cells. */
> +int dt_row_length(struct property *prop, struct marker *row);
> +
> +uint64_t dt_uint(struct property *prop, struct marker *row, int col);
> +bool dt_cell_is_phandle(struct property *prop, struct marker *row, int col);
> +const char *dt_string(struct property *prop, struct marker *row);
> +
> +/* Accessing metadata */
> +enum dt_type dt_row_type(struct marker *row);
> +const char *dt_row_type_name(struct marker *row);
> +int dt_cell_width_bytes(struct marker *row);
> +
> +const char *dt_node_name(struct node *node);
> +struct srcpos *dt_node_srcpos(struct node *node);
> +
> +const char *dt_property_name(struct property *prop);
> +struct srcpos *dt_property_srcpos(struct property *prop);
> +
> +const char *dt_srcpos_dir(struct srcpos *srcpos);
> +const char *dt_srcpos_file_name(struct srcpos *srcpos);
> +int dt_srcpos_first_line(struct srcpos *srcpos);

How about adding full comments for these functions?

> +
> +#endif /* DT_H */
> --
> 2.17.1
>

Regards,
Simon

Re: [RFC PATCH v3 2/4] dtc: Add a live tree API

[Andrei Ziureaev]

On 07/09/2020 02:44, Simon Glass wrote:
> Hi Andrei,
> 
> On Sun, 6 Sep 2020 at 07:13, Andrei Ziureaev <andreiziureaev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> The purpose of this API is to completely abstract away the internal live
>> tree implementation.
>>
>> This API can traverse the tree and read its values. This is enough to
>> validate the tree in Python, for example. We could add more
>> functionality, documentation and tests, and turn it into a separate
>> library.
>>
>> Signed-off-by: Andrei Ziureaev <andrei.ziureaev-5wv7dgnIgG8@public.gmane.org>
>> Signed-off-by: Andrei Ziureaev <andreiziureaev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   Makefile.dtc |   1 +
>>   dt.c         | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   dt.h         |  63 ++++++++++++++++++++
>>   3 files changed, 222 insertions(+)
>>   create mode 100644 dt.c
>>   create mode 100644 dt.h
>>
>> diff --git a/Makefile.dtc b/Makefile.dtc
>> index 9c467b0..09b3123 100644
>> --- a/Makefile.dtc
>> +++ b/Makefile.dtc
>> @@ -7,6 +7,7 @@
>>   DTC_SRCS = \
>>          checks.c \
>>          data.c \
>> +       dt.c \
>>          dtc.c \
>>          flattree.c \
>>          fstree.c \
>> diff --git a/dt.c b/dt.c
>> new file mode 100644
>> index 0000000..5024e63
>> --- /dev/null
>> +++ b/dt.c
>> @@ -0,0 +1,158 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * (C) Copyright Arm Holdings.  2020
>> + */
>> +
>> +#include "dt.h"
>> +#include "dtc.h"
>> +
>> +struct node *dt_root(struct dt_info *dti)
>> +{
>> +       return dti->dt;
>> +}
>> +
>> +struct node *dt_first_child(struct node *node)
>> +{
>> +       return node->children;
>> +}
>> +
>> +struct node *dt_next_sibling(struct node *node)
>> +{
>> +       return node->next_sibling;
>> +}
>> +
>> +struct property *dt_first_property(struct node *node)
>> +{
>> +       return node->proplist;
>> +}
>> +
>> +struct property *dt_next_property(struct property *prop)
>> +{
>> +       return prop->next;
>> +}
>> +
>> +struct marker *dt_first_row(struct property *prop)
>> +{
>> +       return next_type_marker(prop->val.markers);
>> +}
>> +
>> +struct marker *dt_next_row(struct marker *row)
>> +{
>> +       return next_type_marker(row->next);
>> +}
>> +
>> +int dt_row_length(struct property *prop, struct marker *row)
>> +{
>> +       int length_bytes = type_marker_length(row) ? : prop->val.len - row->offset;
>> +       return length_bytes / dt_cell_width_bytes(row);
>> +}
>> +
>> +uint64_t dt_uint(struct property *prop, struct marker *row, int col)
>> +{
>> +       int width = dt_cell_width_bytes(row);
>> +       const char *p = &dt_string(prop, row)[col * width];
>> +
>> +       switch(width) {
>> +       case 2:
>> +               return dtb_ld16(p);
>> +       case 4:
>> +               return dtb_ld32(p);
>> +       case 8:
>> +               return dtb_ld64(p);
>> +       default:
>> +               return *(const uint8_t*)p;
>> +       }
>> +}
>> +
>> +bool dt_cell_is_phandle(struct property *prop, struct marker *row, int col)
>> +{
>> +       int width = dt_cell_width_bytes(row);
>> +       int off = row->offset + col * width;
>> +
>> +       if (width != 4)
>> +               return false;
>> +
>> +       return markers_have_type_at_offset(row, REF_PHANDLE, off);
>> +}
>> +
>> +const char *dt_string(struct property *prop, struct marker *row)
>> +{
>> +       return &prop->val.val[row->offset];
>> +}
>> +
>> +enum dt_type dt_row_type(struct marker *row)
>> +{
>> +       switch (row->type) {
>> +       /* fallthrough */
>> +       case TYPE_UINT8:
>> +       case TYPE_UINT16:
>> +       case TYPE_UINT32:
>> +       case TYPE_UINT64:
>> +               return DT_TYPE_UINT;
>> +       case TYPE_STRING:
>> +               return DT_TYPE_STRING;
>> +       default:
>> +               return DT_TYPE_NONE;
>> +       }
>> +}
>> +
>> +const char *dt_row_type_name(struct marker *row)
>> +{
>> +       switch (dt_row_type(row)) {
>> +       case DT_TYPE_UINT:
>> +               return "uint";
>> +       case DT_TYPE_STRING:
>> +               return "string";
>> +       default:
>> +               return "none";
>> +       }
>> +}
>> +
>> +int dt_cell_width_bytes(struct marker *row)
>> +{
>> +       switch (row->type) {
>> +       case TYPE_UINT16:
>> +               return 2;
>> +       case TYPE_UINT32:
>> +               return 4;
>> +       case TYPE_UINT64:
>> +               return 8;
>> +       default:
>> +               return 1;
>> +       }
>> +}
>> +
>> +const char *dt_node_name(struct node *node)
>> +{
>> +       return *node->name ? node->name : "/";
>> +}
>> +
>> +struct srcpos *dt_node_srcpos(struct node *node)
>> +{
>> +       return node->srcpos;
>> +}
>> +
>> +const char *dt_property_name(struct property *prop)
>> +{
>> +       return prop->name;
>> +}
>> +
>> +struct srcpos *dt_property_srcpos(struct property *prop)
>> +{
>> +       return prop->srcpos;
>> +}
>> +
>> +const char *dt_srcpos_dir(struct srcpos *srcpos)
>> +{
>> +       return srcpos->file->dir;
>> +}
>> +
>> +const char *dt_srcpos_file_name(struct srcpos *srcpos)
>> +{
>> +       return srcpos->file->name;
>> +}
>> +
>> +int dt_srcpos_first_line(struct srcpos *srcpos)
>> +{
>> +       return srcpos->first_line;
>> +}
>> diff --git a/dt.h b/dt.h
>> new file mode 100644
>> index 0000000..c6af112
>> --- /dev/null
>> +++ b/dt.h
>> @@ -0,0 +1,63 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +#ifndef DT_H
>> +#define DT_H
>> +
>> +/*
>> + * (C) Copyright Arm Holdings.  2020
>> + */
>> +
>> +#include <stdint.h>
>> +#include <stdbool.h>
>> +
>> +#include "srcpos.h"
>> +
>> +struct dt_info;
>> +struct node;
>> +struct property;
>> +struct marker;
>> +
>> +enum dt_type {
>> +       DT_TYPE_NONE,
>> +       DT_TYPE_UINT,
>> +       DT_TYPE_STRING,
>> +};
>> +
>> +/* Traversal */
>> +struct node *dt_root(struct dt_info *dti);
>> +
>> +struct node *dt_first_child(struct node *node);
>> +struct node *dt_next_sibling(struct node *node);
>> +
>> +struct property *dt_first_property(struct node *node);
>> +struct property *dt_next_property(struct property *prop);
>> +
>> +/* A property value resembles a matrix (or a table). A marker with data
>> + * type information can represent a row.
>> + *
>> + * So, each property has a linked list of rows. */
>> +struct marker *dt_first_row(struct property *prop);
>> +struct marker *dt_next_row(struct marker *row);
>> +
>> +/* Accessing data. Each row is an array of cells. */
>> +int dt_row_length(struct property *prop, struct marker *row);
>> +
>> +uint64_t dt_uint(struct property *prop, struct marker *row, int col);
>> +bool dt_cell_is_phandle(struct property *prop, struct marker *row, int col);
>> +const char *dt_string(struct property *prop, struct marker *row);
>> +
>> +/* Accessing metadata */
>> +enum dt_type dt_row_type(struct marker *row);
>> +const char *dt_row_type_name(struct marker *row);
>> +int dt_cell_width_bytes(struct marker *row);
>> +
>> +const char *dt_node_name(struct node *node);
>> +struct srcpos *dt_node_srcpos(struct node *node);
>> +
>> +const char *dt_property_name(struct property *prop);
>> +struct srcpos *dt_property_srcpos(struct property *prop);
>> +
>> +const char *dt_srcpos_dir(struct srcpos *srcpos);
>> +const char *dt_srcpos_file_name(struct srcpos *srcpos);
>> +int dt_srcpos_first_line(struct srcpos *srcpos);
> 
> How about adding full comments for these functions?

OK, will do.

> 
>> +
>> +#endif /* DT_H */
>> --
>> 2.17.1
>>
> 
> Regards,
> Simon
> 

Re: [RFC PATCH v3 0/4] dtc: Add a plugin interface

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4144 bytes --]

On Sun, Sep 06, 2020 at 02:12:16PM +0100, Andrei Ziureaev wrote:
> I added a simple live tree API. There's no longer a need to move any
> definitions. The live tree struct is hidden from plugins, so it's not an ABI.
> The API doesn't allow plugins to modify the live tree yet, but that will
> probably change later.

Hrm.  So.  Before we get into any of the details of implementation, I
think we need to be clear on what it means to expose the "live tree".
The live tree in dtc was never designed to be an exposed structure, so
we need to think about that carefully before we do expose it, because
we now have to consider stability of it across releases which we never
did before.

AFAICT the fact the tree is "live" (i.e. a pointer-based random access
representation) isn't really the relevant point here.  We could easily
enough serialize/deserialize the tree to get it into plugins, using
whatever format - and dtb is the obvious one.

What you're really after here is access to some of the internal
metadata that dtc maintains about the tree: line numbers at least,
possibly other things.

I think we really need to pin down *what* parts of this information
we need to expose to the plugins.  The data model for the information
that's going back and forth is more important than exactly how we
encode the data across the boundary.

> 
> I also removed the EXPORT_FUNCTION macro. Plugins now have to implement
> certain prototypes, defined in dtc-plugin.h. This allows plugins to be
> generated by tools.
> ===========
> 
> This is one possible implementation of a plugin interface for DTC.
> 
> A plugin interface would allow us to add checks in languages other than
> C. Specifically, it would allow DTC and DT Schema (which is written in
> Python) to be integrated.
> 
> It would also allow for better debugging of dts files. For example, DT
> Schema would be able to print dts source line information, without us
> having to add source line annotations to yaml files.
> 
> In the future, plugins will be allowed to modify the live tree and pass
> it back for further processing, but the API doesn't support that yet.
> 
> There's a question of whether we should relicense some headers to
> dual BSD to be able call Python code.
> 
> Any thoughts would be much appreciated.
> 
> Thanks,
> Andrei.
> 
> Changes in v3:
> - live tree API (dt.h and dt.c)
> - additional functionality in dtc.h
> - plugins have to implement prototypes
> - improved documentation
> 
> Changes in v2:
> - improved documentation
> - plugins must register with the build system
> - the "validate_fn_t" hook can return a status
> - define "struct reserve_info" in "dtc-plugin.h"
> - specify that minor versions are compatible
> - check if plugin_dir is NULL, just in case
> - better variable names in register_plugin_info
> 
> Andrei Ziureaev (4):
>   dtc: Add marker type functionality to dtc.h
>   dtc: Add a live tree API
>   dtc: Add plugin documentation and examples
>   dtc: Add a plugin interface
> 
>  Documentation/manual.txt         | 146 ++++++++++++++++++++++++++++
>  Makefile                         |  29 +++++-
>  Makefile.dtc                     |   1 +
>  dt.c                             | 158 +++++++++++++++++++++++++++++++
>  dt.h                             |  63 ++++++++++++
>  dtc-plugin.h                     |  76 +++++++++++++++
>  dtc.c                            | 142 ++++++++++++++++++++++++++-
>  dtc.h                            |  57 +++++++++++
>  plugins/example/Makefile.example |  27 ++++++
>  plugins/example/example.c        |  33 +++++++
>  treesource.c                     |   8 +-
>  11 files changed, 734 insertions(+), 6 deletions(-)
>  create mode 100644 dt.c
>  create mode 100644 dt.h
>  create mode 100644 dtc-plugin.h
>  create mode 100644 plugins/example/Makefile.example
>  create mode 100644 plugins/example/example.c
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH v3 0/4] dtc: Add a plugin interface

[Andrei Ziureaev]

On 11/09/2020 08:06, David Gibson wrote:
> On Sun, Sep 06, 2020 at 02:12:16PM +0100, Andrei Ziureaev wrote:
>> I added a simple live tree API. There's no longer a need to move any
>> definitions. The live tree struct is hidden from plugins, so it's not an ABI.
>> The API doesn't allow plugins to modify the live tree yet, but that will
>> probably change later.
> 
> Hrm.  So.  Before we get into any of the details of implementation, I
> think we need to be clear on what it means to expose the "live tree".
> The live tree in dtc was never designed to be an exposed structure, so
> we need to think about that carefully before we do expose it, because
> we now have to consider stability of it across releases which we never
> did before.

Thanks for the comments.

Unlike in previous versions, in this version I'm passing opaque pointers 
between DTC and the plugins. So,
"struct dt_info" and all the other related structs are not an ABI, but 
the new live tree "library" is. The signatures in "dt.h" need to be 
designed carefully.
> 
> AFAICT the fact the tree is "live" (i.e. a pointer-based random access
> representation) isn't really the relevant point here.  We could easily
> enough serialize/deserialize the tree to get it into plugins, using
> whatever format - and dtb is the obvious one.

Yes, we could use any format, such as dtb, yaml or json, but there 
should be access to line numbers and it should be fast.
> 
> What you're really after here is access to some of the internal
> metadata that dtc maintains about the tree: line numbers at least,
> possibly other things.
> 
> I think we really need to pin down *what* parts of this information
> we need to expose to the plugins.  The data model for the information
> that's going back and forth is more important than exactly how we
> encode the data across the boundary.

The way I came up with this live tree API is I looked at Rob's code that 
converts the tree to Python structures and calls DT Schema
(https://github.com/robherring/dtc/blob/lib-plugin/yamlchecks.c),
and I tried to see what data it needs to access. Turns out, it needs 
read-only access to at least:
- the file name, line number, node name and property name
- the value of the property (strings and ints)
- if the value is an int, its width and whether it's a phandle

So those are the functions I defined in "dt.h". I've added some docs to 
them, so I should probably send a v4 soon.
> 
>>
>> I also removed the EXPORT_FUNCTION macro. Plugins now have to implement
>> certain prototypes, defined in dtc-plugin.h. This allows plugins to be
>> generated by tools.
>> ===========
>>
>> This is one possible implementation of a plugin interface for DTC.
>>
>> A plugin interface would allow us to add checks in languages other than
>> C. Specifically, it would allow DTC and DT Schema (which is written in
>> Python) to be integrated.
>>
>> It would also allow for better debugging of dts files. For example, DT
>> Schema would be able to print dts source line information, without us
>> having to add source line annotations to yaml files.
>>
>> In the future, plugins will be allowed to modify the live tree and pass
>> it back for further processing, but the API doesn't support that yet.
>>
>> There's a question of whether we should relicense some headers to
>> dual BSD to be able call Python code.
>>
>> Any thoughts would be much appreciated.
>>
>> Thanks,
>> Andrei.
>>
>> Changes in v3:
>> - live tree API (dt.h and dt.c)
>> - additional functionality in dtc.h
>> - plugins have to implement prototypes
>> - improved documentation
>>
>> Changes in v2:
>> - improved documentation
>> - plugins must register with the build system
>> - the "validate_fn_t" hook can return a status
>> - define "struct reserve_info" in "dtc-plugin.h"
>> - specify that minor versions are compatible
>> - check if plugin_dir is NULL, just in case
>> - better variable names in register_plugin_info
>>
>> Andrei Ziureaev (4):
>>    dtc: Add marker type functionality to dtc.h
>>    dtc: Add a live tree API
>>    dtc: Add plugin documentation and examples
>>    dtc: Add a plugin interface
>>
>>   Documentation/manual.txt         | 146 ++++++++++++++++++++++++++++
>>   Makefile                         |  29 +++++-
>>   Makefile.dtc                     |   1 +
>>   dt.c                             | 158 +++++++++++++++++++++++++++++++
>>   dt.h                             |  63 ++++++++++++
>>   dtc-plugin.h                     |  76 +++++++++++++++
>>   dtc.c                            | 142 ++++++++++++++++++++++++++-
>>   dtc.h                            |  57 +++++++++++
>>   plugins/example/Makefile.example |  27 ++++++
>>   plugins/example/example.c        |  33 +++++++
>>   treesource.c                     |   8 +-
>>   11 files changed, 734 insertions(+), 6 deletions(-)
>>   create mode 100644 dt.c
>>   create mode 100644 dt.h
>>   create mode 100644 dtc-plugin.h
>>   create mode 100644 plugins/example/Makefile.example
>>   create mode 100644 plugins/example/example.c
>>
>