Re: [PATCH v2 13/24] sh/cpu: Make sure play_dead() doesn't return
On 14/2/23 08:05, Josh Poimboeuf wrote:
play_dead() doesn't return. Make that more explicit with a BUG().
BUG() is preferable to unreachable() because BUG() is a more explicit
failure mode and avoids undefined behavior like falling off the edge of
the function into whatever code happens to be next.
Signed-off-by: Josh Poimboeuf
---
arch/sh/include/asm/smp-ops.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/sh/include/asm/smp-ops.h b/arch/sh/include/asm/smp-ops.h
index e27702130eb6..63866b1595a0 100644
--- a/arch/sh/include/asm/smp-ops.h
+++ b/arch/sh/include/asm/smp-ops.h
@@ -27,6 +27,7 @@ static inline void plat_smp_setup(void)
static inline void play_dead(void)
{
mp_ops->play_dead();
+ BUG();
}
Shouldn't we decorate plat_smp_ops::play_dead() as noreturn first?
Re: [PATCH v2 19/24] xtensa/cpu: Make sure cpu_die() doesn't return
Hi Josh, On 14/2/23 08:05, Josh Poimboeuf wrote: cpu_die() doesn't return. Make that more explicit with a BUG(). BUG() is preferable to unreachable() because BUG() is a more explicit failure mode and avoids undefined behavior like falling off the edge of the function into whatever code happens to be next. Signed-off-by: Josh Poimboeuf --- arch/xtensa/kernel/smp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/xtensa/kernel/smp.c b/arch/xtensa/kernel/smp.c index 4dc109dd6214..7bad78495536 100644 --- a/arch/xtensa/kernel/smp.c +++ b/arch/xtensa/kernel/smp.c Can you update the documentation along? Currently we have: /* * Called from the idle thread for the CPU which has been shutdown. * * Note that we disable IRQs here, but do not re-enable them * before returning to the caller. This is also the behaviour * of the other hotplug-cpu capable cores, so presumably coming * out of idle fixes this. */ @@ -341,6 +341,8 @@ void __ref cpu_die(void) __asm__ __volatile__( " movia2, cpu_restart\n" " jx a2\n"); + + BUG(); } #endif /* CONFIG_HOTPLUG_CPU */
Re: [PATCH v2 16/24] sparc/cpu: Mark cpu_play_dead() __noreturn
On 14/2/23 08:05, Josh Poimboeuf wrote: cpu_play_dead() doesn't return. Annotate it as such. By extension this also makes arch_cpu_idle_dead() noreturn. Signed-off-by: Josh Poimboeuf --- arch/sparc/include/asm/smp_64.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 11/24] mips/cpu: Mark play_dead() __noreturn
On 14/2/23 08:05, Josh Poimboeuf wrote: play_dead() doesn't return. Annotate it as such. By extension this also makes arch_cpu_idle_dead() noreturn. Acked-by: Florian Fainelli Signed-off-by: Josh Poimboeuf --- arch/mips/include/asm/smp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 10/24] mips/cpu: Make sure play_dead() doesn't return
On 14/2/23 08:05, Josh Poimboeuf wrote: play_dead() doesn't return. Make that more explicit with a BUG(). BUG() is preferable to unreachable() because BUG() is a more explicit failure mode and avoids undefined behavior like falling off the edge of the function into whatever code happens to be next. Acked-by: Florian Fainelli Signed-off-by: Josh Poimboeuf --- arch/mips/kernel/smp-bmips.c | 2 ++ arch/mips/loongson64/smp.c | 1 + 2 files changed, 3 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 09/24] mips/cpu: Expose play_dead()'s prototype definition
Hi Josh, On 14/2/23 08:05, Josh Poimboeuf wrote: Include to make sure play_dead() matches its prototype going forward. Acked-by: Florian Fainelli Signed-off-by: Josh Poimboeuf --- arch/mips/kernel/smp-bmips.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/mips/kernel/smp-bmips.c b/arch/mips/kernel/smp-bmips.c index f5d7bfa3472a..df9158e8329d 100644 --- a/arch/mips/kernel/smp-bmips.c +++ b/arch/mips/kernel/smp-bmips.c @@ -38,6 +38,7 @@ #include #include #include +#include What about the other implementations? $ git grep -L asm/smp.h $(git grep -wlF 'play_dead(void)' arch/mips) arch/mips/cavium-octeon/smp.c arch/mips/kernel/smp-bmips.c arch/mips/kernel/smp-cps.c arch/mips/loongson64/smp.c
Re: [PATCH v2 12/24] powerpc/cpu: Mark start_secondary_resume() __noreturn
Le 14/02/2023 à 08:05, Josh Poimboeuf a écrit : > start_secondary_resume() doesn't return. Annotate it as such. By > extension this also makes arch_cpu_idle_dead() noreturn. > > Acked-by: Michael Ellerman (powerpc) > Signed-off-by: Josh Poimboeuf Reviewed-by: Christophe Leroy > --- > arch/powerpc/include/asm/smp.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h > index f63505d74932..cfd42ca8765c 100644 > --- a/arch/powerpc/include/asm/smp.h > +++ b/arch/powerpc/include/asm/smp.h > @@ -66,7 +66,7 @@ void start_secondary(void *unused); > extern int smp_send_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 > delay_us); > extern int smp_send_safe_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 > delay_us); > extern void smp_send_debugger_break(void); > -extern void start_secondary_resume(void); > +extern void __noreturn start_secondary_resume(void); Would have been a good opportunity to drop the pointless 'extern' keyword. Checkpatch reports: CHECK: extern prototypes should be avoided in .h files #19: FILE: arch/powerpc/include/asm/smp.h:69: +extern void __noreturn start_secondary_resume(void); > extern void smp_generic_give_timebase(void); > extern void smp_generic_take_timebase(void); >
[PATCH v2 24/24] sched/idle: Mark arch_cpu_idle_dead() __noreturn
Before commit 076cbf5d2163 ("x86/xen: don't let xen_pv_play_dead()
return"), in Xen, when a previously offlined CPU was brought back
online, it unexpectedly resumed execution where it left off in the
middle of the idle loop.
There were some hacks to make that work, but the behavior was surprising
as do_idle() doesn't expect an offlined CPU to return from the dead (in
arch_cpu_idle_dead()).
Now that Xen has been fixed, and the arch-specific implementations of
arch_cpu_idle_dead() also don't return, give it a __noreturn attribute.
This will cause the compiler to complain if an arch-specific
implementation might return. It also improves code generation for both
caller and callee.
Also fixes the following warning:
vmlinux.o: warning: objtool: do_idle+0x25f: unreachable instruction
Reported-by: Paul E. McKenney
Tested-by: Paul E. McKenney
Signed-off-by: Josh Poimboeuf
---
arch/alpha/kernel/process.c | 2 +-
arch/arm/kernel/smp.c | 2 +-
arch/arm64/kernel/process.c | 2 +-
arch/csky/kernel/smp.c | 2 +-
arch/ia64/kernel/process.c | 2 +-
arch/loongarch/kernel/process.c | 2 +-
arch/mips/kernel/process.c | 2 +-
arch/parisc/kernel/process.c| 2 +-
arch/powerpc/kernel/smp.c | 2 +-
arch/riscv/kernel/cpu-hotplug.c | 2 +-
arch/s390/kernel/idle.c | 2 +-
arch/sh/kernel/idle.c | 2 +-
arch/sparc/kernel/process_64.c | 2 +-
arch/x86/kernel/process.c | 2 +-
arch/xtensa/kernel/smp.c| 2 +-
include/linux/cpu.h | 2 +-
kernel/sched/idle.c | 2 +-
tools/objtool/check.c | 1 +
18 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
index d0ff06eda8fa..9bae2fc4910f 100644
--- a/arch/alpha/kernel/process.c
+++ b/arch/alpha/kernel/process.c
@@ -60,7 +60,7 @@ void arch_cpu_idle(void)
wtint(0);
}
-void arch_cpu_idle_dead(void)
+void __noreturn arch_cpu_idle_dead(void)
{
wtint(INT_MAX);
BUG();
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index adcd417c526b..c2daa0f2f784 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -320,7 +320,7 @@ void __cpu_die(unsigned int cpu)
* of the other hotplug-cpu capable cores, so presumably coming
* out of idle fixes this.
*/
-void arch_cpu_idle_dead(void)
+void __noreturn arch_cpu_idle_dead(void)
{
unsigned int cpu = smp_processor_id();
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 269ac1c25ae2..8d606bbb4a9a 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -69,7 +69,7 @@ void (*pm_power_off)(void);
EXPORT_SYMBOL_GPL(pm_power_off);
#ifdef CONFIG_HOTPLUG_CPU
-void arch_cpu_idle_dead(void)
+void __noreturn arch_cpu_idle_dead(void)
{
cpu_die();
}
diff --git a/arch/csky/kernel/smp.c b/arch/csky/kernel/smp.c
index 0ec20efaf5fd..9c7a20b73ac6 100644
--- a/arch/csky/kernel/smp.c
+++ b/arch/csky/kernel/smp.c
@@ -300,7 +300,7 @@ void __cpu_die(unsigned int cpu)
pr_notice("CPU%u: shutdown\n", cpu);
}
-void arch_cpu_idle_dead(void)
+void __noreturn arch_cpu_idle_dead(void)
{
idle_task_exit();
diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index 78f5794b2dde..9a5cd9fad3a9 100644
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -225,7 +225,7 @@ static inline void __noreturn play_dead(void)
}
#endif /* CONFIG_HOTPLUG_CPU */
-void arch_cpu_idle_dead(void)
+void __noreturn arch_cpu_idle_dead(void)
{
play_dead();
}
diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c
index edfd220a3737..ba70e94eb996 100644
--- a/arch/loongarch/kernel/process.c
+++ b/arch/loongarch/kernel/process.c
@@ -61,7 +61,7 @@ unsigned long boot_option_idle_override = IDLE_NO_OVERRIDE;
EXPORT_SYMBOL(boot_option_idle_override);
#ifdef CONFIG_HOTPLUG_CPU
-void arch_cpu_idle_dead(void)
+void __noreturn arch_cpu_idle_dead(void)
{
play_dead();
}
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 093dbbd6b843..a3225912c862 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -40,7 +40,7 @@
#include
#ifdef CONFIG_HOTPLUG_CPU
-void arch_cpu_idle_dead(void)
+void __noreturn arch_cpu_idle_dead(void)
{
play_dead();
}
diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index c064719b49b0..97c6f875bd0e 100644
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -159,7 +159,7 @@ EXPORT_SYMBOL(running_on_qemu);
/*
* Called from the idle thread for the CPU which has been shutdown.
*/
-void arch_cpu_idle_dead(void)
+void __noreturn arch_cpu_idle_dead(void)
{
#ifdef CONFIG_HOTPLUG_CPU
idle_task_exit();
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 6b90f10a6c81..f62e5e651bcd 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1752,7 +1752,7
[PATCH v2 23/24] init: Make arch_call_rest_init() and rest_init() __noreturn
arch_call_rest_init() and rest_init() don't return. Annotate them as
such.
Fixes the following warning:
init/main.o: warning: objtool: start_kernel+0x4ad: unreachable instruction
Signed-off-by: Josh Poimboeuf
---
arch/s390/kernel/setup.c | 2 +-
include/linux/start_kernel.h | 4 ++--
init/main.c | 4 ++--
tools/objtool/check.c| 2 ++
4 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
index 696c9e007a36..85abff362f29 100644
--- a/arch/s390/kernel/setup.c
+++ b/arch/s390/kernel/setup.c
@@ -393,7 +393,7 @@ int __init arch_early_irq_init(void)
return 0;
}
-void __init arch_call_rest_init(void)
+void __init __noreturn arch_call_rest_init(void)
{
unsigned long stack;
diff --git a/include/linux/start_kernel.h b/include/linux/start_kernel.h
index 8b369a41c03c..864921e54c92 100644
--- a/include/linux/start_kernel.h
+++ b/include/linux/start_kernel.h
@@ -9,7 +9,7 @@
up something else. */
extern asmlinkage void __init start_kernel(void);
-extern void __init arch_call_rest_init(void);
-extern void __ref rest_init(void);
+extern void __init __noreturn arch_call_rest_init(void);
+extern void __ref __noreturn rest_init(void);
#endif /* _LINUX_START_KERNEL_H */
diff --git a/init/main.c b/init/main.c
index e1c3911d7c70..ef71a9902e47 100644
--- a/init/main.c
+++ b/init/main.c
@@ -683,7 +683,7 @@ static void __init setup_command_line(char *command_line)
static __initdata DECLARE_COMPLETION(kthreadd_done);
-noinline void __ref rest_init(void)
+noinline void __ref __noreturn rest_init(void)
{
struct task_struct *tsk;
int pid;
@@ -889,7 +889,7 @@ static int __init early_randomize_kstack_offset(char *buf)
early_param("randomize_kstack_offset", early_randomize_kstack_offset);
#endif
-void __init __weak arch_call_rest_init(void)
+void __init __weak __noreturn arch_call_rest_init(void)
{
rest_init();
}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0a1cf867d9b2..f79baae3e338 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -167,6 +167,7 @@ static bool __dead_end_function(struct objtool_file *file,
struct symbol *func,
"__reiserfs_panic",
"__stack_chk_fail",
"__ubsan_handle_builtin_unreachable",
+ "arch_call_rest_init",
"cpu_bringup_and_idle",
"cpu_startup_entry",
"do_exit",
@@ -181,6 +182,7 @@ static bool __dead_end_function(struct objtool_file *file,
struct symbol *func,
"machine_real_restart",
"make_task_dead",
"panic",
+ "rest_init",
"rewind_stack_and_make_dead",
"sev_es_terminate",
"snp_abort",
--
2.39.1
[PATCH v2 22/24] objtool: Include weak functions in 'global_noreturns' check
If a global __noreturn function prototype has a corresponding weak function, it should also be __noreturn. Signed-off-by: Josh Poimboeuf --- tools/objtool/check.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index ba07a8ebaf73..0a1cf867d9b2 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -193,14 +193,14 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func, if (!func) return false; - if (func->bind == STB_WEAK) - return false; - - if (func->bind == STB_GLOBAL) + if (func->bind == STB_GLOBAL || func->bind == STB_WEAK) for (i = 0; i < ARRAY_SIZE(global_noreturns); i++) if (!strcmp(func->name, global_noreturns[i])) return true; + if (func->bind == STB_WEAK) + return false; + if (!func->len) return false; -- 2.39.1
[PATCH v2 21/24] sched/idle: Make sure weak version of arch_cpu_idle_dead() doesn't return
arch_cpu_idle_dead() should never return. Make it so.
Signed-off-by: Josh Poimboeuf
---
kernel/sched/idle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index e9ef66be2870..56e152f06d0f 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -75,7 +75,7 @@ static noinline int __cpuidle cpu_idle_poll(void)
void __weak arch_cpu_idle_prepare(void) { }
void __weak arch_cpu_idle_enter(void) { }
void __weak arch_cpu_idle_exit(void) { }
-void __weak arch_cpu_idle_dead(void) { }
+void __weak arch_cpu_idle_dead(void) { while (1); }
void __weak arch_cpu_idle(void)
{
cpu_idle_force_poll = 1;
--
2.39.1
[PATCH v2 20/24] xtensa/cpu: Mark cpu_die() __noreturn
cpu_die() doesn't return. Annotate it as such. By extension this also makes arch_cpu_idle_dead() noreturn. Signed-off-by: Josh Poimboeuf --- arch/xtensa/include/asm/smp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/xtensa/include/asm/smp.h b/arch/xtensa/include/asm/smp.h index 4e43f5643891..5dc5bf8cdd77 100644 --- a/arch/xtensa/include/asm/smp.h +++ b/arch/xtensa/include/asm/smp.h @@ -33,7 +33,7 @@ void show_ipi_list(struct seq_file *p, int prec); void __cpu_die(unsigned int cpu); int __cpu_disable(void); -void cpu_die(void); +void __noreturn cpu_die(void); void cpu_restart(void); #endif /* CONFIG_HOTPLUG_CPU */ -- 2.39.1
[PATCH v2 19/24] xtensa/cpu: Make sure cpu_die() doesn't return
cpu_die() doesn't return. Make that more explicit with a BUG(). BUG() is preferable to unreachable() because BUG() is a more explicit failure mode and avoids undefined behavior like falling off the edge of the function into whatever code happens to be next. Signed-off-by: Josh Poimboeuf --- arch/xtensa/kernel/smp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/xtensa/kernel/smp.c b/arch/xtensa/kernel/smp.c index 4dc109dd6214..7bad78495536 100644 --- a/arch/xtensa/kernel/smp.c +++ b/arch/xtensa/kernel/smp.c @@ -341,6 +341,8 @@ void __ref cpu_die(void) __asm__ __volatile__( " movia2, cpu_restart\n" " jx a2\n"); + + BUG(); } #endif /* CONFIG_HOTPLUG_CPU */ -- 2.39.1
[PATCH v2 18/24] x86/cpu: Mark play_dead() __noreturn
play_dead() doesn't return. Annotate it as such. By extension this
also makes arch_cpu_idle_dead() noreturn.
Signed-off-by: Josh Poimboeuf
---
arch/x86/include/asm/smp.h | 2 +-
arch/x86/kernel/process.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 8f628e08b25a..e6d1d2810e38 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -93,7 +93,7 @@ static inline void __cpu_die(unsigned int cpu)
smp_ops.cpu_die(cpu);
}
-static inline void play_dead(void)
+static inline void __noreturn play_dead(void)
{
smp_ops.play_dead();
BUG();
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index e57cd31bfec4..4433d13edd44 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -715,7 +715,7 @@ static bool x86_idle_set(void)
}
#ifndef CONFIG_SMP
-static inline void play_dead(void)
+static inline void __noreturn play_dead(void)
{
BUG();
}
--
2.39.1
[PATCH v2 17/24] x86/cpu: Make sure play_dead() doesn't return
After commit 076cbf5d2163 ("x86/xen: don't let xen_pv_play_dead()
return"), play_dead() never returns. Make that more explicit with a
BUG().
BUG() is preferable to unreachable() because BUG() is a more explicit
failure mode and avoids undefined behavior like falling off the edge of
the function into whatever code happens to be next.
Signed-off-by: Josh Poimboeuf
---
arch/x86/include/asm/smp.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index b4dbb20dab1a..8f628e08b25a 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -96,6 +96,7 @@ static inline void __cpu_die(unsigned int cpu)
static inline void play_dead(void)
{
smp_ops.play_dead();
+ BUG();
}
static inline void smp_send_reschedule(int cpu)
--
2.39.1
[PATCH v2 16/24] sparc/cpu: Mark cpu_play_dead() __noreturn
cpu_play_dead() doesn't return. Annotate it as such. By extension this also makes arch_cpu_idle_dead() noreturn. Signed-off-by: Josh Poimboeuf --- arch/sparc/include/asm/smp_64.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/sparc/include/asm/smp_64.h b/arch/sparc/include/asm/smp_64.h index e75783b6abc4..505b6700805d 100644 --- a/arch/sparc/include/asm/smp_64.h +++ b/arch/sparc/include/asm/smp_64.h @@ -49,7 +49,7 @@ int hard_smp_processor_id(void); void smp_fill_in_cpu_possible_map(void); void smp_fill_in_sib_core_maps(void); -void cpu_play_dead(void); +void __noreturn cpu_play_dead(void); void smp_fetch_global_regs(void); void smp_fetch_global_pmu(void); -- 2.39.1
[PATCH v2 15/24] sh/cpu: Expose arch_cpu_idle_dead()'s prototype definition
Include to make sure arch_cpu_idle_dead() matches its prototype going forward. Signed-off-by: Josh Poimboeuf --- arch/sh/kernel/idle.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/sh/kernel/idle.c b/arch/sh/kernel/idle.c index 3418c40f0099..114f0c4abeac 100644 --- a/arch/sh/kernel/idle.c +++ b/arch/sh/kernel/idle.c @@ -4,6 +4,7 @@ * * Copyright (C) 2002 - 2009 Paul Mundt */ +#include #include #include #include -- 2.39.1
[PATCH v2 14/24] sh/cpu: Mark play_dead() __noreturn
play_dead() doesn't return. Annotate it as such. By extension this
also makes arch_cpu_idle_dead() noreturn.
Signed-off-by: Josh Poimboeuf
---
arch/sh/include/asm/smp-ops.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/sh/include/asm/smp-ops.h b/arch/sh/include/asm/smp-ops.h
index 63866b1595a0..97331fcb7b85 100644
--- a/arch/sh/include/asm/smp-ops.h
+++ b/arch/sh/include/asm/smp-ops.h
@@ -24,7 +24,7 @@ static inline void plat_smp_setup(void)
mp_ops->smp_setup();
}
-static inline void play_dead(void)
+static inline void __noreturn play_dead(void)
{
mp_ops->play_dead();
BUG();
@@ -43,7 +43,7 @@ static inline void register_smp_ops(struct plat_smp_ops *ops)
{
}
-static inline void play_dead(void)
+static inline void __noreturn play_dead(void)
{
BUG();
}
--
2.39.1
[PATCH v2 13/24] sh/cpu: Make sure play_dead() doesn't return
play_dead() doesn't return. Make that more explicit with a BUG().
BUG() is preferable to unreachable() because BUG() is a more explicit
failure mode and avoids undefined behavior like falling off the edge of
the function into whatever code happens to be next.
Signed-off-by: Josh Poimboeuf
---
arch/sh/include/asm/smp-ops.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/sh/include/asm/smp-ops.h b/arch/sh/include/asm/smp-ops.h
index e27702130eb6..63866b1595a0 100644
--- a/arch/sh/include/asm/smp-ops.h
+++ b/arch/sh/include/asm/smp-ops.h
@@ -27,6 +27,7 @@ static inline void plat_smp_setup(void)
static inline void play_dead(void)
{
mp_ops->play_dead();
+ BUG();
}
extern void register_smp_ops(struct plat_smp_ops *ops);
--
2.39.1
[PATCH v2 12/24] powerpc/cpu: Mark start_secondary_resume() __noreturn
start_secondary_resume() doesn't return. Annotate it as such. By extension this also makes arch_cpu_idle_dead() noreturn. Acked-by: Michael Ellerman (powerpc) Signed-off-by: Josh Poimboeuf --- arch/powerpc/include/asm/smp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h index f63505d74932..cfd42ca8765c 100644 --- a/arch/powerpc/include/asm/smp.h +++ b/arch/powerpc/include/asm/smp.h @@ -66,7 +66,7 @@ void start_secondary(void *unused); extern int smp_send_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 delay_us); extern int smp_send_safe_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 delay_us); extern void smp_send_debugger_break(void); -extern void start_secondary_resume(void); +extern void __noreturn start_secondary_resume(void); extern void smp_generic_give_timebase(void); extern void smp_generic_take_timebase(void); -- 2.39.1
[PATCH v2 11/24] mips/cpu: Mark play_dead() __noreturn
play_dead() doesn't return. Annotate it as such. By extension this also makes arch_cpu_idle_dead() noreturn. Acked-by: Florian Fainelli Signed-off-by: Josh Poimboeuf --- arch/mips/include/asm/smp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/mips/include/asm/smp.h b/arch/mips/include/asm/smp.h index 5d9ff61004ca..4eee29b7845c 100644 --- a/arch/mips/include/asm/smp.h +++ b/arch/mips/include/asm/smp.h @@ -88,7 +88,7 @@ static inline void __cpu_die(unsigned int cpu) mp_ops->cpu_die(cpu); } -extern void play_dead(void); +extern void __noreturn play_dead(void); #endif #ifdef CONFIG_KEXEC -- 2.39.1
[PATCH v2 10/24] mips/cpu: Make sure play_dead() doesn't return
play_dead() doesn't return. Make that more explicit with a BUG(). BUG() is preferable to unreachable() because BUG() is a more explicit failure mode and avoids undefined behavior like falling off the edge of the function into whatever code happens to be next. Acked-by: Florian Fainelli Signed-off-by: Josh Poimboeuf --- arch/mips/kernel/smp-bmips.c | 2 ++ arch/mips/loongson64/smp.c | 1 + 2 files changed, 3 insertions(+) diff --git a/arch/mips/kernel/smp-bmips.c b/arch/mips/kernel/smp-bmips.c index df9158e8329d..be85fa075830 100644 --- a/arch/mips/kernel/smp-bmips.c +++ b/arch/mips/kernel/smp-bmips.c @@ -414,6 +414,8 @@ void __ref play_dead(void) " wait\n" " j bmips_secondary_reentry\n" : : : "memory"); + + BUG(); } #endif /* CONFIG_HOTPLUG_CPU */ diff --git a/arch/mips/loongson64/smp.c b/arch/mips/loongson64/smp.c index 660e1de4412a..c81c2bd07c62 100644 --- a/arch/mips/loongson64/smp.c +++ b/arch/mips/loongson64/smp.c @@ -822,6 +822,7 @@ void play_dead(void) state_addr = _cpu(cpu_state, cpu); mb(); play_dead_at_ckseg1(state_addr); + BUG(); } static int loongson3_disable_clock(unsigned int cpu) -- 2.39.1
[PATCH v2 09/24] mips/cpu: Expose play_dead()'s prototype definition
Include to make sure play_dead() matches its prototype going forward. Acked-by: Florian Fainelli Signed-off-by: Josh Poimboeuf --- arch/mips/kernel/smp-bmips.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/mips/kernel/smp-bmips.c b/arch/mips/kernel/smp-bmips.c index f5d7bfa3472a..df9158e8329d 100644 --- a/arch/mips/kernel/smp-bmips.c +++ b/arch/mips/kernel/smp-bmips.c @@ -38,6 +38,7 @@ #include #include #include +#include static int __maybe_unused max_cpus = 1; -- 2.39.1
[PATCH v2 08/24] loongarch/cpu: Mark play_dead() __noreturn
play_dead() doesn't return. Annotate it as such. By extension this also makes arch_cpu_idle_dead() noreturn. Signed-off-by: Josh Poimboeuf --- arch/loongarch/include/asm/smp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h index d82687390b4a..416b653bccb4 100644 --- a/arch/loongarch/include/asm/smp.h +++ b/arch/loongarch/include/asm/smp.h @@ -99,7 +99,7 @@ static inline void __cpu_die(unsigned int cpu) loongson_cpu_die(cpu); } -extern void play_dead(void); +extern void __noreturn play_dead(void); #endif #endif /* __ASM_SMP_H */ -- 2.39.1
[PATCH v2 07/24] loongarch/cpu: Make sure play_dead() doesn't return
play_dead() doesn't return. Make that more explicit with a BUG(). BUG() is preferable to unreachable() because BUG() is a more explicit failure mode and avoids undefined behavior like falling off the edge of the function into whatever code happens to be next. Signed-off-by: Josh Poimboeuf --- arch/loongarch/kernel/smp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c index 8c6e227cb29d..51f328169a7b 100644 --- a/arch/loongarch/kernel/smp.c +++ b/arch/loongarch/kernel/smp.c @@ -336,7 +336,7 @@ void play_dead(void) iocsr_write32(0x, LOONGARCH_IOCSR_IPI_CLEAR); init_fn(); - unreachable(); + BUG(); } #endif -- 2.39.1
[PATCH v2 06/24] ia64/cpu: Mark play_dead() __noreturn
play_dead() doesn't return. Annotate it as such. By extension this
also makes arch_cpu_idle_dead() noreturn.
Signed-off-by: Josh Poimboeuf
---
arch/ia64/kernel/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index f6195a0a00ae..78f5794b2dde 100644
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -201,7 +201,7 @@ __setup("nohalt", nohalt_setup);
#ifdef CONFIG_HOTPLUG_CPU
/* We don't actually take CPU down, just spin without interrupts. */
-static inline void play_dead(void)
+static inline void __noreturn play_dead(void)
{
unsigned int this_cpu = smp_processor_id();
@@ -219,7 +219,7 @@ static inline void play_dead(void)
BUG();
}
#else
-static inline void play_dead(void)
+static inline void __noreturn play_dead(void)
{
BUG();
}
--
2.39.1
[PATCH v2 05/24] csky/cpu: Make sure arch_cpu_idle_dead() doesn't return
arch_cpu_idle_dead() doesn't return. Make that more explicit with a BUG(). BUG() is preferable to unreachable() because BUG() is a more explicit failure mode and avoids undefined behavior like falling off the edge of the function into whatever code happens to be next. Acked-by: Guo Ren Signed-off-by: Josh Poimboeuf --- arch/csky/kernel/smp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/csky/kernel/smp.c b/arch/csky/kernel/smp.c index b45d1073307f..0ec20efaf5fd 100644 --- a/arch/csky/kernel/smp.c +++ b/arch/csky/kernel/smp.c @@ -317,5 +317,7 @@ void arch_cpu_idle_dead(void) "jmpi csky_start_secondary" : : "r" (secondary_stack)); + + BUG(); } #endif -- 2.39.1
[PATCH v2 04/24] arm64/cpu: Mark cpu_die() __noreturn
cpu_die() doesn't return. Annotate it as such. By extension this also makes arch_cpu_idle_dead() noreturn. Signed-off-by: Josh Poimboeuf --- arch/arm64/include/asm/smp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h index fc55f5a57a06..5733a31bab08 100644 --- a/arch/arm64/include/asm/smp.h +++ b/arch/arm64/include/asm/smp.h @@ -100,7 +100,7 @@ static inline void arch_send_wakeup_ipi_mask(const struct cpumask *mask) extern int __cpu_disable(void); extern void __cpu_die(unsigned int cpu); -extern void cpu_die(void); +extern void __noreturn cpu_die(void); extern void cpu_die_early(void); static inline void cpu_park_loop(void) -- 2.39.1
[PATCH v2 03/24] arm/cpu: Make sure arch_cpu_idle_dead() doesn't return
arch_cpu_idle_dead() doesn't return. Make that more explicit with a BUG(). BUG() is preferable to unreachable() because BUG() is a more explicit failure mode and avoids undefined behavior like falling off the edge of the function into whatever code happens to be next. Signed-off-by: Josh Poimboeuf --- arch/arm/kernel/smp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 0b8c25763adc..adcd417c526b 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -382,6 +382,8 @@ void arch_cpu_idle_dead(void) : "r" (task_stack_page(current) + THREAD_SIZE - 8), "r" (current) : "r0"); + + BUG(); } #endif /* CONFIG_HOTPLUG_CPU */ -- 2.39.1
[PATCH v2 01/24] alpha/cpu: Expose arch_cpu_idle_dead()'s prototype declaration
Include to make sure arch_cpu_idle_dead() matches its prototype going forward. Signed-off-by: Josh Poimboeuf --- arch/alpha/kernel/process.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c index ce20c31828a0..d1f2e8b6b107 100644 --- a/arch/alpha/kernel/process.c +++ b/arch/alpha/kernel/process.c @@ -9,6 +9,7 @@ * This file handles the architecture-dependent parts of process handling. */ +#include #include #include #include -- 2.39.1
[PATCH v2 02/24] alpha/cpu: Make sure arch_cpu_idle_dead() doesn't return
arch_cpu_idle_dead() doesn't return. Make that more explicit with a
BUG().
BUG() is preferable to unreachable() because BUG() is a more explicit
failure mode and avoids undefined behavior like falling off the edge of
the function into whatever code happens to be next.
Signed-off-by: Josh Poimboeuf
---
arch/alpha/kernel/process.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
index d1f2e8b6b107..d0ff06eda8fa 100644
--- a/arch/alpha/kernel/process.c
+++ b/arch/alpha/kernel/process.c
@@ -63,6 +63,7 @@ void arch_cpu_idle(void)
void arch_cpu_idle_dead(void)
{
wtint(INT_MAX);
+ BUG();
}
#endif /* ALPHA_WTINT */
--
2.39.1
[PATCH v2 00/24] cpu,sched: Mark arch_cpu_idle_dead() __noreturn
v2: - make arch_call_rest_init() and rest_init() __noreturn - make objtool 'global_returns' work for weak functions - rebase on tip/objtool/core with dependencies merged in (mingo) - add acks v1.1: - add __noreturn to all arch_cpu_idle_dead() implementations (mpe) Josh Poimboeuf (24): alpha/cpu: Expose arch_cpu_idle_dead()'s prototype declaration alpha/cpu: Make sure arch_cpu_idle_dead() doesn't return arm/cpu: Make sure arch_cpu_idle_dead() doesn't return arm64/cpu: Mark cpu_die() __noreturn csky/cpu: Make sure arch_cpu_idle_dead() doesn't return ia64/cpu: Mark play_dead() __noreturn loongarch/cpu: Make sure play_dead() doesn't return loongarch/cpu: Mark play_dead() __noreturn mips/cpu: Expose play_dead()'s prototype definition mips/cpu: Make sure play_dead() doesn't return mips/cpu: Mark play_dead() __noreturn powerpc/cpu: Mark start_secondary_resume() __noreturn sh/cpu: Make sure play_dead() doesn't return sh/cpu: Mark play_dead() __noreturn sh/cpu: Expose arch_cpu_idle_dead()'s prototype definition sparc/cpu: Mark cpu_play_dead() __noreturn x86/cpu: Make sure play_dead() doesn't return x86/cpu: Mark play_dead() __noreturn xtensa/cpu: Make sure cpu_die() doesn't return xtensa/cpu: Mark cpu_die() __noreturn sched/idle: Make sure weak version of arch_cpu_idle_dead() doesn't return objtool: Include weak functions in 'global_noreturns' check init: Make arch_call_rest_init() and rest_init() __noreturn sched/idle: Mark arch_cpu_idle_dead() __noreturn arch/alpha/kernel/process.c | 4 +++- arch/arm/kernel/smp.c| 4 +++- arch/arm64/include/asm/smp.h | 2 +- arch/arm64/kernel/process.c | 2 +- arch/csky/kernel/smp.c | 4 +++- arch/ia64/kernel/process.c | 6 +++--- arch/loongarch/include/asm/smp.h | 2 +- arch/loongarch/kernel/process.c | 2 +- arch/loongarch/kernel/smp.c | 2 +- arch/mips/include/asm/smp.h | 2 +- arch/mips/kernel/process.c | 2 +- arch/mips/kernel/smp-bmips.c | 3 +++ arch/mips/loongson64/smp.c | 1 + arch/parisc/kernel/process.c | 2 +- arch/powerpc/include/asm/smp.h | 2 +- arch/powerpc/kernel/smp.c| 2 +- arch/riscv/kernel/cpu-hotplug.c | 2 +- arch/s390/kernel/idle.c | 2 +- arch/s390/kernel/setup.c | 2 +- arch/sh/include/asm/smp-ops.h| 5 +++-- arch/sh/kernel/idle.c| 3 ++- arch/sparc/include/asm/smp_64.h | 2 +- arch/sparc/kernel/process_64.c | 2 +- arch/x86/include/asm/smp.h | 3 ++- arch/x86/kernel/process.c| 4 ++-- arch/xtensa/include/asm/smp.h| 2 +- arch/xtensa/kernel/smp.c | 4 +++- include/linux/cpu.h | 2 +- include/linux/start_kernel.h | 4 ++-- init/main.c | 4 ++-- kernel/sched/idle.c | 2 +- tools/objtool/check.c| 11 +++ 32 files changed, 57 insertions(+), 39 deletions(-) -- 2.39.1
Re: [PATCH v6 7/7] powerpc: mm: Support page table check
Le 14/02/2023 à 02:59, Rohan McLure a écrit :
> On creation and clearing of a page table mapping, instrument such calls
> by invoking page_table_check_pte_set and page_table_check_pte_clear
> respectively. These calls serve as a sanity check against illegal
> mappings.
Please also explaing the changes around set_pte_at() versus set_pte().
>
> Enable ARCH_SUPPORTS_PAGE_TABLE_CHECK for all ppc64, and 32-bit
> platforms implementing Book3S.
As far as I can see below, it is implemented for all plateforms,
including nohash/32.
>
> Change pud_pfn to be a runtime bug rather than a build bug as it is
> consumed by page_table_check_pud_{clear,set} which are not called.
Isn't this done in another patch ?
>
> See also:
>
> riscv support in commit 3fee229a8eb9 ("riscv/mm: enable
> ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> arm64 in commit 42b2547137f5 ("arm64/mm: enable
> ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> x86_64 in commit d283d422c6c4 ("x86: mm: add x86_64 support for page table
> check")
>
> Signed-off-by: Rohan McLure
> ---
> V2: Update spacing and types assigned to pte_update calls.
> V3: Update one last pte_update call to remove __pte invocation.
> V5: Fix 32-bit nohash double set
> V6: Omit __set_pte_at instrumentation - should be instrumented by
> set_pte_at, with set_pte in between, performing all prior checks.
> Instrument pmds. Use set_pte where needed.
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/book3s/32/pgtable.h | 8 +++-
> arch/powerpc/include/asm/book3s/64/pgtable.h | 44
> arch/powerpc/include/asm/nohash/32/pgtable.h | 7 +++-
> arch/powerpc/include/asm/nohash/64/pgtable.h | 8 +++-
> arch/powerpc/include/asm/pgtable.h | 11 -
> arch/powerpc/mm/book3s64/hash_pgtable.c | 2 +-
> arch/powerpc/mm/book3s64/pgtable.c | 16 ---
> arch/powerpc/mm/book3s64/radix_pgtable.c | 10 ++---
> arch/powerpc/mm/nohash/book3e_pgtable.c | 2 +-
> arch/powerpc/mm/pgtable_32.c | 2 +-
> 11 files changed, 84 insertions(+), 27 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 2c9cdf1d8761..2474e2699037 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -154,6 +154,7 @@ config PPC
> select ARCH_STACKWALK
> select ARCH_SUPPORTS_ATOMIC_RMW
> select ARCH_SUPPORTS_DEBUG_PAGEALLOCif PPC_BOOK3S || PPC_8xx || 40x
> + select ARCH_SUPPORTS_PAGE_TABLE_CHECK
> select ARCH_USE_BUILTIN_BSWAP
> select ARCH_USE_CMPXCHG_LOCKREF if PPC64
> select ARCH_USE_MEMTEST
> diff --git a/arch/powerpc/include/asm/pgtable.h
> b/arch/powerpc/include/asm/pgtable.h
> index b76fdb80b6c9..df016a0a3135 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -48,7 +48,16 @@ struct mm_struct;
> /* 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))
> -#define set_pte_at set_pte
> +#define set_pte_at(mm, addr, ptep, pte) \
> +({ \
> + struct mm_struct *_mm = (mm); \
> + unsigned long _addr = (addr); \
> + pte_t *_ptep = (ptep), _pte = (pte);\
> + \
> + page_table_check_pte_set(_mm, _addr, _ptep, _pte); \
> + set_pte(_mm, _addr, _ptep, _pte); \
> +})
Can you make it a static inline function instead of a macro ?
> +
> /*
>* Select all bits except the pfn
>*/
Re: linux-next: qemu boot log difference today
Hi Nathan, On Mon, 13 Feb 2023 23:19:43 -0600 Nathan Lynch wrote: > > Stephen Rothwell writes: > > > > Today's qemu boot log shows 256k extra in reserved memory: > > > > - Memory: 2046080K/2097152K available (14720K kernel code, 2944K rwdata, > > 18048K rodata, 5184K init, 1431K bss, 51072K reserved, 0K cma-reserved) > > + Memory: 2045824K/2097152K available (14720K kernel code, 2944K rwdata, > > 18048K rodata, 5184K init, 1439K bss, 51328K reserved, 0K cma-reserved) > > > > I don't know what has caused this. > > Assuming it's pseries, it's the RTAS work area allocator reserving the > memory. > > 43033bc62d34 powerpc/pseries: add RTAS work area allocator Yeah, pseries_le_defconfig. Thanks, I will stop worrying about it. -- Cheers, Stephen Rothwell pgpP0vzOdLDG5.pgp Description: OpenPGP digital signature
Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic secure boot
On Mon, 2023-02-13 at 22:32 +1100, Michael Ellerman wrote: > > > + memcpy(, data, sizeof(flags)); > > > > conversion from bytestream to integer: I think in this case it > > would be better to use > > > > flags = cpu_to_be64p((__u64*)data); > > > > so that the flags always in hypervisor/big endian format > > I don't think it's correct to byte swap the flags here. They must be > in > big endian format, but that's up to the caller. That was what I initially thought, until I went and tested it properly and found it was indeed broken (at least in our qemu environment, this is slightly tricky for me to test right now on real hardware with real PowerVM) depending on kernel endianness. - Userspace writes the flags into the buffer in BE order - The first 8 bytes of the buffer are memcpy()ed, in BE order, into flags (a u64) - plpar_hcall9() is called with flags as an argument, loaded into r9 - r9 is moved to r8 before jumping into the hypervisor On a BE system, this works fine. On an LE system, this results in the bytes in the flags variable being loaded into the register in LE order, so the conversion is necessary. > The powernv secvar backend doesn't byte swap the flags, if the > pseries > one did then the final content of the variable, written either by > phyp > or OPAL, would differ depending on which backend is active. > > Or am I missing something? The powernv secvar backend doesn't have a notion of flags at all. (The flags interface for pseries is a little ugly, but it gets the job done - userspace can read the format attribute to discover what it needs to do.) -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v6 2/7] powerpc/64s: mm: Introduce __pmdp_collapse_flush with mm_struct argument
Le 14/02/2023 à 02:59, Rohan McLure a écrit :
> pmdp_collapse_flush has references in generic code with just three
> parameters, due to the choice of mm context being implied by the vm_area
> context parameter.
>
> Define __pmdp_collapse_flush to accept an additional mm_struct *
> parameter, with pmdp_collapse_flush a macro that unpacks the vma and
> calls __pmdp_collapse_flush. The mm_struct * parameter is needed in a
> future patch providing Page Table Check support, which is defined in
> terms of mm context objects.
>
> Signed-off-by: Rohan McLure
> ---
> v6: New patch
> ---
> arch/powerpc/include/asm/book3s/64/pgtable.h | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
> b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index cb4c67bf45d7..9d8b4e25f5ed 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -1244,14 +1244,22 @@ static inline pmd_t pmdp_huge_get_and_clear(struct
> mm_struct *mm,
> return hash__pmdp_huge_get_and_clear(mm, addr, pmdp);
> }
>
> -static inline pmd_t pmdp_collapse_flush(struct vm_area_struct *vma,
> - unsigned long address, pmd_t *pmdp)
> +static inline pmd_t __pmdp_collapse_flush(struct vm_area_struct *vma, struct
> mm_struct *mm,
> + unsigned long address, pmd_t *pmdp)
> {
> if (radix_enabled())
> return radix__pmdp_collapse_flush(vma, address, pmdp);
> return hash__pmdp_collapse_flush(vma, address, pmdp);
> }
> -#define pmdp_collapse_flush pmdp_collapse_flush
> +#define pmdp_collapse_flush(vma, addr, pmdp) \
> +({ \
> + struct vm_area_struct *_vma = (vma);\
> + pmd_t _r; \
> + \
> + _r = __pmdp_collapse_flush(_vma, _vma->vm_mm, (addr), (pmdp)); \
> + \
> + _r; \
> +})
Can you make it a static inline function instead of a ugly macro ?
>
> #define __HAVE_ARCH_PMDP_HUGE_GET_AND_CLEAR_FULL
> pmd_t pmdp_huge_get_and_clear_full(struct vm_area_struct *vma,
Re: [PATCH v6 1/7] powerpc: mm: Separate set_pte, set_pte_at for internal, external use
Le 14/02/2023 à 02:59, Rohan McLure a écrit :
> Produce separate symbols for set_pte, which is to be used in
> arch/powerpc for reassignment of pte's, and set_pte_at, used in generic
> code.
>
> The reason for this distinction is to support the Page Table Check
> sanitiser. Having this distinction allows for set_pte_at to
> instrumented, but set_pte not to be, permitting for uninstrumented
> internal mappings. This distinction in names is also present in x86.
>
> Signed-off-by: Rohan McLure
> ---
> v6: new patch
> ---
> arch/powerpc/include/asm/book3s/pgtable.h | 4 ++--
> arch/powerpc/include/asm/nohash/pgtable.h | 4 ++--
> arch/powerpc/include/asm/pgtable.h| 1 +
> arch/powerpc/mm/pgtable.c | 4 ++--
> 4 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/pgtable.h
> b/arch/powerpc/include/asm/book3s/pgtable.h
> index d18b748ea3ae..dbcdc2103c59 100644
> --- a/arch/powerpc/include/asm/book3s/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/pgtable.h
> @@ -12,8 +12,8 @@
> /* Insert a PTE, top-level function is out of line. It uses an inline
>* low level function in the respective pgtable-* files
>*/
> -extern void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
> -pte_t pte);
> +extern void set_pte(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
> + pte_t pte);
Remove 'extern' keyword, it's pointless and deprecated, checkpatch
--strict is likely complaining about it too.
Then have the protoype fit on a single line.
>
>
> #define __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
> diff --git a/arch/powerpc/include/asm/nohash/pgtable.h
> b/arch/powerpc/include/asm/nohash/pgtable.h
> index 69c3a050a3d8..ac3e69a18253 100644
> --- a/arch/powerpc/include/asm/nohash/pgtable.h
> +++ b/arch/powerpc/include/asm/nohash/pgtable.h
> @@ -154,8 +154,8 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t
> newprot)
> /* Insert a PTE, top-level function is out of line. It uses an inline
>* low level function in the respective pgtable-* files
>*/
> -extern void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
> -pte_t pte);
> +extern void set_pte(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
> + pte_t pte);
Remove 'extern' keyword and have the protoype fit on a single line.
>
> /* This low level function performs the actual PTE insertion
>* Setting the PTE depends on the MMU type and other factors. It's
> diff --git a/arch/powerpc/include/asm/pgtable.h
> b/arch/powerpc/include/asm/pgtable.h
> index 9972626ddaf6..17d30359d1f4 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -48,6 +48,7 @@ struct mm_struct;
> /* 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))
> +#define set_pte_at set_pte
> /*
>* Select all bits except the pfn
>*/
> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index cb2dcdb18f8e..e9a464e0d081 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -187,8 +187,8 @@ static pte_t set_access_flags_filter(pte_t pte, struct
> vm_area_struct *vma,
> /*
>* set_pte stores a linux PTE into the linux page table.
>*/
> -void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
> - pte_t pte)
> +void set_pte(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
> + pte_t pte)
Have it fit on a single line.
> {
> /*
>* Make sure hardware valid bit is not set. We don't do
Re: [PATCH v2 3/8] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
Le 13/02/2023 à 21:15, Pali Rohár a écrit :
> On Monday 13 February 2023 20:05:09 Christophe Leroy wrote:
>> Le 24/12/2022 à 22:14, Pali Rohár a écrit :
>>> +#ifdef CONFIG_MPC85xx_RDB
>>> +static void __init mpc85xx_rdb_pic_init(void)
>>> +{
>>> + struct mpic *mpic;
>>> +
>>> + mpic = mpic_alloc(NULL, 0,
>>> + MPIC_BIG_ENDIAN |
>>> + MPIC_SINGLE_DEST_CPU,
>>> + 0, 256, " OpenPIC ");
>>
>> Can you make it a single line ? At least no more than two lines ?
>
> All these lines are already present in kernel tree.
> So it could be fixed in separate patch.
>
> If needed I can do it as part of this series in new patch.
>
Well, that goes away in patch 5. So it is not really worth it.
Re: [PATCH v2 0/8] powerpc/85xx: p2020: Create one unified machine description
Le 13/02/2023 à 21:11, Pali Rohár a écrit : > On Monday 13 February 2023 19:58:15 Christophe Leroy wrote: >> Le 09/02/2023 à 01:15, Pali Rohár a écrit : This patch moves all p2020 boards from mpc85xx_rdb.c and mpc85xx_ds.c files into new p2020.c file, and plus it copies all helper functions which p2020 boards requires. This patch does not introduce any new code or functional change. It should be really plain copy/move. >> >> Yes after looking into it in more details, it is exactly that. You >> copied all helper functions but this is not said in the commit message. >> I think it should be said, and more important it should be explained why. >> Because this is exactly what I was not understanding, why I couldn't see >> all moved functions: just because many of them were not moved but copied. >> >> In the two first pages you made some function static, and then you >> duplicated it. Why ? Why not keep it global and just use it from one >> place to the other ? >> >> Because after patch 3 we have: >> >> arch/powerpc/platforms/85xx/mpc85xx_rdb.c:static void __init >> mpc85xx_rdb_pic_init(void) >> arch/powerpc/platforms/85xx/p2020.c:static void __init >> mpc85xx_rdb_pic_init(void) >> >> arch/powerpc/platforms/85xx/mpc85xx_ds.c:static void __init >> mpc85xx_ds_pic_init(void) >> arch/powerpc/platforms/85xx/p2020.c:static void __init >> mpc85xx_ds_pic_init(void) >> >> Why not just drop patches 1 and 2 and keep those two functions and all >> the other common functions like mpc85xx_8259_cascade() >> mpc85xx_ds_uli_init() and a lot more in a separate common file ? >> >> Christophe > > After applying all patches there is no mpc85xx_rdb_pic_init() / > mpc85xx_ds_pic_init() function in p2020.c file. There is > p2020_pic_init() in p2020.c but it is slightly different than previous > two functions. Ok, fair enough, but then please explain in the commit message that you copy the functions and then they will be re-written in following patches. That way we know exactly what we are reviewing. > > Maybe it could be possible to create one function mpc85xx_pic_init() as > unification of previous 3 functions, but such change would be needed to > test on lot of mpc85xx boards, which would be affected by such change. > And I do not have them for testing. I have only P2020. No, if the function are different it's better each platform has its own. My comment was for functions that are exactly the same. > > So I wrote *_pic_init() function which is p2020 specific, like already > existing ds and rdb specific functions in their own source files.
Re: linux-next: qemu boot log difference today
Stephen Rothwell writes: > > Today's qemu boot log shows 256k extra in reserved memory: > > - Memory: 2046080K/2097152K available (14720K kernel code, 2944K rwdata, > 18048K rodata, 5184K init, 1431K bss, 51072K reserved, 0K cma-reserved) > + Memory: 2045824K/2097152K available (14720K kernel code, 2944K rwdata, > 18048K rodata, 5184K init, 1439K bss, 51328K reserved, 0K cma-reserved) > > I don't know what has caused this. Assuming it's pseries, it's the RTAS work area allocator reserving the memory. 43033bc62d34 powerpc/pseries: add RTAS work area allocator
[powerpc:next-test] BUILD SUCCESS 631bb79c0c5654bdd79b6df186f9e41981a2b1fa
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test branch HEAD: 631bb79c0c5654bdd79b6df186f9e41981a2b1fa powerpc/nohash: Fix build error with binutils >= 2.38 elapsed time: 900m configs tested: 42 configs skipped: 3 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: alphaallyesconfig alpha defconfig arc allyesconfig arc defconfig arm allmodconfig arm allyesconfig arm defconfig arm64allyesconfig arm64 defconfig cskydefconfig i386 allyesconfig i386 debian-10.3 i386defconfig ia64 allmodconfig ia64defconfig loongarchallmodconfig loongarch allnoconfig loongarch defconfig m68k allmodconfig m68kdefconfig mips allmodconfig mips allyesconfig nios2 defconfig parisc defconfig parisc64defconfig powerpc allmodconfig powerpc allnoconfig riscvallmodconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig s390 allmodconfig s390 allyesconfig s390defconfig sh allmodconfig sparc defconfig um i386_defconfig um x86_64_defconfig x86_64 allyesconfig x86_64 defconfig x86_64 kexec x86_64 rhel-8.3 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests
linux-next: qemu boot log difference today
Hi all, Today's qemu boot log shows 256k extra in reserved memory: - Memory: 2046080K/2097152K available (14720K kernel code, 2944K rwdata, 18048K rodata, 5184K init, 1431K bss, 51072K reserved, 0K cma-reserved) + Memory: 2045824K/2097152K available (14720K kernel code, 2944K rwdata, 18048K rodata, 5184K init, 1439K bss, 51328K reserved, 0K cma-reserved) I don't know what has caused this. -- Cheers, Stephen Rothwell pgp_v0PSqNg5R.pgp Description: OpenPGP digital signature
[PATCH v6 7/7] powerpc: mm: Support page table check
On creation and clearing of a page table mapping, instrument such calls
by invoking page_table_check_pte_set and page_table_check_pte_clear
respectively. These calls serve as a sanity check against illegal
mappings.
Enable ARCH_SUPPORTS_PAGE_TABLE_CHECK for all ppc64, and 32-bit
platforms implementing Book3S.
Change pud_pfn to be a runtime bug rather than a build bug as it is
consumed by page_table_check_pud_{clear,set} which are not called.
See also:
riscv support in commit 3fee229a8eb9 ("riscv/mm: enable
ARCH_SUPPORTS_PAGE_TABLE_CHECK")
arm64 in commit 42b2547137f5 ("arm64/mm: enable
ARCH_SUPPORTS_PAGE_TABLE_CHECK")
x86_64 in commit d283d422c6c4 ("x86: mm: add x86_64 support for page table
check")
Signed-off-by: Rohan McLure
---
V2: Update spacing and types assigned to pte_update calls.
V3: Update one last pte_update call to remove __pte invocation.
V5: Fix 32-bit nohash double set
V6: Omit __set_pte_at instrumentation - should be instrumented by
set_pte_at, with set_pte in between, performing all prior checks.
Instrument pmds. Use set_pte where needed.
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/book3s/32/pgtable.h | 8 +++-
arch/powerpc/include/asm/book3s/64/pgtable.h | 44
arch/powerpc/include/asm/nohash/32/pgtable.h | 7 +++-
arch/powerpc/include/asm/nohash/64/pgtable.h | 8 +++-
arch/powerpc/include/asm/pgtable.h | 11 -
arch/powerpc/mm/book3s64/hash_pgtable.c | 2 +-
arch/powerpc/mm/book3s64/pgtable.c | 16 ---
arch/powerpc/mm/book3s64/radix_pgtable.c | 10 ++---
arch/powerpc/mm/nohash/book3e_pgtable.c | 2 +-
arch/powerpc/mm/pgtable_32.c | 2 +-
11 files changed, 84 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2c9cdf1d8761..2474e2699037 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -154,6 +154,7 @@ config PPC
select ARCH_STACKWALK
select ARCH_SUPPORTS_ATOMIC_RMW
select ARCH_SUPPORTS_DEBUG_PAGEALLOCif PPC_BOOK3S || PPC_8xx || 40x
+ select ARCH_SUPPORTS_PAGE_TABLE_CHECK
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF if PPC64
select ARCH_USE_MEMTEST
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h
b/arch/powerpc/include/asm/book3s/32/pgtable.h
index afd672e84791..8850b4fb22a4 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -53,6 +53,8 @@
#ifndef __ASSEMBLY__
+#include
+
static inline bool pte_user(pte_t pte)
{
return pte_val(pte) & _PAGE_USER;
@@ -338,7 +340,11 @@ static inline int __ptep_test_and_clear_young(struct
mm_struct *mm,
static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long
addr,
pte_t *ptep)
{
- return __pte(pte_update(mm, addr, ptep, ~_PAGE_HASHPTE, 0, 0));
+ pte_t old_pte = __pte(pte_update(mm, addr, ptep, ~_PAGE_HASHPTE, 0, 0));
+
+ page_table_check_pte_clear(mm, addr, old_pte);
+
+ return old_pte;
}
#define __HAVE_ARCH_PTEP_SET_WRPROTECT
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 93601ef4c665..1835ba2c2309 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -162,6 +162,8 @@
#define PAGE_KERNEL_ROX__pgprot(_PAGE_BASE | _PAGE_KERNEL_ROX)
#ifndef __ASSEMBLY__
+#include
+
/*
* page table defines
*/
@@ -431,8 +433,11 @@ static inline void huge_ptep_set_wrprotect(struct
mm_struct *mm,
static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
unsigned long addr, pte_t *ptep)
{
- unsigned long old = pte_update(mm, addr, ptep, ~0UL, 0, 0);
- return __pte(old);
+ pte_t old_pte = __pte(pte_update(mm, addr, ptep, ~0UL, 0, 0));
+
+ page_table_check_pte_clear(mm, addr, old_pte);
+
+ return old_pte;
}
#define __HAVE_ARCH_PTEP_GET_AND_CLEAR_FULL
@@ -441,11 +446,16 @@ static inline pte_t ptep_get_and_clear_full(struct
mm_struct *mm,
pte_t *ptep, int full)
{
if (full && radix_enabled()) {
+ pte_t old_pte;
+
/*
* We know that this is a full mm pte clear and
* hence can be sure there is no parallel set_pte.
*/
- return radix__ptep_get_and_clear_full(mm, addr, ptep, full);
+ old_pte = radix__ptep_get_and_clear_full(mm, addr, ptep, full);
+ page_table_check_pte_clear(mm, addr, old_pte);
+
+ return old_pte;
}
return ptep_get_and_clear(mm, addr, ptep);
}
@@ -1249,17 +1259,33 @@ extern int pmdp_test_and_clear_young(struct
vm_area_struct *vma,
static inline pmd_t pmdp_huge_get_and_clear(struct mm_struct *mm,
[PATCH v6 5/7] powerpc: mm: Add common pud_pfn stub for all platforms
Prior to this commit, pud_pfn was implemented with BUILD_BUG as the inline
function for 64-bit Book3S systems but is never included, as its
invocations in generic code are guarded by calls to pud_devmap which return
zero on such systems. A future patch will provide support for page table
checks, the generic code for which depends on a pud_pfn stub being
implemented, even while the patch will not interact with puds directly.
Remove the 64-bit Book3S stub and define pud_pfn to warn on all
platforms. pud_pfn may be defined properly on a per-platform basis
should it grow real usages in future.
Signed-off-by: Rohan McLure
---
V2: Remove conditional BUILD_BUG and BUG. Instead warn on usage.
V3: Replace WARN with WARN_ONCE, which should suffice to demonstrate
misuse of puds.
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 10 --
arch/powerpc/include/asm/pgtable.h | 14 ++
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 5be0a4c8bf32..8bbd3e1df93e 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1330,16 +1330,6 @@ static inline int pgd_devmap(pgd_t pgd)
}
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
-static inline int pud_pfn(pud_t pud)
-{
- /*
-* Currently all calls to pud_pfn() are gated around a pud_devmap()
-* check so this should never be used. If it grows another user we
-* want to know about it.
-*/
- BUILD_BUG();
- return 0;
-}
#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
pte_t ptep_modify_prot_start(struct vm_area_struct *, unsigned long, pte_t *);
void ptep_modify_prot_commit(struct vm_area_struct *, unsigned long,
diff --git a/arch/powerpc/include/asm/pgtable.h
b/arch/powerpc/include/asm/pgtable.h
index 284408829fa3..ad0829f816e9 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -153,6 +153,20 @@ struct seq_file;
void arch_report_meminfo(struct seq_file *m);
#endif /* CONFIG_PPC64 */
+/*
+ * Currently only consumed by page_table_check_pud_{set,clear}. Since clears
+ * and sets to page table entries at any level are done through
+ * page_table_check_pte_{set,clear}, provide stub implementation.
+ */
+#ifndef pud_pfn
+#define pud_pfn pud_pfn
+static inline int pud_pfn(pud_t pud)
+{
+ WARN_ONCE(1, "pud: platform does not use pud entries directly");
+ return 0;
+}
+#endif
+
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_PGTABLE_H */
--
2.37.2
[PATCH v6 6/7] powerpc: mm: Add p{te,md,ud}_user_accessible_page helpers
Add the following helpers for detecting whether a page table entry
is a leaf and is accessible to user space.
* pte_user_accessible_page
* pmd_user_accessible_page
* pud_user_accessible_page
Also implement missing pud_user definitions for both Book3S/nohash 64-bit
systems, and pmd_user for Book3S/nohash 32-bit systems.
Signed-off-by: Rohan McLure
---
V2: Provide missing pud_user implementations, use p{u,m}d_is_leaf.
V3: Provide missing pmd_user implementations as stubs in 32-bit.
V4: Use pmd_leaf, pud_leaf, and define pmd_user for 32 Book3E with
static inline method rather than macro.
---
arch/powerpc/include/asm/book3s/32/pgtable.h | 4
arch/powerpc/include/asm/book3s/64/pgtable.h | 10 ++
arch/powerpc/include/asm/nohash/32/pgtable.h | 5 +
arch/powerpc/include/asm/nohash/64/pgtable.h | 10 ++
arch/powerpc/include/asm/pgtable.h | 15 +++
5 files changed, 44 insertions(+)
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h
b/arch/powerpc/include/asm/book3s/32/pgtable.h
index a090cb13a4a0..afd672e84791 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -516,6 +516,10 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
return __pte((pte_val(pte) & _PAGE_CHG_MASK) | pgprot_val(newprot));
}
+static inline bool pmd_user(pmd_t pmd)
+{
+ return 0;
+}
/* This low level function performs the actual PTE insertion
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 8bbd3e1df93e..93601ef4c665 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -538,6 +538,16 @@ static inline bool pte_user(pte_t pte)
return !(pte_raw(pte) & cpu_to_be64(_PAGE_PRIVILEGED));
}
+static inline bool pmd_user(pmd_t pmd)
+{
+ return !(pmd_raw(pmd) & cpu_to_be64(_PAGE_PRIVILEGED));
+}
+
+static inline bool pud_user(pud_t pud)
+{
+ return !(pud_raw(pud) & cpu_to_be64(_PAGE_PRIVILEGED));
+}
+
#define pte_access_permitted pte_access_permitted
static inline bool pte_access_permitted(pte_t pte, bool write)
{
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h
b/arch/powerpc/include/asm/nohash/32/pgtable.h
index 70edad44dff6..d953533c56ff 100644
--- a/arch/powerpc/include/asm/nohash/32/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
@@ -209,6 +209,11 @@ static inline void pmd_clear(pmd_t *pmdp)
*pmdp = __pmd(0);
}
+static inline bool pmd_user(pmd_t pmd)
+{
+ return false;
+}
+
/*
* PTE updates. This function is called whenever an existing
* valid PTE is updated. This does -not- include set_pte_at()
diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h
b/arch/powerpc/include/asm/nohash/64/pgtable.h
index d391a45e0f11..14e69ebad31f 100644
--- a/arch/powerpc/include/asm/nohash/64/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/64/pgtable.h
@@ -123,6 +123,11 @@ static inline pte_t pmd_pte(pmd_t pmd)
return __pte(pmd_val(pmd));
}
+static inline bool pmd_user(pmd_t pmd)
+{
+ return (pmd_val(pmd) & _PAGE_USER) == _PAGE_USER;
+}
+
#define pmd_none(pmd) (!pmd_val(pmd))
#definepmd_bad(pmd)(!is_kernel_addr(pmd_val(pmd)) \
|| (pmd_val(pmd) & PMD_BAD_BITS))
@@ -164,6 +169,11 @@ static inline pte_t pud_pte(pud_t pud)
return __pte(pud_val(pud));
}
+static inline bool pud_user(pud_t pud)
+{
+ return (pud_val(pud) & _PAGE_USER) == _PAGE_USER;
+}
+
static inline pud_t pte_pud(pte_t pte)
{
return __pud(pte_val(pte));
diff --git a/arch/powerpc/include/asm/pgtable.h
b/arch/powerpc/include/asm/pgtable.h
index ad0829f816e9..b76fdb80b6c9 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -167,6 +167,21 @@ static inline int pud_pfn(pud_t pud)
}
#endif
+static inline bool pte_user_accessible_page(pte_t pte)
+{
+ return pte_present(pte) && pte_user(pte);
+}
+
+static inline bool pmd_user_accessible_page(pmd_t pmd)
+{
+ return pmd_leaf(pmd) && pmd_present(pmd) && pmd_user(pmd);
+}
+
+static inline bool pud_user_accessible_page(pud_t pud)
+{
+ return pud_leaf(pud) && pud_present(pud) && pud_user(pud);
+}
+
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_PGTABLE_H */
--
2.37.2
[PATCH v6 4/7] powerpc: mm: Implement p{m,u,4}d_leaf on all platforms
The check that a higher-level entry in multi-level pages contains a page
translation entry (pte) is performed by p{m,u,4}d_leaf stubs, which may
be specialised for each choice of mmu. In a prior commit, we replace
uses to the catch-all stubs, p{m,u,4}d_is_leaf with p{m,u,4}d_leaf.
Replace the catch-all stub definitions for p{m,u,4}d_is_leaf with
definitions for p{m,u,4}d_leaf. A future patch will assume that
p{m,u,4}d_leaf is defined on all platforms.
In particular, implement pud_leaf for Book3E-64, pmd_leaf for all Book3E
and Book3S-64 platforms, with a catch-all definition for p4d_leaf.
Signed-off-by: Rohan McLure
---
v5: Split patch that replaces p{m,u,4}d_is_leaf into two patches, first
replacing callsites and afterward providing generic definition.
Remove ifndef-defines implementing p{m,u}d_leaf in favour of
implementing stubs in headers belonging to the particular platforms
needing them.
---
arch/powerpc/include/asm/book3s/32/pgtable.h | 5 +
arch/powerpc/include/asm/book3s/64/pgtable.h | 10 -
arch/powerpc/include/asm/nohash/64/pgtable.h | 6 ++
arch/powerpc/include/asm/nohash/pgtable.h| 6 ++
arch/powerpc/include/asm/pgtable.h | 22 ++--
5 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h
b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 75823f39e042..a090cb13a4a0 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -242,6 +242,11 @@ static inline void pmd_clear(pmd_t *pmdp)
*pmdp = __pmd(0);
}
+#define pmd_leaf pmd_leaf
+static inline bool pmd_leaf(pmd_t pmd)
+{
+ return false;
+}
/*
* When flushing the tlb entry for a page, we also need to flush the hash
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 9d8b4e25f5ed..5be0a4c8bf32 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1362,16 +1362,14 @@ static inline bool is_pte_rw_upgrade(unsigned long
old_val, unsigned long new_va
/*
* Like pmd_huge() and pmd_large(), but works regardless of config options
*/
-#define pmd_is_leaf pmd_is_leaf
-#define pmd_leaf pmd_is_leaf
-static inline bool pmd_is_leaf(pmd_t pmd)
+#define pmd_leaf pmd_leaf
+static inline bool pmd_leaf(pmd_t pmd)
{
return !!(pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE));
}
-#define pud_is_leaf pud_is_leaf
-#define pud_leaf pud_is_leaf
-static inline bool pud_is_leaf(pud_t pud)
+#define pud_leaf pud_leaf
+static inline bool pud_leaf(pud_t pud)
{
return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PTE));
}
diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h
b/arch/powerpc/include/asm/nohash/64/pgtable.h
index 879e9a6e5a87..d391a45e0f11 100644
--- a/arch/powerpc/include/asm/nohash/64/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/64/pgtable.h
@@ -141,6 +141,12 @@ static inline void pud_clear(pud_t *pudp)
*pudp = __pud(0);
}
+#define pud_leaf pud_leaf
+static inline bool pud_leaf(pud_t pud)
+{
+ return false;
+}
+
#define pud_none(pud) (!pud_val(pud))
#definepud_bad(pud)(!is_kernel_addr(pud_val(pud)) \
|| (pud_val(pud) & PUD_BAD_BITS))
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h
b/arch/powerpc/include/asm/nohash/pgtable.h
index ac3e69a18253..cc3941a4790f 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -60,6 +60,12 @@ static inline bool pte_hw_valid(pte_t pte)
return pte_val(pte) & _PAGE_PRESENT;
}
+#define pmd_leaf pmd_leaf
+static inline bool pmd_leaf(pmd_t pmd)
+{
+ return false;
+}
+
/*
* Don't just check for any non zero bits in __PAGE_USER, since for book3e
* and PTE_64BIT, PAGE_KERNEL_X contains _PAGE_BAP_SR which is also in
diff --git a/arch/powerpc/include/asm/pgtable.h
b/arch/powerpc/include/asm/pgtable.h
index 17d30359d1f4..284408829fa3 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -128,29 +128,11 @@ static inline void pte_frag_set(mm_context_t *ctx, void
*p)
}
#endif
-#ifndef pmd_is_leaf
-#define pmd_is_leaf pmd_is_leaf
-static inline bool pmd_is_leaf(pmd_t pmd)
+#define p4d_leaf p4d_leaf
+static inline bool p4d_leaf(p4d_t p4d)
{
return false;
}
-#endif
-
-#ifndef pud_is_leaf
-#define pud_is_leaf pud_is_leaf
-static inline bool pud_is_leaf(pud_t pud)
-{
- return false;
-}
-#endif
-
-#ifndef p4d_is_leaf
-#define p4d_is_leaf p4d_is_leaf
-static inline bool p4d_is_leaf(p4d_t p4d)
-{
- return false;
-}
-#endif
#define pmd_pgtable pmd_pgtable
static inline pgtable_t pmd_pgtable(pmd_t pmd)
--
2.37.2
[PATCH v6 3/7] powerpc: mm: Replace p{u,m,4}d_is_leaf with p{u,m,4}_leaf
Replace occurrences of p{u,m,4}d_is_leaf with p{u,m,4}_leaf, as the
latter is the name given to checking that a higher-level entry in
multi-level paging contains a page translation entry (pte) throughout
all other archs.
A future patch will implement p{u,m,4}_leaf stubs on all platforms so
that they may be referenced in generic code.
Signed-off-by: Rohan McLure
---
V4: New patch
V5: Previously replaced stub definition for *_is_leaf with *_leaf. Do
that in a later patch
---
arch/powerpc/kvm/book3s_64_mmu_radix.c | 12 ++--
arch/powerpc/mm/book3s64/radix_pgtable.c | 14 +++---
arch/powerpc/mm/pgtable.c| 6 +++---
arch/powerpc/mm/pgtable_64.c | 6 +++---
arch/powerpc/xmon/xmon.c | 6 +++---
5 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c
b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index 9d3743ca16d5..0d24fd984d16 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -497,7 +497,7 @@ static void kvmppc_unmap_free_pmd(struct kvm *kvm, pmd_t
*pmd, bool full,
for (im = 0; im < PTRS_PER_PMD; ++im, ++p) {
if (!pmd_present(*p))
continue;
- if (pmd_is_leaf(*p)) {
+ if (pmd_leaf(*p)) {
if (full) {
pmd_clear(p);
} else {
@@ -526,7 +526,7 @@ static void kvmppc_unmap_free_pud(struct kvm *kvm, pud_t
*pud,
for (iu = 0; iu < PTRS_PER_PUD; ++iu, ++p) {
if (!pud_present(*p))
continue;
- if (pud_is_leaf(*p)) {
+ if (pud_leaf(*p)) {
pud_clear(p);
} else {
pmd_t *pmd;
@@ -629,12 +629,12 @@ int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable,
pte_t pte,
new_pud = pud_alloc_one(kvm->mm, gpa);
pmd = NULL;
- if (pud && pud_present(*pud) && !pud_is_leaf(*pud))
+ if (pud && pud_present(*pud) && !pud_leaf(*pud))
pmd = pmd_offset(pud, gpa);
else if (level <= 1)
new_pmd = kvmppc_pmd_alloc();
- if (level == 0 && !(pmd && pmd_present(*pmd) && !pmd_is_leaf(*pmd)))
+ if (level == 0 && !(pmd && pmd_present(*pmd) && !pmd_leaf(*pmd)))
new_ptep = kvmppc_pte_alloc();
/* Check if we might have been invalidated; let the guest retry if so */
@@ -652,7 +652,7 @@ int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable,
pte_t pte,
new_pud = NULL;
}
pud = pud_offset(p4d, gpa);
- if (pud_is_leaf(*pud)) {
+ if (pud_leaf(*pud)) {
unsigned long hgpa = gpa & PUD_MASK;
/* Check if we raced and someone else has set the same thing */
@@ -703,7 +703,7 @@ int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable,
pte_t pte,
new_pmd = NULL;
}
pmd = pmd_offset(pud, gpa);
- if (pmd_is_leaf(*pmd)) {
+ if (pmd_leaf(*pmd)) {
unsigned long lgpa = gpa & PMD_MASK;
/* Check if we raced and someone else has set the same thing */
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c
b/arch/powerpc/mm/book3s64/radix_pgtable.c
index 26245aaf12b8..4e46e001c3c3 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -205,14 +205,14 @@ static void radix__change_memory_range(unsigned long
start, unsigned long end,
pudp = pud_alloc(_mm, p4dp, idx);
if (!pudp)
continue;
- if (pud_is_leaf(*pudp)) {
+ if (pud_leaf(*pudp)) {
ptep = (pte_t *)pudp;
goto update_the_pte;
}
pmdp = pmd_alloc(_mm, pudp, idx);
if (!pmdp)
continue;
- if (pmd_is_leaf(*pmdp)) {
+ if (pmd_leaf(*pmdp)) {
ptep = pmdp_ptep(pmdp);
goto update_the_pte;
}
@@ -786,7 +786,7 @@ static void __meminit remove_pmd_table(pmd_t *pmd_start,
unsigned long addr,
if (!pmd_present(*pmd))
continue;
- if (pmd_is_leaf(*pmd)) {
+ if (pmd_leaf(*pmd)) {
if (!IS_ALIGNED(addr, PMD_SIZE) ||
!IS_ALIGNED(next, PMD_SIZE)) {
WARN_ONCE(1, "%s: unaligned range\n", __func__);
@@ -816,7 +816,7 @@ static void __meminit remove_pud_table(pud_t *pud_start,
unsigned long addr,
if (!pud_present(*pud))
continue;
- if (pud_is_leaf(*pud)) {
+ if (pud_leaf(*pud)) {
if (!IS_ALIGNED(addr, PUD_SIZE) ||
!IS_ALIGNED(next,
[PATCH v6 2/7] powerpc/64s: mm: Introduce __pmdp_collapse_flush with mm_struct argument
pmdp_collapse_flush has references in generic code with just three
parameters, due to the choice of mm context being implied by the vm_area
context parameter.
Define __pmdp_collapse_flush to accept an additional mm_struct *
parameter, with pmdp_collapse_flush a macro that unpacks the vma and
calls __pmdp_collapse_flush. The mm_struct * parameter is needed in a
future patch providing Page Table Check support, which is defined in
terms of mm context objects.
Signed-off-by: Rohan McLure
---
v6: New patch
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index cb4c67bf45d7..9d8b4e25f5ed 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1244,14 +1244,22 @@ static inline pmd_t pmdp_huge_get_and_clear(struct
mm_struct *mm,
return hash__pmdp_huge_get_and_clear(mm, addr, pmdp);
}
-static inline pmd_t pmdp_collapse_flush(struct vm_area_struct *vma,
- unsigned long address, pmd_t *pmdp)
+static inline pmd_t __pmdp_collapse_flush(struct vm_area_struct *vma, struct
mm_struct *mm,
+ unsigned long address, pmd_t *pmdp)
{
if (radix_enabled())
return radix__pmdp_collapse_flush(vma, address, pmdp);
return hash__pmdp_collapse_flush(vma, address, pmdp);
}
-#define pmdp_collapse_flush pmdp_collapse_flush
+#define pmdp_collapse_flush(vma, addr, pmdp) \
+({ \
+ struct vm_area_struct *_vma = (vma);\
+ pmd_t _r; \
+ \
+ _r = __pmdp_collapse_flush(_vma, _vma->vm_mm, (addr), (pmdp)); \
+ \
+ _r; \
+})
#define __HAVE_ARCH_PMDP_HUGE_GET_AND_CLEAR_FULL
pmd_t pmdp_huge_get_and_clear_full(struct vm_area_struct *vma,
--
2.37.2
[PATCH v6 1/7] powerpc: mm: Separate set_pte, set_pte_at for internal, external use
Produce separate symbols for set_pte, which is to be used in
arch/powerpc for reassignment of pte's, and set_pte_at, used in generic
code.
The reason for this distinction is to support the Page Table Check
sanitiser. Having this distinction allows for set_pte_at to
instrumented, but set_pte not to be, permitting for uninstrumented
internal mappings. This distinction in names is also present in x86.
Signed-off-by: Rohan McLure
---
v6: new patch
---
arch/powerpc/include/asm/book3s/pgtable.h | 4 ++--
arch/powerpc/include/asm/nohash/pgtable.h | 4 ++--
arch/powerpc/include/asm/pgtable.h| 1 +
arch/powerpc/mm/pgtable.c | 4 ++--
4 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/pgtable.h
b/arch/powerpc/include/asm/book3s/pgtable.h
index d18b748ea3ae..dbcdc2103c59 100644
--- a/arch/powerpc/include/asm/book3s/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/pgtable.h
@@ -12,8 +12,8 @@
/* Insert a PTE, top-level function is out of line. It uses an inline
* low level function in the respective pgtable-* files
*/
-extern void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
- pte_t pte);
+extern void set_pte(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
+ pte_t pte);
#define __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h
b/arch/powerpc/include/asm/nohash/pgtable.h
index 69c3a050a3d8..ac3e69a18253 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -154,8 +154,8 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
/* Insert a PTE, top-level function is out of line. It uses an inline
* low level function in the respective pgtable-* files
*/
-extern void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
- pte_t pte);
+extern void set_pte(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
+ pte_t pte);
/* This low level function performs the actual PTE insertion
* Setting the PTE depends on the MMU type and other factors. It's
diff --git a/arch/powerpc/include/asm/pgtable.h
b/arch/powerpc/include/asm/pgtable.h
index 9972626ddaf6..17d30359d1f4 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -48,6 +48,7 @@ struct mm_struct;
/* 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))
+#define set_pte_at set_pte
/*
* Select all bits except the pfn
*/
diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
index cb2dcdb18f8e..e9a464e0d081 100644
--- a/arch/powerpc/mm/pgtable.c
+++ b/arch/powerpc/mm/pgtable.c
@@ -187,8 +187,8 @@ static pte_t set_access_flags_filter(pte_t pte, struct
vm_area_struct *vma,
/*
* set_pte stores a linux PTE into the linux page table.
*/
-void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
- pte_t pte)
+void set_pte(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
+pte_t pte)
{
/*
* Make sure hardware valid bit is not set. We don't do
--
2.37.2
[PATCH v6 0/7] Support page table check
Support the page table check sanitiser on all PowerPC platforms. This
sanitiser works by serialising assignments, reassignments and clears of
page table entries at each level in order to ensure that anonymous
mappings have at most one writable consumer, and likewise that
file-backed mappings are not simultaneously also anonymous mappings.
In order to support this infrastructure, a number of stubs must be
defined for all powerpc platforms. Additionally, seperate set_pte_at
and set_pte, to allow for internal, uninstrumented mappings.
v6:
* Support huge pages and p{m,u}d accounting.
* Remove instrumentation from set_pte from kernel internal pages.
* 64s: Implement pmdp_collapse_flush in terms of __pmdp_collapse_flush
as access to the mm_struct * is required.
v5:
Link:
Rohan McLure (7):
powerpc: mm: Separate set_pte, set_pte_at for internal, external use
powerpc/64s: mm: Introduce __pmdp_collapse_flush with mm_struct
argument
powerpc: mm: Replace p{u,m,4}d_is_leaf with p{u,m,4}_leaf
powerpc: mm: Implement p{m,u,4}d_leaf on all platforms
powerpc: mm: Add common pud_pfn stub for all platforms
powerpc: mm: Add p{te,md,ud}_user_accessible_page helpers
powerpc: mm: Support page table check
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/book3s/32/pgtable.h | 17 +++-
arch/powerpc/include/asm/book3s/64/pgtable.h | 88 +---
arch/powerpc/include/asm/book3s/pgtable.h| 4 +-
arch/powerpc/include/asm/nohash/32/pgtable.h | 12 ++-
arch/powerpc/include/asm/nohash/64/pgtable.h | 24 +-
arch/powerpc/include/asm/nohash/pgtable.h| 10 ++-
arch/powerpc/include/asm/pgtable.h | 61 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 12 +--
arch/powerpc/mm/book3s64/hash_pgtable.c | 2 +-
arch/powerpc/mm/book3s64/pgtable.c | 16 ++--
arch/powerpc/mm/book3s64/radix_pgtable.c | 24 +++---
arch/powerpc/mm/nohash/book3e_pgtable.c | 2 +-
arch/powerpc/mm/pgtable.c| 10 +--
arch/powerpc/mm/pgtable_32.c | 2 +-
arch/powerpc/mm/pgtable_64.c | 6 +-
arch/powerpc/xmon/xmon.c | 6 +-
17 files changed, 204 insertions(+), 93 deletions(-)
--
2.37.2
Re: Build regressions/improvements in v6.2-rc8
Geert Uytterhoeven writes:
> On Mon, 13 Feb 2023, Geert Uytterhoeven wrote:
>> JFYI, when comparing v6.2-rc8[1] to v6.2-rc7[3], the summaries are:
>> - build errors: +11/-1
>
>+ {standard input}: Error: unrecognized opcode: `dcbfl': => 5736, 4743,
> 4327, 4476, 4447, 5067, 4602, 5212, 5224, 4298, 5594, 4315, 5050, 5195, 4464,
> 5079
>+ {standard input}: Error: unrecognized opcode: `dlmzb.': => 2848, 18800,
> 2842, 2383, 106, 2377, 3327, 112
>+ {standard input}: Error: unrecognized opcode: `iccci': => 204, 163, 510
>+ {standard input}: Error: unrecognized opcode: `lbarx': => 570, 196
>+ {standard input}: Error: unrecognized opcode: `mbar': => 887, 558,
> 1172, 539, 516, 837, 1457, 1125, 815, 7523, 1100, 1385, 368, 703, 662, 468,
> 441, 1410
>+ {standard input}: Error: unrecognized opcode: `mfdcr': => 3589, 4358,
> 3565, 3493, 3614, 128, 3445, 276, 3518, 3541, 3469, 4413
>+ {standard input}: Error: unrecognized opcode: `mtdcr': => 265, 4402,
> 4430, 4375, 4388, 4347, 117, 4443
>+ {standard input}: Error: unrecognized opcode: `stbcx.': => 196, 570
>+ {standard input}: Error: unrecognized opcode: `tlbwe': => 475, 476, 477
>
> powerpc-gcc11/ppc64_book3e_allmodconfig
> powerpc-gcc11/powerpc-allmodconfig
> powerpc-gcc11/corenet64_smp_defconfig
> powerpc-gcc11/powerpc-allyesconfig
> powerpc-gcc11/44x/fsp2_defconfig
That was me updating the GCC 11 toolchain from 11.1 to 11.3 (and more
importantly binutils 2.36 to 2.38), so not an actual regression.
I've backed that change out for now for powerpc builds.
cheers
Re: [PATCH AUTOSEL 6.1 17/38] powerpc/85xx: Fix unannotated intra-function call warning
On Fri, Feb 10, 2023 at 04:55:54PM +0530, Sathvika Vasireddy wrote: Hi Sasha, On 09/02/23 16:44, Sasha Levin wrote: From: Sathvika Vasireddy [ Upstream commit 8afffce6aa3bddc940ac1909627ff1e772b6cbf1 ] objtool throws the following warning: arch/powerpc/kernel/head_85xx.o: warning: objtool: .head.text+0x1a6c: unannotated intra-function call Fix the warning by annotating KernelSPE symbol with SYM_FUNC_START_LOCAL and SYM_FUNC_END macros. Reported-by: kernel test robot Signed-off-by: Sathvika Vasireddy Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/20230128124138.1066176-1...@linux.ibm.com Signed-off-by: Sasha Levin --- arch/powerpc/kernel/head_85xx.S | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/head_85xx.S b/arch/powerpc/kernel/head_85xx.S index 52c0ab416326a..d3939849f4550 100644 --- a/arch/powerpc/kernel/head_85xx.S +++ b/arch/powerpc/kernel/head_85xx.S @@ -862,7 +862,7 @@ _GLOBAL(load_up_spe) * SPE unavailable trap from kernel - print a message, but let * the task use SPE in the kernel until it returns to user mode. */ -KernelSPE: +SYM_FUNC_START_LOCAL(KernelSPE) lwz r3,_MSR(r1) orisr3,r3,MSR_SPE@h stw r3,_MSR(r1) /* enable use of SPE after return */ @@ -879,6 +879,7 @@ KernelSPE: #endif .align 4,0 +SYM_FUNC_END(KernelSPE) #endif /* CONFIG_SPE */ /* Please drop this patch because objtool enablement patches for powerpc are not a part of kernel v6.1. Ack, I'll drop this and the other one you've pointed out. Thanks! -- Thanks, Sasha
RE: [PATCH RFC] PCI/AER: Enable internal AER errors by default
From: Bjorn Helgaas > Sent: 13 February 2023 21:38 > > On Fri, Feb 10, 2023 at 02:33:23PM -0800, Ira Weiny wrote: > > The CXL driver expects internal error reporting to be enabled via > > pci_enable_pcie_error_reporting(). It is likely other drivers expect the > > same. > > Dave submitted a patch to enable the CXL side[1] but the PCI AER registers > > still mask errors. > > > > PCIe v6.0 Uncorrectable Mask Register (7.8.4.3) and Correctable Mask > > Register (7.8.4.6) default to masking internal errors. The > > Uncorrectable Error Severity Register (7.8.4.4) defaults internal errors > > as fatal. > > > > Enable internal errors to be reported via the standard > > pci_enable_pcie_error_reporting() call. Ensure uncorrectable errors are set > > non-fatal to limit any impact to other drivers. > > Do you have any background on why the spec makes these errors masked > by default? I'm sympathetic to wanting to learn about all the errors > we can, but I'm a little wary if the spec authors thought it was > important to mask these by default. I'd guess that it is for backwards compatibility with older hardware and/or software that that didn't support error notifications. Then there are the x86 systems that manage to take the AER error into some 'board management hardware' which finally interrupts the kernel with an NMI - and the obvious consequence. These systems are NEBS? 'qualified' for telecoms use, but take out a PCIe link and the system crashes. It is pretty easy to generate a PCIe error. Any endpoint with two (or more) different sized BARs leaves a big chunk of PCIe address space that is forwarded by the upstream bridge but is not responded to. The requirement to put the MSI-X area in its own BAR pretty much ensures that such addresses exist. (Never mind reprogramming the fpga that is terminating the link.) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH RFC] PCI/AER: Enable internal AER errors by default
On Fri, Feb 10, 2023 at 02:33:23PM -0800, Ira Weiny wrote:
> The CXL driver expects internal error reporting to be enabled via
> pci_enable_pcie_error_reporting(). It is likely other drivers expect the
> same.
> Dave submitted a patch to enable the CXL side[1] but the PCI AER registers
> still mask errors.
>
> PCIe v6.0 Uncorrectable Mask Register (7.8.4.3) and Correctable Mask
> Register (7.8.4.6) default to masking internal errors. The
> Uncorrectable Error Severity Register (7.8.4.4) defaults internal errors
> as fatal.
>
> Enable internal errors to be reported via the standard
> pci_enable_pcie_error_reporting() call. Ensure uncorrectable errors are set
> non-fatal to limit any impact to other drivers.
Do you have any background on why the spec makes these errors masked
by default? I'm sympathetic to wanting to learn about all the errors
we can, but I'm a little wary if the spec authors thought it was
important to mask these by default.
> [1]
> https://lore.kernel.org/all/167604864163.2392965.5102660329807283871.stgit@djiang5-mobl3.local/
>
> Cc: Bjorn Helgaas
> Cc: Jonathan Cameron
> Cc: Dan Williams
> Cc: Dave Jiang
> Cc: Stefan Roese
> Cc: "Kuppuswamy Sathyanarayanan"
> Cc: Mahesh J Salgaonkar
> Cc: Oliver O'Halloran
> Cc: linux-...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Ira Weiny
> ---
> This is RFC to see if it is acceptable to be part of the standard
> pci_enable_pcie_error_reporting() call or perhaps a separate pci core
> call should be introduced. It is anticipated that enabling this error
> reporting is what existing drivers are expecting. The errors are marked
> non-fatal therefore it should not adversely affect existing devices.
> ---
> drivers/pci/pcie/aer.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 625f7b2cafe4..9d3ed3a5fc23 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -229,11 +229,28 @@ int pcie_aer_is_native(struct pci_dev *dev)
>
> int pci_enable_pcie_error_reporting(struct pci_dev *dev)
> {
> + int pos_cap_err;
> + u32 reg;
> int rc;
>
> if (!pcie_aer_is_native(dev))
> return -EIO;
>
> + pos_cap_err = dev->aer_cap;
> +
> + /* Unmask correctable and uncorrectable (non-fatal) internal errors */
> + pci_read_config_dword(dev, pos_cap_err + PCI_ERR_COR_MASK, );
> + reg &= ~PCI_ERR_COR_INTERNAL;
> + pci_write_config_dword(dev, pos_cap_err + PCI_ERR_COR_MASK, reg);
> +
> + pci_read_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_SEVER, );
> + reg &= ~PCI_ERR_UNC_INTN;
> + pci_write_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_SEVER, reg);
> +
> + pci_read_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_MASK, );
> + reg &= ~PCI_ERR_UNC_INTN;
> + pci_write_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_MASK, reg);
> +
> rc = pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS);
> return pcibios_err_to_errno(rc);
> }
>
> ---
> base-commit: e5ab7f206ffc873160bd0f1a52cae17ab692a9d1
> change-id: 20230209-cxl-pci-aer-18dda61c8239
>
> Best regards,
> --
> Ira Weiny
>
Re: [PATCH v2] powerpc/machdep: warn when machine_is() used too early
Christophe Leroy writes:
> Le 13/02/2023 à 20:23, Nathan Lynch via B4 Submission Endpoint a écrit :
>> From: Nathan Lynch
>>
>> machine_is() can't provide correct results before probe_machine() has
>> run. Warn when it's used too early in boot, placing the WARN_ON() in a
>> helper function so the reported file:line indicates exactly what went
>> wrong.
>>
>> checkpatch complains about __attribute__((weak)) in the patch, so
>> change that to __weak, and align the line continuations as well.
>>
>> Signed-off-by: Nathan Lynch
>
> Reviewed-by: Christophe Leroy
thanks!
>> @@ -220,11 +221,16 @@ extern struct machdep_calls *machine_id;
>> EXPORT_SYMBOL(mach_##name); \
>> struct machdep_calls mach_##name __machine_desc =
>>
>> -#define machine_is(name) \
>> -({ \
>> -extern struct machdep_calls mach_##name \
>> -__attribute__((weak)); \
>> -machine_id == _##name; \
>> +static inline bool __machine_is(const struct machdep_calls *md)
>> +{
>> +WARN_ON(!machine_id); // complain if used before probe_machine()
>> +return machine_id == md;
>> +}
>> +
>> +#define machine_is(name)\
>
> Misaligned back-slash ?
An artifact of the patch format + tabs for indentation. It should be
aligned with the rest when looking at the file itself after the patch is
applied.
>
>> +({ \
>> +extern struct machdep_calls mach_##name __weak; \
>> +__machine_is(_##name); \
>> })
>>
>> static inline void log_error(char *buf, unsigned int err_type, int fatal)
Re: [PATCH v2 6/8] powerpc/85xx: p2020: Define just one machine description
On Monday 13 February 2023 20:08:52 Christophe Leroy wrote:
> Le 24/12/2022 à 22:14, Pali Rohár a écrit :
> > Combine machine descriptions and code of all P2020 boards into just one
> > generic unified P2020 machine description. This allows kernel to boot on
> > any P2020-based board with P2020 DTS file without need to patch kernel and
> > define a new machine description in 85xx powerpc platform directory.
> >
> > Signed-off-by: Pali Rohár
> > ---
> > arch/powerpc/platforms/85xx/p2020.c | 83 +++--
> > 1 file changed, 19 insertions(+), 64 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/85xx/p2020.c
> > b/arch/powerpc/platforms/85xx/p2020.c
> > index adf3750abef9..b3fb600e1d83 100644
> > --- a/arch/powerpc/platforms/85xx/p2020.c
> > +++ b/arch/powerpc/platforms/85xx/p2020.c
> > @@ -156,83 +156,38 @@ static void __init p2020_setup_arch(void)
> > #endif
> > }
> >
> > -#ifdef CONFIG_MPC85xx_DS
> > -machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
> > -#endif /* CONFIG_MPC85xx_DS */
> > -
> > -#ifdef CONFIG_MPC85xx_RDB
> > -machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
> > -machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
> > -#endif /* CONFIG_MPC85xx_RDB */
> > +machine_arch_initcall(p2020, mpc85xx_common_publish_devices);
> >
> > /*
> >* Called very early, device-tree isn't unflattened
> >*/
> > -#ifdef CONFIG_MPC85xx_DS
> > -static int __init p2020_ds_probe(void)
> > -{
> > - return !!of_machine_is_compatible("fsl,P2020DS");
> > -}
> > -#endif /* CONFIG_MPC85xx_DS */
> > -
> > -#ifdef CONFIG_MPC85xx_RDB
> > -static int __init p2020_rdb_probe(void)
> > -{
> > - if (of_machine_is_compatible("fsl,P2020RDB"))
> > - return 1;
> > - return 0;
> > -}
> > -
> > -static int __init p2020_rdb_pc_probe(void)
> > +static int __init p2020_probe(void)
> > {
> > - if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> > - return 1;
> > - return 0;
> > + struct device_node *p2020_cpu;
> > +
> > + /*
> > +* There is no common compatible string for all P2020 boards.
> > +* The only common thing is "PowerPC,P2020@0" cpu node.
> > +* So check for P2020 board via this cpu node.
> > +*/
> > + p2020_cpu = of_find_node_by_path("/cpus/PowerPC,P2020@0");
> > + if (!p2020_cpu)
> > + return 0;
> > +
> > + of_node_put(p2020_cpu);
>
> of_node_put() already checks for nullity of its parameter, so you can
> simplify stuff here, something like
>
> p2020_cpu = of_find_node_by_path("/cpus/PowerPC,P2020@0");
> of_node_put(p2020_cpu);
>
> return !!p2020_cpu;
Ok!
> > + return 1;
> > }
> > -#endif /* CONFIG_MPC85xx_RDB */
> > -
> > -#ifdef CONFIG_MPC85xx_DS
> > -define_machine(p2020_ds) {
> > - .name = "P2020 DS",
> > - .probe = p2020_ds_probe,
> > - .setup_arch = p2020_setup_arch,
> > - .init_IRQ = p2020_pic_init,
> > -#ifdef CONFIG_PCI
> > - .pcibios_fixup_bus = fsl_pcibios_fixup_bus,
> > - .pcibios_fixup_phb = fsl_pcibios_fixup_phb,
> > -#endif
> > - .get_irq= mpic_get_irq,
> > - .calibrate_decr = generic_calibrate_decr,
> > - .progress = udbg_progress,
> > -};
> > -#endif /* CONFIG_MPC85xx_DS */
> > -
> > -#ifdef CONFIG_MPC85xx_RDB
> > -define_machine(p2020_rdb) {
> > - .name = "P2020 RDB",
> > - .probe = p2020_rdb_probe,
> > - .setup_arch = p2020_setup_arch,
> > - .init_IRQ = p2020_pic_init,
> > -#ifdef CONFIG_PCI
> > - .pcibios_fixup_bus = fsl_pcibios_fixup_bus,
> > - .pcibios_fixup_phb = fsl_pcibios_fixup_phb,
> > -#endif
> > - .get_irq= mpic_get_irq,
> > - .calibrate_decr = generic_calibrate_decr,
> > - .progress = udbg_progress,
> > -};
> >
> > -define_machine(p2020_rdb_pc) {
> > - .name = "P2020RDB-PC",
> > - .probe = p2020_rdb_pc_probe,
> > +define_machine(p2020) {
> > + .name = "Freescale P2020",
> > + .probe = p2020_probe,
> > .setup_arch = p2020_setup_arch,
> > .init_IRQ = p2020_pic_init,
> > #ifdef CONFIG_PCI
> > .pcibios_fixup_bus = fsl_pcibios_fixup_bus,
> > - .pcibios_fixup_phb = fsl_pcibios_fixup_phb,
> > + .pcibios_fixup_phb = fsl_pcibios_fixup_phb,
> > #endif
> > .get_irq= mpic_get_irq,
> > .calibrate_decr = generic_calibrate_decr,
> > .progress = udbg_progress,
> > };
> > -#endif /* CONFIG_MPC85xx_RDB */
Re: [PATCH v2 4/8] powerpc/85xx: p2020: Move i8259 code into own function
On Monday 13 February 2023 20:06:27 Christophe Leroy wrote:
>
>
> Le 24/12/2022 à 22:14, Pali Rohár a écrit :
> > Splits mpic and i8259 initialization codes into separate functions.
> >
> > Signed-off-by: Pali Rohár
> > ---
> > arch/powerpc/platforms/85xx/p2020.c | 37 -
> > 1 file changed, 21 insertions(+), 16 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/85xx/p2020.c
> > b/arch/powerpc/platforms/85xx/p2020.c
> > index d65d4c88ac47..b8584bf307b0 100644
> > --- a/arch/powerpc/platforms/85xx/p2020.c
> > +++ b/arch/powerpc/platforms/85xx/p2020.c
> > @@ -45,6 +45,7 @@
> > #ifdef CONFIG_MPC85xx_DS
> >
> > #ifdef CONFIG_PPC_I8259
> > +
> > static void mpc85xx_8259_cascade(struct irq_desc *desc)
> > {
> > struct irq_chip *chip = irq_desc_get_chip(desc);
> > @@ -55,27 +56,13 @@ static void mpc85xx_8259_cascade(struct irq_desc *desc)
> > }
> > chip->irq_eoi(>irq_data);
> > }
> > -#endif /* CONFIG_PPC_I8259 */
> >
> > -static void __init mpc85xx_ds_pic_init(void)
> > +static void __init mpc85xx_8259_init(void)
> > {
> > - struct mpic *mpic;
> > -#ifdef CONFIG_PPC_I8259
> > struct device_node *np;
> > struct device_node *cascade_node = NULL;
> > int cascade_irq;
> > -#endif
> > -
> > - mpic = mpic_alloc(NULL, 0,
> > - MPIC_BIG_ENDIAN |
> > - MPIC_SINGLE_DEST_CPU,
> > - 0, 256, " OpenPIC ");
> > -
> > - BUG_ON(mpic == NULL);
> > - mpic_init(mpic);
> >
> > -#ifdef CONFIG_PPC_I8259
> > - /* Initialize the i8259 controller */
> > for_each_node_by_type(np, "interrupt-controller")
> > if (of_device_is_compatible(np, "chrp,iic")) {
> > cascade_node = np;
> > @@ -93,13 +80,31 @@ static void __init mpc85xx_ds_pic_init(void)
> > return;
> > }
> >
> > - DBG("mpc85xxds: cascade mapped to irq %d\n", cascade_irq);
> > + DBG("i8259: cascade mapped to irq %d\n", cascade_irq);
> >
> > i8259_init(cascade_node, 0);
> > of_node_put(cascade_node);
> >
> > irq_set_chained_handler(cascade_irq, mpc85xx_8259_cascade);
> > +}
> > +
> > #endif/* CONFIG_PPC_I8259 */
> > +
> > +static void __init mpc85xx_ds_pic_init(void)
> > +{
> > + struct mpic *mpic;
> > +
> > + mpic = mpic_alloc(NULL, 0,
> > + MPIC_BIG_ENDIAN |
> > + MPIC_SINGLE_DEST_CPU,
> > + 0, 256, " OpenPIC ");
> > +
> > + BUG_ON(mpic == NULL);
> > + mpic_init(mpic);
> > +
> > +#ifdef CONFIG_PPC_I8259
>
> Ca we minimise number of #ifdef CONFIG_PPC_I8259 by using
> IS_ENABLED(CONFIG_PPC_I8259) inside if/else ?
>
> > + mpc85xx_8259_init();
> > +#endif
Ok, I can change code to:
+if (IS_ENABLED(CONFIG_PPC_I8259))
+mpc85xx_8259_init();
I guess it should be equivalent.
> > }
> >
> > #ifdef CONFIG_PCI
Re: [PATCH v2 3/8] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
On Monday 13 February 2023 20:05:09 Christophe Leroy wrote:
> Le 24/12/2022 à 22:14, Pali Rohár a écrit :
> > This moves machine descriptions and all related code for all P2020 boards
> > into new p2020.c source file. This change also copies helper static
> > functions from other mpc85xx*.c files into p2020.c, which are required for
> > machine descriptions. This is preparation for code de-duplication and
> > providing one unified machine description for all P2020 boards. In
> > follow-up patches would be copied functions refactored and simplified to be
> > specific just for P2020 boards.
> >
> > Signed-off-by: Pali Rohár
> > ---
> > arch/powerpc/platforms/85xx/Makefile | 2 +
> > arch/powerpc/platforms/85xx/mpc85xx_ds.c | 23 --
> > arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 44
> > arch/powerpc/platforms/85xx/p2020.c | 273 ++
> > 4 files changed, 275 insertions(+), 67 deletions(-)
> > create mode 100644 arch/powerpc/platforms/85xx/p2020.c
> >
> > diff --git a/arch/powerpc/platforms/85xx/Makefile
> > b/arch/powerpc/platforms/85xx/Makefile
> > index 260fbad7967b..1ad261b4eeb6 100644
> > --- a/arch/powerpc/platforms/85xx/Makefile
> > +++ b/arch/powerpc/platforms/85xx/Makefile
> > @@ -23,6 +23,8 @@ obj-$(CONFIG_P1010_RDB) += p1010rdb.o
> > obj-$(CONFIG_P1022_DS)+= p1022_ds.o
> > obj-$(CONFIG_P1022_RDK) += p1022_rdk.o
> > obj-$(CONFIG_P1023_RDB) += p1023_rdb.o
> > +obj-$(CONFIG_MPC85xx_DS) += p2020.o
> > +obj-$(CONFIG_MPC85xx_RDB) += p2020.o
> > obj-$(CONFIG_TWR_P102x) += twr_p102x.o
> > obj-$(CONFIG_CORENET_GENERIC) += corenet_generic.o
> > obj-$(CONFIG_FB_FSL_DIU) += t1042rdb_diu.o
> > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > index 9a6d637ef54a..05aac997b5ed 100644
> > --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> > @@ -168,7 +168,6 @@ static int __init mpc8544_ds_probe(void)
> >
> > machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
> > machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
> > -machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
> >
> > /*
> >* Called very early, device-tree isn't unflattened
> > @@ -178,14 +177,6 @@ static int __init mpc8572_ds_probe(void)
> > return !!of_machine_is_compatible("fsl,MPC8572DS");
> > }
> >
> > -/*
> > - * Called very early, device-tree isn't unflattened
> > - */
> > -static int __init p2020_ds_probe(void)
> > -{
> > - return !!of_machine_is_compatible("fsl,P2020DS");
> > -}
> > -
> > define_machine(mpc8544_ds) {
> > .name = "MPC8544 DS",
> > .probe = mpc8544_ds_probe,
> > @@ -213,17 +204,3 @@ define_machine(mpc8572_ds) {
> > .calibrate_decr = generic_calibrate_decr,
> > .progress = udbg_progress,
> > };
> > -
> > -define_machine(p2020_ds) {
> > - .name = "P2020 DS",
> > - .probe = p2020_ds_probe,
> > - .setup_arch = mpc85xx_ds_setup_arch,
> > - .init_IRQ = mpc85xx_ds_pic_init,
> > -#ifdef CONFIG_PCI
> > - .pcibios_fixup_bus = fsl_pcibios_fixup_bus,
> > - .pcibios_fixup_phb = fsl_pcibios_fixup_phb,
> > -#endif
> > - .get_irq= mpic_get_irq,
> > - .calibrate_decr = generic_calibrate_decr,
> > - .progress = udbg_progress,
> > -};
> > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> > b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> > index b6129c148fea..05f1ed635735 100644
> > --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> > +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> > @@ -108,8 +108,6 @@ static void __init mpc85xx_rdb_setup_arch(void)
> > printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
> > }
> >
> > -machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
> > -machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
> > machine_arch_initcall(p1020_mbg_pc, mpc85xx_common_publish_devices);
> > machine_arch_initcall(p1020_rdb, mpc85xx_common_publish_devices);
> > machine_arch_initcall(p1020_rdb_pc, mpc85xx_common_publish_devices);
> > @@ -122,13 +120,6 @@ machine_arch_initcall(p1024_rdb,
> > mpc85xx_common_publish_devices);
> > /*
> >* Called very early, device-tree isn't unflattened
> >*/
> > -static int __init p2020_rdb_probe(void)
> > -{
> > - if (of_machine_is_compatible("fsl,P2020RDB"))
> > - return 1;
> > - return 0;
> > -}
> > -
> > static int __init p1020_rdb_probe(void)
> > {
> > if (of_machine_is_compatible("fsl,P1020RDB"))
> > @@ -153,13 +144,6 @@ static int __init p1021_rdb_pc_probe(void)
> > return 0;
> > }
> >
> > -static int __init p2020_rdb_pc_probe(void)
> > -{
> > - if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> > - return 1;
> > - return 0;
> >
Re: [PATCH v2] powerpc/machdep: warn when machine_is() used too early
Le 13/02/2023 à 20:23, Nathan Lynch via B4 Submission Endpoint a écrit :
> From: Nathan Lynch
>
> machine_is() can't provide correct results before probe_machine() has
> run. Warn when it's used too early in boot, placing the WARN_ON() in a
> helper function so the reported file:line indicates exactly what went
> wrong.
>
> checkpatch complains about __attribute__((weak)) in the patch, so
> change that to __weak, and align the line continuations as well.
>
> Signed-off-by: Nathan Lynch
Reviewed-by: Christophe Leroy
> ---
> Prompted by my attempts to do some pseries-specific setup during
> rtas_initialize() and being puzzled for a while that it wasn't
> working.
>
> Changes in v2:
> - Use WARN_ON(), not WARN().
> - Introduce __machine_is() helper function so the line reported is
>accurate.
> - Update __attribute__((weak)) to __weak for checkpatch's sake.
> - Link to v1:
> https://lore.kernel.org/r/20230210-warn-on-machine-is-before-probe-machine-v1-1-f0cba5712...@linux.ibm.com
> ---
> arch/powerpc/include/asm/machdep.h | 16 +++-
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/machdep.h
> b/arch/powerpc/include/asm/machdep.h
> index 378b8d5836a7..459736d5e511 100644
> --- a/arch/powerpc/include/asm/machdep.h
> +++ b/arch/powerpc/include/asm/machdep.h
> @@ -3,6 +3,7 @@
> #define _ASM_POWERPC_MACHDEP_H
> #ifdef __KERNEL__
>
> +#include
> #include
> #include
> #include
> @@ -220,11 +221,16 @@ extern struct machdep_calls *machine_id;
> EXPORT_SYMBOL(mach_##name); \
> struct machdep_calls mach_##name __machine_desc =
>
> -#define machine_is(name) \
> - ({ \
> - extern struct machdep_calls mach_##name \
> - __attribute__((weak)); \
> - machine_id == _##name; \
> +static inline bool __machine_is(const struct machdep_calls *md)
> +{
> + WARN_ON(!machine_id); // complain if used before probe_machine()
> + return machine_id == md;
> +}
> +
> +#define machine_is(name)\
Misaligned back-slash ?
> + ({ \
> + extern struct machdep_calls mach_##name __weak; \
> + __machine_is(_##name); \
> })
>
> static inline void log_error(char *buf, unsigned int err_type, int fatal)
>
> ---
> base-commit: 0bfb97203f5f300777624a2ad6f8f84aea3e8658
> change-id: 20230210-warn-on-machine-is-before-probe-machine-37515b1f43bb
>
> Best regards,
Re: [PATCH v2 0/8] powerpc/85xx: p2020: Create one unified machine description
On Monday 13 February 2023 19:58:15 Christophe Leroy wrote: > Le 09/02/2023 à 01:15, Pali Rohár a écrit : > >> > >> This patch moves all p2020 boards from mpc85xx_rdb.c and mpc85xx_ds.c > >> files into new p2020.c file, and plus it copies all helper functions > >> which p2020 boards requires. This patch does not introduce any new code > >> or functional change. It should be really plain copy/move. > > Yes after looking into it in more details, it is exactly that. You > copied all helper functions but this is not said in the commit message. > I think it should be said, and more important it should be explained why. > Because this is exactly what I was not understanding, why I couldn't see > all moved functions: just because many of them were not moved but copied. > > In the two first pages you made some function static, and then you > duplicated it. Why ? Why not keep it global and just use it from one > place to the other ? > > Because after patch 3 we have: > > arch/powerpc/platforms/85xx/mpc85xx_rdb.c:static void __init > mpc85xx_rdb_pic_init(void) > arch/powerpc/platforms/85xx/p2020.c:static void __init > mpc85xx_rdb_pic_init(void) > > arch/powerpc/platforms/85xx/mpc85xx_ds.c:static void __init > mpc85xx_ds_pic_init(void) > arch/powerpc/platforms/85xx/p2020.c:static void __init > mpc85xx_ds_pic_init(void) > > Why not just drop patches 1 and 2 and keep those two functions and all > the other common functions like mpc85xx_8259_cascade() > mpc85xx_ds_uli_init() and a lot more in a separate common file ? > > Christophe After applying all patches there is no mpc85xx_rdb_pic_init() / mpc85xx_ds_pic_init() function in p2020.c file. There is p2020_pic_init() in p2020.c but it is slightly different than previous two functions. Maybe it could be possible to create one function mpc85xx_pic_init() as unification of previous 3 functions, but such change would be needed to test on lot of mpc85xx boards, which would be affected by such change. And I do not have them for testing. I have only P2020. So I wrote *_pic_init() function which is p2020 specific, like already existing ds and rdb specific functions in their own source files.
Re: [PATCH v2 6/8] powerpc/85xx: p2020: Define just one machine description
Le 24/12/2022 à 22:14, Pali Rohár a écrit :
> Combine machine descriptions and code of all P2020 boards into just one
> generic unified P2020 machine description. This allows kernel to boot on
> any P2020-based board with P2020 DTS file without need to patch kernel and
> define a new machine description in 85xx powerpc platform directory.
>
> Signed-off-by: Pali Rohár
> ---
> arch/powerpc/platforms/85xx/p2020.c | 83 +++--
> 1 file changed, 19 insertions(+), 64 deletions(-)
>
> diff --git a/arch/powerpc/platforms/85xx/p2020.c
> b/arch/powerpc/platforms/85xx/p2020.c
> index adf3750abef9..b3fb600e1d83 100644
> --- a/arch/powerpc/platforms/85xx/p2020.c
> +++ b/arch/powerpc/platforms/85xx/p2020.c
> @@ -156,83 +156,38 @@ static void __init p2020_setup_arch(void)
> #endif
> }
>
> -#ifdef CONFIG_MPC85xx_DS
> -machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
> -#endif /* CONFIG_MPC85xx_DS */
> -
> -#ifdef CONFIG_MPC85xx_RDB
> -machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
> -machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
> -#endif /* CONFIG_MPC85xx_RDB */
> +machine_arch_initcall(p2020, mpc85xx_common_publish_devices);
>
> /*
>* Called very early, device-tree isn't unflattened
>*/
> -#ifdef CONFIG_MPC85xx_DS
> -static int __init p2020_ds_probe(void)
> -{
> - return !!of_machine_is_compatible("fsl,P2020DS");
> -}
> -#endif /* CONFIG_MPC85xx_DS */
> -
> -#ifdef CONFIG_MPC85xx_RDB
> -static int __init p2020_rdb_probe(void)
> -{
> - if (of_machine_is_compatible("fsl,P2020RDB"))
> - return 1;
> - return 0;
> -}
> -
> -static int __init p2020_rdb_pc_probe(void)
> +static int __init p2020_probe(void)
> {
> - if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> - return 1;
> - return 0;
> + struct device_node *p2020_cpu;
> +
> + /*
> + * There is no common compatible string for all P2020 boards.
> + * The only common thing is "PowerPC,P2020@0" cpu node.
> + * So check for P2020 board via this cpu node.
> + */
> + p2020_cpu = of_find_node_by_path("/cpus/PowerPC,P2020@0");
> + if (!p2020_cpu)
> + return 0;
> +
> + of_node_put(p2020_cpu);
of_node_put() already checks for nullity of its parameter, so you can
simplify stuff here, something like
p2020_cpu = of_find_node_by_path("/cpus/PowerPC,P2020@0");
of_node_put(p2020_cpu);
return !!p2020_cpu;
> + return 1;
> }
> -#endif /* CONFIG_MPC85xx_RDB */
> -
> -#ifdef CONFIG_MPC85xx_DS
> -define_machine(p2020_ds) {
> - .name = "P2020 DS",
> - .probe = p2020_ds_probe,
> - .setup_arch = p2020_setup_arch,
> - .init_IRQ = p2020_pic_init,
> -#ifdef CONFIG_PCI
> - .pcibios_fixup_bus = fsl_pcibios_fixup_bus,
> - .pcibios_fixup_phb = fsl_pcibios_fixup_phb,
> -#endif
> - .get_irq= mpic_get_irq,
> - .calibrate_decr = generic_calibrate_decr,
> - .progress = udbg_progress,
> -};
> -#endif /* CONFIG_MPC85xx_DS */
> -
> -#ifdef CONFIG_MPC85xx_RDB
> -define_machine(p2020_rdb) {
> - .name = "P2020 RDB",
> - .probe = p2020_rdb_probe,
> - .setup_arch = p2020_setup_arch,
> - .init_IRQ = p2020_pic_init,
> -#ifdef CONFIG_PCI
> - .pcibios_fixup_bus = fsl_pcibios_fixup_bus,
> - .pcibios_fixup_phb = fsl_pcibios_fixup_phb,
> -#endif
> - .get_irq= mpic_get_irq,
> - .calibrate_decr = generic_calibrate_decr,
> - .progress = udbg_progress,
> -};
>
> -define_machine(p2020_rdb_pc) {
> - .name = "P2020RDB-PC",
> - .probe = p2020_rdb_pc_probe,
> +define_machine(p2020) {
> + .name = "Freescale P2020",
> + .probe = p2020_probe,
> .setup_arch = p2020_setup_arch,
> .init_IRQ = p2020_pic_init,
> #ifdef CONFIG_PCI
> .pcibios_fixup_bus = fsl_pcibios_fixup_bus,
> - .pcibios_fixup_phb = fsl_pcibios_fixup_phb,
> + .pcibios_fixup_phb = fsl_pcibios_fixup_phb,
> #endif
> .get_irq= mpic_get_irq,
> .calibrate_decr = generic_calibrate_decr,
> .progress = udbg_progress,
> };
> -#endif /* CONFIG_MPC85xx_RDB */
Re: [PATCH v2 4/8] powerpc/85xx: p2020: Move i8259 code into own function
Le 24/12/2022 à 22:14, Pali Rohár a écrit :
> Splits mpic and i8259 initialization codes into separate functions.
>
> Signed-off-by: Pali Rohár
> ---
> arch/powerpc/platforms/85xx/p2020.c | 37 -
> 1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/platforms/85xx/p2020.c
> b/arch/powerpc/platforms/85xx/p2020.c
> index d65d4c88ac47..b8584bf307b0 100644
> --- a/arch/powerpc/platforms/85xx/p2020.c
> +++ b/arch/powerpc/platforms/85xx/p2020.c
> @@ -45,6 +45,7 @@
> #ifdef CONFIG_MPC85xx_DS
>
> #ifdef CONFIG_PPC_I8259
> +
> static void mpc85xx_8259_cascade(struct irq_desc *desc)
> {
> struct irq_chip *chip = irq_desc_get_chip(desc);
> @@ -55,27 +56,13 @@ static void mpc85xx_8259_cascade(struct irq_desc *desc)
> }
> chip->irq_eoi(>irq_data);
> }
> -#endif /* CONFIG_PPC_I8259 */
>
> -static void __init mpc85xx_ds_pic_init(void)
> +static void __init mpc85xx_8259_init(void)
> {
> - struct mpic *mpic;
> -#ifdef CONFIG_PPC_I8259
> struct device_node *np;
> struct device_node *cascade_node = NULL;
> int cascade_irq;
> -#endif
> -
> - mpic = mpic_alloc(NULL, 0,
> - MPIC_BIG_ENDIAN |
> - MPIC_SINGLE_DEST_CPU,
> - 0, 256, " OpenPIC ");
> -
> - BUG_ON(mpic == NULL);
> - mpic_init(mpic);
>
> -#ifdef CONFIG_PPC_I8259
> - /* Initialize the i8259 controller */
> for_each_node_by_type(np, "interrupt-controller")
> if (of_device_is_compatible(np, "chrp,iic")) {
> cascade_node = np;
> @@ -93,13 +80,31 @@ static void __init mpc85xx_ds_pic_init(void)
> return;
> }
>
> - DBG("mpc85xxds: cascade mapped to irq %d\n", cascade_irq);
> + DBG("i8259: cascade mapped to irq %d\n", cascade_irq);
>
> i8259_init(cascade_node, 0);
> of_node_put(cascade_node);
>
> irq_set_chained_handler(cascade_irq, mpc85xx_8259_cascade);
> +}
> +
> #endif /* CONFIG_PPC_I8259 */
> +
> +static void __init mpc85xx_ds_pic_init(void)
> +{
> + struct mpic *mpic;
> +
> + mpic = mpic_alloc(NULL, 0,
> + MPIC_BIG_ENDIAN |
> + MPIC_SINGLE_DEST_CPU,
> + 0, 256, " OpenPIC ");
> +
> + BUG_ON(mpic == NULL);
> + mpic_init(mpic);
> +
> +#ifdef CONFIG_PPC_I8259
Ca we minimise number of #ifdef CONFIG_PPC_I8259 by using
IS_ENABLED(CONFIG_PPC_I8259) inside if/else ?
> + mpc85xx_8259_init();
> +#endif
> }
>
> #ifdef CONFIG_PCI
Re: [PATCH v2 3/8] powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
Le 24/12/2022 à 22:14, Pali Rohár a écrit :
> This moves machine descriptions and all related code for all P2020 boards
> into new p2020.c source file. This change also copies helper static
> functions from other mpc85xx*.c files into p2020.c, which are required for
> machine descriptions. This is preparation for code de-duplication and
> providing one unified machine description for all P2020 boards. In
> follow-up patches would be copied functions refactored and simplified to be
> specific just for P2020 boards.
>
> Signed-off-by: Pali Rohár
> ---
> arch/powerpc/platforms/85xx/Makefile | 2 +
> arch/powerpc/platforms/85xx/mpc85xx_ds.c | 23 --
> arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 44
> arch/powerpc/platforms/85xx/p2020.c | 273 ++
> 4 files changed, 275 insertions(+), 67 deletions(-)
> create mode 100644 arch/powerpc/platforms/85xx/p2020.c
>
> diff --git a/arch/powerpc/platforms/85xx/Makefile
> b/arch/powerpc/platforms/85xx/Makefile
> index 260fbad7967b..1ad261b4eeb6 100644
> --- a/arch/powerpc/platforms/85xx/Makefile
> +++ b/arch/powerpc/platforms/85xx/Makefile
> @@ -23,6 +23,8 @@ obj-$(CONFIG_P1010_RDB) += p1010rdb.o
> obj-$(CONFIG_P1022_DS)+= p1022_ds.o
> obj-$(CONFIG_P1022_RDK) += p1022_rdk.o
> obj-$(CONFIG_P1023_RDB) += p1023_rdb.o
> +obj-$(CONFIG_MPC85xx_DS) += p2020.o
> +obj-$(CONFIG_MPC85xx_RDB) += p2020.o
> obj-$(CONFIG_TWR_P102x) += twr_p102x.o
> obj-$(CONFIG_CORENET_GENERIC) += corenet_generic.o
> obj-$(CONFIG_FB_FSL_DIU)+= t1042rdb_diu.o
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> index 9a6d637ef54a..05aac997b5ed 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> @@ -168,7 +168,6 @@ static int __init mpc8544_ds_probe(void)
>
> machine_arch_initcall(mpc8544_ds, mpc85xx_common_publish_devices);
> machine_arch_initcall(mpc8572_ds, mpc85xx_common_publish_devices);
> -machine_arch_initcall(p2020_ds, mpc85xx_common_publish_devices);
>
> /*
>* Called very early, device-tree isn't unflattened
> @@ -178,14 +177,6 @@ static int __init mpc8572_ds_probe(void)
> return !!of_machine_is_compatible("fsl,MPC8572DS");
> }
>
> -/*
> - * Called very early, device-tree isn't unflattened
> - */
> -static int __init p2020_ds_probe(void)
> -{
> - return !!of_machine_is_compatible("fsl,P2020DS");
> -}
> -
> define_machine(mpc8544_ds) {
> .name = "MPC8544 DS",
> .probe = mpc8544_ds_probe,
> @@ -213,17 +204,3 @@ define_machine(mpc8572_ds) {
> .calibrate_decr = generic_calibrate_decr,
> .progress = udbg_progress,
> };
> -
> -define_machine(p2020_ds) {
> - .name = "P2020 DS",
> - .probe = p2020_ds_probe,
> - .setup_arch = mpc85xx_ds_setup_arch,
> - .init_IRQ = mpc85xx_ds_pic_init,
> -#ifdef CONFIG_PCI
> - .pcibios_fixup_bus = fsl_pcibios_fixup_bus,
> - .pcibios_fixup_phb = fsl_pcibios_fixup_phb,
> -#endif
> - .get_irq= mpic_get_irq,
> - .calibrate_decr = generic_calibrate_decr,
> - .progress = udbg_progress,
> -};
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> index b6129c148fea..05f1ed635735 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> @@ -108,8 +108,6 @@ static void __init mpc85xx_rdb_setup_arch(void)
> printk(KERN_INFO "MPC85xx RDB board from Freescale Semiconductor\n");
> }
>
> -machine_arch_initcall(p2020_rdb, mpc85xx_common_publish_devices);
> -machine_arch_initcall(p2020_rdb_pc, mpc85xx_common_publish_devices);
> machine_arch_initcall(p1020_mbg_pc, mpc85xx_common_publish_devices);
> machine_arch_initcall(p1020_rdb, mpc85xx_common_publish_devices);
> machine_arch_initcall(p1020_rdb_pc, mpc85xx_common_publish_devices);
> @@ -122,13 +120,6 @@ machine_arch_initcall(p1024_rdb,
> mpc85xx_common_publish_devices);
> /*
>* Called very early, device-tree isn't unflattened
>*/
> -static int __init p2020_rdb_probe(void)
> -{
> - if (of_machine_is_compatible("fsl,P2020RDB"))
> - return 1;
> - return 0;
> -}
> -
> static int __init p1020_rdb_probe(void)
> {
> if (of_machine_is_compatible("fsl,P1020RDB"))
> @@ -153,13 +144,6 @@ static int __init p1021_rdb_pc_probe(void)
> return 0;
> }
>
> -static int __init p2020_rdb_pc_probe(void)
> -{
> - if (of_machine_is_compatible("fsl,P2020RDB-PC"))
> - return 1;
> - return 0;
> -}
> -
> static int __init p1025_rdb_probe(void)
> {
> return of_machine_is_compatible("fsl,P1025RDB");
> @@ -180,20 +164,6 @@ static int __init p1024_rdb_probe(void)
> return of_machine_is_compatible("fsl,P1024RDB");
> }
>
Re: [PATCH v2 0/8] powerpc/85xx: p2020: Create one unified machine description
Le 09/02/2023 à 01:15, Pali Rohár a écrit : >> >> This patch moves all p2020 boards from mpc85xx_rdb.c and mpc85xx_ds.c >> files into new p2020.c file, and plus it copies all helper functions >> which p2020 boards requires. This patch does not introduce any new code >> or functional change. It should be really plain copy/move. Yes after looking into it in more details, it is exactly that. You copied all helper functions but this is not said in the commit message. I think it should be said, and more important it should be explained why. Because this is exactly what I was not understanding, why I couldn't see all moved functions: just because many of them were not moved but copied. In the two first pages you made some function static, and then you duplicated it. Why ? Why not keep it global and just use it from one place to the other ? Because after patch 3 we have: arch/powerpc/platforms/85xx/mpc85xx_rdb.c:static void __init mpc85xx_rdb_pic_init(void) arch/powerpc/platforms/85xx/p2020.c:static void __init mpc85xx_rdb_pic_init(void) arch/powerpc/platforms/85xx/mpc85xx_ds.c:static void __init mpc85xx_ds_pic_init(void) arch/powerpc/platforms/85xx/p2020.c:static void __init mpc85xx_ds_pic_init(void) Why not just drop patches 1 and 2 and keep those two functions and all the other common functions like mpc85xx_8259_cascade() mpc85xx_ds_uli_init() and a lot more in a separate common file ? Christophe
[PATCH v2] powerpc/machdep: warn when machine_is() used too early
From: Nathan Lynch
machine_is() can't provide correct results before probe_machine() has
run. Warn when it's used too early in boot, placing the WARN_ON() in a
helper function so the reported file:line indicates exactly what went
wrong.
checkpatch complains about __attribute__((weak)) in the patch, so
change that to __weak, and align the line continuations as well.
Signed-off-by: Nathan Lynch
---
Prompted by my attempts to do some pseries-specific setup during
rtas_initialize() and being puzzled for a while that it wasn't
working.
Changes in v2:
- Use WARN_ON(), not WARN().
- Introduce __machine_is() helper function so the line reported is
accurate.
- Update __attribute__((weak)) to __weak for checkpatch's sake.
- Link to v1:
https://lore.kernel.org/r/20230210-warn-on-machine-is-before-probe-machine-v1-1-f0cba5712...@linux.ibm.com
---
arch/powerpc/include/asm/machdep.h | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/machdep.h
b/arch/powerpc/include/asm/machdep.h
index 378b8d5836a7..459736d5e511 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -3,6 +3,7 @@
#define _ASM_POWERPC_MACHDEP_H
#ifdef __KERNEL__
+#include
#include
#include
#include
@@ -220,11 +221,16 @@ extern struct machdep_calls *machine_id;
EXPORT_SYMBOL(mach_##name); \
struct machdep_calls mach_##name __machine_desc =
-#define machine_is(name) \
- ({ \
- extern struct machdep_calls mach_##name \
- __attribute__((weak)); \
- machine_id == _##name; \
+static inline bool __machine_is(const struct machdep_calls *md)
+{
+ WARN_ON(!machine_id); // complain if used before probe_machine()
+ return machine_id == md;
+}
+
+#define machine_is(name)\
+ ({ \
+ extern struct machdep_calls mach_##name __weak; \
+ __machine_is(_##name); \
})
static inline void log_error(char *buf, unsigned int err_type, int fatal)
---
base-commit: 0bfb97203f5f300777624a2ad6f8f84aea3e8658
change-id: 20230210-warn-on-machine-is-before-probe-machine-37515b1f43bb
Best regards,
--
Nathan Lynch
Re: [PATCH 04/11] drivers/ps3: Read video= option with fb_get_option()
Hi, On 2/13/23 03:29, Thomas Zimmermann wrote: > Am 12.02.23 um 17:53 schrieb Geoff Levand: >> On 2/9/23 05:55, Thomas Zimmermann wrote: >>> Get the kernel's global video= parameter with fb_get_option(). Done >>> to unexport the internal fbdev state fb_mode_config. No functional >>> changes. >>> >>> Signed-off-by: Thomas Zimmermann >>> --- >>> drivers/ps3/ps3av.c | 11 +-- >>> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> I wanted to test these changes on the PS3, but got this >> error when trying to apply this patch set to Linux-6.2-rc7: >> >> Applying: fbdev: Handle video= parameter in video/cmdline.c >> error: patch failed: drivers/gpu/drm/Kconfig:10 >> error: drivers/gpu/drm/Kconfig: patch does not apply >> >> Is there a Linux kernel revision that these will apply to, >> or is there a git repository I can pull them from? > > Thanks for testing. My base version is a recent DRM development tree. The > repo is at https://cgit.freedesktop.org/drm/drm-tip/, the branch is drm-tip. I tested the drm-tip branch at c54b5fcf3e68 on PS3 and it seemed to work OK. Tested-by: Geoff Levand
Re: [PATCH] powerpc/pseries/cpuhp: respect current SMT when adding new CPU
Michal Suchánek writes: > On Mon, Feb 13, 2023 at 08:46:50AM -0600, Nathan Lynch wrote: >> Laurent Dufour writes: >> > When a new CPU is added, the kernel is activating all its threads. This >> > leads to weird, but functional, result when adding CPU on a SMT 4 system >> > for instance. >> > >> > Here the newly added CPU 1 has 8 threads while the other one has 4 threads >> > active (system has been booted with the 'smt-enabled=4' kernel option): >> > >> > ltcden3-lp12:~ # ppc64_cpu --info >> > Core 0:0*1*2*3*4 5 6 7 >> > Core 1:8*9* 10* 11* 12* 13* 14* 15* >> > >> > There is no SMT value in the kernel. It is possible to run unbalanced LPAR >> > with 2 threads for a CPU, 4 for another one, and 5 on the latest. >> > >> > To work around this possibility, and assuming that the LPAR run with the >> > same number of threads for each CPU, which is the common case, >> >> I am skeptical at best of baking that assumption into this code. Mixed >> SMT modes within a partition doesn't strike me as an unreasonable >> possibility for some use cases. And if that's wrong, then we should just >> add a global smt value instead of using heuristics. >> >> > the number >> > of active threads of the CPU doing the hot-plug operation is computed. Only >> > that number of threads will be activated for the newly added CPU. >> > >> > This way on a LPAR running in SMT=4, newly added CPU will be running 4 >> > threads, which is what a end user would expect. >> >> I could see why most users would prefer this new behavior. But surely >> some users have come to expect the existing behavior, which has been in >> place for years, and developed workarounds that might be broken by this >> change? >> >> I would suggest that to handle this well, we need to give user space >> more ability to tell the kernel what actions to take on added cores, on >> an opt-in basis. >> >> This could take the form of extending the DLPAR sysfs command set: >> >> Option 1 - Add a flag that tells the kernel not to online any threads at >> all; user space will online the desired threads later. >> >> Option 2 - Add an option that tells the kernel which SMT mode to apply. > > powerpc-utils grew some drmgr hooks recently so maybe the policy can be > moved to userspace? I'm not sure whether the hook mechanism would come into play, but yes, I am suggesting that user space be given the option of overriding the kernel's current behavior.
Re: [PATCH] powerpc/pseries/cpuhp: respect current SMT when adding new CPU
Hello, On Mon, Feb 13, 2023 at 08:46:50AM -0600, Nathan Lynch wrote: > Laurent Dufour writes: > > When a new CPU is added, the kernel is activating all its threads. This > > leads to weird, but functional, result when adding CPU on a SMT 4 system > > for instance. > > > > Here the newly added CPU 1 has 8 threads while the other one has 4 threads > > active (system has been booted with the 'smt-enabled=4' kernel option): > > > > ltcden3-lp12:~ # ppc64_cpu --info > > Core 0:0*1*2*3*4 5 6 7 > > Core 1:8*9* 10* 11* 12* 13* 14* 15* > > > > There is no SMT value in the kernel. It is possible to run unbalanced LPAR > > with 2 threads for a CPU, 4 for another one, and 5 on the latest. > > > > To work around this possibility, and assuming that the LPAR run with the > > same number of threads for each CPU, which is the common case, > > I am skeptical at best of baking that assumption into this code. Mixed > SMT modes within a partition doesn't strike me as an unreasonable > possibility for some use cases. And if that's wrong, then we should just > add a global smt value instead of using heuristics. > > > the number > > of active threads of the CPU doing the hot-plug operation is computed. Only > > that number of threads will be activated for the newly added CPU. > > > > This way on a LPAR running in SMT=4, newly added CPU will be running 4 > > threads, which is what a end user would expect. > > I could see why most users would prefer this new behavior. But surely > some users have come to expect the existing behavior, which has been in > place for years, and developed workarounds that might be broken by this > change? > > I would suggest that to handle this well, we need to give user space > more ability to tell the kernel what actions to take on added cores, on > an opt-in basis. > > This could take the form of extending the DLPAR sysfs command set: > > Option 1 - Add a flag that tells the kernel not to online any threads at > all; user space will online the desired threads later. > > Option 2 - Add an option that tells the kernel which SMT mode to apply. powerpc-utils grew some drmgr hooks recently so maybe the policy can be moved to userspace? Thanks Michal
[PATCH 6.1 024/114] of: Make OF framebuffer device names unique
From: Michal Suchanek
[ Upstream commit 241d2fb56a18473af5f2ff0d512992a996eb64dd ]
Since Linux 5.19 this error is observed:
sysfs: cannot create duplicate filename '/devices/platform/of-display'
This is because multiple devices with the same name 'of-display' are
created on the same bus. Update the code to create numbered device names
for the displays.
Also, fix a node refcounting issue when exiting the boot display loop.
cc: linuxppc-dev@lists.ozlabs.org
Fixes: 52b1b46c39ae ("of: Create platform devices for OF framebuffers")
Reported-by: Erhard F.
Suggested-by: Thomas Zimmermann
Signed-off-by: Michal Suchanek
Link: https://lore.kernel.org/r/20230201162247.3575506-1-r...@kernel.org
[robh: Rework to avoid node refcount leaks]
Signed-off-by: Rob Herring
Signed-off-by: Sasha Levin
---
drivers/of/platform.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 3507095a69f69..6e93fd37ccd1a 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -526,6 +526,7 @@ static int __init of_platform_default_populate_init(void)
if (IS_ENABLED(CONFIG_PPC)) {
struct device_node *boot_display = NULL;
struct platform_device *dev;
+ int display_number = 0;
int ret;
/* Check if we have a MacOS display without a node spec */
@@ -556,16 +557,23 @@ static int __init of_platform_default_populate_init(void)
if (!of_get_property(node, "linux,opened", NULL) ||
!of_get_property(node, "linux,boot-display", NULL))
continue;
- dev = of_platform_device_create(node, "of-display",
NULL);
+ dev = of_platform_device_create(node, "of-display.0",
NULL);
+ of_node_put(node);
if (WARN_ON(!dev))
return -ENOMEM;
boot_display = node;
+ display_number++;
break;
}
for_each_node_by_type(node, "display") {
+ char buf[14];
+ const char *of_display_format = "of-display.%d";
+
if (!of_get_property(node, "linux,opened", NULL) ||
node == boot_display)
continue;
- of_platform_device_create(node, "of-display", NULL);
+ ret = snprintf(buf, sizeof(buf), of_display_format,
display_number++);
+ if (ret < sizeof(buf))
+ of_platform_device_create(node, buf, NULL);
}
} else {
--
2.39.0
Re: [PATCH] powerpc/pseries/cpuhp: respect current SMT when adding new CPU
Laurent Dufour writes: > When a new CPU is added, the kernel is activating all its threads. This > leads to weird, but functional, result when adding CPU on a SMT 4 system > for instance. > > Here the newly added CPU 1 has 8 threads while the other one has 4 threads > active (system has been booted with the 'smt-enabled=4' kernel option): > > ltcden3-lp12:~ # ppc64_cpu --info > Core 0:0*1*2*3*4 5 6 7 > Core 1:8*9* 10* 11* 12* 13* 14* 15* > > There is no SMT value in the kernel. It is possible to run unbalanced LPAR > with 2 threads for a CPU, 4 for another one, and 5 on the latest. > > To work around this possibility, and assuming that the LPAR run with the > same number of threads for each CPU, which is the common case, I am skeptical at best of baking that assumption into this code. Mixed SMT modes within a partition doesn't strike me as an unreasonable possibility for some use cases. And if that's wrong, then we should just add a global smt value instead of using heuristics. > the number > of active threads of the CPU doing the hot-plug operation is computed. Only > that number of threads will be activated for the newly added CPU. > > This way on a LPAR running in SMT=4, newly added CPU will be running 4 > threads, which is what a end user would expect. I could see why most users would prefer this new behavior. But surely some users have come to expect the existing behavior, which has been in place for years, and developed workarounds that might be broken by this change? I would suggest that to handle this well, we need to give user space more ability to tell the kernel what actions to take on added cores, on an opt-in basis. This could take the form of extending the DLPAR sysfs command set: Option 1 - Add a flag that tells the kernel not to online any threads at all; user space will online the desired threads later. Option 2 - Add an option that tells the kernel which SMT mode to apply.
Re: [PATCH v6 6/7] PCI: pciehp: Rely on `link_active_reporting'
On Sun, Feb 05, 2023 at 03:49:21PM +, Maciej W. Rozycki wrote:
> Use `link_active_reporting' to determine whether Data Link Layer Link
> Active Reporting is available rather than re-retrieving the capability.
>
> Signed-off-by: Maciej W. Rozycki
Reviewed-by: Lukas Wunner
I believe this should work without the preceding patches in the series,
hence can be applied independently.
Thanks,
Lukas
> drivers/pci/hotplug/pciehp_hpc.c |7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> linux-pcie-link-active-reporting-hpc.diff
> Index: linux-macro/drivers/pci/hotplug/pciehp_hpc.c
> ===
> --- linux-macro.orig/drivers/pci/hotplug/pciehp_hpc.c
> +++ linux-macro/drivers/pci/hotplug/pciehp_hpc.c
> @@ -984,7 +984,7 @@ static inline int pcie_hotplug_depth(str
> struct controller *pcie_init(struct pcie_device *dev)
> {
> struct controller *ctrl;
> - u32 slot_cap, slot_cap2, link_cap;
> + u32 slot_cap, slot_cap2;
> u8 poweron;
> struct pci_dev *pdev = dev->port;
> struct pci_bus *subordinate = pdev->subordinate;
> @@ -1030,9 +1030,6 @@ struct controller *pcie_init(struct pcie
> if (dmi_first_match(inband_presence_disabled_dmi_table))
> ctrl->inband_presence_disabled = 1;
>
> - /* Check if Data Link Layer Link Active Reporting is implemented */
> - pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, _cap);
> -
> /* Clear all remaining event bits in Slot Status register. */
> pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
> PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD |
> @@ -1051,7 +1048,7 @@ struct controller *pcie_init(struct pcie
> FLAG(slot_cap, PCI_EXP_SLTCAP_EIP),
> FLAG(slot_cap, PCI_EXP_SLTCAP_NCCS),
> FLAG(slot_cap2, PCI_EXP_SLTCAP2_IBPD),
> - FLAG(link_cap, PCI_EXP_LNKCAP_DLLLARC),
> + FLAG(pdev->link_active_reporting, true),
> pdev->broken_cmd_compl ? " (with Cmd Compl erratum)" : "");
>
> /*
Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic secure boot
On 2/13/23 06:32, Michael Ellerman wrote:
Stefan Berger writes:
On 2/10/23 03:03, Andrew Donnellan wrote:
From: Russell Currey
...
+static int plpks_set_variable(const char *key, u64 key_len, u8 *data,
+ u64 data_size)
+{
+ struct plpks_var var = {0};
+ int rc = 0;
+ u64 flags;
+
+ // Secure variables need to be prefixed with 8 bytes of flags.
+ // We only want to perform the write if we have at least one byte of
data.
+ if (data_size <= sizeof(flags))
+ return -EINVAL;
+
+ // We subtract 1 from key_len because we don't need to include the
+ // null terminator at the end of the string
+ var.name = kcalloc(key_len - 1, sizeof(wchar_t), GFP_KERNEL);
+ if (!var.name)
+ return -ENOMEM;
+ rc = utf8s_to_utf16s(key, key_len - 1, UTF16_LITTLE_ENDIAN, (wchar_t
*)var.name,
+key_len - 1);
+ if (rc < 0)
+ goto err;
+ var.namelen = rc * 2;
+
+ memcpy(, data, sizeof(flags));
conversion from bytestream to integer: I think in this case it would be better
to use
flags = cpu_to_be64p((__u64*)data);
so that the flags always in hypervisor/big endian format
I don't think it's correct to byte swap the flags here. They must be in
big endian format, but that's up to the caller.
The powernv secvar backend doesn't byte swap the flags, if the pseries
one did then the final content of the variable, written either by phyp
or OPAL, would differ depending on which backend is active.
Or am I missing something?
It seems wrong to not use the cpu_to_be64p() API to convert a byte stream to
flags... That's why I suggested this.
Stefan
cheers
Re: [PATCH] powerpc/machdep: warn when machine_is() used too early
Michael Ellerman writes:
> Christophe Leroy writes:
>> Le 11/02/2023 à 00:56, Nathan Lynch via B4 Submission Endpoint a écrit :
>>> From: Nathan Lynch
>>>
>>> machine_is() can't provide correct results before probe_machine() has
>>> run. Warn when it's used too early in boot.
>>>
>>> Signed-off-by: Nathan Lynch
>>> ---
>>> Prompted by my attempts to do some pseries-specific setup during
>>> rtas_initialize() and being puzzled for a while that it wasn't
>>> working.
>>> ---
>>> arch/powerpc/include/asm/machdep.h | 12 +++-
>>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/machdep.h
>>> b/arch/powerpc/include/asm/machdep.h
>>> index 378b8d5836a7..8c0a799d18cd 100644
>>> --- a/arch/powerpc/include/asm/machdep.h
>>> +++ b/arch/powerpc/include/asm/machdep.h
>>> @@ -220,11 +220,13 @@ extern struct machdep_calls *machine_id;
>>> EXPORT_SYMBOL(mach_##name); \
>>> struct machdep_calls mach_##name __machine_desc =
>>>
>>> -#define machine_is(name) \
>>> - ({ \
>>> - extern struct machdep_calls mach_##name \
>>> - __attribute__((weak)); \
>>> - machine_id == _##name; \
>>> +#define machine_is(name)\
>>> + ({ \
>>> + extern struct machdep_calls mach_##name \
>>> + __attribute__((weak)); \
>>> + WARN(!machine_id, \
>>> +"machine_is() called before probe_machine()"); \
>>
>> Is a WARN() really necessary ? WARN() is less optimised than WARN_ON(),
>> especially on PPC64.
>>
>> This should never ever happen so a WARN_ON(!machine_id) should be
>> enough, the developper that hits it is able to go to the given file:line
>> and understand what happened.
>
> Yeah I agree, WARN_ON() should be sufficient here, and should generate
> slightly better code. We have > 100 uses of machine_is(), so keeping
> each small is desirable.
Sure, I'll change it.
[PATCH] powerpc/pseries/cpuhp: respect current SMT when adding new CPU
When a new CPU is added, the kernel is activating all its threads. This
leads to weird, but functional, result when adding CPU on a SMT 4 system
for instance.
Here the newly added CPU 1 has 8 threads while the other one has 4 threads
active (system has been booted with the 'smt-enabled=4' kernel option):
ltcden3-lp12:~ # ppc64_cpu --info
Core 0:0*1*2*3*4 5 6 7
Core 1:8*9* 10* 11* 12* 13* 14* 15*
There is no SMT value in the kernel. It is possible to run unbalanced LPAR
with 2 threads for a CPU, 4 for another one, and 5 on the latest.
To work around this possibility, and assuming that the LPAR run with the
same number of threads for each CPU, which is the common case, the number
of active threads of the CPU doing the hot-plug operation is computed. Only
that number of threads will be activated for the newly added CPU.
This way on a LPAR running in SMT=4, newly added CPU will be running 4
threads, which is what a end user would expect.
Cc: Srikar Dronamraju
Signed-off-by: Laurent Dufour
---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 24
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c
b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 090ae5a1e0f5..58a7c97fc475 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -382,7 +382,7 @@ static int dlpar_online_cpu(struct device_node *dn)
{
int rc = 0;
unsigned int cpu;
- int len, nthreads, i;
+ int len, nthreads, i, smt;
const __be32 *intserv;
u32 thread;
@@ -392,6 +392,17 @@ static int dlpar_online_cpu(struct device_node *dn)
nthreads = len / sizeof(u32);
+ /*
+* Compute the number of active threads for the current CPU, assuming
+* the system is homogeus, we don't want to active more threads than the
+* current SMT setting.
+*/
+ for (cpu = cpu_first_thread_sibling(raw_smp_processor_id()), smt = 0;
+cpu <= cpu_last_thread_sibling(raw_smp_processor_id()); cpu++) {
+ if (cpu_online(cpu))
+ smt++;
+ }
+
cpu_maps_update_begin();
for (i = 0; i < nthreads; i++) {
thread = be32_to_cpu(intserv[i]);
@@ -400,10 +411,13 @@ static int dlpar_online_cpu(struct device_node *dn)
continue;
cpu_maps_update_done();
find_and_update_cpu_nid(cpu);
- rc = device_online(get_cpu_device(cpu));
- if (rc) {
- dlpar_offline_cpu(dn);
- goto out;
+ /* Don't active CPU over the current SMT setting */
+ if (smt-- > 0) {
+ rc = device_online(get_cpu_device(cpu));
+ if (rc) {
+ dlpar_offline_cpu(dn);
+ goto out;
+ }
}
cpu_maps_update_begin();
--
2.39.1
Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic secure boot
Stefan Berger writes:
> On 2/10/23 03:03, Andrew Donnellan wrote:
>> From: Russell Currey
...
>> +static int plpks_set_variable(const char *key, u64 key_len, u8 *data,
>> + u64 data_size)
>> +{
>> +struct plpks_var var = {0};
>> +int rc = 0;
>> +u64 flags;
>> +
>> +// Secure variables need to be prefixed with 8 bytes of flags.
>> +// We only want to perform the write if we have at least one byte of
>> data.
>> +if (data_size <= sizeof(flags))
>> +return -EINVAL;
>> +
>> +// We subtract 1 from key_len because we don't need to include the
>> +// null terminator at the end of the string
>> +var.name = kcalloc(key_len - 1, sizeof(wchar_t), GFP_KERNEL);
>> +if (!var.name)
>> +return -ENOMEM;
>> +rc = utf8s_to_utf16s(key, key_len - 1, UTF16_LITTLE_ENDIAN, (wchar_t
>> *)var.name,
>> + key_len - 1);
>> +if (rc < 0)
>> +goto err;
>> +var.namelen = rc * 2;
>> +
>> +memcpy(, data, sizeof(flags));
>
> conversion from bytestream to integer: I think in this case it would be
> better to use
>
> flags = cpu_to_be64p((__u64*)data);
>
> so that the flags always in hypervisor/big endian format
I don't think it's correct to byte swap the flags here. They must be in
big endian format, but that's up to the caller.
The powernv secvar backend doesn't byte swap the flags, if the pseries
one did then the final content of the variable, written either by phyp
or OPAL, would differ depending on which backend is active.
Or am I missing something?
cheers
Re: [PATCH 04/11] drivers/ps3: Read video= option with fb_get_option()
Hi Am 12.02.23 um 17:53 schrieb Geoff Levand: Hi Thomas, On 2/9/23 05:55, Thomas Zimmermann wrote: Get the kernel's global video= parameter with fb_get_option(). Done to unexport the internal fbdev state fb_mode_config. No functional changes. Signed-off-by: Thomas Zimmermann --- drivers/ps3/ps3av.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) I wanted to test these changes on the PS3, but got this error when trying to apply this patch set to Linux-6.2-rc7: Applying: fbdev: Handle video= parameter in video/cmdline.c error: patch failed: drivers/gpu/drm/Kconfig:10 error: drivers/gpu/drm/Kconfig: patch does not apply Is there a Linux kernel revision that these will apply to, or is there a git repository I can pull them from? Thanks for testing. My base version is a recent DRM development tree. The repo is at https://cgit.freedesktop.org/drm/drm-tip/, the branch is drm-tip. If acceptable, I'd later like to merge the PS3 patches through DRM trees. Best regards Thomas -Geoff -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
[PATCH] powerpc/nohash: Fix build error with binutils >= 2.38
With bintils >= 2.38 the ppc64_book3e_allmodconfig build fails:
{standard input}: Assembler messages:
{standard input}:196: Error: unrecognized opcode: `lbarx'
{standard input}:196: Error: unrecognized opcode: `stbcx.'
make[5]: *** [scripts/Makefile.build:252:
arch/powerpc/mm/nohash/e500_hugetlbpage.o] Error 1
That happens because the default CPU for that config is e5500, set via
CONFIG_TARGET_CPU, and so the assembler is building for e5500, which
doesn't support those instructions.
Fix it by using machine directives to tell the assembler to assemble the
relevant code for e6500, which does support lbarx/stbcx.
That is safe because the code already has the CPU_FTR_SMT check, which
ensures the lbarx sequence doesn't run on e5500, which doesn't support
SMT.
Signed-off-by: Michael Ellerman
---
arch/powerpc/mm/nohash/e500_hugetlbpage.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/nohash/e500_hugetlbpage.c
b/arch/powerpc/mm/nohash/e500_hugetlbpage.c
index c7d4b317a823..58c8d9849cb1 100644
--- a/arch/powerpc/mm/nohash/e500_hugetlbpage.c
+++ b/arch/powerpc/mm/nohash/e500_hugetlbpage.c
@@ -45,7 +45,9 @@ static inline void book3e_tlb_lock(void)
if (!cpu_has_feature(CPU_FTR_SMT))
return;
- asm volatile("1: lbarx %0, 0, %1;"
+ asm volatile(".machine push;"
+".machine e6500;"
+"1: lbarx %0, 0, %1;"
"cmpwi %0, 0;"
"bne 2f;"
"stbcx. %2, 0, %1;"
@@ -56,6 +58,7 @@ static inline void book3e_tlb_lock(void)
"bne 2b;"
"b 1b;"
"3:"
+".machine pop;"
: "=" (tmp)
: "r" (>tcd_ptr->lock), "r" (token)
: "memory");
--
2.39.1
Re: [PATCH] powerpc/machdep: warn when machine_is() used too early
Christophe Leroy writes:
> Le 11/02/2023 à 00:56, Nathan Lynch via B4 Submission Endpoint a écrit :
>> From: Nathan Lynch
>>
>> machine_is() can't provide correct results before probe_machine() has
>> run. Warn when it's used too early in boot.
>>
>> Signed-off-by: Nathan Lynch
>> ---
>> Prompted by my attempts to do some pseries-specific setup during
>> rtas_initialize() and being puzzled for a while that it wasn't
>> working.
>> ---
>> arch/powerpc/include/asm/machdep.h | 12 +++-
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/machdep.h
>> b/arch/powerpc/include/asm/machdep.h
>> index 378b8d5836a7..8c0a799d18cd 100644
>> --- a/arch/powerpc/include/asm/machdep.h
>> +++ b/arch/powerpc/include/asm/machdep.h
>> @@ -220,11 +220,13 @@ extern struct machdep_calls *machine_id;
>> EXPORT_SYMBOL(mach_##name); \
>> struct machdep_calls mach_##name __machine_desc =
>>
>> -#define machine_is(name) \
>> -({ \
>> -extern struct machdep_calls mach_##name \
>> -__attribute__((weak)); \
>> -machine_id == _##name; \
>> +#define machine_is(name)\
>> +({ \
>> +extern struct machdep_calls mach_##name \
>> +__attribute__((weak)); \
>> +WARN(!machine_id, \
>> + "machine_is() called before probe_machine()"); \
>
> Is a WARN() really necessary ? WARN() is less optimised than WARN_ON(),
> especially on PPC64.
>
> This should never ever happen so a WARN_ON(!machine_id) should be
> enough, the developper that hits it is able to go to the given file:line
> and understand what happened.
Yeah I agree, WARN_ON() should be sufficient here, and should generate
slightly better code. We have > 100 uses of machine_is(), so keeping
each small is desirable.
cheers
Re: Build regressions/improvements in v6.2-rc8
On Mon, 13 Feb 2023, Geert Uytterhoeven wrote:
JFYI, when comparing v6.2-rc8[1] to v6.2-rc7[3], the summaries are:
- build errors: +11/-1
+ {standard input}: Error: unrecognized opcode: `dcbfl': => 5736, 4743,
4327, 4476, 4447, 5067, 4602, 5212, 5224, 4298, 5594, 4315, 5050, 5195, 4464, 5079
+ {standard input}: Error: unrecognized opcode: `dlmzb.': => 2848, 18800,
2842, 2383, 106, 2377, 3327, 112
+ {standard input}: Error: unrecognized opcode: `iccci': => 204, 163, 510
+ {standard input}: Error: unrecognized opcode: `lbarx': => 570, 196
+ {standard input}: Error: unrecognized opcode: `mbar': => 887, 558, 1172,
539, 516, 837, 1457, 1125, 815, 7523, 1100, 1385, 368, 703, 662, 468, 441, 1410
+ {standard input}: Error: unrecognized opcode: `mfdcr': => 3589, 4358,
3565, 3493, 3614, 128, 3445, 276, 3518, 3541, 3469, 4413
+ {standard input}: Error: unrecognized opcode: `mtdcr': => 265, 4402, 4430,
4375, 4388, 4347, 117, 4443
+ {standard input}: Error: unrecognized opcode: `stbcx.': => 196, 570
+ {standard input}: Error: unrecognized opcode: `tlbwe': => 475, 476, 477
powerpc-gcc11/ppc64_book3e_allmodconfig
powerpc-gcc11/powerpc-allmodconfig
powerpc-gcc11/corenet64_smp_defconfig
powerpc-gcc11/powerpc-allyesconfig
powerpc-gcc11/44x/fsp2_defconfig
[1]
http://kisskb.ellerman.id.au/kisskb/branch/linus/head/ceaa837f96adb69c0df0397937cd74991d5d821a/
(all 152 configs)
[3]
http://kisskb.ellerman.id.au/kisskb/branch/linus/head/4ec5183ec48656cec489c49f989c508b68b518e3/
(all 152 configs)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
