[PATCH 04/13] livepatch: move klp_find_object_module to module.c
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
---
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(_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(_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(_mutex);
+ mod = find_module(name);
+ if (mod && !mod->klp_alive)
+ mod = NULL;
+ mutex_unlock(_mutex);
+
+ return mod;
+}
+#endif /* CONFIG_LIVEPATCH */
+
#ifdef CONFIG_SMP
static inline void __percpu *mod_percpu(struct module *mod)
--
2.29.2
Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit
On Thu, Jan 21, 2021 at 8:55 AM Christoph Hellwig wrote:
>
> drm_fb_helper_modinit has a lot of boilerplate for what is not very
> simple functionality. Just open code it in the only caller using
> IS_ENABLED and IS_MODULE.
>
> Signed-off-by: Christoph Hellwig
I didn't spot any dependencies with your series, should I just merge
this through drm trees? Or do you want an ack?
-Daniel
> ---
> drivers/gpu/drm/drm_crtc_helper_internal.h | 10 -
> drivers/gpu/drm/drm_fb_helper.c| 16 -
> drivers/gpu/drm/drm_kms_helper_common.c| 26 +++---
> 3 files changed, 13 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h
> b/drivers/gpu/drm/drm_crtc_helper_internal.h
> index 25ce42e799952c..61e09f8a8d0ff0 100644
> --- a/drivers/gpu/drm/drm_crtc_helper_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
> @@ -32,16 +32,6 @@
> #include
> #include
>
> -/* drm_fb_helper.c */
> -#ifdef CONFIG_DRM_FBDEV_EMULATION
> -int drm_fb_helper_modinit(void);
> -#else
> -static inline int drm_fb_helper_modinit(void)
> -{
> - return 0;
> -}
> -#endif
> -
> /* drm_dp_aux_dev.c */
> #ifdef CONFIG_DRM_DP_AUX_CHARDEV
> int drm_dp_aux_dev_init(void);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index ce6d63ca75c32a..0b9f1ae1b7864c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2499,19 +2499,3 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
> drm_client_register(_helper->client);
> }
> EXPORT_SYMBOL(drm_fbdev_generic_setup);
> -
> -/* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
> - * but the module doesn't depend on any fb console symbols. At least
> - * attempt to load fbcon to avoid leaving the system without a usable
> console.
> - */
> -int __init drm_fb_helper_modinit(void)
> -{
> -#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
> - const char name[] = "fbcon";
> -
> - if (!module_loaded(name))
> - request_module_nowait(name);
> -#endif
> - return 0;
> -}
> -EXPORT_SYMBOL(drm_fb_helper_modinit);
> diff --git a/drivers/gpu/drm/drm_kms_helper_common.c
> b/drivers/gpu/drm/drm_kms_helper_common.c
> index 221a8528c9937a..b694a7da632eae 100644
> --- a/drivers/gpu/drm/drm_kms_helper_common.c
> +++ b/drivers/gpu/drm/drm_kms_helper_common.c
> @@ -64,19 +64,19 @@ MODULE_PARM_DESC(edid_firmware,
>
> static int __init drm_kms_helper_init(void)
> {
> - int ret;
> -
> - /* Call init functions from specific kms helpers here */
> - ret = drm_fb_helper_modinit();
> - if (ret < 0)
> - goto out;
> -
> - ret = drm_dp_aux_dev_init();
> - if (ret < 0)
> - goto out;
> -
> -out:
> - return ret;
> + /*
> +* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
> +* but the module doesn't depend on any fb console symbols. At least
> +* attempt to load fbcon to avoid leaving the system without a usable
> +* console.
> +*/
> + if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
> + IS_MODULE(CONFIG_FRAMEBUFFER_CONSOLE) &&
> + !IS_ENABLED(CONFIG_EXPERT) &&
> + !module_loaded("fbcon"))
> + request_module_nowait("fbcon");
> +
> + return drm_dp_aux_dev_init();
> }
>
> static void __exit drm_kms_helper_exit(void)
> --
> 2.29.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH 09/13] module: remove each_symbol_in_section
each_symbol_in_section just contains a trivial loop over its arguments.
Just open code the loop in the two callers.
Signed-off-by: Christoph Hellwig
---
kernel/module.c | 29 +++--
1 file changed, 7 insertions(+), 22 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index d163c78ca8ed69..a9d092765c4eab 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -433,30 +433,13 @@ extern const s32 __start___kcrctab_unused_gpl[];
#define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
#endif
-static bool each_symbol_in_section(const struct symsearch *arr,
- unsigned int arrsize,
- struct module *owner,
- bool (*fn)(const struct symsearch *syms,
- struct module *owner,
- void *data),
- void *data)
-{
- unsigned int j;
-
- for (j = 0; j < arrsize; j++) {
- if (fn([j], owner, data))
- return true;
- }
-
- return false;
-}
-
/* Returns true as soon as fn returns true, otherwise false. */
static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
struct module *owner,
void *data),
void *data)
{
+ unsigned int i;
struct module *mod;
static const struct symsearch arr[] = {
{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
@@ -479,8 +462,9 @@ static bool each_symbol_section(bool (*fn)(const struct
symsearch *arr,
module_assert_mutex_or_preempt();
- if (each_symbol_in_section(arr, ARRAY_SIZE(arr), NULL, fn, data))
- return true;
+ for (i = 0; i < ARRAY_SIZE(arr); i++)
+ if (fn([i], NULL, data))
+ return true;
list_for_each_entry_rcu(mod, , list,
lockdep_is_held(_mutex)) {
@@ -509,8 +493,9 @@ static bool each_symbol_section(bool (*fn)(const struct
symsearch *arr,
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- if (each_symbol_in_section(arr, ARRAY_SIZE(arr), mod, fn, data))
- return true;
+ for (i = 0; i < ARRAY_SIZE(arr); i++)
+ if (fn([i], mod, data))
+ return true;
}
return false;
}
--
2.29.2
Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit
On Thu, Jan 21, 2021 at 09:25:40AM +0100, Daniel Vetter wrote: > On Thu, Jan 21, 2021 at 8:55 AM Christoph Hellwig wrote: > > > > drm_fb_helper_modinit has a lot of boilerplate for what is not very > > simple functionality. Just open code it in the only caller using > > IS_ENABLED and IS_MODULE. > > > > Signed-off-by: Christoph Hellwig > > I didn't spot any dependencies with your series, should I just merge > this through drm trees? Or do you want an ack? I'd prefer an ACK - module_loaded() is only introduced earlier in this series.
[PATCH 03/13] livepatch: refactor klp_init_object
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
---
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(_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(_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(>kobj, >kobj, "%s", name);
if (ret)
return ret;
--
2.29.2
[PATCH 05/13] kallsyms: refactor {,module_}kallsyms_on_each_symbol
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
---
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(_mutex);
- if (objname)
+ if (objname || !kallsyms_on_each_symbol(klp_find_callback, ))
module_kallsyms_on_each_symbol(klp_find_callback, );
- else
- kallsyms_on_each_symbol(klp_find_callback, );
- mutex_unlock(_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(_mutex);
list_for_each_entry(mod, , 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(_mutex);
+ return ret;
}
#endif /* CONFIG_KALLSYMS */
--
2.29.2
[PATCH 08/13] drm: remove drm_fb_helper_modinit
drm_fb_helper_modinit has a lot of boilerplate for what is not very
simple functionality. Just open code it in the only caller using
IS_ENABLED and IS_MODULE.
Signed-off-by: Christoph Hellwig
---
drivers/gpu/drm/drm_crtc_helper_internal.h | 10 -
drivers/gpu/drm/drm_fb_helper.c| 16 -
drivers/gpu/drm/drm_kms_helper_common.c| 26 +++---
3 files changed, 13 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h
b/drivers/gpu/drm/drm_crtc_helper_internal.h
index 25ce42e799952c..61e09f8a8d0ff0 100644
--- a/drivers/gpu/drm/drm_crtc_helper_internal.h
+++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
@@ -32,16 +32,6 @@
#include
#include
-/* drm_fb_helper.c */
-#ifdef CONFIG_DRM_FBDEV_EMULATION
-int drm_fb_helper_modinit(void);
-#else
-static inline int drm_fb_helper_modinit(void)
-{
- return 0;
-}
-#endif
-
/* drm_dp_aux_dev.c */
#ifdef CONFIG_DRM_DP_AUX_CHARDEV
int drm_dp_aux_dev_init(void);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index ce6d63ca75c32a..0b9f1ae1b7864c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2499,19 +2499,3 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
drm_client_register(_helper->client);
}
EXPORT_SYMBOL(drm_fbdev_generic_setup);
-
-/* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
- * but the module doesn't depend on any fb console symbols. At least
- * attempt to load fbcon to avoid leaving the system without a usable console.
- */
-int __init drm_fb_helper_modinit(void)
-{
-#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
- const char name[] = "fbcon";
-
- if (!module_loaded(name))
- request_module_nowait(name);
-#endif
- return 0;
-}
-EXPORT_SYMBOL(drm_fb_helper_modinit);
diff --git a/drivers/gpu/drm/drm_kms_helper_common.c
b/drivers/gpu/drm/drm_kms_helper_common.c
index 221a8528c9937a..b694a7da632eae 100644
--- a/drivers/gpu/drm/drm_kms_helper_common.c
+++ b/drivers/gpu/drm/drm_kms_helper_common.c
@@ -64,19 +64,19 @@ MODULE_PARM_DESC(edid_firmware,
static int __init drm_kms_helper_init(void)
{
- int ret;
-
- /* Call init functions from specific kms helpers here */
- ret = drm_fb_helper_modinit();
- if (ret < 0)
- goto out;
-
- ret = drm_dp_aux_dev_init();
- if (ret < 0)
- goto out;
-
-out:
- return ret;
+ /*
+* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
+* but the module doesn't depend on any fb console symbols. At least
+* attempt to load fbcon to avoid leaving the system without a usable
+* console.
+*/
+ if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
+ IS_MODULE(CONFIG_FRAMEBUFFER_CONSOLE) &&
+ !IS_ENABLED(CONFIG_EXPERT) &&
+ !module_loaded("fbcon"))
+ request_module_nowait("fbcon");
+
+ return drm_dp_aux_dev_init();
}
static void __exit drm_kms_helper_exit(void)
--
2.29.2
[PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL*
EXPORT_UNUSED_SYMBOL* is not actually used anywhere. Remove the unused functionality as we generally just remove unused code anyway. Signed-off-by: Christoph Hellwig --- 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/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 +- include/asm-generic/vmlinux.lds.h | 28 - include/linux/export.h | 8 --- include/linux/module.h | 13 init/Kconfig| 17 - kernel/module.c | 69 ++--- scripts/checkpatch.pl | 6 +- scripts/mod/modpost.c | 39 +--- scripts/mod/modpost.h | 2 - scripts/module.lds.S| 4 -- tools/include/linux/export.h| 2 - 24 files changed, 13 insertions(+), 192 deletions(-) diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig index 44ff9cd88d8161..d6c6c2e031c43a 100644 --- a/arch/arm/configs/bcm2835_defconfig +++ b/arch/arm/configs/bcm2835_defconfig @@ -177,7 +177,6 @@ CONFIG_BOOT_PRINTK_DELAY=y CONFIG_DYNAMIC_DEBUG=y CONFIG_DEBUG_INFO=y # CONFIG_ENABLE_MUST_CHECK is not set -CONFIG_UNUSED_SYMBOLS=y CONFIG_DEBUG_MEMORY_INIT=y CONFIG_LOCKUP_DETECTOR=y CONFIG_SCHED_TRACER=y diff --git a/arch/arm/configs/mxs_defconfig b/arch/arm/configs/mxs_defconfig index a9c6f32a9b1c9d..ca32446b187f5d 100644 --- a/arch/arm/configs/mxs_defconfig +++ b/arch/arm/configs/mxs_defconfig @@ -164,7 +164,6 @@ CONFIG_FONTS=y CONFIG_PRINTK_TIME=y CONFIG_DEBUG_INFO=y CONFIG_FRAME_WARN=2048 -CONFIG_UNUSED_SYMBOLS=y CONFIG_MAGIC_SYSRQ=y CONFIG_DEBUG_KERNEL=y CONFIG_SOFTLOCKUP_DETECTOR=y diff --git a/arch/mips/configs/nlm_xlp_defconfig b/arch/mips/configs/nlm_xlp_defconfig index 72a211d2d556fd..32c29061172325 100644 --- a/arch/mips/configs/nlm_xlp_defconfig +++ b/arch/mips/configs/nlm_xlp_defconfig @@ -549,7 +549,6 @@ CONFIG_PRINTK_TIME=y CONFIG_DEBUG_INFO=y # CONFIG_ENABLE_MUST_CHECK is not set CONFIG_FRAME_WARN=1024 -CONFIG_UNUSED_SYMBOLS=y CONFIG_DEBUG_MEMORY_INIT=y CONFIG_DETECT_HUNG_TASK=y CONFIG_SCHEDSTATS=y diff --git a/arch/mips/configs/nlm_xlr_defconfig b/arch/mips/configs/nlm_xlr_defconfig index 4ecb157e56d427..bf9b9244929ecd 100644 --- a/arch/mips/configs/nlm_xlr_defconfig +++ b/arch/mips/configs/nlm_xlr_defconfig @@ -500,7 +500,6 @@ CONFIG_CRC7=m CONFIG_PRINTK_TIME=y CONFIG_DEBUG_INFO=y # CONFIG_ENABLE_MUST_CHECK is not set -CONFIG_UNUSED_SYMBOLS=y CONFIG_DEBUG_MEMORY_INIT=y CONFIG_DETECT_HUNG_TASK=y CONFIG_SCHEDSTATS=y diff --git a/arch/parisc/configs/generic-32bit_defconfig b/arch/parisc/configs/generic-32bit_defconfig index 3cbcfad5f7249d..7611d48c599e01 100644 --- a/arch/parisc/configs/generic-32bit_defconfig +++ b/arch/parisc/configs/generic-32bit_defconfig @@ -22,7 +22,6 @@ CONFIG_PCI_LBA=y CONFIG_MODULES=y CONFIG_MODULE_UNLOAD=y CONFIG_MODULE_FORCE_UNLOAD=y -CONFIG_UNUSED_SYMBOLS=y # CONFIG_BLK_DEV_BSG is not set # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set CONFIG_BINFMT_MISC=m diff --git a/arch/parisc/configs/generic-64bit_defconfig b/arch/parisc/configs/generic-64bit_defconfig index 8f81fcbf04c413..53054b81461a10 100644 --- a/arch/parisc/configs/generic-64bit_defconfig +++ b/arch/parisc/configs/generic-64bit_defconfig @@ -31,7 +31,6 @@ CONFIG_MODULE_FORCE_LOAD=y CONFIG_MODULE_UNLOAD=y CONFIG_MODULE_FORCE_UNLOAD=y CONFIG_MODVERSIONS=y -CONFIG_UNUSED_SYMBOLS=y CONFIG_BLK_DEV_INTEGRITY=y CONFIG_BINFMT_MISC=m # CONFIG_COMPACTION is not set diff --git a/arch/powerpc/configs/ppc6xx_defconfig b/arch/powerpc/configs/ppc6xx_defconfig index ef09f3cce1fa85..34c3859040f9f7 100644 --- a/arch/powerpc/configs/ppc6xx_defconfig +++ b/arch/powerpc/configs/ppc6xx_defconfig @@ -1072,7 +1072,6 @@ CONFIG_NLS_ISO8859_15=m CONFIG_NLS_KOI8_R=m CONFIG_NLS_KOI8_U=m CONFIG_DEBUG_INFO=y -CONFIG_UNUSED_SYMBOLS=y CONFIG_HEADERS_INSTALL=y CONFIG_MAGIC_SYSRQ=y CONFIG_DEBUG_KERNEL=y diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig index c4f6ff98a612cd..58e54d17e3154b 100644 --- a/arch/s390/configs/debug_defconfig +++ b/arch/s390/configs/debug_defconfig @@ -71,7 +71,6 @@ CONFIG_MODULE_FORCE_UNLOAD=y CONFIG_MODVERSIONS=y CONFIG_MODULE_SRCVERSION_ALL=y CONFIG_MODULE_SIG_SHA256=y -CONFIG_UNUSED_SYMBOLS=y
[PATCH 06/13] kallsyms: only build {, module_}kallsyms_on_each_symbol when required
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
---
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(_mutex);
return ret;
}
+#endif
[PATCH 10/13] module: merge each_symbol_section into find_symbol
each_symbol_section is only called by find_symbol, so merge the two
functions.
Signed-off-by: Christoph Hellwig
---
kernel/module.c | 148 ++--
1 file changed, 69 insertions(+), 79 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index a9d092765c4eab..644dda52dae38c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -433,73 +433,6 @@ extern const s32 __start___kcrctab_unused_gpl[];
#define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
#endif
-/* Returns true as soon as fn returns true, otherwise false. */
-static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
- struct module *owner,
- void *data),
-void *data)
-{
- unsigned int i;
- struct module *mod;
- static const struct symsearch arr[] = {
- { __start___ksymtab, __stop___ksymtab, __start___kcrctab,
- NOT_GPL_ONLY, false },
- { __start___ksymtab_gpl, __stop___ksymtab_gpl,
- __start___kcrctab_gpl,
- GPL_ONLY, false },
- { __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
- __start___kcrctab_gpl_future,
- WILL_BE_GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
- { __start___ksymtab_unused, __stop___ksymtab_unused,
- __start___kcrctab_unused,
- NOT_GPL_ONLY, true },
- { __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
- __start___kcrctab_unused_gpl,
- GPL_ONLY, true },
-#endif
- };
-
- module_assert_mutex_or_preempt();
-
- for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (fn([i], NULL, data))
- return true;
-
- list_for_each_entry_rcu(mod, , list,
- lockdep_is_held(_mutex)) {
- struct symsearch arr[] = {
- { mod->syms, mod->syms + mod->num_syms, mod->crcs,
- NOT_GPL_ONLY, false },
- { mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
- mod->gpl_crcs,
- GPL_ONLY, false },
- { mod->gpl_future_syms,
- mod->gpl_future_syms + mod->num_gpl_future_syms,
- mod->gpl_future_crcs,
- WILL_BE_GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
- { mod->unused_syms,
- mod->unused_syms + mod->num_unused_syms,
- mod->unused_crcs,
- NOT_GPL_ONLY, true },
- { mod->unused_gpl_syms,
- mod->unused_gpl_syms + mod->num_unused_gpl_syms,
- mod->unused_gpl_crcs,
- GPL_ONLY, true },
-#endif
- };
-
- if (mod->state == MODULE_STATE_UNFORMED)
- continue;
-
- for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (fn([i], mod, data))
- return true;
- }
- return false;
-}
-
struct find_symbol_arg {
/* Input */
const char *name;
@@ -610,24 +543,81 @@ static const struct kernel_symbol *find_symbol(const char
*name,
bool gplok,
bool warn)
{
- struct find_symbol_arg fsa;
+ static const struct symsearch arr[] = {
+ { __start___ksymtab, __stop___ksymtab, __start___kcrctab,
+ NOT_GPL_ONLY, false },
+ { __start___ksymtab_gpl, __stop___ksymtab_gpl,
+ __start___kcrctab_gpl,
+ GPL_ONLY, false },
+ { __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
+ __start___kcrctab_gpl_future,
+ WILL_BE_GPL_ONLY, false },
+#ifdef CONFIG_UNUSED_SYMBOLS
+ { __start___ksymtab_unused, __stop___ksymtab_unused,
+ __start___kcrctab_unused,
+ NOT_GPL_ONLY, true },
+ { __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
+ __start___kcrctab_unused_gpl,
+ GPL_ONLY, true },
+#endif
+ };
+ struct find_symbol_arg fsa = {
+ .name = name,
+ .gplok = gplok,
+ .warn = warn,
+ };
+ struct module *mod;
+ unsigned int i;
+
+ module_assert_mutex_or_preempt();
+
+ for (i = 0; i < ARRAY_SIZE(arr); i++)
+ if (find_exported_symbol_in_section([i], NULL, ))
+ goto found;
- fsa.name = name;
- fsa.gplok = gplok;
- fsa.warn = warn;
+ list_for_each_entry_rcu(mod, , list,
+ lockdep_is_held(_mutex)) {
[PATCH 11/13] module: pass struct find_symbol_args to find_symbol
Simplify the calling convention by passing the find_symbol_args structure
to find_symbol instead of initializing it inside the function.
Signed-off-by: Christoph Hellwig
---
kernel/module.c | 113 ++--
1 file changed, 52 insertions(+), 61 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 644dda52dae38c..7a88b71736ff5c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -536,12 +536,7 @@ static bool find_exported_symbol_in_section(const struct
symsearch *syms,
* Find an exported symbol and return it, along with, (optional) crc and
* (optional) module which owns it. Needs preempt disabled or module_mutex.
*/
-static const struct kernel_symbol *find_symbol(const char *name,
- struct module **owner,
- const s32 **crc,
- enum mod_license *license,
- bool gplok,
- bool warn)
+static bool find_symbol(struct find_symbol_arg *fsa)
{
static const struct symsearch arr[] = {
{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
@@ -561,19 +556,14 @@ static const struct kernel_symbol *find_symbol(const char
*name,
GPL_ONLY, true },
#endif
};
- struct find_symbol_arg fsa = {
- .name = name,
- .gplok = gplok,
- .warn = warn,
- };
struct module *mod;
unsigned int i;
module_assert_mutex_or_preempt();
for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (find_exported_symbol_in_section([i], NULL, ))
- goto found;
+ if (find_exported_symbol_in_section([i], NULL, fsa))
+ return true;
list_for_each_entry_rcu(mod, , list,
lockdep_is_held(_mutex)) {
@@ -603,21 +593,12 @@ static const struct kernel_symbol *find_symbol(const char
*name,
continue;
for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (find_exported_symbol_in_section([i], mod, ))
- goto found;
+ if (find_exported_symbol_in_section([i], mod, fsa))
+ return true;
}
- pr_debug("Failed to find symbol %s\n", name);
- return NULL;
-
-found:
- if (owner)
- *owner = fsa.owner;
- if (crc)
- *crc = fsa.crc;
- if (license)
- *license = fsa.license;
- return fsa.sym;
+ pr_debug("Failed to find symbol %s\n", fsa->name);
+ return false;
}
/*
@@ -1107,12 +1088,15 @@ static inline void print_unload_info(struct seq_file
*m, struct module *mod)
void __symbol_put(const char *symbol)
{
- struct module *owner;
+ struct find_symbol_arg fsa = {
+ .name = symbol,
+ .gplok = true,
+ };
preempt_disable();
- if (!find_symbol(symbol, , NULL, NULL, true, false))
+ if (!find_symbol())
BUG();
- module_put(owner);
+ module_put(fsa.owner);
preempt_enable();
}
EXPORT_SYMBOL(__symbol_put);
@@ -1381,19 +1365,22 @@ static int check_version(const struct load_info *info,
static inline int check_modstruct_version(const struct load_info *info,
struct module *mod)
{
- const s32 *crc;
+ struct find_symbol_arg fsa = {
+ .name = "module_layout",
+ .gplok = true,
+ };
/*
* Since this should be found in kernel (which can't be removed), no
* locking is necessary -- use preempt_disable() to placate lockdep.
*/
preempt_disable();
- if (!find_symbol("module_layout", NULL, , NULL, true, false)) {
+ if (!find_symbol()) {
preempt_enable();
BUG();
}
preempt_enable();
- return check_version(info, "module_layout", mod, crc);
+ return check_version(info, "module_layout", mod, fsa.crc);
}
/* First part is kernel version, which we ignore if module has crcs. */
@@ -1487,10 +1474,11 @@ static const struct kernel_symbol
*resolve_symbol(struct module *mod,
const char *name,
char ownername[])
{
- struct module *owner;
- const struct kernel_symbol *sym;
- const s32 *crc;
- enum mod_license license;
+ struct find_symbol_arg fsa = {
+ .name = name,
+ .gplok = !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)),
+ .warn = true,
+ };
int err;
/*
@@ -1500,42 +1488,40 @@ static const struct kernel_symbol
*resolve_symbol(struct module *mod,
*/
[PATCH 07/13] module: mark module_mutex static
Except for two lockdep asserts module_mutex is only used in module.c.
Remove the two asserts given that the functions they are in are not
exported and just called from the module code, and mark module_mutex
static.
Signed-off-by: Christoph Hellwig
---
include/linux/module.h | 2 --
kernel/module.c| 2 +-
lib/bug.c | 3 ---
3 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 695f127745af10..c92c30a285144f 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -550,8 +550,6 @@ static inline unsigned long kallsyms_symbol_value(const
Elf_Sym *sym)
}
#endif
-extern struct mutex module_mutex;
-
/* FIXME: It'd be nice to isolate modules during init, too, so they
aren't used before they (may) fail. But presently too much code
(IDE & SCSI) require entry into the module during init.*/
diff --git a/kernel/module.c b/kernel/module.c
index e141e5d1d7beaf..d163c78ca8ed69 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -87,7 +87,7 @@
* 3) module_addr_min/module_addr_max.
* (delete and add uses RCU list operations).
*/
-DEFINE_MUTEX(module_mutex);
+static DEFINE_MUTEX(module_mutex);
static LIST_HEAD(modules);
/* Work queue for freeing init sections in success case */
diff --git a/lib/bug.c b/lib/bug.c
index 7103440c0ee1af..8f9d537bfb2a59 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -91,8 +91,6 @@ void module_bug_finalize(const Elf_Ehdr *hdr, const Elf_Shdr
*sechdrs,
char *secstrings;
unsigned int i;
- lockdep_assert_held(_mutex);
-
mod->bug_table = NULL;
mod->num_bugs = 0;
@@ -118,7 +116,6 @@ void module_bug_finalize(const Elf_Ehdr *hdr, const
Elf_Shdr *sechdrs,
void module_bug_cleanup(struct module *mod)
{
- lockdep_assert_held(_mutex);
list_del_rcu(>bug_list);
}
--
2.29.2
[PATCH 12/13] module: remove EXPORT_SYMBOL_GPL_FUTURE
As far as I can tell this has never been used at all, and certainly
not any time recently.
Signed-off-by: Christoph Hellwig
---
arch/x86/tools/relocs.c | 4 ++--
include/asm-generic/vmlinux.lds.h | 14 --
include/linux/export.h| 1 -
include/linux/module.h| 5 -
kernel/module.c | 17 -
scripts/mod/modpost.c | 13 +
scripts/mod/modpost.h | 1 -
scripts/module.lds.S | 2 --
tools/include/linux/export.h | 1 -
9 files changed, 3 insertions(+), 55 deletions(-)
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae58a..0d210d0e83e241 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -61,8 +61,8 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = {
"(__iommu_table|__apicdrivers|__smp_locks)(|_end)|"
"__(start|end)_pci_.*|"
"__(start|end)_builtin_fw|"
- "__(start|stop)___ksymtab(|_gpl|_unused|_unused_gpl|_gpl_future)|"
- "__(start|stop)___kcrctab(|_gpl|_unused|_unused_gpl|_gpl_future)|"
+ "__(start|stop)___ksymtab(|_gpl|_unused|_unused_gpl)|"
+ "__(start|stop)___kcrctab(|_gpl|_unused|_unused_gpl)|"
"__(start|stop)___param|"
"__(start|stop)___modver|"
"__(start|stop)___bug_table|"
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index b2b3d81b1535a5..83243506e68b00 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -495,13 +495,6 @@
__stop___ksymtab_unused_gpl = .;\
} \
\
- /* Kernel symbol table: GPL-future-only symbols */ \
- __ksymtab_gpl_future : AT(ADDR(__ksymtab_gpl_future) - LOAD_OFFSET) { \
- __start___ksymtab_gpl_future = .; \
- KEEP(*(SORT(___ksymtab_gpl_future+*))) \
- __stop___ksymtab_gpl_future = .;\
- } \
- \
/* Kernel symbol table: Normal symbols */ \
__kcrctab : AT(ADDR(__kcrctab) - LOAD_OFFSET) { \
__start___kcrctab = .; \
@@ -530,13 +523,6 @@
__stop___kcrctab_unused_gpl = .;\
} \
\
- /* Kernel symbol table: GPL-future-only symbols */ \
- __kcrctab_gpl_future : AT(ADDR(__kcrctab_gpl_future) - LOAD_OFFSET) { \
- __start___kcrctab_gpl_future = .; \
- KEEP(*(SORT(___kcrctab_gpl_future+*))) \
- __stop___kcrctab_gpl_future = .;\
- } \
- \
/* Kernel symbol table: strings */ \
__ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
diff --git a/include/linux/export.h b/include/linux/export.h
index fceb5e85571711..362b64f8d4a7c2 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -157,7 +157,6 @@ struct kernel_symbol {
#define EXPORT_SYMBOL(sym) _EXPORT_SYMBOL(sym, "")
#define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "_gpl")
-#define EXPORT_SYMBOL_GPL_FUTURE(sym) _EXPORT_SYMBOL(sym, "_gpl_future")
#define EXPORT_SYMBOL_NS(sym, ns) __EXPORT_SYMBOL(sym, "", #ns)
#define EXPORT_SYMBOL_NS_GPL(sym, ns) __EXPORT_SYMBOL(sym, "_gpl", #ns)
diff --git a/include/linux/module.h b/include/linux/module.h
index c92c30a285144f..8f4d577d4707c2 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -411,11 +411,6 @@ struct module {
bool async_probe_requested;
- /* symbols that will be GPL-only in the near future. */
- const struct kernel_symbol *gpl_future_syms;
- const s32 *gpl_future_crcs;
- unsigned int num_gpl_future_syms;
-
/* Exception table */
unsigned int num_exentries;
struct exception_table_entry *extable;
diff --git a/kernel/module.c b/kernel/module.c
index 7a88b71736ff5c..917fd1b5c95a42 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -413,11 +413,8 @@ extern const struct kernel_symbol __start___ksymtab[];
extern const struct kernel_symbol __stop___ksymtab[];
extern const struct kernel_symbol __start___ksymtab_gpl[];
Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
Hi Saravana,
On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan wrote:
> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle wrote:
> > Am 2021-01-20 20:47, schrieb Saravana Kannan:
> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle
> > > 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 wrote:
> > >> >>
> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle
> > >> >> 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?
And dropping them will increase memory usage.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 02/13] module: add a module_loaded helper
Le 21/01/2021 à 08:49, Christoph Hellwig a écrit : > 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 > --- > 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/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); Maybe module_is_loaded() would be a better name.
Re: [PATCH 01/13] powerpc/powernv: remove get_cxl_module
On 21/1/21 6:49 pm, Christoph Hellwig wrote:
The static inline get_cxl_module function is entirely unused,
remove it.
Signed-off-by: Christoph Hellwig
The one user of this was removed in 8bf6b91a5125a ("Revert
"powerpc/powernv: Add support for the cxl kernel api on the real phb").
Thanks for picking this up.
Reviewed-by: Andrew Donnellan
--
Andrew Donnellan OzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
RE: [PATCH 02/13] module: add a module_loaded helper
From: Christophe Leroy > Sent: 21 January 2021 10:00 > > Le 21/01/2021 à 08:49, Christoph Hellwig a écrit : > > 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 > > --- > > 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/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); > > Maybe module_is_loaded() would be a better name. I can't see the original patch. What is the point of the function. By the time it returns the information is stale - so mostly useless. Surely you need to use try_module_get() instead? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH 02/13] module: add a module_loaded helper
Le 21/01/2021 à 08:49, Christoph Hellwig a écrit : 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 --- 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/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); Maybe module_is_loaded() would be a better name.
Re: [PATCH 02/13] module: add a module_loaded helper
Le 21/01/2021 à 11:13, David Laight a écrit : From: Christophe Leroy Sorry I hit "Reply to the list" instead of "Reply all" Cc Christoph H. who is the author. Sent: 21 January 2021 10:00 Le 21/01/2021 à 08:49, Christoph Hellwig a écrit : 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 --- 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/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); Maybe module_is_loaded() would be a better name. I can't see the original patch. You have it there https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210121074959.31-3-...@lst.de/ What is the point of the function. By the time it returns the information is stale - so mostly useless. Surely you need to use try_module_get() instead? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit
On Thu, Jan 21, 2021 at 9:28 AM Christoph Hellwig wrote: > > On Thu, Jan 21, 2021 at 09:25:40AM +0100, Daniel Vetter wrote: > > On Thu, Jan 21, 2021 at 8:55 AM Christoph Hellwig wrote: > > > > > > drm_fb_helper_modinit has a lot of boilerplate for what is not very > > > simple functionality. Just open code it in the only caller using > > > IS_ENABLED and IS_MODULE. > > > > > > Signed-off-by: Christoph Hellwig > > > > I didn't spot any dependencies with your series, should I just merge > > this through drm trees? Or do you want an ack? > > I'd prefer an ACK - module_loaded() is only introduced earlier in this > series. I was looking for that but didn't find the hunk touching drm somehow ... Acked-by: Daniel Vetter Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 1/2] crypto: talitos - Work around SEC6 ERRATA (AES-CTR mode data size error)
Le 21/01/2021 à 08:31, Ard Biesheuvel a écrit :
On Thu, 21 Jan 2021 at 06:35, Christophe Leroy
wrote:
Le 20/01/2021 à 23:23, Ard Biesheuvel a écrit :
On Wed, 20 Jan 2021 at 19:59, Christophe Leroy
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
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?
What is the IOMMU/SMMU ?
The mpc8xx, mpc82xx and mpc83xx which embed the Talitos Security Engine don't have such thing, the
security engine uses DMA and has direct access to the memory bus for reading and writing.
Christophe
Re: [PATCH 1/2] crypto: talitos - Work around SEC6 ERRATA (AES-CTR mode data size error)
On Thu, 21 Jan 2021 at 10:54, Christophe Leroy
wrote:
>
>
>
> Le 21/01/2021 à 08:31, Ard Biesheuvel a écrit :
> > On Thu, 21 Jan 2021 at 06:35, Christophe Leroy
> > wrote:
> >>
> >>
> >>
> >> Le 20/01/2021 à 23:23, Ard Biesheuvel a écrit :
> >>> On Wed, 20 Jan 2021 at 19:59, Christophe Leroy
> >>> 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
> >>>
> >>> 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?
> >
>
> What is the IOMMU/SMMU ?
>
> The mpc8xx, mpc82xx and mpc83xx which embed the Talitos Security Engine don't
> have such thing, the
> security engine uses DMA and has direct access to the memory bus for reading
> and writing.
>
OK, good. So the only case where this could break is when the DMA
access spills over into a page that does not exist, and I suppose this
could only happen if the transfer involves a buffer located at the
very top of DRAM, right?
Re: [PATCH 1/2] crypto: talitos - Work around SEC6 ERRATA (AES-CTR mode data size error)
Le 21/01/2021 à 11:02, Ard Biesheuvel a écrit :
On Thu, 21 Jan 2021 at 10:54, Christophe Leroy
wrote:
Le 21/01/2021 à 08:31, Ard Biesheuvel a écrit :
On Thu, 21 Jan 2021 at 06:35, Christophe Leroy
wrote:
Le 20/01/2021 à 23:23, Ard Biesheuvel a écrit :
On Wed, 20 Jan 2021 at 19:59, Christophe Leroy
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
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?
What is the IOMMU/SMMU ?
The mpc8xx, mpc82xx and mpc83xx which embed the Talitos Security Engine don't
have such thing, the
security engine uses DMA and has direct access to the memory bus for reading
and writing.
OK, good. So the only case where this could break is when the DMA
access spills over into a page that does not exist, and I suppose this
could only happen if the transfer involves a buffer located at the
very top of DRAM, right?
Right.
Christophe
Re: [PATCH 5/6] powerpc/rtas: rename RTAS_RMOBUF_MAX to RTAS_USER_REGION_SIZE
Alexey Kardashevskiy writes:
> On 20/01/2021 12:17, Nathan Lynch wrote:
>> Alexey Kardashevskiy writes:
>>> On 16/01/2021 02:56, Nathan Lynch wrote:
Alexey Kardashevskiy writes:
> On 15/01/2021 09:00, Nathan Lynch wrote:
>> +#define RTAS_WORK_AREA_SIZE 4096
>> +
>> +/* Work areas allocated for user space access. */
>> +#define RTAS_USER_REGION_SIZE (RTAS_WORK_AREA_SIZE * 16)
>
> This is still 64K but no clarity why. There is 16 of something, what
> is it?
There are 16 4KB work areas in the region. I can name it
RTAS_NR_USER_WORK_AREAS or similar.
>>>
>>>
>>> Why 16? PAPR (then add "per PAPR") or we just like 16 ("should be
>>> enough")?
>>
>> PAPR doesn't know anything about the user region; it's a Linux
>> construct. It's been 64KB since pre-git days and I'm not sure what the
>> original reason is. At this point, maintaining a kernel-user ABI seems
>> like enough justification for the value.
>
> I am not arguing keeping the numbers but you are replacing one magic
> number with another and for neither it is horribly obvious where they
> came from.
When I wrote it I viewed it as changing one of the factors in (64 *
1024) to a named constant that better expresses how the region is used
and adjusting the remaining factor to arrive at the same end result. I
considered it a net improvement even if we're not sure how 64K was
arrived at in the first place, although I suspect it was chosen to
support multiple concurrent users, and to be compatible with both 4K
and 64K page sizes. Then again 64K pages came a bit after this was
introduced.
The change that introduced RTAS_RMOBUF_MAX (here renamed to
RTAS_USER_REGION_SIZE) does not explain how the value was derived:
Author: Andrew Morton
Date: Sun Jan 18 18:17:30 2004 -0800
[PATCH] ppc64: add rtas syscall, from John Rose
From: Anton Blanchard
Added RTAS syscall. Reserved lowmem rtas_rmo_buf for userspace use.
Created
"rmo_buffer" proc file to export bounds of rtas_rmo_buf.
[...]
diff --git a/include/asm-ppc64/rtas.h b/include/asm-ppc64/rtas.h
index 42a0b484077c..d9e426161044 100644
--- a/include/asm-ppc64/rtas.h
+++ b/include/asm-ppc64/rtas.h
@@ -19,6 +19,9 @@
#define RTAS_UNKNOWN_SERVICE (-1)
#define RTAS_INSTANTIATE_MAX (1UL<<30) /* Don't instantiate rtas at/above this
value */
+/* Buffer size for ppc_rtas system call. */
+#define RTAS_RMOBUF_MAX (64 * 1024)
+
The comment "Buffer size for ppc_rtas system call" (removed by my
change) is not really appropriate because 1. not all sys_rtas
invocations use the buffer, and 2. no callers use the entire buffer.
> Is 16 the max number of concurrently running sys_rtas system
> calls? Does the userspace ensure there is no more than 16?
No and no; not all calls to sys_rtas need to use a work area. However,
librtas uses record locking to arbitrate access to the user region, and
the unit of allocation is 4KB. This is a reasonable choice: many RTAS
calls which take a work area require 4KB alignment. But some do not
(ibm,get-system-parameter), and librtas conceivably could be made to
perform finer-grained allocations.
It's not the kernel's concern how librtas partitions the user region, so
I'm inclined to leave the (64 * 1024) expression alone now. Thanks for
your review.
> btw where is that userspace code? I thought
> https://github.com/power-ras/ppc64-diag.git but no. Thanks,
librtas, of which ppc64-diag and powerpc-utils are users:
https://github.com/ibm-power-utilities/librtas
Re: [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool
On Wed, Jan 20, 2021 at 7:10 PM Robin Murphy wrote:
>
> On 2021-01-20 21:31, Rob Herring wrote:
> > On Wed, Jan 20, 2021 at 11:30 AM Robin Murphy 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.
Yesterday's crap system design is today's security feature. Couldn't
this feature make crap system design work better?
> 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
May have been constrained, but that's entirely optional.
In the optional case where the setup is entirely up to the OS, I don't
think this belongs in the DT at all. Perhaps that should be solved
first.
> 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.
Either we have some heuristic based on the size or we add some hint.
The point is let's build on what we already have for defining DMA
accessible memory in DT rather than some parallel mechanism.
Rob
Re: [PATCH 6/6] powerpc/rtas: constrain user region allocation to RMA
Michael Ellerman writes: > Nathan Lynch writes: >> Alexey Kardashevskiy writes: >>> On 16/01/2021 02:38, Nathan Lynch wrote: Alexey Kardashevskiy writes: > On 15/01/2021 09:00, Nathan Lynch wrote: >> 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. (512MB is >> a common size of the first memory block according to a small sample of >> LPARs I have checked.) This leads to failures when user space invokes >> an RTAS function that uses a work area, such as >> ibm,configure-connector. >> >> Alter the determination of the upper limit for rtas_rmo_buf's >> allocation to consult the device tree directly, ensuring placement >> within the RMA regardless of the MMU in use. > > Can we tie this with RTAS (which also needs to be in RMA) and simply add > extra 64K in prom_instantiate_rtas() and advertise this address > (ALIGH_UP(rtas-base + rtas-size, PAGE_SIZE)) to the user space? We do > not need this RMO area before that point. Can you explain more about what advantage that would bring? I'm not seeing it. It's a more significant change than what I've written here. >>> >>> >>> We already allocate space for RTAS and (like RMO) it needs to be in RMA, >>> and RMO is useless without RTAS. We can reuse RTAS allocation code for >>> RMO like this: >> >> When you say RMO I assume you are referring to rtas_rmo_buf? (I don't >> think it is well-named.) > ... > > RMO (Real mode offset) is the old term we used to use to refer to what > is now called the RMA (Real mode area). There are still many references > to RMO in Linux, but they almost certainly all refer to what we now call > the RMA. Yes... but I think in this discussion Alexey was using RMO to stand in for rtas_rmo_buf, which was what I was trying to clarify. >>> May be store in the FDT as "linux,rmo-base" next to "linux,rtas-base", >>> for clarity, as sharing symbols between prom and main kernel is a bit >>> tricky. >>> >>> The benefit is that we do not do the same thing (== find 64K in RMA) >>> in 2 different ways and if the RMO allocated my way is broken - we'll >>> know it much sooner as RTAS itself will break too. >> >> Implementation details aside... I'll grant that combining the >> allocations into one in prom_init reduces some duplication in the sense >> that both are subject to the same constraints (mostly - the RTAS data >> area must not cross a 256MB boundary, while the user region may). But >> they really are distinct concerns. The RTAS private data area is >> specified in the platform architecture, the OS is obligated to allocate >> it and pass it to instantiate-rtas, etc etc. However the user region >> (rtas_rmo_buf) is purely a Linux construct which is there to support >> sys_rtas. >> >> Now, there are multiple sites in the kernel proper that must allocate >> memory suitable for passing to RTAS. Obviously there is value in >> consolidating the logic for that purpose in one place, so I'll work on >> adding that in v2. OK? > > I don't think we want to move any allocations into prom_init.c unless we > have to. > > It's best thought of as a trampoline, that runs before the kernel > proper, to transition from live OF to a flat DT environment. One thing > that must be done as part of that is instantiating RTAS, because it's > basically a runtime copy of the live OF. But any other allocs are for > Linux to handle later, IMHO. Agreed.
Re: [PATCH] powerpc/mm: Limit allocation of SWIOTLB on server machines
On Fri, Jan 08, 2021 at 09:27:01PM -0300, Thiago Jung Bauermann wrote: > > Ram Pai writes: > > > On Wed, Dec 23, 2020 at 09:06:01PM -0300, Thiago Jung Bauermann wrote: > >> > >> Hi Ram, > >> > >> Thanks for reviewing this patch. > >> > >> Ram Pai writes: > >> > >> > On Fri, Dec 18, 2020 at 03:21:03AM -0300, Thiago Jung Bauermann wrote: > >> >> On server-class POWER machines, we don't need the SWIOTLB unless we're a > >> >> secure VM. Nevertheless, if CONFIG_SWIOTLB is enabled we unconditionally > >> >> allocate it. > >> >> > >> >> In most cases this is harmless, but on a few machine configurations > >> >> (e.g., > >> >> POWER9 powernv systems with 4 GB area reserved for crashdump kernel) it > >> >> can > >> >> happen that memblock can't find a 64 MB chunk of memory for the SWIOTLB > >> >> and > >> >> fails with a scary-looking WARN_ONCE: > >> >> > >> >> [ cut here ] > >> >> memblock: bottom-up allocation failed, memory hotremove may be affected > >> >> WARNING: CPU: 0 PID: 0 at mm/memblock.c:332 > >> >> memblock_find_in_range_node+0x328/0x340 > >> >> Modules linked in: > >> >> CPU: 0 PID: 0 Comm: swapper Not tainted 5.10.0-rc2-orig+ #6 > >> >> NIP: c0442f38 LR: c0442f34 CTR: c01e0080 > >> >> REGS: c1def900 TRAP: 0700 Not tainted (5.10.0-rc2-orig+) > >> >> MSR: 92021033 CR: 2802 XER: > >> >> 2004 > >> >> CFAR: c014b7b4 IRQMASK: 1 > >> >> GPR00: c0442f34 c1defba0 c1deff00 > >> >> 0047 > >> >> GPR04: 7fff c1def828 c1def820 > >> >> > >> >> GPR08: 001ffc3e c1b75478 c1b75478 > >> >> 0001 > >> >> GPR12: 2000 c203 > >> >> > >> >> GPR16: > >> >> 0203 > >> >> GPR20: 0001 0001 > >> >> c1defc10 > >> >> GPR24: c1defc08 c1c91868 c1defc18 > >> >> c1c91890 > >> >> GPR28: 0400 > >> >> > >> >> NIP [c0442f38] memblock_find_in_range_node+0x328/0x340 > >> >> LR [c0442f34] memblock_find_in_range_node+0x324/0x340 > >> >> Call Trace: > >> >> [c1defba0] [c0442f34] > >> >> memblock_find_in_range_node+0x324/0x340 (unreliable) > >> >> [c1defc90] [c15ac088] > >> >> memblock_alloc_range_nid+0xec/0x1b0 > >> >> [c1defd40] [c15ac1f8] > >> >> memblock_alloc_internal+0xac/0x110 > >> >> [c1defda0] [c15ac4d0] memblock_alloc_try_nid+0x94/0xcc > >> >> [c1defe30] [c159c3c8] swiotlb_init+0x78/0x104 > >> >> [c1defea0] [c158378c] mem_init+0x4c/0x98 > >> >> [c1defec0] [c157457c] start_kernel+0x714/0xac8 > >> >> [c1deff90] [c000d244] start_here_common+0x1c/0x58 > >> >> Instruction dump: > >> >> 2c23 4182ffd4 ea610088 ea810090 4bfffe84 3921 3d42fff4 3c62ff60 > >> >> 3863c560 992a8bfc 4bd0881d 6000 <0fe0> ea610088 4bfffd94 > >> >> 6000 > >> >> random: get_random_bytes called from __warn+0x128/0x184 with > >> >> crng_init=0 > >> >> ---[ end trace ]--- > >> >> software IO TLB: Cannot allocate buffer > >> >> > >> >> Unless this is a secure VM the message can actually be ignored, because > >> >> the > >> >> SWIOTLB isn't needed. Therefore, let's avoid the SWIOTLB in those cases. > >> > > >> > The above warn_on is conveying a genuine warning. Should it be silenced? > >> > >> Not sure I understand your point. This patch doesn't silence the > >> warning, it avoids the problem it is warning about. > > > > Sorry, I should have explained it better. My point is... > > > > If CONFIG_SWIOTLB is enabled, it means that the kernel is > > promising the bounce buffering capability. I know, currently we > > do not have any kernel subsystems that use bounce buffers on > > non-secure-pseries-kernel or powernv-kernel. But that does not > > mean, there wont be any. In case there is such a third-party > > module needing bounce buffering, it wont be able to operate, > > because of the proposed change in your patch. > > > > Is that a good thing or a bad thing, I do not know. I will let > > the experts opine. > > Ping? Does anyone else has an opinion on this? The other option I can > think of is changing the crashkernel code to not reserve so much memory > below 4 GB. Other people are considering this option, but it's not > planned for the near future. That seems a more suitable solution regardless, but there is always the danger of not being enough or being too big. There was some autocrashkernel allocation patches going around for x86 and ARM that perhaps could be re-used? > > Also, there's a patch currently in linux-next which
Re: [PATCH] powerpc/64: prevent replayed interrupt handlers from running softirqs
Nicholas Piggin writes:
> Running softirqs enables interrupts, which can then end up recursing
> into the irq soft-mask code we're adjusting, including replaying
> interrupts itself, which might be theoretically unbounded.
>
> This abridged trace shows how this can occur:
>
> ! NIP replay_soft_interrupts
> LR interrupt_exit_kernel_prepare
> Call Trace:
> interrupt_exit_kernel_prepare (unreliable)
> interrupt_return
> --- interrupt: ea0 at __rb_reserve_next
> NIP __rb_reserve_next
> LR __rb_reserve_next
> Call Trace:
> ring_buffer_lock_reserve
> trace_function
> function_trace_call
> ftrace_call
> __do_softirq
> irq_exit
> timer_interrupt
> ! replay_soft_interrupts
> interrupt_exit_kernel_prepare
> interrupt_return
> --- interrupt: ea0 at arch_local_irq_restore
>
> Fix this by disabling bhs (softirqs) around the interrupt replay.
>
> I don't know that commit 3282a3da25bd ("powerpc/64: Implement soft
> interrupt replay in C") actually introduced the problem. Prior to that
> change, the interrupt replay seems like it should still be subect to
> this recusion, however it's done after all the irq state has been fixed
> up at the end of the replay, so it seems reasonable to fix back to this
> commit.
This seems very unhappy for me (on P9 bare metal):
[0.038571] Mountpoint-cache hash table entries: 131072 (order: 4, 1048576
bytes, linear)
[0.040194] [ cut here ]
[0.040228] WARNING: CPU: 0 PID: 0 at kernel/softirq.c:176
__local_bh_enable_ip+0x150/0x210
[0.040263] Modules linked in:
[0.040280] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
5.11.0-rc2-8-g4899f32e4f2a #1
[0.040321] NIP: c0114bc0 LR: c00172a0 CTR: c002a020
[0.040360] REGS: c177f670 TRAP: 0700 Not tainted
(5.11.0-rc2-8-g4899f32e4f2a)
[0.040410] MSR: 92021033 CR: 28000224
XER: 2004
[0.040472] CFAR: c0114ae8 IRQMASK: 3
GPR00: c00172a0 c177f910 c1783900
c0017290
GPR04: 0200 4000 0002
0001312d
GPR08: c16f3480 0202
GPR12: c002a020 c23a
GPR16:
GPR20: 0001 100051c6
0009
GPR24: 0e60 0900 0500
0a00
GPR28: 0f00 0002 0003
0200
[0.040824] NIP [c0114bc0] __local_bh_enable_ip+0x150/0x210
[0.040863] LR [c00172a0] replay_soft_interrupts+0x2e0/0x340
[0.040904] Call Trace:
[0.040926] [c177f910] [0500] 0x500 (unreliable)
[0.040962] [c177f950] [c00172a0]
replay_soft_interrupts+0x2e0/0x340
[0.041008] [c177fb50] [c0017370]
arch_local_irq_restore+0x70/0xe0
[0.041042] [c177fb80] [c0476514]
kmem_cache_alloc+0x474/0x520
[0.041066] [c177fc00] [c04e394c] __d_alloc+0x4c/0x2e0
[0.041109] [c177fc50] [c04e40ac] d_make_root+0x3c/0xa0
[0.041142] [c177fc80] [c0679ce0] ramfs_fill_super+0x80/0xb0
[0.041186] [c177fcb0] [c04c1b04] get_tree_nodev+0xb4/0x130
[0.041230] [c177fcf0] [c0679578] ramfs_get_tree+0x28/0x40
[0.041282] [c177fd10] [c04bee78] vfs_get_tree+0x48/0x120
[0.041325] [c177fd80] [c04f7fe0]
vfs_kern_mount.part.0+0xd0/0x130
[0.041368] [c177fdc0] [c1366700] mnt_init+0x1c8/0x2fc
[0.041420] [c177fe60] [c1366178] vfs_caches_init+0x110/0x138
[0.041454] [c177fee0] [c1331020] start_kernel+0x6d8/0x780
[0.041497] [c177ff90] [c000d354]
start_here_common+0x1c/0x5c8
[0.041539] Instruction dump:
[0.041555] e9490002 394a0001 9149 e90d0028 3d42ffcc 394a4730 7d0a42aa
e9490002
[0.041608] 2c28 394a 9149 4082ff30 <0fe0> 892d0988 3941
994d0988
[0.041660] irq event stamp: 555
[0.041674] hardirqs last enabled at (553): []
kmem_cache_alloc+0x4ac/0x520
[0.041707] hardirqs last disabled at (554): []
arch_local_irq_restore+0x68/0xe0
[0.041750] softirqs last enabled at (0): [<>] 0x0
[0.041778] softirqs last disabled at (555): []
replay_soft_interrupts+0x10/0x340
[0.041824] ---[ end trace aa6f9769e07e43db ]---
And lots and lots of these, or similar:
[ 14.369838] =
[ 14.369839] WARNING: suspicious RCU usage
[ 14.369841] 5.11.0-rc2-8-g4899f32e4f2a #1 Tainted: GW
[ 14.369843] -
[ 14.369844]
Re: [PATCH 02/13] module: add a module_loaded helper
On Thu, Jan 21, 2021 at 11:00:20AM +0100, Christophe Leroy wrote: > > +bool module_loaded(const char *name); > > Maybe module_is_loaded() would be a better name. Fine with me.
[PATCH] lib/sstep: Fix incorrect return from analyze_instr()
We currently just percolate the return value from analyze_instr()
to the caller of emulate_step(), especially if it is a -1.
For one particular case (opcode = 4) for instructions that
aren't currently emulated, we are returning 'should not be
single-stepped' while we should have returned 0 which says
'did not emulate, may have to single-step'.
Signed-off-by: Ananth N Mavinakayanahalli
Tested-by: Naveen N. Rao
---
arch/powerpc/lib/sstep.c | 49 +-
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 5a425a4a1d88..a3a0373843cd 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1445,34 +1445,39 @@ int analyse_instr(struct instruction_op *op, const
struct pt_regs *regs,
#ifdef __powerpc64__
case 4:
- if (!cpu_has_feature(CPU_FTR_ARCH_300))
- return -1;
-
- switch (word & 0x3f) {
- case 48:/* maddhd */
- asm volatile(PPC_MADDHD(%0, %1, %2, %3) :
-"=r" (op->val) : "r" (regs->gpr[ra]),
-"r" (regs->gpr[rb]), "r" (regs->gpr[rc]));
- goto compute_done;
+ /*
+* There are very many instructions with this primary opcode
+* introduced in the ISA as early as v2.03. However, the ones
+* we currently emulate were all introduced with ISA 3.0
+*/
+ if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+ switch (word & 0x3f) {
+ case 48:/* maddhd */
+ asm volatile(PPC_MADDHD(%0, %1, %2, %3) :
+"=r" (op->val) : "r"
(regs->gpr[ra]),
+"r" (regs->gpr[rb]), "r"
(regs->gpr[rc]));
+ goto compute_done;
- case 49:/* maddhdu */
- asm volatile(PPC_MADDHDU(%0, %1, %2, %3) :
-"=r" (op->val) : "r" (regs->gpr[ra]),
-"r" (regs->gpr[rb]), "r" (regs->gpr[rc]));
- goto compute_done;
+ case 49:/* maddhdu */
+ asm volatile(PPC_MADDHDU(%0, %1, %2, %3) :
+"=r" (op->val) : "r"
(regs->gpr[ra]),
+"r" (regs->gpr[rb]), "r"
(regs->gpr[rc]));
+ goto compute_done;
- case 51:/* maddld */
- asm volatile(PPC_MADDLD(%0, %1, %2, %3) :
-"=r" (op->val) : "r" (regs->gpr[ra]),
-"r" (regs->gpr[rb]), "r" (regs->gpr[rc]));
- goto compute_done;
+ case 51:/* maddld */
+ asm volatile(PPC_MADDLD(%0, %1, %2, %3) :
+"=r" (op->val) : "r"
(regs->gpr[ra]),
+"r" (regs->gpr[rb]), "r"
(regs->gpr[rc]));
+ goto compute_done;
+ }
}
/*
-* There are other instructions from ISA 3.0 with the same
-* primary opcode which do not have emulation support yet.
+* Rest of the instructions with this primary opcode do not
+* have emulation support yet.
*/
- return -1;
+ op->type = UNKNOWN;
+ return 0;
#endif
case 7: /* mulli */
Re: [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool
On 2021-01-21 15:48, Rob Herring wrote:
On Wed, Jan 20, 2021 at 7:10 PM Robin Murphy
wrote:
On 2021-01-20 21:31, Rob Herring wrote:
On Wed, Jan 20, 2021 at 11:30 AM Robin Murphy
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.
Yesterday's crap system design is today's security feature. Couldn't
this feature make crap system design work better?
Indeed! Say you bring out your shiny new "Strawberry Flan 4" machine
with all the latest connectivity, but tragically its PCIe can only
address 25% of the RAM. So you decide to support deploying it in two
configurations: one where it runs normally for best performance, and
another "secure" one where it dedicates that quarter of RAM as a
restricted DMA pool for any PCIe devices - that way, even if that hotel
projector you plug in turns out to be a rogue Thunderbolt endpoint, it
can never snarf your private keys off your eMMC out of the page cache.
(Yes, is is the thinnest of strawmen, but it sets the scene for the
point you raised...)
...which is that in both cases the dma-ranges will still be identical.
So how is the kernel going to know whether to steal that whole area from
memblock before anything else can allocate from it, or not?
I don't disagree that even in Claire's original intended case it would
be semantically correct to describe the hardware-firewalled region with
dma-ranges. It just turns out not to be necessary, and you're already
arguing for not adding anything in DT that doesn't need to be.
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
May have been constrained, but that's entirely optional.
In the optional case where the setup is entirely up to the OS, I
don't think this belongs in the DT at all. Perhaps that should be
solved first.
Yes! Let's definitely consider that case! Say you don't have any
security or physical limitations but want to use a bounce pool for some
device anyway because reasons (perhaps copying streaming DMA data to a
better guaranteed alignment gives an overall performance win). Now the
*only* relevant thing to communicate to the kernel is to, ahem, reserve
a large chunk of memory, and use it for this special purpose. Isn't that
literally what reserved-memory bindings are for?
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.
Either we have some heuristic based on the size or we add some hint.
The point is let's build on what we already have for defining DMA
accessible memory in DT rather than some parallel
Re: [PATCH] powerpc: fix AKEBONO build failures
Randy Dunlap writes: > 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 > > 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 > Cc: Michael Ellerman > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: linuxppc-dev@lists.ozlabs.org > Cc: Yury Norov > --- > 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 I think the problem here is too much use of select, for things that should instead be in the defconfig. The patch below results in the same result for make 44x/akebono_defconfig. Does it fix the original issue? We don't need to add ETHERNET or NET_VENDOR_IBM to the defconfig because they're both default y. cheers diff --git a/arch/powerpc/configs/44x/akebono_defconfig b/arch/powerpc/configs/44x/akebono_defconfig index 3894ba8f8ffc..6b08a85f4ce6 100644 --- a/arch/powerpc/configs/44x/akebono_defconfig +++ b/arch/powerpc/configs/44x/akebono_defconfig @@ -21,6 +21,7 @@ CONFIG_IRQ_ALL_CPUS=y # CONFIG_COMPACTION is not set # CONFIG_SUSPEND is not set CONFIG_NET=y +CONFIG_NETDEVICES=y CONFIG_PACKET=y CONFIG_UNIX=y CONFIG_INET=y @@ -98,6 +99,8 @@ CONFIG_USB_OHCI_HCD=y # CONFIG_USB_OHCI_HCD_PCI is not set CONFIG_USB_STORAGE=y CONFIG_MMC=y +CONFIG_MMC_SDHCI=y +CONFIG_MMC_SDHCI_PLTFM=y CONFIG_RTC_CLASS=y CONFIG_RTC_DRV_M41T80=y CONFIG_EXT2_FS=y diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig index 78ac6d67a935..509b329c112f 100644 --- a/arch/powerpc/platforms/44x/Kconfig +++ b/arch/powerpc/platforms/44x/Kconfig @@ -206,15 +206,10 @@ config AKEBONO select PPC4xx_HSTA_MSI select I2C select I2C_IBM_IIC - select NETDEVICES - select ETHERNET - select NET_VENDOR_IBM select IBM_EMAC_EMAC4 if IBM_EMAC 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_SDHCI - select MMC_SDHCI_PLTFM select ATA select SATA_AHCI_PLATFORM help
[powerpc:next-test] BUILD SUCCESS 534e43a737e9ad2b438eda651272f2774484b922
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test branch HEAD: 534e43a737e9ad2b438eda651272f2774484b922 powerpc/44x: Remove STDBINUTILS kconfig option elapsed time: 735m configs tested: 95 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig openrisc alldefconfig armlart_defconfig armpleb_defconfig arm assabet_defconfig sh se7619_defconfig armtrizeps4_defconfig arm pxa3xx_defconfig armmmp2_defconfig c6x dsk6455_defconfig m68kstmark2_defconfig arm cm_x300_defconfig mips ip27_defconfig m68k amcore_defconfig powerpc sequoia_defconfig mips ath25_defconfig arm orion5x_defconfig powerpc xes_mpc85xx_defconfig sh kfr2r09_defconfig mips pic32mzda_defconfig mips loongson3_defconfig s390 zfcpdump_defconfig arm aspeed_g4_defconfig mips malta_defconfig csky alldefconfig powerpc ppa8548_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386 tinyconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a001-20210121 i386 randconfig-a002-20210121 i386 randconfig-a004-20210121 i386 randconfig-a006-20210121 i386 randconfig-a005-20210121 i386 randconfig-a003-20210121 x86_64 randconfig-a002-20210121 x86_64 randconfig-a003-20210121 x86_64 randconfig-a001-20210121 x86_64 randconfig-a005-20210121 x86_64 randconfig-a006-20210121 x86_64 randconfig-a004-20210121 i386 randconfig-a013-20210121 i386 randconfig-a011-20210121 i386 randconfig-a012-20210121 i386 randconfig-a014-20210121 i386 randconfig-a015-20210121 i386 randconfig-a016-20210121 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 rhel-8.3-kbuiltin x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
[PATCH] powerpc/8xx: export 'cpm_setbrg' for modules
Fix missing export for a loadable module build:
ERROR: modpost: "cpm_setbrg" [drivers/tty/serial/cpm_uart/cpm_uart.ko]
undefined!
Fixes: 4128a89ac80d ("powerpc/8xx: move CPM1 related files from sysdev/ to
platforms/8xx")
Signed-off-by: Randy Dunlap
Reported-by: kernel test robot
Cc: Nick Desaulniers
Cc: clang-built-li...@googlegroups.com
Cc: Andrew Morton
Cc: Christophe Leroy
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/platforms/8xx/cpm1.c |1 +
1 file changed, 1 insertion(+)
--- linux-next-20210121.orig/arch/powerpc/platforms/8xx/cpm1.c
+++ linux-next-20210121/arch/powerpc/platforms/8xx/cpm1.c
@@ -280,6 +280,7 @@ cpm_setbrg(uint brg, uint rate)
out_be32(bp, (((BRG_UART_CLK_DIV16 / rate) - 1) << 1) |
CPM_BRG_EN | CPM_BRG_DIV16);
}
+EXPORT_SYMBOL(cpm_setbrg);
struct cpm_ioport16 {
__be16 dir, par, odr_sor, dat, intr;
[powerpc:fixes] BUILD SUCCESS 08685be7761d69914f08c3d6211c543a385a5b9c
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes branch HEAD: 08685be7761d69914f08c3d6211c543a385a5b9c powerpc/64s: fix scv entry fallback flush vs interrupt elapsed time: 735m configs tested: 91 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig powerpc mpc8313_rdb_defconfig arm sama5_defconfig mipsar7_defconfig m68k m5275evb_defconfig arm h5000_defconfig sh se7619_defconfig h8300 h8s-sim_defconfig mipsmaltaup_xpa_defconfig sparc64 alldefconfig armcerfcube_defconfig m68kstmark2_defconfig powerpc xes_mpc85xx_defconfig sh kfr2r09_defconfig mips pic32mzda_defconfig mips loongson3_defconfig arm imx_v4_v5_defconfig arm eseries_pxa_defconfig mipsjmr3927_defconfig sh sh7770_generic_defconfig arm tango4_defconfig arm omap1_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386 tinyconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a001-20210121 i386 randconfig-a002-20210121 i386 randconfig-a004-20210121 i386 randconfig-a006-20210121 i386 randconfig-a005-20210121 i386 randconfig-a003-20210121 x86_64 randconfig-a002-20210121 x86_64 randconfig-a003-20210121 x86_64 randconfig-a001-20210121 x86_64 randconfig-a005-20210121 x86_64 randconfig-a006-20210121 x86_64 randconfig-a004-20210121 i386 randconfig-a013-20210121 i386 randconfig-a011-20210121 i386 randconfig-a012-20210121 i386 randconfig-a014-20210121 i386 randconfig-a015-20210121 i386 randconfig-a016-20210121 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 rhel-8.3-kbuiltin x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH v2 2/2] memblock: do not start bottom-up allocations with kernel_end
Mike Rapoport writes:
> > Signed-off-by: Roman Gushchin
>
> Reviewed-by: Mike Rapoport
I've seen a couple of spurious triggers of the WARN_ONCE() removed by this
patch. This happens on some ppc64le bare metal (powernv) server machines with
CONFIG_SWIOTLB=y and crashkernel=4G, as described in a candidate patch I posted
to solve this issue in a different way:
https://lore.kernel.org/linuxppc-dev/20201218062103.76102-1-bauer...@linux.ibm.com/
Since this patch solves that problem, is it possible to include it in the next
feasible v5.11-rcX, with the following tag?
Fixes: 8fabc623238e ("powerpc: Ensure that swiotlb buffer is allocated from low
memory")
This is because reverting the commit above also solves the problem on the
machines where I've seen this issue.
--
Thiago Jung Bauermann
IBM Linux Technology Center
[powerpc:fixes-test] BUILD SUCCESS 4899f32e4f2a936dc20fbfc4fde85b003387c5c2
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test branch HEAD: 4899f32e4f2a936dc20fbfc4fde85b003387c5c2 powerpc/64: prevent replayed interrupt handlers from running softirqs elapsed time: 735m configs tested: 91 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig powerpc chrp32_defconfig sh rsk7203_defconfig arm h3600_defconfig m68km5407c3_defconfig sh sdk7786_defconfig c6x dsk6455_defconfig xtensageneric_kc705_defconfig powerpcsocrates_defconfig m68k atari_defconfig arm mxs_defconfig powerpc currituck_defconfig armtrizeps4_defconfig h8300 h8s-sim_defconfig c6xevmc6678_defconfig arm tct_hammer_defconfig mips bigsur_defconfig powerpc makalu_defconfig powerpc xes_mpc85xx_defconfig sh kfr2r09_defconfig mips pic32mzda_defconfig mips loongson3_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386 tinyconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a001-20210121 i386 randconfig-a002-20210121 i386 randconfig-a004-20210121 i386 randconfig-a006-20210121 i386 randconfig-a005-20210121 i386 randconfig-a003-20210121 x86_64 randconfig-a002-20210121 x86_64 randconfig-a003-20210121 x86_64 randconfig-a001-20210121 x86_64 randconfig-a005-20210121 x86_64 randconfig-a006-20210121 x86_64 randconfig-a004-20210121 i386 randconfig-a013-20210121 i386 randconfig-a011-20210121 i386 randconfig-a012-20210121 i386 randconfig-a014-20210121 i386 randconfig-a015-20210121 i386 randconfig-a016-20210121 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 rhel-8.3-kbuiltin x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH v3] powerpc/mce: Remove per cpu variables from MCE handlers
On 1/19/21 9:28 AM, Nicholas Piggin wrote:
Excerpts from Ganesh Goudar's message of January 15, 2021 10:58 pm:
Access to per-cpu variables requires translation to be enabled on
pseries machine running in hash mmu mode, Since part of MCE handler
runs in realmode and part of MCE handling code is shared between ppc
architectures pseries and powernv, it becomes difficult to manage
these variables differently on different architectures, So have
these variables in paca instead of having them as per-cpu variables
to avoid complications.
Seems okay.
Maximum recursive depth of MCE is 4, Considering the maximum depth
allowed reduce the size of event to 10 from 100.
Could you make this a separate patch, with memory saving numbers?
"Delayed" MCEs are not necessarily the same as recursive (several
sequential MCEs can occur before the first event is processed).
But I agree 100 is pretty overboard (as is 4 recursive MCEs really).
Sure.
Signed-off-by: Ganesh Goudar
---
v2: Dynamically allocate memory for machine check event info
v3: Remove check for hash mmu lpar, use memblock_alloc_try_nid
to allocate memory.
---
arch/powerpc/include/asm/mce.h | 21 -
arch/powerpc/include/asm/paca.h| 4 ++
arch/powerpc/kernel/mce.c | 76 +-
arch/powerpc/kernel/setup-common.c | 2 +-
4 files changed, 69 insertions(+), 34 deletions(-)
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index e6c27ae843dc..8d6e3a7a9f37 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -204,7 +204,18 @@ struct mce_error_info {
boolignore_event;
};
-#define MAX_MC_EVT 100
+#define MAX_MC_EVT 10
+
+struct mce_info {
+ int mce_nest_count;
+ struct machine_check_event mce_event[MAX_MC_EVT];
+ /* Queue for delayed MCE events. */
+ int mce_queue_count;
+ struct machine_check_event mce_event_queue[MAX_MC_EVT];
+ /* Queue for delayed MCE UE events. */
+ int mce_ue_count;
+ struct machine_check_event mce_ue_event_queue[MAX_MC_EVT];
+};
/* Release flags for get_mce_event() */
#define MCE_EVENT_RELEASE true
@@ -233,5 +244,13 @@ long __machine_check_early_realmode_p7(struct pt_regs
*regs);
long __machine_check_early_realmode_p8(struct pt_regs *regs);
long __machine_check_early_realmode_p9(struct pt_regs *regs);
long __machine_check_early_realmode_p10(struct pt_regs *regs);
+#define get_mce_info() local_paca->mce_info
I don't think this adds anything. Could you open code it?
ok
Thanks,
Nick
Re: [PATCH] powerpc: fix AKEBONO build failures
On January 21, 2021 5:14:23 PM PST, Michael Ellerman wrote: >Randy Dunlap writes: >> 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 >> >> 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 >> Cc: Michael Ellerman >> Cc: Benjamin Herrenschmidt >> Cc: Paul Mackerras >> Cc: linuxppc-dev@lists.ozlabs.org >> Cc: Yury Norov >> --- >> 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 > >I think the problem here is too much use of select, for things that >should instead be in the defconfig. > >The patch below results in the same result for make >44x/akebono_defconfig. Does it fix the original issue? > >We don't need to add ETHERNET or NET_VENDOR_IBM to the defconfig >because >they're both default y. > >cheers > > >diff --git a/arch/powerpc/configs/44x/akebono_defconfig >b/arch/powerpc/configs/44x/akebono_defconfig >index 3894ba8f8ffc..6b08a85f4ce6 100644 >--- a/arch/powerpc/configs/44x/akebono_defconfig >+++ b/arch/powerpc/configs/44x/akebono_defconfig >@@ -21,6 +21,7 @@ CONFIG_IRQ_ALL_CPUS=y > # CONFIG_COMPACTION is not set > # CONFIG_SUSPEND is not set > CONFIG_NET=y >+CONFIG_NETDEVICES=y > CONFIG_PACKET=y > CONFIG_UNIX=y > CONFIG_INET=y >@@ -98,6 +99,8 @@ CONFIG_USB_OHCI_HCD=y > # CONFIG_USB_OHCI_HCD_PCI is not set > CONFIG_USB_STORAGE=y > CONFIG_MMC=y >+CONFIG_MMC_SDHCI=y >+CONFIG_MMC_SDHCI_PLTFM=y > CONFIG_RTC_CLASS=y > CONFIG_RTC_DRV_M41T80=y > CONFIG_EXT2_FS=y >diff --git a/arch/powerpc/platforms/44x/Kconfig >b/arch/powerpc/platforms/44x/Kconfig >index 78ac6d67a935..509b329c112f 100644 >--- a/arch/powerpc/platforms/44x/Kconfig >+++ b/arch/powerpc/platforms/44x/Kconfig >@@ -206,15 +206,10 @@ config AKEBONO >select PPC4xx_HSTA_MSI >select I2C >select I2C_IBM_IIC >- select NETDEVICES >- select ETHERNET >- select NET_VENDOR_IBM >select IBM_EMAC_EMAC4 if IBM_EMAC >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_SDHCI >- select MMC_SDHCI_PLTFM >select ATA >select SATA_AHCI_PLATFORM >help Sure. I thought that lots of what was already there should be in the defconfig. I was just going with the flow. Thanks for fixing it. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: [PATCH] powerpc/8xx: export 'cpm_setbrg' for modules
Le 22/01/2021 à 02:08, Randy Dunlap a écrit :
Fix missing export for a loadable module build:
ERROR: modpost: "cpm_setbrg" [drivers/tty/serial/cpm_uart/cpm_uart.ko]
undefined!
Fixes: 4128a89ac80d ("powerpc/8xx: move CPM1 related files from sysdev/ to
platforms/8xx")
I don't understand. Before that commit cpm_setbrg() wasn't exported either.
For me, it fixes the commit that brought the capability to build the cpm uart driver as a module,
that is commit 1da177e4c3f4 ("Linux-2.6.12-rc")
Signed-off-by: Randy Dunlap
Reported-by: kernel test robot
Cc: Nick Desaulniers
Cc: clang-built-li...@googlegroups.com
Cc: Andrew Morton
Cc: Christophe Leroy
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/platforms/8xx/cpm1.c |1 +
1 file changed, 1 insertion(+)
--- linux-next-20210121.orig/arch/powerpc/platforms/8xx/cpm1.c
+++ linux-next-20210121/arch/powerpc/platforms/8xx/cpm1.c
@@ -280,6 +280,7 @@ cpm_setbrg(uint brg, uint rate)
out_be32(bp, (((BRG_UART_CLK_DIV16 / rate) - 1) << 1) |
CPM_BRG_EN | CPM_BRG_DIV16);
}
+EXPORT_SYMBOL(cpm_setbrg);
struct cpm_ioport16 {
__be16 dir, par, odr_sor, dat, intr;
Re: [PATCH 04/13] livepatch: move klp_find_object_module to module.c
On Thu, Jan 21, 2021 at 08:49:50AM +0100, Christoph Hellwig wrote:
> @@ -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;
Why was this line removed?
> 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);
These comments don't make sense in this context, they should be kept
with the code in find_klp_module().
--
Josh
Re: [PATCH 1/2] ima: Free IMA measurement buffer on error
On 2021-01-21 09:30:02, Lakshmi Ramasubramanian wrote:
> IMA allocates kernel virtual memory to carry forward the measurement
> list, from the current kernel to the next kernel on kexec system call,
> in ima_add_kexec_buffer() function. In error code paths this memory
> is not freed resulting in memory leak.
>
> Free the memory allocated for the IMA measurement list in
> the error code paths in ima_add_kexec_buffer() function.
>
> Signed-off-by: Lakshmi Ramasubramanian
> Suggested-by: Tyler Hicks
> Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list")
Reviewed-by: Tyler Hicks
Tyler
> ---
> security/integrity/ima/ima_kexec.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/security/integrity/ima/ima_kexec.c
> b/security/integrity/ima/ima_kexec.c
> index 121de3e04af2..212145008a01 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -119,12 +119,14 @@ void ima_add_kexec_buffer(struct kimage *image)
> ret = kexec_add_buffer();
> if (ret) {
> pr_err("Error passing over kexec measurement buffer.\n");
> + vfree(kexec_buffer);
> return;
> }
>
> ret = arch_ima_add_kexec_buffer(image, kbuf.mem, kexec_segment_size);
> if (ret) {
> pr_err("Error passing over kexec measurement buffer.\n");
> + vfree(kexec_buffer);
> return;
> }
>
> --
> 2.30.0
>
Re: [PATCH] powerpc/64: prevent replayed interrupt handlers from running softirqs
Excerpts from Michael Ellerman's message of January 21, 2021 10:50 pm:
> Nicholas Piggin writes:
>> Running softirqs enables interrupts, which can then end up recursing
>> into the irq soft-mask code we're adjusting, including replaying
>> interrupts itself, which might be theoretically unbounded.
>>
>> This abridged trace shows how this can occur:
>>
>> ! NIP replay_soft_interrupts
>> LR interrupt_exit_kernel_prepare
>> Call Trace:
>> interrupt_exit_kernel_prepare (unreliable)
>> interrupt_return
>> --- interrupt: ea0 at __rb_reserve_next
>> NIP __rb_reserve_next
>> LR __rb_reserve_next
>> Call Trace:
>> ring_buffer_lock_reserve
>> trace_function
>> function_trace_call
>> ftrace_call
>> __do_softirq
>> irq_exit
>> timer_interrupt
>> ! replay_soft_interrupts
>> interrupt_exit_kernel_prepare
>> interrupt_return
>> --- interrupt: ea0 at arch_local_irq_restore
>>
>> Fix this by disabling bhs (softirqs) around the interrupt replay.
>>
>> I don't know that commit 3282a3da25bd ("powerpc/64: Implement soft
>> interrupt replay in C") actually introduced the problem. Prior to that
>> change, the interrupt replay seems like it should still be subect to
>> this recusion, however it's done after all the irq state has been fixed
>> up at the end of the replay, so it seems reasonable to fix back to this
>> commit.
>
> This seems very unhappy for me (on P9 bare metal):
Oh, damn lockdep I always forget it has a bunch of interrupt checks.
In this case it might have a point though. Thanks, I'll try to fix it.
Thanks,
Nick
>
> [0.038571] Mountpoint-cache hash table entries: 131072 (order: 4, 1048576
> bytes, linear)
> [0.040194] [ cut here ]
> [0.040228] WARNING: CPU: 0 PID: 0 at kernel/softirq.c:176
> __local_bh_enable_ip+0x150/0x210
> [0.040263] Modules linked in:
> [0.040280] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 5.11.0-rc2-8-g4899f32e4f2a #1
> [0.040321] NIP: c0114bc0 LR: c00172a0 CTR:
> c002a020
> [0.040360] REGS: c177f670 TRAP: 0700 Not tainted
> (5.11.0-rc2-8-g4899f32e4f2a)
> [0.040410] MSR: 92021033 CR:
> 28000224 XER: 2004
> [0.040472] CFAR: c0114ae8 IRQMASK: 3
>GPR00: c00172a0 c177f910 c1783900
> c0017290
>GPR04: 0200 4000 0002
> 0001312d
>GPR08: c16f3480 0202
>
>GPR12: c002a020 c23a
>
>GPR16:
>
>GPR20: 0001 100051c6
> 0009
>GPR24: 0e60 0900 0500
> 0a00
>GPR28: 0f00 0002 0003
> 0200
> [0.040824] NIP [c0114bc0] __local_bh_enable_ip+0x150/0x210
> [0.040863] LR [c00172a0] replay_soft_interrupts+0x2e0/0x340
> [0.040904] Call Trace:
> [0.040926] [c177f910] [0500] 0x500 (unreliable)
> [0.040962] [c177f950] [c00172a0]
> replay_soft_interrupts+0x2e0/0x340
> [0.041008] [c177fb50] [c0017370]
> arch_local_irq_restore+0x70/0xe0
> [0.041042] [c177fb80] [c0476514]
> kmem_cache_alloc+0x474/0x520
> [0.041066] [c177fc00] [c04e394c] __d_alloc+0x4c/0x2e0
> [0.041109] [c177fc50] [c04e40ac] d_make_root+0x3c/0xa0
> [0.041142] [c177fc80] [c0679ce0]
> ramfs_fill_super+0x80/0xb0
> [0.041186] [c177fcb0] [c04c1b04] get_tree_nodev+0xb4/0x130
> [0.041230] [c177fcf0] [c0679578] ramfs_get_tree+0x28/0x40
> [0.041282] [c177fd10] [c04bee78] vfs_get_tree+0x48/0x120
> [0.041325] [c177fd80] [c04f7fe0]
> vfs_kern_mount.part.0+0xd0/0x130
> [0.041368] [c177fdc0] [c1366700] mnt_init+0x1c8/0x2fc
> [0.041420] [c177fe60] [c1366178]
> vfs_caches_init+0x110/0x138
> [0.041454] [c177fee0] [c1331020] start_kernel+0x6d8/0x780
> [0.041497] [c177ff90] [c000d354]
> start_here_common+0x1c/0x5c8
> [0.041539] Instruction dump:
> [0.041555] e9490002 394a0001 9149 e90d0028 3d42ffcc 394a4730 7d0a42aa
> e9490002
> [0.041608] 2c28 394a 9149 4082ff30 <0fe0> 892d0988
> 3941 994d0988
> [0.041660] irq event stamp: 555
> [0.041674] hardirqs last enabled at (553): []
> kmem_cache_alloc+0x4ac/0x520
> [0.041707] hardirqs last disabled at (554): []
> arch_local_irq_restore+0x68/0xe0
> [0.041750] softirqs last enabled at (0): [<>] 0x0
> [0.041778]
Re: [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall
On 2021-01-21 09:30:03, Lakshmi Ramasubramanian wrote:
> IMA allocates kernel virtual memory to carry forward the measurement
> list, from the current kernel to the next kernel on kexec system call,
> in ima_add_kexec_buffer() function. This buffer is not freed before
> completing the kexec system call resulting in memory leak.
>
> Add ima_buffer field in "struct kimage" to store the virtual address
> of the buffer allocated for the IMA measurement list.
> Free the memory allocated for the IMA measurement list in
> kimage_file_post_load_cleanup() function.
>
> Signed-off-by: Lakshmi Ramasubramanian
> Suggested-by: Tyler Hicks
> Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list")
Reviewed-by: Tyler Hicks
Tyler
> ---
> include/linux/kexec.h | 5 +
> kernel/kexec_file.c| 5 +
> security/integrity/ima/ima_kexec.c | 2 ++
> 3 files changed, 12 insertions(+)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 9e93bef52968..5f61389f5f36 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -300,6 +300,11 @@ struct kimage {
> /* Information for loading purgatory */
> struct purgatory_info purgatory_info;
> #endif
> +
> +#ifdef CONFIG_IMA_KEXEC
> + /* Virtual address of IMA measurement buffer for kexec syscall */
> + void *ima_buffer;
> +#endif
> };
>
> /* kexec interface functions */
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index b02086d70492..5c3447cf7ad5 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -166,6 +166,11 @@ void kimage_file_post_load_cleanup(struct kimage *image)
> vfree(pi->sechdrs);
> pi->sechdrs = NULL;
>
> +#ifdef CONFIG_IMA_KEXEC
> + vfree(image->ima_buffer);
> + image->ima_buffer = NULL;
> +#endif /* CONFIG_IMA_KEXEC */
> +
> /* See if architecture has anything to cleanup post load */
> arch_kimage_file_post_load_cleanup(image);
>
> diff --git a/security/integrity/ima/ima_kexec.c
> b/security/integrity/ima/ima_kexec.c
> index 212145008a01..8eadd0674629 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -130,6 +130,8 @@ void ima_add_kexec_buffer(struct kimage *image)
> return;
> }
>
> + image->ima_buffer = kexec_buffer;
> +
> pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
>kbuf.mem);
> }
> --
> 2.30.0
>
Re: [PATCH net] ibmvnic: device remove has higher precedence over reset
Lijun Pan [lijunp...@gmail.com] wrote:
> > > 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(>state_lock, flags);
> > >
> > > - if (adapter->state == VNIC_REMOVING ||
> > > - adapter->state == VNIC_REMOVED) {
> > > + if (adapter->state == VNIC_REMOVED) {
If the adapter is in REMOVING state, there is no point going
through the reset process. We could just bail out here. We
should also drain any other resets in the queue (something
my other patch set was addressing).
Sukadev
> >
> > If we do get here, we would crash because ibmvnic_remove() happened. It
> > frees the adapter struct already.
>
> Not exactly. viodev is gone; netdev is gone; ibmvnic_adapter is still there.
>
> Lijun
[PATCH] powerpc/32: Use r2 in wrtspr() instead of r0
wrtspr() is a function to write an arbitrary value in a special
register. It is used on 8xx to write to SPRN_NRI, SPRN_EID and
SPRN_EIE. Writing any value to one of those will play with MSR EE
and MSR RI regardless of that value.
r0 is used many places in the generated code and using r0 for
that creates an unnecessary dependency of this instruction with
preceding ones using r0 in a few places in vmlinux.
r2 is most likely the most stable register as it contains the
pointer to 'current'.
Using r2 instead of r0 avoids that unnecessary dependency.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/reg.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index e40a921d78f9..32a9020e93ab 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1392,8 +1392,7 @@ static inline void mtmsr_isync(unsigned long val)
: "r" ((unsigned long)(v)) \
: "memory")
#endif
-#define wrtspr(rn) asm volatile("mtspr " __stringify(rn) ",0" : \
-: : "memory")
+#define wrtspr(rn) asm volatile("mtspr " __stringify(rn) ",2" : : :
"memory")
static inline void wrtee(unsigned long val)
{
--
2.25.0
Re: [PATCH] lib/sstep: Fix incorrect return from analyze_instr()
On 21/01/21 10:18 pm, Ananth N Mavinakayanahalli wrote: > We currently just percolate the return value from analyze_instr() > to the caller of emulate_step(), especially if it is a -1. > > For one particular case (opcode = 4) for instructions that > aren't currently emulated, we are returning 'should not be > single-stepped' while we should have returned 0 which says > 'did not emulate, may have to single-step'. > > Signed-off-by: Ananth N Mavinakayanahalli > Tested-by: Naveen N. Rao > --- > arch/powerpc/lib/sstep.c | 49 > +- > 1 file changed, 27 insertions(+), 22 deletions(-) > Thanks for fixing this. Reviewed-by: Sandipan Das
[PATCH] powerpc/prom: Fix "ibm,arch-vec-5-platform-support" scan
The "ibm,arch-vec-5-platform-support" property is a list of pairs of
bytes representing the options and values supported by the platform
firmware. At boot time, Linux scans this list and activates the
available features it recognizes : Radix and XIVE.
A recent change modified the number of entries to loop on and 8 bytes,
4 pairs of { options, values } entries are always scanned. This is
fine on KVM but not on PowerVM which can advertises less. As a
consequence on this platform, Linux reads extra entries pointing to
random data, interprets these as available features and tries to
activate them, leading to a firmware crash in
ibm,client-architecture-support.
Fix that by using the property length of "ibm,arch-vec-5-platform-support".
Cc: sta...@vger.kernel.org # v4.20+
Fixes: ab91239942a9 ("powerpc/prom: Remove VLA in
prom_check_platform_support()")
Signed-off-by: Cédric Le Goater
---
arch/powerpc/kernel/prom_init.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index e9d4eb6144e1..ccf77b985c8f 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1331,14 +1331,10 @@ static void __init prom_check_platform_support(void)
if (prop_len > sizeof(vec))
prom_printf("WARNING: ibm,arch-vec-5-platform-support
longer than expected (len: %d)\n",
prop_len);
- prom_getprop(prom.chosen, "ibm,arch-vec-5-platform-support",
-, sizeof(vec));
- for (i = 0; i < sizeof(vec); i += 2) {
- prom_debug("%d: index = 0x%x val = 0x%x\n", i / 2
- , vec[i]
- , vec[i + 1]);
- prom_parse_platform_support(vec[i], vec[i + 1],
- );
+ prom_getprop(prom.chosen, "ibm,arch-vec-5-platform-support",
, sizeof(vec));
+ for (i = 0; i < prop_len; i += 2) {
+ prom_debug("%d: index = 0x%x val = 0x%x\n", i / 2,
vec[i], vec[i + 1]);
+ prom_parse_platform_support(vec[i], vec[i + 1],
);
}
}
--
2.26.2
Re: [PATCH] powerpc/8xx: export 'cpm_setbrg' for modules
On 1/21/21 9:51 PM, Christophe Leroy wrote:
>
>
> Le 22/01/2021 à 02:08, Randy Dunlap a écrit :
>> Fix missing export for a loadable module build:
>>
>> ERROR: modpost: "cpm_setbrg" [drivers/tty/serial/cpm_uart/cpm_uart.ko]
>> undefined!
>>
>> Fixes: 4128a89ac80d ("powerpc/8xx: move CPM1 related files from sysdev/ to
>> platforms/8xx")
>
> I don't understand. Before that commit cpm_setbrg() wasn't exported either.
>
> For me, it fixes the commit that brought the capability to build the cpm uart
> driver as a module, that is commit 1da177e4c3f4 ("Linux-2.6.12-rc")
OK, I didn't have a lot of confidence in that tag.
Thanks for commenting.
>> Signed-off-by: Randy Dunlap
>> Reported-by: kernel test robot
>> Cc: Nick Desaulniers
>> Cc: clang-built-li...@googlegroups.com
>> Cc: Andrew Morton
>> Cc: Christophe Leroy
>> Cc: Michael Ellerman
>> Cc: Benjamin Herrenschmidt
>> Cc: Paul Mackerras
>> Cc: linuxppc-dev@lists.ozlabs.org
>> ---
>> arch/powerpc/platforms/8xx/cpm1.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> --- linux-next-20210121.orig/arch/powerpc/platforms/8xx/cpm1.c
>> +++ linux-next-20210121/arch/powerpc/platforms/8xx/cpm1.c
>> @@ -280,6 +280,7 @@ cpm_setbrg(uint brg, uint rate)
>> out_be32(bp, (((BRG_UART_CLK_DIV16 / rate) - 1) << 1) |
>> CPM_BRG_EN | CPM_BRG_DIV16);
>> }
>> +EXPORT_SYMBOL(cpm_setbrg);
>> struct cpm_ioport16 {
>> __be16 dir, par, odr_sor, dat, intr;
>>
--
~Randy
RFC: Features and documentation: http://lwn.net/Articles/260136/
Re: [PATCH] lib/sstep: Fix incorrect return from analyze_instr()
On 2021/01/21 10:18PM, Ananth N Mavinakayanahalli wrote:
> We currently just percolate the return value from analyze_instr()
> to the caller of emulate_step(), especially if it is a -1.
>
> For one particular case (opcode = 4) for instructions that
> aren't currently emulated, we are returning 'should not be
> single-stepped' while we should have returned 0 which says
> 'did not emulate, may have to single-step'.
>
> Signed-off-by: Ananth N Mavinakayanahalli
> Tested-by: Naveen N. Rao
> ---
> arch/powerpc/lib/sstep.c | 49
> +-
> 1 file changed, 27 insertions(+), 22 deletions(-)
Fixes: 930d6288a26787 ("powerpc: sstep: Add support for maddhd, maddhdu, maddld
instructions")
Reviewed-by: Naveen N. Rao < naveen.n@linux.vnet.ibm.com>
- Naveen
Re: [PATCH net] ibmvnic: device remove has higher precedence over reset
> > 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(>state_lock, flags);
> >
> > - if (adapter->state == VNIC_REMOVING ||
> > - adapter->state == VNIC_REMOVED) {
> > + if (adapter->state == VNIC_REMOVED) {
>
> If we do get here, we would crash because ibmvnic_remove() happened. It
> frees the adapter struct already.
Not exactly. viodev is gone; netdev is gone; ibmvnic_adapter is still there.
Lijun
Re: [PATCH net] ibmvnic: device remove has higher precedence over reset
On Thu, Jan 21, 2021 at 12:42 PM Dany Madden wrote:
>
> On 2021-01-20 22:20, Lijun Pan wrote:
> > 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.dpwayq6wdgfdz...@pengutronix.de/T/#m48f5befd96bc9842ece2a3ad14f4c27747206a53
> > Reported-by: Uwe Kleine-König
> > Fixes: 7d7195a026ba ("ibmvnic: Do not process device remove during
> > device reset")
> > Signed-off-by: Lijun Pan
> > ---
> > 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(>state_lock, flags);
> >
> > - if (adapter->state == VNIC_REMOVING ||
> > - adapter->state == VNIC_REMOVED) {
> > + if (adapter->state == VNIC_REMOVED) {
>
> If we do get here, we would crash because ibmvnic_remove() happened. It
> frees the adapter struct already.
Not exactly. viodev is gone; netdev is done; ibmvnic_adapter is still there.
Lijun
>
> > spin_unlock_irqrestore(>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(>state_lock, flags);
> > - if (test_bit(0, >resetting)) {
> > - spin_unlock_irqrestore(>state_lock, flags);
> > - return -EBUSY;
> > - }
> > -
> > adapter->state = VNIC_REMOVING;
> > spin_unlock_irqrestore(>state_lock, flags);
[PATCH 1/2] ima: Free IMA measurement buffer on error
IMA allocates kernel virtual memory to carry forward the measurement
list, from the current kernel to the next kernel on kexec system call,
in ima_add_kexec_buffer() function. In error code paths this memory
is not freed resulting in memory leak.
Free the memory allocated for the IMA measurement list in
the error code paths in ima_add_kexec_buffer() function.
Signed-off-by: Lakshmi Ramasubramanian
Suggested-by: Tyler Hicks
Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list")
---
security/integrity/ima/ima_kexec.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/integrity/ima/ima_kexec.c
b/security/integrity/ima/ima_kexec.c
index 121de3e04af2..212145008a01 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -119,12 +119,14 @@ void ima_add_kexec_buffer(struct kimage *image)
ret = kexec_add_buffer();
if (ret) {
pr_err("Error passing over kexec measurement buffer.\n");
+ vfree(kexec_buffer);
return;
}
ret = arch_ima_add_kexec_buffer(image, kbuf.mem, kexec_segment_size);
if (ret) {
pr_err("Error passing over kexec measurement buffer.\n");
+ vfree(kexec_buffer);
return;
}
--
2.30.0
[PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall
IMA allocates kernel virtual memory to carry forward the measurement
list, from the current kernel to the next kernel on kexec system call,
in ima_add_kexec_buffer() function. This buffer is not freed before
completing the kexec system call resulting in memory leak.
Add ima_buffer field in "struct kimage" to store the virtual address
of the buffer allocated for the IMA measurement list.
Free the memory allocated for the IMA measurement list in
kimage_file_post_load_cleanup() function.
Signed-off-by: Lakshmi Ramasubramanian
Suggested-by: Tyler Hicks
Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list")
---
include/linux/kexec.h | 5 +
kernel/kexec_file.c| 5 +
security/integrity/ima/ima_kexec.c | 2 ++
3 files changed, 12 insertions(+)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 9e93bef52968..5f61389f5f36 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -300,6 +300,11 @@ struct kimage {
/* Information for loading purgatory */
struct purgatory_info purgatory_info;
#endif
+
+#ifdef CONFIG_IMA_KEXEC
+ /* Virtual address of IMA measurement buffer for kexec syscall */
+ void *ima_buffer;
+#endif
};
/* kexec interface functions */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b02086d70492..5c3447cf7ad5 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -166,6 +166,11 @@ void kimage_file_post_load_cleanup(struct kimage *image)
vfree(pi->sechdrs);
pi->sechdrs = NULL;
+#ifdef CONFIG_IMA_KEXEC
+ vfree(image->ima_buffer);
+ image->ima_buffer = NULL;
+#endif /* CONFIG_IMA_KEXEC */
+
/* See if architecture has anything to cleanup post load */
arch_kimage_file_post_load_cleanup(image);
diff --git a/security/integrity/ima/ima_kexec.c
b/security/integrity/ima/ima_kexec.c
index 212145008a01..8eadd0674629 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -130,6 +130,8 @@ void ima_add_kexec_buffer(struct kimage *image)
return;
}
+ image->ima_buffer = kexec_buffer;
+
pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
kbuf.mem);
}
--
2.30.0
RE: [PATCH 02/13] module: add a module_loaded helper
> > On Thu, Jan 21, 2021 at 11:00:20AM +0100, Christophe Leroy wrote: > > > +bool module_loaded(const char *name); > > > > Maybe module_is_loaded() would be a better name. > > Fine with me. It does look like both callers aren't 'unsafe'. But it is a bit strange returning a stale value. It did make be look a bit more deeply at try_module_get(). For THIS_MODULE (eg to get an extra reference for a kthread) I doubt it can ever fail. But the other cases require a 'module' structure be passed in. ISTM that unloading the module could invalidate the pointer that has just been read from some other structure. I'm guessing that some other lock must always be held. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH net] ibmvnic: device remove has higher precedence over reset
On 2021-01-20 22:20, Lijun Pan wrote:
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.dpwayq6wdgfdz...@pengutronix.de/T/#m48f5befd96bc9842ece2a3ad14f4c27747206a53
Reported-by: Uwe Kleine-König
Fixes: 7d7195a026ba ("ibmvnic: Do not process device remove during
device reset")
Signed-off-by: Lijun Pan
---
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(>state_lock, flags);
- if (adapter->state == VNIC_REMOVING ||
- adapter->state == VNIC_REMOVED) {
+ if (adapter->state == VNIC_REMOVED) {
If we do get here, we would crash because ibmvnic_remove() happened. It
frees the adapter struct already.
spin_unlock_irqrestore(>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(>state_lock, flags);
- if (test_bit(0, >resetting)) {
- spin_unlock_irqrestore(>state_lock, flags);
- return -EBUSY;
- }
-
adapter->state = VNIC_REMOVING;
spin_unlock_irqrestore(>state_lock, flags);
