From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6BE8BC433EF for ; Mon, 7 Mar 2022 04:26:37 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B3EF481DD0; Mon, 7 Mar 2022 05:26:34 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="l5i8vsze"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 89D2781CFC; Mon, 7 Mar 2022 05:26:32 +0100 (CET) Received: from mail-pf1-x431.google.com (mail-pf1-x431.google.com [IPv6:2607:f8b0:4864:20::431]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id EB98A81DD0 for ; Mon, 7 Mar 2022 05:26:24 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=takahiro.akashi@linaro.org Received: by mail-pf1-x431.google.com with SMTP id p8so12665282pfh.8 for ; Sun, 06 Mar 2022 20:26:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=iU1ktVgOcUgCn/B39JR4+ry0TcJkwGB6hxgDum2vALc=; b=l5i8vszeXum/zXebgWYO7aMbkKw3FjVdIUetviUZsImGhxpdMMM16RngD77fCUAAJi 8CaT7g5cmBX5uHEOHOkE/YHMAfgJipNuaEYkNRzQKRBoeGqL1WaOAK0fldoYQkVSlHYl P8olV2U3i1lVjY70izmga1tEpVG95sbdd8syoeJqWyGpldVv2d40qG07158QrxNnOQ/G kZyM2xR9/ArPMy8KSI3CrJ8Ep1Y0SZ/WHs28YamSdmgs4Dzm7/04I3PgfdppsKzA+KLS 1+GMoNmK4/vNpJB/0U5ExTblGyQaGgGwwSOTHH1UjAtdA7Slp/RLr4EaouMpOXndgNR/ EyHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=iU1ktVgOcUgCn/B39JR4+ry0TcJkwGB6hxgDum2vALc=; b=yGXTg6XkuxVhfSsIRSmSklA3DCNxfO2L2Tx7E4QEHJR6AR0mgjqG4OarIOadNUTeC9 e99w8kZFNWQzLfY/jErlyM7afyOwTQwJmQhFKX98PkNnFmUg3N3pbWMtGCrv1gdk4aqM YPX3JYajmA4gO9r8EArcrzqUizJ4x39DLq0bzA2VbDiLidCPeH+GQweyKvVfDFX809Q3 UXjkGakyijR/V9g3trv+diG2uKIO/9CHcQ36OcKqhjhR6O6GI48ojMgs8HQ5fzuoVWQ4 cX7GiHX8AjycryjO0agGdzT1liKrz+cuAQynKc0Y3Ht0XswWnb2yLP84rttqLhqaqo5b afxA== X-Gm-Message-State: AOAM5337jEFGeu2J38nEIo96p4WcZ7c8Y3IJgoPVBJVjJMUrug7cWWbs i6nBv1BehHd2hsqvw3Ye9SR73Q== X-Google-Smtp-Source: ABdhPJwdJ948vyP69o2v7QepD+4JMdPwzcjcbB3ypLDN34o6sbCaEPgncQE+EHZX4TjN1+QXvpSwXQ== X-Received: by 2002:a63:920f:0:b0:378:9ef8:7978 with SMTP id o15-20020a63920f000000b003789ef87978mr8143265pgd.587.1646627182965; Sun, 06 Mar 2022 20:26:22 -0800 (PST) Received: from laputa ([2400:4050:c3e1:100:a84a:efbf:c31f:5412]) by smtp.gmail.com with ESMTPSA id oo16-20020a17090b1c9000b001b89e05e2b2sm11427170pjb.34.2022.03.06.20.26.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 06 Mar 2022 20:26:22 -0800 (PST) Date: Mon, 7 Mar 2022 13:26:18 +0900 From: AKASHI Takahiro To: Simon Glass Cc: U-Boot Mailing List , Marek Vasut , Masahiro Yamada , Tom Rini , Marek Beh??n , Heinrich Schuchardt Subject: Re: [PATCH v2 05/13] event: Add basic support for events Message-ID: <20220307042618.GA145463@laputa> Mail-Followup-To: AKASHI Takahiro , Simon Glass , U-Boot Mailing List , Marek Vasut , Masahiro Yamada , Tom Rini , Marek Beh??n , Heinrich Schuchardt References: <20220304154308.2547711-1-sjg@chromium.org> <20220304154308.2547711-6-sjg@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220304154308.2547711-6-sjg@chromium.org> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean 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 > --- > > 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 > +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 > + */ > + > +#define LOG_CATEGORY LOGC_EVENT > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +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 > #include > #include > #include > @@ -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 > + */ > + > +#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 > + */ > + > +#ifndef __event_internal_h > +#define __event_internal_h > + > +#include > +#include > + > +/** > + * 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 >