* [PATCH 06/13] kallsyms: only build {, module_}kallsyms_on_each_symbol when required
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
kallsyms_on_each_symbol and module_kallsyms_on_each_symbol are only used
by the livepatching code, so don't build them if livepatching is not
enabled.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/kallsyms.h | 17 ++++-------------
include/linux/module.h | 16 ++++------------
kernel/kallsyms.c | 2 ++
kernel/module.c | 2 ++
4 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 481273f0c72d42..465060acc9816f 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -71,15 +71,14 @@ static inline void *dereference_symbol_descriptor(void *ptr)
return ptr;
}
-#ifdef CONFIG_KALLSYMS
-/* Lookup the address for a symbol. Returns 0 if not found. */
-unsigned long kallsyms_lookup_name(const char *name);
-
-/* Call a function on each kallsyms symbol in the core kernel */
int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
unsigned long),
void *data);
+#ifdef CONFIG_KALLSYMS
+/* Lookup the address for a symbol. Returns 0 if not found. */
+unsigned long kallsyms_lookup_name(const char *name);
+
extern int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset);
@@ -108,14 +107,6 @@ static inline unsigned long kallsyms_lookup_name(const char *name)
return 0;
}
-static inline int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
- struct module *,
- unsigned long),
- void *data)
-{
- return 0;
-}
-
static inline int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset)
diff --git a/include/linux/module.h b/include/linux/module.h
index 8588482bde4116..695f127745af10 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -610,10 +610,6 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
/* Look for this name: can be of form module:name. */
unsigned long module_kallsyms_lookup_name(const char *name);
-int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
- struct module *, unsigned long),
- void *data);
-
extern void __noreturn __module_put_and_exit(struct module *mod,
long code);
#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code)
@@ -797,14 +793,6 @@ static inline unsigned long module_kallsyms_lookup_name(const char *name)
return 0;
}
-static inline int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
- struct module *,
- unsigned long),
- void *data)
-{
- return 0;
-}
-
static inline int register_module_notifier(struct notifier_block *nb)
{
/* no events will happen anyway, so this can always succeed */
@@ -893,4 +881,8 @@ static inline bool module_sig_ok(struct module *module)
}
#endif /* CONFIG_MODULE_SIG */
+int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
+ struct module *, unsigned long),
+ void *data);
+
#endif /* _LINUX_MODULE_H */
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index a0d3f0865916f9..8043a90aa50ed3 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -177,6 +177,7 @@ unsigned long kallsyms_lookup_name(const char *name)
return module_kallsyms_lookup_name(name);
}
+#ifdef CONFIG_LIVEPATCH
/*
* Iterate over all symbols in vmlinux. For symbols from modules use
* module_kallsyms_on_each_symbol instead.
@@ -198,6 +199,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
}
return 0;
}
+#endif /* CONFIG_LIVEPATCH */
static unsigned long get_symbol_pos(unsigned long addr,
unsigned long *symbolsize,
diff --git a/kernel/module.c b/kernel/module.c
index 885feec64c1b6f..e141e5d1d7beaf 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4399,6 +4399,7 @@ unsigned long module_kallsyms_lookup_name(const char *name)
return ret;
}
+#ifdef CONFIG_LIVEPATCH
int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
struct module *, unsigned long),
void *data)
@@ -4429,6 +4430,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
mutex_unlock(&module_mutex);
return ret;
}
+#endif /* CONFIG_LIVEPATCH */
#endif /* CONFIG_KALLSYMS */
/* Maximum number of characters written by module_flags() */
--
2.29.2
^ permalink raw reply related
* [PATCH 05/13] kallsyms: refactor {,module_}kallsyms_on_each_symbol
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
Require an explicit cll to module_kallsyms_on_each_symbol to look
for symbols in modules instead of the call from kallsyms_on_each_symbol,
and acquire module_mutex inside of module_kallsyms_on_each_symbol instead
of leaving that up to the caller.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
kernel/kallsyms.c | 6 +++++-
kernel/livepatch/core.c | 6 +-----
kernel/module.c | 8 ++++----
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index fe9de067771c34..a0d3f0865916f9 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -177,6 +177,10 @@ unsigned long kallsyms_lookup_name(const char *name)
return module_kallsyms_lookup_name(name);
}
+/*
+ * Iterate over all symbols in vmlinux. For symbols from modules use
+ * module_kallsyms_on_each_symbol instead.
+ */
int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
unsigned long),
void *data)
@@ -192,7 +196,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
if (ret != 0)
return ret;
}
- return module_kallsyms_on_each_symbol(fn, data);
+ return 0;
}
static unsigned long get_symbol_pos(unsigned long addr,
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 878759baadd81c..8063b9089bd2f8 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -135,12 +135,8 @@ static int klp_find_object_symbol(const char *objname, const char *name,
.pos = sympos,
};
- mutex_lock(&module_mutex);
- if (objname)
+ if (objname || !kallsyms_on_each_symbol(klp_find_callback, &args))
module_kallsyms_on_each_symbol(klp_find_callback, &args);
- else
- kallsyms_on_each_symbol(klp_find_callback, &args);
- mutex_unlock(&module_mutex);
/*
* Ensure an address was found. If sympos is 0, ensure symbol is unique;
diff --git a/kernel/module.c b/kernel/module.c
index 299cbac0775cf2..885feec64c1b6f 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4407,8 +4407,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
unsigned int i;
int ret;
- module_assert_mutex();
-
+ mutex_lock(&module_mutex);
list_for_each_entry(mod, &modules, list) {
/* We hold module_mutex: no need for rcu_dereference_sched */
struct mod_kallsyms *kallsyms = mod->kallsyms;
@@ -4424,10 +4423,11 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
ret = fn(data, kallsyms_symbol_name(kallsyms, i),
mod, kallsyms_symbol_value(sym));
if (ret != 0)
- return ret;
+ break;
}
}
- return 0;
+ mutex_unlock(&module_mutex);
+ return ret;
}
#endif /* CONFIG_KALLSYMS */
--
2.29.2
^ permalink raw reply related
* [PATCH 04/13] livepatch: move klp_find_object_module to module.c
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
To uncouple the livepatch code from module loader internals move a
slightly refactored version of klp_find_object_module to module.c
This allows to mark find_module static and removes one of the last
users of module_mutex outside of module.c.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/module.h | 3 +--
kernel/livepatch/core.c | 39 +++++++++++++--------------------------
kernel/module.c | 17 ++++++++++++++++-
3 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index b4654f8a408134..8588482bde4116 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -586,8 +586,7 @@ static inline bool within_module(unsigned long addr, const struct module *mod)
return within_module_init(addr, mod) || within_module_core(addr, mod);
}
-/* Search for module by name: must hold module_mutex. */
-struct module *find_module(const char *name);
+struct module *find_klp_module(const char *name);
/* Check if a module is loaded. */
bool module_loaded(const char *name);
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index a7f625dc24add3..878759baadd81c 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -49,30 +49,6 @@ static bool klp_is_module(struct klp_object *obj)
return obj->name;
}
-/* sets obj->mod if object is not vmlinux and module is found */
-static void klp_find_object_module(struct klp_object *obj)
-{
- struct module *mod;
-
- mutex_lock(&module_mutex);
- /*
- * We do not want to block removal of patched modules and therefore
- * we do not take a reference here. The patches are removed by
- * klp_module_going() instead.
- */
- mod = find_module(obj->name);
- /*
- * Do not mess work of klp_module_coming() and klp_module_going().
- * Note that the patch might still be needed before klp_module_going()
- * is called. Module functions can be called even in the GOING state
- * until mod->exit() finishes. This is especially important for
- * patches that modify semantic of the functions.
- */
- if (mod && mod->klp_alive)
- obj->mod = mod;
- mutex_unlock(&module_mutex);
-}
-
static bool klp_initialized(void)
{
return !!klp_root_kobj;
@@ -820,14 +796,25 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
const char *name;
obj->patched = false;
- obj->mod = NULL;
if (klp_is_module(obj)) {
if (strlen(obj->name) >= MODULE_NAME_LEN)
return -EINVAL;
name = obj->name;
- klp_find_object_module(obj);
+ /*
+ * We do not want to block removal of patched modules and
+ * therefore we do not take a reference here. The patches are
+ * removed by klp_module_going() instead.
+ *
+ * Do not mess work of klp_module_coming() and
+ * klp_module_going(). Note that the patch might still be
+ * needed before klp_module_going() is called. Module functions
+ * can be called even in the GOING state until mod->exit()
+ * finishes. This is especially important for patches that
+ * modify semantic of the functions.
+ */
+ obj->mod = find_klp_module(obj->name);
} else {
name = "vmlinux";
}
diff --git a/kernel/module.c b/kernel/module.c
index 619ea682e64cd1..299cbac0775cf2 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -666,7 +666,7 @@ static struct module *find_module_all(const char *name, size_t len,
return NULL;
}
-struct module *find_module(const char *name)
+static struct module *find_module(const char *name)
{
module_assert_mutex();
return find_module_all(name, strlen(name), false);
@@ -684,6 +684,21 @@ bool module_loaded(const char *name)
}
EXPORT_SYMBOL_GPL(module_loaded);
+#ifdef CONFIG_LIVEPATCH
+struct module *find_klp_module(const char *name)
+{
+ struct module *mod;
+
+ mutex_lock(&module_mutex);
+ mod = find_module(name);
+ if (mod && !mod->klp_alive)
+ mod = NULL;
+ mutex_unlock(&module_mutex);
+
+ return mod;
+}
+#endif /* CONFIG_LIVEPATCH */
+
#ifdef CONFIG_SMP
static inline void __percpu *mod_percpu(struct module *mod)
--
2.29.2
^ permalink raw reply related
* [PATCH 03/13] livepatch: refactor klp_init_object
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
Merge three calls to klp_is_module (including one hidden inside
klp_find_object_module) into a single one to simplify the code a bit.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
kernel/livepatch/core.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index f76fdb9255323d..a7f625dc24add3 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -54,9 +54,6 @@ static void klp_find_object_module(struct klp_object *obj)
{
struct module *mod;
- if (!klp_is_module(obj))
- return;
-
mutex_lock(&module_mutex);
/*
* We do not want to block removal of patched modules and therefore
@@ -73,7 +70,6 @@ static void klp_find_object_module(struct klp_object *obj)
*/
if (mod && mod->klp_alive)
obj->mod = mod;
-
mutex_unlock(&module_mutex);
}
@@ -823,15 +819,19 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
int ret;
const char *name;
- if (klp_is_module(obj) && strlen(obj->name) >= MODULE_NAME_LEN)
- return -EINVAL;
-
obj->patched = false;
obj->mod = NULL;
- klp_find_object_module(obj);
+ if (klp_is_module(obj)) {
+ if (strlen(obj->name) >= MODULE_NAME_LEN)
+ return -EINVAL;
+ name = obj->name;
+
+ klp_find_object_module(obj);
+ } else {
+ name = "vmlinux";
+ }
- name = klp_is_module(obj) ? obj->name : "vmlinux";
ret = kobject_add(&obj->kobj, &patch->kobj, "%s", name);
if (ret)
return ret;
--
2.29.2
^ permalink raw reply related
* [PATCH 02/13] module: add a module_loaded helper
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
Add a helper that takes modules_mutex and uses find_module to check if a
given module is loaded. This provides a better abstraction for the two
callers, and allows to unexport modules_mutex and find_module.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/drm_fb_helper.c | 7 +------
include/linux/module.h | 3 +++
kernel/module.c | 14 ++++++++++++--
kernel/trace/trace_kprobe.c | 4 +---
4 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4b81195106875d..ce6d63ca75c32a 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2508,13 +2508,8 @@ int __init drm_fb_helper_modinit(void)
{
#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
const char name[] = "fbcon";
- struct module *fbcon;
- mutex_lock(&module_mutex);
- fbcon = find_module(name);
- mutex_unlock(&module_mutex);
-
- if (!fbcon)
+ if (!module_loaded(name))
request_module_nowait(name);
#endif
return 0;
diff --git a/include/linux/module.h b/include/linux/module.h
index 7a0bcb5b1ffccd..b4654f8a408134 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -589,6 +589,9 @@ static inline bool within_module(unsigned long addr, const struct module *mod)
/* Search for module by name: must hold module_mutex. */
struct module *find_module(const char *name);
+/* Check if a module is loaded. */
+bool module_loaded(const char *name);
+
struct symsearch {
const struct kernel_symbol *start, *stop;
const s32 *crcs;
diff --git a/kernel/module.c b/kernel/module.c
index 4bf30e4b3eaaa1..619ea682e64cd1 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -88,7 +88,6 @@
* (delete and add uses RCU list operations).
*/
DEFINE_MUTEX(module_mutex);
-EXPORT_SYMBOL_GPL(module_mutex);
static LIST_HEAD(modules);
/* Work queue for freeing init sections in success case */
@@ -672,7 +671,18 @@ struct module *find_module(const char *name)
module_assert_mutex();
return find_module_all(name, strlen(name), false);
}
-EXPORT_SYMBOL_GPL(find_module);
+
+bool module_loaded(const char *name)
+{
+ bool ret;
+
+ mutex_lock(&module_mutex);
+ ret = !!find_module(name);
+ mutex_unlock(&module_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(module_loaded);
#ifdef CONFIG_SMP
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index e6fba1798771b4..c2e453f88bce70 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -124,9 +124,7 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
if (!p)
return true;
*p = '\0';
- mutex_lock(&module_mutex);
- ret = !!find_module(tk->symbol);
- mutex_unlock(&module_mutex);
+ ret = module_loaded(tk->symbol);
*p = ':';
return ret;
--
2.29.2
^ permalink raw reply related
* [PATCH 01/13] powerpc/powernv: remove get_cxl_module
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
The static inline get_cxl_module function is entirely unused,
remove it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/powerpc/platforms/powernv/pci-cxl.c | 22 ----------------------
1 file changed, 22 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-cxl.c b/arch/powerpc/platforms/powernv/pci-cxl.c
index 8c739c94ed28d6..53172862d23bd3 100644
--- a/arch/powerpc/platforms/powernv/pci-cxl.c
+++ b/arch/powerpc/platforms/powernv/pci-cxl.c
@@ -150,25 +150,3 @@ int pnv_cxl_ioda_msi_setup(struct pci_dev *dev, unsigned int hwirq,
return 0;
}
EXPORT_SYMBOL(pnv_cxl_ioda_msi_setup);
-
-#if IS_MODULE(CONFIG_CXL)
-static inline int get_cxl_module(void)
-{
- struct module *cxl_module;
-
- mutex_lock(&module_mutex);
-
- cxl_module = find_module("cxl");
- if (cxl_module)
- __module_get(cxl_module);
-
- mutex_unlock(&module_mutex);
-
- if (!cxl_module)
- return -ENODEV;
-
- return 0;
-}
-#else
-static inline int get_cxl_module(void) { return 0; }
-#endif
--
2.29.2
^ permalink raw reply related
* module loader dead code removal and cleanusp
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
Hi all,
this series removes support for long term unused export types and
cleans up various loose ends in the module loader.
Diffstat:
arch/arm/configs/bcm2835_defconfig | 1
arch/arm/configs/mxs_defconfig | 1
arch/mips/configs/nlm_xlp_defconfig | 1
arch/mips/configs/nlm_xlr_defconfig | 1
arch/parisc/configs/generic-32bit_defconfig | 1
arch/parisc/configs/generic-64bit_defconfig | 1
arch/powerpc/configs/ppc6xx_defconfig | 1
arch/powerpc/platforms/powernv/pci-cxl.c | 22 -
arch/s390/configs/debug_defconfig | 1
arch/s390/configs/defconfig | 1
arch/sh/configs/edosk7760_defconfig | 1
arch/sh/configs/sdk7780_defconfig | 1
arch/x86/configs/i386_defconfig | 1
arch/x86/configs/x86_64_defconfig | 1
arch/x86/tools/relocs.c | 4
drivers/gpu/drm/drm_crtc_helper_internal.h | 10
drivers/gpu/drm/drm_fb_helper.c | 21 -
drivers/gpu/drm/drm_kms_helper_common.c | 26 +-
include/asm-generic/vmlinux.lds.h | 42 ---
include/linux/export.h | 9
include/linux/kallsyms.h | 17 -
include/linux/module.h | 42 ---
init/Kconfig | 17 -
kernel/kallsyms.c | 8
kernel/livepatch/core.c | 61 +----
kernel/module.c | 319 ++++++++++------------------
kernel/trace/trace_kprobe.c | 4
lib/bug.c | 3
scripts/checkpatch.pl | 6
scripts/mod/modpost.c | 50 ----
scripts/mod/modpost.h | 3
scripts/module.lds.S | 6
tools/include/linux/export.h | 3
33 files changed, 181 insertions(+), 505 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/2] crypto: talitos - Work around SEC6 ERRATA (AES-CTR mode data size error)
From: Ard Biesheuvel @ 2021-01-21 7:31 UTC (permalink / raw)
To: Christophe Leroy
Cc: Linux Crypto Mailing List, Linux Kernel Mailing List,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Herbert Xu,
David S. Miller
In-Reply-To: <6b804eff-bc9f-5e05-d479-f398de4e2b30@csgroup.eu>
On Thu, 21 Jan 2021 at 06:35, Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 20/01/2021 à 23:23, Ard Biesheuvel a écrit :
> > On Wed, 20 Jan 2021 at 19:59, Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >>
> >> Talitos Security Engine AESU considers any input
> >> data size that is not a multiple of 16 bytes to be an error.
> >> This is not a problem in general, except for Counter mode
> >> that is a stream cipher and can have an input of any size.
> >>
> >> Test Manager for ctr(aes) fails on 4th test vector which has
> >> a length of 499 while all previous vectors which have a 16 bytes
> >> multiple length succeed.
> >>
> >> As suggested by Freescale, round up the input data length to the
> >> nearest 16 bytes.
> >>
> >> Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> >
> > Doesn't this cause the hardware to write outside the given buffer?
>
>
> Only the input length is modified. Not the output length.
>
> The ERRATA says:
>
> The input data length (in the descriptor) can be rounded up to the nearest 16B. Set the
> data-in length (in the descriptor) to include X bytes of data beyond the payload. Set the
> data-out length to only output the relevant payload (don't need to output the padding).
> SEC reads from memory are not destructive, so the extra bytes included in the AES-CTR
> operation can be whatever bytes are contiguously trailing the payload.
So what happens if the input is not 16 byte aligned, and rounding it
up causes it to extend across a page boundary into a page that is not
mapped by the IOMMU/SMMU?
^ permalink raw reply
* Re: [PATCH] powerpc: fix AKEBONO build failures
From: Yury Norov @ 2021-01-21 7:17 UTC (permalink / raw)
To: Randy Dunlap; +Cc: linuxppc-dev, Linux Kernel Mailing List, Paul Mackerras
In-Reply-To: <6c442012-3bef-321b-bbc3-09c54608661f@infradead.org>
On Wed, Jan 20, 2021 at 10:10 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 1/20/21 1:29 PM, Yury Norov wrote:
> > Hi all,
> >
> > I found the power pc build broken on today's
> > linux-next (647060f3b592).
>
> Darn, I was building linux-5.11-rc4.
>
> I'll try linux-next after I send this.
>
> ---
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Fulfill AKEBONO Kconfig requirements.
>
> Fixes these Kconfig warnings (and more) and fixes the subsequent
> build errors:
>
> WARNING: unmet direct dependencies detected for NETDEVICES
> Depends on [n]: NET [=n]
> Selected by [y]:
> - AKEBONO [=y] && PPC_47x [=y]
>
> WARNING: unmet direct dependencies detected for MMC_SDHCI
> Depends on [n]: MMC [=n] && HAS_DMA [=y]
> Selected by [y]:
> - AKEBONO [=y] && PPC_47x [=y]
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Yury Norov <yury.norov@gmail.com>
> ---
> arch/powerpc/platforms/44x/Kconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- lnx-511-rc4.orig/arch/powerpc/platforms/44x/Kconfig
> +++ lnx-511-rc4/arch/powerpc/platforms/44x/Kconfig
> @@ -206,6 +206,7 @@ config AKEBONO
> select PPC4xx_HSTA_MSI
> select I2C
> select I2C_IBM_IIC
> + select NET
> select NETDEVICES
> select ETHERNET
> select NET_VENDOR_IBM
> @@ -213,6 +214,7 @@ config AKEBONO
> select USB if USB_SUPPORT
> select USB_OHCI_HCD_PLATFORM if USB_OHCI_HCD
> select USB_EHCI_HCD_PLATFORM if USB_EHCI_HCD
> + select MMC
> select MMC_SDHCI
> select MMC_SDHCI_PLTFM
> select ATA
Looks working, thanks.
Tested-by: Yury Norov <yury.norov@gmail.com>
^ permalink raw reply
* [PATCH net] ibmvnic: device remove has higher precedence over reset
From: Lijun Pan @ 2021-01-21 6:20 UTC (permalink / raw)
To: netdev
Cc: Lijun Pan, gregkh, julietk, Uwe Kleine-König, paulus, kernel,
drt, kuba, sukadev, linuxppc-dev, davem
Returning -EBUSY in ibmvnic_remove() does not actually hold the
removal procedure since driver core doesn't care for the return
value (see __device_release_driver() in drivers/base/dd.c
calling dev->bus->remove()) though vio_bus_remove
(in arch/powerpc/platforms/pseries/vio.c) records the
return value and passes it on. [1]
During the device removal precedure, we should not schedule
any new reset (ibmvnic_reset check for REMOVING and exit),
and should rely on the flush_work and flush_delayed_work
to complete the pending resets, specifically we need to
let __ibmvnic_reset() keep running while in REMOVING state since
flush_work and flush_delayed_work shall call __ibmvnic_reset finally.
So we skip the checking for REMOVING in __ibmvnic_reset.
[1] https://lore.kernel.org/linuxppc-dev/20210117101242.dpwayq6wdgfdzirl@pengutronix.de/T/#m48f5befd96bc9842ece2a3ad14f4c27747206a53
Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Fixes: 7d7195a026ba ("ibmvnic: Do not process device remove during device reset")
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
v1 versus RFC:
1/ articulate why remove the REMOVING checking in __ibmvnic_reset
and why keep the current checking for REMOVING in ibmvnic_reset.
2/ The locking issue mentioned by Uwe are being addressed separately
by https://lists.openwall.net/netdev/2021/01/08/89
3/ This patch does not have merge conflict with 2/
drivers/net/ethernet/ibm/ibmvnic.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index aed985e08e8a..11f28fd03057 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2235,8 +2235,7 @@ static void __ibmvnic_reset(struct work_struct *work)
while (rwi) {
spin_lock_irqsave(&adapter->state_lock, flags);
- if (adapter->state == VNIC_REMOVING ||
- adapter->state == VNIC_REMOVED) {
+ if (adapter->state == VNIC_REMOVED) {
spin_unlock_irqrestore(&adapter->state_lock, flags);
kfree(rwi);
rc = EBUSY;
@@ -5372,11 +5371,6 @@ static int ibmvnic_remove(struct vio_dev *dev)
unsigned long flags;
spin_lock_irqsave(&adapter->state_lock, flags);
- if (test_bit(0, &adapter->resetting)) {
- spin_unlock_irqrestore(&adapter->state_lock, flags);
- return -EBUSY;
- }
-
adapter->state = VNIC_REMOVING;
spin_unlock_irqrestore(&adapter->state_lock, flags);
--
2.22.0
^ permalink raw reply related
* [PATCH] powerpc: fix AKEBONO build failures
From: Randy Dunlap @ 2021-01-21 6:09 UTC (permalink / raw)
To: Yury Norov, linuxppc-dev, Linux Kernel Mailing List
Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <CAAH8bW8-6Dp29fe6rrnA4eL1vo+mu0HuAVJ-5yjbwxDSvaHdeQ@mail.gmail.com>
On 1/20/21 1:29 PM, Yury Norov wrote:
> Hi all,
>
> I found the power pc build broken on today's
> linux-next (647060f3b592).
Darn, I was building linux-5.11-rc4.
I'll try linux-next after I send this.
---
From: Randy Dunlap <rdunlap@infradead.org>
Fulfill AKEBONO Kconfig requirements.
Fixes these Kconfig warnings (and more) and fixes the subsequent
build errors:
WARNING: unmet direct dependencies detected for NETDEVICES
Depends on [n]: NET [=n]
Selected by [y]:
- AKEBONO [=y] && PPC_47x [=y]
WARNING: unmet direct dependencies detected for MMC_SDHCI
Depends on [n]: MMC [=n] && HAS_DMA [=y]
Selected by [y]:
- AKEBONO [=y] && PPC_47x [=y]
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Yury Norov <yury.norov@gmail.com>
---
arch/powerpc/platforms/44x/Kconfig | 2 ++
1 file changed, 2 insertions(+)
--- lnx-511-rc4.orig/arch/powerpc/platforms/44x/Kconfig
+++ lnx-511-rc4/arch/powerpc/platforms/44x/Kconfig
@@ -206,6 +206,7 @@ config AKEBONO
select PPC4xx_HSTA_MSI
select I2C
select I2C_IBM_IIC
+ select NET
select NETDEVICES
select ETHERNET
select NET_VENDOR_IBM
@@ -213,6 +214,7 @@ config AKEBONO
select USB if USB_SUPPORT
select USB_OHCI_HCD_PLATFORM if USB_OHCI_HCD
select USB_EHCI_HCD_PLATFORM if USB_EHCI_HCD
+ select MMC
select MMC_SDHCI
select MMC_SDHCI_PLTFM
select ATA
^ permalink raw reply
* Re: [PATCH 1/2] crypto: talitos - Work around SEC6 ERRATA (AES-CTR mode data size error)
From: Christophe Leroy @ 2021-01-21 5:35 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linux Crypto Mailing List, Linux Kernel Mailing List,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Herbert Xu,
David S. Miller
In-Reply-To: <CAMj1kXE7B05eAnR7KoDCym09Cw5qnzrV8KfNT2zJrko+mFic+w@mail.gmail.com>
Le 20/01/2021 à 23:23, Ard Biesheuvel a écrit :
> On Wed, 20 Jan 2021 at 19:59, Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>> Talitos Security Engine AESU considers any input
>> data size that is not a multiple of 16 bytes to be an error.
>> This is not a problem in general, except for Counter mode
>> that is a stream cipher and can have an input of any size.
>>
>> Test Manager for ctr(aes) fails on 4th test vector which has
>> a length of 499 while all previous vectors which have a 16 bytes
>> multiple length succeed.
>>
>> As suggested by Freescale, round up the input data length to the
>> nearest 16 bytes.
>>
>> Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>
> Doesn't this cause the hardware to write outside the given buffer?
Only the input length is modified. Not the output length.
The ERRATA says:
The input data length (in the descriptor) can be rounded up to the nearest 16B. Set the
data-in length (in the descriptor) to include X bytes of data beyond the payload. Set the
data-out length to only output the relevant payload (don't need to output the padding).
SEC reads from memory are not destructive, so the extra bytes included in the AES-CTR
operation can be whatever bytes are contiguously trailing the payload.
^ permalink raw reply
* Re: [PATCH v3] [PATCH] powerpc/sstep: Check ISA 3.0 instruction validity before emulation
From: Sandipan Das @ 2021-01-21 4:36 UTC (permalink / raw)
To: Ananth N Mavinakayanahalli; +Cc: naveen.n.rao, ravi.bangoria, linuxppc-dev, dja
In-Reply-To: <161114113785.214433.12934683302522893921.stgit@thinktux.local>
On 20/01/21 4:43 pm, Ananth N Mavinakayanahalli wrote:
> We currently unconditionally try to emulate newer instructions on older
> Power versions that could cause issues. Gate it.
>
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@linux.ibm.com>
> ---
>
> [v3] Addressed Naveen's comments on scv and addpcis
> [v2] Fixed description
>
> arch/powerpc/lib/sstep.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 44 insertions(+), 2 deletions(-)
>
Reviewed-by: Sandipan Das <sandipan@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v5 00/21] ibmvfc: initial MQ development/enablement
From: Martin K. Petersen @ 2021-01-21 3:34 UTC (permalink / raw)
To: james.bottomley, Tyrel Datwyler
Cc: brking, linuxppc-dev, linux-kernel, Martin K . Petersen,
linux-scsi
In-Reply-To: <20210114203148.246656-1-tyreld@linux.ibm.com>
On Thu, 14 Jan 2021 14:31:27 -0600, Tyrel Datwyler wrote:
> Recent updates in pHyp Firmware and VIOS releases provide new infrastructure
> towards enabling Subordinate Command Response Queues (Sub-CRQs) such that each
> Sub-CRQ is a channel backed by an actual hardware queue in the FC stack on the
> partner VIOS. Sub-CRQs are registered with the firmware via hypercalls and then
> negotiated with the VIOS via new Management Datagrams (MADs) for channel setup.
>
> This initial implementation adds the necessary Sub-CRQ framework and implements
> the new MADs for negotiating and assigning a set of Sub-CRQs to associated VIOS
> HW backed channels.
>
> [...]
Applied to 5.12/scsi-queue, thanks!
[01/21] ibmvfc: add vhost fields and defaults for MQ enablement
https://git.kernel.org/mkp/scsi/c/6ae208e5d2db
[02/21] ibmvfc: move event pool init/free routines
https://git.kernel.org/mkp/scsi/c/225acf5f1aba
[03/21] ibmvfc: init/free event pool during queue allocation/free
https://git.kernel.org/mkp/scsi/c/003d91a1393d
[04/21] ibmvfc: add size parameter to ibmvfc_init_event_pool
https://git.kernel.org/mkp/scsi/c/bb35ecb2a949
[05/21] ibmvfc: define hcall wrapper for registering a Sub-CRQ
https://git.kernel.org/mkp/scsi/c/9e6b6b81aafa
[06/21] ibmvfc: add Subordinate CRQ definitions
https://git.kernel.org/mkp/scsi/c/6d07f129dce2
[07/21] ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels
https://git.kernel.org/mkp/scsi/c/3034ebe26389
[08/21] ibmvfc: add Sub-CRQ IRQ enable/disable routine
https://git.kernel.org/mkp/scsi/c/d20046e64c09
[09/21] ibmvfc: add handlers to drain and complete Sub-CRQ responses
https://git.kernel.org/mkp/scsi/c/1d956ad853fc
[10/21] ibmvfc: define Sub-CRQ interrupt handler routine
https://git.kernel.org/mkp/scsi/c/80a9e8eaed63
[11/21] ibmvfc: map/request irq and register Sub-CRQ interrupt handler
https://git.kernel.org/mkp/scsi/c/39e461fddff0
[12/21] ibmvfc: implement channel enquiry and setup commands
https://git.kernel.org/mkp/scsi/c/e95eef3fc0bc
[13/21] ibmvfc: advertise client support for using hardware channels
https://git.kernel.org/mkp/scsi/c/c53408baa502
[14/21] ibmvfc: set and track hw queue in ibmvfc_event struct
https://git.kernel.org/mkp/scsi/c/cb72477be729
[15/21] ibmvfc: send commands down HW Sub-CRQ when channelized
https://git.kernel.org/mkp/scsi/c/31750fbd7b6d
[16/21] ibmvfc: register Sub-CRQ handles with VIOS during channel setup
https://git.kernel.org/mkp/scsi/c/b88a5d9b7f56
[17/21] ibmvfc: add cancel mad initialization helper
https://git.kernel.org/mkp/scsi/c/a61236da7f9c
[18/21] ibmvfc: send Cancel MAD down each hw scsi channel
https://git.kernel.org/mkp/scsi/c/a835f386f970
[19/21] ibmvfc: purge scsi channels after transport loss/reset
https://git.kernel.org/mkp/scsi/c/7eb3ccd884ae
[20/21] ibmvfc: enable MQ and set reasonable defaults
https://git.kernel.org/mkp/scsi/c/9000cb998bcf
[21/21] ibmvfc: provide modules parameters for MQ settings
https://git.kernel.org/mkp/scsi/c/032d1900869f
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH][next] scsi: ibmvfc: Fix spelling mistake "succeded" -> "succeeded"
From: Martin K. Petersen @ 2021-01-21 2:42 UTC (permalink / raw)
To: Colin King
Cc: Tyrel Datwyler, linux-scsi, Martin K . Petersen,
James E . J . Bottomley, kernel-janitors, linux-kernel,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20210118111346.70798-1-colin.king@canonical.com>
Colin,
> There is a spelling mistake in a ibmvfc_dbg debug message. Fix it.
Applied to 5.12/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool
From: Robin Murphy @ 2021-01-21 1:09 UTC (permalink / raw)
To: Rob Herring
Cc: Heikki Krogerus, Peter Zijlstra, Grant Likely, Paul Mackerras,
Frank Rowand, Ingo Molnar, Marek Szyprowski, Stefano Stabellini,
Saravana Kannan, Heinrich Schuchardt, Joerg Roedel,
Wysocki, Rafael J, Christoph Hellwig, Bartosz Golaszewski,
xen-devel, Thierry Reding, devicetree, Will Deacon,
Konrad Rzeszutek Wilk, Dan Williams, Nicolas Boichat,
Claire Chang, Boris Ostrovsky, Andy Shevchenko, Juergen Gross,
Greg Kroah-Hartman, Randy Dunlap, linux-kernel@vger.kernel.org,
Tomasz Figa, Linux IOMMU, linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <CAL_JsqKjTqcCbCLksRbCh7=f-A3Y09A3jNqtUApaA+p=RKd_Eg@mail.gmail.com>
On 2021-01-20 21:31, Rob Herring wrote:
> On Wed, Jan 20, 2021 at 11:30 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2021-01-20 16:53, Rob Herring wrote:
>>> On Wed, Jan 06, 2021 at 11:41:23AM +0800, Claire Chang wrote:
>>>> Introduce the new compatible string, restricted-dma-pool, for restricted
>>>> DMA. One can specify the address and length of the restricted DMA memory
>>>> region by restricted-dma-pool in the device tree.
>>>
>>> If this goes into DT, I think we should be able to use dma-ranges for
>>> this purpose instead. Normally, 'dma-ranges' is for physical bus
>>> restrictions, but there's no reason it can't be used for policy or to
>>> express restrictions the firmware has enabled.
>>
>> There would still need to be some way to tell SWIOTLB to pick up the
>> corresponding chunk of memory and to prevent the kernel from using it
>> for anything else, though.
>
> Don't we already have that problem if dma-ranges had a very small
> range? We just get lucky because the restriction is generally much
> more RAM than needed.
Not really - if a device has a naturally tiny addressing capability that
doesn't even cover ZONE_DMA32 where the regular SWIOTLB buffer will be
allocated then it's unlikely to work well, but that's just crap system
design. Yes, memory pressure in ZONE_DMA{32} is particularly problematic
for such limited devices, but it's irrelevant to the issue at hand here.
What we have here is a device that's not allowed to see *kernel* memory
at all. It's been artificially constrained to a particular region by a
TZASC or similar, and the only data which should ever be placed in that
region is data intended for that device to see. That way if it tries to
go rogue it physically can't start slurping data intended for other
devices or not mapped for DMA at all. The bouncing is an important part
of this - I forget the title off-hand but there was an interesting paper
a few years ago which demonstrated that even with an IOMMU, streaming
DMA of in-place buffers could reveal enough adjacent data from the same
page to mount an attack on the system. Memory pressure should be
immaterial since the size of each bounce pool carveout will presumably
be tuned for the needs of the given device.
> In any case, wouldn't finding all the dma-ranges do this? We're
> already walking the tree to find the max DMA address now.
If all you can see are two "dma-ranges" properties, how do you propose
to tell that one means "this is the extent of what I can address, please
set my masks and dma-range-map accordingly and try to allocate things
where I can reach them" while the other means "take this output range
away from the page allocator and hook it up as my dedicated bounce pool,
because it is Serious Security Time"? Especially since getting that
choice wrong either way would be a Bad Thing.
Robin.
>>>> Signed-off-by: Claire Chang <tientzu@chromium.org>
>>>> ---
>>>> .../reserved-memory/reserved-memory.txt | 24 +++++++++++++++++++
>>>> 1 file changed, 24 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>>>> index e8d3096d922c..44975e2a1fd2 100644
>>>> --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>>>> +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>>>> @@ -51,6 +51,20 @@ compatible (optional) - standard definition
>>>> used as a shared pool of DMA buffers for a set of devices. It can
>>>> be used by an operating system to instantiate the necessary pool
>>>> management subsystem if necessary.
>>>> + - restricted-dma-pool: This indicates a region of memory meant to be
>>>> + used as a pool of restricted DMA buffers for a set of devices. The
>>>> + memory region would be the only region accessible to those devices.
>>>> + When using this, the no-map and reusable properties must not be set,
>>>> + so the operating system can create a virtual mapping that will be used
>>>> + for synchronization. The main purpose for restricted DMA is to
>>>> + mitigate the lack of DMA access control on systems without an IOMMU,
>>>> + which could result in the DMA accessing the system memory at
>>>> + unexpected times and/or unexpected addresses, possibly leading to data
>>>> + leakage or corruption. The feature on its own provides a basic level
>>>> + of protection against the DMA overwriting buffer contents at
>>>> + unexpected times. However, to protect against general data leakage and
>>>> + system memory corruption, the system needs to provide way to restrict
>>>> + the DMA to a predefined memory region.
>>>> - vendor specific string in the form <vendor>,[<device>-]<usage>
>>>> no-map (optional) - empty property
>>>> - Indicates the operating system must not create a virtual mapping
>>>> @@ -120,6 +134,11 @@ one for multimedia processing (named multimedia-memory@77000000, 64MiB).
>>>> compatible = "acme,multimedia-memory";
>>>> reg = <0x77000000 0x4000000>;
>>>> };
>>>> +
>>>> + restricted_dma_mem_reserved: restricted_dma_mem_reserved {
>>>> + compatible = "restricted-dma-pool";
>>>> + reg = <0x50000000 0x400000>;
>>>> + };
>>>> };
>>>>
>>>> /* ... */
>>>> @@ -138,4 +157,9 @@ one for multimedia processing (named multimedia-memory@77000000, 64MiB).
>>>> memory-region = <&multimedia_reserved>;
>>>> /* ... */
>>>> };
>>>> +
>>>> + pcie_device: pcie_device@0,0 {
>>>> + memory-region = <&restricted_dma_mem_reserved>;
>>>
>>> PCI hosts often have inbound window configurations that limit the
>>> address range and translate PCI to bus addresses. Those windows happen
>>> to be configured by dma-ranges. In any case, wouldn't you want to put
>>> the configuration in the PCI host node? Is there a usecase of
>>> restricting one PCIe device and not another?
>>
>> The general design seems to accommodate devices having their own pools
>> such that they can't even snoop on each others' transient DMA data. If
>> the interconnect had a way of wiring up, say, PCI RIDs to AMBA NSAIDs,
>> then in principle you could certainly apply that to PCI endpoints too
>> (presumably you'd also disallow them from peer-to-peer transactions at
>> the PCI level too).
>
> At least for PCI, I think we can handle this. We have the BDF in the
> 3rd address cell in dma-ranges. The Openfirmware spec says those are 0
> in the case of ranges. It doesn't talk about dma-ranges though. But I
> think we could extend it to allow for BDF. Though typically with PCIe
> every device is behind its own bridge and each bridge node can have a
> dma-ranges.
>
> Rob
>
^ permalink raw reply
* Re: [PATCH 6/6] powerpc/rtas: constrain user region allocation to RMA
From: Nathan Lynch @ 2021-01-21 0:26 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: aik, tyreld, brking, ajd, aneesh.kumar
In-Reply-To: <87czxzrel3.fsf@mpe.ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> Nathan Lynch <nathanl@linux.ibm.com> writes:
>> Michael Ellerman <mpe@ellerman.id.au> writes:
>>> Nathan Lynch <nathanl@linux.ibm.com> writes:
>>>> Memory locations passed as arguments from the OS to RTAS usually need
>>>> to be addressable in 32-bit mode and must reside in the Real Mode
>>>> Area. On PAPR guests, the RMA starts at logical address 0 and is the
>>>> first logical memory block reported in the LPAR’s device tree.
>>>>
>>>> On powerpc targets with RTAS, Linux makes available to user space a
>>>> region of memory suitable for arguments to be passed to RTAS via
>>>> sys_rtas(). This region (rtas_rmo_buf) is allocated via the memblock
>>>> API during boot in order to ensure that it satisfies the requirements
>>>> described above.
>>>>
>>>> With radix MMU, the upper limit supplied to the memblock allocation
>>>> can exceed the bounds of the first logical memory block, since
>>>> ppc64_rma_size is ULONG_MAX and RTAS_INSTANTIATE_MAX is 1GB.
>>>
>>> Why does the size of the first memory block matter for radix?
>>
>> Here is my understanding: in the platform architecture, the size of the
>> first memory block equals the RMA, regardless of the MMU mode. It just
>> so happens that when using radix, Linux can pass ibm,configure-connector
>> a work area address outside of the RMA because the allocation
>> constraints for the work area are computed differently. It would be
>> wrong of the OS to pass RTAS arguments outside of this region with hash
>> MMU as well.
>
> If that's the requirement then shouldn't we be adjusting ppc64_rma_size?
> Otherwise aren't other uses of ppc64_rma_size going to run into similar
> problems.
Not all allocations limited by ppc64_rma_size set up memory that is
passed to RTAS though, do they? e.g. emergency_stack_init and
init_fallback_flush? Those shouldn't be confined to the first LMB
unnecessarily.
That's why I'm thinking what I've written here should be generalized a
bit and placed in an early allocator function that can be used to set up
the user region and the per-cpu reentrant RTAS argument buffers
(see allocate_paca_ptrs/new_rtas_args). So far those two sites are the
only ones I'm convinced need attention.
^ permalink raw reply
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
From: Saravana Kannan @ 2021-01-20 23:58 UTC (permalink / raw)
To: Michael Walle
Cc: Rob Herring, Lorenzo Pieralisi, Roy Zang, PCI, LKML,
Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <f706c0e4b684e07635396fcf02f4c9a6@walle.cc>
On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> wrote:
>
> Am 2021-01-20 20:47, schrieb Saravana Kannan:
> > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc>
> > wrote:
> >>
> >> [RESEND, fat-fingered the buttons of my mail client and converted
> >> all CCs to BCCs :(]
> >>
> >> Am 2021-01-20 20:02, schrieb Saravana Kannan:
> >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote:
> >> >>
> >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc>
> >> >> wrote:
> >> >> >
> >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't
> >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe
> >> >> > deferral. Convert it to builtin_platform_driver().
> >> >>
> >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then
> >> >> shouldn't it be fixed or removed?
> >> >
> >> > I was actually thinking about this too. The problem with fixing
> >> > builtin_platform_driver_probe() to behave like
> >> > builtin_platform_driver() is that these probe functions could be
> >> > marked with __init. But there are also only 20 instances of
> >> > builtin_platform_driver_probe() in the kernel:
> >> > $ git grep ^builtin_platform_driver_probe | wc -l
> >> > 20
> >> >
> >> > So it might be easier to just fix them to not use
> >> > builtin_platform_driver_probe().
> >> >
> >> > Michael,
> >> >
> >> > Any chance you'd be willing to help me by converting all these to
> >> > builtin_platform_driver() and delete builtin_platform_driver_probe()?
> >>
> >> If it just moving the probe function to the _driver struct and
> >> remove the __init annotations. I could look into that.
> >
> > Yup. That's pretty much it AFAICT.
> >
> > builtin_platform_driver_probe() also makes sure the driver doesn't ask
> > for async probe, etc. But I doubt anyone is actually setting async
> > flags and still using builtin_platform_driver_probe().
>
> Hasn't module_platform_driver_probe() the same problem? And there
> are ~80 drivers which uses that.
Yeah. The biggest problem with all of these is the __init markers.
Maybe some familiar with coccinelle can help?
-Saravana
^ permalink raw reply
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
From: Michael Walle @ 2021-01-20 23:53 UTC (permalink / raw)
To: Saravana Kannan
Cc: Rob Herring, Lorenzo Pieralisi, Roy Zang, PCI, LKML,
Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <CAGETcx8eZRd1fJ3yuO_t2UXBFHObeNdv-c8oFH3mXw6zi=zOkQ@mail.gmail.com>
Am 2021-01-20 20:47, schrieb Saravana Kannan:
> On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc>
> wrote:
>>
>> [RESEND, fat-fingered the buttons of my mail client and converted
>> all CCs to BCCs :(]
>>
>> Am 2021-01-20 20:02, schrieb Saravana Kannan:
>> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote:
>> >>
>> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc>
>> >> wrote:
>> >> >
>> >> > fw_devlink will defer the probe until all suppliers are ready. We can't
>> >> > use builtin_platform_driver_probe() because it doesn't retry after probe
>> >> > deferral. Convert it to builtin_platform_driver().
>> >>
>> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then
>> >> shouldn't it be fixed or removed?
>> >
>> > I was actually thinking about this too. The problem with fixing
>> > builtin_platform_driver_probe() to behave like
>> > builtin_platform_driver() is that these probe functions could be
>> > marked with __init. But there are also only 20 instances of
>> > builtin_platform_driver_probe() in the kernel:
>> > $ git grep ^builtin_platform_driver_probe | wc -l
>> > 20
>> >
>> > So it might be easier to just fix them to not use
>> > builtin_platform_driver_probe().
>> >
>> > Michael,
>> >
>> > Any chance you'd be willing to help me by converting all these to
>> > builtin_platform_driver() and delete builtin_platform_driver_probe()?
>>
>> If it just moving the probe function to the _driver struct and
>> remove the __init annotations. I could look into that.
>
> Yup. That's pretty much it AFAICT.
>
> builtin_platform_driver_probe() also makes sure the driver doesn't ask
> for async probe, etc. But I doubt anyone is actually setting async
> flags and still using builtin_platform_driver_probe().
Hasn't module_platform_driver_probe() the same problem? And there
are ~80 drivers which uses that.
-michael
^ permalink raw reply
* Re: [PATCH 1/2] crypto: talitos - Work around SEC6 ERRATA (AES-CTR mode data size error)
From: Ard Biesheuvel @ 2021-01-20 22:23 UTC (permalink / raw)
To: Christophe Leroy
Cc: Linux Crypto Mailing List, Linux Kernel Mailing List,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Herbert Xu,
David S. Miller
In-Reply-To: <4b7a870573f485b9fea496b13c9b02d86dd97314.1611169001.git.christophe.leroy@csgroup.eu>
On Wed, 20 Jan 2021 at 19:59, Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> Talitos Security Engine AESU considers any input
> data size that is not a multiple of 16 bytes to be an error.
> This is not a problem in general, except for Counter mode
> that is a stream cipher and can have an input of any size.
>
> Test Manager for ctr(aes) fails on 4th test vector which has
> a length of 499 while all previous vectors which have a 16 bytes
> multiple length succeed.
>
> As suggested by Freescale, round up the input data length to the
> nearest 16 bytes.
>
> Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Doesn't this cause the hardware to write outside the given buffer?
> ---
> drivers/crypto/talitos.c | 28 ++++++++++++++++------------
> drivers/crypto/talitos.h | 1 +
> 2 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> index 4fd85f31630a..b656983c1ef4 100644
> --- a/drivers/crypto/talitos.c
> +++ b/drivers/crypto/talitos.c
> @@ -1093,11 +1093,12 @@ static void ipsec_esp_decrypt_hwauth_done(struct device *dev,
> */
> static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
> unsigned int offset, int datalen, int elen,
> - struct talitos_ptr *link_tbl_ptr)
> + struct talitos_ptr *link_tbl_ptr, int align)
> {
> int n_sg = elen ? sg_count + 1 : sg_count;
> int count = 0;
> int cryptlen = datalen + elen;
> + int padding = ALIGN(cryptlen, align) - cryptlen;
>
> while (cryptlen && sg && n_sg--) {
> unsigned int len = sg_dma_len(sg);
> @@ -1121,7 +1122,7 @@ static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
> offset += datalen;
> }
> to_talitos_ptr(link_tbl_ptr + count,
> - sg_dma_address(sg) + offset, len, 0);
> + sg_dma_address(sg) + offset, sg_next(sg) ? len : len + padding, 0);
> to_talitos_ptr_ext_set(link_tbl_ptr + count, 0, 0);
> count++;
> cryptlen -= len;
> @@ -1144,10 +1145,11 @@ static int talitos_sg_map_ext(struct device *dev, struct scatterlist *src,
> unsigned int len, struct talitos_edesc *edesc,
> struct talitos_ptr *ptr, int sg_count,
> unsigned int offset, int tbl_off, int elen,
> - bool force)
> + bool force, int align)
> {
> struct talitos_private *priv = dev_get_drvdata(dev);
> bool is_sec1 = has_ftr_sec1(priv);
> + int aligned_len = ALIGN(len, align);
>
> if (!src) {
> to_talitos_ptr(ptr, 0, 0, is_sec1);
> @@ -1155,22 +1157,22 @@ static int talitos_sg_map_ext(struct device *dev, struct scatterlist *src,
> }
> to_talitos_ptr_ext_set(ptr, elen, is_sec1);
> if (sg_count == 1 && !force) {
> - to_talitos_ptr(ptr, sg_dma_address(src) + offset, len, is_sec1);
> + to_talitos_ptr(ptr, sg_dma_address(src) + offset, aligned_len, is_sec1);
> return sg_count;
> }
> if (is_sec1) {
> - to_talitos_ptr(ptr, edesc->dma_link_tbl + offset, len, is_sec1);
> + to_talitos_ptr(ptr, edesc->dma_link_tbl + offset, aligned_len, is_sec1);
> return sg_count;
> }
> sg_count = sg_to_link_tbl_offset(src, sg_count, offset, len, elen,
> - &edesc->link_tbl[tbl_off]);
> + &edesc->link_tbl[tbl_off], align);
> if (sg_count == 1 && !force) {
> /* Only one segment now, so no link tbl needed*/
> copy_talitos_ptr(ptr, &edesc->link_tbl[tbl_off], is_sec1);
> return sg_count;
> }
> to_talitos_ptr(ptr, edesc->dma_link_tbl +
> - tbl_off * sizeof(struct talitos_ptr), len, is_sec1);
> + tbl_off * sizeof(struct talitos_ptr), aligned_len, is_sec1);
> to_talitos_ptr_ext_or(ptr, DESC_PTR_LNKTBL_JUMP, is_sec1);
>
> return sg_count;
> @@ -1182,7 +1184,7 @@ static int talitos_sg_map(struct device *dev, struct scatterlist *src,
> unsigned int offset, int tbl_off)
> {
> return talitos_sg_map_ext(dev, src, len, edesc, ptr, sg_count, offset,
> - tbl_off, 0, false);
> + tbl_off, 0, false, 1);
> }
>
> /*
> @@ -1251,7 +1253,7 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
>
> ret = talitos_sg_map_ext(dev, areq->src, cryptlen, edesc, &desc->ptr[4],
> sg_count, areq->assoclen, tbl_off, elen,
> - false);
> + false, 1);
>
> if (ret > 1) {
> tbl_off += ret;
> @@ -1271,7 +1273,7 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
> elen = 0;
> ret = talitos_sg_map_ext(dev, areq->dst, cryptlen, edesc, &desc->ptr[5],
> sg_count, areq->assoclen, tbl_off, elen,
> - is_ipsec_esp && !encrypt);
> + is_ipsec_esp && !encrypt, 1);
> tbl_off += ret;
>
> if (!encrypt && is_ipsec_esp) {
> @@ -1577,6 +1579,8 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
> bool sync_needed = false;
> struct talitos_private *priv = dev_get_drvdata(dev);
> bool is_sec1 = has_ftr_sec1(priv);
> + bool is_ctr = (desc->hdr & DESC_HDR_SEL0_MASK) == DESC_HDR_SEL0_AESU &&
> + (desc->hdr & DESC_HDR_MODE0_AESU_MASK) == DESC_HDR_MODE0_AESU_CTR;
>
> /* first DWORD empty */
>
> @@ -1597,8 +1601,8 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
> /*
> * cipher in
> */
> - sg_count = talitos_sg_map(dev, areq->src, cryptlen, edesc,
> - &desc->ptr[3], sg_count, 0, 0);
> + sg_count = talitos_sg_map_ext(dev, areq->src, cryptlen, edesc, &desc->ptr[3],
> + sg_count, 0, 0, 0, false, is_ctr ? 16 : 1);
> if (sg_count > 1)
> sync_needed = true;
>
> diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
> index 1469b956948a..32825119e880 100644
> --- a/drivers/crypto/talitos.h
> +++ b/drivers/crypto/talitos.h
> @@ -344,6 +344,7 @@ static inline bool has_ftr_sec1(struct talitos_private *priv)
>
> /* primary execution unit mode (MODE0) and derivatives */
> #define DESC_HDR_MODE0_ENCRYPT cpu_to_be32(0x00100000)
> +#define DESC_HDR_MODE0_AESU_MASK cpu_to_be32(0x00600000)
> #define DESC_HDR_MODE0_AESU_CBC cpu_to_be32(0x00200000)
> #define DESC_HDR_MODE0_AESU_CTR cpu_to_be32(0x00600000)
> #define DESC_HDR_MODE0_DEU_CBC cpu_to_be32(0x00400000)
> --
> 2.25.0
>
^ permalink raw reply
* Re: [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool
From: Rob Herring @ 2021-01-20 21:31 UTC (permalink / raw)
To: Robin Murphy
Cc: Heikki Krogerus, Peter Zijlstra, Grant Likely, Paul Mackerras,
Frank Rowand, Ingo Molnar, Marek Szyprowski, Stefano Stabellini,
Saravana Kannan, Heinrich Schuchardt, Joerg Roedel,
Wysocki, Rafael J, Christoph Hellwig, Bartosz Golaszewski,
xen-devel, Thierry Reding, devicetree, Will Deacon,
Konrad Rzeszutek Wilk, Dan Williams, Nicolas Boichat,
Claire Chang, Boris Ostrovsky, Andy Shevchenko, Juergen Gross,
Greg Kroah-Hartman, Randy Dunlap, linux-kernel@vger.kernel.org,
Tomasz Figa, Linux IOMMU, linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <313f8052-a591-75de-c4c2-ee9ea8f02e7f@arm.com>
On Wed, Jan 20, 2021 at 11:30 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-01-20 16:53, Rob Herring wrote:
> > On Wed, Jan 06, 2021 at 11:41:23AM +0800, Claire Chang wrote:
> >> Introduce the new compatible string, restricted-dma-pool, for restricted
> >> DMA. One can specify the address and length of the restricted DMA memory
> >> region by restricted-dma-pool in the device tree.
> >
> > If this goes into DT, I think we should be able to use dma-ranges for
> > this purpose instead. Normally, 'dma-ranges' is for physical bus
> > restrictions, but there's no reason it can't be used for policy or to
> > express restrictions the firmware has enabled.
>
> There would still need to be some way to tell SWIOTLB to pick up the
> corresponding chunk of memory and to prevent the kernel from using it
> for anything else, though.
Don't we already have that problem if dma-ranges had a very small
range? We just get lucky because the restriction is generally much
more RAM than needed.
In any case, wouldn't finding all the dma-ranges do this? We're
already walking the tree to find the max DMA address now.
> >> Signed-off-by: Claire Chang <tientzu@chromium.org>
> >> ---
> >> .../reserved-memory/reserved-memory.txt | 24 +++++++++++++++++++
> >> 1 file changed, 24 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> >> index e8d3096d922c..44975e2a1fd2 100644
> >> --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> >> +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> >> @@ -51,6 +51,20 @@ compatible (optional) - standard definition
> >> used as a shared pool of DMA buffers for a set of devices. It can
> >> be used by an operating system to instantiate the necessary pool
> >> management subsystem if necessary.
> >> + - restricted-dma-pool: This indicates a region of memory meant to be
> >> + used as a pool of restricted DMA buffers for a set of devices. The
> >> + memory region would be the only region accessible to those devices.
> >> + When using this, the no-map and reusable properties must not be set,
> >> + so the operating system can create a virtual mapping that will be used
> >> + for synchronization. The main purpose for restricted DMA is to
> >> + mitigate the lack of DMA access control on systems without an IOMMU,
> >> + which could result in the DMA accessing the system memory at
> >> + unexpected times and/or unexpected addresses, possibly leading to data
> >> + leakage or corruption. The feature on its own provides a basic level
> >> + of protection against the DMA overwriting buffer contents at
> >> + unexpected times. However, to protect against general data leakage and
> >> + system memory corruption, the system needs to provide way to restrict
> >> + the DMA to a predefined memory region.
> >> - vendor specific string in the form <vendor>,[<device>-]<usage>
> >> no-map (optional) - empty property
> >> - Indicates the operating system must not create a virtual mapping
> >> @@ -120,6 +134,11 @@ one for multimedia processing (named multimedia-memory@77000000, 64MiB).
> >> compatible = "acme,multimedia-memory";
> >> reg = <0x77000000 0x4000000>;
> >> };
> >> +
> >> + restricted_dma_mem_reserved: restricted_dma_mem_reserved {
> >> + compatible = "restricted-dma-pool";
> >> + reg = <0x50000000 0x400000>;
> >> + };
> >> };
> >>
> >> /* ... */
> >> @@ -138,4 +157,9 @@ one for multimedia processing (named multimedia-memory@77000000, 64MiB).
> >> memory-region = <&multimedia_reserved>;
> >> /* ... */
> >> };
> >> +
> >> + pcie_device: pcie_device@0,0 {
> >> + memory-region = <&restricted_dma_mem_reserved>;
> >
> > PCI hosts often have inbound window configurations that limit the
> > address range and translate PCI to bus addresses. Those windows happen
> > to be configured by dma-ranges. In any case, wouldn't you want to put
> > the configuration in the PCI host node? Is there a usecase of
> > restricting one PCIe device and not another?
>
> The general design seems to accommodate devices having their own pools
> such that they can't even snoop on each others' transient DMA data. If
> the interconnect had a way of wiring up, say, PCI RIDs to AMBA NSAIDs,
> then in principle you could certainly apply that to PCI endpoints too
> (presumably you'd also disallow them from peer-to-peer transactions at
> the PCI level too).
At least for PCI, I think we can handle this. We have the BDF in the
3rd address cell in dma-ranges. The Openfirmware spec says those are 0
in the case of ranges. It doesn't talk about dma-ranges though. But I
think we could extend it to allow for BDF. Though typically with PCIe
every device is behind its own bridge and each bridge node can have a
dma-ranges.
Rob
^ permalink raw reply
* linux-next: build failure on power pc
From: Yury Norov @ 2021-01-20 21:29 UTC (permalink / raw)
To: linuxppc-dev, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 457 bytes --]
Hi all,
I found the power pc build broken on today's
linux-next (647060f3b592).
My compiler is:
yury:linux$ powerpc-linux-gnu-gcc --version
powerpc-linux-gnu-gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
The config and error log are attached.
Thanks,
Yury
[-- Attachment #2: ppc.tar.gz --]
[-- Type: application/gzip, Size: 69105 bytes --]
^ permalink raw reply
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
From: Saravana Kannan @ 2021-01-20 19:47 UTC (permalink / raw)
To: Michael Walle
Cc: Rob Herring, Lorenzo Pieralisi, Roy Zang, PCI, LKML,
Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <CAGETcx8eZRd1fJ3yuO_t2UXBFHObeNdv-c8oFH3mXw6zi=zOkQ@mail.gmail.com>
On Wed, Jan 20, 2021 at 11:47 AM Saravana Kannan <saravanak@google.com> wrote:
>
> On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> wrote:
> >
> > [RESEND, fat-fingered the buttons of my mail client and converted
> > all CCs to BCCs :(]
> >
> > Am 2021-01-20 20:02, schrieb Saravana Kannan:
> > > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote:
> > >>
> > >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc>
> > >> wrote:
> > >> >
> > >> > fw_devlink will defer the probe until all suppliers are ready. We can't
> > >> > use builtin_platform_driver_probe() because it doesn't retry after probe
> > >> > deferral. Convert it to builtin_platform_driver().
> > >>
> > >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then
> > >> shouldn't it be fixed or removed?
> > >
> > > I was actually thinking about this too. The problem with fixing
> > > builtin_platform_driver_probe() to behave like
> > > builtin_platform_driver() is that these probe functions could be
> > > marked with __init. But there are also only 20 instances of
> > > builtin_platform_driver_probe() in the kernel:
> > > $ git grep ^builtin_platform_driver_probe | wc -l
> > > 20
> > >
> > > So it might be easier to just fix them to not use
> > > builtin_platform_driver_probe().
> > >
> > > Michael,
> > >
> > > Any chance you'd be willing to help me by converting all these to
> > > builtin_platform_driver() and delete builtin_platform_driver_probe()?
> >
> > If it just moving the probe function to the _driver struct and
> > remove the __init annotations. I could look into that.
>
> Yup. That's pretty much it AFAICT.
>
> builtin_platform_driver_probe() also makes sure the driver doesn't ask
> for async probe, etc. But I doubt anyone is actually setting async
> flags and still using builtin_platform_driver_probe().
>
And thanks for agreeing to help!
-Saravana
^ permalink raw reply
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
From: Saravana Kannan @ 2021-01-20 19:47 UTC (permalink / raw)
To: Michael Walle
Cc: Rob Herring, Lorenzo Pieralisi, Roy Zang, PCI, LKML,
Minghuan Lian, Mingkai Hu, Greg Kroah-Hartman, Bjorn Helgaas,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <c3e35b90e173b15870a859fd7001a712@walle.cc>
On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc> wrote:
>
> [RESEND, fat-fingered the buttons of my mail client and converted
> all CCs to BCCs :(]
>
> Am 2021-01-20 20:02, schrieb Saravana Kannan:
> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote:
> >>
> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc>
> >> wrote:
> >> >
> >> > fw_devlink will defer the probe until all suppliers are ready. We can't
> >> > use builtin_platform_driver_probe() because it doesn't retry after probe
> >> > deferral. Convert it to builtin_platform_driver().
> >>
> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then
> >> shouldn't it be fixed or removed?
> >
> > I was actually thinking about this too. The problem with fixing
> > builtin_platform_driver_probe() to behave like
> > builtin_platform_driver() is that these probe functions could be
> > marked with __init. But there are also only 20 instances of
> > builtin_platform_driver_probe() in the kernel:
> > $ git grep ^builtin_platform_driver_probe | wc -l
> > 20
> >
> > So it might be easier to just fix them to not use
> > builtin_platform_driver_probe().
> >
> > Michael,
> >
> > Any chance you'd be willing to help me by converting all these to
> > builtin_platform_driver() and delete builtin_platform_driver_probe()?
>
> If it just moving the probe function to the _driver struct and
> remove the __init annotations. I could look into that.
Yup. That's pretty much it AFAICT.
builtin_platform_driver_probe() also makes sure the driver doesn't ask
for async probe, etc. But I doubt anyone is actually setting async
flags and still using builtin_platform_driver_probe().
-Saravana
^ permalink raw reply
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
From: Michael Walle @ 2021-01-20 19:25 UTC (permalink / raw)
In-Reply-To: <CAGETcx86HMo=gaDdXFyJ4QQ-pGXWzw2G0J=SjC-eq4K7B1zQHg@mail.gmail.com>
Am 2021-01-20 20:02, schrieb Saravana Kannan:
> On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote:
>>
>> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc>
>> wrote:
>> >
>> > fw_devlink will defer the probe until all suppliers are ready. We can't
>> > use builtin_platform_driver_probe() because it doesn't retry after probe
>> > deferral. Convert it to builtin_platform_driver().
>>
>> If builtin_platform_driver_probe() doesn't work with fw_devlink, then
>> shouldn't it be fixed or removed?
>
> I was actually thinking about this too. The problem with fixing
> builtin_platform_driver_probe() to behave like
> builtin_platform_driver() is that these probe functions could be
> marked with __init. But there are also only 20 instances of
> builtin_platform_driver_probe() in the kernel:
> $ git grep ^builtin_platform_driver_probe | wc -l
> 20
>
> So it might be easier to just fix them to not use
> builtin_platform_driver_probe().
>
> Michael,
>
> Any chance you'd be willing to help me by converting all these to
> builtin_platform_driver() and delete builtin_platform_driver_probe()?
If it just moving the probe function to the _driver struct and
remove the __init annotations. I could look into that.
-michael
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox