On 7/23/20 3:46 PM, Rob Herring wrote:
On Tue, Jul 21, 2020 at 9:59 AM Andrei Ziureaev <andrei.ziureaev@xxxxxxx> wrote: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@xxxxxxx> --- Makefile | 32 +++++++++++- dtc-plugin.h | 27 ++++++++++ dtc.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++- dtc.h | 47 +++++++++++++++++ 4 files changed, 243 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index cb256e8..76bba53 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 @@ -65,14 +67,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 # @@ -186,7 +191,32 @@ ifneq ($(MAKECMDGOALS),libfdt) endif endif +# +# Rules for plugins +# +PLUGIN_dir = plugins +PLUGIN_subdirs = $(patsubst %/,%,$(filter-out $(PLUGIN_dir)/,$(dir $(wildcard $(PLUGIN_dir)/*/)))) +PLUGIN_names = $(notdir $(PLUGIN_subdirs)) +PLUGIN_libs = $(join $(PLUGIN_subdirs),$(patsubst %,/%.$(SHAREDLIB_EXT),$(PLUGIN_names))) +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 @@ -330,7 +360,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 index 35c3d19..9ec6b81 100644 --- a/dtc-plugin.h +++ b/dtc-plugin.h @@ -10,6 +10,8 @@ #include <stdint.h> #include <stdbool.h> +#include "srcpos.h" +This belongs in patch 3.struct dt_info { const char *version;Okay, so we don't need this as you are checking the version in a different way.
Oops, I forgot to remove this.
unsigned int dtsflags; @@ -170,4 +172,29 @@ static inline size_t type_marker_length(struct marker *m) return 0; } +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; /* Somewhat incompatible changes */A minor bump should be compatible changes. A new field added to the end of a struct for example.+}; + +#define DTC_PLUGIN_API_VERSION ((struct plugin_version){ .major = 0, .minor = 0 }) + +static inline bool dtc_plugin_default_version_check(struct plugin_version v)Maybe 'dtc_ver' instead of 'v'.+{ + struct plugin_version this_v = DTC_PLUGIN_API_VERSION; + return v.major == this_v.major && v.minor == this_v.minor;I think, by default, minor version differences should be okay.
I originally wanted this whole interface to be similar to gcc's (it turned out nothing like gcc's), and gcc's default version check is the strictest check possible. I guess the idea is that plugin authors can write their own less strict checks if they want. But if minor versions are compatible, then maybe there's no point in checking them, even by default.
+} + +/* Hook definitions */ +typedef int (*init_fn_t)(struct plugin_version v, int argc, struct plugin_arg *argv); +typedef void (*validate_fn_t)(struct dt_info *dti); + +#define TYPE_TO_NAME(type) dtc_plugin_v0_##type +#define EXPORT_FUNCTION(type, fn) type TYPE_TO_NAME(type) = (fn) + #endif /* DTC_PLUGIN_H */ diff --git a/dtc.c b/dtc.c index bdb3f59..5a9b118 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 - can be omitted\n" + "\tExample: dtc -lsome-plugin,o,out.dts -lsome-plugin,help -l path/to/another-plugin [...]",Should be '-L': -L path/to/another-plugin
"-L" specifies the directory and "-l" - a plugin name with an optional path and an argument. I guess we can simplify this, get rid of "-l", and just have "-L" take a full path and an argument. Since this is going to be called mostly from makefiles, the extra typing shouldn't be a problem. We could also pass all the arguments at once with something like: -L path/plugin-name,"key=value key2=value2" I do, however, want the plugins to receive key-value pairs and not have to parse the arguments themselves.
+ "\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 == ',') + 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 *pp; + struct plugin p; + struct plugin_arg a; + + parse_plugin_info(arg, plugin_dir, &path, &key, &value); + + if (path == NULL) + return; + + a = (struct plugin_arg){ .key = key, .value = value }; + pp = get_plugin_with_path(plugins, path); + + if (pp == NULL) { + p = (struct plugin){ .path = path, .args = empty_array }; + + if (key != NULL) + array_add(&p.args, a); + + array_add(plugins, p); + return; + } + + if (key != NULL) + array_add(&pp->args, a); + + free(path); +} + +static void load_plugins(struct plugin_array *plugins, + const struct string_array *plugin_args, + const char *plugin_dir) +{ + 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 = GET_SYMBOL(p, init_fn_t); + if (init == NULL) + die("Plugin %s needs to export function of type init_fn_t\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,12 @@ int main(int argc, char *argv[]) process_checks(force, dti); + array_for_each(&plugins, plugin) { + validate_fn_t *validate = GET_SYMBOL(plugin, validate_fn_t); + if (validate) + (*validate)(dti); + } + if (auto_label_aliases) generate_label_tree(dti, "aliases", false); @@ -365,5 +501,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 f31aed7..4b6280e 100644 --- a/dtc.h +++ b/dtc.h @@ -186,4 +186,51 @@ 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 GET_SYMBOL(plugin, type) ((type *)dlsym((plugin)->handle, stringify(TYPE_TO_NAME(type)))) + +#define DIR_SEPARATOR '/' +#define SHAREDLIB_EXT ".so" + #endif /* DTC_H */ -- 2.17.1
