diff mbox series

[v2,05/13] event: Add basic support for events

Message ID 20220304154308.2547711-6-sjg@chromium.org
State Accepted
Commit 87a5d1b5d012b0663517bfa36f5e01c8028f121a
Delegated to: Tom Rini
Headers show
Series event: Provide support for events to connect subsystems | expand

Commit Message

Simon Glass March 4, 2022, 3:43 p.m. UTC
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@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

Comments

AKASHI Takahiro March 7, 2022, 4:26 a.m. UTC | #1
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@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@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@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@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@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
>
Simon Glass March 8, 2022, 4:05 p.m. UTC | #2
Hi Takahiro,

On Sun, 6 Mar 2022 at 21:26, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> 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@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@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".

But events can be added for other reasons. For example a board may
want to detect that a USB controller is about to be probed and enable
power to devices on that USB bus so that the devices are enumerated.
This series is partly about replacing all the various hooks we
currently have but goes further than that.

[..]

Regards,
Simon
Tom Rini March 10, 2022, 1:25 p.m. UTC | #3
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@chromium.org>

Applied to u-boot/next, thanks!
diff mbox series

Patch

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@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"
+	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@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@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@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 */