* Re: [PATCH 0/8] add generic builtin command line
From: Ard Biesheuvel @ 2018-09-27 17:05 UTC (permalink / raw)
To: Maksym Kokhan
Cc: Andrew Morton, Rob Herring, Frank Rowand, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin,
the arch/x86 maintainers, Russell King, Catalin Marinas,
Will Deacon, Ralf Baechle, Paul Burton, James Hogan,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Daniel Walker, Daniel Walker, Andrii Bordunov, Ruslan Bilovol,
Linux Kernel Mailing List, Devicetree List, linux-efi, linux-mips,
linuxppc-dev
In-Reply-To: <1538067309-5711-1-git-send-email-maksym.kokhan@globallogic.com>
On 27 September 2018 at 18:55, Maksym Kokhan
<maksym.kokhan@globallogic.com> wrote:
> There were series of patches [1] for 4.3.0-rc3, that allowed
> architectures to use a generic builtin command line. I have rebased
> these patches on kernel 4.19.0-rc4.
>
Could you please elaborate on the purpose of this series? Is it simply
to align between architectures? Does it solve an actual problem?
> Things, modified in comparison with original patches:
> * There was some bug for mips, in the case when CONFIG_CMDLINE_PREPEND
> and CONFIG_CMDLINE_APPEND are empty and CMDLINE_OVERRIDE is not set,
> command line from bootloader was ignored, so I fixed it, modifying
> patch "add generic builtin command line".
>
> * Implemented new patch to resolve conflict with new kernel, which
> modify EFI stub code. Unfortunately, I don't have capability to test
> this modification on real arm board with EFI.
>
> * Removed new realisation of mips builtin command line, which was
> created after 4.3.0-rc3.
>
> * Kernel 4.3.0-rc3 with original patches could not be compiled for
> powerpc due to prom_init.c checking by prom_init_check.sh. So I added
> strlcat (which is used by cmdline_add_builtin macro) to
> prom_init_check.sh whitelist.
>
> Patches have been tested in QEMU for x86, arm (little-endian), arm64
> (little-endian), mips (little-endian, 32-bit) and powerpc
> (big-endian, 64-bit), everything works perfectly. Also it was tested
> on linux-next (next-20180924 tag) for all listed above architectures.
>
> [1] : https://lore.kernel.org/patchwork/patch/604992/
>
> Daniel Walker (7):
> add generic builtin command line
> drivers: of: ifdef out cmdline section
> x86: convert to generic builtin command line
> arm: convert to generic builtin command line
> arm64: convert to generic builtin command line
> mips: convert to generic builtin command line
> powerpc: convert to generic builtin command line
>
> Maksym Kokhan (1):
> efi: modify EFI stub code for arm/arm64
>
> arch/arm/Kconfig | 38 +-----------------
> arch/arm/kernel/atags_parse.c | 14 ++-----
> arch/arm/kernel/devtree.c | 2 +
> arch/arm64/Kconfig | 17 +-------
> arch/arm64/kernel/setup.c | 3 ++
> arch/mips/Kconfig | 24 +----------
> arch/mips/Kconfig.debug | 47 ----------------------
> arch/mips/kernel/setup.c | 41 ++-----------------
> arch/powerpc/Kconfig | 23 +----------
> arch/powerpc/kernel/prom.c | 4 ++
> arch/powerpc/kernel/prom_init.c | 8 ++--
> arch/powerpc/kernel/prom_init_check.sh | 2 +-
> arch/x86/Kconfig | 44 +--------------------
> arch/x86/kernel/setup.c | 19 ++-------
> drivers/firmware/efi/libstub/arm-stub.c | 10 ++---
> drivers/of/fdt.c | 2 +-
> include/linux/cmdline.h | 70 +++++++++++++++++++++++++++++++++
> init/Kconfig | 68 ++++++++++++++++++++++++++++++++
> 18 files changed, 173 insertions(+), 263 deletions(-)
> create mode 100644 include/linux/cmdline.h
>
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH 0/8] add generic builtin command line
From: Ard Biesheuvel @ 2018-09-27 17:05 UTC (permalink / raw)
To: Maksym Kokhan
Cc: linux-mips, linux-efi, Ruslan Bilovol, Will Deacon,
Paul Mackerras, H. Peter Anvin, Daniel Walker, Frank Rowand,
the arch/x86 maintainers, Russell King, Ingo Molnar,
Catalin Marinas, James Hogan, Daniel Walker, Devicetree List,
Rob Herring, Borislav Petkov, Thomas Gleixner, Andrii Bordunov,
Linux Kernel Mailing List, Ralf Baechle, Paul Burton,
Andrew Morton, linuxppc-dev
In-Reply-To: <1538067309-5711-1-git-send-email-maksym.kokhan@globallogic.com>
On 27 September 2018 at 18:55, Maksym Kokhan
<maksym.kokhan@globallogic.com> wrote:
> There were series of patches [1] for 4.3.0-rc3, that allowed
> architectures to use a generic builtin command line. I have rebased
> these patches on kernel 4.19.0-rc4.
>
Could you please elaborate on the purpose of this series? Is it simply
to align between architectures? Does it solve an actual problem?
> Things, modified in comparison with original patches:
> * There was some bug for mips, in the case when CONFIG_CMDLINE_PREPEND
> and CONFIG_CMDLINE_APPEND are empty and CMDLINE_OVERRIDE is not set,
> command line from bootloader was ignored, so I fixed it, modifying
> patch "add generic builtin command line".
>
> * Implemented new patch to resolve conflict with new kernel, which
> modify EFI stub code. Unfortunately, I don't have capability to test
> this modification on real arm board with EFI.
>
> * Removed new realisation of mips builtin command line, which was
> created after 4.3.0-rc3.
>
> * Kernel 4.3.0-rc3 with original patches could not be compiled for
> powerpc due to prom_init.c checking by prom_init_check.sh. So I added
> strlcat (which is used by cmdline_add_builtin macro) to
> prom_init_check.sh whitelist.
>
> Patches have been tested in QEMU for x86, arm (little-endian), arm64
> (little-endian), mips (little-endian, 32-bit) and powerpc
> (big-endian, 64-bit), everything works perfectly. Also it was tested
> on linux-next (next-20180924 tag) for all listed above architectures.
>
> [1] : https://lore.kernel.org/patchwork/patch/604992/
>
> Daniel Walker (7):
> add generic builtin command line
> drivers: of: ifdef out cmdline section
> x86: convert to generic builtin command line
> arm: convert to generic builtin command line
> arm64: convert to generic builtin command line
> mips: convert to generic builtin command line
> powerpc: convert to generic builtin command line
>
> Maksym Kokhan (1):
> efi: modify EFI stub code for arm/arm64
>
> arch/arm/Kconfig | 38 +-----------------
> arch/arm/kernel/atags_parse.c | 14 ++-----
> arch/arm/kernel/devtree.c | 2 +
> arch/arm64/Kconfig | 17 +-------
> arch/arm64/kernel/setup.c | 3 ++
> arch/mips/Kconfig | 24 +----------
> arch/mips/Kconfig.debug | 47 ----------------------
> arch/mips/kernel/setup.c | 41 ++-----------------
> arch/powerpc/Kconfig | 23 +----------
> arch/powerpc/kernel/prom.c | 4 ++
> arch/powerpc/kernel/prom_init.c | 8 ++--
> arch/powerpc/kernel/prom_init_check.sh | 2 +-
> arch/x86/Kconfig | 44 +--------------------
> arch/x86/kernel/setup.c | 19 ++-------
> drivers/firmware/efi/libstub/arm-stub.c | 10 ++---
> drivers/of/fdt.c | 2 +-
> include/linux/cmdline.h | 70 +++++++++++++++++++++++++++++++++
> init/Kconfig | 68 ++++++++++++++++++++++++++++++++
> 18 files changed, 173 insertions(+), 263 deletions(-)
> create mode 100644 include/linux/cmdline.h
>
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v2] kdb: use correct pointer when 'btc' calls 'btt'
From: Christophe Leroy @ 2018-09-27 17:17 UTC (permalink / raw)
To: Jason Wessel, Daniel Thompson
Cc: Michael Ellerman, linux-kernel, linuxppc-dev, kgdb-bugreport,
stable
On a powerpc 8xx, 'btc' fails as follows:
Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
kdb> btc
btc: cpu status: Currently on cpu 0
Available cpus: 0
kdb_getarea: Bad address 0x0
when booting the kernel with 'debug_boot_weak_hash', it fails as well
Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
kdb> btc
btc: cpu status: Currently on cpu 0
Available cpus: 0
kdb_getarea: Bad address 0xba99ad80
On other platforms, Oopses have been observed too, see
https://github.com/linuxppc/linux/issues/139
This is due to btc calling 'btt' with %p pointer as an argument.
This patch replaces %p by %px to get the real pointer value as
expected by 'btt'
Fixes: ad67b74d2469 ("printk: hash addresses printed with %p")
Cc: <stable@vger.kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v2: Added fixes: and Daniel's reviewed-by: in commit's log. No code change.
kernel/debug/kdb/kdb_bt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c
index 6ad4a9fcbd6f..7921ae4fca8d 100644
--- a/kernel/debug/kdb/kdb_bt.c
+++ b/kernel/debug/kdb/kdb_bt.c
@@ -179,14 +179,14 @@ kdb_bt(int argc, const char **argv)
kdb_printf("no process for cpu %ld\n", cpu);
return 0;
}
- sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
+ sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
kdb_parse(buf);
return 0;
}
kdb_printf("btc: cpu status: ");
kdb_parse("cpu\n");
for_each_online_cpu(cpu) {
- sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
+ sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
kdb_parse(buf);
touch_nmi_watchdog();
}
--
2.13.3
^ permalink raw reply related
* [PATCH] kdb: print real address of pointers instead of hashed addresses
From: Christophe Leroy @ 2018-09-27 17:17 UTC (permalink / raw)
To: Jason Wessel, Daniel Thompson
Cc: Michael Ellerman, linux-kernel, linuxppc-dev, kgdb-bugreport,
stable
Since commit ad67b74d2469 ("printk: hash addresses printed with %p"),
all pointers printed with %p are printed with hashed addresses
instead of real addresses in order to avoid leaking addresses in
dmesg and syslog. But this applies to kdb too, with is unfortunate:
Entering kdb (current=0x(ptrval), pid 329) due to Keyboard Entry
kdb> ps
15 sleeping system daemon (state M) processes suppressed,
use 'ps A' to see all.
Task Addr Pid Parent [*] cpu State Thread Command
0x(ptrval) 329 328 1 0 R 0x(ptrval) *sh
0x(ptrval) 1 0 0 0 S 0x(ptrval) init
0x(ptrval) 3 2 0 0 D 0x(ptrval) rcu_gp
0x(ptrval) 4 2 0 0 D 0x(ptrval) rcu_par_gp
0x(ptrval) 5 2 0 0 D 0x(ptrval) kworker/0:0
0x(ptrval) 6 2 0 0 D 0x(ptrval) kworker/0:0H
0x(ptrval) 7 2 0 0 D 0x(ptrval) kworker/u2:0
0x(ptrval) 8 2 0 0 D 0x(ptrval) mm_percpu_wq
0x(ptrval) 10 2 0 0 D 0x(ptrval) rcu_preempt
The whole purpose of kdb is to debug, and for debugging real addresses
need to be known. In addition, data displayed by kdb doesn't go into
dmesg.
This patch replaces all %p by %px in kdb in order to display real
addresses.
Fixes: ad67b74d2469 ("printk: hash addresses printed with %p")
Cc: <stable@vger.kernel.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
kernel/debug/kdb/kdb_main.c | 14 +++++++-------
kernel/debug/kdb/kdb_support.c | 12 ++++++------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 2ddfce8f1e8f..f338d23b112b 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1192,7 +1192,7 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
if (reason == KDB_REASON_DEBUG) {
/* special case below */
} else {
- kdb_printf("\nEntering kdb (current=0x%p, pid %d) ",
+ kdb_printf("\nEntering kdb (current=0x%px, pid %d) ",
kdb_current, kdb_current ? kdb_current->pid : 0);
#if defined(CONFIG_SMP)
kdb_printf("on processor %d ", raw_smp_processor_id());
@@ -1208,7 +1208,7 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
*/
switch (db_result) {
case KDB_DB_BPT:
- kdb_printf("\nEntering kdb (0x%p, pid %d) ",
+ kdb_printf("\nEntering kdb (0x%px, pid %d) ",
kdb_current, kdb_current->pid);
#if defined(CONFIG_SMP)
kdb_printf("on processor %d ", raw_smp_processor_id());
@@ -2048,7 +2048,7 @@ static int kdb_lsmod(int argc, const char **argv)
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- kdb_printf("%-20s%8u 0x%p ", mod->name,
+ kdb_printf("%-20s%8u 0x%px ", mod->name,
mod->core_layout.size, (void *)mod);
#ifdef CONFIG_MODULE_UNLOAD
kdb_printf("%4d ", module_refcount(mod));
@@ -2059,7 +2059,7 @@ static int kdb_lsmod(int argc, const char **argv)
kdb_printf(" (Loading)");
else
kdb_printf(" (Live)");
- kdb_printf(" 0x%p", mod->core_layout.base);
+ kdb_printf(" 0x%px", mod->core_layout.base);
#ifdef CONFIG_MODULE_UNLOAD
{
@@ -2341,7 +2341,7 @@ void kdb_ps1(const struct task_struct *p)
return;
cpu = kdb_process_cpu(p);
- kdb_printf("0x%p %8d %8d %d %4d %c 0x%p %c%s\n",
+ kdb_printf("0x%px %8d %8d %d %4d %c 0x%px %c%s\n",
(void *)p, p->pid, p->parent->pid,
kdb_task_has_cpu(p), kdb_process_cpu(p),
kdb_task_state_char(p),
@@ -2354,7 +2354,7 @@ void kdb_ps1(const struct task_struct *p)
} else {
if (KDB_TSK(cpu) != p)
kdb_printf(" Error: does not match running "
- "process table (0x%p)\n", KDB_TSK(cpu));
+ "process table (0x%px)\n", KDB_TSK(cpu));
}
}
}
@@ -2692,7 +2692,7 @@ int kdb_register_flags(char *cmd,
for_each_kdbcmd(kp, i) {
if (kp->cmd_name && (strcmp(kp->cmd_name, cmd) == 0)) {
kdb_printf("Duplicate kdb command registered: "
- "%s, func %p help %s\n", cmd, func, help);
+ "%s, func %px help %s\n", cmd, func, help);
return 1;
}
}
diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c
index 990b3cc526c8..987eb73284d2 100644
--- a/kernel/debug/kdb/kdb_support.c
+++ b/kernel/debug/kdb/kdb_support.c
@@ -40,7 +40,7 @@
int kdbgetsymval(const char *symname, kdb_symtab_t *symtab)
{
if (KDB_DEBUG(AR))
- kdb_printf("kdbgetsymval: symname=%s, symtab=%p\n", symname,
+ kdb_printf("kdbgetsymval: symname=%s, symtab=%px\n", symname,
symtab);
memset(symtab, 0, sizeof(*symtab));
symtab->sym_start = kallsyms_lookup_name(symname);
@@ -88,7 +88,7 @@ int kdbnearsym(unsigned long addr, kdb_symtab_t *symtab)
char *knt1 = NULL;
if (KDB_DEBUG(AR))
- kdb_printf("kdbnearsym: addr=0x%lx, symtab=%p\n", addr, symtab);
+ kdb_printf("kdbnearsym: addr=0x%lx, symtab=%px\n", addr, symtab);
memset(symtab, 0, sizeof(*symtab));
if (addr < 4096)
@@ -149,7 +149,7 @@ int kdbnearsym(unsigned long addr, kdb_symtab_t *symtab)
symtab->mod_name = "kernel";
if (KDB_DEBUG(AR))
kdb_printf("kdbnearsym: returns %d symtab->sym_start=0x%lx, "
- "symtab->mod_name=%p, symtab->sym_name=%p (%s)\n", ret,
+ "symtab->mod_name=%px, symtab->sym_name=%px (%s)\n", ret,
symtab->sym_start, symtab->mod_name, symtab->sym_name,
symtab->sym_name);
@@ -887,13 +887,13 @@ void debug_kusage(void)
__func__, dah_first);
if (dah_first) {
h_used = (struct debug_alloc_header *)debug_alloc_pool;
- kdb_printf("%s: h_used %p size %d\n", __func__, h_used,
+ kdb_printf("%s: h_used %px size %d\n", __func__, h_used,
h_used->size);
}
do {
h_used = (struct debug_alloc_header *)
((char *)h_free + dah_overhead + h_free->size);
- kdb_printf("%s: h_used %p size %d caller %p\n",
+ kdb_printf("%s: h_used %px size %d caller %px\n",
__func__, h_used, h_used->size, h_used->caller);
h_free = (struct debug_alloc_header *)
(debug_alloc_pool + h_free->next);
@@ -902,7 +902,7 @@ void debug_kusage(void)
((char *)h_free + dah_overhead + h_free->size);
if ((char *)h_used - debug_alloc_pool !=
sizeof(debug_alloc_pool_aligned))
- kdb_printf("%s: h_used %p size %d caller %p\n",
+ kdb_printf("%s: h_used %px size %d caller %px\n",
__func__, h_used, h_used->size, h_used->caller);
out:
spin_unlock(&dap_lock);
--
2.13.3
^ permalink raw reply related
* [PATCH v2] kdb: use correct pointer when 'btc' calls 'btt'
From: Christophe Leroy @ 2018-09-27 17:17 UTC (permalink / raw)
To: Jason Wessel, Daniel Thompson
Cc: linuxppc-dev, linux-kernel, stable, kgdb-bugreport
On a powerpc 8xx, 'btc' fails as follows:
Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
kdb> btc
btc: cpu status: Currently on cpu 0
Available cpus: 0
kdb_getarea: Bad address 0x0
when booting the kernel with 'debug_boot_weak_hash', it fails as well
Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
kdb> btc
btc: cpu status: Currently on cpu 0
Available cpus: 0
kdb_getarea: Bad address 0xba99ad80
On other platforms, Oopses have been observed too, see
https://github.com/linuxppc/linux/issues/139
This is due to btc calling 'btt' with %p pointer as an argument.
This patch replaces %p by %px to get the real pointer value as
expected by 'btt'
Fixes: ad67b74d2469 ("printk: hash addresses printed with %p")
Cc: <stable@vger.kernel.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v2: Added fixes: and Daniel's reviewed-by: in commit's log. No code change.
kernel/debug/kdb/kdb_bt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c
index 6ad4a9fcbd6f..7921ae4fca8d 100644
--- a/kernel/debug/kdb/kdb_bt.c
+++ b/kernel/debug/kdb/kdb_bt.c
@@ -179,14 +179,14 @@ kdb_bt(int argc, const char **argv)
kdb_printf("no process for cpu %ld\n", cpu);
return 0;
}
- sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
+ sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
kdb_parse(buf);
return 0;
}
kdb_printf("btc: cpu status: ");
kdb_parse("cpu\n");
for_each_online_cpu(cpu) {
- sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
+ sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
kdb_parse(buf);
touch_nmi_watchdog();
}
--
2.13.3
^ permalink raw reply related
* [PATCH] kdb: print real address of pointers instead of hashed addresses
From: Christophe Leroy @ 2018-09-27 17:17 UTC (permalink / raw)
To: Jason Wessel, Daniel Thompson
Cc: linuxppc-dev, linux-kernel, stable, kgdb-bugreport
Since commit ad67b74d2469 ("printk: hash addresses printed with %p"),
all pointers printed with %p are printed with hashed addresses
instead of real addresses in order to avoid leaking addresses in
dmesg and syslog. But this applies to kdb too, with is unfortunate:
Entering kdb (current=0x(ptrval), pid 329) due to Keyboard Entry
kdb> ps
15 sleeping system daemon (state M) processes suppressed,
use 'ps A' to see all.
Task Addr Pid Parent [*] cpu State Thread Command
0x(ptrval) 329 328 1 0 R 0x(ptrval) *sh
0x(ptrval) 1 0 0 0 S 0x(ptrval) init
0x(ptrval) 3 2 0 0 D 0x(ptrval) rcu_gp
0x(ptrval) 4 2 0 0 D 0x(ptrval) rcu_par_gp
0x(ptrval) 5 2 0 0 D 0x(ptrval) kworker/0:0
0x(ptrval) 6 2 0 0 D 0x(ptrval) kworker/0:0H
0x(ptrval) 7 2 0 0 D 0x(ptrval) kworker/u2:0
0x(ptrval) 8 2 0 0 D 0x(ptrval) mm_percpu_wq
0x(ptrval) 10 2 0 0 D 0x(ptrval) rcu_preempt
The whole purpose of kdb is to debug, and for debugging real addresses
need to be known. In addition, data displayed by kdb doesn't go into
dmesg.
This patch replaces all %p by %px in kdb in order to display real
addresses.
Fixes: ad67b74d2469 ("printk: hash addresses printed with %p")
Cc: <stable@vger.kernel.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
kernel/debug/kdb/kdb_main.c | 14 +++++++-------
kernel/debug/kdb/kdb_support.c | 12 ++++++------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 2ddfce8f1e8f..f338d23b112b 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1192,7 +1192,7 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
if (reason == KDB_REASON_DEBUG) {
/* special case below */
} else {
- kdb_printf("\nEntering kdb (current=0x%p, pid %d) ",
+ kdb_printf("\nEntering kdb (current=0x%px, pid %d) ",
kdb_current, kdb_current ? kdb_current->pid : 0);
#if defined(CONFIG_SMP)
kdb_printf("on processor %d ", raw_smp_processor_id());
@@ -1208,7 +1208,7 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
*/
switch (db_result) {
case KDB_DB_BPT:
- kdb_printf("\nEntering kdb (0x%p, pid %d) ",
+ kdb_printf("\nEntering kdb (0x%px, pid %d) ",
kdb_current, kdb_current->pid);
#if defined(CONFIG_SMP)
kdb_printf("on processor %d ", raw_smp_processor_id());
@@ -2048,7 +2048,7 @@ static int kdb_lsmod(int argc, const char **argv)
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- kdb_printf("%-20s%8u 0x%p ", mod->name,
+ kdb_printf("%-20s%8u 0x%px ", mod->name,
mod->core_layout.size, (void *)mod);
#ifdef CONFIG_MODULE_UNLOAD
kdb_printf("%4d ", module_refcount(mod));
@@ -2059,7 +2059,7 @@ static int kdb_lsmod(int argc, const char **argv)
kdb_printf(" (Loading)");
else
kdb_printf(" (Live)");
- kdb_printf(" 0x%p", mod->core_layout.base);
+ kdb_printf(" 0x%px", mod->core_layout.base);
#ifdef CONFIG_MODULE_UNLOAD
{
@@ -2341,7 +2341,7 @@ void kdb_ps1(const struct task_struct *p)
return;
cpu = kdb_process_cpu(p);
- kdb_printf("0x%p %8d %8d %d %4d %c 0x%p %c%s\n",
+ kdb_printf("0x%px %8d %8d %d %4d %c 0x%px %c%s\n",
(void *)p, p->pid, p->parent->pid,
kdb_task_has_cpu(p), kdb_process_cpu(p),
kdb_task_state_char(p),
@@ -2354,7 +2354,7 @@ void kdb_ps1(const struct task_struct *p)
} else {
if (KDB_TSK(cpu) != p)
kdb_printf(" Error: does not match running "
- "process table (0x%p)\n", KDB_TSK(cpu));
+ "process table (0x%px)\n", KDB_TSK(cpu));
}
}
}
@@ -2692,7 +2692,7 @@ int kdb_register_flags(char *cmd,
for_each_kdbcmd(kp, i) {
if (kp->cmd_name && (strcmp(kp->cmd_name, cmd) == 0)) {
kdb_printf("Duplicate kdb command registered: "
- "%s, func %p help %s\n", cmd, func, help);
+ "%s, func %px help %s\n", cmd, func, help);
return 1;
}
}
diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c
index 990b3cc526c8..987eb73284d2 100644
--- a/kernel/debug/kdb/kdb_support.c
+++ b/kernel/debug/kdb/kdb_support.c
@@ -40,7 +40,7 @@
int kdbgetsymval(const char *symname, kdb_symtab_t *symtab)
{
if (KDB_DEBUG(AR))
- kdb_printf("kdbgetsymval: symname=%s, symtab=%p\n", symname,
+ kdb_printf("kdbgetsymval: symname=%s, symtab=%px\n", symname,
symtab);
memset(symtab, 0, sizeof(*symtab));
symtab->sym_start = kallsyms_lookup_name(symname);
@@ -88,7 +88,7 @@ int kdbnearsym(unsigned long addr, kdb_symtab_t *symtab)
char *knt1 = NULL;
if (KDB_DEBUG(AR))
- kdb_printf("kdbnearsym: addr=0x%lx, symtab=%p\n", addr, symtab);
+ kdb_printf("kdbnearsym: addr=0x%lx, symtab=%px\n", addr, symtab);
memset(symtab, 0, sizeof(*symtab));
if (addr < 4096)
@@ -149,7 +149,7 @@ int kdbnearsym(unsigned long addr, kdb_symtab_t *symtab)
symtab->mod_name = "kernel";
if (KDB_DEBUG(AR))
kdb_printf("kdbnearsym: returns %d symtab->sym_start=0x%lx, "
- "symtab->mod_name=%p, symtab->sym_name=%p (%s)\n", ret,
+ "symtab->mod_name=%px, symtab->sym_name=%px (%s)\n", ret,
symtab->sym_start, symtab->mod_name, symtab->sym_name,
symtab->sym_name);
@@ -887,13 +887,13 @@ void debug_kusage(void)
__func__, dah_first);
if (dah_first) {
h_used = (struct debug_alloc_header *)debug_alloc_pool;
- kdb_printf("%s: h_used %p size %d\n", __func__, h_used,
+ kdb_printf("%s: h_used %px size %d\n", __func__, h_used,
h_used->size);
}
do {
h_used = (struct debug_alloc_header *)
((char *)h_free + dah_overhead + h_free->size);
- kdb_printf("%s: h_used %p size %d caller %p\n",
+ kdb_printf("%s: h_used %px size %d caller %px\n",
__func__, h_used, h_used->size, h_used->caller);
h_free = (struct debug_alloc_header *)
(debug_alloc_pool + h_free->next);
@@ -902,7 +902,7 @@ void debug_kusage(void)
((char *)h_free + dah_overhead + h_free->size);
if ((char *)h_used - debug_alloc_pool !=
sizeof(debug_alloc_pool_aligned))
- kdb_printf("%s: h_used %p size %d caller %p\n",
+ kdb_printf("%s: h_used %px size %d caller %px\n",
__func__, h_used, h_used->size, h_used->caller);
out:
spin_unlock(&dap_lock);
--
2.13.3
^ permalink raw reply related
* Re: [PATCH] powerpc/rtas: Fix a potential race between CPU-Offline & Migration
From: Nathan Fontenot @ 2018-09-27 17:31 UTC (permalink / raw)
To: Gautham R. Shenoy, Michael Ellerman, Tyrel Datwyler; +Cc: linuxppc-dev
In-Reply-To: <1538067112-11493-1-git-send-email-ego@linux.vnet.ibm.com>
On 09/27/2018 11:51 AM, Gautham R. Shenoy wrote:
> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>
> Live Partition Migrations require all the present CPUs to execute the
> H_JOIN call, and hence rtas_ibm_suspend_me() onlines any offline CPUs
> before initiating the migration for this purpose.
>
> The commit 85a88cabad57
> ("powerpc/pseries: Disable CPU hotplug across migrations")
> disables any CPU-hotplug operations once all the offline CPUs are
> brought online to prevent any further state change. Once the
> CPU-Hotplug operation is disabled, the code assumes that all the CPUs
> are online.
>
> However, there is a minor window in rtas_ibm_suspend_me() between
> onlining the offline CPUs and disabling CPU-Hotplug when a concurrent
> CPU-offline operations initiated by the userspace can succeed thereby
> nullifying the the aformentioned assumption. In this unlikely case
> these offlined CPUs will not call H_JOIN, resulting in a system hang.
>
> Fix this by verifying that all the present CPUs are actually online
> after CPU-Hotplug has been disabled, failing which we return from
> rtas_ibm_suspend_me() with -EBUSY.
Would we also want to havr the ability to re-try onlining all of the cpus
before failing the migration?
This would involve a bigger code change as the current code to online all
CPUs would work in its current form.
-Nathan
>
> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Cc: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/rtas.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 2c7ed31..27f6fd3 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -982,6 +982,16 @@ int rtas_ibm_suspend_me(u64 handle)
> }
>
> cpu_hotplug_disable();
> +
> + /* Check if we raced with a CPU-Offline Operation */
> + if (unlikely(!cpumask_equal(cpu_present_mask, cpu_online_mask))) {
> + pr_err("%s: Raced against a concurrent CPU-Offline\n",
> + __func__);
> + atomic_set(&data.error, -EBUSY);
> + cpu_hotplug_enable();
> + goto out;
> + }
> +
> stop_topology_update();
>
> /* Call function on all CPUs. One of us will make the
>
^ permalink raw reply
* Re: [PATCH] powerpc/rtas: Fix a potential race between CPU-Offline & Migration
From: Nathan Fontenot @ 2018-09-27 17:31 UTC (permalink / raw)
To: Gautham R. Shenoy, Michael Ellerman, Tyrel Datwyler; +Cc: linuxppc-dev
In-Reply-To: <1538067112-11493-1-git-send-email-ego@linux.vnet.ibm.com>
On 09/27/2018 11:51 AM, Gautham R. Shenoy wrote:
> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>
> Live Partition Migrations require all the present CPUs to execute the
> H_JOIN call, and hence rtas_ibm_suspend_me() onlines any offline CPUs
> before initiating the migration for this purpose.
>
> The commit 85a88cabad57
> ("powerpc/pseries: Disable CPU hotplug across migrations")
> disables any CPU-hotplug operations once all the offline CPUs are
> brought online to prevent any further state change. Once the
> CPU-Hotplug operation is disabled, the code assumes that all the CPUs
> are online.
>
> However, there is a minor window in rtas_ibm_suspend_me() between
> onlining the offline CPUs and disabling CPU-Hotplug when a concurrent
> CPU-offline operations initiated by the userspace can succeed thereby
> nullifying the the aformentioned assumption. In this unlikely case
> these offlined CPUs will not call H_JOIN, resulting in a system hang.
>
> Fix this by verifying that all the present CPUs are actually online
> after CPU-Hotplug has been disabled, failing which we return from
> rtas_ibm_suspend_me() with -EBUSY.
Would we also want to havr the ability to re-try onlining all of the cpus
before failing the migration?
This would involve a bigger code change as the current code to online all
CPUs would work in its current form.
-Nathan
>
> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Cc: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/rtas.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 2c7ed31..27f6fd3 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -982,6 +982,16 @@ int rtas_ibm_suspend_me(u64 handle)
> }
>
> cpu_hotplug_disable();
> +
> + /* Check if we raced with a CPU-Offline Operation */
> + if (unlikely(!cpumask_equal(cpu_present_mask, cpu_online_mask))) {
> + pr_err("%s: Raced against a concurrent CPU-Offline\n",
> + __func__);
> + atomic_set(&data.error, -EBUSY);
> + cpu_hotplug_enable();
> + goto out;
> + }
> +
> stop_topology_update();
>
> /* Call function on all CPUs. One of us will make the
>
^ permalink raw reply
* Re: [PATCH 0/8] add generic builtin command line
From: Daniel Walker @ 2018-09-27 18:08 UTC (permalink / raw)
To: Ard Biesheuvel, Maksym Kokhan
Cc: Andrew Morton, Rob Herring, Frank Rowand, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin,
the arch/x86 maintainers, Russell King, Catalin Marinas,
Will Deacon, Ralf Baechle, Paul Burton, James Hogan,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Daniel Walker, Andrii Bordunov, Ruslan Bilovol,
Linux Kernel Mailing List, Devicetree List, linux-efi, linux-mips,
linuxppc-dev
In-Reply-To: <CAKv+Gu-AxtOO04iwPSri12tkb9NRugXV9E2LGrfJT-LJjf4_ow@mail.gmail.com>
On 09/27/2018 10:05 AM, Ard Biesheuvel wrote:
> On 27 September 2018 at 18:55, Maksym Kokhan
> <maksym.kokhan@globallogic.com> wrote:
>> There were series of patches [1] for 4.3.0-rc3, that allowed
>> architectures to use a generic builtin command line. I have rebased
>> these patches on kernel 4.19.0-rc4.
>>
>
> Could you please elaborate on the purpose of this series? Is it simply
> to align between architectures? Does it solve an actual problem?
1) It removed a lot of code duplication between architecture
2) At Cisco we have issues where our bootloaders having default boot
arguments. Some platforms we can't update the boot loader and it's
helpful to be able to have boot arguments which are prepended to the
bootloader arguments , and some parameters which are appended. These
changes allow that.
Daniel
^ permalink raw reply
* Re: [PATCH 0/8] add generic builtin command line
From: Daniel Walker @ 2018-09-27 18:08 UTC (permalink / raw)
To: Ard Biesheuvel, Maksym Kokhan
Cc: linux-mips, linux-efi, Ruslan Bilovol, Will Deacon,
Paul Mackerras, H. Peter Anvin, Daniel Walker, Frank Rowand,
the arch/x86 maintainers, Russell King, Ingo Molnar,
Catalin Marinas, James Hogan, Devicetree List, Rob Herring,
Borislav Petkov, Thomas Gleixner, Andrii Bordunov,
Linux Kernel Mailing List, Ralf Baechle, Paul Burton,
Andrew Morton, linuxppc-dev
In-Reply-To: <CAKv+Gu-AxtOO04iwPSri12tkb9NRugXV9E2LGrfJT-LJjf4_ow@mail.gmail.com>
On 09/27/2018 10:05 AM, Ard Biesheuvel wrote:
> On 27 September 2018 at 18:55, Maksym Kokhan
> <maksym.kokhan@globallogic.com> wrote:
>> There were series of patches [1] for 4.3.0-rc3, that allowed
>> architectures to use a generic builtin command line. I have rebased
>> these patches on kernel 4.19.0-rc4.
>>
>
> Could you please elaborate on the purpose of this series? Is it simply
> to align between architectures? Does it solve an actual problem?
1) It removed a lot of code duplication between architecture
2) At Cisco we have issues where our bootloaders having default boot
arguments. Some platforms we can't update the boot loader and it's
helpful to be able to have boot arguments which are prepended to the
bootloader arguments , and some parameters which are appended. These
changes allow that.
Daniel
^ permalink raw reply
* Re: [PATCH 1/2] soc: fsl: qbman: qman_portal: defer probing when qman is not available
From: Li Yang @ 2018-09-27 19:24 UTC (permalink / raw)
To: alexandre.belloni, Laurentiu Tudor
Cc: Olof Johansson, Roy Pledge, linuxppc-dev,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, lkml
In-Reply-To: <CADRPPNQwWgU6OcE7CjxyrgSXjUjwYe4VY0gQD8jx6D+eT_DWrw@mail.gmail.com>
On Wed, Sep 26, 2018 at 1:15 PM Li Yang <leoyang.li@nxp.com> wrote:
>
> On Wed, Sep 26, 2018 at 4:28 AM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> >
> > On 25/09/2018 21:45:56+0200, Olof Johansson wrote:
> > > Hi,
> > >
> > >
> > > On Thu, Aug 23, 2018 at 11:36 PM Alexandre Belloni
> > > <alexandre.belloni@bootlin.com> wrote:
> > > >
> > > > If the qman driver (qman_ccsr) doesn't probe or fail to probe before
> > > > qman_portal, qm_ccsr_start will be either NULL or a stale pointer to an
> > > > unmapped page.
> > > >
> > > > This leads to a crash when probing qman_portal as the init_pcfg function
> > > > calls qman_liodn_fixup that tries to read qman registers.
> > > >
> > > > Assume that qman didn't probe when the pool mask is 0.
> > > >
> > > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > > > ---
> > > > drivers/soc/fsl/qbman/qman_portal.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/soc/fsl/qbman/qman_portal.c b/drivers/soc/fsl/qbman/qman_portal.c
> > > > index a120002b630e..4fc80d2c8feb 100644
> > > > --- a/drivers/soc/fsl/qbman/qman_portal.c
> > > > +++ b/drivers/soc/fsl/qbman/qman_portal.c
> > > > @@ -277,6 +277,8 @@ static int qman_portal_probe(struct platform_device *pdev)
> > > > }
> > > >
> > > > pcfg->pools = qm_get_pools_sdqcr();
> > > > + if (pcfg->pools == 0)
> > > > + return -EPROBE_DEFER;
> > >
> > > This is quite late in the probe, after a bunch of resources have been claimed.
> > >
> > > Note that the ioremaps above this are doing unwinds, and you'll end up
> > > doing duplicate ioremaps if you come in and probe again.
> > >
> > > You should probably unwind those allocations, or move them to devm_*
> > > or do this check earlier in the function.
> > >
> >
> > The actual chance of having that happen is quite small (this was coming
> > from a non working DT) and I mainly wanted to avoid a crash so the
> > platform could still boot. I would think moving to devm_ would be the
> > right thing to do.
>
> Even if it is not failing with the upstreamed device trees, it is
> still good to harden the driver for possible issues. Moving to devm_
> is definitely a right thing to do. But I also think checking if the
> qman is already probed should be the first thing to do before starting
> to allocate resources and etc and rolling back later. Probably we can
> move the qm_get_pools_sdqcr() to the begining of the probe to
> determine if qman is probed as it doesn't seem to depend on any of the
> setups done right now.
I just find out Laurentiu also included the following patches in his
SMMU patch series (although not neccessarily related to SMMU) which
also fix the same problem. I think they are more straightforward and
can deal with the case that qman failed to probe. So we can take
these to fix this problem instead in 4.19.
https://patchwork.kernel.org/patch/10616021/
https://patchwork.kernel.org/patch/10616019/
https://patchwork.kernel.org/patch/10615971/
Regards,
Leo
^ permalink raw reply
* Re: [PATCH 1/2] soc: fsl: qbman: qman_portal: defer probing when qman is not available
From: Li Yang @ 2018-09-27 19:24 UTC (permalink / raw)
To: alexandre.belloni, Laurentiu Tudor
Cc: Olof Johansson, Roy Pledge, linuxppc-dev, lkml,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CADRPPNQwWgU6OcE7CjxyrgSXjUjwYe4VY0gQD8jx6D+eT_DWrw@mail.gmail.com>
On Wed, Sep 26, 2018 at 1:15 PM Li Yang <leoyang.li@nxp.com> wrote:
>
> On Wed, Sep 26, 2018 at 4:28 AM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> >
> > On 25/09/2018 21:45:56+0200, Olof Johansson wrote:
> > > Hi,
> > >
> > >
> > > On Thu, Aug 23, 2018 at 11:36 PM Alexandre Belloni
> > > <alexandre.belloni@bootlin.com> wrote:
> > > >
> > > > If the qman driver (qman_ccsr) doesn't probe or fail to probe before
> > > > qman_portal, qm_ccsr_start will be either NULL or a stale pointer to an
> > > > unmapped page.
> > > >
> > > > This leads to a crash when probing qman_portal as the init_pcfg function
> > > > calls qman_liodn_fixup that tries to read qman registers.
> > > >
> > > > Assume that qman didn't probe when the pool mask is 0.
> > > >
> > > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > > > ---
> > > > drivers/soc/fsl/qbman/qman_portal.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/soc/fsl/qbman/qman_portal.c b/drivers/soc/fsl/qbman/qman_portal.c
> > > > index a120002b630e..4fc80d2c8feb 100644
> > > > --- a/drivers/soc/fsl/qbman/qman_portal.c
> > > > +++ b/drivers/soc/fsl/qbman/qman_portal.c
> > > > @@ -277,6 +277,8 @@ static int qman_portal_probe(struct platform_device *pdev)
> > > > }
> > > >
> > > > pcfg->pools = qm_get_pools_sdqcr();
> > > > + if (pcfg->pools == 0)
> > > > + return -EPROBE_DEFER;
> > >
> > > This is quite late in the probe, after a bunch of resources have been claimed.
> > >
> > > Note that the ioremaps above this are doing unwinds, and you'll end up
> > > doing duplicate ioremaps if you come in and probe again.
> > >
> > > You should probably unwind those allocations, or move them to devm_*
> > > or do this check earlier in the function.
> > >
> >
> > The actual chance of having that happen is quite small (this was coming
> > from a non working DT) and I mainly wanted to avoid a crash so the
> > platform could still boot. I would think moving to devm_ would be the
> > right thing to do.
>
> Even if it is not failing with the upstreamed device trees, it is
> still good to harden the driver for possible issues. Moving to devm_
> is definitely a right thing to do. But I also think checking if the
> qman is already probed should be the first thing to do before starting
> to allocate resources and etc and rolling back later. Probably we can
> move the qm_get_pools_sdqcr() to the begining of the probe to
> determine if qman is probed as it doesn't seem to depend on any of the
> setups done right now.
I just find out Laurentiu also included the following patches in his
SMMU patch series (although not neccessarily related to SMMU) which
also fix the same problem. I think they are more straightforward and
can deal with the case that qman failed to probe. So we can take
these to fix this problem instead in 4.19.
https://patchwork.kernel.org/patch/10616021/
https://patchwork.kernel.org/patch/10616019/
https://patchwork.kernel.org/patch/10615971/
Regards,
Leo
^ permalink raw reply
* Re: [PATCH v3 1/6] dt-bindings: arm64: add compatible for LX2160A
From: Rob Herring @ 2018-09-27 19:32 UTC (permalink / raw)
To: Vabhav Sharma
Cc: sudeep.holla, oss, linux-kernel, devicetree, robh+dt,
mark.rutland, linuxppc-dev, linux-arm-kernel, mturquette, sboyd,
rjw, viresh.kumar, linux-clk, linux-pm, linux-kernel-owner,
catalin.marinas, will.deacon, gregkh, arnd, kstewart,
yamada.masahiro, udit.kumar, linux, Vabhav Sharma, V.Sethi
In-Reply-To: <1537747741-6245-2-git-send-email-vabhav.sharma@nxp.com>
On Mon, 24 Sep 2018 05:38:56 +0530, Vabhav Sharma wrote:
> Add compatible for LX2160A SoC,QDS and RDB board
>
> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
> ---
> Documentation/devicetree/bindings/arm/fsl.txt | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 1/6] dt-bindings: arm64: add compatible for LX2160A
From: Rob Herring @ 2018-09-27 19:32 UTC (permalink / raw)
To: Vabhav Sharma
Cc: mark.rutland, kstewart, linux-kernel-owner, catalin.marinas,
mturquette, will.deacon, yamada.masahiro, linux-clk, udit.kumar,
linux, viresh.kumar, devicetree, arnd, linux-pm, oss, robh+dt,
V.Sethi, Vabhav Sharma, linux-arm-kernel, sboyd, gregkh, rjw,
linux-kernel, sudeep.holla, linuxppc-dev
In-Reply-To: <1537747741-6245-2-git-send-email-vabhav.sharma@nxp.com>
On Mon, 24 Sep 2018 05:38:56 +0530, Vabhav Sharma wrote:
> Add compatible for LX2160A SoC,QDS and RDB board
>
> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
> ---
> Documentation/devicetree/bindings/arm/fsl.txt | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [RFC PATCH 00/11] New TM Model
From: Breno Leitao @ 2018-09-27 20:13 UTC (permalink / raw)
To: Michael Neuling, linuxppc-dev; +Cc: paulus, gromero, mpe, ldufour, Cyril Bur
In-Reply-To: <b1ed898835f2e0805088d215e8afb193a7c4768c.camel@neuling.org>
Hi Mikey,
First of all, thanks for you detailed review. I really appreciate your
comments here.
On 09/17/2018 02:25 AM, Michael Neuling wrote:
> On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
>> This patchset for the hardware transactional memory (TM) subsystem aims to
>> avoid spending a lot of time on TM suspended mode in kernel space. It
>> basically
>> changes where the reclaim/recheckpoint will be executed.
>>
>> Once a CPU enters in transactional state it uses a footprint area to track
>> down the load/stores performed in transaction so it can be verified later
>> to decide if a conflict happened due to some change done in that state. If
>> a transaction is active in userspace and there is an exception that takes
>> the CPU to the kernel space the CPU moves the transaction to suspended
>> state but does not discard the footprint area.
>
> In this description, you should differente between memory and register
> (GPR/VSX/SPR) footprints.
Right, reading the ISA, I understand that footprint is a term for memory only
and it represents the modified memory that was stored during a transactional
state (that after tbegin). For registers, the ISA talks about checkpointed
registers, which is the register state *before* a transaction starts.
I.e, for register it is the previous state, and for memory, it is the
current/live state.
That said, if the transactional is aborted, the memory footprint is discarded
and the checkpointed registers replaces the live registers.
> In suspend, the CPU can disregard the memory footprint at any point, but it has> to keep the register footprint.
Yes!
Anyway, I was just trying to describe how the hardware works, it is not
related to the kernel at the paragraph above, but I will make sure I will
re-write it better.
>> This patchset aims to reclaim the checkpointed footprint as soon as the
>> kernel is invoked, in the beginning of the exception handlers, thus freeing
>> room to other CPUs enter in suspended mode, avoiding too many CPUs in
>> suspended
>> state that can cause the CPUs to stall. The same mechanism is done on kernel
>> exit, doing a recheckpoint as late as possible (which will reload the
>> checkpointed state into CPU's room) at the exception return.
>
> OK, but we are still potentially in suspend in userspace, so that doesn't help
> us on the lockup issue.
>
> We need fake suspend in userspace to prevent lockups.
Correct. I will make sure I document it. This patchset is the very first step
to start creating a work around for the hardware limitations.
>> The way to achieve this goal is creating a macro (TM_KERNEL_ENTRY) which
>> will check if userspace was in an active transaction just after getting
>> into kernel and reclaiming if that's the case. Thus all exception handlers
>> will call this macro as soon as possible.
>>
>> All exceptions should reclaim (if necessary) at this stage and only
>> recheckpoint if the task is tagged as TIF_RESTORE_TM (i.e. was in
>> transactional state before being interrupted), which will be done at
>> ret_from_except_lite().
>>
>> Ideally all reclaims will happen at the exception entrance, however during
>> the recheckpoint process another exception can hit the CPU which might
>> cause the current thread to be rescheduled, thus there is another reclaim
>> point to be considered at __switch_to().
>
> Can we do the recheckpoint() later so that it's when we have interrupts off and
> can't be rescheduled?
Yes! After thinking on it for a long time, this is definitely what should be
done. I will send a v2 with this change (and others being discussed here)
Thank you,
Breno
^ permalink raw reply
* Re: [RFC PATCH 00/11] New TM Model
From: Breno Leitao @ 2018-09-27 20:13 UTC (permalink / raw)
To: Michael Neuling, linuxppc-dev; +Cc: ldufour, Cyril Bur, gromero
In-Reply-To: <b1ed898835f2e0805088d215e8afb193a7c4768c.camel@neuling.org>
Hi Mikey,
First of all, thanks for you detailed review. I really appreciate your
comments here.
On 09/17/2018 02:25 AM, Michael Neuling wrote:
> On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
>> This patchset for the hardware transactional memory (TM) subsystem aims to
>> avoid spending a lot of time on TM suspended mode in kernel space. It
>> basically
>> changes where the reclaim/recheckpoint will be executed.
>>
>> Once a CPU enters in transactional state it uses a footprint area to track
>> down the load/stores performed in transaction so it can be verified later
>> to decide if a conflict happened due to some change done in that state. If
>> a transaction is active in userspace and there is an exception that takes
>> the CPU to the kernel space the CPU moves the transaction to suspended
>> state but does not discard the footprint area.
>
> In this description, you should differente between memory and register
> (GPR/VSX/SPR) footprints.
Right, reading the ISA, I understand that footprint is a term for memory only
and it represents the modified memory that was stored during a transactional
state (that after tbegin). For registers, the ISA talks about checkpointed
registers, which is the register state *before* a transaction starts.
I.e, for register it is the previous state, and for memory, it is the
current/live state.
That said, if the transactional is aborted, the memory footprint is discarded
and the checkpointed registers replaces the live registers.
> In suspend, the CPU can disregard the memory footprint at any point, but it has> to keep the register footprint.
Yes!
Anyway, I was just trying to describe how the hardware works, it is not
related to the kernel at the paragraph above, but I will make sure I will
re-write it better.
>> This patchset aims to reclaim the checkpointed footprint as soon as the
>> kernel is invoked, in the beginning of the exception handlers, thus freeing
>> room to other CPUs enter in suspended mode, avoiding too many CPUs in
>> suspended
>> state that can cause the CPUs to stall. The same mechanism is done on kernel
>> exit, doing a recheckpoint as late as possible (which will reload the
>> checkpointed state into CPU's room) at the exception return.
>
> OK, but we are still potentially in suspend in userspace, so that doesn't help
> us on the lockup issue.
>
> We need fake suspend in userspace to prevent lockups.
Correct. I will make sure I document it. This patchset is the very first step
to start creating a work around for the hardware limitations.
>> The way to achieve this goal is creating a macro (TM_KERNEL_ENTRY) which
>> will check if userspace was in an active transaction just after getting
>> into kernel and reclaiming if that's the case. Thus all exception handlers
>> will call this macro as soon as possible.
>>
>> All exceptions should reclaim (if necessary) at this stage and only
>> recheckpoint if the task is tagged as TIF_RESTORE_TM (i.e. was in
>> transactional state before being interrupted), which will be done at
>> ret_from_except_lite().
>>
>> Ideally all reclaims will happen at the exception entrance, however during
>> the recheckpoint process another exception can hit the CPU which might
>> cause the current thread to be rescheduled, thus there is another reclaim
>> point to be considered at __switch_to().
>
> Can we do the recheckpoint() later so that it's when we have interrupts off and
> can't be rescheduled?
Yes! After thinking on it for a long time, this is definitely what should be
done. I will send a v2 with this change (and others being discussed here)
Thank you,
Breno
^ permalink raw reply
* Re: [RFC PATCH 01/11] powerpc/tm: Reclaim transaction on kernel entry
From: Breno Leitao @ 2018-09-27 20:28 UTC (permalink / raw)
To: Michael Neuling, linuxppc-dev; +Cc: paulus, gromero, mpe, ldufour
In-Reply-To: <5013d84a59185cd7a1b941e6cc145a1fbfe52f48.camel@neuling.org>
Hi Mikey,
On 09/17/2018 10:31 PM, Michael Neuling wrote:
> On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
>> This patch creates a macro that will be invoked on all entrance to the
>> kernel, so, in kernel space the transaction will be completely reclaimed
>> and not suspended anymore.
>
> There are still some calls to tm_reclaim_current() in process.c. Should these
> probably go now, right?
Yes, we shouldn't not call tm_reclaim_current() from anywhere else other than
TM_KERNEL_ENTRY anymore.
That said, I think we can still have some references for tm_reclaim_current()
in some specific checkpoints. Something as:
if (WARN_ON(MSR_TM_SUSPENDED(mfmsr())))
tm_reclaim_current(0);
I initially wrote something as BUG_ON(MSR_TM_SUSPENDED(mfmsr()) but
scripts/checkpatch.pl suggested me writing a WARN_ON() and create a recovery
pass, which seems fair.
Anyway, if you think it is not a good strategy, I can get rid of them at v2.
Thank you!
^ permalink raw reply
* Re: [RFC PATCH 01/11] powerpc/tm: Reclaim transaction on kernel entry
From: Breno Leitao @ 2018-09-27 20:28 UTC (permalink / raw)
To: Michael Neuling, linuxppc-dev; +Cc: ldufour, gromero
In-Reply-To: <5013d84a59185cd7a1b941e6cc145a1fbfe52f48.camel@neuling.org>
Hi Mikey,
On 09/17/2018 10:31 PM, Michael Neuling wrote:
> On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
>> This patch creates a macro that will be invoked on all entrance to the
>> kernel, so, in kernel space the transaction will be completely reclaimed
>> and not suspended anymore.
>
> There are still some calls to tm_reclaim_current() in process.c. Should these
> probably go now, right?
Yes, we shouldn't not call tm_reclaim_current() from anywhere else other than
TM_KERNEL_ENTRY anymore.
That said, I think we can still have some references for tm_reclaim_current()
in some specific checkpoints. Something as:
if (WARN_ON(MSR_TM_SUSPENDED(mfmsr())))
tm_reclaim_current(0);
I initially wrote something as BUG_ON(MSR_TM_SUSPENDED(mfmsr()) but
scripts/checkpatch.pl suggested me writing a WARN_ON() and create a recovery
pass, which seems fair.
Anyway, if you think it is not a good strategy, I can get rid of them at v2.
Thank you!
^ permalink raw reply
* Re: [RFC PATCH 06/11] powerpc/tm: Refactor the __switch_to_tm code
From: Breno Leitao @ 2018-09-27 20:48 UTC (permalink / raw)
To: Michael Neuling, linuxppc-dev; +Cc: ldufour, gromero
In-Reply-To: <d06021833414dcfa5c9e04b4d83dfc475d1f1cbd.camel@neuling.org>
hi Mikey
On 09/18/2018 01:04 AM, Michael Neuling wrote:
>> On top of that, both tm_reclaim_task() and tm_recheckpoint_new_task()
>> functions are not used anymore, removing them.
>
> What about tm_reclaim_current(). This is being used in places like signals
> which I would have thought we could avoid with this series
In fact tm_reclaim_current() is the default function to reclaim. It is the
function that is being called by TM_KERNEL_ENTRY, other than that, it should
never be called on the sane path.
>> + * If we got here with an active transaction, then, it was
>> + * aborted by TM_KERNEL_ENTRY and the fix the failure case
>> + * needs to be fixed, so, indepedently how we arrived here, the
>> + * new TM abort case will be TM_CAUSE_RESCHED now.
>
> What does "fix the failure case needs to be fixed" mean?
>
> also s/indepedently/independently/
In fact, I rewrote this paragraph. I try to say that the "TEXASR Failure
code" needs to be updated/fixed, but it became that messy and crazy
paragraph. :-(
>
>> + */
>> + if (MSR_TM_ACTIVE(prev->thread.regs->msr)) {
>> + /*
>> + * If there was an IRQ during trecheckpoint, it will
>> + * cause an IRQ to be replayed. This replayed IRQ can
>> + * invoke SCHEDULE_USER, thus, we arrive here with a TM
>> + * active transaction.
>
> I don't think this can happen. trecheckpoint (and treclaim) are called with IRQs
> hard off (since they change r1).
There will be an IRQ check and replay later. An IRQ being replayed can cause
a process to be reschedule and arrive here after a trecheckpoint.
> I think something else is going on here. I think this code and comment needs to
> go but I assume it's here because you are seeing something.
Right, and it was my major concern about this whole review.
The tm_recheckpoint() was being called with IRQ hard off, as you said, but
the IRQ is being re-enabled later, after "trecheckpoint", when it calls
local_irq_restore().
Since the IRQ was re-enabled, there is a mechanism to check for IRQs that
were pending and execute them (after recheckpoint - hence with MSR[TS]
active). Some of these pending IRQ might cause a task reschedule, bringing us
to __switch_to with MSR[TS] active.
I also checked that there were cases where a pending IRQ was waiting to be
replayed even before tm_recheckpoint() is called. I suspect we were reaching
tm_recheckpoint soft-disabled.
On the v2 patchset, I am following your suggestion, which is calling
restore_tm_state() much later (at the beginning of fast_exception_return),
after the _TIF_USER_WORK_MASK section, and after the IRQ replay section also.
So, after this point, there is no way to rollback the exit to userspace after
trecheckpoint. Therefore, if there is a recheckpoint, a rfid will always come
directly.
In order to do so, I need to remove _TIF_RESTORE_TM as part of
_TIF_USER_WORK_MASK and handle _TIF_RESTORE_TM individually later.
This seems to solve this problem, and I can remove this reclaim in the middle
of __switch_to_tm().
Thank you
^ permalink raw reply
* [PATCH 0/8] add generic builtin command line
From: Maksym Kokhan @ 2018-09-27 16:55 UTC (permalink / raw)
To: Andrew Morton, Rob Herring, Frank Rowand, Ard Biesheuvel,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
x86, Russell King, Catalin Marinas, Will Deacon, Ralf Baechle,
Paul Burton, James Hogan, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: devicetree, linux-efi, Andrii Bordunov, linux-mips,
Ruslan Bilovol, linux-kernel, Daniel Walker, linuxppc-dev,
Daniel Walker
There were series of patches [1] for 4.3.0-rc3, that allowed
architectures to use a generic builtin command line. I have rebased
these patches on kernel 4.19.0-rc4.
Things, modified in comparison with original patches:
* There was some bug for mips, in the case when CONFIG_CMDLINE_PREPEND
and CONFIG_CMDLINE_APPEND are empty and CMDLINE_OVERRIDE is not set,
command line from bootloader was ignored, so I fixed it, modifying
patch "add generic builtin command line".
* Implemented new patch to resolve conflict with new kernel, which
modify EFI stub code. Unfortunately, I don't have capability to test
this modification on real arm board with EFI.
* Removed new realisation of mips builtin command line, which was
created after 4.3.0-rc3.
* Kernel 4.3.0-rc3 with original patches could not be compiled for
powerpc due to prom_init.c checking by prom_init_check.sh. So I added
strlcat (which is used by cmdline_add_builtin macro) to
prom_init_check.sh whitelist.
Patches have been tested in QEMU for x86, arm (little-endian), arm64
(little-endian), mips (little-endian, 32-bit) and powerpc
(big-endian, 64-bit), everything works perfectly. Also it was tested
on linux-next (next-20180924 tag) for all listed above architectures.
[1] : https://lore.kernel.org/patchwork/patch/604992/
Daniel Walker (7):
add generic builtin command line
drivers: of: ifdef out cmdline section
x86: convert to generic builtin command line
arm: convert to generic builtin command line
arm64: convert to generic builtin command line
mips: convert to generic builtin command line
powerpc: convert to generic builtin command line
Maksym Kokhan (1):
efi: modify EFI stub code for arm/arm64
arch/arm/Kconfig | 38 +-----------------
arch/arm/kernel/atags_parse.c | 14 ++-----
arch/arm/kernel/devtree.c | 2 +
arch/arm64/Kconfig | 17 +-------
arch/arm64/kernel/setup.c | 3 ++
arch/mips/Kconfig | 24 +----------
arch/mips/Kconfig.debug | 47 ----------------------
arch/mips/kernel/setup.c | 41 ++-----------------
arch/powerpc/Kconfig | 23 +----------
arch/powerpc/kernel/prom.c | 4 ++
arch/powerpc/kernel/prom_init.c | 8 ++--
arch/powerpc/kernel/prom_init_check.sh | 2 +-
arch/x86/Kconfig | 44 +--------------------
arch/x86/kernel/setup.c | 19 ++-------
drivers/firmware/efi/libstub/arm-stub.c | 10 ++---
drivers/of/fdt.c | 2 +-
include/linux/cmdline.h | 70 +++++++++++++++++++++++++++++++++
init/Kconfig | 68 ++++++++++++++++++++++++++++++++
18 files changed, 173 insertions(+), 263 deletions(-)
create mode 100644 include/linux/cmdline.h
--
2.7.4
^ permalink raw reply
* [PATCH 1/8] add generic builtin command line
From: Maksym Kokhan @ 2018-09-27 16:55 UTC (permalink / raw)
To: Andrew Morton, Rob Herring, Frank Rowand, Ard Biesheuvel,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
x86, Russell King, Catalin Marinas, Will Deacon, Ralf Baechle,
Paul Burton, James Hogan, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: devicetree, linux-efi, Andrii Bordunov, linux-mips,
Ruslan Bilovol, linux-kernel, Daniel Walker, linuxppc-dev,
Daniel Walker
In-Reply-To: <1538067309-5711-1-git-send-email-maksym.kokhan@globallogic.com>
From: Daniel Walker <danielwa@cisco.com>
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 mips and x86 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.
[maksym.kokhan@globallogic.com: fix cmdline_add_builtin() macro]
Cc: Daniel Walker <dwalker@fifo99.com>
Cc: Daniel Walker <danielwa@cisco.com>
Signed-off-by: Daniel Walker <danielwa@cisco.com>
Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
---
include/linux/cmdline.h | 70 +++++++++++++++++++++++++++++++++++++++++++++++++
init/Kconfig | 68 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 138 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 0000000..75ef278
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+
+/*
+ *
+ * Copyright (C) 2015. 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, char *src, char *tmp, unsigned long length)
+{
+ if (src != dest && src != NULL) {
+ strlcpy(dest, " ", length);
+ strlcat(dest, src, length);
+ }
+
+ strlcat(dest, " ", 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, " ", length);
+ strlcat(tmp, dest, length);
+ strlcpy(dest, tmp, length);
+ }
+}
+
+#define cmdline_add_builtin(dest, src, length) \
+{ \
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) { \
+ static char cmdline_tmp_space[length] __initdata; \
+ _cmdline_add_builtin(dest, src, cmdline_tmp_space, length); \
+ } else { \
+ _cmdline_add_builtin(dest, src, NULL, length); \
+ } \
+}
+#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(dest, src, length) { \
+ if (src != NULL) \
+ strlcpy(dest, src, length); \
+}
+#endif /* CONFIG_GENERIC_CMDLINE */
+
+
+#endif /* _LINUX_CMDLINE_H */
diff --git a/init/Kconfig b/init/Kconfig
index 1e234e2..e5aa676 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1751,6 +1751,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.)
+
+ To compile command line arguments into the kernel,
+ set this option to 'Y', then fill in the
+ 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_APPEND
+ string "Built-in kernel command string append"
+ 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, this string is appended to it 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_PREPEND
+ string "Built-in kernel command string prepend"
+ 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, this string is prepended to it 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
+ help
+ Set this option to 'Y' to have the kernel ignore the boot loader
+ command line, and use ONLY the built-in command line. In this case
+ append and prepend strings are concatenated to form the full
+ command line.
+
+ This is used to work around broken boot loaders. This should
+ be set to 'N' under normal conditions.
+endif
+
endmenu # General setup
source "arch/Kconfig"
--
2.7.4
^ permalink raw reply related
* [PATCH 8/8] powerpc: convert to generic builtin command line
From: Maksym Kokhan @ 2018-09-27 16:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: Andrii Bordunov, Ruslan Bilovol, linux-kernel, Daniel Walker,
linuxppc-dev, Daniel Walker
From: Daniel Walker <danielwa@cisco.com>
This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
option.
[maksym.kokhan@globallogic.com: add strlcat to prom_init_check.sh
whitelist]
Cc: Daniel Walker <dwalker@fifo99.com>
Cc: Daniel Walker <danielwa@cisco.com>
Signed-off-by: Daniel Walker <danielwa@cisco.com>
Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
---
arch/powerpc/Kconfig | 23 +----------------------
arch/powerpc/kernel/prom.c | 4 ++++
arch/powerpc/kernel/prom_init.c | 8 ++++----
arch/powerpc/kernel/prom_init_check.sh | 2 +-
4 files changed, 10 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a806692..f87906a 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -172,6 +172,7 @@ config PPC
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select GENERIC_TIME_VSYSCALL
+ select GENERIC_CMDLINE
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_KGDB
@@ -787,28 +788,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_BOOL
- bool "Default bootloader kernel arguments"
-
-config CMDLINE
- string "Initial kernel command string"
- depends on CMDLINE_BOOL
- default "console=ttyS0,9600 console=tty0 root=/dev/sda2"
- 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.
-
-config CMDLINE_FORCE
- bool "Always use the default kernel command string"
- depends on CMDLINE_BOOL
- 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.
-
config EXTRA_TARGETS
string "Additional default image types"
help
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index c4d7078..f8c6e63 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -34,6 +34,7 @@
#include <linux/of_fdt.h>
#include <linux/libfdt.h>
#include <linux/cpu.h>
+#include <linux/cmdline.h>
#include <asm/prom.h>
#include <asm/rtas.h>
@@ -716,6 +717,9 @@ void __init early_init_devtree(void *params)
*/
of_scan_flat_dt(early_init_dt_scan_chosen_ppc, boot_command_line);
+ /* append and prepend any arguments built into the kernel. */
+ cmdline_add_builtin(boot_command_line, NULL, COMMAND_LINE_SIZE);
+
/* Scan memory nodes and rebuild MEMBLOCKs */
of_scan_flat_dt(early_init_dt_scan_root, NULL);
of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 9b38a2e..e9214c6 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -30,6 +30,7 @@
#include <linux/delay.h>
#include <linux/initrd.h>
#include <linux/bitops.h>
+#include <linux/cmdline.h>
#include <asm/prom.h>
#include <asm/rtas.h>
#include <asm/page.h>
@@ -634,11 +635,10 @@ static void __init early_cmdline_parse(void)
p = prom_cmd_line;
if ((long)prom.chosen > 0)
l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
-#ifdef CONFIG_CMDLINE
+
if (l <= 0 || p[0] == '\0') /* dbl check */
- strlcpy(prom_cmd_line,
- CONFIG_CMDLINE, sizeof(prom_cmd_line));
-#endif /* CONFIG_CMDLINE */
+ cmdline_add_builtin(prom_cmd_line, NULL, sizeof(prom_cmd_line));
+
prom_printf("command line: %s\n", prom_cmd_line);
#ifdef CONFIG_PPC64
diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
index acb6b92..b1d7ca1 100644
--- a/arch/powerpc/kernel/prom_init_check.sh
+++ b/arch/powerpc/kernel/prom_init_check.sh
@@ -18,7 +18,7 @@
WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
_end enter_prom memcpy memset reloc_offset __secondary_hold
-__secondary_hold_acknowledge __secondary_hold_spinloop __start
+__secondary_hold_acknowledge __secondary_hold_spinloop __start strlcat
strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
reloc_got2 kernstart_addr memstart_addr linux_banner _stext
__prom_init_toc_start __prom_init_toc_end btext_setup_display TOC."
--
2.7.4
^ permalink raw reply related
* Re: [RFC PATCH 09/11] powerpc/tm: Do not restore default DSCR
From: Breno Leitao @ 2018-09-27 20:51 UTC (permalink / raw)
To: Michael Neuling, linuxppc-dev; +Cc: ldufour, gromero
In-Reply-To: <10da0fdc641b05ea288890a23aa01ad33f2d6987.camel@neuling.org>
Hi Mikey,
On 09/18/2018 02:41 AM, Michael Neuling wrote:
> On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
>> In the previous TM code, trecheckpoint was being executed in the middle of
>> an exception, thus, DSCR was being restored to default kernel DSCR value
>> after trecheckpoint was done.
>>
>> With this current patchset, trecheckpoint is executed just before getting
>> to userspace, at ret_from_except_lite, for example. Thus, we do not need to
>> set default kernel DSCR value anymore, as we are leaving kernel space. It
>> is OK to keep the checkpointed DSCR value into the live SPR, mainly because
>> the transaction is doomed and it will fail soon (after RFID),
>
> What if we are going back to a suspended transaction? It will remain live until
> userspace does a tresume
Hmm, I understand that once we get in kernel space, and call
treclaim/trecheckpoint, the transaction will be doomed and it will abort and
rollback when we leave kernel space. I.e., if we can treclaim/trecheckpointn
in kernel space, the transaction will *always* abort just after RFID, so, a
possible tresume will never be executed. Is my understanding wrong?
>
>> so,
>> continuing with the pre-checkpointed DSCR value is what seems correct.
>
> Reading this description suggests this patch isn't really needed. Right?
Maybe the description is not clear, but I understand this patch is required,
otherwise we will leave userspace with a default DSCR value.
By the way, do you know if there is a change in DSCR inside a transaction,
will it be reverted if the transaction is aborted?
Thank you
^ permalink raw reply
* Re: [RFC PATCH 10/11] powerpc/tm: Set failure summary
From: Breno Leitao @ 2018-09-27 20:52 UTC (permalink / raw)
To: Michael Neuling, linuxppc-dev; +Cc: ldufour, gromero
In-Reply-To: <7647a2d788ceb979e6de8bb8bb988f4d37fb1877.camel@neuling.org>
Hi Mikey,
On 09/18/2018 02:50 AM, Michael Neuling wrote:
> On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
>> Since the transaction will be doomed with treckpt., the TEXASR[FS]
>> should be set, to reflect that the transaction is a failure. This patch
>> ensures it before recheckpointing, and remove changes from other places
>> that were calling recheckpoint.
>
> TEXASR[FS] should be set by the reclaim.
Do you mean that the CPU should set TEXASR[FS] when treclaim is called, or,
that the tm_reclaim?
Looking at the ISA, I didn't see TEXASR[FS] being set automatically when a
reclaim happens, although, I see it needs TEXASR[FS] to be set when executing
a trecheckpoint, otherwise it will cause a TM Bad Thing.
That is why I am forcing TEXASR[FS]=1 to doom the transaction so we can
recheckpoint it, but it seems I understood this wrongly.
> I don't know why you'd need to set this
> explicitly in process.c. The only case is when the user supplies a bad signal
> context, but we should check that in the signals code, not process.c
>
> Hence I think this patch is wrong.
>
> Also, according to the architecture, TEXASR[FS] HAS TO BE SET on trecheckpoint
> otherwise you'll get a TM Bad Thing. You should say that rather than suggesting
> it's because the transaction is doomed. It's ilqlegal to not do it. That's why we
> have this check in arch/powerpc/kernel/tm.S.
When you say "HAS TO BE SET", do you mean that the hardware will set it and
we shouldn't care about this flag? Thus, if I am hitting EMIT_BUG_ENTRY, it
means my TEXASR was messed somehow?
Thank you
^ permalink raw reply
* Re: [RFC PATCH 11/11] selftests/powerpc: Adapt the test
From: Breno Leitao @ 2018-09-27 20:57 UTC (permalink / raw)
To: Michael Neuling, linuxppc-dev; +Cc: ldufour, gromero
In-Reply-To: <b544029461059473cb7b8b6c66815a3ff31ec540.camel@neuling.org>
Hi Mikey,
On 09/18/2018 03:36 AM, Michael Neuling wrote:
> On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
>> The Documentation/powerpc/transactional_memory.txt says:
>>
>> "Syscalls made from within a suspended transaction are performed as normal
>> and the transaction is not explicitly doomed by the kernel. However,
>> what the kernel does to perform the syscall may result in the transaction
>> being doomed by the hardware."
>>
>> With this new TM mechanism, the syscall will continue to be executed if the
>> syscall happens on a suspended syscall, but, the syscall will *fail* if the
>> transaction is still active during the syscall invocation.
>
> Not sure I get this. This doesn't seem any different to before.
>
> An active (not suspended) transaction *will* result in the syscall failing and
> the transaction being doomed.
>
> A syscall in a suspended transaction should succeed and the transaction.
I understand that a transaction will always be doomed if there is a
reclaim/checkpoint, thus, if the you make a system call inside a suspended
transaction, it will reclaim and recheckpoint, thus, dooming the transaction,
and failing it on the next RFID.
So, the syscall executed in suspended mode may succeed, because it will not
be skipped (as in active mode), but the transaction will *always* fail,
because there was a reclaim and recheckpoint.
> You might need to clean up the language. I try to use:
>
> Active == transactional but not suspended (ie MSR[TS] = T)
> Suspended == suspended (ie MSR [TS] = S)
> Doomed == transaction to be rolled back at next opportinity (ie tcheck returns doomed)
>
> (note: the kernel MSR_TM_ACTIVE() macro is not consistent with this since it's
> MSR[TS] == S or T).
Ok, I agree with this language as well. I might want to improve the code to
follow the same language all across the code.
>> On the syscall path, if the transaction is active and not suspended, it
>> will call TM_KERNEL_ENTRY which will reclaim and recheckpoint the
>> transaction, thus, dooming the transaction on userspace return, with
>> failure code TM_CAUSE_SYSCALL.
>
> But the test below is on a suspend transaction?
Sorry, I meant "suspended transaction" above instead of "transaction is
active and not suspended".
>
>> This new model will break part of this test, but I understand that that the
>> documentation above didn't guarantee that the syscall would succeed, and it
>> will never succeed anymore now on.
>
> The syscall should pass in suspend (modulo the normal syscall checks). The
> transaction may fail as a result.
Perfect, and if the transaction fail, the CPU will rollback the changes and
restore the checkpoint registers (replacing the r3 that contains the pid
value), thus, it will be like "getpid" system call didn't execute.
For this test specifically, it assumes the syscall didn't execute if the
transaction failed. Take a look:
FUNC_START(getppid_tm_suspended)
tbegin.
beq 1f
li r0, __NR_getppid
tsuspend.
sc
tresume.
tend.
blr
1:
li r3, -1
blr
So, the tests thinks the syscall failed because the transaction aborted.
Anyway, I can try to improve this test other than removing this test, but I
am not sure how.
Thank you
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox