Re: [RFC PATCH v2 01/31] tracing: Add a comment about ftrace_regs definition
On Thu, Nov 09, 2023 at 08:14:52AM +0900, Masami Hiramatsu wrote: > On Wed, 8 Nov 2023 23:24:32 +0900 > "Masami Hiramatsu (Google)" wrote: > > > From: Masami Hiramatsu (Google) > > > > To clarify what will be expected on ftrace_regs, add a comment to the > > architecture independent definition of the ftrace_regs. > > > > Signed-off-by: Masami Hiramatsu (Google) > > --- > > Changes in v2: > > - newly added. > > --- > > include/linux/ftrace.h | 25 + > > 1 file changed, 25 insertions(+) > > > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > > index e8921871ef9a..b174af91d8be 100644 > > --- a/include/linux/ftrace.h > > +++ b/include/linux/ftrace.h > > @@ -118,6 +118,31 @@ extern int ftrace_enabled; > > > > #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > > > > +/** > > + * ftrace_regs - ftrace partial/optimal register set > > + * > > + * ftrace_regs represents a group of registers which is used at the > > + * function entry and exit. There are three types of registers. > > + * > > + * - Registers for passing the parameters to callee, including the stack > > + * pointer. (e.g. rcx, rdx, rdi, rsi, r8, r9 and rsp on x86_64) > > + * - Registers for passing the return values to caller. > > + * (e.g. rax and rdx on x86_64) > > + * - Registers for hooking the function return including the frame pointer > > + * (the frame pointer is architecture/config dependent) > > + * (e.g. rbp and rsp for x86_64) > > Oops, I found the program counter/instruction pointer must be saved too. > This is used for live patching. One question is that if the IP is modified > at the return handler, what should we do? Return to the specified address? I'm a bit confused here; currently we use fgraph_ret_regs for function returns, are we going to replace that with ftrace_regs? I think it makes sense for the PC/IP to be the address the return handler will eventually return to (and hence allowing it to be overridden), but that does mean we'll need to go recover the return address *before* we invoke any return handlers. Thanks, Mark.
[PATCH 2/2] configure: Check that provided paths are absolute
configure checks that its built-in directory options get an absolute path. Copy the check for custom options. Signed-off-by: Michal Suchanek --- v6: new patch --- configure.ac | 17 + 1 file changed, 17 insertions(+) diff --git a/configure.ac b/configure.ac index d6da5ee9ae9a..de01e08cf2e8 100644 --- a/configure.ac +++ b/configure.ac @@ -97,6 +97,23 @@ AC_ARG_WITH([module_directory], [], [with_module_directory=/lib/modules]) AC_SUBST([module_directory], [$with_module_directory]) +# Check all directory arguments for consistency. +for ac_var in distconfdir rootlibdir module_directory +do + eval ac_val=\$$ac_var + # Remove trailing slashes. + case $ac_val in +*/ ) + ac_val=`expr "X$ac_val" : 'X\(.*@<:@^/@:>@\)' \| "X$ac_val" : 'X\(.*\)'` + eval $ac_var=\$ac_val;; + esac + # Be sure to have absolute directory names. + case $ac_val in +@<:@\\/$@:>@* | ?:@<:@\\/@:>@* ) continue;; + esac + as_fn_error $? "expected an absolute directory name for --$ac_var: $ac_val" +done + AC_ARG_WITH([zstd], AS_HELP_STRING([--with-zstd], [handle Zstandard-compressed modules @<:@default=disabled@:>@]), [], [with_zstd=no]) -- 2.42.0
[PATCH 1/2] libkmod, depmod, modprobe: Make directory for kernel modules configurable
Now that modprobe.d is searched under ${prefix}/lib, allow a complete transition to files only under ${prefix} by adding a ${module_directory} configuration. This specifies the directory where to search for kernel modules and should match the location where the kernel/distro installs them. With this distributions that do not want to ship files in /lib can also move kernel modules to /usr while others can keep them in /lib. Signed-off-by: Michal Suchanek --- v4: Make the whole path configurable v5: More verbose commit message v6: fix docstring still containing module_prefix --- Makefile.am | 3 +- configure.ac | 7 ++ libkmod/libkmod.c| 4 +- man/Makefile.am | 1 + man/depmod.d.xml | 6 +- man/depmod.xml | 4 +- man/modinfo.xml | 2 +- man/modprobe.xml | 2 +- man/modules.dep.xml | 6 +- testsuite/module-playground/Makefile | 2 +- testsuite/setup-rootfs.sh| 109 +++ testsuite/test-depmod.c | 16 ++-- testsuite/test-testsuite.c | 8 +- tools/depmod.c | 6 +- tools/kmod.pc.in | 1 + tools/modinfo.c | 4 +- tools/modprobe.c | 4 +- tools/static-nodes.c | 6 +- 18 files changed, 107 insertions(+), 84 deletions(-) diff --git a/Makefile.am b/Makefile.am index 2a54c25bd631..4062d81227df 100644 --- a/Makefile.am +++ b/Makefile.am @@ -20,6 +20,7 @@ AM_CPPFLAGS = \ -I$(top_srcdir) \ -DSYSCONFDIR=\""$(sysconfdir)"\" \ -DDISTCONFDIR=\""$(distconfdir)"\" \ + -DMODULE_DIRECTORY=\""$(module_directory)"\" \ ${zlib_CFLAGS} AM_CFLAGS = $(OUR_CFLAGS) @@ -220,7 +221,7 @@ EXTRA_DIST += testsuite/setup-rootfs.sh MODULE_PLAYGROUND = testsuite/module-playground ROOTFS = testsuite/rootfs ROOTFS_PRISTINE = $(top_srcdir)/testsuite/rootfs-pristine -CREATE_ROOTFS = $(AM_V_GEN) $(top_srcdir)/testsuite/setup-rootfs.sh $(ROOTFS_PRISTINE) $(ROOTFS) $(MODULE_PLAYGROUND) $(top_builddir)/config.h $(sysconfdir) +CREATE_ROOTFS = $(AM_V_GEN) MODULE_DIRECTORY=$(module_directory) $(top_srcdir)/testsuite/setup-rootfs.sh $(ROOTFS_PRISTINE) $(ROOTFS) $(MODULE_PLAYGROUND) $(top_builddir)/config.h $(sysconfdir) build-module-playground: $(AM_V_GEN)if test "$(top_srcdir)" != "$(top_builddir)"; then \ diff --git a/configure.ac b/configure.ac index a6b8fa0308b6..d6da5ee9ae9a 100644 --- a/configure.ac +++ b/configure.ac @@ -91,6 +91,12 @@ AC_ARG_WITH([rootlibdir], [], [with_rootlibdir=$libdir]) AC_SUBST([rootlibdir], [$with_rootlibdir]) +# Ideally this would be $prefix/lib/modules but default to /lib/modules for compatibility with earlier versions +AC_ARG_WITH([module_directory], +AS_HELP_STRING([--with-module-directory=DIR], [directory in which to look for kernel modules - typically '/lib/modules' or '${prefix}/lib/modules']), +[], [with_module_directory=/lib/modules]) +AC_SUBST([module_directory], [$with_module_directory]) + AC_ARG_WITH([zstd], AS_HELP_STRING([--with-zstd], [handle Zstandard-compressed modules @<:@default=disabled@:>@]), [], [with_zstd=no]) @@ -319,6 +325,7 @@ AC_MSG_RESULT([ $PACKAGE $VERSION === + module_directory: ${module_directory} prefix: ${prefix} sysconfdir: ${sysconfdir} distconfdir:${distconfdir} diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c index 57fac1cb9f7b..213b42482fb6 100644 --- a/libkmod/libkmod.c +++ b/libkmod/libkmod.c @@ -210,7 +210,7 @@ static int log_priority(const char *priority) return 0; } -static const char *dirname_default_prefix = "/lib/modules"; +static const char *dirname_default_prefix = MODULE_DIRECTORY; static char *get_kernel_release(const char *dirname) { @@ -266,7 +266,7 @@ static enum kmod_file_compression_type get_kernel_compression(struct kmod_ctx *c /** * kmod_new: * @dirname: what to consider as linux module's directory, if NULL - * defaults to /lib/modules/`uname -r`. If it's relative, + * defaults to $MODULE_DIRECTORY/`uname -r`. If it's relative, * it's treated as relative to the current working directory. * Otherwise, give an absolute dirname. * @config_paths: ordered array of paths (directories or files) where diff --git a/man/Makefile.am b/man/Makefile.am index 2fea8e46bf2f..f550091a216a 100644 --- a/man/Makefile.am +++ b/man/Makefile.am @@ -22,6 +22,7 @@ CLEANFILES = $(dist_man_MANS) else \ sed -e '/@DISTCONFDIR@/d' $< ; \ fi | \ + sed -e 's|@MODULE_DIRECTORY@|$(module_directory)|g' | \ $(XSLT) \ -o $@ \ --nonet \ diff --git a/man/depmod.d.xml b/man/depmod.d.xml i
[PATCH 0/2] kmod /usr support
Hello, This is resend of the last patch in the series that adds prefix support to kernel module location together with additional patch for validating the user supplied input to options that are interpreted as directories. Thanks Michal Michal Suchanek (2): libkmod, depmod, modprobe: Make directory for kernel modules configurable configure: Check that provided paths are absolute Makefile.am | 3 +- configure.ac | 24 ++ libkmod/libkmod.c| 4 +- man/Makefile.am | 1 + man/depmod.d.xml | 6 +- man/depmod.xml | 4 +- man/modinfo.xml | 2 +- man/modprobe.xml | 2 +- man/modules.dep.xml | 6 +- testsuite/module-playground/Makefile | 2 +- testsuite/setup-rootfs.sh| 109 +++ testsuite/test-depmod.c | 16 ++-- testsuite/test-testsuite.c | 8 +- tools/depmod.c | 6 +- tools/kmod.pc.in | 1 + tools/modinfo.c | 4 +- tools/modprobe.c | 4 +- tools/static-nodes.c | 6 +- 18 files changed, 124 insertions(+), 84 deletions(-) -- 2.42.0
Re: [PATCH for-next] tracing/kprobes: Add symbol counting check when module loads
Hi! Le dimanche 29 octobre 2023, 05:10:46 EET Masami Hiramatsu (Google) a écrit : > From: Masami Hiramatsu (Google) > > Check the number of probe target symbols in the target module when > the module is loaded. If the probe is not on the unique name symbols > in the module, it will be rejected at that point. > > Note that the symbol which has a unique name in the target module, > it will be accepted even if there are same-name symbols in the > kernel or other modules, Sorry for the delay in my review, I just remember I should review your patch. I tested it on top of latest master and it return EINVAL instead of EADDRNOTAVAIL: # Behavior without the patch: root@vm-amd64:~# uname -r 6.6.0-15859-g89cdf9d55601 root@vm-amd64:~# echo 'p:myprobe name_show' > /sys/kernel/tracing/ kprobe_events -bash: echo: write error: Cannot assign requested address # With the patch: root@vm-amd64:~# uname -r 6.6.0-15860-gd6a7e0f39ec5 root@vm-amd64:~# echo 'p:myprobe name_show' > /sys/kernel/tracing/ kprobe_events -bash: echo: write error: Invalid argument I did not GDB to track down from where it comes (it is planned nonetheless), but is this behavior wanted? > Signed-off-by: Masami Hiramatsu (Google) > --- > kernel/trace/trace_kprobe.c | 112 > ++- 1 file changed, 68 > insertions(+), 44 deletions(-) > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c > index e834f149695b..90cf2219adb4 100644 > --- a/kernel/trace/trace_kprobe.c > +++ b/kernel/trace/trace_kprobe.c > @@ -670,6 +670,21 @@ static int register_trace_kprobe(struct trace_kprobe > *tk) return ret; > } > > +static int validate_module_probe_symbol(const char *modname, const char > *symbol); + > +static int register_module_trace_kprobe(struct module *mod, struct > trace_kprobe *tk) +{ > + const char *p; > + int ret = 0; > + > + p = strchr(trace_kprobe_symbol(tk), ':'); > + if (p) > + ret = validate_module_probe_symbol(module_name(mod), p++); > + if (!ret) > + ret = register_trace_kprobe(tk); > + return ret; > +} > + > /* Module notifier call back, checking event on the module */ > static int trace_kprobe_module_callback(struct notifier_block *nb, > unsigned long val, void *data) > @@ -688,7 +703,7 @@ static int trace_kprobe_module_callback(struct > notifier_block *nb, if (trace_kprobe_within_module(tk, mod)) { > /* Don't need to check busy - this should have gone. */ > __unregister_trace_kprobe(tk); > - ret = __register_trace_kprobe(tk); > + ret = register_module_trace_kprobe(mod, tk); > if (ret) > pr_warn("Failed to re-register probe %s on %s: %d\n", > trace_probe_name(&tk->tp), > @@ -729,17 +744,55 @@ static int count_mod_symbols(void *data, const char > *name, unsigned long unused) return 0; > } > > -static unsigned int number_of_same_symbols(char *func_name) > +static unsigned int number_of_same_symbols(const char *mod, const char > *func_name) { > struct sym_count_ctx ctx = { .count = 0, .name = func_name }; > > - kallsyms_on_each_match_symbol(count_symbols, func_name, &ctx.count); > + if (!mod) > + kallsyms_on_each_match_symbol(count_symbols, func_name, &ctx.count); > > - module_kallsyms_on_each_symbol(NULL, count_mod_symbols, &ctx); > + module_kallsyms_on_each_symbol(mod, count_mod_symbols, &ctx); > > return ctx.count; > } > > +static int validate_module_probe_symbol(const char *modname, const char > *symbol) +{ > + unsigned int count = number_of_same_symbols(modname, symbol); > + > + if (count > 1) { > + /* > + * Users should use ADDR to remove the ambiguity of > + * using KSYM only. > + */ > + return -EADDRNOTAVAIL; > + } else if (count == 0) { > + /* > + * We can return ENOENT earlier than when register the > + * kprobe. > + */ > + return -ENOENT; > + } > + return 0; > +} > + > +static int validate_probe_symbol(char *symbol) > +{ > + char *mod = NULL, *p; > + int ret; > + > + p = strchr(symbol, ':'); > + if (p) { > + mod = symbol; > + symbol = p + 1; > + *p = '\0'; > + } > + ret = validate_module_probe_symbol(mod, symbol); > + if (p) > + *p = ':'; > + return ret; > +} > + > static int __trace_kprobe_create(int argc, const char *argv[]) > { > /* > @@ -859,6 +912,14 @@ static int __trace_kprobe_create(int argc, const char > *argv[]) trace_probe_log_err(0, BAD_PROBE_ADDR); > goto parse_error; > } > + ret = validate_probe_symbol(symbol); > + if (ret) { > + if (ret == -EADDRNOTAV
Re: [RFC v1 0/8] vhost-vdpa: add support for iommufd
On Thu, Nov 09, 2023 at 06:48:46PM -0500, Michael S. Tsirkin wrote: > On Tue, Nov 07, 2023 at 11:52:17AM -0400, Jason Gunthorpe wrote: > > On Tue, Nov 07, 2023 at 09:30:21AM -0500, Michael S. Tsirkin wrote: > > > On Tue, Nov 07, 2023 at 10:12:37AM -0400, Jason Gunthorpe wrote: > > > > Big company's should take the responsibility to train and provide > > > > skill development for their own staff. > > > > > > That would result in a beautiful cathedral of a patch. I know this is > > > how some companies work. We are doing more of a bazaar thing here, > > > though. In a bunch of subsystems it seems that you don't get the > > > necessary skills until you have been publically shouted at by > > > maintainers - better to start early ;). Not a nice environment for > > > novices, for sure. > > > > In my view the "shouting from maintainers" is harmful to the people > > buidling skills and it is an unkind thing to dump employees into that > > kind of situation. > > > > They should have help to establish the basic level of competence where > > they may do the wrong thing, but all the process and presentation of > > the wrong thing is top notch. You get a much better reception. > > What - like e.g. mechanically fixing checkpatch warnings without > understanding? No, not at all. I mean actually going through and explaining what the idea is to another person and ensuing that the commit messages convey that idea, that the patches reflect the idea, that everything is convayed, and it isn't obviously internally illogical. Like, why did this series have a giant block of #ifdef 0'd code with no explanation at all? That isn't checkpatch nitpicks, that is not meeting the minimum standard to convey an idea in an RFC. Jason
[POC 0/7] livepatch: Make livepatch states, callbacks, and shadow variables work together
This POC is a material for the discussion "Simplify Livepatch Callbacks, Shadow Variables, and States handling" at LPC 2013, see https://lpc.events/event/17/contributions/1541/ It obsoletes the patchset adding the garbage collection of shadow variables. This new solution is based on ideas from Nicolai Stange. And it should also be in sync with Josh's ideas mentioned into the thread about the garbage collection, see https://lore.kernel.org/r/20230204235910.4j4ame5ntqogqi7m@treble What is this all about? There are three features provided by the kernel livepatching support: + callbacks: They allow doing system modifications where the "simple" redirection to the fixed code is not enough. For example, allocate some data and allow to use them when all processes are patched. There are four optional callbacks which might be called either when the livepatch or the livepatched object is loaded. It depends who is loaded first. The are called at different stages of the livepatch transition: pre_enable, post_enable, pre_disable, post_disable. Only callbacks from the new livepatch are called during atomic replace. The motivation was that new livepatches should know how to handle the existing changes correctly. Also it simplified the semantic because it would be horrible when both callbacks from the old and new livepatch are called. The later one might break changes done by the earlier one. They are defined per-object. The idea was that they might be needed when a livepatched module is loaded or unloaded. + shadow variables: They allow attaching extra data to any existing data. For example, they allow to extend a structure. Or they allow to create a spin lock which might stay even when the livepatch gets atomically replaced. They are defined per-patch but there is no real connection. There is just an allocate/get/free API. + states: They were introduced to manage the life-cycle of changes done by the callbacks and shadow variables. They should help especially when atomic replace is used. The new livepatch need to know what changes have already been done or which need to be reverted. The states are defined per-patch. There was proposal to make them per-object but it was decided that it was not worth the complexity. Each state might have a version which allows to maintain compatibility between the livepatches. Otherwise, there is no connection with the other features. The is just an API to check whether the state was in the previous patch so that the callbacks might do an informed decisions. Observation: + States were supposed to help with the life-time of changes done by callbacks. But states are per-patch and callbacks are per-object. Also the API is hard to use. + Shadow variables were not connected with the states at all. It needs to be done by callbacks. + The decision that only the callbacks from the new livepatch gets called during atomic replace make downgrades complicated. Better solution implemented by this POC: + Transform per-object callbacks to per-state callbacks so that the state might really control the life-cycle of the changes. + Change the semantic of the callbacks, so that they are called when the state is introduced or removed. No callbacks are called when the state is just transferred during the atomic replace. + The disable/remove callbacks from the old livepatch are called from the old livepatch when the new one does not support them. These callbacks have to be there anyway so that the livepatch can get disabled. This nicely solves the problem with downgrades while keeping simple semantic. + A state might be associated with a shadow variable with the same ID. It helps to maintain the life-cycle of the shadow variable. The variable is automatically freed when the state is not longer supported during atomic replace or when the livepatch gets disabled. Also the state callbacks might help to allocate the variable do do some checks before the transition starts. But it can be enabled only after all processes are transitioned. It would prevent loading the livepatch when the shadow variable could not be used and the livepatch could cause problems. + State version is replaced with "block_disable" flag. The versions are too generic and make things complicated. In practice, the main question is whether the changes introduced by the state (callbacks) can be reverted or not. The livepatch could not be disabled or downgraded when the revert (state disable) is not supported. What is done in this POC: + All changes in livepatch code are implemented. + The existi
[POC 1/7] livepatch: Add callbacks for introducing and removing states
The basic livepatch functionality is to redirect problematic functions to a fixed or improved variants. In addition, there are two features helping with more problematic situations: + pre_patch(), post_patch(), pre_unpatch(), post_unpatch() callbacks might be called before and after the respective transitions. For example, post_patch() callback might enable some functionality at the end of the transition when the entire system is using the new code. + Shadow variables allow to add new items into structures or other data objects. The practice has shown that these features were hard to use with the atomic replace feature. The new livepatch usually just adds more fixes. But it might also remove problematic ones. Originally, any version of the livepatch was allowed to replace any older or newer version of the patch. It was not clear how to handle the extra features. The new patch did not know whether to run the callbacks or if the changes were already done by the current livepatch. Or if it has to revert some changes or free shadow variables whey they would not longer be supported. It was even more complicated because only the callbacks from the newly installed livepatch were called. It means that older livepatch might not be able to revert changes supported only by newer livepatches. The above problems were supposed to be solved by adding livepatch states. Each livepatch might define which states are supported. The states are versioned. The livepatch core checks if the newly installed livepatch is able to handle all states used by the currently installed livepatch. Though the practice has shown that the states API was not easy to use either. It was not well connected with the callbacks and shadow variables. The states are per-patch. The callbacks are per-object. The livepatch does not know about the supported shadow variables at all. As a first step, new per-state callbacks are introduced: + "setup" is called before the livepatch is applied but only when the state is new. It might be used to allocate some memory. Or it might check if the state change is safe on the running system. If it fails, the patch will not be enabled. + "enable" is called after the livepatch is applied but only when the state is new. It might be used to enable using some functionality provided by the livepatch after the entire system is livepatched. + "disable" is called before the livepatch is disabled or replaced. When replacing, the callback is called only when the new livepatch does not support the related state. And it uses the implementation from the to-be-replaced livepatch. The to-be-replaced livepatch needed the callback to allow disabling the livepatch anyway. The new livepatch does not need to know anything about the state. It might be used to disable using some functionality which will not longer be supported after the livepatch gets disabled. + "release" is called after the livepatch was disabled or replaced. There are the same rules for replacement as for "disable" callback. It might be used for freeing some memory or unused shadow variables. These callbacks are going to replace the existing ones. It would cause some changes: + The new callbacks are not called when a livepatched object is loaded or removed later. The practice shows that per-object callbacks are not worth supporting. In a rare case, when a per-object callback is needed. the livepatch might register a custom module notifier. + The new callbacks are called only when the state is introduced or removed. It does not handle the situation when the newly installed livepatch continues using an existing state. The practice shows that this is exactly what is needed. In the rare case when this is not enough, an extra takeover might be done in the module->init() callback. The per-state callbacks are called in similar code paths as the per-object ones. Especially, the ordering against the other operations is the same. Though, there are some obvious and less obvious changes: + The per-state callbacks are called for the entire patch instead of loaded object. It means that they called outside the for-each-object cycle. + The per-state callbacks are called when a state is introduced or obsoleted. Both variants might happen when the atomic replace is used. + In __klp_enable_patch(), the per-state callbacks are called before the smp_wmb() while the per-object ones are called later. The new location makes more sense. The setup of the state should be ready before processes start being transferred. The per-object callbacks were called after the barrier. They were using and already existing for-cycle. And nobody did mind about the ordering. Signed-off-by: Petr Mladek --- include/linux/livepatch.h | 28 kernel/livepa
[POC 2/7] livepatch: Allow to handle lifetime of shadow variables using the livepatch state
The handling of the lifetime of the shadow variables is not easy when the atomic replace is used. The new patch does not know if a shadow variable has already been used by the previous livepatch. Or if there is a shadow variable which will not longer be used. Shadow variables are almost always used together with callbacks. At least @post_unpatch callback is used to free not longer used shadow variables. And sometimes @post_patch and @pre_unpatch callbacks are used to enable or disable the use of the shadow variables. It is needed when the shadow variable can be used only when the entire system is able to handle them. All this gets even more complicated because the original callbacks are called only from the new livepatch when atomic replace is used. Newly created livepatches might be able to handle obsolete shadow variables so the upgrade would work. But older livepatches do not know anything about later introduced shadow variables. They might leak during downgrade. And they might contain outdated information when another upgrade would start using them again. All problems are better solved with the new callbacks associated with a livepatch state. They are called when the state is first introduced and when it gets obsolete. Also the callbacks are called from the patch where the state was supported. So that even downgrade might be safe. Let's make it official. A shadow variable might be associated with a livepatch state by setting the new "state.is_shadow" flag and using the same "id" in both struct klp_shadow and struct klp_state. The shadow variable will then have the same lifetime as the livepatch state. It allows to free obsolete shadow variables automatically without the need to add a callback. The generic callback will free the shadow variables using state->callbacks.shadow_dtor callback when provided. Signed-off-by: Petr Mladek --- include/linux/livepatch.h | 15 ++- kernel/livepatch/state.c | 14 ++ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index c2a39e5f5b66..189ec7c6a89f 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -132,12 +132,18 @@ struct klp_object { struct klp_patch; struct klp_state; +typedef int (*klp_shadow_ctor_t)(void *obj, +void *shadow_data, +void *ctor_data); +typedef void (*klp_shadow_dtor_t)(void *obj, void *shadow_data); + /** * struct klp_state_callbacks - callbacks manipulating the state * @setup: executed before code patching when the state is added * @enable:executed after code patching when the state is added * @disable: executed before code unpatching when the state is removed * @release: executed after code unpatching when the state is removed + * @shadow_dtor: destructor for the related shadow variable * @setup_succeeded: internal state used by a rollback on error * * All callbacks are optional. @@ -152,6 +158,7 @@ struct klp_state_callbacks { void (*enable)(struct klp_patch *patch, struct klp_state *state); void (*disable)(struct klp_patch *patch, struct klp_state *state); void (*release)(struct klp_patch *patch, struct klp_state *state); + klp_shadow_dtor_t shadow_dtor; bool setup_succeeded; }; @@ -160,12 +167,15 @@ struct klp_state_callbacks { * @id:system state identifier (non-zero) * @version: version of the change * @callbacks: optional callbacks used when introducing or removing the state + * @is_shadow: the state handles lifetime of a shadow variable + * with the same @id * @data: custom data */ struct klp_state { unsigned long id; unsigned int version; struct klp_state_callbacks callbacks; + bool is_shadow; void *data; }; @@ -240,11 +250,6 @@ static inline bool klp_have_reliable_stack(void) IS_ENABLED(CONFIG_HAVE_RELIABLE_STACKTRACE); } -typedef int (*klp_shadow_ctor_t)(void *obj, -void *shadow_data, -void *ctor_data); -typedef void (*klp_shadow_dtor_t)(void *obj, void *shadow_data); - void *klp_shadow_get(void *obj, unsigned long id); void *klp_shadow_alloc(void *obj, unsigned long id, size_t size, gfp_t gfp_flags, diff --git a/kernel/livepatch/state.c b/kernel/livepatch/state.c index 6693d808106b..4ec65afe3a43 100644 --- a/kernel/livepatch/state.c +++ b/kernel/livepatch/state.c @@ -198,11 +198,17 @@ void klp_release_states(struct klp_patch *patch) if (is_state_in_other_patches(patch, state)) continue; - if (!state->callbacks.release) - continue; - - if (state->callbacks.setup_succeeded) + if (state->callbacks.release && state->callbacks.setup_succeeded) state->callbacks.rel
[POC 3/7] livepatch: Use per-state callbacks in state API tests
Recent changes in the livepatch core have allowed to connect states, shadow variables, and callbacks. Use these new features in the state tests. Use the shadow variable API to store the original loglevel. It is better suited for this purpose than directly accessing the .data pointer in state klp_state. Another big advantage is that the shadow variable is preserved when the current patch is replaced by a new version. As a result, there is not need to copy the pointer. Finally, the lifetime of the shadow variable is connected with the lifetime of the state. It is freed automatically when it is not longer supported. This results into the following changes in the code: + Rename CONSOLE_LOGLEVEL_STATE -> CONSOLE_LOGLEVEL_FIX_ID because it will be used also the for shadow variable + Remove the extra code for module coming and going states because the new callback are per-state. + Remove callbacks needed to transfer the pointer between states. + Keep the versioning of the state to prevent downgrade. The problem is artificial because no callbacks are needed to transfer or free the shadow variable anymore. Signed-off-by: Petr Mladek --- lib/livepatch/test_klp_state.c| 113 +-- lib/livepatch/test_klp_state2.c | 190 +- .../testing/selftests/livepatch/test-state.sh | 44 ++-- 3 files changed, 76 insertions(+), 271 deletions(-) diff --git a/lib/livepatch/test_klp_state.c b/lib/livepatch/test_klp_state.c index 57a4253acb01..b3d1ee48dfcc 100644 --- a/lib/livepatch/test_klp_state.c +++ b/lib/livepatch/test_klp_state.c @@ -9,108 +9,109 @@ #include #include -#define CONSOLE_LOGLEVEL_STATE 1 -/* Version 1 does not support migration. */ -#define CONSOLE_LOGLEVEL_STATE_VERSION 1 +#define CONSOLE_LOGLEVEL_FIX_ID 1 -static const char *const module_state[] = { - [MODULE_STATE_LIVE] = "[MODULE_STATE_LIVE] Normal state", - [MODULE_STATE_COMING] = "[MODULE_STATE_COMING] Full formed, running module_init", - [MODULE_STATE_GOING]= "[MODULE_STATE_GOING] Going away", - [MODULE_STATE_UNFORMED] = "[MODULE_STATE_UNFORMED] Still setting it up", -}; - -static void callback_info(const char *callback, struct klp_object *obj) -{ - if (obj->mod) - pr_info("%s: %s -> %s\n", callback, obj->mod->name, - module_state[obj->mod->state]); - else - pr_info("%s: vmlinux\n", callback); -} +/* + * Version of the state which defines compatibility of livepaches. + * The value is artificial. It set just for testing the compatibility + * checks. In reality, all versions are compatible because all + * the callbacks do nothing and the shadow variable clean up + * is done by the core. + */ +#ifndef CONSOLE_LOGLEVEL_FIX_VERSION +#define CONSOLE_LOGLEVEL_FIX_VERSION 1 +#endif static struct klp_patch patch; static int allocate_loglevel_state(void) { - struct klp_state *loglevel_state; + int *shadow_console_loglevel; - loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE); - if (!loglevel_state) - return -EINVAL; + /* Make sure that the shadow variable does not exist yet. */ + shadow_console_loglevel = + klp_shadow_alloc(&console_loglevel, CONSOLE_LOGLEVEL_FIX_ID, +sizeof(*shadow_console_loglevel), GFP_KERNEL, +NULL, NULL); - loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL); - if (!loglevel_state->data) + if (!shadow_console_loglevel) { + pr_err("%s: failed to allocated shadow variable for storing original loglevel\n", + __func__); return -ENOMEM; + } pr_info("%s: allocating space to store console_loglevel\n", __func__); + return 0; } static void fix_console_loglevel(void) { - struct klp_state *loglevel_state; + int *shadow_console_loglevel; - loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE); - if (!loglevel_state) + shadow_console_loglevel = + (int *)klp_shadow_get(&console_loglevel, CONSOLE_LOGLEVEL_FIX_ID); + if (!shadow_console_loglevel) return; pr_info("%s: fixing console_loglevel\n", __func__); - *(int *)loglevel_state->data = console_loglevel; + *shadow_console_loglevel = console_loglevel; console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH; } static void restore_console_loglevel(void) { - struct klp_state *loglevel_state; + int *shadow_console_loglevel; - loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE); - if (!loglevel_state) + shadow_console_loglevel = + (int *)klp_shadow_get(&console_loglevel, CONSOLE_LOGLEVEL_FIX_ID); + if (!shadow_console_loglevel) return; pr_info("%s: restoring
[POC 4/7] livepatch: Do not use callbacks when testing sysfs interface
The per-object callbacks have been obsoleted by per-state callbacks. As a result, the callbacks test modules have been obsoleted by updated klp state tests. The callbacks test modules are re-used in the sysfs selftests. It would be possible to replace them by klp state test modules but the newly generated logs are hard to review because there is a lot of noise caused by the callbacks. Instead, introduce a simple test module in a "Hello World" style and corresponding livepatch. The expected log can be reviewed easily. The test module might be later extended to provide more functionality which might be used in more tests. It would allow to create tests focusing on some particular feature with an easier output. Signed-off-by: Petr Mladek --- lib/livepatch/Makefile| 2 + lib/livepatch/test_klp_speaker.c | 34 + lib/livepatch/test_klp_speaker_livepatch.c| 50 +++ .../testing/selftests/livepatch/test-sysfs.sh | 48 -- 4 files changed, 106 insertions(+), 28 deletions(-) create mode 100644 lib/livepatch/test_klp_speaker.c create mode 100644 lib/livepatch/test_klp_speaker_livepatch.c diff --git a/lib/livepatch/Makefile b/lib/livepatch/Makefile index dcc912b3478f..a8a5f6597633 100644 --- a/lib/livepatch/Makefile +++ b/lib/livepatch/Makefile @@ -9,6 +9,8 @@ obj-$(CONFIG_TEST_LIVEPATCH) += test_klp_atomic_replace.o \ test_klp_callbacks_mod.o \ test_klp_livepatch.o \ test_klp_shadow_vars.o \ + test_klp_speaker.o \ + test_klp_speaker_livepatch.o \ test_klp_state.o \ test_klp_state2.o \ test_klp_state3.o diff --git a/lib/livepatch/test_klp_speaker.c b/lib/livepatch/test_klp_speaker.c new file mode 100644 index ..d2d31072639a --- /dev/null +++ b/lib/livepatch/test_klp_speaker.c @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2023 SUSE + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#ifndef _VER_NAME +#define _VER_NAME(name) name +#endif + +#include +#include + +noinline +void speaker_welcome(void) +{ + pr_info("%s: Hello, World!\n", __func__); +} + +static int test_klp_speaker_init(void) +{ + pr_info("%s\n", __func__); + + return 0; +} + +static void test_klp_speaker_exit(void) +{ + pr_info("%s\n", __func__); +} + +module_init(test_klp_speaker_init); +module_exit(test_klp_speaker_exit); +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("Livepatch test: test functions"); diff --git a/lib/livepatch/test_klp_speaker_livepatch.c b/lib/livepatch/test_klp_speaker_livepatch.c new file mode 100644 index ..0317a4937b78 --- /dev/null +++ b/lib/livepatch/test_klp_speaker_livepatch.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0 + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include + + +void livepatch_speaker_welcome(void) +{ + pr_info("%s: Ladies and gentleman, ...\n", __func__); +} + + +static struct klp_func test_klp_speaker_funcs[] = { + { + .old_name = "speaker_welcome", + .new_func = livepatch_speaker_welcome, + }, + { } +}; + +static struct klp_object objs[] = { + { + .name = "test_klp_speaker", + .funcs = test_klp_speaker_funcs, + }, + { } +}; + +static struct klp_patch patch = { + .mod = THIS_MODULE, + .objs = objs, +}; + +static int test_klp_speaker_livepatch_init(void) +{ + return klp_enable_patch(&patch); +} + +static void test_klp_speaker_livepatch_exit(void) +{ +} + +module_init(test_klp_speaker_livepatch_init); +module_exit(test_klp_speaker_livepatch_exit); +MODULE_LICENSE("GPL"); +MODULE_INFO(livepatch, "Y"); diff --git a/tools/testing/selftests/livepatch/test-sysfs.sh b/tools/testing/selftests/livepatch/test-sysfs.sh index 7f76f280189a..424b6af32c99 100755 --- a/tools/testing/selftests/livepatch/test-sysfs.sh +++ b/tools/testing/selftests/livepatch/test-sysfs.sh @@ -42,8 +42,8 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete start_test "sysfs test object/patched" -MOD_LIVEPATCH=test_klp_callbacks_demo -MOD_TARGET=test_klp_callbacks_mod +MOD_LIVEPATCH=test_klp_speaker_livepatch +MOD_TARGET=test_klp_speaker load_lp $MOD_LIVEPATCH # check the "patch" file changes as target module loads/unloads @@ -56,31 +56,23 @@ check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0" disable_lp $MOD_LIVEPATCH unload_lp $MOD_LIVEPATCH -check_result "% modprobe test_klp_callbacks_demo -livepatch: enabling patch 'test_klp_callbacks_demo' -livepatch: 'test_klp_callbacks_demo': initializing patching transition -test_klp_callbacks_demo: pre_patch_callback: vmlinux -livepatch: 'test_klp_callbacks_demo': starting patching transition -livepatch: 'test
[POC 5/7] livepatch: Convert klp module callbacks tests into livepatch module tests
The original livepatch callbacks has been obsoleted by the livepatch state callbacks. The important difference is that the original callbacks were associated with the livepatched object. The state callbacks are associated with the livepatch states which are associated with the livepatch. As a result, some per-object callbacks functionality could not be preserved with the new per-state callbacks. Namely, it is not possible to change a state when a livepatched module is loaded or unloaded. This usecase it considered rare and not worth the complexity. Instead, the callback self-tests are transformed into testing the new state callbacks. Some tests do not use the callbacks any longer. But they still test some basic scenarios which is not tested by other tests. Many features are added to the test_klp_speaker module: - The parameter "welcome" allows to call the livepatched function speaker_welcome. - The parameter "waiting_welcome" allows to a trigger a workqueue work which might busy wait in a livepatched function and block the transition. It requires storing pointer to speaker_wait_and_welcome() so that the original one could be called from the livepatch. Alternative solution would be to livepatch speaker_wait_and_welcome() but it would require passing pointer to the completion structure from the livepatched module. Another tricky part is the initialization of the work structure. It must be initialized in the module init() callback so that it can be flushed in the exit() callback even when it was not used. But it must be initialized also in waiting_welcome_set() because the module parameter callbacks are proceed before calling the module init() callback. - The same sources are used to build two target modules. The livepatched functions have versioned names. It makes it easier to distinguish messages printed by this functions and the livepatched variants. It is solved by using macro for definition of the functions. Many features are added also into the livepatch module: - The parameter "add_applause" allows to add "[APPLAUSE " string into the message printed by speaker_welcome(). The string is stored in a shadow variable which is: - associated with a state. - allocated and initialized to "[] " in setup() state callback. - set to "[APPLAUSE ]" in enable() state callback. - set back to "[] " in disable() state callback. - freed in release() state callback. Allows testing state callbacks. It even allows to distinguish the state after setup() and enable() callbacks. - The parameter "setup_ret" allow to force a negative return value to the callback allocation the shadow variable. - The parameter "noreplace" allows to disable patch.replace parameter and install the livepatch in parallel with others. - The same sources are used for building two livepatches. It allows writing tests with more livepatches. The livepatched functions are versioned. Both livepatches can livepatch both speaker test modules. But they use the same shadow variable for storing the "add_applause" feature. It allows testing passing the states and shadow variables with atomic replace. But obviously, "add_applause" parameter could no be used for both livepatches when they are loaded in parallel using the "noreplace" parameter. Finally, remove the obsolete and unused "callback" test modules and livepatches. TODO: - Debug why "waiting_welcome" does not block the transition. The livepatched function is not on the stack from unknown reasons. See the comments in test-module.h for more details. - Better organize the tests. test-module.sh combines tests of various aspects which might better be suited somewhere else. As a first step, test-callbacks.sh has been renamed to test-modules.sh. But there still might be a better name. - Split this huge patch. Add the various features and tests in more steps. Signed-off-by: Petr Mladek --- lib/livepatch/Makefile| 6 +- lib/livepatch/test_klp_callbacks_busy.c | 70 --- lib/livepatch/test_klp_callbacks_demo.c | 121 lib/livepatch/test_klp_callbacks_demo2.c | 93 --- lib/livepatch/test_klp_callbacks_mod.c| 24 - lib/livepatch/test_klp_speaker.c | 153 - lib/livepatch/test_klp_speaker_livepatch.c| 209 ++- tools/testing/selftests/livepatch/Makefile| 2 +- .../testing/selftests/livepatch/functions.sh | 29 + .../selftests/livepatch/test-callbacks.sh | 553 -- .../selftests/livepatch/test-modules.sh | 539 + 11 files changed, 929 insertions(+), 870 deletions(-) delete mode 100644 lib/livepatch/test_klp_callbacks_busy.c delete mode 100644 lib/livepatch/test_klp_callbacks_demo.c delete mode 100644 lib/livepatch/test
[POC 6/7] livepatch: Remove the obsolete per-object callbacks
All selftests have been migrated to the new per-state callbacks. And the obsoleted per-object callbacks can be safely removed. FIXME: This patch is removing the sample module and documentation without any replacement. They obviously have to be converted for the state-callbacks. It has been postponed until the approach has been approved in the POC stage. Signed-off-by: Petr Mladek --- Documentation/livepatch/callbacks.rst | 133 Documentation/livepatch/index.rst | 1 - include/linux/livepatch.h | 25 --- kernel/livepatch/core.c | 29 --- kernel/livepatch/core.h | 33 --- kernel/livepatch/transition.c | 9 - samples/livepatch/Makefile| 3 - .../livepatch/livepatch-callbacks-busymod.c | 60 -- samples/livepatch/livepatch-callbacks-demo.c | 196 -- samples/livepatch/livepatch-callbacks-mod.c | 41 10 files changed, 530 deletions(-) delete mode 100644 Documentation/livepatch/callbacks.rst delete mode 100644 samples/livepatch/livepatch-callbacks-busymod.c delete mode 100644 samples/livepatch/livepatch-callbacks-demo.c delete mode 100644 samples/livepatch/livepatch-callbacks-mod.c diff --git a/Documentation/livepatch/callbacks.rst b/Documentation/livepatch/callbacks.rst deleted file mode 100644 index 470944aa8658.. --- a/Documentation/livepatch/callbacks.rst +++ /dev/null @@ -1,133 +0,0 @@ -== -(Un)patching Callbacks -== - -Livepatch (un)patch-callbacks provide a mechanism for livepatch modules -to execute callback functions when a kernel object is (un)patched. They -can be considered a **power feature** that **extends livepatching abilities** -to include: - - - Safe updates to global data - - - "Patches" to init and probe functions - - - Patching otherwise unpatchable code (i.e. assembly) - -In most cases, (un)patch callbacks will need to be used in conjunction -with memory barriers and kernel synchronization primitives, like -mutexes/spinlocks, or even stop_machine(), to avoid concurrency issues. - -1. Motivation -= - -Callbacks differ from existing kernel facilities: - - - Module init/exit code doesn't run when disabling and re-enabling a -patch. - - - A module notifier can't stop a to-be-patched module from loading. - -Callbacks are part of the klp_object structure and their implementation -is specific to that klp_object. Other livepatch objects may or may not -be patched, irrespective of the target klp_object's current state. - -2. Callback types -= - -Callbacks can be registered for the following livepatch actions: - - * Pre-patch - - before a klp_object is patched - - * Post-patch - - after a klp_object has been patched and is active - across all tasks - - * Pre-unpatch - - before a klp_object is unpatched (ie, patched code is - active), used to clean up post-patch callback - resources - - * Post-unpatch - - after a klp_object has been patched, all code has - been restored and no tasks are running patched code, - used to cleanup pre-patch callback resources - -3. How it works -=== - -Each callback is optional, omitting one does not preclude specifying any -other. However, the livepatching core executes the handlers in -symmetry: pre-patch callbacks have a post-unpatch counterpart and -post-patch callbacks have a pre-unpatch counterpart. An unpatch -callback will only be executed if its corresponding patch callback was -executed. Typical use cases pair a patch handler that acquires and -configures resources with an unpatch handler tears down and releases -those same resources. - -A callback is only executed if its host klp_object is loaded. For -in-kernel vmlinux targets, this means that callbacks will always execute -when a livepatch is enabled/disabled. For patch target kernel modules, -callbacks will only execute if the target module is loaded. When a -module target is (un)loaded, its callbacks will execute only if the -livepatch module is enabled. - -The pre-patch callback, if specified, is expected to return a status -code (0 for success, -ERRNO on error). An error status code indicates -to the livepatching core that patching of the current klp_object is not -safe and to stop the current patching request. (When no pre-patch -callback is provided, the transition is assumed to be safe.) If a -pre-patch callback returns failure, the kernel's module loader will: - - - Refuse to load a livepatch, if the livepatch is loaded after -targeted code. - -or: - - - Refuse to load a module, if the livepatch was already successfully -loaded. - -No post-patch, pre-unpatch, or post-unpatch callbacks will be executed -for a given klp_object if the object fail
[POC 7/7] livepatching: Remove per-state version
The livepatch state API was added to help with maintaining: + changes done by livepatch callbasks + lifetime of shadow variables The original API was hard to use. Both objectives are better handled by the new per-state callbacks. They are called when the state is introduced or removed. There is also support for automatically freeing obsolete shadow variables. The new callbacks changed the view of compatibility. The livepatch can be replaced to any older one as long the current livepatch is able to disable the obsolete state. As a result, the new patch does not need to support the currently used states. The current patch will be able to disable them. The remaining question is what to do with the per-state version. It was supposed to allow doing more modifications on an existing state. The experience shows that it is not needed in practice. Well, it still might make sense to prevent downgrade when the state could not be disabled easily or when the author does not want to deal with it. Replace the per-state version with per-state block_disable flag. It allows to handle several scenarios: + prevent disabling the livepatch when it does not support a particular state disablement. + prevent replacing the livepatch when the state is not supported by the new patch and the current patch does not support a particular state disablement. + allow to replace the livepatch with a new one which would support the particular state disablement. Signed-off-by: Petr Mladek --- include/linux/livepatch.h | 7 +- kernel/livepatch/core.c | 17 ++- kernel/livepatch/state.c | 27 +++-- kernel/livepatch/state.h | 1 + lib/livepatch/test_klp_state.c| 101 +- lib/livepatch/test_klp_state2.c | 2 - lib/livepatch/test_klp_state3.c | 2 +- .../testing/selftests/livepatch/functions.sh | 17 +++ .../testing/selftests/livepatch/test-state.sh | 40 --- 9 files changed, 158 insertions(+), 56 deletions(-) diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index 3807c7bb0281..c7c2c013b380 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -140,16 +140,16 @@ struct klp_state_callbacks { /** * struct klp_state - state of the system modified by the livepatch * @id:system state identifier (non-zero) - * @version: version of the change * @callbacks: optional callbacks used when introducing or removing the state + * @block_disable: the state disablement is not supported * @is_shadow: the state handles lifetime of a shadow variable * with the same @id * @data: custom data */ struct klp_state { unsigned long id; - unsigned int version; struct klp_state_callbacks callbacks; + bool block_disable; bool is_shadow; void *data; }; @@ -159,7 +159,9 @@ struct klp_state { * @mod: reference to the live patch module * @objs: object entries for kernel objects to be patched * @states:system states that can get modified + * version:livepatch version (optional) * @replace: replace all actively used patches + * * @list: list node for global list of actively used patches * @kobj: kobject for sysfs resources * @obj_list: dynamic list of the object entries @@ -173,6 +175,7 @@ struct klp_patch { struct module *mod; struct klp_object *objs; struct klp_state *states; + unsigned int version; bool replace; /* internal */ diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index d982365777f1..81fbe0778742 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -373,6 +373,13 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr, goto out; } + if (patch->enabled && klp_patch_disable_blocked(patch)) { + pr_err("The livepatch '%s' does not support disable\n", + patch->mod->name); + ret = -EINVAL; + goto out; + } + /* * Allow to reverse a pending transition in both ways. It might be * necessary to complete the transition without forcing and breaking @@ -1097,10 +1104,10 @@ int klp_enable_patch(struct klp_patch *patch) if (!klp_is_patch_compatible(patch)) { pr_err("Livepatch patch (%s) is not compatible with the already installed livepatches.\n", - patch->mod->name); + patch->mod->name); mutex_unlock(&klp_mutex); return -EINVAL; - } + } if (!try_module_get(patch->mod)) { mutex_unlock(&klp_mutex); @@ -,17 +1118,17 @@ int klp_enable_patch(struct klp_patch *patch) ret = klp_init_patch(patch); if (ret) - goto er
Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB
On Wed, Oct 18, 2023 at 03:12:41AM +0200, Jan Engelhardt wrote: > On Tuesday 2023-10-17 17:10, Michal Suchánek wrote: > > > >> In my system (Ubuntu), I see the directory paths > >> > >> /usr/aarch64-linux-gnu/lib/ > >> /usr/i686-linux-gnu/lib/ > >> /usr/x86_64-linux-gnu/lib/ > >> > >> If there were such a crazy distro that supports multiple kernel arches > >> within a single image, modules might be installed: > >> /usr/x86_64-linux-gnu/lib/module// > > > >For me it's /usr/lib/i386-linux-gnu/. > > > >Did they change the scheme at some point? > > It's a complicated mumble-jumble. Prior art exists as in: > > /opt/vendorThing/bin/... > /usr/X11R6/lib/libXi.so.6 [host binary] > /usr/x86_64-w64-mingw32/bin/as [host binary] > /usr/x86_64-w64-mingw32/sys-root/mingw/bin/as.exe [foreign binary] > /usr/platform/SUNW,Ultra-2/lib/libprtdiag_psr.so.1 [looks foreign] > > The use of suffix-based naming must have been established sometime > near the end of the 90s or the start of 2000s as the first biarch > Linux distros emerged. Probably in gcc or glibc sources one will find > the root of where the use of suffix identifiers like /usr/lib64 > started. Leaves the question open "why". That's pretty clear: to be able to install libraries for multiple architectures at the same time. Thanks Michal
[PATCH v6 02/11] clk: qcom: ipq5332: remove q6 bring up clocks
In multipd model Q6 firmware takes care of bringup clocks, so remove them from gcc driver. Signed-off-by: Manikanta Mylavarapu --- Changes in v6: - Rebased on linux-next. Changes in v5: - Rebased on linux-next. Changes in v4: - In V3 series this patch is [05/11]. Here it's moved to [02/11] because to compile dt-bindings patches. drivers/clk/qcom/gcc-ipq5332.c | 380 - 1 file changed, 380 deletions(-) diff --git a/drivers/clk/qcom/gcc-ipq5332.c b/drivers/clk/qcom/gcc-ipq5332.c index f98591148a97..dbaf7aa60520 100644 --- a/drivers/clk/qcom/gcc-ipq5332.c +++ b/drivers/clk/qcom/gcc-ipq5332.c @@ -2194,150 +2194,6 @@ static struct clk_branch gcc_prng_ahb_clk = { }, }; -static struct clk_branch gcc_q6_ahb_clk = { - .halt_reg = 0x25014, - .halt_check = BRANCH_HALT_VOTED, - .clkr = { - .enable_reg = 0x25014, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_q6_ahb_clk", - .parent_hws = (const struct clk_hw*[]) { - &gcc_wcss_ahb_clk_src.clkr.hw, - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - -static struct clk_branch gcc_q6_ahb_s_clk = { - .halt_reg = 0x25018, - .halt_check = BRANCH_HALT_VOTED, - .clkr = { - .enable_reg = 0x25018, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_q6_ahb_s_clk", - .parent_hws = (const struct clk_hw*[]) { - &gcc_wcss_ahb_clk_src.clkr.hw, - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - -static struct clk_branch gcc_q6_axim_clk = { - .halt_reg = 0x2500c, - .halt_check = BRANCH_HALT_VOTED, - .clkr = { - .enable_reg = 0x2500c, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_q6_axim_clk", - .parent_hws = (const struct clk_hw*[]) { - &gcc_q6_axim_clk_src.clkr.hw, - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - -static struct clk_branch gcc_q6_axis_clk = { - .halt_reg = 0x25010, - .halt_check = BRANCH_HALT_VOTED, - .clkr = { - .enable_reg = 0x25010, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_q6_axis_clk", - .parent_hws = (const struct clk_hw*[]) { - &gcc_system_noc_bfdcd_clk_src.clkr.hw, - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - -static struct clk_branch gcc_q6_tsctr_1to2_clk = { - .halt_reg = 0x25020, - .halt_check = BRANCH_HALT_VOTED, - .clkr = { - .enable_reg = 0x25020, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_q6_tsctr_1to2_clk", - .parent_hws = (const struct clk_hw*[]) { - &gcc_qdss_tsctr_div2_clk_src.hw, - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - -static struct clk_branch gcc_q6ss_atbm_clk = { - .halt_reg = 0x2501c, - .halt_check = BRANCH_HALT_VOTED, - .clkr = { - .enable_reg = 0x2501c, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_q6ss_atbm_clk", - .parent_hws = (const struct clk_hw*[]) { - &gcc_qdss_at_clk_src.clkr.hw, - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - -static struct clk_branch gcc_q6ss_pclkdbg_clk = { - .halt_reg = 0x25024, - .halt_check = BRANCH_HALT_VOTED, - .clkr = { - .enable_reg = 0x25024, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data
[PATCH v6 06/11] firmware: qcom_scm: ipq5332: add support to pass metadata size
IPQ5332 security software running under trustzone requires metadata size. With V2 cmd, pass metadata size as well. Signed-off-by: Manikanta Mylavarapu --- Changes in v6: - Rebased on linux-next Changes in v5: - Rebased on linux-next Changes in v4: - Rebased on linux-next drivers/firmware/qcom/qcom_scm.c | 8 drivers/firmware/qcom/qcom_scm.h | 1 + 2 files changed, 9 insertions(+) diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index 520de9b5633a..865f07c020a7 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -573,6 +573,14 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size, desc.args[1] = mdata_phys; + if (__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL, +QCOM_SCM_PAS_INIT_IMAGE_V2)) { + desc.cmd = QCOM_SCM_PAS_INIT_IMAGE_V2; + desc.arginfo = + QCOM_SCM_ARGS(3, QCOM_SCM_VAL, QCOM_SCM_RW, QCOM_SCM_VAL); + desc.args[2] = size; + } + ret = qcom_scm_call(__scm->dev, &desc, &res); qcom_scm_bw_disable(); diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h index 4532907e8489..4342d795940b 100644 --- a/drivers/firmware/qcom/qcom_scm.h +++ b/drivers/firmware/qcom/qcom_scm.h @@ -93,6 +93,7 @@ int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc, #define QCOM_SCM_SVC_PIL 0x02 #define QCOM_SCM_PIL_PAS_INIT_IMAGE0x01 +#define QCOM_SCM_PAS_INIT_IMAGE_V2 0x1a #define QCOM_SCM_PIL_PAS_MEM_SETUP 0x02 #define QCOM_SCM_PIL_PAS_AUTH_AND_RESET0x05 #define QCOM_SCM_PIL_PAS_SHUTDOWN 0x06 -- 2.34.1
[PATCH v6 09/11] remoteproc: qcom: Add Hexagon based multipd rproc driver
It adds support to bring up remoteproc's on multipd model. Pd means protection domain. It's similar to process in Linux. Here QDSP6 processor runs each wifi radio functionality on a separate process. One process can't access other process resources, so this is termed as PD i.e protection domain. Here we have two pd's called root and user pd. We can correlate Root pd as root and user pd as user in linux. Root pd has more privileges than user pd. Root will provide services to user pd. >From remoteproc driver perspective, root pd corresponds to QDSP6 processor bring up and user pd corresponds to Wifi radio (WCSS) bring up. Here WCSS(user) PD is dependent on Q6(root) PD, so first q6 pd should be up before wcss pd. After wcss pd goes down, q6 pd should be turned off. APPS QDSP6 --- - | | Crash notification | | -- | |<-|--|---|User| | | | ||->|PD1 | | | | --- || -- | | | | | || |Root | Start/stop Q6 | | R | || |PD |<-|->| | || |rproc| Crash notification | | O | || | | | | | || |User |Start/stop UserPD1| | O | || |PD1 |--|->| |-|| |rproc| | | T | || | | | | | || |User |Start/stop UserPD2| | P | || |PD2 |--|->| |-|| |rproc| | | D | || | | | --- || -- | | Crash notification | ||->|User| | |<-|--|---|PD2 | --- | | -- IPQ5332, IPQ9574 supports multipd remoteproc driver. Signed-off-by: Manikanta Mylavarapu --- Changes in v6: - Rebased on linux-next Changes in v5: - Fixed all comments and rebased on linux-next. - Removed EPROBE_DEFER dance. Changes in v4: - Fixed all comments and rebased on linux-next. - All userpd's rproc handles stored in linked list. Get userpd rproc handles whenever required from list instead of traversing with 'for_each_available_child_of_node'. - Removed data members from compatible specific data structure. Because these are required for user pd's. Since we removed user pd compatible, then no need of this data members. - In probe itself, traverse for each userpd and call 'q6_register_userpd()'. In case of failure, call 'q6_release_resources()' to clear already allocated user pd rproc's. Changes in v3: - Fixed all comments and rebased on linux-next. - Removed WCSS userpd compatibles. - Removed AHB/PCIE terms from driver. - Removed logic to get ASID from DT node, instead computed from UserPD spawn bit no. - IPQ5018 support is dropped because it's base port patches not yet merged so added IPQ5332 support. - Added msa lock, unlock scm calls for WCSS user pd up/down. - Added bootinfo support to share userpd load-address & size to QDSP6 root pd. Changes in v2: - Common functionalities moved to seperate patches - qcom_get_pd_asid() moved to mpd driver - Last DMA block alone memset to zero - Added diagram to show how userpd data is organized and sent to trustzone - Rewritten commit message since most of the content available in cover page - Removed 'remote_id' becuase it's not required for bring up. drivers/remoteproc/Kconfig | 19 + drivers/remoteproc/Makefile| 1 + drivers/remoteproc/qcom_q6v5_mpd.c | 802 + 3 files changed, 822 insertions(+) create mode 100644 drivers/remoteproc/qcom_q6v5_mpd.c diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index 48845dc8fa85..f5592e91c1a2 100644 --- a/drivers/remoteproc/Kconfig +++ b/drivers/remoteproc/Kconfig @@ -234,6 +234,25 @@ config QCOM_Q6V5_PAS CDSP (Compute DSP), MPSS (Modem Peripheral SubSystem), and SLPI (Sensor Low Power Island). +config QCOM_Q6V5_MPD + tristate "Qualcomm Hexagon based MPD model Peripheral Image Loader" + depends on OF && ARCH_QCOM + depends on QCOM_SMEM + depends on RPMSG_QCOM_SMD || RPMSG_QCOM_SMD=n + depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n + depends on QCOM_SYSMON || QCOM_SYSMON=n + depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n + depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n + select QCOM_MDT_LOADER + select QCOM_PIL_INFO + select QCOM_Q6V5_COMMON + select QCOM_RPROC_COMMON + select Q
Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB
On Friday 2023-11-10 18:44, Michal Suchánek wrote: >> It's a complicated mumble-jumble. Prior art exists as in: >> >> /opt/vendorThing/bin/... >> /usr/X11R6/lib/libXi.so.6 [host binary] >> /usr/x86_64-w64-mingw32/bin/as [host binary] >> /usr/x86_64-w64-mingw32/sys-root/mingw/bin/as.exe [foreign binary] >> /usr/platform/SUNW,Ultra-2/lib/libprtdiag_psr.so.1 [looks foreign] >> >> The use of suffix-based naming must have been established sometime >> near the end of the 90s or the start of 2000s as the first biarch >> Linux distros emerged. Probably in gcc or glibc sources one will find >> the root of where the use of suffix identifiers like /usr/lib64 >> started. Leaves the question open "why". > >That's pretty clear: to be able to install libraries for multiple >architectures at the same time. Well, what I tried to express or imply was something like: “ we could (should?) have used /usr//lib rather than /usr/lib all along, because at some point, there *will* be someone who wants to provide not only arch-different libraries, but *also* arch-different binaries (for whatever reason).
[PATCH v6 07/11] firmware: qcom_scm: ipq5332: add msa lock/unlock support
IPQ5332 user pd remoteproc firmwares need to be locked with MSA(modem secure access) features. This patch add support to lock/unlock MSA features. Signed-off-by: Manikanta Mylavarapu --- Changes in v6: - Removed extern keyword from msa lock/unlock function prototype. Changes in v5: - Moved to EXPORT_SYMBOL_GPL() because scm driver moved to using EXPORT_SYMBOL_GPL() now. Changes in v4: - Rebased on linux-next drivers/firmware/qcom/qcom_scm.c | 78 ++ drivers/firmware/qcom/qcom_scm.h | 2 + include/linux/firmware/qcom/qcom_scm.h | 2 + 3 files changed, 82 insertions(+) diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index 865f07c020a7..965164e3506b 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -754,6 +754,84 @@ bool qcom_scm_pas_supported(u32 peripheral) } EXPORT_SYMBOL_GPL(qcom_scm_pas_supported); +/** + * qcom_scm_msa_lock() - Lock given peripheral firmware region as MSA + * + * @peripheral:peripheral id + * + * Return 0 on success. + */ +int qcom_scm_msa_lock(u32 peripheral) +{ + int ret; + struct qcom_scm_desc desc = { + .svc = QCOM_SCM_SVC_PIL, + .cmd = QCOM_SCM_MSA_LOCK, + .arginfo = QCOM_SCM_ARGS(1), + .args[0] = peripheral, + .owner = ARM_SMCCC_OWNER_SIP, + }; + struct qcom_scm_res res; + + if (!__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL, + QCOM_SCM_MSA_LOCK)) + return 0; + + ret = qcom_scm_clk_enable(); + if (ret) + return ret; + + ret = qcom_scm_bw_enable(); + if (ret) + return ret; + + ret = qcom_scm_call(__scm->dev, &desc, &res); + qcom_scm_bw_disable(); + qcom_scm_clk_disable(); + + return ret ? : res.result[0]; +} +EXPORT_SYMBOL_GPL(qcom_scm_msa_lock); + +/** + * qcom_scm_msa_unlock() - Unlock given peripheral MSA firmware region + * + * @peripheral:peripheral id + * + * Return 0 on success. + */ +int qcom_scm_msa_unlock(u32 peripheral) +{ + int ret; + struct qcom_scm_desc desc = { + .svc = QCOM_SCM_SVC_PIL, + .cmd = QCOM_SCM_MSA_UNLOCK, + .arginfo = QCOM_SCM_ARGS(1), + .args[0] = peripheral, + .owner = ARM_SMCCC_OWNER_SIP, + }; + struct qcom_scm_res res; + + if (!__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL, + QCOM_SCM_MSA_UNLOCK)) + return 0; + + ret = qcom_scm_clk_enable(); + if (ret) + return ret; + + ret = qcom_scm_bw_enable(); + if (ret) + return ret; + + ret = qcom_scm_call(__scm->dev, &desc, &res); + qcom_scm_bw_disable(); + qcom_scm_clk_disable(); + + return ret ? : res.result[0]; +} +EXPORT_SYMBOL_GPL(qcom_scm_msa_unlock); + static int __qcom_scm_pas_mss_reset(struct device *dev, bool reset) { struct qcom_scm_desc desc = { diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h index 4342d795940b..3b8fd08e8033 100644 --- a/drivers/firmware/qcom/qcom_scm.h +++ b/drivers/firmware/qcom/qcom_scm.h @@ -99,6 +99,8 @@ int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc, #define QCOM_SCM_PIL_PAS_SHUTDOWN 0x06 #define QCOM_SCM_PIL_PAS_IS_SUPPORTED 0x07 #define QCOM_SCM_PIL_PAS_MSS_RESET 0x0a +#define QCOM_SCM_MSA_LOCK 0x24 +#define QCOM_SCM_MSA_UNLOCK0x25 #define QCOM_SCM_SVC_IO0x05 #define QCOM_SCM_IO_READ 0x01 diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h index ccaf28846054..5526369d9568 100644 --- a/include/linux/firmware/qcom/qcom_scm.h +++ b/include/linux/firmware/qcom/qcom_scm.h @@ -79,6 +79,8 @@ int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size); int qcom_scm_pas_auth_and_reset(u32 peripheral); int qcom_scm_pas_shutdown(u32 peripheral); bool qcom_scm_pas_supported(u32 peripheral); +int qcom_scm_msa_lock(u32 peripheral); +int qcom_scm_msa_unlock(u32 peripheral); int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val); int qcom_scm_io_writel(phys_addr_t addr, unsigned int val); -- 2.34.1
[PATCH v6 03/11] clk: qcom: ipq9574: remove q6 bring up clocks
In multipd model Q6 firmware takes care of bringup clocks, so remove them from gcc driver. Signed-off-by: Manikanta Mylavarapu --- Changes in v6: - Rebased on linux-next. Changes in v5: - Rebased on linux-next. Changes in v4: - In V3 series this patch is [04/11]. Here it's moved to [03/11] because to compile dt-bindings patches. Changes in v3: - Rebased on linux-next. drivers/clk/qcom/gcc-ipq9574.c | 326 - 1 file changed, 326 deletions(-) diff --git a/drivers/clk/qcom/gcc-ipq9574.c b/drivers/clk/qcom/gcc-ipq9574.c index e8190108e1ae..e6d8ab5fbf29 100644 --- a/drivers/clk/qcom/gcc-ipq9574.c +++ b/drivers/clk/qcom/gcc-ipq9574.c @@ -2567,24 +2567,6 @@ static struct clk_rcg2 system_noc_bfdcd_clk_src = { }, }; -static struct clk_branch gcc_q6ss_boot_clk = { - .halt_reg = 0x25080, - .halt_check = BRANCH_HALT_SKIP, - .clkr = { - .enable_reg = 0x25080, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_q6ss_boot_clk", - .parent_hws = (const struct clk_hw *[]) { - &system_noc_bfdcd_clk_src.clkr.hw - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - static struct clk_branch gcc_nssnoc_snoc_clk = { .halt_reg = 0x17028, .clkr = { @@ -2655,91 +2637,6 @@ static struct clk_rcg2 wcss_ahb_clk_src = { }, }; -static struct clk_branch gcc_q6_ahb_clk = { - .halt_reg = 0x25014, - .clkr = { - .enable_reg = 0x25014, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_q6_ahb_clk", - .parent_hws = (const struct clk_hw *[]) { - &wcss_ahb_clk_src.clkr.hw - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - -static struct clk_branch gcc_q6_ahb_s_clk = { - .halt_reg = 0x25018, - .clkr = { - .enable_reg = 0x25018, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_q6_ahb_s_clk", - .parent_hws = (const struct clk_hw *[]) { - &wcss_ahb_clk_src.clkr.hw - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - -static struct clk_branch gcc_wcss_ecahb_clk = { - .halt_reg = 0x25058, - .clkr = { - .enable_reg = 0x25058, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_wcss_ecahb_clk", - .parent_hws = (const struct clk_hw *[]) { - &wcss_ahb_clk_src.clkr.hw - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - -static struct clk_branch gcc_wcss_acmt_clk = { - .halt_reg = 0x2505c, - .clkr = { - .enable_reg = 0x2505c, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_wcss_acmt_clk", - .parent_hws = (const struct clk_hw *[]) { - &wcss_ahb_clk_src.clkr.hw - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - -static struct clk_branch gcc_sys_noc_wcss_ahb_clk = { - .halt_reg = 0x2e030, - .clkr = { - .enable_reg = 0x2e030, - .enable_mask = BIT(0), - .hw.init = &(const struct clk_init_data) { - .name = "gcc_sys_noc_wcss_ahb_clk", - .parent_hws = (const struct clk_hw *[]) { - &wcss_ahb_clk_src.clkr.hw - }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, - .ops = &clk_branch2_ops, - }, - }, -}; - static const struct freq_tbl ftbl_wcss_axi_m_clk_src[] = { F(2400, P_XO, 1, 0, 0), F(1, P_GPLL0, 6, 0, 0), @@ -2760,23 +2657,6 @@ static struct clk_rcg2 wcss_axi_m_clk_src = { }, }; -static struct clk_branch gcc
[PATCH v6 08/11] remoteproc: qcom: q6v5: Add multipd interrupts support
In multipd model, root & user pd remoteproc's interrupts are different. User pd needs additional interrupts like spawn. Instead of going with qcom_q6v5_init(), we defined a new function to register userpd rproc interrupts in mpd driver. Since userpd rproc uses some of common interrupts like fatal, ready, static is removed from ISR handler and used in userpd interrupt registration. Signed-off-by: Manikanta Mylavarapu --- Changes in v6: - Rebased on linux-next Changes in v5: - Exported symbols to resolve errors reported here https://lore.kernel.org/oe-kbuild-all/202307301307.lgjsxmy8-...@intel.com/ Changes in v4: - Rebased on linux-next Changes in v3: - Rebased on linux-next drivers/remoteproc/qcom_q6v5.c | 41 +++--- drivers/remoteproc/qcom_q6v5.h | 11 + 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c index 4ee5e67a9f03..0e32f13c196d 100644 --- a/drivers/remoteproc/qcom_q6v5.c +++ b/drivers/remoteproc/qcom_q6v5.c @@ -112,7 +112,7 @@ static irqreturn_t q6v5_wdog_interrupt(int irq, void *data) return IRQ_HANDLED; } -static irqreturn_t q6v5_fatal_interrupt(int irq, void *data) +irqreturn_t q6v5_fatal_interrupt(int irq, void *data) { struct qcom_q6v5 *q6v5 = data; size_t len; @@ -132,8 +132,9 @@ static irqreturn_t q6v5_fatal_interrupt(int irq, void *data) return IRQ_HANDLED; } +EXPORT_SYMBOL_GPL(q6v5_fatal_interrupt); -static irqreturn_t q6v5_ready_interrupt(int irq, void *data) +irqreturn_t q6v5_ready_interrupt(int irq, void *data) { struct qcom_q6v5 *q6v5 = data; @@ -141,6 +142,7 @@ static irqreturn_t q6v5_ready_interrupt(int irq, void *data) return IRQ_HANDLED; } +EXPORT_SYMBOL_GPL(q6v5_ready_interrupt); /** * qcom_q6v5_wait_for_start() - wait for remote processor start signal @@ -177,7 +179,17 @@ static irqreturn_t q6v5_handover_interrupt(int irq, void *data) return IRQ_HANDLED; } -static irqreturn_t q6v5_stop_interrupt(int irq, void *data) +irqreturn_t q6v5_spawn_interrupt(int irq, void *data) +{ + struct qcom_q6v5 *q6v5 = data; + + complete(&q6v5->spawn_done); + + return IRQ_HANDLED; +} +EXPORT_SYMBOL_GPL(q6v5_spawn_interrupt); + +irqreturn_t q6v5_stop_interrupt(int irq, void *data) { struct qcom_q6v5 *q6v5 = data; @@ -185,6 +197,7 @@ static irqreturn_t q6v5_stop_interrupt(int irq, void *data) return IRQ_HANDLED; } +EXPORT_SYMBOL_GPL(q6v5_stop_interrupt); /** * qcom_q6v5_request_stop() - request the remote processor to stop @@ -214,6 +227,28 @@ int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5, struct qcom_sysmon *sysmon) } EXPORT_SYMBOL_GPL(qcom_q6v5_request_stop); +/** + * qcom_q6v5_request_spawn() - request the remote processor to spawn + * @q6v5: reference to qcom_q6v5 context + * + * Return: 0 on success, negative errno on failure + */ +int qcom_q6v5_request_spawn(struct qcom_q6v5 *q6v5) +{ + int ret; + + ret = qcom_smem_state_update_bits(q6v5->spawn_state, + BIT(q6v5->spawn_bit), BIT(q6v5->spawn_bit)); + + ret = wait_for_completion_timeout(&q6v5->spawn_done, 5 * HZ); + + qcom_smem_state_update_bits(q6v5->spawn_state, + BIT(q6v5->spawn_bit), 0); + + return ret == 0 ? -ETIMEDOUT : 0; +} +EXPORT_SYMBOL_GPL(qcom_q6v5_request_spawn); + /** * qcom_q6v5_panic() - panic handler to invoke a stop on the remote * @q6v5: reference to qcom_q6v5 context diff --git a/drivers/remoteproc/qcom_q6v5.h b/drivers/remoteproc/qcom_q6v5.h index 5a859c41896e..4e1bb1a68284 100644 --- a/drivers/remoteproc/qcom_q6v5.h +++ b/drivers/remoteproc/qcom_q6v5.h @@ -18,21 +18,27 @@ struct qcom_q6v5 { struct qcom_smem_state *state; struct qmp *qmp; + struct qcom_smem_state *shutdown_state; + struct qcom_smem_state *spawn_state; struct icc_path *path; unsigned stop_bit; + unsigned shutdown_bit; + unsigned spawn_bit; int wdog_irq; int fatal_irq; int ready_irq; int handover_irq; int stop_irq; + int spawn_irq; bool handover_issued; struct completion start_done; struct completion stop_done; + struct completion spawn_done; int crash_reason; @@ -50,7 +56,12 @@ void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5); int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5); int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5); int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5, struct qcom_sysmon *sysmon); +int qcom_q6v5_request_spawn(struct qcom_q6v5 *q6v5); int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout); unsigned long qcom_q6v5_panic(struct qcom_q6v5 *q6v5); +irqreturn_t q6v5_fatal_interrupt(int irq, void *data); +irqreturn_t q6v5_ready_interrupt(int irq, void *data); +irqreturn_t q6v5_spawn_i
[PATCH v6 00/11] Add multipd remoteproc support
APSS brings Q6 out of reset and then Q6 brings WCSS block (wifi radio's) out of reset. --- --> |WiFi 2G radio| |-- | --- | | APSS | ---> |QDSP6| -| - --- | | | | -- --> |WiFi 5G radio| -- Problem here is if any radio crashes, subsequently other radio also should crash because Q6 crashed. Let's say 2G radio crashed, Q6 should pass this info to APSS. Only Q6 processor interrupts registered with APSS. Obviously Q6 should crash and raise fatal interrupt to APSS. Due to this 5G radio also crashed. But no issue in 5G radio, because of 2G radio crash 5G radio also impacted. In multi pd model, this problem is resolved. Here WCSS functionality (WiFi radio's) moved out from Q6 root pd to a separate user pd. Due to this, radio's independently pass their status info to APPS with out crashing Q6. So other radio's won't be impacted. Pd means protection domain. It's similar to process in Linux. Here QDSP6 processor runs each wifi radio functionality on a separate process. One process can't access other process resources, so this is termed as PD i.e protection domain. APPS QDSP6 --- - | | Crash notification | | -- | |<-|--|---|WiFi| | | | ||->|2G radio| | | | --- || -- | | | | | || |Root | Start/stop Q6 | | R | || |PD |<-|->| | || |rproc| Crash notification | | O | || | | | | | || |User |Start/stop UserPD1(2G)| | O | || |PD1 |--|->| |-|| |rproc| | | T | || | | | | | || |User |Start/stop UserPD2(5G)| | P | || |PD2 |--|->| |-|| |rproc| | | D | || | | | --- || --- | | Crash notification | ||->|WiFi | | |<-|--|---|5G radio | --- | | --- According to linux terminology, here consider Q6 as root i.e it provide all services, WCSS (wifi radio's) as user i.e it uses services provided by root. Since Q6 root & WCSS user pd's able to communicate with APSS individually, multipd remoteproc driver registers each PD with rproc framework. Here clients (Wifi host drivers) intrested on WCSS PD rproc, so multipd driver start's root pd in the context of WCSS user pd rproc start. Similarly on down path, root pd will be stopped after wcss user pd stopped. Here WCSS(user) PD is dependent on Q6(root) PD, so first q6 pd should be up before wcss pd. After wcss pd goes down, q6 pd should be turned off. IPQ5332, IPQ9574 supports multipd remoteproc driver. [V6]: - Added user pd remoteproc nodes in RDP442 dts - Removed extern keyword from msa lock/unlock function prototype [V5]: - Fixed all comments and rebased on linux-next. - Exported symbols to resolve errors reported here https://lore.kernel.org/oe-kbuild-all/202307301307.lgjsxmy8-...@intel.com/ [V4]: - Fixed all comments and rebased on linux-next. - All userpd's rproc handles stored in linked list. - Removed data members from compatible specific data structure. - In probe itself, traverse for each userpd and call 'q6_register_userpd()'. [V3]: - Fixed all comments and rebased on linux-next. - IPQ5018 support is dropped because it's base port patches not yet merged. - IPQ5332 support is added with below patches. [03/11], [05/11], [06/11], [07/11], [10/11]. [V2]: - Fixed all comments and rebased on linux-next. - since clocks handled by QDSP6 firmware Added [04/13], [05/13], [06/13], [07/13] patches. Manikanta Mylavarapu (11): dt-bindings: remoteproc: qcom: Add support for multipd model clk: qcom: ipq5332: remove q6 bring up clocks clk: qcom: ipq9574: remove q6 bring up clocks dt-bindings: clock: qcom: gcc-ipq5332: remove q6 bring up clock macros dt-bindings: clock: qcom: gcc-ipq9574: remove q6 bring up clock macros firmware: qcom_scm: ipq5332: add support to pass metadata size firmware: qcom_scm: ipq5332: add msa lock/unlock support remoteproc: qcom: q6v5: Add multipd interrupts support remoteproc: qcom: Add Hexagon based multipd rproc driver arm64: dts: qc
[PATCH v6 05/11] dt-bindings: clock: qcom: gcc-ipq9574: remove q6 bring up clock macros
In multipd model Q6 firmware takes care of bringup clocks, so remove them. Signed-off-by: Manikanta Mylavarapu Reviewed-by: Krzysztof Kozlowski --- Changes in v6: - No changes Changes in v5: - No changes Changes in v4: - Pick up R-b tag Changes in v3: - Rebased on linux-next include/dt-bindings/clock/qcom,ipq9574-gcc.h | 18 -- 1 file changed, 18 deletions(-) diff --git a/include/dt-bindings/clock/qcom,ipq9574-gcc.h b/include/dt-bindings/clock/qcom,ipq9574-gcc.h index 08fd3a37acaa..9217b90f6847 100644 --- a/include/dt-bindings/clock/qcom,ipq9574-gcc.h +++ b/include/dt-bindings/clock/qcom,ipq9574-gcc.h @@ -132,16 +132,8 @@ #define GCC_NSSNOC_SNOC_1_CLK 123 #define GCC_QDSS_ETR_USB_CLK 124 #define WCSS_AHB_CLK_SRC 125 -#define GCC_Q6_AHB_CLK 126 -#define GCC_Q6_AHB_S_CLK 127 -#define GCC_WCSS_ECAHB_CLK 128 -#define GCC_WCSS_ACMT_CLK 129 -#define GCC_SYS_NOC_WCSS_AHB_CLK 130 #define WCSS_AXI_M_CLK_SRC 131 -#define GCC_ANOC_WCSS_AXI_M_CLK132 #define QDSS_AT_CLK_SRC133 -#define GCC_Q6SS_ATBM_CLK 134 -#define GCC_WCSS_DBG_IFC_ATB_CLK 135 #define GCC_NSSNOC_ATB_CLK 136 #define GCC_QDSS_AT_CLK137 #define GCC_SYS_NOC_AT_CLK 138 @@ -154,27 +146,18 @@ #define QDSS_TRACECLKIN_CLK_SRC145 #define GCC_QDSS_TRACECLKIN_CLK146 #define QDSS_TSCTR_CLK_SRC 147 -#define GCC_Q6_TSCTR_1TO2_CLK 148 -#define GCC_WCSS_DBG_IFC_NTS_CLK 149 #define GCC_QDSS_TSCTR_DIV2_CLK150 #define GCC_QDSS_TS_CLK151 #define GCC_QDSS_TSCTR_DIV4_CLK152 #define GCC_NSS_TS_CLK 153 #define GCC_QDSS_TSCTR_DIV8_CLK154 #define GCC_QDSS_TSCTR_DIV16_CLK 155 -#define GCC_Q6SS_PCLKDBG_CLK 156 -#define GCC_Q6SS_TRIG_CLK 157 -#define GCC_WCSS_DBG_IFC_APB_CLK 158 -#define GCC_WCSS_DBG_IFC_DAPBUS_CLK159 #define GCC_QDSS_DAP_CLK 160 #define GCC_QDSS_APB2JTAG_CLK 161 #define GCC_QDSS_TSCTR_DIV3_CLK162 #define QPIC_IO_MACRO_CLK_SRC 163 #define GCC_QPIC_IO_MACRO_CLK 164 #define Q6_AXI_CLK_SRC 165 -#define GCC_Q6_AXIM_CLK166 -#define GCC_WCSS_Q6_TBU_CLK167 -#define GCC_MEM_NOC_Q6_AXI_CLK 168 #define Q6_AXIM2_CLK_SRC 169 #define NSSNOC_MEMNOC_BFDCD_CLK_SRC170 #define GCC_NSSNOC_MEMNOC_CLK 171 @@ -199,7 +182,6 @@ #define GCC_UNIPHY2_SYS_CLK190 #define GCC_CMN_12GPLL_SYS_CLK 191 #define GCC_NSSNOC_XO_DCD_CLK 192 -#define GCC_Q6SS_BOOT_CLK 193 #define UNIPHY_SYS_CLK_SRC 194 #define NSS_TS_CLK_SRC 195 #define GCC_ANOC_PCIE0_1LANE_M_CLK 196 -- 2.34.1
[PATCH v6 11/11] arm64: dts: qcom: ipq9574: Add nodes to bring up multipd
Enable nodes required for multipd remoteproc bring up. Signed-off-by: Manikanta Mylavarapu --- Changes in v6: - Rebased on linux-next Changes in v5: - Rebased on linux-next Changes in v4: - Rebased on linux-next Changes in v3: - Fixed all comments and rebased on linux-next. - Removed WCSS userpd nodes from dtsi file, because it vary based on no of radio's connected. Changes in v2: - Corrected syntax like alignmnet and kept nodes in sorted order. - Added 'firmware-name' property. arch/arm64/boot/dts/qcom/ipq9574.dtsi | 59 +++ 1 file changed, 59 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi index 5f83ee42a719..96cc5bb2e0be 100644 --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi @@ -212,6 +212,11 @@ smem@4aa0 { hwlocks = <&tcsr_mutex 3>; no-map; }; + + q6_region: wcnss@4ab0 { + reg = <0x0 0x4ab0 0x0 0x2b0>; + no-map; + }; }; soc: soc@0 { @@ -741,6 +746,36 @@ frame@b128000 { status = "disabled"; }; }; + + q6v5_wcss: remoteproc@cd0 { + compatible = "qcom,ipq9574-q6-mpd"; + reg = <0x0cd0 0x4040>; + firmware-name = "ath11k/IPQ9574/hw1.0/q6_fw.mdt", + "ath11k/IPQ9574/hw1.0/m3_fw.mdt"; + interrupts-extended = <&intc GIC_SPI 325 IRQ_TYPE_EDGE_RISING>, + <&wcss_smp2p_in 0 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 1 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 2 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 3 IRQ_TYPE_NONE>; + interrupt-names = "wdog", + "fatal", + "ready", + "handover", + "stop-ack"; + + qcom,smem-states = <&wcss_smp2p_out 0>, + <&wcss_smp2p_out 1>; + qcom,smem-state-names = "shutdown", + "stop"; + memory-region = <&q6_region>; + + glink-edge { + interrupts = ; + label = "rtr"; + qcom,remote-pid = <1>; + mboxes = <&apcs_glb 8>; + }; + }; }; thermal-zones { @@ -998,4 +1033,28 @@ timer { , ; }; + + wcss: wcss-smp2p { + compatible = "qcom,smp2p"; + qcom,smem = <435>, <428>; + + interrupt-parent = <&intc>; + interrupts = ; + + mboxes = <&apcs_glb 9>; + + qcom,local-pid = <0>; + qcom,remote-pid = <1>; + + wcss_smp2p_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + wcss_smp2p_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; }; -- 2.34.1
[PATCH v6 04/11] dt-bindings: clock: qcom: gcc-ipq5332: remove q6 bring up clock macros
In multipd model Q6 firmware takes care of bringup clocks, so remove them. Signed-off-by: Manikanta Mylavarapu Reviewed-by: Krzysztof Kozlowski --- Changes in v6: - No changes. Changes in v5: - No changes. Changes in v4: - Pick up R-b tag Changes in v3: - Rebased on linux-next include/dt-bindings/clock/qcom,ipq5332-gcc.h | 20 1 file changed, 20 deletions(-) diff --git a/include/dt-bindings/clock/qcom,ipq5332-gcc.h b/include/dt-bindings/clock/qcom,ipq5332-gcc.h index 8a405a0a96d0..da9b507c30bf 100644 --- a/include/dt-bindings/clock/qcom,ipq5332-gcc.h +++ b/include/dt-bindings/clock/qcom,ipq5332-gcc.h @@ -96,15 +96,7 @@ #define GCC_PCNOC_BFDCD_CLK_SRC87 #define GCC_PCNOC_LPASS_CLK88 #define GCC_PRNG_AHB_CLK 89 -#define GCC_Q6_AHB_CLK 90 -#define GCC_Q6_AHB_S_CLK 91 -#define GCC_Q6_AXIM_CLK92 #define GCC_Q6_AXIM_CLK_SRC93 -#define GCC_Q6_AXIS_CLK94 -#define GCC_Q6_TSCTR_1TO2_CLK 95 -#define GCC_Q6SS_ATBM_CLK 96 -#define GCC_Q6SS_PCLKDBG_CLK 97 -#define GCC_Q6SS_TRIG_CLK 98 #define GCC_QDSS_AT_CLK99 #define GCC_QDSS_AT_CLK_SRC100 #define GCC_QDSS_CFG_AHB_CLK 101 @@ -134,7 +126,6 @@ #define GCC_SNOC_PCIE3_2LANE_S_CLK 125 #define GCC_SNOC_USB_CLK 126 #define GCC_SYS_NOC_AT_CLK 127 -#define GCC_SYS_NOC_WCSS_AHB_CLK 128 #define GCC_SYSTEM_NOC_BFDCD_CLK_SRC 129 #define GCC_UNIPHY0_AHB_CLK130 #define GCC_UNIPHY0_SYS_CLK131 @@ -155,17 +146,6 @@ #define GCC_USB0_PIPE_CLK 146 #define GCC_USB0_SLEEP_CLK 147 #define GCC_WCSS_AHB_CLK_SRC 148 -#define GCC_WCSS_AXIM_CLK 149 -#define GCC_WCSS_AXIS_CLK 150 -#define GCC_WCSS_DBG_IFC_APB_BDG_CLK 151 -#define GCC_WCSS_DBG_IFC_APB_CLK 152 -#define GCC_WCSS_DBG_IFC_ATB_BDG_CLK 153 -#define GCC_WCSS_DBG_IFC_ATB_CLK 154 -#define GCC_WCSS_DBG_IFC_NTS_BDG_CLK 155 -#define GCC_WCSS_DBG_IFC_NTS_CLK 156 -#define GCC_WCSS_ECAHB_CLK 157 -#define GCC_WCSS_MST_ASYNC_BDG_CLK 158 -#define GCC_WCSS_SLV_ASYNC_BDG_CLK 159 #define GCC_XO_CLK 160 #define GCC_XO_CLK_SRC 161 #define GCC_XO_DIV4_CLK162 -- 2.34.1
[PATCH v6 01/11] dt-bindings: remoteproc: qcom: Add support for multipd model
Add new binding document for multipd model remoteproc. IPQ5332, IPQ9574 follows multipd model. Signed-off-by: Manikanta Mylavarapu Reviewed-by: Krzysztof Kozlowski --- Changes in v6: - No changes. Changes in v5: - No changes. Changes in v4: - No changes. Changes in v3: - Pick up R-b tag Changes in v2: - Fixed all comments and rebased on linux-next. - Changed to BSD-3-Clause - Added firmware-name. .../bindings/remoteproc/qcom,multipd-pil.yaml | 189 ++ 1 file changed, 189 insertions(+) create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml new file mode 100644 index ..c52ac1640d7a --- /dev/null +++ b/Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml @@ -0,0 +1,189 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/remoteproc/qcom,multipd-pil.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Multipd Secure Peripheral Image Loader + +maintainers: + - Manikanta Mylavarapu + +description: + Multipd Peripheral Image Loader loads firmware and boots Q6 protection domain, + WCSS protection domain remoteproc's on the Qualcomm IPQ9574, IPQ5332 SoC. + Protection domain is similar to process in Linux. Here QDSP6 processor runs + each wifi radio functionality on a separate process. One process can't access + other process resources, so this is termed as PD i.e protection domain. + +properties: + compatible: +enum: + - qcom,ipq5332-q6-mpd + - qcom,ipq9574-q6-mpd + + reg: +maxItems: 1 + + firmware-name: +maxItems: 2 + + interrupts: +items: + - description: Watchdog interrupt + - description: Fatal interrupt + - description: Ready interrupt + - description: Handover interrupt + - description: Stop acknowledge interrupt + + interrupt-names: +items: + - const: wdog + - const: fatal + - const: ready + - const: handover + - const: stop-ack + + qcom,smem-states: +$ref: /schemas/types.yaml#/definitions/phandle-array +description: States used by the AP to signal the remote processor +items: + - description: Shutdown Q6 + - description: Stop Q6 + + qcom,smem-state-names: +description: + Names of the states used by the AP to signal the remote processor +items: + - const: shutdown + - const: stop + + memory-region: +items: + - description: Q6 reserved region + + glink-edge: +$ref: /schemas/remoteproc/qcom,glink-edge.yaml# +description: + Qualcomm G-Link subnode which represents communication edge, channels + and devices related to the Modem. +unevaluatedProperties: false + +patternProperties: + "^pd-1|pd-2|pd-3": +type: object +description: + WCSS means 'wireless connectivity sub system', in simple words it's a + wifi radio block. In multipd model, Q6 root protection domain will + provide services to WCSS user protection domain. In other words WCSS + protection domains depends on Q6 protection domain i.e Q6 should be up + before WCSS, similarly Q6 should shut down after all WCSS domains are + down. It can be achieved by building parent-child topology between Q6 + and WCSS domain's i.e keeping wcss node as sub node of Q6 device node. + +properties: + firmware-name: +maxItems: 1 + + interrupts: +items: + - description: Fatal interrupt + - description: Ready interrupt + - description: Spawn acknowledge interrupt + - description: Stop acknowledge interrupt + + interrupt-names: +items: + - const: fatal + - const: ready + - const: spawn-ack + - const: stop-ack + + qcom,smem-states: +$ref: /schemas/types.yaml#/definitions/phandle-array +description: States used by the AP to signal the remote processor +items: + - description: Shutdown WCSS pd + - description: Stop WCSS pd + - description: Spawn WCSS pd + + qcom,smem-state-names: +description: + Names of the states used by the AP to signal the remote processor +items: + - const: shutdown + - const: stop + - const: spawn + +required: + - firmware-name + - interrupts + - interrupt-names + - qcom,smem-states + - qcom,smem-state-names + +additionalProperties: false + +required: + - compatible + - firmware-name + - reg + - interrupts + - interrupt-names + - qcom,smem-states + - qcom,smem-state-names + - memory-region + +additionalProperties: false + +examples: + - | +#include +q6v5_wcss: remoteproc@d10 { + compatible =
[PATCH v6 10/11] arm64: dts: qcom: ipq5332: Add nodes to bringup multipd
Enable nodes required for multipd remoteproc bring up. Signed-off-by: Manikanta Mylavarapu --- Changes in v6: - Added user pd remoteproc nodes in RDP442 dts Changes in v5: - Rebased on linux-next Changes in v4: - Rebased on linux-next arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts | 21 arch/arm64/boot/dts/qcom/ipq5332-rdp442.dts | 59 + arch/arm64/boot/dts/qcom/ipq5332.dtsi | 59 + 3 files changed, 139 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts index e89e2e948603..e0e2f9238b47 100644 --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts @@ -21,6 +21,27 @@ &blsp1_i2c1 { status = "okay"; }; +&q6v5_wcss { + pd-1 { + firmware-name = "ath11k/IPQ5332/hw1.0/q6_fw1.mdt"; + interrupts-extended = <&wcss_smp2p_in 8 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 9 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 12 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 11 IRQ_TYPE_NONE>; + interrupt-names = "fatal", + "ready", + "spawn-ack", + "stop-ack"; + + qcom,smem-states = <&wcss_smp2p_out 8>, + <&wcss_smp2p_out 9>, + <&wcss_smp2p_out 10>; + qcom,smem-state-names = "shutdown", + "stop", + "spawn"; + }; +}; + &sdhc { bus-width = <4>; max-frequency = <19200>; diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp442.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp442.dts index efd480a7afdf..9b145c62e18d 100644 --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp442.dts +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp442.dts @@ -35,6 +35,65 @@ flash@0 { }; }; +&q6v5_wcss { + pd-1 { + firmware-name = "ath11k/IPQ5332/hw1.0/q6_fw1.mdt"; + interrupts-extended = <&wcss_smp2p_in 8 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 9 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 12 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 11 IRQ_TYPE_NONE>; + interrupt-names = "fatal", + "ready", + "spawn-ack", + "stop-ack"; + + qcom,smem-states = <&wcss_smp2p_out 8>, + <&wcss_smp2p_out 9>, + <&wcss_smp2p_out 10>; + qcom,smem-state-names = "shutdown", + "stop", + "spawn"; + }; + + pd-2 { + firmware-name = "ath11k/IPQ5332/hw1.0/q6_fw2.mdt"; + interrupts-extended = <&wcss_smp2p_in 16 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 17 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 20 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 19 IRQ_TYPE_NONE>; + interrupt-names = "fatal", + "ready", + "spawn-ack", + "stop-ack"; + + qcom,smem-states = <&wcss_smp2p_out 16>, + <&wcss_smp2p_out 17>, + <&wcss_smp2p_out 18>; + qcom,smem-state-names = "shutdown", + "stop", + "spawn"; + }; + + pd-3 { + firmware-name = "ath11k/IPQ5332/hw1.0/q6_fw3.mdt"; + interrupts-extended = <&wcss_smp2p_in 24 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 25 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 28 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 27 IRQ_TYPE_NONE>; + interrupt-names = "fatal", + "ready", + "spawn-ack", + "stop-ack"; + + qcom,smem-states = <&wcss_smp2p_out 24>, + <&wcss_smp2p_out 25>, + <&wcss_smp2p_out 26>; + qcom,smem-state-names = "shutdown", + "stop", + "spawn"; + }; +}; + &sdhc { bus-width = <4>; max-frequency = <19200>; diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi index 42e2e48b2bc3..0fdaece3dc8f 100644 --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
Re: [POC 0/7] livepatch: Make livepatch states, callbacks, and shadow variables work together
On Fri, Nov 10, 2023 at 06:04:21PM +0100, Petr Mladek wrote: > This POC is a material for the discussion "Simplify Livepatch Callbacks, > Shadow Variables, and States handling" at LPC 2013, see > https://lpc.events/event/17/contributions/1541/ > > It obsoletes the patchset adding the garbage collection of shadow > variables. This new solution is based on ideas from Nicolai Stange. > And it should also be in sync with Josh's ideas mentioned into > the thread about the garbage collection, see > https://lore.kernel.org/r/20230204235910.4j4ame5ntqogqi7m@treble Nice! I like how it brings the "features" together and makes them easy to use. This looks like a vast improvement. Was there a reason to change the naming? I'm thinking setup / enable / disable / release is less precise than pre_patch / post_patch / pre_unpatch / post_unpatch. Also, I'm thinking "replaced" instead of "obsolete" would be more consistent with the existing terminology. For example, in __klp_enable_patch(): ret = klp_setup_states(patch); if (ret) goto err; if (patch->replace) klp_disable_obsolete_states(patch); it's not immediately clear why "disable obsolete" would be the "replace" counterpart to "setup". Similarly, in klp_complete_transition(): if (klp_transition_patch->replace && klp_target_state == KLP_PATCHED) { klp_unpatch_replaced_patches(klp_transition_patch); klp_discard_nops(klp_transition_patch); klp_release_obsolete_states(klp_transition_patch); } it's a little jarring to have "unpatch replaced" followed by "release obsolete". So instead of: int klp_setup_states(struct klp_patch *patch); void klp_enable_states(struct klp_patch *patch); void klp_disable_states(struct klp_patch *patch); void klp_release_states(struct klp_patch *patch); void klp_enable_obsolete_states(struct klp_patch *patch); void klp_disable_obsolete_states(struct klp_patch *patch); void klp_release_obsolete_states(struct klp_patch *patch); how about something like: int klp_states_pre_patch(void); void klp_states_post_patch(void); void klp_states_pre_unpatch(void); void klp_states_post_unpatch(void); void klp_states_post_patch_replaced(void); void klp_states_pre_unpatch_replaced(void); void klp_states_post_unpatch_replaced(void); ? (note that passing klp_transition_patch might be optional since state.c already has visibility to it anyway) -- Josh
Re: [PATCH] tracing/kprobes: Fix the order of argument descriptions
On Tue, 31 Oct 2023 15:18:14 +0530 Mukesh Ojha wrote: > > > On 10/31/2023 9:43 AM, Yujie Liu wrote: > > The order of descriptions should be consistent with the argument list of > > the function, so "kretprobe" should be the second one. > > > > int __kprobe_event_gen_cmd_start(struct dynevent_cmd *cmd, bool kretprobe, > > const char *name, const char *loc, ...) > > > > Fixes: 2a588dd1d5d6 ("tracing: Add kprobe event command generation > > functions") > > Suggested-by: Mukesh Ojha > > Signed-off-by: Yujie Liu > > Thanks. > > Reviewed-by: Mukesh Ojha OK, let me pick this to probes/fixes. Thanks! > > -Mukesh > > > --- > > kernel/trace/trace_kprobe.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c > > index e834f149695b..47812aa16bb5 100644 > > --- a/kernel/trace/trace_kprobe.c > > +++ b/kernel/trace/trace_kprobe.c > > @@ -1020,9 +1020,9 @@ EXPORT_SYMBOL_GPL(kprobe_event_cmd_init); > > /** > >* __kprobe_event_gen_cmd_start - Generate a kprobe event command from > > arg list > >* @cmd: A pointer to the dynevent_cmd struct representing the new event > > + * @kretprobe: Is this a return probe? > >* @name: The name of the kprobe event > >* @loc: The location of the kprobe event > > - * @kretprobe: Is this a return probe? > >* @...: Variable number of arg (pairs), one pair for each field > >* > >* NOTE: Users normally won't want to call this function directly, but -- Masami Hiramatsu (Google)
Re: [POC 1/7] livepatch: Add callbacks for introducing and removing states
Hi Petr, kernel test robot noticed the following build warnings: [auto build test WARNING on shuah-kselftest/next] [also build test WARNING on shuah-kselftest/fixes linus/master v6.6 next-20231110] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Petr-Mladek/livepatch-Add-callbacks-for-introducing-and-removing-states/2023-014906 base: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next patch link: https://lore.kernel.org/r/20231110170428.6664-2-pmladek%40suse.com patch subject: [POC 1/7] livepatch: Add callbacks for introducing and removing states config: x86_64-randconfig-006-2023 (https://download.01.org/0day-ci/archive/2023/20230829.ukc9giug-...@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/2023/20230829.ukc9giug-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/20230829.ukc9giug-...@intel.com/ All warnings (new ones prefixed by >>): >> kernel/livepatch/state.c:121:6: warning: no previous prototype for >> 'is_state_in_other_patches' [-Wmissing-prototypes] 121 | bool is_state_in_other_patches(struct klp_patch *patch, struct klp_state *state) | ^ vim +/is_state_in_other_patches +121 kernel/livepatch/state.c 120 > 121 bool is_state_in_other_patches(struct klp_patch *patch, struct > klp_state *state) 122 { 123 struct klp_patch *old_patch; 124 struct klp_state *old_state; 125 126 klp_for_each_patch(old_patch) { 127 if (old_patch == patch) 128 continue; 129 130 klp_for_each_state(old_patch, old_state) { 131 if (old_state->id == state->id) 132 return true; 133 } 134 } 135 136 return false; 137 } 138 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
[ANNOUNCE] 5.10.199-rt97
Hello RT-list! I'm pleased to announce the 5.10.199-rt97 stable release. This release is an update to the new stable 5.10.199 version and no RT-specific changes were made, with the exception of a fix to a build failure due to upstream changes affecting only the PREEMPT_RT code. You can get this release via the git tree at: git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git branch: v5.10-rt Head SHA1: 7f300821d150a083008d6a73224acfc7e3283b04 Or to build 5.10.199-rt97 directly, the following patches should be applied: https://www.kernel.org/pub/linux/kernel/v5.x/linux-5.10.tar.xz https://www.kernel.org/pub/linux/kernel/v5.x/patch-5.10.199.xz https://www.kernel.org/pub/linux/kernel/projects/rt/5.10/older/patch-5.10.199-rt97.patch.xz Signing key fingerprint: 9354 0649 9972 8D31 D464 D140 F394 A423 F8E6 7C26 All keys used for the above files and repositories can be found on the following git repository: git://git.kernel.org/pub/scm/docs/kernel/pgpkeys.git Enjoy! Luis RT Changes since v5.10.197-rt96: --- Luis Claudio R. Goncalves (3): net: replace raw_write_seqcount_t_begin by do_raw_write_seqcount_begin diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 72be68652bb84..eee11a1c93216 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -195,7 +195,7 @@ static inline bool qdisc_run_begin(struct Qdisc *qdisc) * Variant of write_seqcount_t_begin() telling lockdep that a * trylock was attempted. */ - raw_write_seqcount_t_begin(s); + do_raw_write_seqcount_begin(s); seqcount_acquire(&s->dep_map, 0, 1, _RET_IP_); return true; }
Re: [POC 5/7] livepatch: Convert klp module callbacks tests into livepatch module tests
Hi Petr, kernel test robot noticed the following build errors: [auto build test ERROR on shuah-kselftest/next] [also build test ERROR on shuah-kselftest/fixes linus/master v6.6 next-20231110] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Petr-Mladek/livepatch-Add-callbacks-for-introducing-and-removing-states/2023-014906 base: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next patch link: https://lore.kernel.org/r/20231110170428.6664-6-pmladek%40suse.com patch subject: [POC 5/7] livepatch: Convert klp module callbacks tests into livepatch module tests config: s390-defconfig (https://download.01.org/0day-ci/archive/2023/20230928.ui5nizht-...@intel.com/config) compiler: s390-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/2023/20230928.ui5nizht-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/20230928.ui5nizht-...@intel.com/ All errors (new ones prefixed by >>): >> make[5]: *** No rule to make target 'lib/livepatch/test_klp_speaker2.o', >> needed by 'lib/livepatch/'. >> make[5]: *** No rule to make target >> 'lib/livepatch/test_klp_speaker_livepatch2.o', needed by 'lib/livepatch/'. make[5]: *** [scripts/Makefile.build:243: lib/livepatch/test_klp_speaker.o] Error 1 make[5]: *** [scripts/Makefile.build:243: lib/livepatch/test_klp_speaker_livepatch.o] Error 1 make[5]: Target 'lib/livepatch/' not remade because of errors. -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
[ANNOUNCE] 4.14.328-rt156
Hello RT-list! I'm pleased to announce the 4.14.328-rt156 stable release. This release is just an update to the new stable 4.14.328 version and no RT specific changes have been made. You can get this release via the git tree at: git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git branch: v4.14-rt Head SHA1: 45b1e061478c963779792ea50ecd2462e75a76a4 Or to build 4.14.328-rt156 directly, the following patches should be applied: https://www.kernel.org/pub/linux/kernel/v4.x/linux-4.14.tar.xz https://www.kernel.org/pub/linux/kernel/v4.x/patch-4.14.328.xz https://www.kernel.org/pub/linux/kernel/projects/rt/4.14/older/patch-4.14.328-rt156.patch.xz Signing key fingerprint: 9354 0649 9972 8D31 D464 D140 F394 A423 F8E6 7C26 All keys used for the above files and repositories can be found on the following git repository: git://git.kernel.org/pub/scm/docs/kernel/pgpkeys.git Enjoy! Luis
Re: [RFC PATCH v2 26/31] fprobe: Rewrite fprobe on function-graph tracer
On Fri, 10 Nov 2023 16:17:39 +0900 Masami Hiramatsu (Google) wrote: > > + used = 0; > > + hlist_for_each_entry_from_rcu(node, hlist) { > > + if (node->addr != func) > > + break; > > + fp = READ_ONCE(node->fp); > > + if (!fp || fprobe_disabled(fp)) > > + continue; > > + > > + if (fprobe_shared_with_kprobes(fp)) > > + ret = __fprobe_kprobe_handler(func, ret_ip, > > + fp, fregs, fgraph_data + used); > > + else > > + ret = __fprobe_handler(func, ret_ip, fp, > > + fregs, fgraph_data + used); > > > Since the fgraph callback is under rcu-locked but not preempt-disabled, rcu-locked? The only rcu-locked is task rcu. > fprobe unittest fails. I need to add preempt_disable_notrace() and > preempt_enable_notrace() around this. Note that kprobe_busy_begin()/end() > also access to per-cpu variable, so it requires to disable preemption. Just around the __fprobe_*handler()? Or the loop? -- Steve
[PATCH v3] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which helps in debugging the issues when there is Channel state & MHI state change and also when we receive data and control events: 1. mhi_intvec_mhi_states 2. mhi_process_data_event_ring 3. mhi_process_ctrl_ev_ring 4. mhi_gen_tre 5. mhi_update_channel_state 6. mhi_tryset_pm_state 7. mhi_pm_st_worker Where ever we added trace events removing debug messages. Signed-off-by: Krishna chaitanya chundru --- Changes in v3: - move trace header file from include/trace/events to drivers/bus/mhi/host/ so that - we can include driver header files. - Use macros directly in the trace events as suggested Jeffrey Hugo. - Reorder the structure in the events as suggested by steve to avoid holes in the buffer. - removed the mhi_to_physical function as this can give security issues. - removed macros to define strings as we can get those from driver headers. - Link to v2: https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com Changes in v2: - Passing the raw state into the trace event and using __print_symbolic() as suggested by bjorn. - Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn. - Fixed the kernel test rebot issues. - Link to v1: https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com --- drivers/bus/mhi/host/init.c | 3 + drivers/bus/mhi/host/internal.h | 1 + drivers/bus/mhi/host/main.c | 24 +++-- drivers/bus/mhi/host/pm.c | 6 +- drivers/bus/mhi/host/trace.h| 224 5 files changed, 245 insertions(+), 13 deletions(-) diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c index f78aefd2d7a3..6acb85f4c5f8 100644 --- a/drivers/bus/mhi/host/init.c +++ b/drivers/bus/mhi/host/init.c @@ -20,6 +20,9 @@ #include #include "internal.h" +#define CREATE_TRACE_POINTS +#include "trace.h" + static DEFINE_IDA(mhi_controller_ida); const char * const mhi_ee_str[MHI_EE_MAX] = { diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h index 2e139e76de4c..a02a71605907 100644 --- a/drivers/bus/mhi/host/internal.h +++ b/drivers/bus/mhi/host/internal.h @@ -7,6 +7,7 @@ #ifndef _MHI_INT_H #define _MHI_INT_H +#include "trace.h" #include "../common.h" extern struct bus_type mhi_bus_type; diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c index dcf627b36e82..7fe8b2dd9e24 100644 --- a/drivers/bus/mhi/host/main.c +++ b/drivers/bus/mhi/host/main.c @@ -491,11 +491,9 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *priv) state = mhi_get_mhi_state(mhi_cntrl); ee = mhi_get_exec_env(mhi_cntrl); - dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n", - TO_MHI_EXEC_STR(mhi_cntrl->ee), - mhi_state_str(mhi_cntrl->dev_state), - TO_MHI_EXEC_STR(ee), mhi_state_str(state)); + trace_mhi_intvec_states(mhi_cntrl->mhi_dev->name, mhi_cntrl->ee, + mhi_cntrl->dev_state, ee, state); if (state == MHI_STATE_SYS_ERR) { dev_dbg(dev, "System error detected\n"); pm_state = mhi_tryset_pm_state(mhi_cntrl, @@ -832,6 +830,10 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller *mhi_cntrl, while (dev_rp != local_rp) { enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp); + trace_mhi_process_ctrl_ev_ring(mhi_cntrl->mhi_dev->name, (u64)local_rp, + local_rp->ptr, local_rp->dword[0], + local_rp->dword[1]); + switch (type) { case MHI_PKT_TYPE_BW_REQ_EVENT: { @@ -997,6 +999,9 @@ int mhi_process_data_event_ring(struct mhi_controller *mhi_cntrl, while (dev_rp != local_rp && event_quota > 0) { enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp); + trace_mhi_process_data_event_ring(mhi_cntrl->mhi_dev->name, local_rp->ptr, + local_rp->dword[0], local_rp->dword[1]); + chan = MHI_TRE_GET_EV_CHID(local_rp); WARN_ON(chan >= mhi_cntrl->max_chan); @@ -1235,6 +1240,8 @@ int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan, mhi_tre->dword[0] = MHI_TRE_DATA_DWORD0(info->len); mhi_tre->dword[1] = MHI_TRE_DATA_DWORD1(bei, eot, eob, chain); + trace_mhi_gen_tre(mhi_cntrl->mhi_dev->name, mhi_chan->chan, (u64)mhi_tre, + mhi_tre->ptr, mhi_tre->dword[0], mhi_tre->dword[1]); /* increment WP */ mhi_add_ring_element(mhi_cntrl, tre_ring); mhi_add_ring_element(mhi_cntrl, buf_ring); @@ -1327,9 +1334,8 @@ static int mhi_update_channel_state(struct mhi_controller *mhi_cntrl, enum mhi_cmd_type cmd = MHI_CMD_NOP; int ret; - dev_dbg(dev, "%d: Updatin
Re: [RFC PATCH v2 01/31] tracing: Add a comment about ftrace_regs definition
On Fri, 10 Nov 2023 11:11:31 + Mark Rutland wrote: > On Thu, Nov 09, 2023 at 08:14:52AM +0900, Masami Hiramatsu wrote: > > On Wed, 8 Nov 2023 23:24:32 +0900 > > "Masami Hiramatsu (Google)" wrote: > > > > > From: Masami Hiramatsu (Google) > > > > > > To clarify what will be expected on ftrace_regs, add a comment to the > > > architecture independent definition of the ftrace_regs. > > > > > > Signed-off-by: Masami Hiramatsu (Google) > > > --- > > > Changes in v2: > > > - newly added. > > > --- > > > include/linux/ftrace.h | 25 + > > > 1 file changed, 25 insertions(+) > > > > > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > > > index e8921871ef9a..b174af91d8be 100644 > > > --- a/include/linux/ftrace.h > > > +++ b/include/linux/ftrace.h > > > @@ -118,6 +118,31 @@ extern int ftrace_enabled; > > > > > > #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS > > > > > > +/** > > > + * ftrace_regs - ftrace partial/optimal register set > > > + * > > > + * ftrace_regs represents a group of registers which is used at the > > > + * function entry and exit. There are three types of registers. > > > + * > > > + * - Registers for passing the parameters to callee, including the stack > > > + * pointer. (e.g. rcx, rdx, rdi, rsi, r8, r9 and rsp on x86_64) > > > + * - Registers for passing the return values to caller. > > > + * (e.g. rax and rdx on x86_64) > > > + * - Registers for hooking the function return including the frame > > > pointer > > > + * (the frame pointer is architecture/config dependent) > > > + * (e.g. rbp and rsp for x86_64) > > > > Oops, I found the program counter/instruction pointer must be saved too. > > This is used for live patching. One question is that if the IP is modified > > at the return handler, what should we do? Return to the specified address? > > I'm a bit confused here; currently we use fgraph_ret_regs for function > returns, > are we going to replace that with ftrace_regs? Yes. It is limited and does not have APIs compatibility. > > I think it makes sense for the PC/IP to be the address the return handler will > eventually return to (and hence allowing it to be overridden), but that does > mean we'll need to go recover the return address *before* we invoke any return > handlers. The actual return address has been recovered from shadow stack at first, and callback the handlers. See __ftrace_return_to_handler() and ftrace_pop_return_trace(). So it is easy to set it to the ftrace_regs :) Thank you! > > Thanks, > Mark. -- Masami Hiramatsu (Google)
Re: [RFC PATCH v2 26/31] fprobe: Rewrite fprobe on function-graph tracer
On Fri, 10 Nov 2023 20:44:22 -0500 Steven Rostedt wrote: > On Fri, 10 Nov 2023 16:17:39 +0900 > Masami Hiramatsu (Google) wrote: > > > > + used = 0; > > > + hlist_for_each_entry_from_rcu(node, hlist) { > > > + if (node->addr != func) > > > + break; > > > + fp = READ_ONCE(node->fp); > > > + if (!fp || fprobe_disabled(fp)) > > > + continue; > > > + > > > + if (fprobe_shared_with_kprobes(fp)) > > > + ret = __fprobe_kprobe_handler(func, ret_ip, > > > + fp, fregs, fgraph_data + used); > > > + else > > > + ret = __fprobe_handler(func, ret_ip, fp, > > > + fregs, fgraph_data + used); > > > > > > Since the fgraph callback is under rcu-locked but not preempt-disabled, > > rcu-locked? The only rcu-locked is task rcu. Hmm, it might be my misread. But I don't get any warning from find_first_fprobe_node(), which uses hlist_for_each_entry_rcu() so isn't it rcu locked? > > > fprobe unittest fails. I need to add preempt_disable_notrace() and > > preempt_enable_notrace() around this. Note that kprobe_busy_begin()/end() > > also access to per-cpu variable, so it requires to disable preemption. > > > Just around the __fprobe_*handler()? Or the loop? Just around the __fprobe*handler(). Thank you, > > -- Steve -- Masami Hiramatsu (Google)
Re: [POC 1/7] livepatch: Add callbacks for introducing and removing states
Hi Petr, kernel test robot noticed the following build warnings: [auto build test WARNING on shuah-kselftest/next] [also build test WARNING on shuah-kselftest/fixes linus/master v6.6 next-20231110] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Petr-Mladek/livepatch-Add-callbacks-for-introducing-and-removing-states/2023-014906 base: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next patch link: https://lore.kernel.org/r/20231110170428.6664-2-pmladek%40suse.com patch subject: [POC 1/7] livepatch: Add callbacks for introducing and removing states config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/2023/20231107.avvipri2-...@intel.com/config) compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/2023/20231107.avvipri2-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/20231107.avvipri2-...@intel.com/ All warnings (new ones prefixed by >>): >> kernel/livepatch/state.c:121:6: warning: no previous prototype for function >> 'is_state_in_other_patches' [-Wmissing-prototypes] bool is_state_in_other_patches(struct klp_patch *patch, struct klp_state *state) ^ kernel/livepatch/state.c:121:1: note: declare 'static' if the function is not intended to be used outside of this translation unit bool is_state_in_other_patches(struct klp_patch *patch, struct klp_state *state) ^ static 1 warning generated. vim +/is_state_in_other_patches +121 kernel/livepatch/state.c 120 > 121 bool is_state_in_other_patches(struct klp_patch *patch, struct > klp_state *state) 122 { 123 struct klp_patch *old_patch; 124 struct klp_state *old_state; 125 126 klp_for_each_patch(old_patch) { 127 if (old_patch == patch) 128 continue; 129 130 klp_for_each_state(old_patch, old_state) { 131 if (old_state->id == state->id) 132 return true; 133 } 134 } 135 136 return false; 137 } 138 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH v3] bus: mhi: host: Add tracing support
Hi Krishna, kernel test robot noticed the following build errors: [auto build test ERROR on 3006adf3be79cde4d14b1800b963b82b6e5572e0] url: https://github.com/intel-lab-lkp/linux/commits/Krishna-chaitanya-chundru/bus-mhi-host-Add-tracing-support/2023-101955 base: 3006adf3be79cde4d14b1800b963b82b6e5572e0 patch link: https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911a74%40quicinc.com patch subject: [PATCH v3] bus: mhi: host: Add tracing support config: sh-allmodconfig (https://download.01.org/0day-ci/archive/2023/20231237.ztm5nblh-...@intel.com/config) compiler: sh4-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/2023/20231237.ztm5nblh-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/20231237.ztm5nblh-...@intel.com/ All errors (new ones prefixed by >>): In file included from include/trace/define_trace.h:102, from drivers/bus/mhi/host/trace.h:224, from drivers/bus/mhi/host/init.c:24: drivers/bus/mhi/host/./trace.h: In function 'trace_event_raw_event_mhi_process_ctrl_ev_ring': >> drivers/bus/mhi/host/../common.h:144:58: error: invalid type argument of >> '->' (have 'u64' {aka 'long long unsigned int'}) 144 | #define MHI_TRE_GET_DWORD(tre, word) le32_to_cpu((tre)->dword[(word)]) | ^~ include/trace/trace_events.h:402:11: note: in definition of macro 'DECLARE_EVENT_CLASS' 402 | { assign; } \ | ^~ include/trace/trace_events.h:44:30: note: in expansion of macro 'PARAMS' 44 | PARAMS(assign), \ | ^~ drivers/bus/mhi/host/./trace.h:123:1: note: in expansion of macro 'TRACE_EVENT' 123 | TRACE_EVENT(mhi_process_ctrl_ev_ring, | ^~~ drivers/bus/mhi/host/./trace.h:138:9: note: in expansion of macro 'TP_fast_assign' 138 | TP_fast_assign( | ^~ include/linux/compiler_types.h:413:9: note: in expansion of macro '__compiletime_assert' 413 | __compiletime_assert(condition, msg, prefix, suffix) | ^~~~ include/linux/compiler_types.h:425:9: note: in expansion of macro '_compiletime_assert' 425 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^~~ include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert' 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) | ^~ include/linux/bitfield.h:71:17: note: in expansion of macro 'BUILD_BUG_ON_MSG' 71 | BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) > \ | ^~~~ include/linux/bitfield.h:61:43: note: in expansion of macro '__unsigned_scalar_typeof' 61 | #define __bf_cast_unsigned(type, x) ((__unsigned_scalar_typeof(type))(x)) | ^~~~ include/linux/bitfield.h:72:34: note: in expansion of macro '__bf_cast_unsigned' 72 | __bf_cast_unsigned(_reg, ~0ull), \ | ^~ include/linux/bitfield.h:154:17: note: in expansion of macro '__BF_FIELD_CHECK' 154 | __BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \ | ^~~~ drivers/bus/mhi/host/../common.h:159:41: note: in expansion of macro 'FIELD_GET' 159 | #define MHI_TRE_GET_EV_STATE(tre) FIELD_GET(GENMASK(31, 24), (MHI_TRE_GET_DWORD(tre, 0))) | ^ include/linux/byteorder/generic.h:89:21: note: in expansion of macro '__le32_to_cpu' 89 | #define le32_to_cpu __le32_to_cpu | ^ drivers/bus/mhi/host/../common.h:159:69: note: in expansion of macro 'MHI_TRE_GET_DWORD' 159 | #define MHI_TRE_GET_EV_STATE(tre) FIELD_GET(GENMASK(31, 24), (MHI_TRE_GET_DWORD(tre, 0))) | ^ drivers/bus/mhi/host/./trace.h:144:34: note: in expansion of macro 'MHI_TRE_GET_EV_STATE' 144 | __entry->state = MHI_TRE_GET_EV_STATE(rp); | ^~~~ >> drivers/bus/mhi/host/../common.h:144:58: error: invalid type argument of >> '->' (have 'u64' {aka 'long long unsigned int'}
[PATCH v4] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which helps in debugging the issues when there is Channel state & MHI state change and also when we receive data and control events: 1. mhi_intvec_mhi_states 2. mhi_process_data_event_ring 3. mhi_process_ctrl_ev_ring 4. mhi_gen_tre 5. mhi_update_channel_state 6. mhi_tryset_pm_state 7. mhi_pm_st_worker Where ever we added trace events removing debug messages. Signed-off-by: Krishna chaitanya chundru --- Changes in v4: - Fix compilation issues in previous patch which happended due to rebasing. - In the defconfig FTRACE config is not enabled due to that the compilation issue is not - seen in my workspace. - Link to v3: https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com Changes in v3: - move trace header file from include/trace/events to drivers/bus/mhi/host/ so that - we can include driver header files. - Use macros directly in the trace events as suggested Jeffrey Hugo. - Reorder the structure in the events as suggested by steve to avoid holes in the buffer. - removed the mhi_to_physical function as this can give security issues. - removed macros to define strings as we can get those from driver headers. - Link to v2: https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com Changes in v2: - Passing the raw state into the trace event and using __print_symbolic() as suggested by bjorn. - Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn. - Fixed the kernel test rebot issues. - Link to v1: https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com --- drivers/bus/mhi/host/init.c | 3 + drivers/bus/mhi/host/internal.h | 1 + drivers/bus/mhi/host/main.c | 24 +++-- drivers/bus/mhi/host/pm.c | 6 +- drivers/bus/mhi/host/trace.h| 225 5 files changed, 246 insertions(+), 13 deletions(-) diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c index f78aefd2d7a3..6acb85f4c5f8 100644 --- a/drivers/bus/mhi/host/init.c +++ b/drivers/bus/mhi/host/init.c @@ -20,6 +20,9 @@ #include #include "internal.h" +#define CREATE_TRACE_POINTS +#include "trace.h" + static DEFINE_IDA(mhi_controller_ida); const char * const mhi_ee_str[MHI_EE_MAX] = { diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h index 2e139e76de4c..a02a71605907 100644 --- a/drivers/bus/mhi/host/internal.h +++ b/drivers/bus/mhi/host/internal.h @@ -7,6 +7,7 @@ #ifndef _MHI_INT_H #define _MHI_INT_H +#include "trace.h" #include "../common.h" extern struct bus_type mhi_bus_type; diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c index dcf627b36e82..923d51904f35 100644 --- a/drivers/bus/mhi/host/main.c +++ b/drivers/bus/mhi/host/main.c @@ -491,11 +491,9 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *priv) state = mhi_get_mhi_state(mhi_cntrl); ee = mhi_get_exec_env(mhi_cntrl); - dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n", - TO_MHI_EXEC_STR(mhi_cntrl->ee), - mhi_state_str(mhi_cntrl->dev_state), - TO_MHI_EXEC_STR(ee), mhi_state_str(state)); + trace_mhi_intvec_states(mhi_cntrl->mhi_dev->name, mhi_cntrl->ee, + mhi_cntrl->dev_state, ee, state); if (state == MHI_STATE_SYS_ERR) { dev_dbg(dev, "System error detected\n"); pm_state = mhi_tryset_pm_state(mhi_cntrl, @@ -832,6 +830,10 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller *mhi_cntrl, while (dev_rp != local_rp) { enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp); + trace_mhi_process_ctrl_ev_ring(mhi_cntrl->mhi_dev->name, local_rp, + local_rp->ptr, local_rp->dword[0], + local_rp->dword[1]); + switch (type) { case MHI_PKT_TYPE_BW_REQ_EVENT: { @@ -997,6 +999,9 @@ int mhi_process_data_event_ring(struct mhi_controller *mhi_cntrl, while (dev_rp != local_rp && event_quota > 0) { enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp); + trace_mhi_process_data_event_ring(mhi_cntrl->mhi_dev->name, local_rp->ptr, + local_rp->dword[0], local_rp->dword[1]); + chan = MHI_TRE_GET_EV_CHID(local_rp); WARN_ON(chan >= mhi_cntrl->max_chan); @@ -1235,6 +1240,8 @@ int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan, mhi_tre->dword[0] = MHI_TRE_DATA_DWORD0(info->len); mhi_tre->dword[1] = MHI_TRE_DATA_DWORD1(bei, eot, eob, chain); + trace_mhi_gen_tre(mhi_cntrl->mhi_dev->name, mhi_chan->chan, (u64)mhi_tre, + mhi_tre->ptr, mhi_tre->dword[0], mhi_tre->dword[1]); /* increment WP
Re: [PATCH v4] bus: mhi: host: Add tracing support
Hi Krishna, kernel test robot noticed the following build warnings: [auto build test WARNING on 3006adf3be79cde4d14b1800b963b82b6e5572e0] url: https://github.com/intel-lab-lkp/linux/commits/Krishna-chaitanya-chundru/bus-mhi-host-Add-tracing-support/2023-135816 base: 3006adf3be79cde4d14b1800b963b82b6e5572e0 patch link: https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399461%40quicinc.com patch subject: [PATCH v4] bus: mhi: host: Add tracing support config: csky-randconfig-002-2023 (https://download.01.org/0day-ci/archive/2023/20231502.weci1okb-...@intel.com/config) compiler: csky-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/2023/20231502.weci1okb-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/20231502.weci1okb-...@intel.com/ All warnings (new ones prefixed by >>): In file included from include/trace/define_trace.h:102, from drivers/bus/mhi/host/trace.h:225, from drivers/bus/mhi/host/init.c:24: drivers/bus/mhi/host/./trace.h: In function 'trace_event_raw_event_mhi_process_ctrl_ev_ring': >> drivers/bus/mhi/host/./trace.h:141:31: warning: cast from pointer to integer >> of different size [-Wpointer-to-int-cast] 141 | __entry->rp = (u64)rp; | ^ include/trace/trace_events.h:402:11: note: in definition of macro 'DECLARE_EVENT_CLASS' 402 | { assign; } \ | ^~ include/trace/trace_events.h:44:30: note: in expansion of macro 'PARAMS' 44 | PARAMS(assign), \ | ^~ drivers/bus/mhi/host/./trace.h:123:1: note: in expansion of macro 'TRACE_EVENT' 123 | TRACE_EVENT(mhi_process_ctrl_ev_ring, | ^~~ drivers/bus/mhi/host/./trace.h:139:9: note: in expansion of macro 'TP_fast_assign' 139 | TP_fast_assign( | ^~ In file included from include/trace/define_trace.h:103: drivers/bus/mhi/host/./trace.h: In function 'perf_trace_mhi_process_ctrl_ev_ring': >> drivers/bus/mhi/host/./trace.h:141:31: warning: cast from pointer to integer >> of different size [-Wpointer-to-int-cast] 141 | __entry->rp = (u64)rp; | ^ include/trace/perf.h:51:11: note: in definition of macro 'DECLARE_EVENT_CLASS' 51 | { assign; } \ | ^~ include/trace/trace_events.h:44:30: note: in expansion of macro 'PARAMS' 44 | PARAMS(assign), \ | ^~ drivers/bus/mhi/host/./trace.h:123:1: note: in expansion of macro 'TRACE_EVENT' 123 | TRACE_EVENT(mhi_process_ctrl_ev_ring, | ^~~ drivers/bus/mhi/host/./trace.h:139:9: note: in expansion of macro 'TP_fast_assign' 139 | TP_fast_assign( | ^~ vim +141 drivers/bus/mhi/host/./trace.h 124 125 TP_PROTO(const char *name, struct mhi_ring_element *rp, __le64 ptr, 126 __le32 dword0, __le32 dword1), 127 128 TP_ARGS(name, rp, ptr, dword0, dword1), 129 130 TP_STRUCT__entry( 131 __field(u64, rp) 132 __field(__le64, ptr) 133 __string(name, name) 134 __field(__le32, dword0) 135 __field(__le32, dword1) 136 __field(int, state) 137 ), 138 139 TP_fast_assign( 140 __assign_str(name, name); > 141 __entry->rp = (u64)rp; 142 __entry->ptr = ptr; 143 __entry->dword0 = dword0; 144 __entry->dword1 = dword1; 145 __entry->state = MHI_TRE_GET_EV_STATE(rp); 146 ), 147 148 TP_printk("%s: RP:0x%llx Processing Event:0x%llx 0x%08x 0x%08x state:%s\n", 149__get_str(name), __entry->rp, __entry->ptr, __entry->dword0, 150__entry->dword1, mhi_state_str(__entry->state)) 151 ); 152 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki