Re: [PATCH 1/5] CMDLINE: add generic builtin command line
Le 04/03/2021 à 05:47, Daniel Walker a écrit :
This code allows architectures to use a generic builtin command line.
The state of the builtin command line options across architecture is
diverse. On x86 and mips they have pretty much the same code and the
code prepends the builtin command line onto the boot loader provided
one. On powerpc there is only a builtin override and nothing else.
The code in this commit unifies the code into a generic
header file under the CONFIG_GENERIC_CMDLINE option. When this
option is enabled the architecture can call the cmdline_add_builtin()
to add the builtin command line.
WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in
line 1
#32: FILE: include/linux/cmdline.h:1:
+#ifndef _LINUX_CMDLINE_H
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#56: FILE: include/linux/cmdline.h:25:
+__cmdline_add_builtin(char *dest, const char *src, char *tmp, unsigned long
length,
+ size_t (*strlcpy)(char *dest, const char *src, size_t size),
WARNING:STRLCPY: Prefer strscpy over strlcpy - see:
https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=v6a6g1ouzcprm...@mail.gmail.com/
#61: FILE: include/linux/cmdline.h:30:
+ strlcpy(dest, " ", length);
WARNING:STRLCPY: Prefer strscpy over strlcpy - see:
https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=v6a6g1ouzcprm...@mail.gmail.com/
#69: FILE: include/linux/cmdline.h:38:
+ strlcpy(tmp, CONFIG_CMDLINE_PREPEND " ", length);
WARNING:STRLCPY: Prefer strscpy over strlcpy - see:
https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=v6a6g1ouzcprm...@mail.gmail.com/
#71: FILE: include/linux/cmdline.h:40:
+ strlcpy(dest, tmp, length);
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#75: FILE: include/linux/cmdline.h:44:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat)
^I^I^I\$
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'dest' - possible side-effects?
#75: FILE: include/linux/cmdline.h:44:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat)
\
+{
\
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
\
+ static label char cmdline_tmp_space[length];
\
+ __cmdline_add_builtin(dest, src, cmdline_tmp_space, length,
strlcpy, strlcat); \
+ } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) {
\
+ __cmdline_add_builtin(dest, src, NULL, length, strlcpy,
strlcat); \
+ }
\
+}
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'src' - possible side-effects?
#75: FILE: include/linux/cmdline.h:44:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat)
\
+{
\
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
\
+ static label char cmdline_tmp_space[length];
\
+ __cmdline_add_builtin(dest, src, cmdline_tmp_space, length,
strlcpy, strlcat); \
+ } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) {
\
+ __cmdline_add_builtin(dest, src, NULL, length, strlcpy,
strlcat); \
+ }
\
+}
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'length' - possible side-effects?
#75: FILE: include/linux/cmdline.h:44:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat)
\
+{
\
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
\
+ static label char cmdline_tmp_space[length];
\
+ __cmdline_add_builtin(dest, src, cmdline_tmp_space, length,
strlcpy, strlcat); \
+ } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) {
\
+ __cmdline_add_builtin(dest, src, NULL, length, strlcpy,
strlcat); \
+ }
\
+}
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'strlcpy' - possible side-effects?
#75: FILE: include/linux/cmdline.h:44:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat)
\
+{
Re: [PATCH v3] powerpc/uprobes: Validation for prefixed instruction
@@ -41,6 +41,14 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, if (addr & 0x03) return -EINVAL; + if (!IS_ENABLED(CONFIG_PPC64) || !cpu_has_feature(CPU_FTR_ARCH_31)) + return 0; Sorry, I missed this last time, but I think we can drop the above checks. ppc_inst_prefixed() already factors in the dependency on CONFIG_PPC64, Yeah, makes sense. I initially added CONFIG_PPC64 check because I was using ppc_inst_prefix(x, y) macro which is not available for !CONFIG_PPC64. and I don't think we need to confirm if we're running on a ISA V3.1 for the below check. Prefixed instructions would be supported only when ARCH_31 is set. (Ignoring insane scenario where user probes on prefixed instruction on p10 predecessors). So I guess I still need ARCH_31 check? Ravi
Re: [PATCH 3/5] CMDLINE: powerpc: convert to generic builtin command line
Le 04/03/2021 à 05:48, Daniel Walker a écrit :
This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
option.
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#143: FILE: arch/powerpc/kernel/prom_init.c:788:
+ cmdline_add_builtin_custom(prom_cmd_line, NULL,
sizeof(prom_cmd_line),
+ __prombss, _strlcpy,
_strlcat);
total: 0 errors, 0 warnings, 1 checks, 117 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
the.patch has style problems, please review.
NOTE: Ignored message types: ARCH_INCLUDE_LINUX BIT_MACRO COMPARISON_TO_NULL DT_SPLIT_BINDING_PATCH
EMAIL_SUBJECT FILE_PATH_CHANGES GLOBAL_INITIALISERS LINE_SPACING MULTIPLE_ASSIGNMENTS
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
Cc: xe-linux-exter...@cisco.com
Signed-off-by: Ruslan Ruslichenko
Signed-off-by: Ruslan Bilovol
Signed-off-by: Daniel Walker
---
arch/powerpc/Kconfig| 37 +
arch/powerpc/kernel/prom.c | 1 +
arch/powerpc/kernel/prom_init.c | 31 +++
3 files changed, 20 insertions(+), 49 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 107bb4319e0e..276b06d5c961 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -167,6 +167,7 @@ config PPC
select EDAC_SUPPORT
select GENERIC_ATOMIC64 if PPC32
select GENERIC_CLOCKEVENTS_BROADCASTif SMP
+ select GENERIC_CMDLINE
select GENERIC_CMOS_UPDATE
select GENERIC_CPU_AUTOPROBE
select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
@@ -906,42 +907,6 @@ config PPC_DENORMALISATION
Add support for handling denormalisation of single precision
values. Useful for bare metal only. If unsure say Y here.
-config CMDLINE
- string "Initial kernel command string"
- default ""
- help
- On some platforms, there is currently no way for the boot loader to
- pass arguments to the kernel. For these platforms, you can supply
- some command-line options at build time by entering them here. In
- most cases you will need to specify the root device here.
-
-choice
- prompt "Kernel command line type" if CMDLINE != ""
- default CMDLINE_FROM_BOOTLOADER
-
-config CMDLINE_FROM_BOOTLOADER
- bool "Use bootloader kernel arguments if available"
- help
- Uses the command-line options passed by the boot loader. If
- the boot loader doesn't provide any, the default kernel command
- string provided in CMDLINE will be used.
-
-config CMDLINE_EXTEND
- bool "Extend bootloader kernel arguments"
- help
- The command-line arguments provided by the boot loader will be
- appended to the default kernel command string.
-
-config CMDLINE_FORCE
- bool "Always use the default kernel command string"
- help
- Always use the default kernel command string, even if the boot
- loader passes other arguments to the kernel.
- This is useful if you cannot or don't want to change the
- command-line options your boot loader passes to the kernel.
-
-endchoice
-
config EXTRA_TARGETS
string "Additional default image types"
help
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index ae3c41730367..96d0a01be1b4 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index e9d4eb6144e1..d752be688b62 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -242,15 +243,6 @@ static int __init prom_strcmp(const char *cs, const char
*ct)
return 0;
}
-static char __init *prom_strcpy(char *dest, const char *src)
-{
- char *tmp = dest;
-
- while ((*dest++ = *src++) != '\0')
- /* nothing */;
- return tmp;
-}
-
static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
{
unsigned char c1, c2;
@@ -276,6 +268,19 @@ static size_t __init prom_strlen(const char *s)
return sc - s;
}
+static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)
+{
+ size_t ret = prom_strlen(src);
+
+ if (size) {
+ size_t len = (ret >= size) ? size - 1 : ret;
+ memcpy(dest, src, len);
+ dest[len] = '\0';
+ }
+ return ret;
+}
+
+
static int __init prom_memcmp(const void *cs, const void *ct, size_t count)
{
Re: [PATCH 3/5] CMDLINE: powerpc: convert to generic builtin command line
Le 04/03/2021 à 05:48, Daniel Walker a écrit :
This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
option.
In file included from arch/powerpc/kernel/prom_init.c:30:
arch/powerpc/kernel/prom_init.c: In function 'early_cmdline_parse':
arch/powerpc/kernel/prom_init.c:788:17: error: lvalue required as unary '&'
operand
788 | __prombss, _strlcpy, _strlcat);
| ^
./include/linux/cmdline.h:66:3: note: in definition of macro
'cmdline_add_builtin_custom'
66 | strlcpy(dest, src, length); \
| ^~~
At top level:
arch/powerpc/kernel/prom_init.c:312:22: error: 'prom_strlcat' defined but not used
[-Werror=unused-function]
312 | static size_t __init prom_strlcat(char *dest, const char *src, size_t
count)
| ^~~~
cc1: all warnings being treated as errors
make[2]: *** [scripts/Makefile.build:279: arch/powerpc/kernel/prom_init.o]
Error 1
make[1]: *** [scripts/Makefile.build:496: arch/powerpc/kernel] Error 2
make[1]: *** Waiting for unfinished jobs
make: *** [Makefile:1805: arch/powerpc] Error 2
make: *** Waiting for unfinished jobs
Cc: xe-linux-exter...@cisco.com
Signed-off-by: Ruslan Ruslichenko
Signed-off-by: Ruslan Bilovol
Signed-off-by: Daniel Walker
---
arch/powerpc/Kconfig| 37 +
arch/powerpc/kernel/prom.c | 1 +
arch/powerpc/kernel/prom_init.c | 31 +++
3 files changed, 20 insertions(+), 49 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 107bb4319e0e..276b06d5c961 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -167,6 +167,7 @@ config PPC
select EDAC_SUPPORT
select GENERIC_ATOMIC64 if PPC32
select GENERIC_CLOCKEVENTS_BROADCASTif SMP
+ select GENERIC_CMDLINE
select GENERIC_CMOS_UPDATE
select GENERIC_CPU_AUTOPROBE
select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
@@ -906,42 +907,6 @@ config PPC_DENORMALISATION
Add support for handling denormalisation of single precision
values. Useful for bare metal only. If unsure say Y here.
-config CMDLINE
- string "Initial kernel command string"
- default ""
- help
- On some platforms, there is currently no way for the boot loader to
- pass arguments to the kernel. For these platforms, you can supply
- some command-line options at build time by entering them here. In
- most cases you will need to specify the root device here.
-
-choice
- prompt "Kernel command line type" if CMDLINE != ""
- default CMDLINE_FROM_BOOTLOADER
-
-config CMDLINE_FROM_BOOTLOADER
- bool "Use bootloader kernel arguments if available"
- help
- Uses the command-line options passed by the boot loader. If
- the boot loader doesn't provide any, the default kernel command
- string provided in CMDLINE will be used.
-
-config CMDLINE_EXTEND
- bool "Extend bootloader kernel arguments"
- help
- The command-line arguments provided by the boot loader will be
- appended to the default kernel command string.
-
-config CMDLINE_FORCE
- bool "Always use the default kernel command string"
- help
- Always use the default kernel command string, even if the boot
- loader passes other arguments to the kernel.
- This is useful if you cannot or don't want to change the
- command-line options your boot loader passes to the kernel.
-
-endchoice
-
config EXTRA_TARGETS
string "Additional default image types"
help
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index ae3c41730367..96d0a01be1b4 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index e9d4eb6144e1..d752be688b62 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -242,15 +243,6 @@ static int __init prom_strcmp(const char *cs, const char
*ct)
return 0;
}
-static char __init *prom_strcpy(char *dest, const char *src)
-{
- char *tmp = dest;
-
- while ((*dest++ = *src++) != '\0')
- /* nothing */;
- return tmp;
-}
-
static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
{
unsigned char c1, c2;
@@ -276,6 +268,19 @@ static size_t __init prom_strlen(const char *s)
return sc - s;
}
+static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)
+{
+ size_t ret = prom_strlen(src);
+
+ if (size) {
+ size_t len = (ret >= size) ? size - 1 : ret;
+
Re: [PATCH v3] powerpc/uprobes: Validation for prefixed instruction
Le 04/03/2021 à 06:05, Ravi Bangoria a écrit :
As per ISA 3.1, prefixed instruction should not cross 64-byte
boundary. So don't allow Uprobe on such prefixed instruction.
There are two ways probed instruction is changed in mapped pages.
First, when Uprobe is activated, it searches for all the relevant
pages and replace instruction in them. In this case, if that probe
is on the 64-byte unaligned prefixed instruction, error out
directly. Second, when Uprobe is already active and user maps a
relevant page via mmap(), instruction is replaced via mmap() code
path. But because Uprobe is invalid, entire mmap() operation can
not be stopped. In this case just print an error and continue.
Signed-off-by: Ravi Bangoria
---
v2:
https://lore.kernel.org/r/20210204104703.273429-1-ravi.bango...@linux.ibm.com
v2->v3:
- Drop restriction for Uprobe on suffix of prefixed instruction.
It needs lot of code change including generic code but what
we get in return is not worth it.
arch/powerpc/kernel/uprobes.c | 8
1 file changed, 8 insertions(+)
diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index e8a63713e655..c400971ebe70 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -41,6 +41,14 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
if (addr & 0x03)
return -EINVAL;
+ if (!IS_ENABLED(CONFIG_PPC64) || !cpu_has_feature(CPU_FTR_ARCH_31))
cpu_has_feature(CPU_FTR_ARCH_31) should return 'false' when CONFIG_PPC64 is not enabled, no need to
double check.
+ return 0;
+
+ if (ppc_inst_prefixed(auprobe->insn) && (addr & 0x3F) == 0x3C) {
Maybe 3C instead of 4F ? : (addr & 0x3C) == 0x3C
What about
(addr & (SZ_64 - 4)) == SZ_64 - 4 to make it more explicit ?
Or ALIGN(addr, SZ_64) != ALIGN(addr + 8, SZ_64)
+ pr_info_ratelimited("Cannot register a uprobe on 64 byte unaligned
prefixed instruction\n");
+ return -EINVAL;
+ }
+
return 0;
}
Christophe
Re: [PATCH 3/5] CMDLINE: powerpc: convert to generic builtin command line
Le 04/03/2021 à 05:48, Daniel Walker a écrit :
This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
option.
Should be split in two patches. The change of strcpy to strlcpy should go in a
first patch.
Cc: xe-linux-exter...@cisco.com
Signed-off-by: Ruslan Ruslichenko
Signed-off-by: Ruslan Bilovol
Signed-off-by: Daniel Walker
---
arch/powerpc/Kconfig| 37 +
arch/powerpc/kernel/prom.c | 1 +
arch/powerpc/kernel/prom_init.c | 31 +++
3 files changed, 20 insertions(+), 49 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 107bb4319e0e..276b06d5c961 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -167,6 +167,7 @@ config PPC
select EDAC_SUPPORT
select GENERIC_ATOMIC64 if PPC32
select GENERIC_CLOCKEVENTS_BROADCASTif SMP
+ select GENERIC_CMDLINE
select GENERIC_CMOS_UPDATE
select GENERIC_CPU_AUTOPROBE
select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
@@ -906,42 +907,6 @@ config PPC_DENORMALISATION
Add support for handling denormalisation of single precision
values. Useful for bare metal only. If unsure say Y here.
-config CMDLINE
- string "Initial kernel command string"
- default ""
- help
- On some platforms, there is currently no way for the boot loader to
- pass arguments to the kernel. For these platforms, you can supply
- some command-line options at build time by entering them here. In
- most cases you will need to specify the root device here.
-
-choice
- prompt "Kernel command line type" if CMDLINE != ""
- default CMDLINE_FROM_BOOTLOADER
-
-config CMDLINE_FROM_BOOTLOADER
- bool "Use bootloader kernel arguments if available"
- help
- Uses the command-line options passed by the boot loader. If
- the boot loader doesn't provide any, the default kernel command
- string provided in CMDLINE will be used.
-
-config CMDLINE_EXTEND
- bool "Extend bootloader kernel arguments"
- help
- The command-line arguments provided by the boot loader will be
- appended to the default kernel command string.
-
-config CMDLINE_FORCE
- bool "Always use the default kernel command string"
- help
- Always use the default kernel command string, even if the boot
- loader passes other arguments to the kernel.
- This is useful if you cannot or don't want to change the
- command-line options your boot loader passes to the kernel.
-
-endchoice
-
config EXTRA_TARGETS
string "Additional default image types"
help
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index ae3c41730367..96d0a01be1b4 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index e9d4eb6144e1..d752be688b62 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -242,15 +243,6 @@ static int __init prom_strcmp(const char *cs, const char
*ct)
return 0;
}
-static char __init *prom_strcpy(char *dest, const char *src)
-{
- char *tmp = dest;
-
- while ((*dest++ = *src++) != '\0')
- /* nothing */;
- return tmp;
-}
-
static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
{
unsigned char c1, c2;
@@ -276,6 +268,19 @@ static size_t __init prom_strlen(const char *s)
return sc - s;
}
+static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)
+{
+ size_t ret = prom_strlen(src);
+
+ if (size) {
+ size_t len = (ret >= size) ? size - 1 : ret;
+ memcpy(dest, src, len);
+ dest[len] = '\0';
+ }
+ return ret;
+}
+
+
static int __init prom_memcmp(const void *cs, const void *ct, size_t count)
{
const unsigned char *su1, *su2;
@@ -778,9 +783,9 @@ static void __init early_cmdline_parse(void)
if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)
You have removed CONFIG_CMDLINE_FORCE and the generic cmdline doesn't provide
it.
l = prom_getprop(prom.chosen, "bootargs", p,
COMMAND_LINE_SIZE-1);
- if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\0')
- prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
-sizeof(prom_cmd_line));
+ if (l <= 0 || p[0] == '\0') /* dbl check */
So it means cmdline_add_builtin_custom() is only called when bootloader does
not provide bootargs ?
+ cmdline_add_builtin_custom(prom_cmd_line,
Re: [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section
Le 04/03/2021 à 05:47, Daniel Walker a écrit :
It looks like there's some seepage of cmdline stuff into
the generic device tree code. This conflicts with the
generic cmdline implementation so I remove it in the case
when that's enabled.
Cc: xe-linux-exter...@cisco.com
Signed-off-by: Ruslan Ruslichenko
Signed-off-by: Daniel Walker
---
drivers/of/fdt.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index feb0f2d67fc5..cfe4f8d3c9f5 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include /* for COMMAND_LINE_SIZE */
#include
@@ -1048,8 +1049,18 @@ int __init early_init_dt_scan_chosen(unsigned long node,
const char *uname,
early_init_dt_check_for_initrd(node);
+#ifdef CONFIG_GENERIC_CMDLINE
/* Retrieve command line */
p = of_get_flat_dt_prop(node, "bootargs", );
+
+ /*
+* The builtin command line will be added here, or it can override
+* with the DT bootargs.
+*/
+ cmdline_add_builtin(data,
+ ((p != NULL && l > 0) ? p : NULL), /* This is
sanity checking */
Can we do more simple ? If p is NULL, p is already NULL, so (l > 0 ? p : NULL)
should be enough.
+ COMMAND_LINE_SIZE);
+#else
What does p contains now that "p = of_get_flat_dt_prop(node, "bootargs", );" is only called when
CONFIG_GENERIC_CMDLINE is defined ?
if (p != NULL && l > 0)
strlcpy(data, p, min(l, COMMAND_LINE_SIZE));
@@ -1070,6 +1081,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
#endif
#endif /* CONFIG_CMDLINE */
+#endif /* CONFIG_GENERIC_CMDLINE */
pr_debug("Command line is: %s\n", (char *)data);
Re: [PATCH 1/5] CMDLINE: add generic builtin command line
Le 04/03/2021 à 05:47, Daniel Walker a écrit :
This code allows architectures to use a generic builtin command line.
The state of the builtin command line options across architecture is
diverse. On x86 and mips they have pretty much the same code and the
code prepends the builtin command line onto the boot loader provided
one. On powerpc there is only a builtin override and nothing else.
This is not exact. powerpc has:
CONFIG_FROM_BOOTLOADER
CONFIG_EXTEND
CONFIG_FORCE
The code in this commit unifies the code into a generic
header file under the CONFIG_GENERIC_CMDLINE option. When this
option is enabled the architecture can call the cmdline_add_builtin()
to add the builtin command line.
Cc: xe-linux-exter...@cisco.com
Signed-off-by: Ruslan Bilovol
Signed-off-by: Daniel Walker
---
include/linux/cmdline.h | 75 +
init/Kconfig| 68 +
2 files changed, 143 insertions(+)
create mode 100644 include/linux/cmdline.h
diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
new file mode 100644
index ..f44011d1a9ee
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,75 @@
Missing the SPDX Licence Identifier
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+
+/*
+ *
+ * Copyright (C) 2006,2021. Cisco Systems, Inc.
+ *
+ * Generic Append/Prepend cmdline support.
+ */
+
+#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_CMDLINE_BOOL)
I think it would be better if we can avoid the CONFIG_CMDLINE_BOOL.
By making the CMDLINEs default to "" at all time, I think we can about that
BOOL.
+
+#ifndef CONFIG_CMDLINE_OVERRIDE
+/*
+ * This function will append or prepend a builtin command line to the command
As far as I understand, it doesn't "append _or_ prepend" but it does "append _and_
prepend"
+ * line provided by the bootloader. Kconfig options can be used to alter
+ * the behavior of this builtin command line.
+ * @dest: The destination of the final appended/prepended string
+ * @src: The starting string or NULL if there isn't one.
+ * @tmp: temporary space used for prepending
+ * @length: the maximum length of the strings above.
Missing some parameters here, but I think we should avoid those 'strlcpy' and 'strlcat', see later
comment.
+ */
+static inline void
+__cmdline_add_builtin(char *dest, const char *src, char *tmp, unsigned long
length,
+ size_t (*strlcpy)(char *dest, const char *src, size_t size),
+ size_t (*strlcat)(char *dest, const char *src, size_t count)
Don't use names that overide names of existing functions.
'count' is __kernel_size_t not size_t
+ )
+{
+ if (src != dest && src != NULL) {
+ strlcpy(dest, " ", length);
Why do you need a space up front in that case ? Why not just copy the source to
the destination ?
+ strlcat(dest, src, length);
+ }
+
+ if (sizeof(CONFIG_CMDLINE_APPEND) > 1)
+ strlcat(dest, " " CONFIG_CMDLINE_APPEND, length);
+
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
+ strlcpy(tmp, CONFIG_CMDLINE_PREPEND " ", length);
+ strlcat(tmp, dest, length);
+ strlcpy(dest, tmp, length);
Could we use memmove(), or implement strmove() and avoid the temporary buffer
at all ?
+ }
+}
+
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat)
\
It is misleading to call parameters 'strlcpy' or 'strlcat', it hides that they
are overriden.
+{
\
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
\
+ static label char cmdline_tmp_space[length];
\
Let the architecture define the temporary space when using the custom variant instead of just asking
the architecture to provide the name of the section to use. powerpc already have prom_scratch for that.
+ __cmdline_add_builtin(dest, src, cmdline_tmp_space, length,
strlcpy, strlcat); \
+ } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) {
\
+ __cmdline_add_builtin(dest, src, NULL, length, strlcpy,
strlcat); \
+ }
\
Ah, so if I understand correctly, the user can set both CONFIG_CMDLINE_PREPEND and
CONFIG_CMDLINE_APPEND but one of them is silently ignored.
Then I think we should just offer the user to set one, name it CONFIG_CMDLINE then ask him to choose
between FORCE, APPEND or PREPEND.
+}
+#define cmdline_add_builtin(dest, src, length)\
+ cmdline_add_builtin_custom(dest, src, length, __initdata, ,
)
+#else
+#define cmdline_add_builtin(dest, src, length)\
+{
Re: [PATCH v5 2/3] KVM: PPC: Book3S HV: Add support for H_RPT_INVALIDATE
On Tue, Mar 02, 2021 at 10:28:53AM +0530, Bharata B Rao wrote:
> On Tue, Mar 02, 2021 at 12:45:18PM +1100, David Gibson wrote:
> > > diff --git a/Documentation/virt/kvm/api.rst
> > > b/Documentation/virt/kvm/api.rst
> > > index 45fd862ac128..38ce3f21b21f 100644
> > > --- a/Documentation/virt/kvm/api.rst
> > > +++ b/Documentation/virt/kvm/api.rst
> > > @@ -6225,6 +6225,24 @@ KVM_RUN_BUS_LOCK flag is used to distinguish
> > > between them.
> > > This capability can be used to check / enable 2nd DAWR feature provided
> > > by POWER10 processor.
> > >
> > > +7.23 KVM_CAP_PPC_RPT_INVALIDATE
> > > +--
> > > +
> > > +:Capability: KVM_CAP_PPC_RPT_INVALIDATE
> > > +:Architectures: ppc
> > > +:Type: vm
> > > +
> > > +This capability indicates that the kernel is capable of handling
> > > +H_RPT_INVALIDATE hcall.
> > > +
> > > +In order to enable the use of H_RPT_INVALIDATE in the guest,
> > > +user space might have to advertise it for the guest. For example,
> > > +IBM pSeries (sPAPR) guest starts using it if "hcall-rpt-invalidate" is
> > > +present in the "ibm,hypertas-functions" device-tree property.
> > > +
> > > +This capability is enabled for hypervisors on platforms like POWER9
> > > +that support radix MMU.
> >
> > Does this mean that KVM will handle the hypercall, even if not
> > explicitly enabled by userspace (qemu)? That's generally not what we
> > want, since we need to allow qemu to set up backwards compatible
> > guests.
>
> This capability only indicates that hypervisor supports the hcall.
>
> QEMU will check for this and conditionally enable the hcall
> (via KVM_CAP_PPC_ENABLE_HCALL ioctl). Enabling the hcall is
> conditional to cap-rpt-invalidate sPAPR machine capability being
> enabled by the user. Will post a followup QEMU patch shortly.
Ok.
> Older QEMU patch can be found here:
> https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg00627.html
>
> >
> > > +
> > > 8. Other capabilities.
> > > ==
> > >
> > > diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> > > b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> > > index 8b33601cdb9d..a46fd37ad552 100644
> > > --- a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> > > +++ b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> > > @@ -4,6 +4,10 @@
> > >
> > > #include
> > >
> > > +#define RIC_FLUSH_TLB 0
> > > +#define RIC_FLUSH_PWC 1
> > > +#define RIC_FLUSH_ALL 2
> > > +
> > > struct vm_area_struct;
> > > struct mm_struct;
> > > struct mmu_gather;
> > > diff --git a/arch/powerpc/include/asm/kvm_book3s.h
> > > b/arch/powerpc/include/asm/kvm_book3s.h
> > > index 2f5f919f6cd3..a1515f94400e 100644
> > > --- a/arch/powerpc/include/asm/kvm_book3s.h
> > > +++ b/arch/powerpc/include/asm/kvm_book3s.h
> > > @@ -305,6 +305,9 @@ void kvmhv_set_ptbl_entry(unsigned int lpid, u64 dw0,
> > > u64 dw1);
> > > void kvmhv_release_all_nested(struct kvm *kvm);
> > > long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu);
> > > long kvmhv_do_nested_tlbie(struct kvm_vcpu *vcpu);
> > > +long kvmhv_h_rpti_nested(struct kvm_vcpu *vcpu, unsigned long lpid,
> > > + unsigned long type, unsigned long pg_sizes,
> > > + unsigned long start, unsigned long end);
> > > int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu,
> > > u64 time_limit, unsigned long lpcr);
> > > void kvmhv_save_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state
> > > *hr);
> > > diff --git a/arch/powerpc/include/asm/mmu_context.h
> > > b/arch/powerpc/include/asm/mmu_context.h
> > > index 652ce85f9410..820caf4e01b7 100644
> > > --- a/arch/powerpc/include/asm/mmu_context.h
> > > +++ b/arch/powerpc/include/asm/mmu_context.h
> > > @@ -124,8 +124,19 @@ static inline bool need_extra_context(struct
> > > mm_struct *mm, unsigned long ea)
> > >
> > > #if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) &&
> > > defined(CONFIG_PPC_RADIX_MMU)
> > > extern void radix_kvm_prefetch_workaround(struct mm_struct *mm);
> > > +void do_h_rpt_invalidate(unsigned long pid, unsigned long lpid,
> > > + unsigned long type, unsigned long page_size,
> > > + unsigned long psize, unsigned long start,
> > > + unsigned long end);
> > > #else
> > > static inline void radix_kvm_prefetch_workaround(struct mm_struct *mm) {
> > > }
> > > +static inline void do_h_rpt_invalidate(unsigned long pid,
> > > +unsigned long lpid,
> > > +unsigned long type,
> > > +unsigned long page_size,
> > > +unsigned long psize,
> > > +unsigned long start,
> > > +unsigned long end) { }
> > > #endif
> > >
> > > extern void switch_cop(struct mm_struct *next);
> > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > > index
Re: [PATCH v5 1/3] powerpc/book3s64/radix: Add H_RPT_INVALIDATE pgsize encodings to mmu_psize_def
On Tue, Mar 02, 2021 at 09:51:28AM +0530, Bharata B Rao wrote:
> On Tue, Mar 02, 2021 at 12:28:34PM +1100, David Gibson wrote:
> > On Wed, Feb 24, 2021 at 01:55:08PM +0530, Bharata B Rao wrote:
> > > Add a field to mmu_psize_def to store the page size encodings
> > > of H_RPT_INVALIDATE hcall. Initialize this while scanning the radix
> > > AP encodings. This will be used when invalidating with required
> > > page size encoding in the hcall.
> > >
> > > Signed-off-by: Bharata B Rao
> > > ---
> > > arch/powerpc/include/asm/book3s/64/mmu.h | 1 +
> > > arch/powerpc/mm/book3s64/radix_pgtable.c | 5 +
> > > 2 files changed, 6 insertions(+)
> > >
> > > diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h
> > > b/arch/powerpc/include/asm/book3s/64/mmu.h
> > > index eace8c3f7b0a..c02f42d1031e 100644
> > > --- a/arch/powerpc/include/asm/book3s/64/mmu.h
> > > +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
> > > @@ -19,6 +19,7 @@ struct mmu_psize_def {
> > > int penc[MMU_PAGE_COUNT]; /* HPTE encoding */
> > > unsigned inttlbiel; /* tlbiel supported for that page size */
> > > unsigned long avpnm; /* bits to mask out in AVPN in the HPTE */
> > > + unsigned long h_rpt_pgsize; /* H_RPT_INVALIDATE page size encoding */
> > > union {
> > > unsigned long sllp; /* SLB L||LP (exact mask to use in
> > > slbmte) */
> > > unsigned long ap; /* Ap encoding used by PowerISA 3.0 */
> > > diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c
> > > b/arch/powerpc/mm/book3s64/radix_pgtable.c
> > > index 98f0b243c1ab..1b749899016b 100644
> > > --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> > > +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> > > @@ -486,6 +486,7 @@ static int __init radix_dt_scan_page_sizes(unsigned
> > > long node,
> > > def = _psize_defs[idx];
> > > def->shift = shift;
> > > def->ap = ap;
> > > + def->h_rpt_pgsize = psize_to_rpti_pgsize(idx);
> > > }
> > >
> > > /* needed ? */
> > > @@ -560,9 +561,13 @@ void __init radix__early_init_devtree(void)
> > >*/
> > > mmu_psize_defs[MMU_PAGE_4K].shift = 12;
> > > mmu_psize_defs[MMU_PAGE_4K].ap = 0x0;
> > > + mmu_psize_defs[MMU_PAGE_4K].h_rpt_pgsize =
> > > + psize_to_rpti_pgsize(MMU_PAGE_4K);
> >
> > Hm. TBH, I was thinking of this as replacing psize_to_rpti_pgsize() -
> > that is, you directly put the correct codes in there, then just have
> > psize_to_rpti_pgsize() look them up in the table.
> >
> > I guess that could be a followup change, though.
> >
> > >
> > > mmu_psize_defs[MMU_PAGE_64K].shift = 16;
> > > mmu_psize_defs[MMU_PAGE_64K].ap = 0x5;
> > > + mmu_psize_defs[MMU_PAGE_64K].h_rpt_pgsize =
> > > + psize_to_rpti_pgsize(MMU_PAGE_64K);
>
> Hmm if you see I got rid of rpti_pgsize_to_psize() by having the
> defines directly in mmu_psize_def[].
I realize that, but I'm talking about the reverse direction:
psize_to_rpti_pgsize(). You should be able to reduce it a table
lookup, so the mmu_psize_defs table is the only place this information
exists.
> There are two cases in the above code (radix__early_init_devtree)
>
> 1. If radix pagesize encodings are present in the DT, we walk
> the page sizes in the loop and populate the enconding for
> H_RPT_INVALIDATE. I am not sure if we can use the direct codes
> in this case.
I'm not understanding the problem.
In any case the existing implementation of psize
Why ever not? You can just update the mmu_psize_defs when you parse
the device tree. Plus AFAICT, the existing psize_to_rpti
implementation doesn't take into account any device tree eencodings.
> 2. If DT doesn't have the radix pagesize encodings, 4K and 64K
> sizes are assumed as fallback sizes where we can use direct
> encodings.
Right... still not seeing the problem.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH] crypto: sha: remove unneeded semicolon
On Mon, Feb 08, 2021 at 05:10:38PM +0800, Yang Li wrote: > Eliminate the following coccicheck warning: > ./arch/powerpc/crypto/sha1-spe-glue.c:110:2-3: Unneeded semicolon > > Reported-by: Abaci Robot > Signed-off-by: Yang Li > --- > arch/powerpc/crypto/sha1-spe-glue.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2] crypto/nx: add missing call to of_node_put()
On Fri, Feb 26, 2021 at 09:23:06AM +0800, Yang Li wrote: > In one of the error paths of the for_each_child_of_node() loop, > add missing call to of_node_put(). > > Fix the following coccicheck warning: > ./drivers/crypto/nx/nx-common-powernv.c:927:1-23: WARNING: Function > "for_each_child_of_node" should have of_node_put() before return around > line 936. > > Reported-by: Abaci Robot > Signed-off-by: Yang Li > --- > > Changes in v2: > -add braces for if > > drivers/crypto/nx/nx-common-powernv.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[PATCH] powerpc/perf: Fix the threshold event selection for memory events in power10
Memory events (mem-loads and mem-stores) currently use the threshold
event selection as issue to finish. Power10 supports issue to complete
as part of thresholding which is more appropriate for mem-loads and
mem-stores. Hence fix the event code for memory events to use issue
to complete.
Fixes: a64e697cef23 ("powerpc/perf: power10 Performance Monitoring support")
Signed-off-by: Athira Rajeev
---
arch/powerpc/perf/power10-events-list.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/perf/power10-events-list.h
b/arch/powerpc/perf/power10-events-list.h
index e45dafe818ed..93be7197d250 100644
--- a/arch/powerpc/perf/power10-events-list.h
+++ b/arch/powerpc/perf/power10-events-list.h
@@ -75,5 +75,5 @@
* thresh end (TE)
*/
-EVENT(MEM_LOADS, 0x34340401e0);
-EVENT(MEM_STORES, 0x343c0401e0);
+EVENT(MEM_LOADS, 0x35340401e0);
+EVENT(MEM_STORES, 0x353c0401e0);
--
1.8.3.1
Re: [PATCH v3 1/2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version
> On 01-Mar-2021, at 5:39 PM, Christophe Leroy
> wrote:
>
> From: Rashmica Gupta
>
> Currently the perf CPU backend drivers detect what CPU they're on using
> cur_cpu_spec->oprofile_cpu_type.
>
> Although that works, it's a bit crufty to be using oprofile related fields,
> especially seeing as oprofile is more or less unused these days.
>
> It also means perf is reliant on the fragile logic in setup_cpu_spec()
> which detects when we're using a logical PVR and copies back the PMU
> related fields from the raw CPU entry. So lets check the PVR directly.
>
> Suggested-by: Michael Ellerman
> Signed-off-by: Rashmica Gupta
> Reviewed-by: Madhavan Srinivasan
> [chleroy: Added power10 and fixed checkpatch issues]
> Signed-off-by: Christophe Leroy
Reviewed-and-tested-by: Athira Rajeev mailto:atraj...@linux.vnet.ibm.com>>
Thanks
Athira
> ---
> arch/powerpc/perf/e500-pmu.c| 9 +
> arch/powerpc/perf/e6500-pmu.c | 5 +++--
> arch/powerpc/perf/hv-24x7.c | 6 +++---
> arch/powerpc/perf/mpc7450-pmu.c | 5 +++--
> arch/powerpc/perf/power10-pmu.c | 6 ++
> arch/powerpc/perf/power5+-pmu.c | 6 +++---
> arch/powerpc/perf/power5-pmu.c | 5 +++--
> arch/powerpc/perf/power6-pmu.c | 5 +++--
> arch/powerpc/perf/power7-pmu.c | 7 ---
> arch/powerpc/perf/power8-pmu.c | 5 +++--
> arch/powerpc/perf/power9-pmu.c | 4 +---
> arch/powerpc/perf/ppc970-pmu.c | 7 ---
> 12 files changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/arch/powerpc/perf/e500-pmu.c b/arch/powerpc/perf/e500-pmu.c
> index a59c33bed32a..e3e1a68eb1d5 100644
> --- a/arch/powerpc/perf/e500-pmu.c
> +++ b/arch/powerpc/perf/e500-pmu.c
> @@ -118,12 +118,13 @@ static struct fsl_emb_pmu e500_pmu = {
>
> static int init_e500_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type)
> - return -ENODEV;
> + unsigned int pvr = mfspr(SPRN_PVR);
>
> - if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500mc"))
> + /* ec500mc */
> + if (PVR_VER(pvr) == PVR_VER_E500MC || PVR_VER(pvr) == PVR_VER_E5500)
> num_events = 256;
> - else if (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500"))
> + /* e500 */
> + else if (PVR_VER(pvr) != PVR_VER_E500V1 && PVR_VER(pvr) !=
> PVR_VER_E500V2)
> return -ENODEV;
>
> return register_fsl_emb_pmu(_pmu);
> diff --git a/arch/powerpc/perf/e6500-pmu.c b/arch/powerpc/perf/e6500-pmu.c
> index 44ad65da82ed..bd779a2338f8 100644
> --- a/arch/powerpc/perf/e6500-pmu.c
> +++ b/arch/powerpc/perf/e6500-pmu.c
> @@ -107,8 +107,9 @@ static struct fsl_emb_pmu e6500_pmu = {
>
> static int init_e6500_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e6500"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_VER_E6500)
> return -ENODEV;
>
> return register_fsl_emb_pmu(_pmu);
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index e5eb33255066..f3f2472fa1c6 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -1718,16 +1718,16 @@ static int hv_24x7_init(void)
> {
> int r;
> unsigned long hret;
> + unsigned int pvr = mfspr(SPRN_PVR);
> struct hv_perf_caps caps;
>
> if (!firmware_has_feature(FW_FEATURE_LPAR)) {
> pr_debug("not a virtualized system, not enabling\n");
> return -ENODEV;
> - } else if (!cur_cpu_spec->oprofile_cpu_type)
> - return -ENODEV;
> + }
>
> /* POWER8 only supports v1, while POWER9 only supports v2. */
> - if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
> + if (PVR_VER(pvr) == PVR_POWER8)
> interface_version = 1;
> else {
> interface_version = 2;
> diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c
> index e39b15b79a83..552d51a925d3 100644
> --- a/arch/powerpc/perf/mpc7450-pmu.c
> +++ b/arch/powerpc/perf/mpc7450-pmu.c
> @@ -417,8 +417,9 @@ struct power_pmu mpc7450_pmu = {
>
> static int __init init_mpc7450_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_7450)
> return -ENODEV;
>
> return register_power_pmu(_pmu);
> diff --git a/arch/powerpc/perf/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
> index a901c1348cad..d1395844a329 100644
> --- a/arch/powerpc/perf/power10-pmu.c
> +++ b/arch/powerpc/perf/power10-pmu.c
> @@ -566,12 +566,10 @@ int init_power10_pmu(void)
> unsigned int pvr;
> int rc;
>
> - /* Comes from cpu_specs[] */
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power10"))
> + pvr = mfspr(SPRN_PVR);
> + if (PVR_VER(pvr) != PVR_POWER10)
> return -ENODEV;
>
> - pvr = mfspr(SPRN_PVR);
> /*
Re: [PATCH v3] powerpc/uprobes: Validation for prefixed instruction
On 2021/03/04 10:35AM, Ravi Bangoria wrote:
> As per ISA 3.1, prefixed instruction should not cross 64-byte
> boundary. So don't allow Uprobe on such prefixed instruction.
>
> There are two ways probed instruction is changed in mapped pages.
> First, when Uprobe is activated, it searches for all the relevant
> pages and replace instruction in them. In this case, if that probe
> is on the 64-byte unaligned prefixed instruction, error out
> directly. Second, when Uprobe is already active and user maps a
> relevant page via mmap(), instruction is replaced via mmap() code
> path. But because Uprobe is invalid, entire mmap() operation can
> not be stopped. In this case just print an error and continue.
>
> Signed-off-by: Ravi Bangoria
> ---
> v2:
> https://lore.kernel.org/r/20210204104703.273429-1-ravi.bango...@linux.ibm.com
> v2->v3:
> - Drop restriction for Uprobe on suffix of prefixed instruction.
> It needs lot of code change including generic code but what
> we get in return is not worth it.
>
> arch/powerpc/kernel/uprobes.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> index e8a63713e655..c400971ebe70 100644
> --- a/arch/powerpc/kernel/uprobes.c
> +++ b/arch/powerpc/kernel/uprobes.c
> @@ -41,6 +41,14 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
> if (addr & 0x03)
> return -EINVAL;
>
> + if (!IS_ENABLED(CONFIG_PPC64) || !cpu_has_feature(CPU_FTR_ARCH_31))
> + return 0;
Sorry, I missed this last time, but I think we can drop the above
checks. ppc_inst_prefixed() already factors in the dependency on
CONFIG_PPC64, and I don't think we need to confirm if we're running on a
ISA V3.1 for the below check.
With that:
Acked-by: Naveen N. Rao
> +
> + if (ppc_inst_prefixed(auprobe->insn) && (addr & 0x3F) == 0x3C) {
> + pr_info_ratelimited("Cannot register a uprobe on 64 byte
> unaligned prefixed instruction\n");
> + return -EINVAL;
> + }
> +
- Naveen
Re: [PATCH v5 0/5] ibmvfc: hard reset fixes
On Tue, 2 Mar 2021 17:05:38 -0600, Tyrel Datwyler wrote: > This series contains a minor simplification of ibmvfc_init_sub_crqs() followed > by a couple fixes for sub-CRQ handling which effect hard reset of the > client/host adapter CRQ pair. > > changes in v5: > Patches 2-5: Corrected upstream commit ids for Fixes: tags > > [...] Applied to 5.12/scsi-fixes, thanks! [1/5] ibmvfc: simplify handling of sub-CRQ initialization https://git.kernel.org/mkp/scsi/c/2e415356fd6f [2/5] ibmvfc: fix invalid sub-CRQ handles after hard reset https://git.kernel.org/mkp/scsi/c/2de4c19179b1 [3/5] ibmvfc: treat H_CLOSED as success during sub-CRQ registration https://git.kernel.org/mkp/scsi/c/98cf9a92b8d6 [4/5] ibmvfc: store return code of H_FREE_SUB_CRQ during cleanup https://git.kernel.org/mkp/scsi/c/5bc26ea9498a [5/5] ibmvfc: reinitialize sub-CRQs and perform channel enquiry after LPM https://git.kernel.org/mkp/scsi/c/f4c5e949056d -- Martin K. Petersen Oracle Linux Engineering
[PATCH v3] powerpc/uprobes: Validation for prefixed instruction
As per ISA 3.1, prefixed instruction should not cross 64-byte
boundary. So don't allow Uprobe on such prefixed instruction.
There are two ways probed instruction is changed in mapped pages.
First, when Uprobe is activated, it searches for all the relevant
pages and replace instruction in them. In this case, if that probe
is on the 64-byte unaligned prefixed instruction, error out
directly. Second, when Uprobe is already active and user maps a
relevant page via mmap(), instruction is replaced via mmap() code
path. But because Uprobe is invalid, entire mmap() operation can
not be stopped. In this case just print an error and continue.
Signed-off-by: Ravi Bangoria
---
v2:
https://lore.kernel.org/r/20210204104703.273429-1-ravi.bango...@linux.ibm.com
v2->v3:
- Drop restriction for Uprobe on suffix of prefixed instruction.
It needs lot of code change including generic code but what
we get in return is not worth it.
arch/powerpc/kernel/uprobes.c | 8
1 file changed, 8 insertions(+)
diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index e8a63713e655..c400971ebe70 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -41,6 +41,14 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
if (addr & 0x03)
return -EINVAL;
+ if (!IS_ENABLED(CONFIG_PPC64) || !cpu_has_feature(CPU_FTR_ARCH_31))
+ return 0;
+
+ if (ppc_inst_prefixed(auprobe->insn) && (addr & 0x3F) == 0x3C) {
+ pr_info_ratelimited("Cannot register a uprobe on 64 byte
unaligned prefixed instruction\n");
+ return -EINVAL;
+ }
+
return 0;
}
--
2.27.0
Re: [PATCH v3 1/2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version
On 3/1/21 5:39 PM, Christophe Leroy wrote:
> From: Rashmica Gupta
>
> Currently the perf CPU backend drivers detect what CPU they're on using
> cur_cpu_spec->oprofile_cpu_type.
>
> Although that works, it's a bit crufty to be using oprofile related fields,
> especially seeing as oprofile is more or less unused these days.
>
> It also means perf is reliant on the fragile logic in setup_cpu_spec()
> which detects when we're using a logical PVR and copies back the PMU
> related fields from the raw CPU entry. So lets check the PVR directly.
>
> Suggested-by: Michael Ellerman
> Signed-off-by: Rashmica Gupta
> Reviewed-by: Madhavan Srinivasan
> [chleroy: Added power10 and fixed checkpatch issues]
> Signed-off-by: Christophe Leroy
Patch looks good to me.
Reviewed-and-tested-By: Kajol Jain [For 24x7 side changes]
Thanks,
Kajol Jain
> ---
> arch/powerpc/perf/e500-pmu.c| 9 +
> arch/powerpc/perf/e6500-pmu.c | 5 +++--
> arch/powerpc/perf/hv-24x7.c | 6 +++---
> arch/powerpc/perf/mpc7450-pmu.c | 5 +++--
> arch/powerpc/perf/power10-pmu.c | 6 ++
> arch/powerpc/perf/power5+-pmu.c | 6 +++---
> arch/powerpc/perf/power5-pmu.c | 5 +++--
> arch/powerpc/perf/power6-pmu.c | 5 +++--
> arch/powerpc/perf/power7-pmu.c | 7 ---
> arch/powerpc/perf/power8-pmu.c | 5 +++--
> arch/powerpc/perf/power9-pmu.c | 4 +---
> arch/powerpc/perf/ppc970-pmu.c | 7 ---
> 12 files changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/arch/powerpc/perf/e500-pmu.c b/arch/powerpc/perf/e500-pmu.c
> index a59c33bed32a..e3e1a68eb1d5 100644
> --- a/arch/powerpc/perf/e500-pmu.c
> +++ b/arch/powerpc/perf/e500-pmu.c
> @@ -118,12 +118,13 @@ static struct fsl_emb_pmu e500_pmu = {
>
> static int init_e500_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type)
> - return -ENODEV;
> + unsigned int pvr = mfspr(SPRN_PVR);
>
> - if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500mc"))
> + /* ec500mc */
> + if (PVR_VER(pvr) == PVR_VER_E500MC || PVR_VER(pvr) == PVR_VER_E5500)
> num_events = 256;
> - else if (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500"))
> + /* e500 */
> + else if (PVR_VER(pvr) != PVR_VER_E500V1 && PVR_VER(pvr) !=
> PVR_VER_E500V2)
> return -ENODEV;
>
> return register_fsl_emb_pmu(_pmu);
> diff --git a/arch/powerpc/perf/e6500-pmu.c b/arch/powerpc/perf/e6500-pmu.c
> index 44ad65da82ed..bd779a2338f8 100644
> --- a/arch/powerpc/perf/e6500-pmu.c
> +++ b/arch/powerpc/perf/e6500-pmu.c
> @@ -107,8 +107,9 @@ static struct fsl_emb_pmu e6500_pmu = {
>
> static int init_e6500_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e6500"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_VER_E6500)
> return -ENODEV;
>
> return register_fsl_emb_pmu(_pmu);
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index e5eb33255066..f3f2472fa1c6 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -1718,16 +1718,16 @@ static int hv_24x7_init(void)
> {
> int r;
> unsigned long hret;
> + unsigned int pvr = mfspr(SPRN_PVR);
> struct hv_perf_caps caps;
>
> if (!firmware_has_feature(FW_FEATURE_LPAR)) {
> pr_debug("not a virtualized system, not enabling\n");
> return -ENODEV;
> - } else if (!cur_cpu_spec->oprofile_cpu_type)
> - return -ENODEV;
> + }
>
> /* POWER8 only supports v1, while POWER9 only supports v2. */
> - if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
> + if (PVR_VER(pvr) == PVR_POWER8)
> interface_version = 1;
> else {
> interface_version = 2;
> diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c
> index e39b15b79a83..552d51a925d3 100644
> --- a/arch/powerpc/perf/mpc7450-pmu.c
> +++ b/arch/powerpc/perf/mpc7450-pmu.c
> @@ -417,8 +417,9 @@ struct power_pmu mpc7450_pmu = {
>
> static int __init init_mpc7450_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_7450)
> return -ENODEV;
>
> return register_power_pmu(_pmu);
> diff --git a/arch/powerpc/perf/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
> index a901c1348cad..d1395844a329 100644
> --- a/arch/powerpc/perf/power10-pmu.c
> +++ b/arch/powerpc/perf/power10-pmu.c
> @@ -566,12 +566,10 @@ int init_power10_pmu(void)
> unsigned int pvr;
> int rc;
>
> - /* Comes from cpu_specs[] */
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power10"))
> + pvr = mfspr(SPRN_PVR);
> + if (PVR_VER(pvr) != PVR_POWER10)
> return -ENODEV;
>
> - pvr =
[PATCH] arch:powerpc:kernel: remove duplicate include in traps.c
From: Zhang Yunkai 'asm/tm.h' included in 'traps.c' is duplicated. It is also included in the 62th line. Signed-off-by: Zhang Yunkai --- arch/powerpc/kernel/traps.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c index 1583fd1c6010..dcdb93588828 100644 --- a/arch/powerpc/kernel/traps.c +++ b/arch/powerpc/kernel/traps.c @@ -53,7 +53,6 @@ #ifdef CONFIG_PPC64 #include #include -#include #endif #include #include -- 2.25.1
[PATCH 5/5] CMDLINE: x86: convert to generic builtin command line
This updates the x86 code to use the CONFIG_GENERIC_CMDLINE
option.
Cc: xe-linux-exter...@cisco.com
Signed-off-by: Ruslan Ruslichenko
Signed-off-by: Ruslan Bilovol
Signed-off-by: Daniel Walker
---
arch/x86/Kconfig| 44 +
arch/x86/kernel/setup.c | 18 ++
drivers/firmware/efi/libstub/x86-stub.c | 2 +-
3 files changed, 4 insertions(+), 60 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 21f851179ff0..3950f9bf9855 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -115,6 +115,7 @@ config X86
select EDAC_SUPPORT
select GENERIC_CLOCKEVENTS_BROADCASTif X86_64 || (X86_32 &&
X86_LOCAL_APIC)
select GENERIC_CLOCKEVENTS_MIN_ADJUST
+ select GENERIC_CMDLINE
select GENERIC_CMOS_UPDATE
select GENERIC_CPU_AUTOPROBE
select GENERIC_CPU_VULNERABILITIES
@@ -2368,49 +2369,6 @@ choice
endchoice
-config CMDLINE_BOOL
- bool "Built-in kernel command line"
- help
- Allow for specifying boot arguments to the kernel at
- build time. On some systems (e.g. embedded ones), it is
- necessary or convenient to provide some or all of the
- kernel boot arguments with the kernel itself (that is,
- to not rely on the boot loader to provide them.)
-
- To compile command line arguments into the kernel,
- set this option to 'Y', then fill in the
- boot arguments in CONFIG_CMDLINE.
-
- Systems with fully functional boot loaders (i.e. non-embedded)
- should leave this option set to 'N'.
-
-config CMDLINE
- string "Built-in kernel command string"
- depends on CMDLINE_BOOL
- default ""
- help
- Enter arguments here that should be compiled into the kernel
- image and used at boot time. If the boot loader provides a
- command line at boot time, it is appended to this string to
- form the full kernel command line, when the system boots.
-
- However, you can use the CONFIG_CMDLINE_OVERRIDE option to
- change this behavior.
-
- In most cases, the command line (whether built-in or provided
- by the boot loader) should specify the device for the root
- file system.
-
-config CMDLINE_OVERRIDE
- bool "Built-in command line overrides boot loader arguments"
- depends on CMDLINE_BOOL && CMDLINE != ""
- help
- Set this option to 'Y' to have the kernel ignore the boot loader
- command line, and use ONLY the built-in command line.
-
- This is used to work around broken boot loaders. This should
- be set to 'N' under normal conditions.
-
config MODIFY_LDT_SYSCALL
bool "Enable the LDT (local descriptor table)" if EXPERT
default y
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 740f3bdb3f61..e748c3e5c1ae 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
/*
* max_low_pfn_mapped: highest directly mapped pfn < 4 GB
@@ -162,9 +163,6 @@ unsigned long saved_video_mode;
#define RAMDISK_LOAD_FLAG 0x4000
static char __initdata command_line[COMMAND_LINE_SIZE];
-#ifdef CONFIG_CMDLINE_BOOL
-static char __initdata builtin_cmdline[COMMAND_LINE_SIZE] = CONFIG_CMDLINE;
-#endif
#if defined(CONFIG_EDD) || defined(CONFIG_EDD_MODULE)
struct edd edd;
@@ -884,19 +882,7 @@ void __init setup_arch(char **cmdline_p)
bss_resource.start = __pa_symbol(__bss_start);
bss_resource.end = __pa_symbol(__bss_stop)-1;
-#ifdef CONFIG_CMDLINE_BOOL
-#ifdef CONFIG_CMDLINE_OVERRIDE
- strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
-#else
- if (builtin_cmdline[0]) {
- /* append boot loader cmdline to builtin */
- strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
- strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
- strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
- }
-#endif
-#endif
-
+ cmdline_add_builtin(boot_command_line, NULL, COMMAND_LINE_SIZE);
strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
*cmdline_p = command_line;
diff --git a/drivers/firmware/efi/libstub/x86-stub.c
b/drivers/firmware/efi/libstub/x86-stub.c
index f14c4ff5839f..9538c9d4a0bc 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -736,7 +736,7 @@ unsigned long efi_main(efi_handle_t handle,
}
#ifdef CONFIG_CMDLINE_BOOL
- status = efi_parse_options(CONFIG_CMDLINE);
+ status = efi_parse_options(CONFIG_CMDLINE_PREPEND " "
CONFIG_CMDLINE_APPEND);
if (status != EFI_SUCCESS) {
efi_err("Failed to parse options\n");
goto fail;
--
2.25.1
[PATCH 1/5] CMDLINE: add generic builtin command line
This code allows architectures to use a generic builtin command line.
The state of the builtin command line options across architecture is
diverse. On x86 and mips they have pretty much the same code and the
code prepends the builtin command line onto the boot loader provided
one. On powerpc there is only a builtin override and nothing else.
The code in this commit unifies the code into a generic
header file under the CONFIG_GENERIC_CMDLINE option. When this
option is enabled the architecture can call the cmdline_add_builtin()
to add the builtin command line.
Cc: xe-linux-exter...@cisco.com
Signed-off-by: Ruslan Bilovol
Signed-off-by: Daniel Walker
---
include/linux/cmdline.h | 75 +
init/Kconfig| 68 +
2 files changed, 143 insertions(+)
create mode 100644 include/linux/cmdline.h
diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
new file mode 100644
index ..f44011d1a9ee
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,75 @@
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+
+/*
+ *
+ * Copyright (C) 2006,2021. Cisco Systems, Inc.
+ *
+ * Generic Append/Prepend cmdline support.
+ */
+
+#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_CMDLINE_BOOL)
+
+#ifndef CONFIG_CMDLINE_OVERRIDE
+/*
+ * This function will append or prepend a builtin command line to the command
+ * line provided by the bootloader. Kconfig options can be used to alter
+ * the behavior of this builtin command line.
+ * @dest: The destination of the final appended/prepended string
+ * @src: The starting string or NULL if there isn't one.
+ * @tmp: temporary space used for prepending
+ * @length: the maximum length of the strings above.
+ */
+static inline void
+__cmdline_add_builtin(char *dest, const char *src, char *tmp, unsigned long
length,
+ size_t (*strlcpy)(char *dest, const char *src, size_t size),
+ size_t (*strlcat)(char *dest, const char *src, size_t count)
+ )
+{
+ if (src != dest && src != NULL) {
+ strlcpy(dest, " ", length);
+ strlcat(dest, src, length);
+ }
+
+ if (sizeof(CONFIG_CMDLINE_APPEND) > 1)
+ strlcat(dest, " " CONFIG_CMDLINE_APPEND, length);
+
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
+ strlcpy(tmp, CONFIG_CMDLINE_PREPEND " ", length);
+ strlcat(tmp, dest, length);
+ strlcpy(dest, tmp, length);
+ }
+}
+
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat)
\
+{
\
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
\
+ static label char cmdline_tmp_space[length];
\
+ __cmdline_add_builtin(dest, src, cmdline_tmp_space, length,
strlcpy, strlcat); \
+ } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) {
\
+ __cmdline_add_builtin(dest, src, NULL, length, strlcpy,
strlcat); \
+ }
\
+}
+#define cmdline_add_builtin(dest, src, length)\
+ cmdline_add_builtin_custom(dest, src, length, __initdata, ,
)
+#else
+#define cmdline_add_builtin(dest, src, length)\
+{ \
+ strlcpy(dest, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND,\
+ length); \
+}
+#endif /* !CONFIG_CMDLINE_OVERRIDE */
+
+#else
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat)
{ \
+ if (src != NULL)
\
+ strlcpy(dest, src, length);
\
+}
+
+#define cmdline_add_builtin(dest, src, length) {
\
+ cmdline_add_builtin_custom(dest, src, length, strlcpy, strlcat);
\
+}
+#endif /* CONFIG_GENERIC_CMDLINE */
+
+
+#endif /* _LINUX_CMDLINE_H */
diff --git a/init/Kconfig b/init/Kconfig
index 29ad68325028..28363ab07cd4 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2032,6 +2032,74 @@ config PROFILING
config TRACEPOINTS
bool
+config GENERIC_CMDLINE
+ bool
+
+if GENERIC_CMDLINE
+
+config CMDLINE_BOOL
+ bool "Built-in kernel command line"
+ help
+ Allow for specifying boot arguments to the kernel at
+ build time. On some systems (e.g. embedded ones), it is
+ necessary or convenient to provide some or all of the
+ kernel boot arguments with the kernel itself (that is,
+ to not rely on the boot loader to provide them.)
+
+
[PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section
It looks like there's some seepage of cmdline stuff into
the generic device tree code. This conflicts with the
generic cmdline implementation so I remove it in the case
when that's enabled.
Cc: xe-linux-exter...@cisco.com
Signed-off-by: Ruslan Ruslichenko
Signed-off-by: Daniel Walker
---
drivers/of/fdt.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index feb0f2d67fc5..cfe4f8d3c9f5 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include /* for COMMAND_LINE_SIZE */
#include
@@ -1048,8 +1049,18 @@ int __init early_init_dt_scan_chosen(unsigned long node,
const char *uname,
early_init_dt_check_for_initrd(node);
+#ifdef CONFIG_GENERIC_CMDLINE
/* Retrieve command line */
p = of_get_flat_dt_prop(node, "bootargs", );
+
+ /*
+* The builtin command line will be added here, or it can override
+* with the DT bootargs.
+*/
+ cmdline_add_builtin(data,
+ ((p != NULL && l > 0) ? p : NULL), /* This is
sanity checking */
+ COMMAND_LINE_SIZE);
+#else
if (p != NULL && l > 0)
strlcpy(data, p, min(l, COMMAND_LINE_SIZE));
@@ -1070,6 +1081,7 @@ int __init early_init_dt_scan_chosen(unsigned long node,
const char *uname,
strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
#endif
#endif /* CONFIG_CMDLINE */
+#endif /* CONFIG_GENERIC_CMDLINE */
pr_debug("Command line is: %s\n", (char *)data);
--
2.25.1
[PATCH 3/5] CMDLINE: powerpc: convert to generic builtin command line
This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
option.
Cc: xe-linux-exter...@cisco.com
Signed-off-by: Ruslan Ruslichenko
Signed-off-by: Ruslan Bilovol
Signed-off-by: Daniel Walker
---
arch/powerpc/Kconfig| 37 +
arch/powerpc/kernel/prom.c | 1 +
arch/powerpc/kernel/prom_init.c | 31 +++
3 files changed, 20 insertions(+), 49 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 107bb4319e0e..276b06d5c961 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -167,6 +167,7 @@ config PPC
select EDAC_SUPPORT
select GENERIC_ATOMIC64 if PPC32
select GENERIC_CLOCKEVENTS_BROADCASTif SMP
+ select GENERIC_CMDLINE
select GENERIC_CMOS_UPDATE
select GENERIC_CPU_AUTOPROBE
select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
@@ -906,42 +907,6 @@ config PPC_DENORMALISATION
Add support for handling denormalisation of single precision
values. Useful for bare metal only. If unsure say Y here.
-config CMDLINE
- string "Initial kernel command string"
- default ""
- help
- On some platforms, there is currently no way for the boot loader to
- pass arguments to the kernel. For these platforms, you can supply
- some command-line options at build time by entering them here. In
- most cases you will need to specify the root device here.
-
-choice
- prompt "Kernel command line type" if CMDLINE != ""
- default CMDLINE_FROM_BOOTLOADER
-
-config CMDLINE_FROM_BOOTLOADER
- bool "Use bootloader kernel arguments if available"
- help
- Uses the command-line options passed by the boot loader. If
- the boot loader doesn't provide any, the default kernel command
- string provided in CMDLINE will be used.
-
-config CMDLINE_EXTEND
- bool "Extend bootloader kernel arguments"
- help
- The command-line arguments provided by the boot loader will be
- appended to the default kernel command string.
-
-config CMDLINE_FORCE
- bool "Always use the default kernel command string"
- help
- Always use the default kernel command string, even if the boot
- loader passes other arguments to the kernel.
- This is useful if you cannot or don't want to change the
- command-line options your boot loader passes to the kernel.
-
-endchoice
-
config EXTRA_TARGETS
string "Additional default image types"
help
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index ae3c41730367..96d0a01be1b4 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index e9d4eb6144e1..d752be688b62 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -242,15 +243,6 @@ static int __init prom_strcmp(const char *cs, const char
*ct)
return 0;
}
-static char __init *prom_strcpy(char *dest, const char *src)
-{
- char *tmp = dest;
-
- while ((*dest++ = *src++) != '\0')
- /* nothing */;
- return tmp;
-}
-
static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
{
unsigned char c1, c2;
@@ -276,6 +268,19 @@ static size_t __init prom_strlen(const char *s)
return sc - s;
}
+static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)
+{
+ size_t ret = prom_strlen(src);
+
+ if (size) {
+ size_t len = (ret >= size) ? size - 1 : ret;
+ memcpy(dest, src, len);
+ dest[len] = '\0';
+ }
+ return ret;
+}
+
+
static int __init prom_memcmp(const void *cs, const void *ct, size_t count)
{
const unsigned char *su1, *su2;
@@ -778,9 +783,9 @@ static void __init early_cmdline_parse(void)
if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)
l = prom_getprop(prom.chosen, "bootargs", p,
COMMAND_LINE_SIZE-1);
- if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\0')
- prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
-sizeof(prom_cmd_line));
+ if (l <= 0 || p[0] == '\0') /* dbl check */
+ cmdline_add_builtin_custom(prom_cmd_line, NULL,
sizeof(prom_cmd_line),
+ __prombss, _strlcpy,
_strlcat);
prom_printf("command line: %s\n", prom_cmd_line);
@@ -2706,7 +2711,7 @@ static void __init flatten_device_tree(void)
/* Add "phandle" in there, we'll need it */
namep = make_room(_start, _end, 16, 1);
- prom_strcpy(namep,
[PATCH 4/5] CMDLINE: mips: convert to generic builtin command line
This updates the mips code to use the CONFIG_GENERIC_CMDLINE
option.
This deletes the option for MIPS_CMDLINE_BUILTIN_EXTEND
and replaces the functionality with generic code.
Cc: xe-linux-exter...@cisco.com
Signed-off-by: Ruslan Ruslichenko
Signed-off-by: Ruslan Bilovol
Signed-off-by: Daniel Walker
---
arch/mips/Kconfig| 4 +---
arch/mips/Kconfig.debug | 44
arch/mips/kernel/setup.c | 37 +
3 files changed, 6 insertions(+), 79 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 0a17bedf4f0d..7c07b3bca9fc 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -23,6 +23,7 @@ config MIPS
select CPU_NO_EFFICIENT_FFS if (TARGET_ISA_REV < 1)
select CPU_PM if CPU_IDLE
select GENERIC_ATOMIC64 if !64BIT
+ select GENERIC_CMDLINE
select GENERIC_CMOS_UPDATE
select GENERIC_CPU_AUTOPROBE
select GENERIC_GETTIMEOFDAY
@@ -3171,9 +3172,6 @@ choice
config MIPS_CMDLINE_FROM_BOOTLOADER
bool "Bootloader kernel arguments if available"
- config MIPS_CMDLINE_BUILTIN_EXTEND
- depends on CMDLINE_BOOL
- bool "Extend builtin kernel arguments with bootloader arguments"
endchoice
endmenu
diff --git a/arch/mips/Kconfig.debug b/arch/mips/Kconfig.debug
index 7a8d94cdd493..b5a099c74eb6 100644
--- a/arch/mips/Kconfig.debug
+++ b/arch/mips/Kconfig.debug
@@ -30,50 +30,6 @@ config EARLY_PRINTK_8250
config USE_GENERIC_EARLY_PRINTK_8250
bool
-config CMDLINE_BOOL
- bool "Built-in kernel command line"
- help
- For most systems, it is firmware or second stage bootloader that
- by default specifies the kernel command line options. However,
- it might be necessary or advantageous to either override the
- default kernel command line or add a few extra options to it.
- For such cases, this option allows you to hardcode your own
- command line options directly into the kernel. For that, you
- should choose 'Y' here, and fill in the extra boot arguments
- in CONFIG_CMDLINE.
-
- The built-in options will be concatenated to the default command
- line if CMDLINE_OVERRIDE is set to 'N'. Otherwise, the default
- command line will be ignored and replaced by the built-in string.
-
- Most MIPS systems will normally expect 'N' here and rely upon
- the command line from the firmware or the second-stage bootloader.
-
-config CMDLINE
- string "Default kernel command string"
- depends on CMDLINE_BOOL
- help
- On some platforms, there is currently no way for the boot loader to
- pass arguments to the kernel. For these platforms, and for the cases
- when you want to add some extra options to the command line or ignore
- the default command line, you can supply some command-line options at
- build time by entering them here. In other cases you can specify
- kernel args so that you don't have to set them up in board prom
- initialization routines.
-
- For more information, see the CMDLINE_BOOL and CMDLINE_OVERRIDE
- options.
-
-config CMDLINE_OVERRIDE
- bool "Built-in command line overrides firmware arguments"
- depends on CMDLINE_BOOL
- help
- By setting this option to 'Y' you will have your kernel ignore
- command line arguments from firmware or second stage bootloader.
- Instead, the built-in command line will be used exclusively.
-
- Normally, you will choose 'N' here.
-
config SB1XXX_CORELIS
bool "Corelis Debugger"
depends on SIBYTE_SB1xxx_SOC
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 7e1f8e277437..b7e9c1c9 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -67,12 +68,6 @@ EXPORT_SYMBOL(mips_machtype);
static char __initdata command_line[COMMAND_LINE_SIZE];
char __initdata arcs_cmdline[COMMAND_LINE_SIZE];
-#ifdef CONFIG_CMDLINE_BOOL
-static const char builtin_cmdline[] __initconst = CONFIG_CMDLINE;
-#else
-static const char builtin_cmdline[] __initconst = "";
-#endif
-
/*
* mips_io_port_base is the begin of the address space to which x86 style
* I/O ports are mapped.
@@ -546,27 +541,7 @@ static void __init bootcmdline_init(void)
{
bool dt_bootargs = false;
- /*
-* If CMDLINE_OVERRIDE is enabled then initializing the command line is
-* trivial - we simply use the built-in command line unconditionally &
-* unmodified.
-*/
- if (IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
- strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
- return;
- }
-
- /*
-* If the user specified a built-in command line &
-
[PATCH] arch:powerpc:kernel: remove duplicate include in setup-common
From: Zhang Yunkai 'asm/udbg.h' included in 'setup-common.c' is duplicated. It is also included in the 61th line. Signed-off-by: Zhang Yunkai --- arch/powerpc/kernel/setup-common.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index bee984b1887b..7221f11acf04 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -69,7 +69,6 @@ #include "setup.h" #ifdef DEBUG -#include #define DBG(fmt...) udbg_printf(fmt) #else #define DBG(fmt...) -- 2.25.1
Re: [PATCH V2] mm: Generalize HUGETLB_PAGE_SIZE_VARIABLE
On 3/2/21 10:43 AM, Anshuman Khandual wrote: > HUGETLB_PAGE_SIZE_VARIABLE need not be defined for each individual > platform subscribing it. Instead just make it generic. > > Cc: Michael Ellerman > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: Andrew Morton > Cc: Christoph Hellwig > Cc: linux-i...@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Cc: linux...@kvack.org > Cc: linux-ker...@vger.kernel.org > Suggested-by: Christoph Hellwig > Signed-off-by: Anshuman Khandual > --- > This change was originally suggested in an earilier discussion. This > applies on v5.12-rc1 and has been build tested on all applicable > platforms i.e ia64 and powerpc. > > https://patchwork.kernel.org/project/linux-mm/patch/1613024531-19040-3-git-send-email-anshuman.khand...@arm.com/ > > Changes in V2: > > - Added a description for HUGETLB_PAGE_SIZE_VARIABLE > - Added HUGETLB_PAGE dependency while selecting HUGETLB_PAGE_SIZE_VARIABLE > > Changes in V1: > > https://patchwork.kernel.org/project/linux-mm/patch/1614577853-7452-1-git-send-email-anshuman.khand...@arm.com/ > > arch/ia64/Kconfig| 6 +- > arch/powerpc/Kconfig | 6 +- > mm/Kconfig | 9 + > 3 files changed, 11 insertions(+), 10 deletions(-) > > diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig > index 2ad7a8d29fcc..dccf5bfebf48 100644 > --- a/arch/ia64/Kconfig > +++ b/arch/ia64/Kconfig > @@ -32,6 +32,7 @@ config IA64 > select TTY > select HAVE_ARCH_TRACEHOOK > select HAVE_VIRT_CPU_ACCOUNTING > + select HUGETLB_PAGE_SIZE_VARIABLE if HUGETLB_PAGE > select VIRT_TO_BUS > select GENERIC_IRQ_PROBE > select GENERIC_PENDING_IRQ if SMP > @@ -82,11 +83,6 @@ config STACKTRACE_SUPPORT > config GENERIC_LOCKBREAK > def_bool n > > -config HUGETLB_PAGE_SIZE_VARIABLE > - bool > - depends on HUGETLB_PAGE > - default y > - > config GENERIC_CALIBRATE_DELAY > bool > default y > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index 3778ad17f56a..3fdec3e53256 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -232,6 +232,7 @@ config PPC > select HAVE_HARDLOCKUP_DETECTOR_PERFif PERF_EVENTS && > HAVE_PERF_EVENTS_NMI && !HAVE_HARDLOCKUP_DETECTOR_ARCH > select HAVE_PERF_REGS > select HAVE_PERF_USER_STACK_DUMP > + select HUGETLB_PAGE_SIZE_VARIABLE if PPC_BOOK3S_64 && HUGETLB_PAGE > select MMU_GATHER_RCU_TABLE_FREE > select MMU_GATHER_PAGE_SIZE > select HAVE_REGS_AND_STACK_ACCESS_API > @@ -416,11 +417,6 @@ config HIGHMEM > > source "kernel/Kconfig.hz" > > -config HUGETLB_PAGE_SIZE_VARIABLE > - bool > - depends on HUGETLB_PAGE && PPC_BOOK3S_64 > - default y > - > config MATH_EMULATION > bool "Math emulation" > depends on 4xx || PPC_8xx || PPC_MPC832x || BOOKE > diff --git a/mm/Kconfig b/mm/Kconfig > index 24c045b24b95..64f1e0503e4f 100644 > --- a/mm/Kconfig > +++ b/mm/Kconfig > @@ -274,6 +274,15 @@ config ARCH_ENABLE_HUGEPAGE_MIGRATION > config ARCH_ENABLE_THP_MIGRATION > bool > > +config HUGETLB_PAGE_SIZE_VARIABLE > + bool "Allows dynamic pageblock_order" > + def_bool n > + depends on HUGETLB_PAGE Seems like this dependency on HUGETLB_PAGE is redundant, as it is already being ensured on the platforms while selecting the config. > + help > + Allows the pageblock_order value to be dynamic instead of just > standard > + HUGETLB_PAGE_ORDER when there are multiple HugeTLB page sizes > available > + on a platform. > + > config CONTIG_ALLOC > def_bool (MEMORY_ISOLATION && COMPACTION) || CMA > >
[PATCH] arch/powerpc/include: remove duplicate include in thread_info.h
From: Zhang Yunkai 'asm/page.h' included in 'arch/powerpc/include/asm/thread_info.h' is duplicated.It is also included in 13th line. Signed-off-by: Zhang Yunkai --- arch/powerpc/include/asm/thread_info.h | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h index 386d576673a1..9d6402402b9b 100644 --- a/arch/powerpc/include/asm/thread_info.h +++ b/arch/powerpc/include/asm/thread_info.h @@ -38,7 +38,6 @@ #ifndef __ASSEMBLY__ #include #include -#include #include #define SLB_PRELOAD_NR 16U -- 2.25.1
[PATCH] arch/powerpc/include: remove duplicate include in pgtable.h
From: Zhang Yunkai 'asm/tlbflush.h' included in 'arch/powerpc/include/asm/pgtable.h' is duplicated.It is also included in the 11th line. Signed-off-by: Zhang Yunkai --- arch/powerpc/include/asm/pgtable.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h index 4eed82172e33..c6a676714f04 100644 --- a/arch/powerpc/include/asm/pgtable.h +++ b/arch/powerpc/include/asm/pgtable.h @@ -41,8 +41,6 @@ struct mm_struct; #ifndef __ASSEMBLY__ -#include - /* Keep these as a macros to avoid include dependency mess */ #define pte_page(x)pfn_to_page(pte_pfn(x)) #define mk_pte(page, pgprot) pfn_pte(page_to_pfn(page), (pgprot)) -- 2.25.1
[PATCH] arch/powerpc/include:fix misspellings in tlbflush.h
From: Zhang Yunkai Some typos are found out.The information at the end of the file does not match the beginning. Signed-off-by: Zhang Yunkai --- arch/powerpc/include/asm/book3s/32/tlbflush.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h b/arch/powerpc/include/asm/book3s/32/tlbflush.h index d941c06d4f2e..ba1743c52b56 100644 --- a/arch/powerpc/include/asm/book3s/32/tlbflush.h +++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h @@ -79,4 +79,4 @@ static inline void local_flush_tlb_mm(struct mm_struct *mm) flush_tlb_mm(mm); } -#endif /* _ASM_POWERPC_TLBFLUSH_H */ +#endif /* _ASM_POWERPC_BOOK3S_32_TLBFLUSH_H */ -- 2.25.1
[PATCH] arch/powerpc/include/asm/book3s/64/: remove duplicate include in mmu-hash.h
From: Zhang Yunkai 'asm/bug.h' included in 'arch/powerpc/include/asm/book3s/64/mmu-hash.h' is duplicated.It is also included in the 12th line. Signed-off-by: Zhang Yunkai --- arch/powerpc/include/asm/book3s/64/mmu-hash.h | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h index f911bdb68d8b..3004f3323144 100644 --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h @@ -18,7 +18,6 @@ * complete pgtable.h but only a portion of it. */ #include -#include #include #include -- 2.25.1
Re: [PATCH][next] ASoC: fsl: fsl_easrc: Fix uninitialized variable st2_mem_alloc
On Wed, 3 Mar 2021 09:18:35 +, Colin King wrote: > A previous cleanup commit removed the ininitialization of st2_mem_alloc. > Fix this by restoring the original behaviour by initializing it to zero. Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: fsl: fsl_easrc: Fix uninitialized variable st2_mem_alloc commit: 84e4eb57ed620adc0371579a5662c4924a72a306 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Re: [PATCH] powerpc: remove redundant space
maqiang a écrit :
These one line of code don't meet the kernel coding style,
so remove the redundant space.
There seems to be several other style issues in this function and in
the following one too. You should fix them all at once I think.
Signed-off-by: maqiang
---
arch/powerpc/kernel/syscalls.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 078608ec2e92..9248288752d5 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -81,7 +81,7 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
int
ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set
__user *exp, struct __kernel_old_timeval __user *tvp)
{
- if ( (unsigned long)n >= 4096 )
+ if ((unsigned long)n >= 4096)
{
unsigned long __user *buffer = (unsigned long __user *)n;
if (!access_ok(buffer, 5*sizeof(unsigned long))
--
2.20.1
[Bug 210749] sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd'
https://bugzilla.kernel.org/show_bug.cgi?id=210749 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Attachment #294195|0 |1 is obsolete|| --- Comment #5 from Erhard F. (erhar...@mailbox.org) --- Created attachment 295621 --> https://bugzilla.kernel.org/attachment.cgi?id=295621=edit kernel .config (kernel 5.12-rc1, Talos II) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 210749] sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd'
https://bugzilla.kernel.org/show_bug.cgi?id=210749 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Attachment #294193|0 |1 is obsolete|| Attachment #294449|0 |1 is obsolete|| --- Comment #4 from Erhard F. (erhar...@mailbox.org) --- Created attachment 295619 --> https://bugzilla.kernel.org/attachment.cgi?id=295619=edit dmesg (kernel 5.12-rc1, Talos II) Same for 5.12-rc1. [...] at24 0-0050: probe at24 0-0050: 65536 byte 24c512 EEPROM, writable, 1 bytes/write at24 1-0050: probe sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd' CPU: 23 PID: 378 Comm: systemd-udevd Not tainted 5.12.0-rc1-TalosII #2 Call Trace: [c000283a6ae0] [c083649c] .dump_stack+0xf8/0x16c (unreliable) [c000283a6b80] [c051e628] .sysfs_warn_dup+0x78/0xb0 [c000283a6c10] [c051ec6c] .sysfs_do_create_link_sd.isra.0+0x13c/0x150 [c000283a6cb0] [c0912b90] .bus_add_device+0x80/0x190 [c000283a6d40] [c090ef18] .device_add+0x438/0xaa0 [c000283a6e60] [c09dff90] .nvmem_register+0x2a0/0xc00 [c000283a6f50] [c09e093c] .devm_nvmem_register+0x4c/0xb0 [c000283a6fe0] [c00806a4427c] .at24_probe+0x67c/0x8a0 [at24] [c000283a7270] [c09a90f8] .i2c_device_probe+0x158/0x3c0 [c000283a7300] [c09144f4] .really_probe+0x134/0x500 [c000283a73b0] [c0914938] .driver_probe_device+0x78/0x110 [c000283a7430] [c0914eac] .device_driver_attach+0xbc/0xf0 [c000283a74c0] [c0914f58] .__driver_attach+0x78/0x140 [c000283a7550] [c091139c] .bus_for_each_dev+0x9c/0x120 [c000283a7600] [c0913bf4] .driver_attach+0x24/0x40 [c000283a7670] [c0913148] .bus_add_driver+0x1c8/0x2a0 [c000283a7710] [c0915948] .driver_register+0x88/0x190 [c000283a7790] [c09aa2d8] .i2c_register_driver+0x58/0xc0 [c000283a7810] [c00806a445f4] .at24_init+0x5c/0x70 [at24] [c000283a7880] [c00110ac] .do_one_initcall+0x7c/0x480 [c000283a7970] [c01f4c3c] .do_init_module+0x6c/0x2f0 [c000283a7a00] [c01f7dcc] .load_module+0x2ccc/0x34a0 [c000283a7c20] [c01f8818] .__do_sys_finit_module+0xc8/0x120 [c000283a7d50] [c0037194] .system_call_exception+0x1b4/0x3b0 [c000283a7e10] [c000c8dc] system_call_common+0xec/0x278 --- interrupt: c00 at 0x3fffa40f78b4 NIP: 3fffa40f78b4 LR: 3fffa42d8b04 CTR: REGS: c000283a7e80 TRAP: 0c00 Not tainted (5.12.0-rc1-TalosII) MSR: 9280f032 CR: 4842 XER: IRQMASK: 0 GPR00: 0161 3fffd30fb260 3fffa41d1300 000f GPR04: 3fffa42e4630 000f GPR08: 0002 GPR12: 3fffa46fc810 0002 3fffd30fb738 GPR16: 000f4240 GPR20: 00015f2623a0 3fffa45e01b8 3fffa45e0178 GPR24: 00015f23d0e0 3fffa45e0158 0002 00015f1a4150 GPR28: 3fffa42e4630 0002 00015f2623a0 NIP [3fffa40f78b4] 0x3fffa40f78b4 LR [3fffa42d8b04] 0x3fffa42d8b04 --- interrupt: c00 at24: probe of 1-0050 failed with error -17 at24 2-0050: probe sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd' [...] -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH] powerpc: remove redundant space
These one line of code don't meet the kernel coding style,
so remove the redundant space.
Signed-off-by: maqiang
---
arch/powerpc/kernel/syscalls.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 078608ec2e92..9248288752d5 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -81,7 +81,7 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
int
ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp,
struct __kernel_old_timeval __user *tvp)
{
- if ( (unsigned long)n >= 4096 )
+ if ((unsigned long)n >= 4096)
{
unsigned long __user *buffer = (unsigned long __user *)n;
if (!access_ok(buffer, 5*sizeof(unsigned long))
--
2.20.1
[Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
https://bugzilla.kernel.org/show_bug.cgi?id=206203 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |OBSOLETE --- Comment #21 from Erhard F. (erhar...@mailbox.org) --- Kmemleak remains silent since at least 5.11.x on these machines. Which is a good sign I guess. ;) Closing for now. In case I hit these of: based memleaks again I will re-open. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v2 34/37] KVM: PPC: Book3S HV: add virtual mode handlers for HPT hcalls and page faults
Nicholas Piggin writes:
> In order to support hash guests in the P9 path (which does not do real
> mode hcalls or page fault handling), these real-mode hash specific
> interrupts need to be implemented in virt mode.
>
> Signed-off-by: Nicholas Piggin
> ---
> arch/powerpc/kvm/book3s_hv.c | 118 +--
> 1 file changed, 113 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 9d2fa21201c1..1bbc46f2cfbf 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -935,6 +935,52 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> return RESUME_HOST;
>
> switch (req) {
> + case H_REMOVE:
> + ret = kvmppc_h_remove(vcpu, kvmppc_get_gpr(vcpu, 4),
> + kvmppc_get_gpr(vcpu, 5),
> + kvmppc_get_gpr(vcpu, 6));
> + if (ret == H_TOO_HARD)
> + return RESUME_HOST;
> + break;
> + case H_ENTER:
> + ret = kvmppc_h_enter(vcpu, kvmppc_get_gpr(vcpu, 4),
> + kvmppc_get_gpr(vcpu, 5),
> + kvmppc_get_gpr(vcpu, 6),
> + kvmppc_get_gpr(vcpu, 7));
> + if (ret == H_TOO_HARD)
> + return RESUME_HOST;
> + break;
> + case H_READ:
> + ret = kvmppc_h_read(vcpu, kvmppc_get_gpr(vcpu, 4),
> + kvmppc_get_gpr(vcpu, 5));
> + if (ret == H_TOO_HARD)
> + return RESUME_HOST;
> + break;
> + case H_CLEAR_MOD:
> + ret = kvmppc_h_clear_mod(vcpu, kvmppc_get_gpr(vcpu, 4),
> + kvmppc_get_gpr(vcpu, 5));
> + if (ret == H_TOO_HARD)
> + return RESUME_HOST;
> + break;
> + case H_CLEAR_REF:
> + ret = kvmppc_h_clear_ref(vcpu, kvmppc_get_gpr(vcpu, 4),
> + kvmppc_get_gpr(vcpu, 5));
> + if (ret == H_TOO_HARD)
> + return RESUME_HOST;
> + break;
> + case H_PROTECT:
> + ret = kvmppc_h_protect(vcpu, kvmppc_get_gpr(vcpu, 4),
> + kvmppc_get_gpr(vcpu, 5),
> + kvmppc_get_gpr(vcpu, 6));
> + if (ret == H_TOO_HARD)
> + return RESUME_HOST;
> + break;
> + case H_BULK_REMOVE:
> + ret = kvmppc_h_bulk_remove(vcpu);
> + if (ret == H_TOO_HARD)
> + return RESUME_HOST;
> + break;
> +
Some of these symbols need to be exported.
ERROR: modpost: "kvmppc_h_bulk_remove" [arch/powerpc/kvm/kvm-hv.ko] undefined!
ERROR: modpost: "kvmppc_h_clear_mod" [arch/powerpc/kvm/kvm-hv.ko] undefined!
ERROR: modpost: "kvmppc_xive_xics_hcall" [arch/powerpc/kvm/kvm-hv.ko] undefined!
ERROR: modpost: "kvmppc_h_remove" [arch/powerpc/kvm/kvm-hv.ko] undefined!
ERROR: modpost: "decrementers_next_tb" [arch/powerpc/kvm/kvm-hv.ko] undefined!
ERROR: modpost: "kvmppc_hpte_hv_fault" [arch/powerpc/kvm/kvm-hv.ko] undefined!
ERROR: modpost: "kvmppc_h_protect" [arch/powerpc/kvm/kvm-hv.ko] undefined!
ERROR: modpost: "kvmppc_h_enter" [arch/powerpc/kvm/kvm-hv.ko] undefined!
ERROR: modpost: "kvmppc_h_clear_ref" [arch/powerpc/kvm/kvm-hv.ko] undefined!
ERROR: modpost: "kvmppc_h_read" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> case H_CEDE:
> break;
> case H_PROD:
> @@ -1134,6 +1180,7 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> default:
> return RESUME_HOST;
> }
> + WARN_ON_ONCE(ret == H_TOO_HARD);
> kvmppc_set_gpr(vcpu, 3, ret);
> vcpu->arch.hcall_needed = 0;
> return RESUME_GUEST;
> @@ -1420,19 +1467,80 @@ static int kvmppc_handle_exit_hv(struct kvm_vcpu
> *vcpu,
>* host page has been paged out. Any other HDSI/HISI interrupts
>* have been handled already.
>*/
> - case BOOK3S_INTERRUPT_H_DATA_STORAGE:
> - r = RESUME_PAGE_FAULT;
> - if (vcpu->arch.fault_dsisr == HDSISR_CANARY)
> + case BOOK3S_INTERRUPT_H_DATA_STORAGE: {
> + unsigned long vsid;
> + long err;
> +
> + if (vcpu->arch.fault_dsisr == HDSISR_CANARY) {
> r = RESUME_GUEST; /* Just retry if it's the canary */
> + break;
> + }
> +
> + if (kvm_is_radix(vcpu->kvm)) {
> + r = RESUME_PAGE_FAULT;
> + break;
> + }
> +
> + if (!(vcpu->arch.fault_dsisr & (DSISR_NOHPTE |
> DSISR_PROTFAULT))) {
> + kvmppc_core_queue_data_storage(vcpu,
> vcpu->arch.fault_dar, vcpu->arch.fault_dsisr);
> + r = RESUME_GUEST;
> + break;
> +
Re: [PATCH next v4 00/15] printk: remove logbuf_lock
On Wed 2021-03-03 11:15:13, John Ogness wrote: > Hello, > > Here is v4 of a series to remove @logbuf_lock, exposing the > ringbuffer locklessly to both readers and writers. v3 is > here [0]. The series look ready. I am going to push it into printk/linux.git the following week unless anyone speaks against it in the meantime. Best Regards, Petr
Re: [PATCH v1] powerpc: Include running function as first entry in save_stack_trace() and friends
On Wed, Mar 03, 2021 at 03:52PM +0100, Christophe Leroy wrote:
> Le 03/03/2021 à 15:38, Marco Elver a écrit :
> > On Wed, 3 Mar 2021 at 15:09, Christophe Leroy
> > wrote:
> > >
> > > It seems like all other sane architectures, namely x86 and arm64
> > > at least, include the running function as top entry when saving
> > > stack trace.
> > >
> > > Functionnalities like KFENCE expect it.
> > >
> > > Do the same on powerpc, it allows KFENCE to properly identify the faulting
> > > function as depicted below. Before the patch KFENCE was identifying
> > > finish_task_switch.isra as the faulting function.
> > >
> > > [ 14.937370]
> > > ==
> > > [ 14.948692] BUG: KFENCE: invalid read in test_invalid_access+0x54/0x108
> > > [ 14.948692]
> > > [ 14.956814] Invalid read at 0xdf98800a:
> > > [ 14.960664] test_invalid_access+0x54/0x108
> > > [ 14.964876] finish_task_switch.isra.0+0x54/0x23c
> > > [ 14.969606] kunit_try_run_case+0x5c/0xd0
> > > [ 14.973658] kunit_generic_run_threadfn_adapter+0x24/0x30
> > > [ 14.979079] kthread+0x15c/0x174
> > > [ 14.982342] ret_from_kernel_thread+0x14/0x1c
> > > [ 14.986731]
> > > [ 14.988236] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB
> > >5.12.0-rc1-01537-g95f6e2088d7e-dirty #4682
> > > [ 14.999795] NIP: c016ec2c LR: c02f517c CTR: c016ebd8
> > > [ 15.004851] REGS: e2449d90 TRAP: 0301 Tainted: GB
> > > (5.12.0-rc1-01537-g95f6e2088d7e-dirty)
> > > [ 15.015274] MSR: 9032 CR: 2204 XER:
> > >
> > > [ 15.022043] DAR: df98800a DSISR: 2000
> > > [ 15.022043] GPR00: c02f517c e2449e50 c1142080 e100dd24 c084b13c
> > > 0008 c084b32b c016ebd8
> > > [ 15.022043] GPR08: c085 df988000 c0d1 e2449eb0 22000288
> > > [ 15.040581] NIP [c016ec2c] test_invalid_access+0x54/0x108
> > > [ 15.046010] LR [c02f517c] kunit_try_run_case+0x5c/0xd0
> > > [ 15.051181] Call Trace:
> > > [ 15.053637] [e2449e50] [c005a68c] finish_task_switch.isra.0+0x54/0x23c
> > > (unreliable)
> > > [ 15.061338] [e2449eb0] [c02f517c] kunit_try_run_case+0x5c/0xd0
> > > [ 15.067215] [e2449ed0] [c02f648c]
> > > kunit_generic_run_threadfn_adapter+0x24/0x30
> > > [ 15.074472] [e2449ef0] [c004e7b0] kthread+0x15c/0x174
> > > [ 15.079571] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c
> > > [ 15.085798] Instruction dump:
> > > [ 15.088784] 8129d608 38e7ebd8 81020280 911f004c 3900 995f0024
> > > 907f0028 90ff001c
> > > [ 15.096613] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0
> > > 812a4b98 3d40c02f
> > > [ 15.104612]
> > > ==
> > >
> > > Signed-off-by: Christophe Leroy
> >
> > Acked-by: Marco Elver
> >
> > Thank you, I think this looks like the right solution. Just a question
> > below:
> >
> ...
>
> > > @@ -59,23 +70,26 @@ void save_stack_trace(struct stack_trace *trace)
> > >
> > > sp = current_stack_frame();
> > >
> > > - save_context_stack(trace, sp, current, 1);
> > > + save_context_stack(trace, sp, (unsigned long)save_stack_trace,
> > > current, 1);
> >
> > This causes ip == save_stack_trace and also below for
> > save_stack_trace_tsk. Does this mean save_stack_trace() is included in
> > the trace? Looking at kernel/stacktrace.c, I think the library wants
> > to exclude itself from the trace, as it does '.skip = skipnr + 1' (and
> > '.skip = skipnr + (current == tsk)' for the _tsk variant).
> >
> > If the arch-helper here is included, should this use _RET_IP_ instead?
> >
>
> Don't really know, I was inspired by arm64 which has:
>
> void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
>struct task_struct *task, struct pt_regs *regs)
> {
> struct stackframe frame;
>
> if (regs)
> start_backtrace(, regs->regs[29], regs->pc);
> else if (task == current)
> start_backtrace(,
> (unsigned long)__builtin_frame_address(0),
> (unsigned long)arch_stack_walk);
> else
> start_backtrace(, thread_saved_fp(task),
> thread_saved_pc(task));
>
> walk_stackframe(task, , consume_entry, cookie);
> }
>
> But looking at x86 you may be right, so what should be done really ?
x86:
[2.843292] calling stack_trace_save:
[2.843705] test_func+0x6c/0x118
[2.844184] do_one_initcall+0x58/0x270
[2.844618] kernel_init_freeable+0x1da/0x23a
[2.845110] kernel_init+0xc/0x166
[2.845494] ret_from_fork+0x22/0x30
[2.867525] calling stack_trace_save_tsk:
[2.868017] test_func+0xa9/0x118
[2.868530] do_one_initcall+0x58/0x270
[2.869003] kernel_init_freeable+0x1da/0x23a
[2.869535] kernel_init+0xc/0x166
[2.869957] ret_from_fork+0x22/0x30
arm64:
[3.786911] calling
Re: [PATCH v2 0/7] Improve boot command line handling
On Wed, Mar 03, 2021 at 07:07:45PM +0100, Christophe Leroy wrote: > > > Le 03/03/2021 à 18:39, Daniel Walker a écrit : > > On Tue, Mar 02, 2021 at 08:01:01PM -0600, Rob Herring wrote: > > > +Will D > > > > > > On Tue, Mar 2, 2021 at 11:36 AM Daniel Walker wrote: > > > > > > > > On Tue, Mar 02, 2021 at 05:25:16PM +, Christophe Leroy wrote: > > > > > The purpose of this series is to improve and enhance the > > > > > handling of kernel boot arguments. > > > > > > > > > > It is first focussed on powerpc but also extends the capability > > > > > for other arches. > > > > > > > > > > This is based on suggestion from Daniel Walker > > > > > > > > > > > > > > > > > I don't see a point in your changes at this time. My changes are much > > > > more > > > > mature, and you changes don't really make improvements. > > > > > > Not really a helpful comment. What we merge here will be from whomever > > > is persistent and timely in their efforts. But please, work together > > > on a common solution. > > > > > > This one meets my requirements of moving the kconfig and code out of > > > the arches, supports prepend/append, and is up to date. > > > > > > Maintainers are capable of merging whatever they want to merge. However, I > > wouldn't make hasty choices. The changes I've been submitting have been > > deployed > > on millions of router instances and are more feature rich. > > > > I believe I worked with you on this change, or something like it, > > > > https://lkml.org/lkml/2019/3/19/970 > > > > I don't think Christophe has even addressed this. > > I thing I have, see > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/3b4291271ce4af4941a771e5af5cbba3c8fa1b2a.1614705851.git.christophe.le...@csgroup.eu/ > > If you see something missing in that patch, can you tell me. Ok, must have missed that one. > > I've converted many > > architectures, and Cisco uses my changes on at least 4 different > > architecture. With products deployed and tested. > > As far as we know, only powerpc was converted in the last series you > submitted, see > https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=98106=* Me and others submitted my changes many times, and other architectures have been included. The patch you submitted I've submitted similar at Rob's request years ago. Here a fuller submissions some time ago, https://lore.kernel.org/patchwork/cover/992768/ You've only been involved in prior powerpc only submissions. Daniel
Re: [PATCH v1] powerpc: Include running function as first entry in save_stack_trace() and friends
On Wed, 3 Mar 2021 at 15:09, Christophe Leroy
wrote:
>
> It seems like all other sane architectures, namely x86 and arm64
> at least, include the running function as top entry when saving
> stack trace.
>
> Functionnalities like KFENCE expect it.
>
> Do the same on powerpc, it allows KFENCE to properly identify the faulting
> function as depicted below. Before the patch KFENCE was identifying
> finish_task_switch.isra as the faulting function.
>
> [ 14.937370]
> ==
> [ 14.948692] BUG: KFENCE: invalid read in test_invalid_access+0x54/0x108
> [ 14.948692]
> [ 14.956814] Invalid read at 0xdf98800a:
> [ 14.960664] test_invalid_access+0x54/0x108
> [ 14.964876] finish_task_switch.isra.0+0x54/0x23c
> [ 14.969606] kunit_try_run_case+0x5c/0xd0
> [ 14.973658] kunit_generic_run_threadfn_adapter+0x24/0x30
> [ 14.979079] kthread+0x15c/0x174
> [ 14.982342] ret_from_kernel_thread+0x14/0x1c
> [ 14.986731]
> [ 14.988236] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB
>5.12.0-rc1-01537-g95f6e2088d7e-dirty #4682
> [ 14.999795] NIP: c016ec2c LR: c02f517c CTR: c016ebd8
> [ 15.004851] REGS: e2449d90 TRAP: 0301 Tainted: GB
> (5.12.0-rc1-01537-g95f6e2088d7e-dirty)
> [ 15.015274] MSR: 9032 CR: 2204 XER:
> [ 15.022043] DAR: df98800a DSISR: 2000
> [ 15.022043] GPR00: c02f517c e2449e50 c1142080 e100dd24 c084b13c 0008
> c084b32b c016ebd8
> [ 15.022043] GPR08: c085 df988000 c0d1 e2449eb0 22000288
> [ 15.040581] NIP [c016ec2c] test_invalid_access+0x54/0x108
> [ 15.046010] LR [c02f517c] kunit_try_run_case+0x5c/0xd0
> [ 15.051181] Call Trace:
> [ 15.053637] [e2449e50] [c005a68c] finish_task_switch.isra.0+0x54/0x23c
> (unreliable)
> [ 15.061338] [e2449eb0] [c02f517c] kunit_try_run_case+0x5c/0xd0
> [ 15.067215] [e2449ed0] [c02f648c]
> kunit_generic_run_threadfn_adapter+0x24/0x30
> [ 15.074472] [e2449ef0] [c004e7b0] kthread+0x15c/0x174
> [ 15.079571] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c
> [ 15.085798] Instruction dump:
> [ 15.088784] 8129d608 38e7ebd8 81020280 911f004c 3900 995f0024 907f0028
> 90ff001c
> [ 15.096613] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0
> 812a4b98 3d40c02f
> [ 15.104612]
> ==
>
> Signed-off-by: Christophe Leroy
Acked-by: Marco Elver
Thank you, I think this looks like the right solution. Just a question below:
> ---
> arch/powerpc/kernel/stacktrace.c | 42 +---
> 1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/kernel/stacktrace.c
> b/arch/powerpc/kernel/stacktrace.c
> index b6440657ef92..67c2b8488035 100644
> --- a/arch/powerpc/kernel/stacktrace.c
> +++ b/arch/powerpc/kernel/stacktrace.c
> @@ -22,16 +22,32 @@
> #include
>
> #include
> +#include
>
> /*
> * Save stack-backtrace addresses into a stack_trace buffer.
> */
> +static void save_entry(struct stack_trace *trace, unsigned long ip, int
> savesched)
> +{
> + if (savesched || !in_sched_functions(ip)) {
> + if (!trace->skip)
> + trace->entries[trace->nr_entries++] = ip;
> + else
> + trace->skip--;
> + }
> +}
> +
> static void save_context_stack(struct stack_trace *trace, unsigned long sp,
> - struct task_struct *tsk, int savesched)
> + unsigned long ip, struct task_struct *tsk, int
> savesched)
> {
> + save_entry(trace, ip, savesched);
> +
> + if (trace->nr_entries >= trace->max_entries)
> + return;
> +
> for (;;) {
> unsigned long *stack = (unsigned long *) sp;
> - unsigned long newsp, ip;
> + unsigned long newsp;
>
> if (!validate_sp(sp, tsk, STACK_FRAME_OVERHEAD))
> return;
> @@ -39,12 +55,7 @@ static void save_context_stack(struct stack_trace *trace,
> unsigned long sp,
> newsp = stack[0];
> ip = stack[STACK_FRAME_LR_SAVE];
>
> - if (savesched || !in_sched_functions(ip)) {
> - if (!trace->skip)
> - trace->entries[trace->nr_entries++] = ip;
> - else
> - trace->skip--;
> - }
> + save_entry(trace, ip, savesched);
>
> if (trace->nr_entries >= trace->max_entries)
> return;
> @@ -59,23 +70,26 @@ void save_stack_trace(struct stack_trace *trace)
>
> sp = current_stack_frame();
>
> - save_context_stack(trace, sp, current, 1);
> + save_context_stack(trace, sp, (unsigned long)save_stack_trace,
> current, 1);
This causes ip == save_stack_trace and also below for
save_stack_trace_tsk. Does this mean
Re: [PATCH v2 4/7] cmdline: Add capability to prepend the command line
On Tue, Mar 02, 2021 at 05:25:20PM +, Christophe Leroy wrote: > This patchs adds an option of prepend a text to the command > line instead of appending it. > > Signed-off-by: Christophe Leroy > --- > include/linux/cmdline.h | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h > index ae3610bb0ee2..144346051e01 100644 > --- a/include/linux/cmdline.h > +++ b/include/linux/cmdline.h > @@ -31,7 +31,7 @@ static __always_inline size_t cmdline_strlcat(char *dest, > const char *src, size_ > } > > /* > - * This function will append a builtin command line to the command > + * This function will append or prepend a builtin command line to the command > * line provided by the bootloader. Kconfig options can be used to alter > * the behavior of this builtin command line. > * @dest: The destination of the final appended/prepended string. > @@ -50,6 +50,9 @@ static __always_inline void cmdline_build(char *dest, const > char *src, size_t le > cmdline_strlcat(dest, CONFIG_CMDLINE, length); > return; > } > + > + if (IS_ENABLED(CONFIG_CMDLINE_PREPEND) && sizeof(CONFIG_CMDLINE) > 1) > + cmdline_strlcat(dest, CONFIG_CMDLINE " ", length); Same comment as the other patch: I don't think we need to worry about the sizeof() here. Will
Re: [PATCH v2 1/7] cmdline: Add generic function to build command line.
On Wed, Mar 03, 2021 at 06:57:09PM +0100, Christophe Leroy wrote:
> Le 03/03/2021 à 18:46, Will Deacon a écrit :
> > On Wed, Mar 03, 2021 at 06:38:16PM +0100, Christophe Leroy wrote:
> > > Le 03/03/2021 à 18:28, Will Deacon a écrit :
> > > > On Tue, Mar 02, 2021 at 05:25:17PM +, Christophe Leroy wrote:
> > > > > This code provides architectures with a way to build command line
> > > > > based on what is built in the kernel and what is handed over by the
> > > > > bootloader, based on selected compile-time options.
> > > > >
> > > > > Signed-off-by: Christophe Leroy
> > > > > ---
> > > > >include/linux/cmdline.h | 62
> > > > > +
> > > > >1 file changed, 62 insertions(+)
> > > > >create mode 100644 include/linux/cmdline.h
> > > > >
> > > > > diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
> > > > > new file mode 100644
> > > > > index ..ae3610bb0ee2
> > > > > --- /dev/null
> > > > > +++ b/include/linux/cmdline.h
> > > > > @@ -0,0 +1,62 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > > +#ifndef _LINUX_CMDLINE_H
> > > > > +#define _LINUX_CMDLINE_H
> > > > > +
> > > > > +static __always_inline size_t cmdline_strlen(const char *s)
> > > > > +{
> > > > > + const char *sc;
> > > > > +
> > > > > + for (sc = s; *sc != '\0'; ++sc)
> > > > > + ; /* nothing */
> > > > > + return sc - s;
> > > > > +}
> > > > > +
> > > > > +static __always_inline size_t cmdline_strlcat(char *dest, const char
> > > > > *src, size_t count)
> > > > > +{
> > > > > + size_t dsize = cmdline_strlen(dest);
> > > > > + size_t len = cmdline_strlen(src);
> > > > > + size_t res = dsize + len;
> > > > > +
> > > > > + /* This would be a bug */
> > > > > + if (dsize >= count)
> > > > > + return count;
> > > > > +
> > > > > + dest += dsize;
> > > > > + count -= dsize;
> > > > > + if (len >= count)
> > > > > + len = count - 1;
> > > > > + memcpy(dest, src, len);
> > > > > + dest[len] = 0;
> > > > > + return res;
> > > > > +}
> > > >
> > > > Why are these needed instead of using strlen and strlcat directly?
> > >
> > > Because on powerpc (at least), it will be used in prom_init, it is very
> > > early in the boot and KASAN shadow memory is not set up yet so calling
> > > generic string functions would crash the board.
> >
> > Hmm. We deliberately setup a _really_ early shadow on arm64 for this, can
> > you not do something similar? Failing that, I think it would be better to
> > offer the option for an arch to implement cmdline_*, but have then point to
> > the normal library routines by default.
>
> I don't think it is possible to setup an earlier early shadow.
>
> At the point we are in prom_init, the code is not yet relocated at the
> address it was linked for, and it is running with the MMU set by the
> bootloader, I can't imagine being able to setup MMU entries for the early
> shadow KASAN yet without breaking everything.
That's very similar to us; we're not relocated, although we are at least
in control of the MMU (which is using a temporary set of page-tables).
> Is it really worth trying to point to the normal library routines by default
> ? It is really only a few lines of code hence only not many bytes, and
> anyway they are in __init section so they get discarded at the end.
I would prefer to use the normal routines by default and allow architectures
to override them based on their needs, otherwise we'll end up trying to
maintain a "lowest-common-dominator" set of string routines that can be
safely run in whatever different constraints different architectures have.
Will
Re: [PATCH v2 0/7] Improve boot command line handling
Le 03/03/2021 à 18:39, Daniel Walker a écrit : On Tue, Mar 02, 2021 at 08:01:01PM -0600, Rob Herring wrote: +Will D On Tue, Mar 2, 2021 at 11:36 AM Daniel Walker wrote: On Tue, Mar 02, 2021 at 05:25:16PM +, Christophe Leroy wrote: The purpose of this series is to improve and enhance the handling of kernel boot arguments. It is first focussed on powerpc but also extends the capability for other arches. This is based on suggestion from Daniel Walker I don't see a point in your changes at this time. My changes are much more mature, and you changes don't really make improvements. Not really a helpful comment. What we merge here will be from whomever is persistent and timely in their efforts. But please, work together on a common solution. This one meets my requirements of moving the kconfig and code out of the arches, supports prepend/append, and is up to date. Maintainers are capable of merging whatever they want to merge. However, I wouldn't make hasty choices. The changes I've been submitting have been deployed on millions of router instances and are more feature rich. I believe I worked with you on this change, or something like it, https://lkml.org/lkml/2019/3/19/970 I don't think Christophe has even addressed this. I thing I have, see https://patchwork.ozlabs.org/project/linuxppc-dev/patch/3b4291271ce4af4941a771e5af5cbba3c8fa1b2a.1614705851.git.christophe.le...@csgroup.eu/ If you see something missing in that patch, can you tell me. I've converted many architectures, and Cisco uses my changes on at least 4 different architecture. With products deployed and tested. As far as we know, only powerpc was converted in the last series you submitted, see https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=98106=* I will resubmit my changes as soon as I can. Christophe
Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation
On Tue, Mar 02, 2021 at 05:25:22PM +, Christophe Leroy wrote: > Most architectures have similar boot command line manipulation > options. This patchs adds the definition in init/Kconfig, gated by > CONFIG_HAVE_CMDLINE that the architectures can select to use them. > > In order to use this, a few architectures will have to change their > CONFIG options: > - riscv has to replace CMDLINE_FALLBACK by CMDLINE_FROM_BOOTLOADER > - architectures using CONFIG_CMDLINE_OVERRIDE or > CONFIG_CMDLINE_OVERWRITE have to replace them by CONFIG_CMDLINE_FORCE. > > Architectures also have to define CONFIG_DEFAULT_CMDLINE. > > Signed-off-by: Christophe Leroy > --- > init/Kconfig | 56 > 1 file changed, 56 insertions(+) > > diff --git a/init/Kconfig b/init/Kconfig > index 22946fe5ded9..a0f2ad9467df 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -117,6 +117,62 @@ config INIT_ENV_ARG_LIMIT > Maximum of each of the number of arguments and environment > variables passed to init from the kernel command line. > > +config HAVE_CMDLINE > + bool > + > +config CMDLINE_BOOL > + bool "Default bootloader kernel arguments" > + depends on HAVE_CMDLINE > + help > + On some platforms, there is currently no way for the boot loader to > + pass arguments to the kernel. For these platforms, you can supply > + some command-line options at build time by entering them here. In > + most cases you will need to specify the root device here. Why is this needed as well as CMDLINE_FROM_BOOTLOADER? IIUC, the latter will use CONFIG_CMDLINE if it fails to get anything from the bootloader, which sounds like the same scenario. > +config CMDLINE > + string "Initial kernel command string" s/Initial/Default which is then consistent with the rest of the text here. > + depends on CMDLINE_BOOL Ah, so this is a bit different and I don't think lines-up with the CMDLINE_BOOL help text. > + default DEFAULT_CMDLINE > + help > + On some platforms, there is currently no way for the boot loader to > + pass arguments to the kernel. For these platforms, you can supply > + some command-line options at build time by entering them here. In > + most cases you will need to specify the root device here. (same stale text) > +choice > + prompt "Kernel command line type" if CMDLINE != "" > + default CMDLINE_FROM_BOOTLOADER > + help > + Selects the way you want to use the default kernel arguments. How about: "Determines how the default kernel arguments are combined with any arguments passed by the bootloader" > +config CMDLINE_FROM_BOOTLOADER > + bool "Use bootloader kernel arguments if available" > + help > + Uses the command-line options passed by the boot loader. If > + the boot loader doesn't provide any, the default kernel command > + string provided in CMDLINE will be used. > + > +config CMDLINE_EXTEND Can we rename this to CMDLINE_APPEND, please? There is code in the tree which disagrees about what CMDLINE_EXTEND means, so that will need be to be updated to be consistent (e.g. the EFI stub parsing order). Having the generic option with a different name means we won't accidentally end up with the same inconsistent behaviours. > + bool "Extend bootloader kernel arguments" "Append to the bootloader kernel arguments" > + help > + The default kernel command string will be appended to the > + command-line arguments provided during boot. s/provided during boot/provided by the bootloader/ > + > +config CMDLINE_PREPEND > + bool "Prepend bootloader kernel arguments" "Prepend to the bootloader kernel arguments" > + help > + The default kernel command string will be prepend to the > + command-line arguments provided during boot. s/prepend/prepended/ s/provided during boot/provided by the bootloader/ > + > +config CMDLINE_FORCE > + bool "Always use the default kernel command string" > + help > + Always use the default kernel command string, even if the boot > + loader passes other arguments to the kernel. > + This is useful if you cannot or don't want to change the > + command-line options your boot loader passes to the kernel. I find the "This is useful if ..." sentence really confusing, perhaps just remove it? I'd then tweak it to be: "Always use the default kernel command string, ignoring any arguments provided by the bootloader." Will
Re: [PATCH v2 1/7] cmdline: Add generic function to build command line.
Le 03/03/2021 à 18:46, Will Deacon a écrit :
On Wed, Mar 03, 2021 at 06:38:16PM +0100, Christophe Leroy wrote:
Le 03/03/2021 à 18:28, Will Deacon a écrit :
On Tue, Mar 02, 2021 at 05:25:17PM +, Christophe Leroy wrote:
This code provides architectures with a way to build command line
based on what is built in the kernel and what is handed over by the
bootloader, based on selected compile-time options.
Signed-off-by: Christophe Leroy
---
include/linux/cmdline.h | 62 +
1 file changed, 62 insertions(+)
create mode 100644 include/linux/cmdline.h
diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
new file mode 100644
index ..ae3610bb0ee2
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+
+static __always_inline size_t cmdline_strlen(const char *s)
+{
+ const char *sc;
+
+ for (sc = s; *sc != '\0'; ++sc)
+ ; /* nothing */
+ return sc - s;
+}
+
+static __always_inline size_t cmdline_strlcat(char *dest, const char *src,
size_t count)
+{
+ size_t dsize = cmdline_strlen(dest);
+ size_t len = cmdline_strlen(src);
+ size_t res = dsize + len;
+
+ /* This would be a bug */
+ if (dsize >= count)
+ return count;
+
+ dest += dsize;
+ count -= dsize;
+ if (len >= count)
+ len = count - 1;
+ memcpy(dest, src, len);
+ dest[len] = 0;
+ return res;
+}
Why are these needed instead of using strlen and strlcat directly?
Because on powerpc (at least), it will be used in prom_init, it is very
early in the boot and KASAN shadow memory is not set up yet so calling
generic string functions would crash the board.
Hmm. We deliberately setup a _really_ early shadow on arm64 for this, can
you not do something similar? Failing that, I think it would be better to
offer the option for an arch to implement cmdline_*, but have then point to
the normal library routines by default.
I don't think it is possible to setup an earlier early shadow.
At the point we are in prom_init, the code is not yet relocated at the address it was linked for,
and it is running with the MMU set by the bootloader, I can't imagine being able to setup MMU
entries for the early shadow KASAN yet without breaking everything.
Is it really worth trying to point to the normal library routines by default ? It is really only a
few lines of code hence only not many bytes, and anyway they are in __init section so they get
discarded at the end.
+/*
+ * This function will append a builtin command line to the command
+ * line provided by the bootloader. Kconfig options can be used to alter
+ * the behavior of this builtin command line.
+ * @dest: The destination of the final appended/prepended string.
+ * @src: The starting string or NULL if there isn't one. Must not equal dest.
+ * @length: the length of dest buffer.
+ */
+static __always_inline void cmdline_build(char *dest, const char *src, size_t
length)
+{
+ if (length <= 0)
+ return;
+
+ dest[0] = 0;
+
+#ifdef CONFIG_CMDLINE
+ if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src || !src[0]) {
+ cmdline_strlcat(dest, CONFIG_CMDLINE, length);
+ return;
+ }
+#endif
CONFIG_CMDLINE_FORCE implies CONFIG_CMDLINE, and even if it didn't,
CONFIG_CMDLINE is at worst an empty string. Can you drop the #ifdef?
Ah yes, since cbe46bd4f510 ("powerpc: remove CONFIG_CMDLINE #ifdef mess") it
is feasible. I can change that now.
+ if (dest != src)
+ cmdline_strlcat(dest, src, length);
+#ifdef CONFIG_CMDLINE
+ if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) && sizeof(CONFIG_CMDLINE) > 1)
+ cmdline_strlcat(dest, " " CONFIG_CMDLINE, length);
+#endif
Likewise, but also I'm not sure why the sizeof() is required.
It is to avoid adding a white space at the end of the command line when
CONFIG_CMDLINE is empty. But maybe it doesn't matter ?
If CONFIG_CMDLINE is empty, I don't think you can select
CONFIG_CMDLINE_EXTEND (but even if you could, I don't think it matters).
Ok I'll simplify that when I re-spin.
Christophe
[PATCH v2 1/8] powerpc/xive: Use cpu_to_node() instead of ibm, chip-id property
The 'chip_id' field of the XIVE CPU structure is used to choose a
target for a source located on the same chip when possible. This field
is assigned on the PowerNV platform using the "ibm,chip-id" property
on pSeries under KVM when NUMA nodes are defined but it is undefined
under PowerVM. The XIVE source structure has a similar field
'src_chip' which is only assigned on the PowerNV platform.
cpu_to_node() returns a compatible value on all platforms, 0 being the
default node. It will also give us the opportunity to set the affinity
of a source on pSeries when we can localize them.
Signed-off-by: Cédric Le Goater
---
arch/powerpc/sysdev/xive/common.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/common.c
b/arch/powerpc/sysdev/xive/common.c
index 595310e056f4..b8e456da28aa 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1335,16 +1335,11 @@ static int xive_prepare_cpu(unsigned int cpu)
xc = per_cpu(xive_cpu, cpu);
if (!xc) {
- struct device_node *np;
-
xc = kzalloc_node(sizeof(struct xive_cpu),
GFP_KERNEL, cpu_to_node(cpu));
if (!xc)
return -ENOMEM;
- np = of_get_cpu_node(cpu, NULL);
- if (np)
- xc->chip_id = of_get_ibm_chip_id(np);
- of_node_put(np);
+ xc->chip_id = cpu_to_node(cpu);
xc->hw_ipi = XIVE_BAD_IRQ;
per_cpu(xive_cpu, cpu) = xc;
--
2.26.2
[PATCH v2 3/8] powerpc/xive: Remove useless check on XIVE_IPI_HW_IRQ
The IPI interrupt has its own domain now. Testing the HW interrupt number is not needed anymore. Signed-off-by: Cédric Le Goater --- arch/powerpc/sysdev/xive/common.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c index e7783760d278..678680531d26 100644 --- a/arch/powerpc/sysdev/xive/common.c +++ b/arch/powerpc/sysdev/xive/common.c @@ -1396,13 +1396,12 @@ static void xive_flush_cpu_queue(unsigned int cpu, struct xive_cpu *xc) struct irq_desc *desc = irq_to_desc(irq); struct irq_data *d = irq_desc_get_irq_data(desc); struct xive_irq_data *xd; - unsigned int hw_irq = (unsigned int)irqd_to_hwirq(d); /* * Ignore anything that isn't a XIVE irq and ignore * IPIs, so can just be dropped. */ - if (d->domain != xive_irq_domain || hw_irq == XIVE_IPI_HW_IRQ) + if (d->domain != xive_irq_domain) continue; /* -- 2.26.2
[PATCH v2 5/8] powerpc/xive: Drop check on irq_data in xive_core_debug_show()
When looping on IRQ descriptor, irq_data is always valid.
Reported-by: kernel test robot
Reported-by: Dan Carpenter
Fixes: 930914b7d528 ("powerpc/xive: Add a debugfs file to dump internal XIVE
state")
Signed-off-by: Cédric Le Goater
---
arch/powerpc/sysdev/xive/common.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/common.c
b/arch/powerpc/sysdev/xive/common.c
index 7581cb12bb53..60ebd6f4b31d 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1586,6 +1586,8 @@ static void xive_debug_show_irq(struct seq_file *m,
struct irq_data *d)
u32 target;
u8 prio;
u32 lirq;
+ struct xive_irq_data *xd;
+ u64 val;
rc = xive_ops->get_irq_config(hw_irq, , , );
if (rc) {
@@ -1596,17 +1598,14 @@ static void xive_debug_show_irq(struct seq_file *m,
struct irq_data *d)
seq_printf(m, "IRQ 0x%08x : target=0x%x prio=%02x lirq=0x%x ",
hw_irq, target, prio, lirq);
- if (d) {
- struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
- u64 val = xive_esb_read(xd, XIVE_ESB_GET);
-
- seq_printf(m, "flags=%c%c%c PQ=%c%c",
- xd->flags & XIVE_IRQ_FLAG_STORE_EOI ? 'S' : ' ',
- xd->flags & XIVE_IRQ_FLAG_LSI ? 'L' : ' ',
- xd->flags & XIVE_IRQ_FLAG_H_INT_ESB ? 'H' : ' ',
- val & XIVE_ESB_VAL_P ? 'P' : '-',
- val & XIVE_ESB_VAL_Q ? 'Q' : '-');
- }
+ xd = irq_data_get_irq_handler_data(d);
+ val = xive_esb_read(xd, XIVE_ESB_GET);
+ seq_printf(m, "flags=%c%c%c PQ=%c%c",
+ xd->flags & XIVE_IRQ_FLAG_STORE_EOI ? 'S' : ' ',
+ xd->flags & XIVE_IRQ_FLAG_LSI ? 'L' : ' ',
+ xd->flags & XIVE_IRQ_FLAG_H_INT_ESB ? 'H' : ' ',
+ val & XIVE_ESB_VAL_P ? 'P' : '-',
+ val & XIVE_ESB_VAL_Q ? 'Q' : '-');
seq_puts(m, "\n");
}
--
2.26.2
[PATCH v2 2/8] powerpc/xive: Introduce an IPI interrupt domain
The IPI interrupt is a special case of the XIVE IRQ domain. When
mapping and unmapping the interrupts in the Linux interrupt number
space, the HW interrupt number 0 (XIVE_IPI_HW_IRQ) is checked to
distinguish the IPI interrupt from other interrupts of the system.
Simplify the XIVE interrupt domain by introducing a specific domain
for the IPI.
Signed-off-by: Cédric Le Goater
---
arch/powerpc/sysdev/xive/common.c | 51 +--
1 file changed, 22 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/common.c
b/arch/powerpc/sysdev/xive/common.c
index b8e456da28aa..e7783760d278 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -63,6 +63,8 @@ static const struct xive_ops *xive_ops;
static struct irq_domain *xive_irq_domain;
#ifdef CONFIG_SMP
+static struct irq_domain *xive_ipi_irq_domain;
+
/* The IPIs all use the same logical irq number */
static u32 xive_ipi_irq;
#endif
@@ -1067,20 +1069,32 @@ static struct irq_chip xive_ipi_chip = {
.irq_unmask = xive_ipi_do_nothing,
};
+/*
+ * IPIs are marked per-cpu. We use separate HW interrupts under the
+ * hood but associated with the same "linux" interrupt
+ */
+static int xive_ipi_irq_domain_map(struct irq_domain *h, unsigned int virq,
+ irq_hw_number_t hw)
+{
+ irq_set_chip_and_handler(virq, _ipi_chip, handle_percpu_irq);
+ return 0;
+}
+
+static const struct irq_domain_ops xive_ipi_irq_domain_ops = {
+ .map = xive_ipi_irq_domain_map,
+};
+
static void __init xive_request_ipi(void)
{
unsigned int virq;
- /*
-* Initialization failed, move on, we might manage to
-* reach the point where we display our errors before
-* the system falls appart
-*/
- if (!xive_irq_domain)
+ xive_ipi_irq_domain = irq_domain_add_linear(NULL, 1,
+ _ipi_irq_domain_ops,
NULL);
+ if (WARN_ON(xive_ipi_irq_domain == NULL))
return;
/* Initialize it */
- virq = irq_create_mapping(xive_irq_domain, XIVE_IPI_HW_IRQ);
+ virq = irq_create_mapping(xive_ipi_irq_domain, XIVE_IPI_HW_IRQ);
xive_ipi_irq = virq;
WARN_ON(request_irq(virq, xive_muxed_ipi_action,
@@ -1178,19 +1192,6 @@ static int xive_irq_domain_map(struct irq_domain *h,
unsigned int virq,
*/
irq_clear_status_flags(virq, IRQ_LEVEL);
-#ifdef CONFIG_SMP
- /* IPIs are special and come up with HW number 0 */
- if (hw == XIVE_IPI_HW_IRQ) {
- /*
-* IPIs are marked per-cpu. We use separate HW interrupts under
-* the hood but associated with the same "linux" interrupt
-*/
- irq_set_chip_and_handler(virq, _ipi_chip,
-handle_percpu_irq);
- return 0;
- }
-#endif
-
rc = xive_irq_alloc_data(virq, hw);
if (rc)
return rc;
@@ -1202,15 +1203,7 @@ static int xive_irq_domain_map(struct irq_domain *h,
unsigned int virq,
static void xive_irq_domain_unmap(struct irq_domain *d, unsigned int virq)
{
- struct irq_data *data = irq_get_irq_data(virq);
- unsigned int hw_irq;
-
- /* XXX Assign BAD number */
- if (!data)
- return;
- hw_irq = (unsigned int)irqd_to_hwirq(data);
- if (hw_irq != XIVE_IPI_HW_IRQ)
- xive_irq_free_data(virq);
+ xive_irq_free_data(virq);
}
static int xive_irq_domain_xlate(struct irq_domain *h, struct device_node *ct,
--
2.26.2
[PATCH v2 4/8] powerpc/xive: Simplify xive_core_debug_show()
Now that the IPI interrupt has its own domain, the checks on the HW
interrupt number XIVE_IPI_HW_IRQ and on the chip can be replaced by a
check on the domain.
Signed-off-by: Cédric Le Goater
---
arch/powerpc/sysdev/xive/common.c | 18 --
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/common.c
b/arch/powerpc/sysdev/xive/common.c
index 678680531d26..7581cb12bb53 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1579,17 +1579,14 @@ static void xive_debug_show_cpu(struct seq_file *m, int
cpu)
seq_puts(m, "\n");
}
-static void xive_debug_show_irq(struct seq_file *m, u32 hw_irq, struct
irq_data *d)
+static void xive_debug_show_irq(struct seq_file *m, struct irq_data *d)
{
- struct irq_chip *chip = irq_data_get_irq_chip(d);
+ unsigned int hw_irq = (unsigned int)irqd_to_hwirq(d);
int rc;
u32 target;
u8 prio;
u32 lirq;
- if (!is_xive_irq(chip))
- return;
-
rc = xive_ops->get_irq_config(hw_irq, , , );
if (rc) {
seq_printf(m, "IRQ 0x%08x : no config rc=%d\n", hw_irq, rc);
@@ -1627,16 +1624,9 @@ static int xive_core_debug_show(struct seq_file *m, void
*private)
for_each_irq_desc(i, desc) {
struct irq_data *d = irq_desc_get_irq_data(desc);
- unsigned int hw_irq;
-
- if (!d)
- continue;
-
- hw_irq = (unsigned int)irqd_to_hwirq(d);
- /* IPIs are special (HW number 0) */
- if (hw_irq != XIVE_IPI_HW_IRQ)
- xive_debug_show_irq(m, hw_irq, d);
+ if (d->domain == xive_irq_domain)
+ xive_debug_show_irq(m, d);
}
return 0;
}
--
2.26.2
[PATCH v2 7/8] powerpc/xive: Fix xmon command "dxi"
When under xmon, the "dxi" command dumps the state of the XIVE
interrupts. If an interrupt number is specified, only the state of
the associated XIVE interrupt is dumped. This form of the command
lacks an irq_data parameter which is nevertheless used by
xmon_xive_get_irq_config(), leading to an xmon crash.
Fix that by doing a lookup in the system IRQ mapping to query the IRQ
descriptor data. Invalid interrupt numbers, or not belonging to the
XIVE IRQ domain, OPAL event interrupt number for instance, should be
caught by the previous query done at the firmware level.
Reported-by: kernel test robot
Reported-by: Dan Carpenter
Fixes: 97ef27507793 ("powerpc/xive: Fix xmon support on the PowerNV platform")
Signed-off-by: Cédric Le Goater
---
arch/powerpc/sysdev/xive/common.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/common.c
b/arch/powerpc/sysdev/xive/common.c
index f6b7b15bbb3a..8eefd152b947 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -255,17 +255,20 @@ notrace void xmon_xive_do_dump(int cpu)
xmon_printf("\n");
}
+static struct irq_data *xive_get_irq_data(u32 hw_irq)
+{
+ unsigned int irq = irq_find_mapping(xive_irq_domain, hw_irq);
+
+ return irq ? irq_get_irq_data(irq) : NULL;
+}
+
int xmon_xive_get_irq_config(u32 hw_irq, struct irq_data *d)
{
- struct irq_chip *chip = irq_data_get_irq_chip(d);
int rc;
u32 target;
u8 prio;
u32 lirq;
- if (!is_xive_irq(chip))
- return -EINVAL;
-
rc = xive_ops->get_irq_config(hw_irq, , , );
if (rc) {
xmon_printf("IRQ 0x%08x : no config rc=%d\n", hw_irq, rc);
@@ -275,6 +278,9 @@ int xmon_xive_get_irq_config(u32 hw_irq, struct irq_data *d)
xmon_printf("IRQ 0x%08x : target=0x%x prio=%02x lirq=0x%x ",
hw_irq, target, prio, lirq);
+ if (!d)
+ d = xive_get_irq_data(hw_irq);
+
if (d) {
struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
u64 val = xive_esb_read(xd, XIVE_ESB_GET);
--
2.26.2
[PATCH v2 0/8] powerpc/xive: Map one IPI interrupt per node
Hello,
ipistorm [*] can be used to benchmark the raw interrupt rate of an
interrupt controller by measuring the number of IPIs a system can
sustain. When applied to the XIVE interrupt controller of POWER9 and
POWER10 systems, a significant drop of the interrupt rate can be
observed when crossing the second node boundary.
This is due to the fact that a single IPI interrupt is used for all
CPUs of the system. The structure is shared and the cache line updates
impact greatly the traffic between nodes and the overall IPI
performance.
As a workaround, the impact can be reduced by deactivating the IRQ
lockup detector ("noirqdebug") which does a lot of accounting in the
Linux IRQ descriptor structure and is responsible for most of the
performance penalty.
As a fix, this proposal allocates an IPI interrupt per node, to be
shared by all CPUs of that node. It solves the scaling issue, the IRQ
lockup detector still has an impact but the XIVE interrupt rate scales
linearly. It also improves the "noirqdebug" case as showed in the
tables below.
* P9 DD2.2 - 2s * 64 threads
"noirqdebug"
Mint/sMint/s
chips cpus IPI/sys IPI/chip IPI/chipIPI/sys
--
1 0-15 4.984023 4.875405 4.996536 5.048892
0-3110.879164 10.544040 10.757632 11.037859
0-4715.345301 14.688764 14.926520 15.310053
0-6317.064907 17.066812 17.613416 17.874511
2 0-7911.768764 21.650749 22.689120 22.566508
0-9510.616812 26.878789 28.434703 28.320324
0-111 10.151693 31.397803 31.771773 32.388122
0-1279.948502 33.139336 34.875716 35.224548
* P10 DD1 - 4s (not homogeneous) 352 threads
"noirqdebug"
Mint/sMint/s
chips cpus IPI/sys IPI/chip IPI/chipIPI/sys
--
1 0-15 2.409402 2.364108 2.383303 2.395091
0-31 6.028325 6.046075 6.08 6.073750
0-47 8.655178 8.644531 8.712830 8.724702
0-6311.629652 11.735953 12.088203 12.055979
0-7914.392321 14.729959 14.986701 14.973073
0-9512.604158 13.004034 17.528748 17.568095
2 0-1119.767753 13.719831 19.968606 20.024218
0-1276.744566 16.418854 22.898066 22.995110
0-1436.005699 19.174421 25.425622 25.417541
0-1595.649719 21.938836 27.952662 28.059603
0-1755.441410 24.109484 31.133915 31.127996
3 0-1915.318341 24.405322 33.999221 33.775354
0-2075.191382 26.449769 36.050161 35.867307
0-2235.102790 29.356943 39.544135 39.508169
0-2395.035295 31.933051 42.135075 42.071975
0-2554.969209 34.477367 44.655395 44.757074
4 0-2714.907652 35.887016 47.080545 47.318537
0-2874.839581 38.076137 50.464307 50.636219
0-3034.786031 40.881319 53.478684 53.310759
0-3194.743750 43.448424 56.388102 55.973969
0-3354.709936 45.623532 59.400930 58.926857
0-3514.681413 45.646151 62.035804 61.830057
[*] https://github.com/antonblanchard/ipistorm
Thanks,
C.
Changes in v2:
- extra simplification on xmon
- fixes on issues reported by the kernel test robot
Cédric Le Goater (8):
powerpc/xive: Use cpu_to_node() instead of ibm,chip-id property
powerpc/xive: Introduce an IPI interrupt domain
powerpc/xive: Remove useless check on XIVE_IPI_HW_IRQ
powerpc/xive: Simplify xive_core_debug_show()
powerpc/xive: Drop check on irq_data in xive_core_debug_show()
powerpc/xive: Simplify the dump of XIVE interrupts under xmon
powerpc/xive: Fix xmon command "dxi"
powerpc/xive: Map one IPI interrupt per node
arch/powerpc/include/asm/xive.h | 1 +
arch/powerpc/sysdev/xive/xive-internal.h | 2 -
arch/powerpc/sysdev/xive/common.c| 163 +--
arch/powerpc/xmon/xmon.c | 28 +---
4 files changed, 93 insertions(+), 101 deletions(-)
--
2.26.2
[PATCH v2 8/8] powerpc/xive: Map one IPI interrupt per node
ipistorm [*] can be used to benchmark the raw interrupt rate of an
interrupt controller by measuring the number of IPIs a system can
sustain. When applied to the XIVE interrupt controller of POWER9 and
POWER10 systems, a significant drop of the interrupt rate can be
observed when crossing the second node boundary.
This is due to the fact that a single IPI interrupt is used for all
CPUs of the system. The structure is shared and the cache line updates
impact greatly the traffic between nodes and the overall IPI
performance.
As a workaround, the impact can be reduced by deactivating the IRQ
lockup detector ("noirqdebug") which does a lot of accounting in the
Linux IRQ descriptor structure and is responsible for most of the
performance penalty.
As a fix, this proposal allocates an IPI interrupt per node, to be
shared by all CPUs of that node. It solves the scaling issue, the IRQ
lockup detector still has an impact but the XIVE interrupt rate scales
linearly. It also improves the "noirqdebug" case as showed in the
tables below.
* P9 DD2.2 - 2s * 64 threads
"noirqdebug"
Mint/sMint/s
chips cpus IPI/sys IPI/chip IPI/chipIPI/sys
--
1 0-15 4.984023 4.875405 4.996536 5.048892
0-3110.879164 10.544040 10.757632 11.037859
0-4715.345301 14.688764 14.926520 15.310053
0-6317.064907 17.066812 17.613416 17.874511
2 0-7911.768764 21.650749 22.689120 22.566508
0-9510.616812 26.878789 28.434703 28.320324
0-111 10.151693 31.397803 31.771773 32.388122
0-1279.948502 33.139336 34.875716 35.224548
* P10 DD1 - 4s (not homogeneous) 352 threads
"noirqdebug"
Mint/sMint/s
chips cpus IPI/sys IPI/chip IPI/chipIPI/sys
--
1 0-15 2.409402 2.364108 2.383303 2.395091
0-31 6.028325 6.046075 6.08 6.073750
0-47 8.655178 8.644531 8.712830 8.724702
0-6311.629652 11.735953 12.088203 12.055979
0-7914.392321 14.729959 14.986701 14.973073
0-9512.604158 13.004034 17.528748 17.568095
2 0-1119.767753 13.719831 19.968606 20.024218
0-1276.744566 16.418854 22.898066 22.995110
0-1436.005699 19.174421 25.425622 25.417541
0-1595.649719 21.938836 27.952662 28.059603
0-1755.441410 24.109484 31.133915 31.127996
3 0-1915.318341 24.405322 33.999221 33.775354
0-2075.191382 26.449769 36.050161 35.867307
0-2235.102790 29.356943 39.544135 39.508169
0-2395.035295 31.933051 42.135075 42.071975
0-2554.969209 34.477367 44.655395 44.757074
4 0-2714.907652 35.887016 47.080545 47.318537
0-2874.839581 38.076137 50.464307 50.636219
0-3034.786031 40.881319 53.478684 53.310759
0-3194.743750 43.448424 56.388102 55.973969
0-3354.709936 45.623532 59.400930 58.926857
0-3514.681413 45.646151 62.035804 61.830057
[*] https://github.com/antonblanchard/ipistorm
Signed-off-by: Cédric Le Goater
---
arch/powerpc/sysdev/xive/xive-internal.h | 2 --
arch/powerpc/sysdev/xive/common.c| 39 ++--
2 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/xive-internal.h
b/arch/powerpc/sysdev/xive/xive-internal.h
index 9cf57c722faa..b3a456fdd3a5 100644
--- a/arch/powerpc/sysdev/xive/xive-internal.h
+++ b/arch/powerpc/sysdev/xive/xive-internal.h
@@ -5,8 +5,6 @@
#ifndef __XIVE_INTERNAL_H
#define __XIVE_INTERNAL_H
-#define XIVE_IPI_HW_IRQ0 /* interrupt source # for IPIs */
-
/*
* A "disabled" interrupt should never fire, to catch problems
* we set its logical number to this
diff --git a/arch/powerpc/sysdev/xive/common.c
b/arch/powerpc/sysdev/xive/common.c
index 8eefd152b947..c27f7bb0494b 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -65,8 +65,16 @@ static struct irq_domain *xive_irq_domain;
#ifdef CONFIG_SMP
static struct irq_domain *xive_ipi_irq_domain;
-/* The IPIs all use the same logical irq number */
-static u32 xive_ipi_irq;
+/* The IPIs use the same logical irq number when on the same chip */
+static struct xive_ipi_desc {
+ unsigned int irq;
+ char name[8]; /* enough bytes to fit IPI-XXX */
+} *xive_ipis;
+
+static unsigned int xive_ipi_cpu_to_irq(unsigned int cpu)
+{
+ return xive_ipis[cpu_to_node(cpu)].irq;
+}
#endif
/* Xive
[PATCH v2 6/8] powerpc/xive: Simplify the dump of XIVE interrupts under xmon
Move the xmon routine under XIVE subsystem and rework the loop on the
interrupts taking into account the xive_irq_domain to filter out IPIs.
Signed-off-by: Cédric Le Goater
---
arch/powerpc/include/asm/xive.h | 1 +
arch/powerpc/sysdev/xive/common.c | 14 ++
arch/powerpc/xmon/xmon.c | 28 ++--
3 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
index 9a312b975ca8..aa094a8655b0 100644
--- a/arch/powerpc/include/asm/xive.h
+++ b/arch/powerpc/include/asm/xive.h
@@ -102,6 +102,7 @@ void xive_flush_interrupt(void);
/* xmon hook */
void xmon_xive_do_dump(int cpu);
int xmon_xive_get_irq_config(u32 hw_irq, struct irq_data *d);
+void xmon_xive_get_irq_all(void);
/* APIs used by KVM */
u32 xive_native_default_eq_shift(void);
diff --git a/arch/powerpc/sysdev/xive/common.c
b/arch/powerpc/sysdev/xive/common.c
index 60ebd6f4b31d..f6b7b15bbb3a 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -291,6 +291,20 @@ int xmon_xive_get_irq_config(u32 hw_irq, struct irq_data
*d)
return 0;
}
+void xmon_xive_get_irq_all(void)
+{
+ unsigned int i;
+ struct irq_desc *desc;
+
+ for_each_irq_desc(i, desc) {
+ struct irq_data *d = irq_desc_get_irq_data(desc);
+ unsigned int hwirq = (unsigned int)irqd_to_hwirq(d);
+
+ if (d->domain == xive_irq_domain)
+ xmon_xive_get_irq_config(hwirq, d);
+ }
+}
+
#endif /* CONFIG_XMON */
static unsigned int xive_get_irq(void)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 3fe37495f63d..80fbf8968f77 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2727,30 +2727,6 @@ static void dump_all_xives(void)
dump_one_xive(cpu);
}
-static void dump_one_xive_irq(u32 num, struct irq_data *d)
-{
- xmon_xive_get_irq_config(num, d);
-}
-
-static void dump_all_xive_irq(void)
-{
- unsigned int i;
- struct irq_desc *desc;
-
- for_each_irq_desc(i, desc) {
- struct irq_data *d = irq_desc_get_irq_data(desc);
- unsigned int hwirq;
-
- if (!d)
- continue;
-
- hwirq = (unsigned int)irqd_to_hwirq(d);
- /* IPIs are special (HW number 0) */
- if (hwirq)
- dump_one_xive_irq(hwirq, d);
- }
-}
-
static void dump_xives(void)
{
unsigned long num;
@@ -2767,9 +2743,9 @@ static void dump_xives(void)
return;
} else if (c == 'i') {
if (scanhex())
- dump_one_xive_irq(num, NULL);
+ xmon_xive_get_irq_config(num, NULL);
else
- dump_all_xive_irq();
+ xmon_xive_get_irq_all();
return;
}
--
2.26.2
Re: [PATCH next v4 12/15] printk: introduce a kmsg_dump iterator
On Wed 2021-03-03 11:15:25, John Ogness wrote: > Rather than storing the iterator information in the registered > kmsg_dumper structure, create a separate iterator structure. The > kmsg_dump_iter structure can reside on the stack of the caller, thus > allowing lockless use of the kmsg_dump functions. > > Update code that accesses the kernel logs using the kmsg_dumper > structure to use the new kmsg_dump_iter structure. For kmsg_dumpers, > this also means adding a call to kmsg_dump_rewind() to initialize > the iterator. > > All this is in preparation for removal of @logbuf_lock. > > Signed-off-by: John Ogness > Reviewed-by: Kees Cook # pstore Reviewed-by: Petr Mladek Best Regards, Petr
Re: [PATCH v2 1/7] cmdline: Add generic function to build command line.
On Wed, Mar 03, 2021 at 06:38:16PM +0100, Christophe Leroy wrote:
> Le 03/03/2021 à 18:28, Will Deacon a écrit :
> > On Tue, Mar 02, 2021 at 05:25:17PM +, Christophe Leroy wrote:
> > > This code provides architectures with a way to build command line
> > > based on what is built in the kernel and what is handed over by the
> > > bootloader, based on selected compile-time options.
> > >
> > > Signed-off-by: Christophe Leroy
> > > ---
> > > include/linux/cmdline.h | 62 +
> > > 1 file changed, 62 insertions(+)
> > > create mode 100644 include/linux/cmdline.h
> > >
> > > diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
> > > new file mode 100644
> > > index ..ae3610bb0ee2
> > > --- /dev/null
> > > +++ b/include/linux/cmdline.h
> > > @@ -0,0 +1,62 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +#ifndef _LINUX_CMDLINE_H
> > > +#define _LINUX_CMDLINE_H
> > > +
> > > +static __always_inline size_t cmdline_strlen(const char *s)
> > > +{
> > > + const char *sc;
> > > +
> > > + for (sc = s; *sc != '\0'; ++sc)
> > > + ; /* nothing */
> > > + return sc - s;
> > > +}
> > > +
> > > +static __always_inline size_t cmdline_strlcat(char *dest, const char
> > > *src, size_t count)
> > > +{
> > > + size_t dsize = cmdline_strlen(dest);
> > > + size_t len = cmdline_strlen(src);
> > > + size_t res = dsize + len;
> > > +
> > > + /* This would be a bug */
> > > + if (dsize >= count)
> > > + return count;
> > > +
> > > + dest += dsize;
> > > + count -= dsize;
> > > + if (len >= count)
> > > + len = count - 1;
> > > + memcpy(dest, src, len);
> > > + dest[len] = 0;
> > > + return res;
> > > +}
> >
> > Why are these needed instead of using strlen and strlcat directly?
>
> Because on powerpc (at least), it will be used in prom_init, it is very
> early in the boot and KASAN shadow memory is not set up yet so calling
> generic string functions would crash the board.
Hmm. We deliberately setup a _really_ early shadow on arm64 for this, can
you not do something similar? Failing that, I think it would be better to
offer the option for an arch to implement cmdline_*, but have then point to
the normal library routines by default.
> > > +/*
> > > + * This function will append a builtin command line to the command
> > > + * line provided by the bootloader. Kconfig options can be used to alter
> > > + * the behavior of this builtin command line.
> > > + * @dest: The destination of the final appended/prepended string.
> > > + * @src: The starting string or NULL if there isn't one. Must not equal
> > > dest.
> > > + * @length: the length of dest buffer.
> > > + */
> > > +static __always_inline void cmdline_build(char *dest, const char *src,
> > > size_t length)
> > > +{
> > > + if (length <= 0)
> > > + return;
> > > +
> > > + dest[0] = 0;
> > > +
> > > +#ifdef CONFIG_CMDLINE
> > > + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src || !src[0]) {
> > > + cmdline_strlcat(dest, CONFIG_CMDLINE, length);
> > > + return;
> > > + }
> > > +#endif
> >
> > CONFIG_CMDLINE_FORCE implies CONFIG_CMDLINE, and even if it didn't,
> > CONFIG_CMDLINE is at worst an empty string. Can you drop the #ifdef?
>
> Ah yes, since cbe46bd4f510 ("powerpc: remove CONFIG_CMDLINE #ifdef mess") it
> is feasible. I can change that now.
>
> >
> > > + if (dest != src)
> > > + cmdline_strlcat(dest, src, length);
> > > +#ifdef CONFIG_CMDLINE
> > > + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) && sizeof(CONFIG_CMDLINE) > 1)
> > > + cmdline_strlcat(dest, " " CONFIG_CMDLINE, length);
> > > +#endif
> >
> > Likewise, but also I'm not sure why the sizeof() is required.
>
> It is to avoid adding a white space at the end of the command line when
> CONFIG_CMDLINE is empty. But maybe it doesn't matter ?
If CONFIG_CMDLINE is empty, I don't think you can select
CONFIG_CMDLINE_EXTEND (but even if you could, I don't think it matters).
Will
Re: [PATCH v2 1/7] cmdline: Add generic function to build command line.
Le 03/03/2021 à 18:28, Will Deacon a écrit :
On Tue, Mar 02, 2021 at 05:25:17PM +, Christophe Leroy wrote:
This code provides architectures with a way to build command line
based on what is built in the kernel and what is handed over by the
bootloader, based on selected compile-time options.
Signed-off-by: Christophe Leroy
---
include/linux/cmdline.h | 62 +
1 file changed, 62 insertions(+)
create mode 100644 include/linux/cmdline.h
diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
new file mode 100644
index ..ae3610bb0ee2
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+
+static __always_inline size_t cmdline_strlen(const char *s)
+{
+ const char *sc;
+
+ for (sc = s; *sc != '\0'; ++sc)
+ ; /* nothing */
+ return sc - s;
+}
+
+static __always_inline size_t cmdline_strlcat(char *dest, const char *src,
size_t count)
+{
+ size_t dsize = cmdline_strlen(dest);
+ size_t len = cmdline_strlen(src);
+ size_t res = dsize + len;
+
+ /* This would be a bug */
+ if (dsize >= count)
+ return count;
+
+ dest += dsize;
+ count -= dsize;
+ if (len >= count)
+ len = count - 1;
+ memcpy(dest, src, len);
+ dest[len] = 0;
+ return res;
+}
Why are these needed instead of using strlen and strlcat directly?
Because on powerpc (at least), it will be used in prom_init, it is very early in the boot and KASAN
shadow memory is not set up yet so calling generic string functions would crash the board.
+/*
+ * This function will append a builtin command line to the command
+ * line provided by the bootloader. Kconfig options can be used to alter
+ * the behavior of this builtin command line.
+ * @dest: The destination of the final appended/prepended string.
+ * @src: The starting string or NULL if there isn't one. Must not equal dest.
+ * @length: the length of dest buffer.
+ */
+static __always_inline void cmdline_build(char *dest, const char *src, size_t
length)
+{
+ if (length <= 0)
+ return;
+
+ dest[0] = 0;
+
+#ifdef CONFIG_CMDLINE
+ if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src || !src[0]) {
+ cmdline_strlcat(dest, CONFIG_CMDLINE, length);
+ return;
+ }
+#endif
CONFIG_CMDLINE_FORCE implies CONFIG_CMDLINE, and even if it didn't,
CONFIG_CMDLINE is at worst an empty string. Can you drop the #ifdef?
Ah yes, since cbe46bd4f510 ("powerpc: remove CONFIG_CMDLINE #ifdef mess") it is feasible. I can
change that now.
+ if (dest != src)
+ cmdline_strlcat(dest, src, length);
+#ifdef CONFIG_CMDLINE
+ if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) && sizeof(CONFIG_CMDLINE) > 1)
+ cmdline_strlcat(dest, " " CONFIG_CMDLINE, length);
+#endif
Likewise, but also I'm not sure why the sizeof() is required.
It is to avoid adding a white space at the end of the command line when CONFIG_CMDLINE is empty. But
maybe it doesn't matter ?
Christophe
Re: [PATCH v2 1/7] cmdline: Add generic function to build command line.
On Tue, Mar 02, 2021 at 05:25:17PM +, Christophe Leroy wrote:
> This code provides architectures with a way to build command line
> based on what is built in the kernel and what is handed over by the
> bootloader, based on selected compile-time options.
>
> Signed-off-by: Christophe Leroy
> ---
> include/linux/cmdline.h | 62 +
> 1 file changed, 62 insertions(+)
> create mode 100644 include/linux/cmdline.h
Sorry, spotted a couple of other things...
> +/*
> + * This function will append a builtin command line to the command
> + * line provided by the bootloader. Kconfig options can be used to alter
> + * the behavior of this builtin command line.
> + * @dest: The destination of the final appended/prepended string.
> + * @src: The starting string or NULL if there isn't one. Must not equal dest.
> + * @length: the length of dest buffer.
> + */
> +static __always_inline void cmdline_build(char *dest, const char *src,
> size_t length)
> +{
> + if (length <= 0)
> + return;
length is unsigned
> +
> + dest[0] = 0;
> +
> +#ifdef CONFIG_CMDLINE
> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src || !src[0]) {
> + cmdline_strlcat(dest, CONFIG_CMDLINE, length);
> + return;
> + }
> +#endif
> + if (dest != src)
The kernel-doc says that @src "Must not equal dest".
Will
Re: [PATCH v2 0/7] Improve boot command line handling
On Tue, Mar 02, 2021 at 08:01:01PM -0600, Rob Herring wrote: > +Will D > > On Tue, Mar 2, 2021 at 11:36 AM Daniel Walker wrote: > > > > On Tue, Mar 02, 2021 at 05:25:16PM +, Christophe Leroy wrote: > > > The purpose of this series is to improve and enhance the > > > handling of kernel boot arguments. > > > > > > It is first focussed on powerpc but also extends the capability > > > for other arches. > > > > > > This is based on suggestion from Daniel Walker > > > > > > > > > I don't see a point in your changes at this time. My changes are much more > > mature, and you changes don't really make improvements. > > Not really a helpful comment. What we merge here will be from whomever > is persistent and timely in their efforts. But please, work together > on a common solution. > > This one meets my requirements of moving the kconfig and code out of > the arches, supports prepend/append, and is up to date. Maintainers are capable of merging whatever they want to merge. However, I wouldn't make hasty choices. The changes I've been submitting have been deployed on millions of router instances and are more feature rich. I believe I worked with you on this change, or something like it, https://lkml.org/lkml/2019/3/19/970 I don't think Christophe has even addressed this. I've converted many architectures, and Cisco uses my changes on at least 4 different architecture. With products deployed and tested. I will resubmit my changes as soon as I can. Daniel
Re: [PATCH v2 1/7] cmdline: Add generic function to build command line.
On Tue, Mar 02, 2021 at 05:25:17PM +, Christophe Leroy wrote:
> This code provides architectures with a way to build command line
> based on what is built in the kernel and what is handed over by the
> bootloader, based on selected compile-time options.
>
> Signed-off-by: Christophe Leroy
> ---
> include/linux/cmdline.h | 62 +
> 1 file changed, 62 insertions(+)
> create mode 100644 include/linux/cmdline.h
>
> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
> new file mode 100644
> index ..ae3610bb0ee2
> --- /dev/null
> +++ b/include/linux/cmdline.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_CMDLINE_H
> +#define _LINUX_CMDLINE_H
> +
> +static __always_inline size_t cmdline_strlen(const char *s)
> +{
> + const char *sc;
> +
> + for (sc = s; *sc != '\0'; ++sc)
> + ; /* nothing */
> + return sc - s;
> +}
> +
> +static __always_inline size_t cmdline_strlcat(char *dest, const char *src,
> size_t count)
> +{
> + size_t dsize = cmdline_strlen(dest);
> + size_t len = cmdline_strlen(src);
> + size_t res = dsize + len;
> +
> + /* This would be a bug */
> + if (dsize >= count)
> + return count;
> +
> + dest += dsize;
> + count -= dsize;
> + if (len >= count)
> + len = count - 1;
> + memcpy(dest, src, len);
> + dest[len] = 0;
> + return res;
> +}
Why are these needed instead of using strlen and strlcat directly?
> +/*
> + * This function will append a builtin command line to the command
> + * line provided by the bootloader. Kconfig options can be used to alter
> + * the behavior of this builtin command line.
> + * @dest: The destination of the final appended/prepended string.
> + * @src: The starting string or NULL if there isn't one. Must not equal dest.
> + * @length: the length of dest buffer.
> + */
> +static __always_inline void cmdline_build(char *dest, const char *src,
> size_t length)
> +{
> + if (length <= 0)
> + return;
> +
> + dest[0] = 0;
> +
> +#ifdef CONFIG_CMDLINE
> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src || !src[0]) {
> + cmdline_strlcat(dest, CONFIG_CMDLINE, length);
> + return;
> + }
> +#endif
CONFIG_CMDLINE_FORCE implies CONFIG_CMDLINE, and even if it didn't,
CONFIG_CMDLINE is at worst an empty string. Can you drop the #ifdef?
> + if (dest != src)
> + cmdline_strlcat(dest, src, length);
> +#ifdef CONFIG_CMDLINE
> + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) && sizeof(CONFIG_CMDLINE) > 1)
> + cmdline_strlcat(dest, " " CONFIG_CMDLINE, length);
> +#endif
Likewise, but also I'm not sure why the sizeof() is required.
Will
[PATCH v2] powerpc: Fix save_stack_trace_regs() to have running function as first entry
It seems like other architectures, namely x86 and arm64
at least, include the running function as top entry when saving
stack trace with save_stack_trace_regs().
Functionnalities like KFENCE expect it.
Do the same on powerpc, it allows KFENCE to properly identify the faulting
function as depicted below. Before the patch KFENCE was identifying
finish_task_switch.isra as the faulting function.
[ 14.937370]
==
[ 14.948692] BUG: KFENCE: invalid read in test_invalid_access+0x54/0x108
[ 14.948692]
[ 14.956814] Invalid read at 0xdf98800a:
[ 14.960664] test_invalid_access+0x54/0x108
[ 14.964876] finish_task_switch.isra.0+0x54/0x23c
[ 14.969606] kunit_try_run_case+0x5c/0xd0
[ 14.973658] kunit_generic_run_threadfn_adapter+0x24/0x30
[ 14.979079] kthread+0x15c/0x174
[ 14.982342] ret_from_kernel_thread+0x14/0x1c
[ 14.986731]
[ 14.988236] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB
5.12.0-rc1-01537-g95f6e2088d7e-dirty #4682
[ 14.999795] NIP: c016ec2c LR: c02f517c CTR: c016ebd8
[ 15.004851] REGS: e2449d90 TRAP: 0301 Tainted: GB
(5.12.0-rc1-01537-g95f6e2088d7e-dirty)
[ 15.015274] MSR: 9032 CR: 2204 XER:
[ 15.022043] DAR: df98800a DSISR: 2000
[ 15.022043] GPR00: c02f517c e2449e50 c1142080 e100dd24 c084b13c 0008
c084b32b c016ebd8
[ 15.022043] GPR08: c085 df988000 c0d1 e2449eb0 22000288
[ 15.040581] NIP [c016ec2c] test_invalid_access+0x54/0x108
[ 15.046010] LR [c02f517c] kunit_try_run_case+0x5c/0xd0
[ 15.051181] Call Trace:
[ 15.053637] [e2449e50] [c005a68c] finish_task_switch.isra.0+0x54/0x23c
(unreliable)
[ 15.061338] [e2449eb0] [c02f517c] kunit_try_run_case+0x5c/0xd0
[ 15.067215] [e2449ed0] [c02f648c]
kunit_generic_run_threadfn_adapter+0x24/0x30
[ 15.074472] [e2449ef0] [c004e7b0] kthread+0x15c/0x174
[ 15.079571] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c
[ 15.085798] Instruction dump:
[ 15.088784] 8129d608 38e7ebd8 81020280 911f004c 3900 995f0024 907f0028
90ff001c
[ 15.096613] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98
3d40c02f
[ 15.104612]
==
Signed-off-by: Christophe Leroy
Fixes: 35de3b1aa168 ("powerpc: Implement save_stack_trace_regs() to enable
kprobe stack tracing")
Cc: sta...@vger.kernel.org
Acked-by: Marco Elver
---
arch/powerpc/kernel/stacktrace.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index b6440657ef92..a99bd3697286 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -23,6 +23,18 @@
#include
+static bool save_entry(struct stack_trace *trace, unsigned long ip, int
savesched)
+{
+ if (savesched || !in_sched_functions(ip)) {
+ if (!trace->skip)
+ trace->entries[trace->nr_entries++] = ip;
+ else
+ trace->skip--;
+ }
+ /* Returns true when the trace is full */
+ return trace->nr_entries >= trace->max_entries;
+}
+
/*
* Save stack-backtrace addresses into a stack_trace buffer.
*/
@@ -39,14 +51,7 @@ static void save_context_stack(struct stack_trace *trace,
unsigned long sp,
newsp = stack[0];
ip = stack[STACK_FRAME_LR_SAVE];
- if (savesched || !in_sched_functions(ip)) {
- if (!trace->skip)
- trace->entries[trace->nr_entries++] = ip;
- else
- trace->skip--;
- }
-
- if (trace->nr_entries >= trace->max_entries)
+ if (save_entry(trace, ip, savesched))
return;
sp = newsp;
@@ -84,6 +89,9 @@ EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
void
save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)
{
+ if (save_entry(trace, regs->nip, 0))
+ return;
+
save_context_stack(trace, regs->gpr[1], current, 0);
}
EXPORT_SYMBOL_GPL(save_stack_trace_regs);
--
2.25.0
lkml delivery: was: Re: [PATCH next v4 00/15] printk: remove logbuf_lock
Hi John, On Wed 2021-03-03 11:15:13, John Ogness wrote: > Hello, > > Here is v4 of a series to remove @logbuf_lock, exposing the > ringbuffer locklessly to both readers and writers. v3 is > here [0]. Have you got some reply from lkml that it has not delivered there, please? I am not able to get the patchset using b4 tool: $> b4 am -o test 20210303101528.29901-1-john.ogn...@linutronix.de Looking up https://lore.kernel.org/r/20210303101528.29901-1-john.ogness%40linutronix.de Grabbing thread from lore.kernel.org/linux-hyperv Analyzing 2 messages in the thread --- Thread incomplete, attempting to backfill Grabbing thread from lore.kernel.org/lkml Server returned an error: 404 Grabbing thread from lore.kernel.org/linux-mtd Server returned an error: 404 Grabbing thread from lore.kernel.org/linuxppc-dev Loaded 2 messages from https://lore.kernel.org/linuxppc-dev/ --- Writing test/v4_20210303_john_ogness_printk_remove_logbuf_lock.mbx ERROR: missing [1/15]! ERROR: missing [2/15]! ERROR: missing [3/15]! ERROR: missing [4/15]! ERROR: missing [5/15]! ERROR: missing [6/15]! ERROR: missing [7/15]! ERROR: missing [8/15]! ERROR: missing [9/15]! ERROR: missing [10/15]! [PATCH next v4 11/15] printk: kmsg_dumper: remove @active field ✓ [PATCH next v4 12/15] printk: introduce a kmsg_dump iterator ERROR: missing [13/15]! [PATCH next v4 14/15] printk: kmsg_dump: remove _nolock() variants ERROR: missing [15/15]! --- Total patches: 3 --- WARNING: Thread incomplete! Cover: test/v4_20210303_john_ogness_printk_remove_logbuf_lock.cover Link: https://lore.kernel.org/r/20210303101528.29901-1-john.ogn...@linutronix.de Base: not found git am test/v4_20210303_john_ogness_printk_remove_logbuf_lock.mbx and I do not see it at lore. It has only found copies in linux-hyperv and linux-ppcdev mailing lists, see https://lore.kernel.org/lkml/20210303101528.29901-2-john.ogn...@linutronix.de/ Best Regards, Petr
Re: [PATCH] ibmvnic: Fix possibly uninitialized old_num_tx_queues variable warning.
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Tue, 2 Mar 2021 20:47:47 +0100 you wrote: > GCC 7.5 reports: > ../drivers/net/ethernet/ibm/ibmvnic.c: In function 'ibmvnic_reset_init': > ../drivers/net/ethernet/ibm/ibmvnic.c:5373:51: warning: 'old_num_tx_queues' > may be used uninitialized in this function [-Wmaybe-uninitialized] > ../drivers/net/ethernet/ibm/ibmvnic.c:5373:6: warning: 'old_num_rx_queues' > may be used uninitialized in this function [-Wmaybe-uninitialized] > > The variable is initialized only if(reset) and used only if(reset && > something) so this is a false positive. However, there is no reason to > not initialize the variables unconditionally avoiding the warning. > > [...] Here is the summary with links: - ibmvnic: Fix possibly uninitialized old_num_tx_queues variable warning. https://git.kernel.org/netdev/net/c/6881b07fdd24 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
On Wed, 3 Mar 2021 at 11:39, Christophe Leroy wrote: > > > > Le 02/03/2021 à 12:39, Marco Elver a écrit : > > On Tue, 2 Mar 2021 at 12:21, Christophe Leroy > > wrote: > > [...] > Booting with 'no_hash_pointers" I get the following. Does it helps ? > > [ 16.837198] > == > [ 16.848521] BUG: KFENCE: invalid read in > finish_task_switch.isra.0+0x54/0x23c > [ 16.848521] > [ 16.857158] Invalid read at 0xdf98800a: > [ 16.861004] finish_task_switch.isra.0+0x54/0x23c > [ 16.865731] kunit_try_run_case+0x5c/0xd0 > [ 16.869780] kunit_generic_run_threadfn_adapter+0x24/0x30 > [ 16.875199] kthread+0x15c/0x174 > [ 16.878460] ret_from_kernel_thread+0x14/0x1c > [ 16.882847] > [ 16.884351] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB > 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674 > [ 16.895908] NIP: c016eb8c LR: c02f50dc CTR: c016eb38 > [ 16.900963] REGS: e2449d90 TRAP: 0301 Tainted: GB > (5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty) > [ 16.911386] MSR: 9032 CR: 2204 XER: > > [ 16.918153] DAR: df98800a DSISR: 2000 > [ 16.918153] GPR00: c02f50dc e2449e50 c1140d00 e100dd24 c084b13c > 0008 c084b32b c016eb38 > [ 16.918153] GPR08: c085 df988000 c0d1 e2449eb0 22000288 > [ 16.936695] NIP [c016eb8c] test_invalid_access+0x54/0x108 > [ 16.942125] LR [c02f50dc] kunit_try_run_case+0x5c/0xd0 > [ 16.947292] Call Trace: > [ 16.949746] [e2449e50] [c005a5ec] > finish_task_switch.isra.0+0x54/0x23c (unreliable) > >>> > >>> The "(unreliable)" might be a clue that it's related to ppc32 stack > >>> unwinding. Any ppc expert know what this is about? > >>> > [ 16.957443] [e2449eb0] [c02f50dc] kunit_try_run_case+0x5c/0xd0 > [ 16.963319] [e2449ed0] [c02f63ec] > kunit_generic_run_threadfn_adapter+0x24/0x30 > [ 16.970574] [e2449ef0] [c004e710] kthread+0x15c/0x174 > [ 16.975670] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c > [ 16.981896] Instruction dump: > [ 16.984879] 8129d608 38e7eb38 81020280 911f004c 3900 995f0024 > 907f0028 90ff001c > [ 16.992710] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 > 812a4b98 3d40c02f > [ 17.000711] > == > [ 17.008223] # test_invalid_access: EXPECTATION FAILED at > mm/kfence/kfence_test.c:636 > [ 17.008223] Expected report_matches() to be true, but is > false > [ 17.023243] not ok 21 - test_invalid_access > >>> > >>> On a fault in test_invalid_access, KFENCE prints the stack trace based > >>> on the information in pt_regs. So we do not think there's anything we > >>> can do to improve stack printing pe-se. > >> > >> stack printing, probably not. Would be good anyway to mark the last level > >> [unreliable] as the ppc does. > > > > We use stack_trace_save_regs() + stack_trace_print(). > > > >> IIUC, on ppc the address in the stack frame of the caller is written by > >> the caller. In most tests, > >> there is some function call being done before the fault, for instance > >> test_kmalloc_aligned_oob_read() does a call to kunit_do_assertion which > >> populates the address of the > >> call in the stack. However this is fragile. > > > > Interesting, this might explain it. > > > >> This works for function calls because in order to call a subfunction, a > >> function has to set up a > >> stack frame in order to same the value in the Link Register, which > >> contains the address of the > >> function's parent and that will be clobbered by the sub-function call. > >> > >> However, it cannot be done by exceptions, because exceptions can happen in > >> a function that has no > >> stack frame (because that function has no need to call a subfunction and > >> doesn't need to same > >> anything on the stack). If the exception handler was writting the caller's > >> address in the stack > >> frame, it would in fact write it in the parent's frame, leading to a mess. > >> > >> But in fact the information is in pt_regs, it is in regs->nip so KFENCE > >> should be able to use that > >> instead of the stack. > > > > Perhaps stack_trace_save_regs() needs fixing for ppc32? Although that > > seems to use arch_stack_walk(). > > > >>> What's confusing is that it's only this test, and none of the others. > >>> Given that, it might be code-gen related, which results in some subtle > >>> issue with stack unwinding. There are a few things to try, if you feel > >>> like it: > >>> > >>> -- Change the unwinder, if it's possible for ppc32. > >> > >> I don't think it is possible. > >> > >>> > >>> -- Add code to test_invalid_access(), to get the compiler to emit > >>> different code. E.g. add a bunch (unnecessary)
Re: [PATCH v1] powerpc: Include running function as first entry in save_stack_trace() and friends
Le 03/03/2021 à 15:38, Marco Elver a écrit :
On Wed, 3 Mar 2021 at 15:09, Christophe Leroy
wrote:
It seems like all other sane architectures, namely x86 and arm64
at least, include the running function as top entry when saving
stack trace.
Functionnalities like KFENCE expect it.
Do the same on powerpc, it allows KFENCE to properly identify the faulting
function as depicted below. Before the patch KFENCE was identifying
finish_task_switch.isra as the faulting function.
[ 14.937370]
==
[ 14.948692] BUG: KFENCE: invalid read in test_invalid_access+0x54/0x108
[ 14.948692]
[ 14.956814] Invalid read at 0xdf98800a:
[ 14.960664] test_invalid_access+0x54/0x108
[ 14.964876] finish_task_switch.isra.0+0x54/0x23c
[ 14.969606] kunit_try_run_case+0x5c/0xd0
[ 14.973658] kunit_generic_run_threadfn_adapter+0x24/0x30
[ 14.979079] kthread+0x15c/0x174
[ 14.982342] ret_from_kernel_thread+0x14/0x1c
[ 14.986731]
[ 14.988236] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB
5.12.0-rc1-01537-g95f6e2088d7e-dirty #4682
[ 14.999795] NIP: c016ec2c LR: c02f517c CTR: c016ebd8
[ 15.004851] REGS: e2449d90 TRAP: 0301 Tainted: GB
(5.12.0-rc1-01537-g95f6e2088d7e-dirty)
[ 15.015274] MSR: 9032 CR: 2204 XER:
[ 15.022043] DAR: df98800a DSISR: 2000
[ 15.022043] GPR00: c02f517c e2449e50 c1142080 e100dd24 c084b13c 0008
c084b32b c016ebd8
[ 15.022043] GPR08: c085 df988000 c0d1 e2449eb0 22000288
[ 15.040581] NIP [c016ec2c] test_invalid_access+0x54/0x108
[ 15.046010] LR [c02f517c] kunit_try_run_case+0x5c/0xd0
[ 15.051181] Call Trace:
[ 15.053637] [e2449e50] [c005a68c] finish_task_switch.isra.0+0x54/0x23c
(unreliable)
[ 15.061338] [e2449eb0] [c02f517c] kunit_try_run_case+0x5c/0xd0
[ 15.067215] [e2449ed0] [c02f648c]
kunit_generic_run_threadfn_adapter+0x24/0x30
[ 15.074472] [e2449ef0] [c004e7b0] kthread+0x15c/0x174
[ 15.079571] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c
[ 15.085798] Instruction dump:
[ 15.088784] 8129d608 38e7ebd8 81020280 911f004c 3900 995f0024 907f0028
90ff001c
[ 15.096613] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98
3d40c02f
[ 15.104612]
==
Signed-off-by: Christophe Leroy
Acked-by: Marco Elver
Thank you, I think this looks like the right solution. Just a question below:
...
@@ -59,23 +70,26 @@ void save_stack_trace(struct stack_trace *trace)
sp = current_stack_frame();
- save_context_stack(trace, sp, current, 1);
+ save_context_stack(trace, sp, (unsigned long)save_stack_trace, current,
1);
This causes ip == save_stack_trace and also below for
save_stack_trace_tsk. Does this mean save_stack_trace() is included in
the trace? Looking at kernel/stacktrace.c, I think the library wants
to exclude itself from the trace, as it does '.skip = skipnr + 1' (and
'.skip = skipnr + (current == tsk)' for the _tsk variant).
If the arch-helper here is included, should this use _RET_IP_ instead?
Don't really know, I was inspired by arm64 which has:
void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
struct task_struct *task, struct pt_regs *regs)
{
struct stackframe frame;
if (regs)
start_backtrace(, regs->regs[29], regs->pc);
else if (task == current)
start_backtrace(,
(unsigned long)__builtin_frame_address(0),
(unsigned long)arch_stack_walk);
else
start_backtrace(, thread_saved_fp(task),
thread_saved_pc(task));
walk_stackframe(task, , consume_entry, cookie);
}
But looking at x86 you may be right, so what should be done really ?
Thanks
Christophe
Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
On Wed, 3 Mar 2021 at 11:32, Christophe Leroy wrote: > > > > Le 02/03/2021 à 10:53, Marco Elver a écrit : > > On Tue, 2 Mar 2021 at 10:27, Christophe Leroy > > wrote: > >> Le 02/03/2021 à 10:21, Alexander Potapenko a écrit : > [ 14.998426] BUG: KFENCE: invalid read in > finish_task_switch.isra.0+0x54/0x23c > [ 14.998426] > [ 15.007061] Invalid read at 0x(ptrval): > [ 15.010906] finish_task_switch.isra.0+0x54/0x23c > [ 15.015633] kunit_try_run_case+0x5c/0xd0 > [ 15.019682] kunit_generic_run_threadfn_adapter+0x24/0x30 > [ 15.025099] kthread+0x15c/0x174 > [ 15.028359] ret_from_kernel_thread+0x14/0x1c > [ 15.032747] > [ 15.034251] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB > 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674 > [ 15.045811] > == > [ 15.053324] # test_invalid_access: EXPECTATION FAILED at > mm/kfence/kfence_test.c:636 > [ 15.053324] Expected report_matches() to be true, but is > false > [ 15.068359] not ok 21 - test_invalid_access > >>> > >>> The test expects the function name to be test_invalid_access, i. e. > >>> the first line should be "BUG: KFENCE: invalid read in > >>> test_invalid_access". > >>> The error reporting function unwinds the stack, skips a couple of > >>> "uninteresting" frames > >>> (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L43) > >>> and uses the first "interesting" one frame to print the report header > >>> (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L226). > >>> > >>> It's strange that test_invalid_access is missing altogether from the > >>> stack trace - is that expected? > >>> Can you try printing the whole stacktrace without skipping any frames > >>> to see if that function is there? > >>> > >> > >> Booting with 'no_hash_pointers" I get the following. Does it helps ? > >> > >> [ 16.837198] > >> == > >> [ 16.848521] BUG: KFENCE: invalid read in > >> finish_task_switch.isra.0+0x54/0x23c > >> [ 16.848521] > >> [ 16.857158] Invalid read at 0xdf98800a: > >> [ 16.861004] finish_task_switch.isra.0+0x54/0x23c > >> [ 16.865731] kunit_try_run_case+0x5c/0xd0 > >> [ 16.869780] kunit_generic_run_threadfn_adapter+0x24/0x30 > >> [ 16.875199] kthread+0x15c/0x174 > >> [ 16.878460] ret_from_kernel_thread+0x14/0x1c > >> [ 16.882847] > >> [ 16.884351] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB > >> 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674 > >> [ 16.895908] NIP: c016eb8c LR: c02f50dc CTR: c016eb38 > >> [ 16.900963] REGS: e2449d90 TRAP: 0301 Tainted: GB > >> (5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty) > >> [ 16.911386] MSR: 9032 CR: 2204 XER: > >> [ 16.918153] DAR: df98800a DSISR: 2000 > >> [ 16.918153] GPR00: c02f50dc e2449e50 c1140d00 e100dd24 c084b13c > >> 0008 c084b32b c016eb38 > >> [ 16.918153] GPR08: c085 df988000 c0d1 e2449eb0 22000288 > >> [ 16.936695] NIP [c016eb8c] test_invalid_access+0x54/0x108 > >> [ 16.942125] LR [c02f50dc] kunit_try_run_case+0x5c/0xd0 > >> [ 16.947292] Call Trace: > >> [ 16.949746] [e2449e50] [c005a5ec] finish_task_switch.isra.0+0x54/0x23c > >> (unreliable) > > > > The "(unreliable)" might be a clue that it's related to ppc32 stack > > unwinding. Any ppc expert know what this is about? > > > >> [ 16.957443] [e2449eb0] [c02f50dc] kunit_try_run_case+0x5c/0xd0 > >> [ 16.963319] [e2449ed0] [c02f63ec] > >> kunit_generic_run_threadfn_adapter+0x24/0x30 > >> [ 16.970574] [e2449ef0] [c004e710] kthread+0x15c/0x174 > >> [ 16.975670] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c > >> [ 16.981896] Instruction dump: > >> [ 16.984879] 8129d608 38e7eb38 81020280 911f004c 3900 995f0024 > >> 907f0028 90ff001c > >> [ 16.992710] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 > >> 812a4b98 3d40c02f > >> [ 17.000711] > >> == > >> [ 17.008223] # test_invalid_access: EXPECTATION FAILED at > >> mm/kfence/kfence_test.c:636 > >> [ 17.008223] Expected report_matches() to be true, but is > >> false > >> [ 17.023243] not ok 21 - test_invalid_access > > > > On a fault in test_invalid_access, KFENCE prints the stack trace based > > on the information in pt_regs. So we do not think there's anything we > > can do to improve stack printing pe-se. > > > > What's confusing is that it's only this test, and none of the others. > > Given that, it might be code-gen related, which results in some subtle > > issue with stack unwinding. There are a few things to try, if you feel > > like it: > > > > -- Change the unwinder, if it's possible for ppc32. > > > > -- Add code to test_invalid_access(), to get the compiler to emit > >
[PATCH 09/10] crypto: nx: nx-aes-cbc: Repair some kernel-doc problems
Fixes the following W=1 kernel build warning(s): drivers/crypto/nx/nx-aes-cbc.c:24: warning: Function parameter or member 'tfm' not described in 'cbc_aes_nx_set_key' drivers/crypto/nx/nx-aes-cbc.c:24: warning: Function parameter or member 'in_key' not described in 'cbc_aes_nx_set_key' drivers/crypto/nx/nx-aes-cbc.c:24: warning: Function parameter or member 'key_len' not described in 'cbc_aes_nx_set_key' drivers/crypto/nx/nx-aes-cbc.c:24: warning: expecting prototype for Nest Accelerators driver(). Prototype was for cbc_aes_nx_set_key() instead drivers/crypto/nx/nx_debugfs.c:34: warning: Function parameter or member 'drv' not described in 'nx_debugfs_init' drivers/crypto/nx/nx_debugfs.c:34: warning: expecting prototype for Nest Accelerators driver(). Prototype was for nx_debugfs_init() instead drivers/crypto/nx/nx.c:31: warning: Incorrect use of kernel-doc format: * nx_hcall_sync - make an H_COP_OP hcall for the passed in op structure drivers/crypto/nx/nx.c:43: warning: Function parameter or member 'nx_ctx' not described in 'nx_hcall_sync' drivers/crypto/nx/nx.c:43: warning: Function parameter or member 'op' not described in 'nx_hcall_sync' drivers/crypto/nx/nx.c:43: warning: Function parameter or member 'may_sleep' not described in 'nx_hcall_sync' drivers/crypto/nx/nx.c:43: warning: expecting prototype for Nest Accelerators driver(). Prototype was for nx_hcall_sync() instead drivers/crypto/nx/nx.c:209: warning: Function parameter or member 'nbytes' not described in 'trim_sg_list' Cc: "Breno Leitão" Cc: Nayna Jain Cc: Paulo Flabiano Smorigo Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Herbert Xu Cc: "David S. Miller" Cc: Kent Yoder Cc: linux-cry...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Lee Jones --- drivers/crypto/nx/nx-aes-cbc.c | 2 +- drivers/crypto/nx/nx.c | 5 +++-- drivers/crypto/nx/nx_debugfs.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c index 92e921eceed75..d6314ea9ae896 100644 --- a/drivers/crypto/nx/nx-aes-cbc.c +++ b/drivers/crypto/nx/nx-aes-cbc.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/** +/* * AES CBC routines supporting the Power 7+ Nest Accelerators driver * * Copyright (C) 2011-2012 International Business Machines Inc. diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c index 1d0e8a1ba1605..010e87d9da36b 100644 --- a/drivers/crypto/nx/nx.c +++ b/drivers/crypto/nx/nx.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/** +/* * Routines supporting the Power 7+ Nest Accelerators driver * * Copyright (C) 2011-2012 International Business Machines Inc. @@ -200,7 +200,8 @@ struct nx_sg *nx_walk_and_build(struct nx_sg *nx_dst, * @sg: sg list head * @end: sg lisg end * @delta: is the amount we need to crop in order to bound the list. - * + * @nbytes: length of data in the scatterlists or data length - whichever + * is greater. */ static long int trim_sg_list(struct nx_sg *sg, struct nx_sg *end, diff --git a/drivers/crypto/nx/nx_debugfs.c b/drivers/crypto/nx/nx_debugfs.c index 1975bcbee9974..ee7cd88bb10a7 100644 --- a/drivers/crypto/nx/nx_debugfs.c +++ b/drivers/crypto/nx/nx_debugfs.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/** +/* * debugfs routines supporting the Power 7+ Nest Accelerators driver * * Copyright (C) 2011-2012 International Business Machines Inc. -- 2.27.0
[PATCH 08/10] crypto: vmx: Source headers are not good kernel-doc candidates
Fixes the following W=1 kernel build warning(s): drivers/crypto/vmx/vmx.c:23: warning: expecting prototype for Routines supporting VMX instructions on the Power 8(). Prototype was for p8_init() instead Cc: "Breno Leitão" Cc: Nayna Jain Cc: Paulo Flabiano Smorigo Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Herbert Xu Cc: "David S. Miller" Cc: Henrique Cerri Cc: linux-cry...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Lee Jones --- drivers/crypto/vmx/vmx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/crypto/vmx/vmx.c b/drivers/crypto/vmx/vmx.c index a40d08e75fc0b..7eb713cc87c8c 100644 --- a/drivers/crypto/vmx/vmx.c +++ b/drivers/crypto/vmx/vmx.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/** +/* * Routines supporting VMX instructions on the Power 8 * * Copyright (C) 2015 International Business Machines Inc. -- 2.27.0
[PATCH v2 00/10] Rid W=1 warnings in Crypto
This set is part of a larger effort attempting to clean-up W=1 kernel builds, which are currently overwhelmingly riddled with niggly little warnings. This is set 1 of 2 sets required to fully clean Crypto. No functional changes since v1. Lee Jones (10): crypto: hisilicon: sec_drv: Supply missing description for 'sec_queue_empty()'s 'queue' param crypto: bcm: Fix a whole host of kernel-doc misdemeanours crypto: chelsio: chcr_core: Fix some kernel-doc issues crypto: ux500: hash: hash_core: Fix worthy kernel-doc headers and remove others crypto: keembay: ocs-hcu: Fix incorrectly named functions/structs crypto: atmel-ecc: Struct headers need to start with keyword 'struct' crypto: caam: caampkc: Provide the name of the function and provide missing descriptions crypto: vmx: Source headers are not good kernel-doc candidates crypto: nx: nx-aes-cbc: Repair some kernel-doc problems crypto: cavium: nitrox_isr: Demote non-compliant kernel-doc headers drivers/crypto/atmel-ecc.c| 2 +- drivers/crypto/bcm/cipher.c | 7 ++-- drivers/crypto/bcm/spu.c | 16 - drivers/crypto/bcm/spu2.c | 43 +-- drivers/crypto/bcm/util.c | 4 +-- drivers/crypto/caam/caamalg_qi2.c | 2 ++ drivers/crypto/caam/caampkc.c | 3 +- drivers/crypto/cavium/nitrox/nitrox_isr.c | 4 +-- drivers/crypto/chelsio/chcr_algo.c| 8 ++--- drivers/crypto/chelsio/chcr_core.c| 2 +- drivers/crypto/hisilicon/sec/sec_drv.c| 1 + drivers/crypto/keembay/ocs-hcu.c | 6 ++-- drivers/crypto/nx/nx-aes-cbc.c| 2 +- drivers/crypto/nx/nx.c| 5 +-- drivers/crypto/nx/nx_debugfs.c| 2 +- drivers/crypto/ux500/cryp/cryp.c | 5 +-- drivers/crypto/ux500/cryp/cryp_core.c | 5 +-- drivers/crypto/ux500/cryp/cryp_irq.c | 2 +- drivers/crypto/ux500/hash/hash_core.c | 15 +++- drivers/crypto/vmx/vmx.c | 2 +- 20 files changed, 71 insertions(+), 65 deletions(-) Cc: Alexandre Belloni Cc: Andreas Westin Cc: Atul Gupta Cc: Aymen Sghaier Cc: Ayush Sawal Cc: Benjamin Herrenschmidt Cc: Berne Hebark Cc: "Breno Leitão" Cc: Daniele Alessandrelli Cc: "David S. Miller" Cc: Declan Murphy Cc: Harsh Jain Cc: Henrique Cerri Cc: Herbert Xu Cc: "Horia Geantă" Cc: Jitendra Lulla Cc: Joakim Bech Cc: Jonas Linde Cc: Jonathan Cameron Cc: Kent Yoder Cc: linux-arm-ker...@lists.infradead.org Cc: linux-cry...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: Ludovic Desroches Cc: Manoj Malviya Cc: Michael Ellerman Cc: M R Gowda Cc: Nayna Jain Cc: Nicolas Ferre Cc: Niklas Hernaeus Cc: Paul Mackerras Cc: Paulo Flabiano Smorigo Cc: Rob Rice Cc: Rohit Maheshwari Cc: Shujuan Chen Cc: Tudor Ambarus Cc: Vinay Kumar Yadav Cc: Zaibo Xu -- 2.27.0
Re: lkml delivery: was: Re: [PATCH next v4 00/15] printk: remove logbuf_lock
On Wed, 3 Mar 2021 14:18:29 +0100 Petr Mladek wrote: > Hi John, > > On Wed 2021-03-03 11:15:13, John Ogness wrote: > > Hello, > > > > Here is v4 of a series to remove @logbuf_lock, exposing the > > ringbuffer locklessly to both readers and writers. v3 is > > here [0]. > > Have you got some reply from lkml that it has not delivered there, > please? vger has been having some issues as of late, and emails have been coming in slowly. I just received emails I sent more than 24 hours a head of time. Those in charge are trying to work things out. -- Steve
[PATCH v1] powerpc: Include running function as first entry in save_stack_trace() and friends
It seems like all other sane architectures, namely x86 and arm64
at least, include the running function as top entry when saving
stack trace.
Functionnalities like KFENCE expect it.
Do the same on powerpc, it allows KFENCE to properly identify the faulting
function as depicted below. Before the patch KFENCE was identifying
finish_task_switch.isra as the faulting function.
[ 14.937370]
==
[ 14.948692] BUG: KFENCE: invalid read in test_invalid_access+0x54/0x108
[ 14.948692]
[ 14.956814] Invalid read at 0xdf98800a:
[ 14.960664] test_invalid_access+0x54/0x108
[ 14.964876] finish_task_switch.isra.0+0x54/0x23c
[ 14.969606] kunit_try_run_case+0x5c/0xd0
[ 14.973658] kunit_generic_run_threadfn_adapter+0x24/0x30
[ 14.979079] kthread+0x15c/0x174
[ 14.982342] ret_from_kernel_thread+0x14/0x1c
[ 14.986731]
[ 14.988236] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB
5.12.0-rc1-01537-g95f6e2088d7e-dirty #4682
[ 14.999795] NIP: c016ec2c LR: c02f517c CTR: c016ebd8
[ 15.004851] REGS: e2449d90 TRAP: 0301 Tainted: GB
(5.12.0-rc1-01537-g95f6e2088d7e-dirty)
[ 15.015274] MSR: 9032 CR: 2204 XER:
[ 15.022043] DAR: df98800a DSISR: 2000
[ 15.022043] GPR00: c02f517c e2449e50 c1142080 e100dd24 c084b13c 0008
c084b32b c016ebd8
[ 15.022043] GPR08: c085 df988000 c0d1 e2449eb0 22000288
[ 15.040581] NIP [c016ec2c] test_invalid_access+0x54/0x108
[ 15.046010] LR [c02f517c] kunit_try_run_case+0x5c/0xd0
[ 15.051181] Call Trace:
[ 15.053637] [e2449e50] [c005a68c] finish_task_switch.isra.0+0x54/0x23c
(unreliable)
[ 15.061338] [e2449eb0] [c02f517c] kunit_try_run_case+0x5c/0xd0
[ 15.067215] [e2449ed0] [c02f648c]
kunit_generic_run_threadfn_adapter+0x24/0x30
[ 15.074472] [e2449ef0] [c004e7b0] kthread+0x15c/0x174
[ 15.079571] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c
[ 15.085798] Instruction dump:
[ 15.088784] 8129d608 38e7ebd8 81020280 911f004c 3900 995f0024 907f0028
90ff001c
[ 15.096613] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98
3d40c02f
[ 15.104612]
==
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/stacktrace.c | 42 +---
1 file changed, 28 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index b6440657ef92..67c2b8488035 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -22,16 +22,32 @@
#include
#include
+#include
/*
* Save stack-backtrace addresses into a stack_trace buffer.
*/
+static void save_entry(struct stack_trace *trace, unsigned long ip, int
savesched)
+{
+ if (savesched || !in_sched_functions(ip)) {
+ if (!trace->skip)
+ trace->entries[trace->nr_entries++] = ip;
+ else
+ trace->skip--;
+ }
+}
+
static void save_context_stack(struct stack_trace *trace, unsigned long sp,
- struct task_struct *tsk, int savesched)
+ unsigned long ip, struct task_struct *tsk, int
savesched)
{
+ save_entry(trace, ip, savesched);
+
+ if (trace->nr_entries >= trace->max_entries)
+ return;
+
for (;;) {
unsigned long *stack = (unsigned long *) sp;
- unsigned long newsp, ip;
+ unsigned long newsp;
if (!validate_sp(sp, tsk, STACK_FRAME_OVERHEAD))
return;
@@ -39,12 +55,7 @@ static void save_context_stack(struct stack_trace *trace,
unsigned long sp,
newsp = stack[0];
ip = stack[STACK_FRAME_LR_SAVE];
- if (savesched || !in_sched_functions(ip)) {
- if (!trace->skip)
- trace->entries[trace->nr_entries++] = ip;
- else
- trace->skip--;
- }
+ save_entry(trace, ip, savesched);
if (trace->nr_entries >= trace->max_entries)
return;
@@ -59,23 +70,26 @@ void save_stack_trace(struct stack_trace *trace)
sp = current_stack_frame();
- save_context_stack(trace, sp, current, 1);
+ save_context_stack(trace, sp, (unsigned long)save_stack_trace, current,
1);
}
EXPORT_SYMBOL_GPL(save_stack_trace);
void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
{
- unsigned long sp;
+ unsigned long sp, ip;
if (!try_get_task_stack(tsk))
return;
- if (tsk == current)
+ if (tsk == current) {
+ ip = (unsigned long)save_stack_trace_tsk;
sp = current_stack_frame();
- else
+ } else {
+ ip = (unsigned
Re: [PATCH] ASoC: hdmi-codec: fix platform_no_drv_owner.cocci warnings
Hi Yang, On Wed, Mar 3, 2021 at 5:54 AM Yang Li wrote: > > ./sound/soc/fsl/imx-hdmi.c:226:3-8: No need to set .owner here. The core > will do it. > > Remove .owner field if calls are used which set it automatically > > Reported-by: Abaci Robot > Signed-off-by: Yang Li The patch looks good, but please send a v2 with the correct Subject. It should mention imx-hdmi instead of hdmi-codec. Thanks
[RESEND 1/1] powerpc: asm: hvconsole: Move 'hvc_vio_init_early's prototype to shared location
Fixes the following W=1 kernel build warning(s): drivers/tty/hvc/hvc_vio.c:385:13: warning: no previous prototype for ‘hvc_vio_init_early’ [-Wmissing-prototypes] 385 | void __init hvc_vio_init_early(void) | ^~ Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Lee Jones Acked-by: Michael Ellerman --- arch/powerpc/include/asm/hvconsole.h | 3 +++ arch/powerpc/platforms/pseries/pseries.h | 3 --- arch/powerpc/platforms/pseries/setup.c | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/hvconsole.h b/arch/powerpc/include/asm/hvconsole.h index 999ed5ac90531..ccb2034506f0f 100644 --- a/arch/powerpc/include/asm/hvconsole.h +++ b/arch/powerpc/include/asm/hvconsole.h @@ -24,5 +24,8 @@ extern int hvc_get_chars(uint32_t vtermno, char *buf, int count); extern int hvc_put_chars(uint32_t vtermno, const char *buf, int count); +/* Provided by HVC VIO */ +void hvc_vio_init_early(void); + #endif /* __KERNEL__ */ #endif /* _PPC64_HVCONSOLE_H */ diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h index 4fe48c04c6c20..a13438fca10a8 100644 --- a/arch/powerpc/platforms/pseries/pseries.h +++ b/arch/powerpc/platforms/pseries/pseries.h @@ -43,9 +43,6 @@ extern void pSeries_final_fixup(void); /* Poweron flag used for enabling auto ups restart */ extern unsigned long rtas_poweron_auto; -/* Provided by HVC VIO */ -extern void hvc_vio_init_early(void); - /* Dynamic logical Partitioning/Mobility */ extern void dlpar_free_cc_nodes(struct device_node *); extern void dlpar_free_cc_property(struct property *); diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 46e1540abc229..145e3f4c999af 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -71,6 +71,7 @@ #include #include #include +#include #include "pseries.h" #include "../../../../drivers/pci/pci.h" -- 2.27.0
Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
On Mär 03 2021, Marco Elver wrote:
> On Wed, 3 Mar 2021 at 11:32, Christophe Leroy
> wrote:
>> ./include/linux/kern_levels.h:5:18: warning: format '%zd' expects argument
>> of type 'signed size_t',
>> but argument 3 has type 'ptrdiff_t' {aka 'const long int'} [-Wformat=]
>> 5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
>>| ^~
>> ./include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
>> 11 | #define KERN_ERR KERN_SOH "3" /* error conditions */
>>| ^~~~
>> ./include/linux/printk.h:343:9: note: in expansion of macro 'KERN_ERR'
>>343 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
>>| ^~~~
>> mm/kfence/report.c:233:3: note: in expansion of macro 'pr_err'
>>233 | pr_err("Invalid free of 0x%p (in kfence-#%zd):\n", (void
>> *)address,
>>| ^~
>>
>> Christophe
>
> No this is not expected. Is 'signed size_t' != 'long int' on ppc32?
If you want to format a ptrdiff_t you should use %td.
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
Le 03/03/2021 à 11:39, Marco Elver a écrit : On Wed, 3 Mar 2021 at 11:32, Christophe Leroy wrote: Le 02/03/2021 à 10:53, Marco Elver a écrit : On Tue, 2 Mar 2021 at 10:27, Christophe Leroy wrote: Le 02/03/2021 à 10:21, Alexander Potapenko a écrit : [ 14.998426] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c [ 14.998426] [ 15.007061] Invalid read at 0x(ptrval): [ 15.010906] finish_task_switch.isra.0+0x54/0x23c [ 15.015633] kunit_try_run_case+0x5c/0xd0 [ 15.019682] kunit_generic_run_threadfn_adapter+0x24/0x30 [ 15.025099] kthread+0x15c/0x174 [ 15.028359] ret_from_kernel_thread+0x14/0x1c [ 15.032747] [ 15.034251] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674 [ 15.045811] == [ 15.053324] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636 [ 15.053324] Expected report_matches() to be true, but is false [ 15.068359] not ok 21 - test_invalid_access The test expects the function name to be test_invalid_access, i. e. the first line should be "BUG: KFENCE: invalid read in test_invalid_access". The error reporting function unwinds the stack, skips a couple of "uninteresting" frames (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L43) and uses the first "interesting" one frame to print the report header (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L226). It's strange that test_invalid_access is missing altogether from the stack trace - is that expected? Can you try printing the whole stacktrace without skipping any frames to see if that function is there? Booting with 'no_hash_pointers" I get the following. Does it helps ? [ 16.837198] == [ 16.848521] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c [ 16.848521] [ 16.857158] Invalid read at 0xdf98800a: [ 16.861004] finish_task_switch.isra.0+0x54/0x23c [ 16.865731] kunit_try_run_case+0x5c/0xd0 [ 16.869780] kunit_generic_run_threadfn_adapter+0x24/0x30 [ 16.875199] kthread+0x15c/0x174 [ 16.878460] ret_from_kernel_thread+0x14/0x1c [ 16.882847] [ 16.884351] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674 [ 16.895908] NIP: c016eb8c LR: c02f50dc CTR: c016eb38 [ 16.900963] REGS: e2449d90 TRAP: 0301 Tainted: GB (5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty) [ 16.911386] MSR: 9032 CR: 2204 XER: [ 16.918153] DAR: df98800a DSISR: 2000 [ 16.918153] GPR00: c02f50dc e2449e50 c1140d00 e100dd24 c084b13c 0008 c084b32b c016eb38 [ 16.918153] GPR08: c085 df988000 c0d1 e2449eb0 22000288 [ 16.936695] NIP [c016eb8c] test_invalid_access+0x54/0x108 [ 16.942125] LR [c02f50dc] kunit_try_run_case+0x5c/0xd0 [ 16.947292] Call Trace: [ 16.949746] [e2449e50] [c005a5ec] finish_task_switch.isra.0+0x54/0x23c (unreliable) The "(unreliable)" might be a clue that it's related to ppc32 stack unwinding. Any ppc expert know what this is about? [ 16.957443] [e2449eb0] [c02f50dc] kunit_try_run_case+0x5c/0xd0 [ 16.963319] [e2449ed0] [c02f63ec] kunit_generic_run_threadfn_adapter+0x24/0x30 [ 16.970574] [e2449ef0] [c004e710] kthread+0x15c/0x174 [ 16.975670] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c [ 16.981896] Instruction dump: [ 16.984879] 8129d608 38e7eb38 81020280 911f004c 3900 995f0024 907f0028 90ff001c [ 16.992710] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98 3d40c02f [ 17.000711] == [ 17.008223] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636 [ 17.008223] Expected report_matches() to be true, but is false [ 17.023243] not ok 21 - test_invalid_access On a fault in test_invalid_access, KFENCE prints the stack trace based on the information in pt_regs. So we do not think there's anything we can do to improve stack printing pe-se. What's confusing is that it's only this test, and none of the others. Given that, it might be code-gen related, which results in some subtle issue with stack unwinding. There are a few things to try, if you feel like it: -- Change the unwinder, if it's possible for ppc32. -- Add code to test_invalid_access(), to get the compiler to emit different code. E.g. add a bunch (unnecessary) function calls, or add barriers, etc. -- Play with compiler options. We already pass -fno-optimize-sibling-calls for kfence_test.o to avoid tail-call optimizations that'd hide stack trace entries. But perhaps there's something ppc-specific we missed? Well, the good thing is that KFENCE detects the bad access just fine. Since, according to the test, everything works from KFENCE's side, I'd be happy to give my Ack: Acked-by: Marco
Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
Le 02/03/2021 à 12:39, Marco Elver a écrit : On Tue, 2 Mar 2021 at 12:21, Christophe Leroy wrote: [...] Booting with 'no_hash_pointers" I get the following. Does it helps ? [ 16.837198] == [ 16.848521] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c [ 16.848521] [ 16.857158] Invalid read at 0xdf98800a: [ 16.861004] finish_task_switch.isra.0+0x54/0x23c [ 16.865731] kunit_try_run_case+0x5c/0xd0 [ 16.869780] kunit_generic_run_threadfn_adapter+0x24/0x30 [ 16.875199] kthread+0x15c/0x174 [ 16.878460] ret_from_kernel_thread+0x14/0x1c [ 16.882847] [ 16.884351] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674 [ 16.895908] NIP: c016eb8c LR: c02f50dc CTR: c016eb38 [ 16.900963] REGS: e2449d90 TRAP: 0301 Tainted: GB (5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty) [ 16.911386] MSR: 9032 CR: 2204 XER: [ 16.918153] DAR: df98800a DSISR: 2000 [ 16.918153] GPR00: c02f50dc e2449e50 c1140d00 e100dd24 c084b13c 0008 c084b32b c016eb38 [ 16.918153] GPR08: c085 df988000 c0d1 e2449eb0 22000288 [ 16.936695] NIP [c016eb8c] test_invalid_access+0x54/0x108 [ 16.942125] LR [c02f50dc] kunit_try_run_case+0x5c/0xd0 [ 16.947292] Call Trace: [ 16.949746] [e2449e50] [c005a5ec] finish_task_switch.isra.0+0x54/0x23c (unreliable) The "(unreliable)" might be a clue that it's related to ppc32 stack unwinding. Any ppc expert know what this is about? [ 16.957443] [e2449eb0] [c02f50dc] kunit_try_run_case+0x5c/0xd0 [ 16.963319] [e2449ed0] [c02f63ec] kunit_generic_run_threadfn_adapter+0x24/0x30 [ 16.970574] [e2449ef0] [c004e710] kthread+0x15c/0x174 [ 16.975670] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c [ 16.981896] Instruction dump: [ 16.984879] 8129d608 38e7eb38 81020280 911f004c 3900 995f0024 907f0028 90ff001c [ 16.992710] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98 3d40c02f [ 17.000711] == [ 17.008223] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636 [ 17.008223] Expected report_matches() to be true, but is false [ 17.023243] not ok 21 - test_invalid_access On a fault in test_invalid_access, KFENCE prints the stack trace based on the information in pt_regs. So we do not think there's anything we can do to improve stack printing pe-se. stack printing, probably not. Would be good anyway to mark the last level [unreliable] as the ppc does. We use stack_trace_save_regs() + stack_trace_print(). IIUC, on ppc the address in the stack frame of the caller is written by the caller. In most tests, there is some function call being done before the fault, for instance test_kmalloc_aligned_oob_read() does a call to kunit_do_assertion which populates the address of the call in the stack. However this is fragile. Interesting, this might explain it. This works for function calls because in order to call a subfunction, a function has to set up a stack frame in order to same the value in the Link Register, which contains the address of the function's parent and that will be clobbered by the sub-function call. However, it cannot be done by exceptions, because exceptions can happen in a function that has no stack frame (because that function has no need to call a subfunction and doesn't need to same anything on the stack). If the exception handler was writting the caller's address in the stack frame, it would in fact write it in the parent's frame, leading to a mess. But in fact the information is in pt_regs, it is in regs->nip so KFENCE should be able to use that instead of the stack. Perhaps stack_trace_save_regs() needs fixing for ppc32? Although that seems to use arch_stack_walk(). What's confusing is that it's only this test, and none of the others. Given that, it might be code-gen related, which results in some subtle issue with stack unwinding. There are a few things to try, if you feel like it: -- Change the unwinder, if it's possible for ppc32. I don't think it is possible. -- Add code to test_invalid_access(), to get the compiler to emit different code. E.g. add a bunch (unnecessary) function calls, or add barriers, etc. The following does the trick diff --git a/mm/kfence/kfence_test.c b/mm/kfence/kfence_test.c index 4acf4251ee04..22550676cd1f 100644 --- a/mm/kfence/kfence_test.c +++ b/mm/kfence/kfence_test.c @@ -631,8 +631,11 @@ static void test_invalid_access(struct kunit *test) .addr = &__kfence_pool[10], .is_write = false, }; + char *buf; + buf = test_alloc(test, 4, GFP_KERNEL, ALLOCATE_RIGHT); READ_ONCE(__kfence_pool[10]); + test_free(buf); KUNIT_EXPECT_TRUE(test, report_matches()); } But as I said above, this is fragile. If for some
Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
Le 02/03/2021 à 10:53, Marco Elver a écrit : On Tue, 2 Mar 2021 at 10:27, Christophe Leroy wrote: Le 02/03/2021 à 10:21, Alexander Potapenko a écrit : [ 14.998426] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c [ 14.998426] [ 15.007061] Invalid read at 0x(ptrval): [ 15.010906] finish_task_switch.isra.0+0x54/0x23c [ 15.015633] kunit_try_run_case+0x5c/0xd0 [ 15.019682] kunit_generic_run_threadfn_adapter+0x24/0x30 [ 15.025099] kthread+0x15c/0x174 [ 15.028359] ret_from_kernel_thread+0x14/0x1c [ 15.032747] [ 15.034251] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674 [ 15.045811] == [ 15.053324] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636 [ 15.053324] Expected report_matches() to be true, but is false [ 15.068359] not ok 21 - test_invalid_access The test expects the function name to be test_invalid_access, i. e. the first line should be "BUG: KFENCE: invalid read in test_invalid_access". The error reporting function unwinds the stack, skips a couple of "uninteresting" frames (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L43) and uses the first "interesting" one frame to print the report header (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L226). It's strange that test_invalid_access is missing altogether from the stack trace - is that expected? Can you try printing the whole stacktrace without skipping any frames to see if that function is there? Booting with 'no_hash_pointers" I get the following. Does it helps ? [ 16.837198] == [ 16.848521] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c [ 16.848521] [ 16.857158] Invalid read at 0xdf98800a: [ 16.861004] finish_task_switch.isra.0+0x54/0x23c [ 16.865731] kunit_try_run_case+0x5c/0xd0 [ 16.869780] kunit_generic_run_threadfn_adapter+0x24/0x30 [ 16.875199] kthread+0x15c/0x174 [ 16.878460] ret_from_kernel_thread+0x14/0x1c [ 16.882847] [ 16.884351] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674 [ 16.895908] NIP: c016eb8c LR: c02f50dc CTR: c016eb38 [ 16.900963] REGS: e2449d90 TRAP: 0301 Tainted: GB (5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty) [ 16.911386] MSR: 9032 CR: 2204 XER: [ 16.918153] DAR: df98800a DSISR: 2000 [ 16.918153] GPR00: c02f50dc e2449e50 c1140d00 e100dd24 c084b13c 0008 c084b32b c016eb38 [ 16.918153] GPR08: c085 df988000 c0d1 e2449eb0 22000288 [ 16.936695] NIP [c016eb8c] test_invalid_access+0x54/0x108 [ 16.942125] LR [c02f50dc] kunit_try_run_case+0x5c/0xd0 [ 16.947292] Call Trace: [ 16.949746] [e2449e50] [c005a5ec] finish_task_switch.isra.0+0x54/0x23c (unreliable) The "(unreliable)" might be a clue that it's related to ppc32 stack unwinding. Any ppc expert know what this is about? [ 16.957443] [e2449eb0] [c02f50dc] kunit_try_run_case+0x5c/0xd0 [ 16.963319] [e2449ed0] [c02f63ec] kunit_generic_run_threadfn_adapter+0x24/0x30 [ 16.970574] [e2449ef0] [c004e710] kthread+0x15c/0x174 [ 16.975670] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c [ 16.981896] Instruction dump: [ 16.984879] 8129d608 38e7eb38 81020280 911f004c 3900 995f0024 907f0028 90ff001c [ 16.992710] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98 3d40c02f [ 17.000711] == [ 17.008223] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636 [ 17.008223] Expected report_matches() to be true, but is false [ 17.023243] not ok 21 - test_invalid_access On a fault in test_invalid_access, KFENCE prints the stack trace based on the information in pt_regs. So we do not think there's anything we can do to improve stack printing pe-se. What's confusing is that it's only this test, and none of the others. Given that, it might be code-gen related, which results in some subtle issue with stack unwinding. There are a few things to try, if you feel like it: -- Change the unwinder, if it's possible for ppc32. -- Add code to test_invalid_access(), to get the compiler to emit different code. E.g. add a bunch (unnecessary) function calls, or add barriers, etc. -- Play with compiler options. We already pass -fno-optimize-sibling-calls for kfence_test.o to avoid tail-call optimizations that'd hide stack trace entries. But perhaps there's something ppc-specific we missed? Well, the good thing is that KFENCE detects the bad access just fine. Since, according to the test, everything works from KFENCE's side, I'd be happy to give my Ack: Acked-by: Marco Elver Thanks. For you information, I've got a pile of warnings from mm/kfence/report.o . Is that
Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
Le 02/03/2021 à 12:40, Michael Ellerman a écrit : Christophe Leroy writes: Le 02/03/2021 à 10:53, Marco Elver a écrit : On Tue, 2 Mar 2021 at 10:27, Christophe Leroy wrote: Le 02/03/2021 à 10:21, Alexander Potapenko a écrit : [ 14.998426] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c [ 14.998426] [ 15.007061] Invalid read at 0x(ptrval): [ 15.010906] finish_task_switch.isra.0+0x54/0x23c [ 15.015633] kunit_try_run_case+0x5c/0xd0 [ 15.019682] kunit_generic_run_threadfn_adapter+0x24/0x30 [ 15.025099] kthread+0x15c/0x174 [ 15.028359] ret_from_kernel_thread+0x14/0x1c [ 15.032747] [ 15.034251] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674 [ 15.045811] == [ 15.053324] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636 [ 15.053324] Expected report_matches() to be true, but is false [ 15.068359] not ok 21 - test_invalid_access The test expects the function name to be test_invalid_access, i. e. the first line should be "BUG: KFENCE: invalid read in test_invalid_access". The error reporting function unwinds the stack, skips a couple of "uninteresting" frames (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L43) and uses the first "interesting" one frame to print the report header (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L226). It's strange that test_invalid_access is missing altogether from the stack trace - is that expected? Can you try printing the whole stacktrace without skipping any frames to see if that function is there? Booting with 'no_hash_pointers" I get the following. Does it helps ? [ 16.837198] == [ 16.848521] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c [ 16.848521] [ 16.857158] Invalid read at 0xdf98800a: [ 16.861004] finish_task_switch.isra.0+0x54/0x23c [ 16.865731] kunit_try_run_case+0x5c/0xd0 [ 16.869780] kunit_generic_run_threadfn_adapter+0x24/0x30 [ 16.875199] kthread+0x15c/0x174 [ 16.878460] ret_from_kernel_thread+0x14/0x1c [ 16.882847] [ 16.884351] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: GB 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674 [ 16.895908] NIP: c016eb8c LR: c02f50dc CTR: c016eb38 [ 16.900963] REGS: e2449d90 TRAP: 0301 Tainted: GB (5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty) [ 16.911386] MSR: 9032 CR: 2204 XER: [ 16.918153] DAR: df98800a DSISR: 2000 [ 16.918153] GPR00: c02f50dc e2449e50 c1140d00 e100dd24 c084b13c 0008 c084b32b c016eb38 [ 16.918153] GPR08: c085 df988000 c0d1 e2449eb0 22000288 [ 16.936695] NIP [c016eb8c] test_invalid_access+0x54/0x108 [ 16.942125] LR [c02f50dc] kunit_try_run_case+0x5c/0xd0 [ 16.947292] Call Trace: [ 16.949746] [e2449e50] [c005a5ec] finish_task_switch.isra.0+0x54/0x23c (unreliable) The "(unreliable)" might be a clue that it's related to ppc32 stack unwinding. Any ppc expert know what this is about? [ 16.957443] [e2449eb0] [c02f50dc] kunit_try_run_case+0x5c/0xd0 [ 16.963319] [e2449ed0] [c02f63ec] kunit_generic_run_threadfn_adapter+0x24/0x30 [ 16.970574] [e2449ef0] [c004e710] kthread+0x15c/0x174 [ 16.975670] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c [ 16.981896] Instruction dump: [ 16.984879] 8129d608 38e7eb38 81020280 911f004c 3900 995f0024 907f0028 90ff001c [ 16.992710] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98 3d40c02f [ 17.000711] == [ 17.008223] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636 [ 17.008223] Expected report_matches() to be true, but is false [ 17.023243] not ok 21 - test_invalid_access On a fault in test_invalid_access, KFENCE prints the stack trace based on the information in pt_regs. So we do not think there's anything we can do to improve stack printing pe-se. stack printing, probably not. Would be good anyway to mark the last level [unreliable] as the ppc does. IIUC, on ppc the address in the stack frame of the caller is written by the caller. In most tests, there is some function call being done before the fault, for instance test_kmalloc_aligned_oob_read() does a call to kunit_do_assertion which populates the address of the call in the stack. However this is fragile. This works for function calls because in order to call a subfunction, a function has to set up a stack frame in order to same the value in the Link Register, which contains the address of the function's parent and that will be clobbered by the sub-function call. However, it cannot be done by exceptions, because exceptions can happen in a function that has no stack frame (because that function has no need to call a
[PATCH next v4 14/15] printk: kmsg_dump: remove _nolock() variants
kmsg_dump_rewind() and kmsg_dump_get_line() are lockless, so there is
no need for _nolock() variants. Remove these functions and switch all
callers of the _nolock() variants.
The functions without _nolock() were chosen because they are already
exported to kernel modules.
Signed-off-by: John Ogness
Reviewed-by: Petr Mladek
---
arch/powerpc/xmon/xmon.c| 4 +--
include/linux/kmsg_dump.h | 16 --
kernel/debug/kdb/kdb_main.c | 8 ++---
kernel/printk/printk.c | 60 +
4 files changed, 14 insertions(+), 74 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 5978b90a885f..bf7d69625a2e 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3013,9 +3013,9 @@ dump_log_buf(void)
catch_memory_errors = 1;
sync();
- kmsg_dump_rewind_nolock();
+ kmsg_dump_rewind();
xmon_start_pagination();
- while (kmsg_dump_get_line_nolock(, false, buf, sizeof(buf), ))
{
+ while (kmsg_dump_get_line(, false, buf, sizeof(buf), )) {
buf[len] = '\0';
printf("%s", buf);
}
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 36c8c57e1051..906521c2329c 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -57,17 +57,12 @@ struct kmsg_dumper {
#ifdef CONFIG_PRINTK
void kmsg_dump(enum kmsg_dump_reason reason);
-bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog,
- char *line, size_t size, size_t *len);
-
bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
char *line, size_t size, size_t *len);
bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
char *buf, size_t size, size_t *len_out);
-void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter);
-
void kmsg_dump_rewind(struct kmsg_dump_iter *iter);
int kmsg_dump_register(struct kmsg_dumper *dumper);
@@ -80,13 +75,6 @@ static inline void kmsg_dump(enum kmsg_dump_reason reason)
{
}
-static inline bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter,
-bool syslog, const char *line,
-size_t size, size_t *len)
-{
- return false;
-}
-
static inline bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
const char *line, size_t size, size_t *len)
{
@@ -99,10 +87,6 @@ static inline bool kmsg_dump_get_buffer(struct
kmsg_dump_iter *iter, bool syslog
return false;
}
-static inline void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter)
-{
-}
-
static inline void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
{
}
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 8544d7a55a57..67d9f2403b52 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2126,8 +2126,8 @@ static int kdb_dmesg(int argc, const char **argv)
kdb_set(2, setargs);
}
- kmsg_dump_rewind_nolock();
- while (kmsg_dump_get_line_nolock(, 1, NULL, 0, NULL))
+ kmsg_dump_rewind();
+ while (kmsg_dump_get_line(, 1, NULL, 0, NULL))
n++;
if (lines < 0) {
@@ -2159,8 +2159,8 @@ static int kdb_dmesg(int argc, const char **argv)
if (skip >= n || skip < 0)
return 0;
- kmsg_dump_rewind_nolock();
- while (kmsg_dump_get_line_nolock(, 1, buf, sizeof(buf), )) {
+ kmsg_dump_rewind();
+ while (kmsg_dump_get_line(, 1, buf, sizeof(buf), )) {
if (skip) {
skip--;
continue;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8994bc192b88..602de86d4e76 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3373,7 +3373,7 @@ void kmsg_dump(enum kmsg_dump_reason reason)
}
/**
- * kmsg_dump_get_line_nolock - retrieve one kmsg log line (unlocked version)
+ * kmsg_dump_get_line - retrieve one kmsg log line
* @iter: kmsg dump iterator
* @syslog: include the "<4>" prefixes
* @line: buffer to copy the line to
@@ -3388,22 +3388,22 @@ void kmsg_dump(enum kmsg_dump_reason reason)
*
* A return value of FALSE indicates that there are no more records to
* read.
- *
- * The function is similar to kmsg_dump_get_line(), but grabs no locks.
*/
-bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog,
- char *line, size_t size, size_t *len)
+bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
+ char *line, size_t size, size_t *len)
{
u64 min_seq = latched_seq_read_nolock(_seq);
struct printk_info info;
unsigned int line_count;
struct printk_record r;
+ unsigned long flags;
size_t l = 0;
bool ret = false;
if (iter->cur_seq <
[PATCH next v4 12/15] printk: introduce a kmsg_dump iterator
Rather than storing the iterator information in the registered
kmsg_dumper structure, create a separate iterator structure. The
kmsg_dump_iter structure can reside on the stack of the caller, thus
allowing lockless use of the kmsg_dump functions.
Update code that accesses the kernel logs using the kmsg_dumper
structure to use the new kmsg_dump_iter structure. For kmsg_dumpers,
this also means adding a call to kmsg_dump_rewind() to initialize
the iterator.
All this is in preparation for removal of @logbuf_lock.
Signed-off-by: John Ogness
Reviewed-by: Kees Cook # pstore
---
arch/powerpc/kernel/nvram_64.c | 8 +++--
arch/powerpc/xmon/xmon.c | 6 ++--
arch/um/kernel/kmsg_dump.c | 5 ++-
drivers/hv/vmbus_drv.c | 4 ++-
drivers/mtd/mtdoops.c | 5 ++-
fs/pstore/platform.c | 5 ++-
include/linux/kmsg_dump.h | 36 ++-
kernel/debug/kdb/kdb_main.c| 10 +++---
kernel/printk/printk.c | 63 +-
9 files changed, 80 insertions(+), 62 deletions(-)
diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index 532f22637783..3c8d9bbb51cf 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -647,6 +647,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
{
struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
static unsigned int oops_count = 0;
+ static struct kmsg_dump_iter iter;
static bool panicking = false;
static DEFINE_SPINLOCK(lock);
unsigned long flags;
@@ -681,13 +682,14 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
return;
if (big_oops_buf) {
- kmsg_dump_get_buffer(dumper, false,
+ kmsg_dump_rewind();
+ kmsg_dump_get_buffer(, false,
big_oops_buf, big_oops_buf_sz, _len);
rc = zip_oops(text_len);
}
if (rc != 0) {
- kmsg_dump_rewind(dumper);
- kmsg_dump_get_buffer(dumper, false,
+ kmsg_dump_rewind();
+ kmsg_dump_get_buffer(, false,
oops_data, oops_data_sz, _len);
err_type = ERR_TYPE_KERNEL_PANIC;
oops_hdr->version = cpu_to_be16(OOPS_HDR_VERSION);
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 80ed3e1becf9..5978b90a885f 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3001,7 +3001,7 @@ print_address(unsigned long addr)
static void
dump_log_buf(void)
{
- struct kmsg_dumper dumper;
+ struct kmsg_dump_iter iter;
unsigned char buf[128];
size_t len;
@@ -3013,9 +3013,9 @@ dump_log_buf(void)
catch_memory_errors = 1;
sync();
- kmsg_dump_rewind_nolock();
+ kmsg_dump_rewind_nolock();
xmon_start_pagination();
- while (kmsg_dump_get_line_nolock(, false, buf, sizeof(buf),
)) {
+ while (kmsg_dump_get_line_nolock(, false, buf, sizeof(buf), ))
{
buf[len] = '\0';
printf("%s", buf);
}
diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
index a765d235e50e..0224fcb36e22 100644
--- a/arch/um/kernel/kmsg_dump.c
+++ b/arch/um/kernel/kmsg_dump.c
@@ -10,6 +10,7 @@
static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
enum kmsg_dump_reason reason)
{
+ static struct kmsg_dump_iter iter;
static DEFINE_SPINLOCK(lock);
static char line[1024];
struct console *con;
@@ -35,8 +36,10 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
if (!spin_trylock_irqsave(, flags))
return;
+ kmsg_dump_rewind();
+
printf("kmsg_dump:\n");
- while (kmsg_dump_get_line(dumper, true, line, sizeof(line), )) {
+ while (kmsg_dump_get_line(, true, line, sizeof(line), )) {
line[len] = '\0';
printf("%s", line);
}
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 10dce9f91216..b341b144bde8 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1391,6 +1391,7 @@ static void vmbus_isr(void)
static void hv_kmsg_dump(struct kmsg_dumper *dumper,
enum kmsg_dump_reason reason)
{
+ struct kmsg_dump_iter iter;
size_t bytes_written;
phys_addr_t panic_pa;
@@ -1404,7 +1405,8 @@ static void hv_kmsg_dump(struct kmsg_dumper *dumper,
* Write dump contents to the page. No need to synchronize; panic should
* be single-threaded.
*/
- kmsg_dump_get_buffer(dumper, false, hv_panic_page, HV_HYP_PAGE_SIZE,
+ kmsg_dump_rewind();
+ kmsg_dump_get_buffer(, false, hv_panic_page, HV_HYP_PAGE_SIZE,
_written);
if (bytes_written)
hyperv_report_panic_msg(panic_pa, bytes_written);
diff
[PATCH next v4 11/15] printk: kmsg_dumper: remove @active field
All 6 kmsg_dumpers do not benefit from the @active flag:
(provide their own synchronization)
- arch/powerpc/kernel/nvram_64.c
- arch/um/kernel/kmsg_dump.c
- drivers/mtd/mtdoops.c
- fs/pstore/platform.c
(only dump on KMSG_DUMP_PANIC, which does not require
synchronization)
- arch/powerpc/platforms/powernv/opal-kmsg.c
- drivers/hv/vmbus_drv.c
The other 2 kmsg_dump users also do not rely on @active:
(hard-code @active to always be true)
- arch/powerpc/xmon/xmon.c
- kernel/debug/kdb/kdb_main.c
Therefore, @active can be removed.
Signed-off-by: John Ogness
Reviewed-by: Petr Mladek
---
arch/powerpc/xmon/xmon.c| 2 +-
include/linux/kmsg_dump.h | 2 --
kernel/debug/kdb/kdb_main.c | 2 +-
kernel/printk/printk.c | 10 +-
4 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 3fe37495f63d..80ed3e1becf9 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3001,7 +3001,7 @@ print_address(unsigned long addr)
static void
dump_log_buf(void)
{
- struct kmsg_dumper dumper = { .active = 1 };
+ struct kmsg_dumper dumper;
unsigned char buf[128];
size_t len;
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 070c994ff19f..84eaa2090efa 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -36,7 +36,6 @@ enum kmsg_dump_reason {
* through the record iterator
* @max_reason:filter for highest reason number that should be dumped
* @registered:Flag that specifies if this is already registered
- * @active:Flag that specifies if this is currently dumping
* @cur_seq: Points to the oldest message to dump
* @next_seq: Points after the newest message to dump
*/
@@ -44,7 +43,6 @@ struct kmsg_dumper {
struct list_head list;
void (*dump)(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason);
enum kmsg_dump_reason max_reason;
- bool active;
bool registered;
/* private state of the kmsg iterator */
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 930ac1b25ec7..315169d5e119 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2101,7 +2101,7 @@ static int kdb_dmesg(int argc, const char **argv)
int adjust = 0;
int n = 0;
int skip = 0;
- struct kmsg_dumper dumper = { .active = 1 };
+ struct kmsg_dumper dumper;
size_t len;
char buf[201];
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index e794a08de00f..ce4cc64ba7c9 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3408,8 +3408,6 @@ void kmsg_dump(enum kmsg_dump_reason reason)
continue;
/* initialize iterator with data about the stored records */
- dumper->active = true;
-
logbuf_lock_irqsave(flags);
dumper->cur_seq = latched_seq_read_nolock(_seq);
dumper->next_seq = prb_next_seq(prb);
@@ -3417,9 +3415,6 @@ void kmsg_dump(enum kmsg_dump_reason reason)
/* invoke dumper which will iterate over records */
dumper->dump(dumper, reason);
-
- /* reset iterator */
- dumper->active = false;
}
rcu_read_unlock();
}
@@ -3454,9 +3449,6 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper
*dumper, bool syslog,
prb_rec_init_rd(, , line, size);
- if (!dumper->active)
- goto out;
-
/* Read text or count text lines? */
if (line) {
if (!prb_read_valid(prb, dumper->cur_seq, ))
@@ -3542,7 +3534,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper,
bool syslog,
bool ret = false;
bool time = printk_time;
- if (!dumper->active || !buf || !size)
+ if (!buf || !size)
goto out;
logbuf_lock_irqsave(flags);
--
2.20.1
[PATCH next v4 00/15] printk: remove logbuf_lock
Hello, Here is v4 of a series to remove @logbuf_lock, exposing the ringbuffer locklessly to both readers and writers. v3 is here [0]. Since @logbuf_lock was protecting much more than just the ringbuffer, this series clarifies and cleans up the various protections using comments, lockless accessors, atomic types, and a new finer-grained @syslog_lock. Removing @logbuf_lock required changing the semantics of the kmsg_dumper callback in order to work locklessly. This series adjusts all kmsg_dumpers and users of the kmsg_dump_get_*() functions for the new semantics. This series is based on next-20210303. Changes since v3: - disable interrupts in the arch/um kmsg_dumper - reduce CONSOLE_LOG_MAX value from 4096 back to 1024 to revert the increasd 3KiB static memory footprint - change the kmsg_dumper() callback prototype back to how it was because some dumpers need the registered object for container_of() usage - for kmsg_dump_get_line()/kmsg_dump_get_buffer() restrict the minimal allowed sequence number to the cleared sequence number John Ogness [0] https://lore.kernel.org/lkml/20210225202438.28985-1-john.ogn...@linutronix.de/ John Ogness (15): um: synchronize kmsg_dumper mtd: mtdoops: synchronize kmsg_dumper printk: limit second loop of syslog_print_all printk: kmsg_dump: remove unused fields printk: refactor kmsg_dump_get_buffer() printk: consolidate kmsg_dump_get_buffer/syslog_print_all code printk: introduce CONSOLE_LOG_MAX printk: use seqcount_latch for clear_seq printk: use atomic64_t for devkmsg_user.seq printk: add syslog_lock printk: kmsg_dumper: remove @active field printk: introduce a kmsg_dump iterator printk: remove logbuf_lock printk: kmsg_dump: remove _nolock() variants printk: console: remove unnecessary safe buffer usage arch/powerpc/kernel/nvram_64.c | 8 +- arch/powerpc/xmon/xmon.c | 6 +- arch/um/kernel/kmsg_dump.c | 13 +- drivers/hv/vmbus_drv.c | 4 +- drivers/mtd/mtdoops.c | 17 +- fs/pstore/platform.c | 5 +- include/linux/kmsg_dump.h | 47 ++-- kernel/debug/kdb/kdb_main.c| 10 +- kernel/printk/internal.h | 4 +- kernel/printk/printk.c | 464 + kernel/printk/printk_safe.c| 27 +- 11 files changed, 310 insertions(+), 295 deletions(-) -- 2.20.1
[PATCH][next] ASoC: fsl: fsl_easrc: Fix uninitialized variable st2_mem_alloc
From: Colin Ian King
A previous cleanup commit removed the ininitialization of st2_mem_alloc.
Fix this by restoring the original behaviour by initializing it to zero.
Addresses-Coverity: ("Uninitialized scalar variable")
Fixes: e80382fe721f ("ASoC: fsl: fsl_easrc: remove useless assignments")
Signed-off-by: Colin Ian King
---
sound/soc/fsl/fsl_easrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
index 725a5d3aaa02..e823c9c13764 100644
--- a/sound/soc/fsl/fsl_easrc.c
+++ b/sound/soc/fsl/fsl_easrc.c
@@ -710,7 +710,7 @@ static int fsl_easrc_max_ch_for_slot(struct fsl_asrc_pair
*ctx,
struct fsl_easrc_slot *slot)
{
struct fsl_easrc_ctx_priv *ctx_priv = ctx->private;
- int st1_mem_alloc = 0, st2_mem_alloc;
+ int st1_mem_alloc = 0, st2_mem_alloc = 0;
int pf_mem_alloc = 0;
int max_channels = 8 - slot->num_channel;
int channels = 0;
--
2.30.0
[PATCH] ASoC: hdmi-codec: fix platform_no_drv_owner.cocci warnings
./sound/soc/fsl/imx-hdmi.c:226:3-8: No need to set .owner here. The core
will do it.
Remove .owner field if calls are used which set it automatically
Reported-by: Abaci Robot
Signed-off-by: Yang Li
---
sound/soc/fsl/imx-hdmi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sound/soc/fsl/imx-hdmi.c b/sound/soc/fsl/imx-hdmi.c
index dbbb761..cd0235a 100644
--- a/sound/soc/fsl/imx-hdmi.c
+++ b/sound/soc/fsl/imx-hdmi.c
@@ -223,7 +223,6 @@ static int imx_hdmi_probe(struct platform_device *pdev)
static struct platform_driver imx_hdmi_driver = {
.driver = {
.name = "imx-hdmi",
- .owner = THIS_MODULE,
.pm = _soc_pm_ops,
.of_match_table = imx_hdmi_dt_ids,
},
--
1.8.3.1
Re: [PATCH 40/44] tty: hvc, drop unneeded forward declarations
Hello Jiri, On Tue, Mar 02, 2021 at 07:22:10AM +0100, Jiri Slaby wrote: > Forward declarations make the code larger and rewrites harder. Harder as > they are often omitted from global changes. Remove forward declarations > which are not really needed, i.e. the definition of the function is > before its first use. > > Signed-off-by: Jiri Slaby > Cc: linuxppc-dev@lists.ozlabs.org > --- > drivers/tty/hvc/hvcs.c | 25 - note this conflicts with commit 386a966f5ce71a0364b158c5d0a6971f4e418ea8 that currently sits in the powerpc tree. I think it's easy to resolve. Other than that: Acked-by: Uwe Kleine-König Best regards Uwe > 1 file changed, 25 deletions(-) > > diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c > index c90848919644..0b89d878a108 100644 > --- a/drivers/tty/hvc/hvcs.c > +++ b/drivers/tty/hvc/hvcs.c > @@ -290,36 +290,11 @@ static LIST_HEAD(hvcs_structs); > static DEFINE_SPINLOCK(hvcs_structs_lock); > static DEFINE_MUTEX(hvcs_init_mutex); > > -static void hvcs_unthrottle(struct tty_struct *tty); > -static void hvcs_throttle(struct tty_struct *tty); > -static irqreturn_t hvcs_handle_interrupt(int irq, void *dev_instance); > - > -static int hvcs_write(struct tty_struct *tty, > - const unsigned char *buf, int count); > -static int hvcs_write_room(struct tty_struct *tty); > -static int hvcs_chars_in_buffer(struct tty_struct *tty); > - > -static int hvcs_has_pi(struct hvcs_struct *hvcsd); > -static void hvcs_set_pi(struct hvcs_partner_info *pi, > - struct hvcs_struct *hvcsd); > static int hvcs_get_pi(struct hvcs_struct *hvcsd); > static int hvcs_rescan_devices_list(void); > > -static int hvcs_partner_connect(struct hvcs_struct *hvcsd); > static void hvcs_partner_free(struct hvcs_struct *hvcsd); > > -static int hvcs_enable_device(struct hvcs_struct *hvcsd, > - uint32_t unit_address, unsigned int irq, struct vio_dev *dev); > - > -static int hvcs_open(struct tty_struct *tty, struct file *filp); > -static void hvcs_close(struct tty_struct *tty, struct file *filp); > -static void hvcs_hangup(struct tty_struct * tty); > - > -static int hvcs_probe(struct vio_dev *dev, > - const struct vio_device_id *id); > -static int hvcs_remove(struct vio_dev *dev); > -static int __init hvcs_module_init(void); > -static void __exit hvcs_module_exit(void); > static int hvcs_initialize(void); > > #define HVCS_SCHED_READ 0x0001 > -- > 2.30.1 > > -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
