[PATCH v2 05/13] event: Add basic support for events
AKASHI Takahiro
takahiro.akashi at linaro.org
Mon Mar 7 05:26:18 CET 2022
Hi Simon,
On Fri, Mar 04, 2022 at 08:43:00AM -0700, Simon Glass wrote:
> Add a way to create and dispatch events without needing to allocate
> memory. Also add a way to 'spy' on events, thus allowing 'hooks' to be
> created.
>
> Use a linker list for static events, which we can use to replace functions
> like arch_cpu_init_f(). Allow an EVENT_DEBUG option which makes it
> easier to see what is going on at runtime, but uses more code space.
>
> Dynamic events allow the creation of a spy at runtime. This is not always
> necessary, but can be enabled with EVENT_DYNAMIC if needed.
>
> A 'test' event is the only option for now.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v2:
> - Add a 'used' attribute to avoid LTO dropping the code
>
> MAINTAINERS | 6 +
> common/Kconfig | 31 +++++
> common/Makefile | 2 +
> common/board_r.c | 1 +
> common/event.c | 186 +++++++++++++++++++++++++++++
> common/log.c | 1 +
> include/asm-generic/global_data.h | 13 +++
> include/event.h | 188 ++++++++++++++++++++++++++++++
> include/event_internal.h | 35 ++++++
> include/log.h | 2 +
> 10 files changed, 465 insertions(+)
> create mode 100644 common/event.c
> create mode 100644 include/event.h
> create mode 100644 include/event_internal.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fb171e0c68..b534ad66b9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -809,6 +809,12 @@ S: Maintained
> F: doc/usage/environment.rst
> F: scripts/env2string.awk
>
> +EVENTS
> +M: Simon Glass <sjg at chromium.org>
> +S: Maintained
> +F: common/event.c
> +F: include/event.h
> +
> FASTBOOT
> S: Orphaned
> F: cmd/fastboot.c
> diff --git a/common/Kconfig b/common/Kconfig
> index 82cd864baf..76c04b2001 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -492,6 +492,37 @@ config DISPLAY_BOARDINFO_LATE
>
> menu "Start-up hooks"
>
> +config EVENT
> + bool "General-purpose event-handling mechanism"
I don't think that this config option needs to be visible (or user-selectable).
Instead, any subsystem that needs it should explicitly select it.
I prefer that subsystem can add "select EVENT or DM_EVENT" rather than
"imply" or "depends on".
-Takahiro Akashi
> + default y if SANDBOX
> + help
> + This enables sending and processing of events, to allow interested
> + parties to be alerted when something happens. This is an attempt to
> + step the flow of weak functions, hooks, functions in board_f.c
> + and board_r.c and the Kconfig options below.
> +
> + See doc/develop/event.rst for more information.
> +
> +if EVENT
> +
> +config EVENT_DYNAMIC
> + bool "Support event registration at runtime"
> + default y if SANDBOX
> + help
> + Enable this to support adding an event spy at runtime, without adding
> + it to the EVENT_SPy() linker list. This increases code size slightly
> + but provides more flexibility for boards and subsystems that need it.
> +
> +config EVENT_DEBUG
> + bool "Enable event debugging assistance"
> + default y if SANDBOX
> + help
> + Enable this get usefui features for seeing what is happening with
> + events, such as event-type names. This adds to the code size of
> + U-Boot so can be turned off for production builds.
> +
> +endif # EVENT
> +
> config ARCH_EARLY_INIT_R
> bool "Call arch-specific init soon after relocation"
> help
> diff --git a/common/Makefile b/common/Makefile
> index 3eff719601..cc2ba30c63 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -89,6 +89,8 @@ obj-y += malloc_simple.o
> endif
> endif
>
> +obj-$(CONFIG_$(SPL_TPL_)EVENT) += event.o
> +
> obj-$(CONFIG_$(SPL_TPL_)HASH) += hash.o
> obj-$(CONFIG_IO_TRACE) += iotrace.o
> obj-y += memsize.o
> diff --git a/common/board_r.c b/common/board_r.c
> index c24d9b4e22..b92c1bb0be 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -594,6 +594,7 @@ static int run_main_loop(void)
> static init_fnc_t init_sequence_r[] = {
> initr_trace,
> initr_reloc,
> + event_init,
> /* TODO: could x86/PPC have this also perhaps? */
> #if defined(CONFIG_ARM) || defined(CONFIG_RISCV)
> initr_caches,
> diff --git a/common/event.c b/common/event.c
> new file mode 100644
> index 0000000000..366be24569
> --- /dev/null
> +++ b/common/event.c
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Events provide a general-purpose way to react to / subscribe to changes
> + * within U-Boot
> + *
> + * Copyright 2021 Google LLC
> + * Written by Simon Glass <sjg at chromium.org>
> + */
> +
> +#define LOG_CATEGORY LOGC_EVENT
> +
> +#include <common.h>
> +#include <event.h>
> +#include <event_internal.h>
> +#include <log.h>
> +#include <linker_lists.h>
> +#include <malloc.h>
> +#include <asm/global_data.h>
> +#include <linux/list.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#if CONFIG_IS_ENABLED(EVENT_DEBUG)
> +const char *const type_name[] = {
> + "none",
> + "test",
> +};
> +
> +_Static_assert(ARRAY_SIZE(type_name) == EVT_COUNT, "event type_name size");
> +#endif
> +
> +static const char *event_type_name(enum event_t type)
> +{
> +#if CONFIG_IS_ENABLED(EVENT_DEBUG)
> + return type_name[type];
> +#else
> + return "(unknown)";
> +#endif
> +}
> +
> +static int notify_static(struct event *ev)
> +{
> + struct evspy_info *start =
> + ll_entry_start(struct evspy_info, evspy_info);
> + const int n_ents = ll_entry_count(struct evspy_info, evspy_info);
> + struct evspy_info *spy;
> +
> + for (spy = start; spy != start + n_ents; spy++) {
> + if (spy->type == ev->type) {
> + int ret;
> +
> + log_debug("Sending event %x/%s to spy '%s'\n", ev->type,
> + event_type_name(ev->type), event_spy_id(spy));
> + ret = spy->func(NULL, ev);
> +
> + /*
> + * TODO: Handle various return codes to
> + *
> + * - claim an event (no others will see it)
> + * - return an error from the event
> + */
> + if (ret)
> + return log_msg_ret("spy", ret);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int notify_dynamic(struct event *ev)
> +{
> + struct event_state *state = gd_event_state();
> + struct event_spy *spy, *next;
> +
> + list_for_each_entry_safe(spy, next, &state->spy_head, sibling_node) {
> + if (spy->type == ev->type) {
> + int ret;
> +
> + log_debug("Sending event %x/%s to spy '%s'\n", ev->type,
> + event_type_name(ev->type), spy->id);
> + ret = spy->func(spy->ctx, ev);
> +
> + /*
> + * TODO: Handle various return codes to
> + *
> + * - claim an event (no others will see it)
> + * - return an error from the event
> + */
> + if (ret)
> + return log_msg_ret("spy", ret);
> + }
> + }
> +
> + return 0;
> +}
> +
> +int event_notify(enum event_t type, void *data, int size)
> +{
> + struct event event;
> + int ret;
> +
> + event.type = type;
> + if (size > sizeof(event.data))
> + return log_msg_ret("size", -E2BIG);
> + memcpy(&event.data, data, size);
> +
> + ret = notify_static(&event);
> + if (ret)
> + return log_msg_ret("dyn", ret);
> +
> + if (CONFIG_IS_ENABLED(EVENT_DYNAMIC)) {
> + ret = notify_dynamic(&event);
> + if (ret)
> + return log_msg_ret("dyn", ret);
> + }
> +
> + return 0;
> +}
> +
> +int event_notify_null(enum event_t type)
> +{
> + return event_notify(type, NULL, 0);
> +}
> +
> +void event_show_spy_list(void)
> +{
> + struct evspy_info *start =
> + ll_entry_start(struct evspy_info, evspy_info);
> + const int n_ents = ll_entry_count(struct evspy_info, evspy_info);
> + struct evspy_info *spy;
> + const int size = sizeof(ulong) * 2;
> +
> + printf("Seq %-24s %*s %s\n", "Type", size, "Function", "ID");
> + for (spy = start; spy != start + n_ents; spy++) {
> + printf("%3x %-3x %-20s %*p %s\n", (uint)(spy - start),
> + spy->type, event_type_name(spy->type), size, spy->func,
> + event_spy_id(spy));
> + }
> +}
> +
> +#if CONFIG_IS_ENABLED(EVENT_DYNAMIC)
> +static void spy_free(struct event_spy *spy)
> +{
> + list_del(&spy->sibling_node);
> +}
> +
> +int event_register(const char *id, enum event_t type, event_handler_t func, void *ctx)
> +{
> + struct event_state *state = gd_event_state();
> + struct event_spy *spy;
> +
> + if (!CONFIG_IS_ENABLED(EVENT_DYNAMIC))
> + return -ENOSYS;
> + spy = malloc(sizeof(*spy));
> + if (!spy)
> + return log_msg_ret("alloc", -ENOMEM);
> +
> + spy->id = id;
> + spy->type = type;
> + spy->func = func;
> + spy->ctx = ctx;
> + list_add_tail(&spy->sibling_node, &state->spy_head);
> +
> + return 0;
> +}
> +
> +int event_uninit(void)
> +{
> + struct event_state *state = gd_event_state();
> + struct event_spy *spy, *next;
> +
> + list_for_each_entry_safe(spy, next, &state->spy_head, sibling_node)
> + spy_free(spy);
> +
> + return 0;
> +}
> +
> +int event_init(void)
> +{
> + struct event_state *state = gd_event_state();
> +
> + INIT_LIST_HEAD(&state->spy_head);
> +
> + return 0;
> +}
> +#endif /* EVENT_DYNAMIC */
> diff --git a/common/log.c b/common/log.c
> index f7e0c0fbf5..7254aa70bf 100644
> --- a/common/log.c
> +++ b/common/log.c
> @@ -28,6 +28,7 @@ static const char *const log_cat_name[] = {
> "devres",
> "acpi",
> "boot",
> + "event",
> };
>
> _Static_assert(ARRAY_SIZE(log_cat_name) == LOGC_COUNT - LOGC_NONE,
> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index c2f8fad1cb..e49f5bf2f7 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -20,6 +20,7 @@
> */
>
> #ifndef __ASSEMBLY__
> +#include <event_internal.h>
> #include <fdtdec.h>
> #include <membuff.h>
> #include <linux/list.h>
> @@ -467,6 +468,12 @@ struct global_data {
> */
> char *smbios_version;
> #endif
> +#if CONFIG_IS_ENABLED(EVENT)
> + /**
> + * @event_state: Points to the current state of events
> + */
> + struct event_state event_state;
> +#endif
> };
> #ifndef DO_DEPS_ONLY
> static_assert(sizeof(struct global_data) == GD_SIZE);
> @@ -532,6 +539,12 @@ static_assert(sizeof(struct global_data) == GD_SIZE);
> #define gd_set_multi_dtb_fit(_dtb)
> #endif
>
> +#if CONFIG_IS_ENABLED(EVENT_DYNAMIC)
> +#define gd_event_state() ((struct event_state *)&gd->event_state)
> +#else
> +#define gd_event_state() NULL
> +#endif
> +
> /**
> * enum gd_flags - global data flags
> *
> diff --git a/include/event.h b/include/event.h
> new file mode 100644
> index 0000000000..effd58c704
> --- /dev/null
> +++ b/include/event.h
> @@ -0,0 +1,188 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Events provide a general-purpose way to react to / subscribe to changes
> + * within U-Boot
> + *
> + * Copyright 2021 Google LLC
> + * Written by Simon Glass <sjg at chromium.org>
> + */
> +
> +#ifndef __event_h
> +#define __event_h
> +
> +/**
> + * enum event_t - Types of events supported by U-Boot
> + *
> + * @EVT_DM_PRE_PROBE: Device is about to be probed
> + */
> +enum event_t {
> + EVT_NONE,
> + EVT_TEST,
> +
> + EVT_COUNT
> +};
> +
> +union event_data {
> + /**
> + * struct event_data_test - test data
> + *
> + * @signal: A value to update the state with
> + */
> + struct event_data_test {
> + int signal;
> + } test;
> +};
> +
> +/**
> + * struct event - an event that can be sent and received
> + *
> + * @type: Event type
> + * @data: Data for this particular event
> + */
> +struct event {
> + enum event_t type;
> + union event_data data;
> +};
> +
> +/** Function type for event handlers */
> +typedef int (*event_handler_t)(void *ctx, struct event *event);
> +
> +/**
> + * struct evspy_info - information about an event spy
> + *
> + * @func: Function to call when the event is activated (must be first)
> + * @type: Event type
> + * @id: Event id string
> + */
> +struct evspy_info {
> + event_handler_t func;
> + enum event_t type;
> +#if CONFIG_IS_ENABLED(EVENT_DEBUG)
> + const char *id;
> +#endif
> +};
> +
> +/* Declare a new event spy */
> +#if CONFIG_IS_ENABLED(EVENT_DEBUG)
> +#define _ESPY_REC(_type, _func) { _func, _type, #_func, }
> +#else
> +#define _ESPY_REC(_type, _func) { _func, _type, }
> +#endif
> +
> +static inline const char *event_spy_id(struct evspy_info *spy)
> +{
> +#if CONFIG_IS_ENABLED(EVENT_DEBUG)
> + return spy->id;
> +#else
> + return "?";
> +#endif
> +}
> +
> +/*
> + * It seems that LTO will drop list entries if it decides they are not used,
> + * although the conditions that cause this are unclear.
> + *
> + * The example found is the following:
> + *
> + * static int sandbox_misc_init_f(void *ctx, struct event *event)
> + * {
> + * return sandbox_early_getopt_check();
> + * }
> + * EVENT_SPY(EVT_MISC_INIT_F, sandbox_misc_init_f);
> + *
> + * where EVENT_SPY uses ll_entry_declare()
> + *
> + * In this case, LTO decides to drop the sandbox_misc_init_f() function
> + * (which is fine) but then drops the linker-list entry too. This means
> + * that the code no longer works, in this case sandbox no-longer checks its
> + * command-line arguments properly.
> + *
> + * Without LTO, the KEEP() command in the .lds file is enough to keep the
> + * entry around. But with LTO it seems that the entry has already been
> + * dropped before the link script is considered.
> + *
> + * The only solution I can think of is to mark linker-list entries as 'used'
> + * using an attribute. This should be safe, since we don't actually want to drop
> + * any of these. However this does slightly limit LTO's optimisation choices.
> + */
> +#define EVENT_SPY(_type, _func) \
> + static __attribute__((used)) ll_entry_declare(struct evspy_info, \
> + _type, evspy_info) = \
> + _ESPY_REC(_type, _func)
> +
> +/**
> + * event_register - register a new spy
> + *
> + * @id: Spy ID
> + * @type: Event type to subscribe to
> + * @func: Function to call when the event is sent
> + * @ctx: Context to pass to the function
> + * @return 0 if OK, -ve on error
> + */
> +int event_register(const char *id, enum event_t type, event_handler_t func,
> + void *ctx);
> +
> +#if CONFIG_IS_ENABLED(EVENT)
> +/**
> + * event_notify() - notify spies about an event
> + *
> + * It is possible to pass in union event_data here but that may not be
> + * convenient if the data is elsewhere, or is one of the members of the union.
> + * So this uses a void * for @data, with a separate @size.
> + *
> + * @type: Event type
> + * @data: Event data to be sent (e.g. union_event_data)
> + * @size: Size of data in bytes
> + * @return 0 if OK, -ve on error
> + */
> +int event_notify(enum event_t type, void *data, int size);
> +
> +/**
> + * event_notify_null() - notify spies about an event
> + *
> + * Data is NULL and the size is 0
> + *
> + * @type: Event type
> + * @return 0 if OK, -ve on error
> + */
> +int event_notify_null(enum event_t type);
> +#else
> +static inline int event_notify(enum event_t type, void *data, int size)
> +{
> + return 0;
> +}
> +
> +static inline int event_notify_null(enum event_t type)
> +{
> + return 0;
> +}
> +#endif
> +
> +#if CONFIG_IS_ENABLED(EVENT_DYNAMIC)
> +/**
> + * event_uninit() - Clean up dynamic events
> + *
> + * This removes all dynamic event handlers
> + */
> +int event_uninit(void);
> +
> +/**
> + * event_uninit() - Set up dynamic events
> + *
> + * Init a list of dynamic event handlers, so that these can be added as
> + * needed
> + */
> +int event_init(void);
> +#else
> +static inline int event_uninit(void)
> +{
> + return 0;
> +}
> +
> +static inline int event_init(void)
> +{
> + return 0;
> +}
> +#endif
> +
> +#endif
> diff --git a/include/event_internal.h b/include/event_internal.h
> new file mode 100644
> index 0000000000..8432c6f0e5
> --- /dev/null
> +++ b/include/event_internal.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Internal definitions for events
> + *
> + * Copyright 2021 Google LLC
> + * Written by Simon Glass <sjg at chromium.org>
> + */
> +
> +#ifndef __event_internal_h
> +#define __event_internal_h
> +
> +#include <event.h>
> +#include <linux/list.h>
> +
> +/**
> + * struct event_spy - a spy that watches for an event of a particular type
> + *
> + * @id: Spy ID
> + * @type: Event type to subscribe to
> + * @func: Function to call when the event is sent
> + * @ctx: Context to pass to the function
> + */
> +struct event_spy {
> + struct list_head sibling_node;
> + const char *id;
> + enum event_t type;
> + event_handler_t func;
> + void *ctx;
> +};
> +
> +struct event_state {
> + struct list_head spy_head;
> +};
> +
> +#endif
> diff --git a/include/log.h b/include/log.h
> index ce48d51446..8f35c10abb 100644
> --- a/include/log.h
> +++ b/include/log.h
> @@ -98,6 +98,8 @@ enum log_category_t {
> LOGC_ACPI,
> /** @LOGC_BOOT: Related to boot process / boot image processing */
> LOGC_BOOT,
> + /** @LOGC_EVENT: Related to event and event handling */
> + LOGC_EVENT,
> /** @LOGC_COUNT: Number of log categories */
> LOGC_COUNT,
> /** @LOGC_END: Sentinel value for lists of log categories */
> --
> 2.35.1.616.g0bdcbb4464-goog
>
More information about the U-Boot
mailing list