* [PATCH v14 08/12] powerpc: regain entire stack space
From: Christophe Leroy @ 2019-01-24 16:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nicholas Piggin, Mike Rapoport
Cc: Mark Rutland, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548346225.git.christophe.leroy@c-s.fr>
thread_info is not anymore in the stack, so the entire stack
can now be used.
There is also no risk anymore of corrupting task_cpu(p) with a
stack overflow so the patch removes the test.
When doing this, an explicit test for NULL stack pointer is
needed in validate_sp() as it is not anymore implicitely covered
by the sizeof(thread_info) gap.
In the meantime, with the previous patch all pointers to the stacks
are not anymore pointers to thread_info so this patch changes them
to void*
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/irq.h | 10 +++++-----
arch/powerpc/include/asm/processor.h | 3 +--
arch/powerpc/kernel/asm-offsets.c | 1 -
arch/powerpc/kernel/entry_32.S | 14 ++++----------
arch/powerpc/kernel/irq.c | 19 +++++++++----------
arch/powerpc/kernel/misc_32.S | 6 ++----
arch/powerpc/kernel/process.c | 32 +++++++++++++-------------------
arch/powerpc/kernel/setup_64.c | 8 ++++----
8 files changed, 38 insertions(+), 55 deletions(-)
diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index 28a7ace0a1b9..c91a60cda4fa 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -48,16 +48,16 @@ struct pt_regs;
* Per-cpu stacks for handling critical, debug and machine check
* level interrupts.
*/
-extern struct thread_info *critirq_ctx[NR_CPUS];
-extern struct thread_info *dbgirq_ctx[NR_CPUS];
-extern struct thread_info *mcheckirq_ctx[NR_CPUS];
+extern void *critirq_ctx[NR_CPUS];
+extern void *dbgirq_ctx[NR_CPUS];
+extern void *mcheckirq_ctx[NR_CPUS];
#endif
/*
* Per-cpu stacks for handling hard and soft interrupts.
*/
-extern struct thread_info *hardirq_ctx[NR_CPUS];
-extern struct thread_info *softirq_ctx[NR_CPUS];
+extern void *hardirq_ctx[NR_CPUS];
+extern void *softirq_ctx[NR_CPUS];
void call_do_softirq(void *sp);
void call_do_irq(struct pt_regs *regs, void *sp);
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 15acb282a876..8179b64871ed 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -325,8 +325,7 @@ struct thread_struct {
#define ARCH_MIN_TASKALIGN 16
#define INIT_SP (sizeof(init_stack) + (unsigned long) &init_stack)
-#define INIT_SP_LIMIT \
- (_ALIGN_UP(sizeof(struct thread_info), 16) + (unsigned long)&init_stack)
+#define INIT_SP_LIMIT ((unsigned long)&init_stack)
#ifdef CONFIG_SPE
#define SPEFSCR_INIT \
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 1fb52206c106..94ac190a0b16 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -92,7 +92,6 @@ int main(void)
DEFINE(SIGSEGV, SIGSEGV);
DEFINE(NMI_MASK, NMI_MASK);
#else
- DEFINE(THREAD_INFO_GAP, _ALIGN_UP(sizeof(struct thread_info), 16));
OFFSET(KSP_LIMIT, thread_struct, ksp_limit);
#endif /* CONFIG_PPC64 */
OFFSET(TASK_STACK, task_struct, stack);
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 29b9258dfff7..367dbf34c8a7 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -97,14 +97,11 @@ crit_transfer_to_handler:
mfspr r0,SPRN_SRR1
stw r0,_SRR1(r11)
- /* set the stack limit to the current stack
- * and set the limit to protect the thread_info
- * struct
- */
+ /* set the stack limit to the current stack */
mfspr r8,SPRN_SPRG_THREAD
lwz r0,KSP_LIMIT(r8)
stw r0,SAVED_KSP_LIMIT(r11)
- rlwimi r0,r1,0,0,(31-THREAD_SHIFT)
+ rlwinm r0,r1,0,0,(31 - THREAD_SHIFT)
stw r0,KSP_LIMIT(r8)
/* fall through */
#endif
@@ -121,14 +118,11 @@ crit_transfer_to_handler:
mfspr r0,SPRN_SRR1
stw r0,crit_srr1@l(0)
- /* set the stack limit to the current stack
- * and set the limit to protect the thread_info
- * struct
- */
+ /* set the stack limit to the current stack */
mfspr r8,SPRN_SPRG_THREAD
lwz r0,KSP_LIMIT(r8)
stw r0,saved_ksp_limit@l(0)
- rlwimi r0,r1,0,0,(31-THREAD_SHIFT)
+ rlwinm r0,r1,0,0,(31 - THREAD_SHIFT)
stw r0,KSP_LIMIT(r8)
/* fall through */
#endif
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 85c48911938a..938944c6e2ee 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -618,9 +618,8 @@ static inline void check_stack_overflow(void)
sp = current_stack_pointer() & (THREAD_SIZE-1);
/* check for stack overflow: is there less than 2KB free? */
- if (unlikely(sp < (sizeof(struct thread_info) + 2048))) {
- pr_err("do_IRQ: stack overflow: %ld\n",
- sp - sizeof(struct thread_info));
+ if (unlikely(sp < 2048)) {
+ pr_err("do_IRQ: stack overflow: %ld\n", sp);
dump_stack();
}
#endif
@@ -660,7 +659,7 @@ void __do_irq(struct pt_regs *regs)
void do_IRQ(struct pt_regs *regs)
{
struct pt_regs *old_regs = set_irq_regs(regs);
- struct thread_info *curtp, *irqtp, *sirqtp;
+ void *curtp, *irqtp, *sirqtp;
/* Switch to the irq stack to handle this */
curtp = (void *)(current_stack_pointer() & ~(THREAD_SIZE - 1));
@@ -686,17 +685,17 @@ void __init init_IRQ(void)
}
#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
-struct thread_info *critirq_ctx[NR_CPUS] __read_mostly;
-struct thread_info *dbgirq_ctx[NR_CPUS] __read_mostly;
-struct thread_info *mcheckirq_ctx[NR_CPUS] __read_mostly;
+void *critirq_ctx[NR_CPUS] __read_mostly;
+void *dbgirq_ctx[NR_CPUS] __read_mostly;
+void *mcheckirq_ctx[NR_CPUS] __read_mostly;
#endif
-struct thread_info *softirq_ctx[NR_CPUS] __read_mostly;
-struct thread_info *hardirq_ctx[NR_CPUS] __read_mostly;
+void *softirq_ctx[NR_CPUS] __read_mostly;
+void *hardirq_ctx[NR_CPUS] __read_mostly;
void do_softirq_own_stack(void)
{
- struct thread_info *irqtp;
+ void *irqtp;
irqtp = softirq_ctx[smp_processor_id()];
call_do_softirq(irqtp);
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index b37b50fde828..6f6127c3760c 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -46,11 +46,10 @@ _GLOBAL(call_do_softirq)
mflr r0
stw r0,4(r1)
lwz r10,THREAD+KSP_LIMIT(r2)
- addi r11,r3,THREAD_INFO_GAP
+ stw r3, THREAD+KSP_LIMIT(r2)
stwu r1,THREAD_SIZE-STACK_FRAME_OVERHEAD(r3)
mr r1,r3
stw r10,8(r1)
- stw r11,THREAD+KSP_LIMIT(r2)
bl __do_softirq
lwz r10,8(r1)
lwz r1,0(r1)
@@ -66,11 +65,10 @@ _GLOBAL(call_do_irq)
mflr r0
stw r0,4(r1)
lwz r10,THREAD+KSP_LIMIT(r2)
- addi r11,r4,THREAD_INFO_GAP
+ stw r4, THREAD+KSP_LIMIT(r2)
stwu r1,THREAD_SIZE-STACK_FRAME_OVERHEAD(r4)
mr r1,r4
stw r10,8(r1)
- stw r11,THREAD+KSP_LIMIT(r2)
bl __do_irq
lwz r10,8(r1)
lwz r1,0(r1)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index d3c8af4c3a61..da82ab5dd743 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1691,8 +1691,7 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
sp -= STACK_FRAME_OVERHEAD;
p->thread.ksp = sp;
#ifdef CONFIG_PPC32
- p->thread.ksp_limit = (unsigned long)task_stack_page(p) +
- _ALIGN_UP(sizeof(struct thread_info), 16);
+ p->thread.ksp_limit = (unsigned long)end_of_stack(p);
#endif
#ifdef CONFIG_HAVE_HW_BREAKPOINT
p->thread.ptrace_bps[0] = NULL;
@@ -1995,21 +1994,14 @@ static inline int valid_irq_stack(unsigned long sp, struct task_struct *p,
unsigned long stack_page;
unsigned long cpu = task_cpu(p);
- /*
- * Avoid crashing if the stack has overflowed and corrupted
- * task_cpu(p), which is in the thread_info struct.
- */
- if (cpu < NR_CPUS && cpu_possible(cpu)) {
- stack_page = (unsigned long) hardirq_ctx[cpu];
- if (sp >= stack_page + sizeof(struct thread_struct)
- && sp <= stack_page + THREAD_SIZE - nbytes)
- return 1;
-
- stack_page = (unsigned long) softirq_ctx[cpu];
- if (sp >= stack_page + sizeof(struct thread_struct)
- && sp <= stack_page + THREAD_SIZE - nbytes)
- return 1;
- }
+ stack_page = (unsigned long)hardirq_ctx[cpu];
+ if (sp >= stack_page && sp <= stack_page + THREAD_SIZE - nbytes)
+ return 1;
+
+ stack_page = (unsigned long)softirq_ctx[cpu];
+ if (sp >= stack_page && sp <= stack_page + THREAD_SIZE - nbytes)
+ return 1;
+
return 0;
}
@@ -2018,8 +2010,10 @@ int validate_sp(unsigned long sp, struct task_struct *p,
{
unsigned long stack_page = (unsigned long)task_stack_page(p);
- if (sp >= stack_page + sizeof(struct thread_struct)
- && sp <= stack_page + THREAD_SIZE - nbytes)
+ if (sp < THREAD_SIZE)
+ return 0;
+
+ if (sp >= stack_page && sp <= stack_page + THREAD_SIZE - nbytes)
return 1;
return valid_irq_stack(sp, p, nbytes);
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 0912948a8ea6..2db1c5f7d141 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -716,19 +716,19 @@ void __init emergency_stack_init(void)
limit = min(ppc64_bolted_size(), ppc64_rma_size);
for_each_possible_cpu(i) {
- struct thread_info *ti;
+ void *ti;
ti = alloc_stack(limit, i);
- paca_ptrs[i]->emergency_sp = (void *)ti + THREAD_SIZE;
+ paca_ptrs[i]->emergency_sp = ti + THREAD_SIZE;
#ifdef CONFIG_PPC_BOOK3S_64
/* emergency stack for NMI exception handling. */
ti = alloc_stack(limit, i);
- paca_ptrs[i]->nmi_emergency_sp = (void *)ti + THREAD_SIZE;
+ paca_ptrs[i]->nmi_emergency_sp = ti + THREAD_SIZE;
/* emergency stack for machine check exception handling. */
ti = alloc_stack(limit, i);
- paca_ptrs[i]->mc_emergency_sp = (void *)ti + THREAD_SIZE;
+ paca_ptrs[i]->mc_emergency_sp = ti + THREAD_SIZE;
#endif
}
}
--
2.13.3
^ permalink raw reply related
* [PATCH v14 09/12] powerpc: 'current_set' is now a table of task_struct pointers
From: Christophe Leroy @ 2019-01-24 16:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nicholas Piggin, Mike Rapoport
Cc: Mark Rutland, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548346225.git.christophe.leroy@c-s.fr>
The table of pointers 'current_set' has been used for retrieving
the stack and current. They used to be thread_info pointers as
they were pointing to the stack and current was taken from the
'task' field of the thread_info.
Now, the pointers of 'current_set' table are now both pointers
to task_struct and pointers to thread_info.
As they are used to get current, and the stack pointer is
retrieved from current's stack field, this patch changes
their type to task_struct, and renames secondary_ti to
secondary_current.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/asm-prototypes.h | 4 ++--
arch/powerpc/kernel/head_32.S | 6 +++---
arch/powerpc/kernel/head_44x.S | 4 ++--
arch/powerpc/kernel/head_fsl_booke.S | 4 ++--
arch/powerpc/kernel/smp.c | 10 ++++------
5 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 1d911f68a23b..1484df6779ab 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -23,8 +23,8 @@
#include <uapi/asm/ucontext.h>
/* SMP */
-extern struct thread_info *current_set[NR_CPUS];
-extern struct thread_info *secondary_ti;
+extern struct task_struct *current_set[NR_CPUS];
+extern struct task_struct *secondary_current;
void start_secondary(void *unused);
/* kexec */
diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index 309a45779ad5..146385b1c2da 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -846,9 +846,9 @@ __secondary_start:
#endif /* CONFIG_PPC_BOOK3S_32 */
/* get current's stack and current */
- lis r1,secondary_ti@ha
- tophys(r1,r1)
- lwz r2,secondary_ti@l(r1)
+ lis r2,secondary_current@ha
+ tophys(r2,r2)
+ lwz r2,secondary_current@l(r2)
tophys(r1,r2)
lwz r1,TASK_STACK(r1)
diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
index f94a93b6c2f2..37117ab11584 100644
--- a/arch/powerpc/kernel/head_44x.S
+++ b/arch/powerpc/kernel/head_44x.S
@@ -1020,8 +1020,8 @@ _GLOBAL(start_secondary_47x)
/* Now we can get our task struct and real stack pointer */
/* Get current's stack and current */
- lis r1,secondary_ti@ha
- lwz r2,secondary_ti@l(r1)
+ lis r2,secondary_current@ha
+ lwz r2,secondary_current@l(r2)
lwz r1,TASK_STACK(r2)
/* Current stack pointer */
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 11f38adbe020..4ed2a7c8e89b 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -1091,8 +1091,8 @@ __secondary_start:
bl call_setup_cpu
/* get current's stack and current */
- lis r1,secondary_ti@ha
- lwz r2,secondary_ti@l(r1)
+ lis r2,secondary_current@ha
+ lwz r2,secondary_current@l(r2)
lwz r1,TASK_STACK(r2)
/* stack */
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index aa4517686f90..a41fa8924004 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -76,7 +76,7 @@
static DEFINE_PER_CPU(int, cpu_state) = { 0 };
#endif
-struct thread_info *secondary_ti;
+struct task_struct *secondary_current;
bool has_big_cores;
DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
@@ -664,7 +664,7 @@ void smp_send_stop(void)
}
#endif /* CONFIG_NMI_IPI */
-struct thread_info *current_set[NR_CPUS];
+struct task_struct *current_set[NR_CPUS];
static void smp_store_cpu_info(int id)
{
@@ -929,7 +929,7 @@ void smp_prepare_boot_cpu(void)
paca_ptrs[boot_cpuid]->__current = current;
#endif
set_numa_node(numa_cpu_lookup_table[boot_cpuid]);
- current_set[boot_cpuid] = task_thread_info(current);
+ current_set[boot_cpuid] = current;
}
#ifdef CONFIG_HOTPLUG_CPU
@@ -1014,15 +1014,13 @@ static bool secondaries_inhibited(void)
static void cpu_idle_thread_init(unsigned int cpu, struct task_struct *idle)
{
- struct thread_info *ti = task_thread_info(idle);
-
#ifdef CONFIG_PPC64
paca_ptrs[cpu]->__current = idle;
paca_ptrs[cpu]->kstack = (unsigned long)task_stack_page(idle) +
THREAD_SIZE - STACK_FRAME_OVERHEAD;
#endif
idle->cpu = cpu;
- secondary_ti = current_set[cpu] = ti;
+ secondary_current = current_set[cpu] = idle;
}
int __cpu_up(unsigned int cpu, struct task_struct *tidle)
--
2.13.3
^ permalink raw reply related
* [PATCH v14 10/12] powerpc/32: Remove CURRENT_THREAD_INFO and rename TI_CPU
From: Christophe Leroy @ 2019-01-24 16:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nicholas Piggin, Mike Rapoport
Cc: Mark Rutland, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548346225.git.christophe.leroy@c-s.fr>
Now that thread_info is similar to task_struct, its address is in r2
so CURRENT_THREAD_INFO() macro is useless. This patch removes it.
At the same time, as the 'cpu' field is not anymore in thread_info,
this patch renames it to TASK_CPU.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/Makefile | 2 +-
arch/powerpc/include/asm/thread_info.h | 2 --
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/kernel/entry_32.S | 43 ++++++++++++----------------------
arch/powerpc/kernel/epapr_hcalls.S | 5 ++--
arch/powerpc/kernel/head_fsl_booke.S | 5 ++--
arch/powerpc/kernel/idle_6xx.S | 8 +++----
arch/powerpc/kernel/idle_e500.S | 8 +++----
arch/powerpc/kernel/misc_32.S | 3 +--
arch/powerpc/mm/hash_low_32.S | 14 ++++-------
arch/powerpc/sysdev/6xx-suspend.S | 5 ++--
11 files changed, 35 insertions(+), 62 deletions(-)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 53ffe935f3b0..7de49889bd5d 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -431,7 +431,7 @@ ifdef CONFIG_SMP
prepare: task_cpu_prepare
task_cpu_prepare: prepare0
- $(eval KBUILD_CFLAGS += -D_TASK_CPU=$(shell awk '{if ($$2 == "TI_CPU") print $$3;}' include/generated/asm-offsets.h))
+ $(eval KBUILD_CFLAGS += -D_TASK_CPU=$(shell awk '{if ($$2 == "TASK_CPU") print $$3;}' include/generated/asm-offsets.h))
endif
# Check toolchain versions:
diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index d91523c2c7d8..c959b8d66cac 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -19,8 +19,6 @@
#ifdef CONFIG_PPC64
#define CURRENT_THREAD_INFO(dest, sp) stringify_in_c(ld dest, PACACURRENT(r13))
-#else
-#define CURRENT_THREAD_INFO(dest, sp) stringify_in_c(mr dest, r2)
#endif
#ifndef __ASSEMBLY__
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 94ac190a0b16..03439785c2ea 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -96,7 +96,7 @@ int main(void)
#endif /* CONFIG_PPC64 */
OFFSET(TASK_STACK, task_struct, stack);
#ifdef CONFIG_SMP
- OFFSET(TI_CPU, task_struct, cpu);
+ OFFSET(TASK_CPU, task_struct, cpu);
#endif
#ifdef CONFIG_LIVEPATCH
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 367dbf34c8a7..bc85d4320283 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -168,8 +168,7 @@ transfer_to_handler:
tophys(r11,r11)
addi r11,r11,global_dbcr0@l
#ifdef CONFIG_SMP
- CURRENT_THREAD_INFO(r9, r1)
- lwz r9,TI_CPU(r9)
+ lwz r9,TASK_CPU(r2)
slwi r9,r9,3
add r11,r11,r9
#endif
@@ -189,8 +188,7 @@ transfer_to_handler:
ble- stack_ovf /* then the kernel stack overflowed */
5:
#if defined(CONFIG_PPC_BOOK3S_32) || defined(CONFIG_E500)
- CURRENT_THREAD_INFO(r9, r1)
- tophys(r9,r9) /* check local flags */
+ tophys(r9,r2) /* check local flags */
lwz r12,TI_LOCAL_FLAGS(r9)
mtcrf 0x01,r12
bt- 31-TLF_NAPPING,4f
@@ -198,8 +196,7 @@ transfer_to_handler:
#endif /* CONFIG_PPC_BOOK3S_32 || CONFIG_E500 */
3:
#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
- CURRENT_THREAD_INFO(r9, r1)
- tophys(r9, r9)
+ tophys(r9, r2)
ACCOUNT_CPU_USER_ENTRY(r9, r11, r12)
#endif
.globl transfer_to_handler_cont
@@ -344,8 +341,7 @@ _GLOBAL(DoSyscall)
mtmsr r11
1:
#endif /* CONFIG_TRACE_IRQFLAGS */
- CURRENT_THREAD_INFO(r10, r1)
- lwz r11,TI_FLAGS(r10)
+ lwz r11,TI_FLAGS(r2)
andi. r11,r11,_TIF_SYSCALL_DOTRACE
bne- syscall_dotrace
syscall_dotrace_cont:
@@ -378,13 +374,12 @@ ret_from_syscall:
lwz r3,GPR3(r1)
#endif
mr r6,r3
- CURRENT_THREAD_INFO(r12, r1)
/* disable interrupts so current_thread_info()->flags can't change */
LOAD_MSR_KERNEL(r10,MSR_KERNEL) /* doesn't include MSR_EE */
/* Note: We don't bother telling lockdep about it */
SYNC
MTMSRD(r10)
- lwz r9,TI_FLAGS(r12)
+ lwz r9,TI_FLAGS(r2)
li r8,-MAX_ERRNO
andi. r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
bne- syscall_exit_work
@@ -431,8 +426,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
andi. r4,r8,MSR_PR
beq 3f
- CURRENT_THREAD_INFO(r4, r1)
- ACCOUNT_CPU_USER_EXIT(r4, r5, r7)
+ ACCOUNT_CPU_USER_EXIT(r2, r5, r7)
3:
#endif
lwz r4,_LINK(r1)
@@ -525,7 +519,7 @@ syscall_exit_work:
/* Clear per-syscall TIF flags if any are set. */
li r11,_TIF_PERSYSCALL_MASK
- addi r12,r12,TI_FLAGS
+ addi r12,r2,TI_FLAGS
3: lwarx r8,0,r12
andc r8,r8,r11
#ifdef CONFIG_IBM405_ERR77
@@ -533,7 +527,6 @@ syscall_exit_work:
#endif
stwcx. r8,0,r12
bne- 3b
- subi r12,r12,TI_FLAGS
4: /* Anything which requires enabling interrupts? */
andi. r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP)
@@ -812,8 +805,7 @@ ret_from_except:
user_exc_return: /* r10 contains MSR_KERNEL here */
/* Check current_thread_info()->flags */
- CURRENT_THREAD_INFO(r9, r1)
- lwz r9,TI_FLAGS(r9)
+ lwz r9,TI_FLAGS(r2)
andi. r0,r9,_TIF_USER_WORK_MASK
bne do_work
@@ -826,8 +818,7 @@ restore_user:
bnel- load_dbcr0
#endif
#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
- CURRENT_THREAD_INFO(r9, r1)
- ACCOUNT_CPU_USER_EXIT(r9, r10, r11)
+ ACCOUNT_CPU_USER_EXIT(r2, r10, r11)
#endif
b restore
@@ -835,8 +826,7 @@ restore_user:
/* N.B. the only way to get here is from the beq following ret_from_except. */
resume_kernel:
/* check current_thread_info, _TIF_EMULATE_STACK_STORE */
- CURRENT_THREAD_INFO(r9, r1)
- lwz r8,TI_FLAGS(r9)
+ lwz r8,TI_FLAGS(r2)
andis. r0,r8,_TIF_EMULATE_STACK_STORE@h
beq+ 1f
@@ -862,7 +852,7 @@ resume_kernel:
/* Clear _TIF_EMULATE_STACK_STORE flag */
lis r11,_TIF_EMULATE_STACK_STORE@h
- addi r5,r9,TI_FLAGS
+ addi r5,r2,TI_FLAGS
0: lwarx r8,0,r5
andc r8,r8,r11
#ifdef CONFIG_IBM405_ERR77
@@ -874,7 +864,7 @@ resume_kernel:
#ifdef CONFIG_PREEMPT
/* check current_thread_info->preempt_count */
- lwz r0,TI_PREEMPT(r9)
+ lwz r0,TI_PREEMPT(r2)
cmpwi 0,r0,0 /* if non-zero, just restore regs and return */
bne restore
andi. r8,r8,_TIF_NEED_RESCHED
@@ -890,8 +880,7 @@ resume_kernel:
bl trace_hardirqs_off
#endif
1: bl preempt_schedule_irq
- CURRENT_THREAD_INFO(r9, r1)
- lwz r3,TI_FLAGS(r9)
+ lwz r3,TI_FLAGS(r2)
andi. r0,r3,_TIF_NEED_RESCHED
bne- 1b
#ifdef CONFIG_TRACE_IRQFLAGS
@@ -1190,8 +1179,7 @@ load_dbcr0:
lis r11,global_dbcr0@ha
addi r11,r11,global_dbcr0@l
#ifdef CONFIG_SMP
- CURRENT_THREAD_INFO(r9, r1)
- lwz r9,TI_CPU(r9)
+ lwz r9,TASK_CPU(r2)
slwi r9,r9,3
add r11,r11,r9
#endif
@@ -1231,8 +1219,7 @@ recheck:
LOAD_MSR_KERNEL(r10,MSR_KERNEL)
SYNC
MTMSRD(r10) /* disable interrupts */
- CURRENT_THREAD_INFO(r9, r1)
- lwz r9,TI_FLAGS(r9)
+ lwz r9,TI_FLAGS(r2)
andi. r0,r9,_TIF_NEED_RESCHED
bne- do_resched
andi. r0,r9,_TIF_USER_WORK_MASK
diff --git a/arch/powerpc/kernel/epapr_hcalls.S b/arch/powerpc/kernel/epapr_hcalls.S
index 52ca2471ee1a..d252f4663a23 100644
--- a/arch/powerpc/kernel/epapr_hcalls.S
+++ b/arch/powerpc/kernel/epapr_hcalls.S
@@ -21,10 +21,9 @@
#ifndef CONFIG_PPC64
/* epapr_ev_idle() was derived from e500_idle() */
_GLOBAL(epapr_ev_idle)
- CURRENT_THREAD_INFO(r3, r1)
- PPC_LL r4, TI_LOCAL_FLAGS(r3) /* set napping bit */
+ PPC_LL r4, TI_LOCAL_FLAGS(r2) /* set napping bit */
ori r4, r4,_TLF_NAPPING /* so when we take an exception */
- PPC_STL r4, TI_LOCAL_FLAGS(r3) /* it will return to our caller */
+ PPC_STL r4, TI_LOCAL_FLAGS(r2) /* it will return to our caller */
wrteei 1
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 4ed2a7c8e89b..1881127682e9 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -244,8 +244,7 @@ set_ivor:
stwu r0,THREAD_SIZE-STACK_FRAME_OVERHEAD(r1)
#ifdef CONFIG_SMP
- CURRENT_THREAD_INFO(r22, r1)
- stw r24, TI_CPU(r22)
+ stw r24, TASK_CPU(r2)
#endif
bl early_init
@@ -719,7 +718,7 @@ finish_tlb_load:
/* Get the next_tlbcam_idx percpu var */
#ifdef CONFIG_SMP
- lwz r15, TI_CPU-THREAD(r12)
+ lwz r15, TASK_CPU-THREAD(r12)
lis r14, __per_cpu_offset@h
ori r14, r14, __per_cpu_offset@l
rlwinm r15, r15, 2, 0, 29
diff --git a/arch/powerpc/kernel/idle_6xx.S b/arch/powerpc/kernel/idle_6xx.S
index ff026c9d3cab..5afd2e236990 100644
--- a/arch/powerpc/kernel/idle_6xx.S
+++ b/arch/powerpc/kernel/idle_6xx.S
@@ -136,10 +136,9 @@ BEGIN_FTR_SECTION
DSSALL
sync
END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
- CURRENT_THREAD_INFO(r9, r1)
- lwz r8,TI_LOCAL_FLAGS(r9) /* set napping bit */
+ lwz r8,TI_LOCAL_FLAGS(r2) /* set napping bit */
ori r8,r8,_TLF_NAPPING /* so when we take an exception */
- stw r8,TI_LOCAL_FLAGS(r9) /* it will return to our caller */
+ stw r8,TI_LOCAL_FLAGS(r2) /* it will return to our caller */
mfmsr r7
ori r7,r7,MSR_EE
oris r7,r7,MSR_POW@h
@@ -159,8 +158,7 @@ _GLOBAL(power_save_ppc32_restore)
stw r9,_NIP(r11) /* make it do a blr */
#ifdef CONFIG_SMP
- CURRENT_THREAD_INFO(r12, r11)
- lwz r11,TI_CPU(r12) /* get cpu number * 4 */
+ lwz r11,TASK_CPU(r2) /* get cpu number * 4 */
slwi r11,r11,2
#else
li r11,0
diff --git a/arch/powerpc/kernel/idle_e500.S b/arch/powerpc/kernel/idle_e500.S
index 583e55ac7d26..69dfcd2ca011 100644
--- a/arch/powerpc/kernel/idle_e500.S
+++ b/arch/powerpc/kernel/idle_e500.S
@@ -22,10 +22,9 @@
.text
_GLOBAL(e500_idle)
- CURRENT_THREAD_INFO(r3, r1)
- lwz r4,TI_LOCAL_FLAGS(r3) /* set napping bit */
+ lwz r4,TI_LOCAL_FLAGS(r2) /* set napping bit */
ori r4,r4,_TLF_NAPPING /* so when we take an exception */
- stw r4,TI_LOCAL_FLAGS(r3) /* it will return to our caller */
+ stw r4,TI_LOCAL_FLAGS(r2) /* it will return to our caller */
#ifdef CONFIG_PPC_E500MC
wrteei 1
@@ -88,8 +87,7 @@ _GLOBAL(power_save_ppc32_restore)
stw r9,_NIP(r11) /* make it do a blr */
#ifdef CONFIG_SMP
- CURRENT_THREAD_INFO(r12, r1)
- lwz r11,TI_CPU(r12) /* get cpu number * 4 */
+ lwz r11,TASK_CPU(r2) /* get cpu number * 4 */
slwi r11,r11,2
#else
li r11,0
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index 6f6127c3760c..0dda4f8e3d7a 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -183,8 +183,7 @@ _GLOBAL(low_choose_750fx_pll)
#ifdef CONFIG_SMP
/* Store new HID1 image */
- CURRENT_THREAD_INFO(r6, r1)
- lwz r6,TI_CPU(r6)
+ lwz r6,TASK_CPU(r2)
slwi r6,r6,2
#else
li r6, 0
diff --git a/arch/powerpc/mm/hash_low_32.S b/arch/powerpc/mm/hash_low_32.S
index 1e2df3e9f9ea..5bee2d982959 100644
--- a/arch/powerpc/mm/hash_low_32.S
+++ b/arch/powerpc/mm/hash_low_32.S
@@ -186,8 +186,7 @@ _GLOBAL(add_hash_page)
add r3,r3,r0 /* note create_hpte trims to 24 bits */
#ifdef CONFIG_SMP
- CURRENT_THREAD_INFO(r8, r1) /* use cpu number to make tag */
- lwz r8,TI_CPU(r8) /* to go in mmu_hash_lock */
+ lwz r8,TASK_CPU(r2) /* to go in mmu_hash_lock */
oris r8,r8,12
#endif /* CONFIG_SMP */
@@ -549,9 +548,8 @@ _GLOBAL(flush_hash_pages)
#ifdef CONFIG_SMP
addis r9,r7,mmu_hash_lock@ha
addi r9,r9,mmu_hash_lock@l
- CURRENT_THREAD_INFO(r8, r1)
- add r8,r8,r7
- lwz r8,TI_CPU(r8)
+ add r8,r2,r7
+ lwz r8,TASK_CPU(r8)
oris r8,r8,9
10: lwarx r0,0,r9
cmpi 0,r0,0
@@ -646,8 +644,7 @@ EXPORT_SYMBOL(flush_hash_pages)
*/
_GLOBAL(_tlbie)
#ifdef CONFIG_SMP
- CURRENT_THREAD_INFO(r8, r1)
- lwz r8,TI_CPU(r8)
+ lwz r8,TASK_CPU(r2)
oris r8,r8,11
mfmsr r10
SYNC
@@ -684,8 +681,7 @@ _GLOBAL(_tlbie)
*/
_GLOBAL(_tlbia)
#if defined(CONFIG_SMP)
- CURRENT_THREAD_INFO(r8, r1)
- lwz r8,TI_CPU(r8)
+ lwz r8,TASK_CPU(r2)
oris r8,r8,10
mfmsr r10
SYNC
diff --git a/arch/powerpc/sysdev/6xx-suspend.S b/arch/powerpc/sysdev/6xx-suspend.S
index cf48e9cb2575..6c4aec25c4ba 100644
--- a/arch/powerpc/sysdev/6xx-suspend.S
+++ b/arch/powerpc/sysdev/6xx-suspend.S
@@ -29,10 +29,9 @@ _GLOBAL(mpc6xx_enter_standby)
ori r5, r5, ret_from_standby@l
mtlr r5
- CURRENT_THREAD_INFO(r5, r1)
- lwz r6, TI_LOCAL_FLAGS(r5)
+ lwz r6, TI_LOCAL_FLAGS(r2)
ori r6, r6, _TLF_SLEEPING
- stw r6, TI_LOCAL_FLAGS(r5)
+ stw r6, TI_LOCAL_FLAGS(r2)
mfmsr r5
ori r5, r5, MSR_EE
--
2.13.3
^ permalink raw reply related
* [PATCH v14 11/12] powerpc/64: Remove CURRENT_THREAD_INFO
From: Christophe Leroy @ 2019-01-24 16:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nicholas Piggin, Mike Rapoport
Cc: Mark Rutland, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548346225.git.christophe.leroy@c-s.fr>
Now that current_thread_info is located at the beginning of 'current'
task struct, CURRENT_THREAD_INFO macro is not really needed any more.
This patch replaces it by loads of the value at PACACURRENT(r13).
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/exception-64s.h | 4 ++--
arch/powerpc/include/asm/thread_info.h | 4 ----
arch/powerpc/kernel/entry_64.S | 10 +++++-----
arch/powerpc/kernel/exceptions-64e.S | 2 +-
arch/powerpc/kernel/exceptions-64s.S | 2 +-
arch/powerpc/kernel/idle_book3e.S | 2 +-
arch/powerpc/kernel/idle_power4.S | 2 +-
arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 6 +++---
8 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
index 3b4767ed3ec5..dd6a5ae7a769 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -671,7 +671,7 @@ END_FTR_SECTION_NESTED(ftr,ftr,943)
#define RUNLATCH_ON \
BEGIN_FTR_SECTION \
- CURRENT_THREAD_INFO(r3, r1); \
+ ld r3, PACACURRENT(r13); \
ld r4,TI_LOCAL_FLAGS(r3); \
andi. r0,r4,_TLF_RUNLATCH; \
beql ppc64_runlatch_on_trampoline; \
@@ -721,7 +721,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CTRL)
#ifdef CONFIG_PPC_970_NAP
#define FINISH_NAP \
BEGIN_FTR_SECTION \
- CURRENT_THREAD_INFO(r11, r1); \
+ ld r11, PACACURRENT(r13); \
ld r9,TI_LOCAL_FLAGS(r11); \
andi. r10,r9,_TLF_NAPPING; \
bnel power4_fixup_nap; \
diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index c959b8d66cac..8e1d0195ac36 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -17,10 +17,6 @@
#define THREAD_SIZE (1 << THREAD_SHIFT)
-#ifdef CONFIG_PPC64
-#define CURRENT_THREAD_INFO(dest, sp) stringify_in_c(ld dest, PACACURRENT(r13))
-#endif
-
#ifndef __ASSEMBLY__
#include <linux/cache.h>
#include <asm/processor.h>
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 01d0706d873f..83bddacd7a17 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -166,7 +166,7 @@ system_call: /* label this so stack traces look sane */
li r10,IRQS_ENABLED
std r10,SOFTE(r1)
- CURRENT_THREAD_INFO(r11, r1)
+ ld r11, PACACURRENT(r13)
ld r10,TI_FLAGS(r11)
andi. r11,r10,_TIF_SYSCALL_DOTRACE
bne .Lsyscall_dotrace /* does not return */
@@ -213,7 +213,7 @@ system_call: /* label this so stack traces look sane */
ld r3,RESULT(r1)
#endif
- CURRENT_THREAD_INFO(r12, r1)
+ ld r12, PACACURRENT(r13)
ld r8,_MSR(r1)
#ifdef CONFIG_PPC_BOOK3S
@@ -348,7 +348,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
/* Repopulate r9 and r10 for the syscall path */
addi r9,r1,STACK_FRAME_OVERHEAD
- CURRENT_THREAD_INFO(r10, r1)
+ ld r10, PACACURRENT(r13)
ld r10,TI_FLAGS(r10)
cmpldi r0,NR_syscalls
@@ -746,7 +746,7 @@ _GLOBAL(ret_from_except_lite)
mtmsrd r10,1 /* Update machine state */
#endif /* CONFIG_PPC_BOOK3E */
- CURRENT_THREAD_INFO(r9, r1)
+ ld r9, PACACURRENT(r13)
ld r3,_MSR(r1)
#ifdef CONFIG_PPC_BOOK3E
ld r10,PACACURRENT(r13)
@@ -860,7 +860,7 @@ resume_kernel:
1: bl preempt_schedule_irq
/* Re-test flags and eventually loop */
- CURRENT_THREAD_INFO(r9, r1)
+ ld r9, PACACURRENT(r13)
ld r4,TI_FLAGS(r9)
andi. r0,r4,_TIF_NEED_RESCHED
bne 1b
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 20f14996281d..04ee24789f80 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -493,7 +493,7 @@ exc_##n##_bad_stack: \
* interrupts happen before the wait instruction.
*/
#define CHECK_NAPPING() \
- CURRENT_THREAD_INFO(r11, r1); \
+ ld r11, PACACURRENT(r13); \
ld r10,TI_LOCAL_FLAGS(r11); \
andi. r9,r10,_TLF_NAPPING; \
beq+ 1f; \
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 9e253ce27e08..c7c4e2d6f98f 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1629,7 +1629,7 @@ do_hash_page:
ori r0,r0,DSISR_BAD_FAULT_64S@l
and. r0,r4,r0 /* weird error? */
bne- handle_page_fault /* if not, try to insert a HPTE */
- CURRENT_THREAD_INFO(r11, r1)
+ ld r11, PACACURRENT(r13)
lwz r0,TI_PREEMPT(r11) /* If we're in an "NMI" */
andis. r0,r0,NMI_MASK@h /* (i.e. an irq when soft-disabled) */
bne 77f /* then don't call hash_page now */
diff --git a/arch/powerpc/kernel/idle_book3e.S b/arch/powerpc/kernel/idle_book3e.S
index 4e0d94d02030..31e732c378ad 100644
--- a/arch/powerpc/kernel/idle_book3e.S
+++ b/arch/powerpc/kernel/idle_book3e.S
@@ -63,7 +63,7 @@ _GLOBAL(\name)
1: /* Let's set the _TLF_NAPPING flag so interrupts make us return
* to the right spot
*/
- CURRENT_THREAD_INFO(r11, r1)
+ ld r11, PACACURRENT(r13)
ld r10,TI_LOCAL_FLAGS(r11)
ori r10,r10,_TLF_NAPPING
std r10,TI_LOCAL_FLAGS(r11)
diff --git a/arch/powerpc/kernel/idle_power4.S b/arch/powerpc/kernel/idle_power4.S
index a09b3c7ca176..61ac89fd0a05 100644
--- a/arch/powerpc/kernel/idle_power4.S
+++ b/arch/powerpc/kernel/idle_power4.S
@@ -68,7 +68,7 @@ BEGIN_FTR_SECTION
DSSALL
sync
END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
- CURRENT_THREAD_INFO(r9, r1)
+ ld r9, PACACURRENT(r13)
ld r8,TI_LOCAL_FLAGS(r9) /* set napping bit */
ori r8,r8,_TLF_NAPPING /* so when we take an exception */
std r8,TI_LOCAL_FLAGS(r9) /* it will return to our caller */
diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index 32476a6e4e9c..202bec086e3b 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -229,7 +229,7 @@ ftrace_call:
* - r0, r11 & r12 are free
*/
livepatch_handler:
- CURRENT_THREAD_INFO(r12, r1)
+ ld r12, PACACURRENT(r13)
/* Allocate 3 x 8 bytes */
ld r11, TI_livepatch_sp(r12)
@@ -256,7 +256,7 @@ livepatch_handler:
* restore it.
*/
- CURRENT_THREAD_INFO(r12, r1)
+ ld r12, PACACURRENT(r13)
ld r11, TI_livepatch_sp(r12)
@@ -273,7 +273,7 @@ livepatch_handler:
ld r2, -24(r11)
/* Pop livepatch stack frame */
- CURRENT_THREAD_INFO(r12, r1)
+ ld r12, PACACURRENT(r13)
subi r11, r11, 24
std r11, TI_livepatch_sp(r12)
--
2.13.3
^ permalink raw reply related
* [PATCH v14 12/12] powerpc: clean stack pointers naming
From: Christophe Leroy @ 2019-01-24 16:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nicholas Piggin, Mike Rapoport
Cc: Mark Rutland, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548346225.git.christophe.leroy@c-s.fr>
Some stack pointers used to also be thread_info pointers
and were called tp. Now that they are only stack pointers,
rename them sp.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/irq.c | 17 +++++++----------
arch/powerpc/kernel/setup_64.c | 11 +++--------
2 files changed, 10 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 938944c6e2ee..8a936723c791 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -659,21 +659,21 @@ void __do_irq(struct pt_regs *regs)
void do_IRQ(struct pt_regs *regs)
{
struct pt_regs *old_regs = set_irq_regs(regs);
- void *curtp, *irqtp, *sirqtp;
+ void *cursp, *irqsp, *sirqsp;
/* Switch to the irq stack to handle this */
- curtp = (void *)(current_stack_pointer() & ~(THREAD_SIZE - 1));
- irqtp = hardirq_ctx[raw_smp_processor_id()];
- sirqtp = softirq_ctx[raw_smp_processor_id()];
+ cursp = (void *)(current_stack_pointer() & ~(THREAD_SIZE - 1));
+ irqsp = hardirq_ctx[raw_smp_processor_id()];
+ sirqsp = softirq_ctx[raw_smp_processor_id()];
/* Already there ? */
- if (unlikely(curtp == irqtp || curtp == sirqtp)) {
+ if (unlikely(cursp == irqsp || cursp == sirqsp)) {
__do_irq(regs);
set_irq_regs(old_regs);
return;
}
/* Switch stack and call */
- call_do_irq(regs, irqtp);
+ call_do_irq(regs, irqsp);
set_irq_regs(old_regs);
}
@@ -695,10 +695,7 @@ void *hardirq_ctx[NR_CPUS] __read_mostly;
void do_softirq_own_stack(void)
{
- void *irqtp;
-
- irqtp = softirq_ctx[smp_processor_id()];
- call_do_softirq(irqtp);
+ call_do_softirq(softirq_ctx[smp_processor_id()]);
}
irq_hw_number_t virq_to_hw(unsigned int virq)
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 2db1c5f7d141..daa361fc6a24 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -716,19 +716,14 @@ void __init emergency_stack_init(void)
limit = min(ppc64_bolted_size(), ppc64_rma_size);
for_each_possible_cpu(i) {
- void *ti;
-
- ti = alloc_stack(limit, i);
- paca_ptrs[i]->emergency_sp = ti + THREAD_SIZE;
+ paca_ptrs[i]->emergency_sp = alloc_stack(limit, i) + THREAD_SIZE;
#ifdef CONFIG_PPC_BOOK3S_64
/* emergency stack for NMI exception handling. */
- ti = alloc_stack(limit, i);
- paca_ptrs[i]->nmi_emergency_sp = ti + THREAD_SIZE;
+ paca_ptrs[i]->nmi_emergency_sp = alloc_stack(limit, i) + THREAD_SIZE;
/* emergency stack for machine check exception handling. */
- ti = alloc_stack(limit, i);
- paca_ptrs[i]->mc_emergency_sp = ti + THREAD_SIZE;
+ paca_ptrs[i]->mc_emergency_sp = alloc_stack(limit, i) + THREAD_SIZE;
#endif
}
}
--
2.13.3
^ permalink raw reply related
* Re: [PATCH v14 02/12] powerpc/irq: use memblock functions returning virtual address
From: Mark Rutland @ 2019-01-24 16:51 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, Nicholas Piggin, Mike Rapoport, Paul Mackerras,
linuxppc-dev
In-Reply-To: <1c8e658c094022a15e4b9f5ef5c191caab3c0dff.1548346225.git.christophe.leroy@c-s.fr>
On Thu, Jan 24, 2019 at 04:19:33PM +0000, Christophe Leroy wrote:
> Since only the virtual address of allocated blocks is used,
> lets use functions returning directly virtual address.
>
> Those functions have the advantage of also zeroing the block.
>
> Suggested-by: Mike Rapoport <rppt@linux.ibm.com>
> Acked-by: Mike Rapoport <rppt@linux.ibm.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
[...]
> +static void *__init alloc_stack(void)
> +{
> + void *ptr = memblock_alloc(THREAD_SIZE, THREAD_SIZE);
> +
> + if (!ptr)
> + panic("cannot allocate stacks");
> +
> + return ptr;
> +}
I believe memblock_alloc() will panic() if it cannot allocate memory,
since that goes:
memblock_alloc()
-> memblock_alloc_try_nid()
-> panic()
So you can get rid of the panic() here, or if you want a custom panic
message, you can use memblock_alloc_nopanic().
[...]
> static void *__init alloc_stack(unsigned long limit, int cpu)
> {
> - unsigned long pa;
> + void *ptr;
>
> BUILD_BUG_ON(STACK_INT_FRAME_SIZE % 16);
>
> - pa = memblock_alloc_base_nid(THREAD_SIZE, THREAD_SIZE, limit,
> - early_cpu_to_node(cpu), MEMBLOCK_NONE);
> - if (!pa) {
> - pa = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
> - if (!pa)
> - panic("cannot allocate stacks");
> - }
> + ptr = memblock_alloc_try_nid(THREAD_SIZE, THREAD_SIZE,
> + MEMBLOCK_LOW_LIMIT, limit,
> + early_cpu_to_node(cpu));
> + if (!ptr)
> + panic("cannot allocate stacks");
The same applies here -- memblock_alloc_try_nid() will panic itself
rather than returning NULL.
Otherwise, this looks like a nice cleanup. With the panics removed (or
using the _nopanic() allocators), feel free to add:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH v14 05/12] powerpc: prep stack walkers for THREAD_INFO_IN_TASK
From: Mark Rutland @ 2019-01-24 17:04 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, Nicholas Piggin, Mike Rapoport, Paul Mackerras,
linuxppc-dev
In-Reply-To: <685ac94ceb1dfac43fbd2ee11a987cbaca63ab0b.1548346225.git.christophe.leroy@c-s.fr>
On Thu, Jan 24, 2019 at 04:19:39PM +0000, Christophe Leroy wrote:
> [text copied from commit 9bbd4c56b0b6
> ("arm64: prep stack walkers for THREAD_INFO_IN_TASK")]
>
> When CONFIG_THREAD_INFO_IN_TASK is selected, task stacks may be freed
> before a task is destroyed. To account for this, the stacks are
> refcounted, and when manipulating the stack of another task, it is
> necessary to get/put the stack to ensure it isn't freed and/or re-used
> while we do so.
>
> This patch reworks the powerpc stack walking code to account for this.
> When CONFIG_THREAD_INFO_IN_TASK is not selected these perform no
> refcounting, and this should only be a structural change that does not
> affect behaviour.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
I'm not familiar with the powerpc code, but AFAICT this is analagous to
the arm64 code, and I'm not aware of any special caveats. FWIW:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> ---
> arch/powerpc/kernel/process.c | 23 +++++++++++++++++++++--
> arch/powerpc/kernel/stacktrace.c | 29 ++++++++++++++++++++++++++---
> 2 files changed, 47 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index ce393df243aa..4ffbb677c9f5 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -2027,7 +2027,7 @@ int validate_sp(unsigned long sp, struct task_struct *p,
>
> EXPORT_SYMBOL(validate_sp);
>
> -unsigned long get_wchan(struct task_struct *p)
> +static unsigned long __get_wchan(struct task_struct *p)
> {
> unsigned long ip, sp;
> int count = 0;
> @@ -2053,6 +2053,20 @@ unsigned long get_wchan(struct task_struct *p)
> return 0;
> }
>
> +unsigned long get_wchan(struct task_struct *p)
> +{
> + unsigned long ret;
> +
> + if (!try_get_task_stack(p))
> + return 0;
> +
> + ret = __get_wchan(p);
> +
> + put_task_stack(p);
> +
> + return ret;
> +}
> +
> static int kstack_depth_to_print = CONFIG_PRINT_STACK_DEPTH;
>
> void show_stack(struct task_struct *tsk, unsigned long *stack)
> @@ -2067,6 +2081,9 @@ void show_stack(struct task_struct *tsk, unsigned long *stack)
> int curr_frame = 0;
> #endif
>
> + if (!try_get_task_stack(tsk))
> + return;
> +
> sp = (unsigned long) stack;
> if (tsk == NULL)
> tsk = current;
> @@ -2081,7 +2098,7 @@ void show_stack(struct task_struct *tsk, unsigned long *stack)
> printk("Call Trace:\n");
> do {
> if (!validate_sp(sp, tsk, STACK_FRAME_OVERHEAD))
> - return;
> + break;
>
> stack = (unsigned long *) sp;
> newsp = stack[0];
> @@ -2121,6 +2138,8 @@ void show_stack(struct task_struct *tsk, unsigned long *stack)
>
> sp = newsp;
> } while (count++ < kstack_depth_to_print);
> +
> + put_task_stack(tsk);
> }
>
> #ifdef CONFIG_PPC64
> diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
> index e2c50b55138f..a5745571e06e 100644
> --- a/arch/powerpc/kernel/stacktrace.c
> +++ b/arch/powerpc/kernel/stacktrace.c
> @@ -67,12 +67,17 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
> {
> unsigned long sp;
>
> + if (!try_get_task_stack(tsk))
> + return;
> +
> if (tsk == current)
> sp = current_stack_pointer();
> else
> sp = tsk->thread.ksp;
>
> save_context_stack(trace, sp, tsk, 0);
> +
> + put_task_stack(tsk);
> }
> EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
>
> @@ -84,9 +89,8 @@ save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)
> EXPORT_SYMBOL_GPL(save_stack_trace_regs);
>
> #ifdef CONFIG_HAVE_RELIABLE_STACKTRACE
> -int
> -save_stack_trace_tsk_reliable(struct task_struct *tsk,
> - struct stack_trace *trace)
> +static int __save_stack_trace_tsk_reliable(struct task_struct *tsk,
> + struct stack_trace *trace)
> {
> unsigned long sp;
> unsigned long stack_page = (unsigned long)task_stack_page(tsk);
> @@ -193,6 +197,25 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
> }
> return 0;
> }
> +
> +int save_stack_trace_tsk_reliable(struct task_struct *tsk,
> + struct stack_trace *trace)
> +{
> + int ret;
> +
> + /*
> + * If the task doesn't have a stack (e.g., a zombie), the stack is
> + * "reliably" empty.
> + */
> + if (!try_get_task_stack(tsk))
> + return 0;
> +
> + ret = __save_stack_trace_tsk_reliable(trace, tsk);
> +
> + put_task_stack(tsk);
> +
> + return ret;
> +}
> EXPORT_SYMBOL_GPL(save_stack_trace_tsk_reliable);
> #endif /* CONFIG_HAVE_RELIABLE_STACKTRACE */
>
> --
> 2.13.3
>
^ permalink raw reply
* Re: [PATCH v14 07/12] powerpc: Activate CONFIG_THREAD_INFO_IN_TASK
From: Mark Rutland @ 2019-01-24 17:18 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, Nicholas Piggin, Mike Rapoport, Paul Mackerras,
linuxppc-dev
In-Reply-To: <067ad3f669bdf5ad562186e9875b588f25732406.1548346225.git.christophe.leroy@c-s.fr>
On Thu, Jan 24, 2019 at 04:19:43PM +0000, Christophe Leroy wrote:
> This patch activates CONFIG_THREAD_INFO_IN_TASK which
> moves the thread_info into task_struct.
>
> Moving thread_info into task_struct has the following advantages:
> - It protects thread_info from corruption in the case of stack
> overflows.
> - Its address is harder to determine if stack addresses are
> leaked, making a number of attacks more difficult.
>
> This has the following consequences:
> - thread_info is now located at the beginning of task_struct.
> - The 'cpu' field is now in task_struct, and only exists when
> CONFIG_SMP is active.
> - thread_info doesn't have anymore the 'task' field.
>
> This patch:
> - Removes all recopy of thread_info struct when the stack changes.
> - Changes the CURRENT_THREAD_INFO() macro to point to current.
> - Selects CONFIG_THREAD_INFO_IN_TASK.
> - Modifies raw_smp_processor_id() to get ->cpu from current without
> including linux/sched.h to avoid circular inclusion and without
> including asm/asm-offsets.h to avoid symbol names duplication
> between ASM constants and C constants.
> - Modifies klp_init_thread_info() to take a task_struct pointer
> argument.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
[...]
> +ifdef CONFIG_SMP
> +prepare: task_cpu_prepare
> +
> +task_cpu_prepare: prepare0
> + $(eval KBUILD_CFLAGS += -D_TASK_CPU=$(shell awk '{if ($$2 == "TI_CPU") print $$3;}' include/generated/asm-offsets.h))
> +endif
[...]
> -#define raw_smp_processor_id() (current_thread_info()->cpu)
> +/*
> + * This is particularly ugly: it appears we can't actually get the definition
> + * of task_struct here, but we need access to the CPU this task is running on.
> + * Instead of using task_struct we're using _TASK_CPU which is extracted from
> + * asm-offsets.h by kbuild to get the current processor ID.
> + *
> + * This also needs to be safeguarded when building asm-offsets.s because at
> + * that time _TASK_CPU is not defined yet. It could have been guarded by
> + * _TASK_CPU itself, but we want the build to fail if _TASK_CPU is missing
> + * when building something else than asm-offsets.s
> + */
> +#ifdef GENERATING_ASM_OFFSETS
> +#define raw_smp_processor_id() (0)
> +#else
> +#define raw_smp_processor_id() (*(unsigned int *)((void *)current + _TASK_CPU))
> +#endif
> #define hard_smp_processor_id() (smp_hw_index[smp_processor_id()])
On arm64 we have the per-cpu offset in a CPU register (TPIDR_EL1), so we
can do:
DEFINE_PER_CPU_READ_MOSTLY(int, cpu_number);
#define raw_smp_processor_id() (*raw_cpu_ptr(&cpu_number))
... but I guess that's not possible on PPC for some reason?
I think I asked that before, but I couldn't find the thread.
Otherwise, this all looks sound to me, but I don't know much about PPC.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH v14 02/12] powerpc/irq: use memblock functions returning virtual address
From: Mike Rapoport @ 2019-01-24 17:25 UTC (permalink / raw)
To: Mark Rutland; +Cc: linux-kernel, Nicholas Piggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190124165153.GB5531@lakrids.cambridge.arm.com>
On Thu, Jan 24, 2019 at 04:51:53PM +0000, Mark Rutland wrote:
> On Thu, Jan 24, 2019 at 04:19:33PM +0000, Christophe Leroy wrote:
> > Since only the virtual address of allocated blocks is used,
> > lets use functions returning directly virtual address.
> >
> > Those functions have the advantage of also zeroing the block.
> >
> > Suggested-by: Mike Rapoport <rppt@linux.ibm.com>
> > Acked-by: Mike Rapoport <rppt@linux.ibm.com>
> > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>
> [...]
>
> > +static void *__init alloc_stack(void)
> > +{
> > + void *ptr = memblock_alloc(THREAD_SIZE, THREAD_SIZE);
> > +
> > + if (!ptr)
> > + panic("cannot allocate stacks");
> > +
> > + return ptr;
> > +}
>
> I believe memblock_alloc() will panic() if it cannot allocate memory,
> since that goes:
>
> memblock_alloc()
> -> memblock_alloc_try_nid()
> -> panic()
>
> So you can get rid of the panic() here, or if you want a custom panic
> message, you can use memblock_alloc_nopanic().
As we've already discussed it in [1], I'm working on removing the
_nopanic() versions and dropping the panic() calls from memblock_alloc()
and friends.
I've posted v2 of the patches earlier this week [2].
> [...]
[1] https://lore.kernel.org/lkml/20190108143428.GB14063@rapoport-lnx/
[2] https://lore.kernel.org/lkml/1548057848-15136-1-git-send-email-rppt@linux.ibm.com/
> > static void *__init alloc_stack(unsigned long limit, int cpu)
> > {
> > - unsigned long pa;
> > + void *ptr;
> >
> > BUILD_BUG_ON(STACK_INT_FRAME_SIZE % 16);
> >
> > - pa = memblock_alloc_base_nid(THREAD_SIZE, THREAD_SIZE, limit,
> > - early_cpu_to_node(cpu), MEMBLOCK_NONE);
> > - if (!pa) {
> > - pa = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
> > - if (!pa)
> > - panic("cannot allocate stacks");
> > - }
> > + ptr = memblock_alloc_try_nid(THREAD_SIZE, THREAD_SIZE,
> > + MEMBLOCK_LOW_LIMIT, limit,
> > + early_cpu_to_node(cpu));
> > + if (!ptr)
> > + panic("cannot allocate stacks");
>
> The same applies here -- memblock_alloc_try_nid() will panic itself
> rather than returning NULL.
>
> Otherwise, this looks like a nice cleanup. With the panics removed (or
> using the _nopanic() allocators), feel free to add:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
> Thanks,
> Mark.
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v14 02/12] powerpc/irq: use memblock functions returning virtual address
From: Mark Rutland @ 2019-01-24 17:28 UTC (permalink / raw)
To: Mike Rapoport; +Cc: linux-kernel, Nicholas Piggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190124172553.GE13790@rapoport-lnx>
On Thu, Jan 24, 2019 at 07:25:53PM +0200, Mike Rapoport wrote:
> On Thu, Jan 24, 2019 at 04:51:53PM +0000, Mark Rutland wrote:
> > On Thu, Jan 24, 2019 at 04:19:33PM +0000, Christophe Leroy wrote:
> > > Since only the virtual address of allocated blocks is used,
> > > lets use functions returning directly virtual address.
> > >
> > > Those functions have the advantage of also zeroing the block.
> > >
> > > Suggested-by: Mike Rapoport <rppt@linux.ibm.com>
> > > Acked-by: Mike Rapoport <rppt@linux.ibm.com>
> > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >
> > [...]
> >
> > > +static void *__init alloc_stack(void)
> > > +{
> > > + void *ptr = memblock_alloc(THREAD_SIZE, THREAD_SIZE);
> > > +
> > > + if (!ptr)
> > > + panic("cannot allocate stacks");
> > > +
> > > + return ptr;
> > > +}
> >
> > I believe memblock_alloc() will panic() if it cannot allocate memory,
> > since that goes:
> >
> > memblock_alloc()
> > -> memblock_alloc_try_nid()
> > -> panic()
> >
> > So you can get rid of the panic() here, or if you want a custom panic
> > message, you can use memblock_alloc_nopanic().
>
> As we've already discussed it in [1], I'm working on removing the
> _nopanic() versions and dropping the panic() calls from memblock_alloc()
> and friends.
>
> I've posted v2 of the patches earlier this week [2].
>
> > [...]
>
> [1] https://lore.kernel.org/lkml/20190108143428.GB14063@rapoport-lnx/
> [2] https://lore.kernel.org/lkml/1548057848-15136-1-git-send-email-rppt@linux.ibm.com/
Fair enough.
Feel free to take the ack regardless, then!
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH] powerpc/ptrace: Mitigate potential Spectre v1
From: Gustavo A. R. Silva @ 2019-01-24 17:25 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev
In-Reply-To: <1548338509-628-1-git-send-email-leitao@debian.org>
On 1/24/19 8:01 AM, Breno Leitao wrote:
> 'regno' is directly controlled by user space, hence leading to a potential
> exploitation of the Spectre variant 1 vulnerability.
>
> On PTRACE_SETREGS and PTRACE_GETREGS requests, user space passes the
> register number that would be read or written. This register number is
> called 'regno' which is part of the 'addr' syscall parameter.
>
> This 'regno' value is checked against the maximum pt_regs structure size,
> and then used to dereference it, which matches the initial part of a
> Spectre v1 (and Spectre v1.1) attack. The dereferenced value, then,
> is returned to userspace in the GETREGS case.
>
Was this reported by any tool?
If so, it might be worth mentioning it.
> This patch sanitizes 'regno' before using it to dereference pt_reg.
>
> Notice that given that speculation windows are large, the policy is
> to kill the speculation on the first load and not worry if it can be
> completed with a dependent load/store [1].
>
> [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> arch/powerpc/kernel/ptrace.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index cdd5d1d3ae41..3eac38a29863 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -33,6 +33,7 @@
> #include <linux/hw_breakpoint.h>
> #include <linux/perf_event.h>
> #include <linux/context_tracking.h>
> +#include <linux/nospec.h>
>
> #include <linux/uaccess.h>
> #include <linux/pkeys.h>
> @@ -298,6 +299,9 @@ int ptrace_get_reg(struct task_struct *task, int regno, unsigned long *data)
> #endif
>
> if (regno < (sizeof(struct user_pt_regs) / sizeof(unsigned long))) {
I would use a variable to store sizeof(struct user_pt_regs) / sizeof(unsigned long).
> + regno = array_index_nospec(regno,
> + (sizeof(struct user_pt_regs) /
> + sizeof(unsigned long)));
See the rest of my comments below.
> *data = ((unsigned long *)task->thread.regs)[regno];
> return 0;
> }
> @@ -321,6 +325,7 @@ int ptrace_put_reg(struct task_struct *task, int regno, unsigned long data)
> return set_user_dscr(task, data);
>
> if (regno <= PT_MAX_PUT_REG) {
> + regno = array_index_nospec(regno, PT_MAX_PUT_REG);
This is wrong. array_index_nospec() will return PT_MAX_PUT_REG - 1 in case regno is equal to
PT_MAX_PUT_REG, and this is not what you want.
Similar reasoning applies to the case above.
> ((unsigned long *)task->thread.regs)[regno] = data;
> return 0;
> }
>
Thanks
--
Gustavo
^ permalink raw reply
* Re: [RFC PATCH] x86, numa: always initialize all possible nodes
From: Mike Rapoport @ 2019-01-24 17:51 UTC (permalink / raw)
To: Michal Hocko
Cc: Tony Luck, linux-ia64, Dave Hansen, Peter Zijlstra, x86, LKML,
Pingfan Liu, linux-mm, linuxppc-dev
In-Reply-To: <20190124141727.GN4087@dhcp22.suse.cz>
On Thu, Jan 24, 2019 at 03:17:27PM +0100, Michal Hocko wrote:
> a friendly ping for this. Does anybody see any problem with this
> approach?
FWIW, it looks fine to me.
It'd just be nice to have a few more words in the changelog about *how* the
x86 init was reworked ;-)
> On Mon 14-01-19 09:24:16, Michal Hocko wrote:
> > From: Michal Hocko <mhocko@suse.com>
> >
> > Pingfan Liu has reported the following splat
> > [ 5.772742] BUG: unable to handle kernel paging request at 0000000000002088
> > [ 5.773618] PGD 0 P4D 0
> > [ 5.773618] Oops: 0000 [#1] SMP NOPTI
> > [ 5.773618] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 4.20.0-rc1+ #3
> > [ 5.773618] Hardware name: Dell Inc. PowerEdge R7425/02MJ3T, BIOS 1.4.3 06/29/2018
> > [ 5.773618] RIP: 0010:__alloc_pages_nodemask+0xe2/0x2a0
> > [ 5.773618] Code: 00 00 44 89 ea 80 ca 80 41 83 f8 01 44 0f 44 ea 89 da c1 ea 08 83 e2 01 88 54 24 20 48 8b 54 24 08 48 85 d2 0f 85 46 01 00 00 <3b> 77 08 0f 82 3d 01 00 00 48 89 f8 44 89 ea 48 89
> > e1 44 89 e6 89
> > [ 5.773618] RSP: 0018:ffffaa600005fb20 EFLAGS: 00010246
> > [ 5.773618] RAX: 0000000000000000 RBX: 00000000006012c0 RCX: 0000000000000000
> > [ 5.773618] RDX: 0000000000000000 RSI: 0000000000000002 RDI: 0000000000002080
> > [ 5.773618] RBP: 00000000006012c0 R08: 0000000000000000 R09: 0000000000000002
> > [ 5.773618] R10: 00000000006080c0 R11: 0000000000000002 R12: 0000000000000000
> > [ 5.773618] R13: 0000000000000001 R14: 0000000000000000 R15: 0000000000000002
> > [ 5.773618] FS: 0000000000000000(0000) GS:ffff8c69afe00000(0000) knlGS:0000000000000000
> > [ 5.773618] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 5.773618] CR2: 0000000000002088 CR3: 000000087e00a000 CR4: 00000000003406e0
> > [ 5.773618] Call Trace:
> > [ 5.773618] new_slab+0xa9/0x570
> > [ 5.773618] ___slab_alloc+0x375/0x540
> > [ 5.773618] ? pinctrl_bind_pins+0x2b/0x2a0
> > [ 5.773618] __slab_alloc+0x1c/0x38
> > [ 5.773618] __kmalloc_node_track_caller+0xc8/0x270
> > [ 5.773618] ? pinctrl_bind_pins+0x2b/0x2a0
> > [ 5.773618] devm_kmalloc+0x28/0x60
> > [ 5.773618] pinctrl_bind_pins+0x2b/0x2a0
> > [ 5.773618] really_probe+0x73/0x420
> > [ 5.773618] driver_probe_device+0x115/0x130
> > [ 5.773618] __driver_attach+0x103/0x110
> > [ 5.773618] ? driver_probe_device+0x130/0x130
> > [ 5.773618] bus_for_each_dev+0x67/0xc0
> > [ 5.773618] ? klist_add_tail+0x3b/0x70
> > [ 5.773618] bus_add_driver+0x41/0x260
> > [ 5.773618] ? pcie_port_setup+0x4d/0x4d
> > [ 5.773618] driver_register+0x5b/0xe0
> > [ 5.773618] ? pcie_port_setup+0x4d/0x4d
> > [ 5.773618] do_one_initcall+0x4e/0x1d4
> > [ 5.773618] ? init_setup+0x25/0x28
> > [ 5.773618] kernel_init_freeable+0x1c1/0x26e
> > [ 5.773618] ? loglevel+0x5b/0x5b
> > [ 5.773618] ? rest_init+0xb0/0xb0
> > [ 5.773618] kernel_init+0xa/0x110
> > [ 5.773618] ret_from_fork+0x22/0x40
> > [ 5.773618] Modules linked in:
> > [ 5.773618] CR2: 0000000000002088
> > [ 5.773618] ---[ end trace 1030c9120a03d081 ]---
> >
> > with his AMD machine with the following topology
> > NUMA node0 CPU(s): 0,8,16,24
> > NUMA node1 CPU(s): 2,10,18,26
> > NUMA node2 CPU(s): 4,12,20,28
> > NUMA node3 CPU(s): 6,14,22,30
> > NUMA node4 CPU(s): 1,9,17,25
> > NUMA node5 CPU(s): 3,11,19,27
> > NUMA node6 CPU(s): 5,13,21,29
> > NUMA node7 CPU(s): 7,15,23,31
> >
> > [ 0.007418] Early memory node ranges
> > [ 0.007419] node 1: [mem 0x0000000000001000-0x000000000008efff]
> > [ 0.007420] node 1: [mem 0x0000000000090000-0x000000000009ffff]
> > [ 0.007422] node 1: [mem 0x0000000000100000-0x000000005c3d6fff]
> > [ 0.007422] node 1: [mem 0x00000000643df000-0x0000000068ff7fff]
> > [ 0.007423] node 1: [mem 0x000000006c528000-0x000000006fffffff]
> > [ 0.007424] node 1: [mem 0x0000000100000000-0x000000047fffffff]
> > [ 0.007425] node 5: [mem 0x0000000480000000-0x000000087effffff]
> >
> > and nr_cpus set to 4. The underlying reason is tha the device is bound
> > to node 2 which doesn't have any memory and init_cpu_to_node only
> > initializes memory-less nodes for possible cpus which nr_cpus restrics.
> > This in turn means that proper zonelists are not allocated and the page
> > allocator blows up.
> >
> > Fix the issue by reworking how x86 initializes the memory less nodes.
> > The current implementation is hacked into the workflow and it doesn't
> > allow any flexibility. There is init_memory_less_node called for each
> > offline node that has a CPU as already mentioned above. This will make
> > sure that we will have a new online node without any memory. Much later
> > on we build a zone list for this node and things seem to work, except
> > they do not (e.g. due to nr_cpus). Not to mention that it doesn't really
> > make much sense to consider an empty node as online because we just
> > consider this node whenever we want to iterate nodes to use and empty
> > node is obviously not the best candidate. This is all just too fragile.
> >
> > Reported-by: Pingfan Liu <kernelfans@gmail.com>
> > Tested-by: Pingfan Liu <kernelfans@gmail.com>
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> > ---
> >
> > Hi,
> > I am sending this as an RFC because I am not sure this is the proper way
> > to go myself. I am especially not sure about other architectures
> > supporting memoryless nodes (ppc and ia64 AFAICS or are there more?).
> >
> > I would appreciate a help with those architectures because I couldn't
> > really grasp how the memoryless nodes are really initialized there. E.g.
> > ppc only seem to call setup_node_data for online nodes but I couldn't
> > find any special treatment for nodes without any memory.
> >
> > Any further help, comments are appreaciated!
> >
> > arch/x86/mm/numa.c | 27 +++------------------------
> > mm/page_alloc.c | 15 +++++++++------
> > 2 files changed, 12 insertions(+), 30 deletions(-)
> >
> > diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> > index 1308f5408bf7..b3621ee4dfe8 100644
> > --- a/arch/x86/mm/numa.c
> > +++ b/arch/x86/mm/numa.c
> > @@ -216,8 +216,6 @@ static void __init alloc_node_data(int nid)
> >
> > node_data[nid] = nd;
> > memset(NODE_DATA(nid), 0, sizeof(pg_data_t));
> > -
> > - node_set_online(nid);
> > }
> >
> > /**
> > @@ -570,7 +568,7 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)
> > return -EINVAL;
> >
> > /* Finally register nodes. */
> > - for_each_node_mask(nid, node_possible_map) {
> > + for_each_node_mask(nid, numa_nodes_parsed) {
> > u64 start = PFN_PHYS(max_pfn);
> > u64 end = 0;
> >
> > @@ -581,9 +579,6 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)
> > end = max(mi->blk[i].end, end);
> > }
> >
> > - if (start >= end)
> > - continue;
> > -
> > /*
> > * Don't confuse VM with a node that doesn't have the
> > * minimum amount of memory:
> > @@ -592,6 +587,8 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)
> > continue;
> >
> > alloc_node_data(nid);
> > + if (end)
> > + node_set_online(nid);
> > }
> >
> > /* Dump memblock with node info and return. */
> > @@ -721,21 +718,6 @@ void __init x86_numa_init(void)
> > numa_init(dummy_numa_init);
> > }
> >
> > -static void __init init_memory_less_node(int nid)
> > -{
> > - unsigned long zones_size[MAX_NR_ZONES] = {0};
> > - unsigned long zholes_size[MAX_NR_ZONES] = {0};
> > -
> > - /* Allocate and initialize node data. Memory-less node is now online.*/
> > - alloc_node_data(nid);
> > - free_area_init_node(nid, zones_size, 0, zholes_size);
> > -
> > - /*
> > - * All zonelists will be built later in start_kernel() after per cpu
> > - * areas are initialized.
> > - */
> > -}
> > -
> > /*
> > * Setup early cpu_to_node.
> > *
> > @@ -763,9 +745,6 @@ void __init init_cpu_to_node(void)
> > if (node == NUMA_NO_NODE)
> > continue;
> >
> > - if (!node_online(node))
> > - init_memory_less_node(node);
> > -
> > numa_set_node(cpu, node);
> > }
> > }
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 2ec9cc407216..2e097f336126 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -5361,10 +5361,11 @@ static void __build_all_zonelists(void *data)
> > if (self && !node_online(self->node_id)) {
> > build_zonelists(self);
> > } else {
> > - for_each_online_node(nid) {
> > + for_each_node(nid) {
> > pg_data_t *pgdat = NODE_DATA(nid);
> >
> > - build_zonelists(pgdat);
> > + if (pgdat)
> > + build_zonelists(pgdat);
> > }
> >
> > #ifdef CONFIG_HAVE_MEMORYLESS_NODES
> > @@ -6644,10 +6645,8 @@ static unsigned long __init find_min_pfn_for_node(int nid)
> > for_each_mem_pfn_range(i, nid, &start_pfn, NULL, NULL)
> > min_pfn = min(min_pfn, start_pfn);
> >
> > - if (min_pfn == ULONG_MAX) {
> > - pr_warn("Could not find start_pfn for node %d\n", nid);
> > + if (min_pfn == ULONG_MAX)
> > return 0;
> > - }
> >
> > return min_pfn;
> > }
> > @@ -6991,8 +6990,12 @@ void __init free_area_init_nodes(unsigned long *max_zone_pfn)
> > mminit_verify_pageflags_layout();
> > setup_nr_node_ids();
> > zero_resv_unavail();
> > - for_each_online_node(nid) {
> > + for_each_node(nid) {
> > pg_data_t *pgdat = NODE_DATA(nid);
> > +
> > + if (!pgdat)
> > + continue;
> > +
> > free_area_init_node(nid, NULL,
> > find_min_pfn_for_node(nid), NULL);
> >
> > --
> > 2.20.1
> >
>
> --
> Michal Hocko
> SUSE Labs
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: powerpc/ps3: Use struct_size() in kzalloc()
From: Gustavo A. R. Silva @ 2019-01-24 17:38 UTC (permalink / raw)
To: Michael Ellerman, Geoff Levand, Benjamin Herrenschmidt,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <43lSbT1lyfz9sN8@ozlabs.org>
On 1/23/19 9:40 PM, Michael Ellerman wrote:
> On Tue, 2019-01-08 at 21:00:10 UTC, "Gustavo A. R. Silva" wrote:
>> One of the more common cases of allocation size calculations is finding the
>> size of a structure that has a zero-sized array at the end, along with memory
>> for some number of elements for that array. For example:
>>
>> struct foo {
>> int stuff;
>> void *entry[];
>> };
>>
>> instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);
>>
>> Instead of leaving these open-coded and prone to type mistakes, we can now
>> use the new struct_size() helper:
>>
>> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>>
>> This code was detected with the help of Coccinelle.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>
> Applied to powerpc next, thanks.
>
> https://git.kernel.org/powerpc/c/31367b9a01d6a3f4f77694bd44f547d6
>
Thanks, Michael.
--
Gustavo
^ permalink raw reply
* Re: [RFC PATCH] x86, numa: always initialize all possible nodes
From: Dave Hansen @ 2019-01-24 19:10 UTC (permalink / raw)
To: Michal Hocko, linux-mm
Cc: Tony Luck, linux-ia64, Peter Zijlstra, x86, LKML, Pingfan Liu,
linuxppc-dev
In-Reply-To: <20190124141727.GN4087@dhcp22.suse.cz>
On 1/24/19 6:17 AM, Michal Hocko wrote:
> and nr_cpus set to 4. The underlying reason is tha the device is bound
> to node 2 which doesn't have any memory and init_cpu_to_node only
> initializes memory-less nodes for possible cpus which nr_cpus restrics.
> This in turn means that proper zonelists are not allocated and the page
> allocator blows up.
This looks OK to me.
Could we add a few DEBUG_VM checks that *look* for these invalid
zonelists? Or, would our existing list debugging have caught this?
Basically, is this bug also a sign that we need better debugging around
this?
^ permalink raw reply
* BUG: memcmp(): Accessing invalid memory location
From: Chandan Rajendra @ 2019-01-24 14:18 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard
When executing fstests' generic/026 test, I hit the following call trace,
[ 417.061038] BUG: Unable to handle kernel data access at 0xc00000062ac40000
[ 417.062172] Faulting instruction address: 0xc000000000092240
[ 417.062242] Oops: Kernel access of bad area, sig: 11 [#1]
[ 417.062299] LE SMP NR_CPUS=2048 DEBUG_PAGEALLOC NUMA pSeries
[ 417.062366] Modules linked in:
[ 417.062401] CPU: 0 PID: 27828 Comm: chacl Not tainted 5.0.0-rc2-next-20190115-00001-g6de6dba64dda #1
[ 417.062495] NIP: c000000000092240 LR: c00000000066a55c CTR: 0000000000000000
[ 417.062567] REGS: c00000062c0c3430 TRAP: 0300 Not tainted (5.0.0-rc2-next-20190115-00001-g6de6dba64dda)
[ 417.062660] MSR: 8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 44000842 XER: 20000000
[ 417.062750] CFAR: 00007fff7f3108ac DAR: c00000062ac40000 DSISR: 40000000 IRQMASK: 0
GPR00: 0000000000000000 c00000062c0c36c0 c0000000017f4c00 c00000000121a660
GPR04: c00000062ac3fff9 0000000000000004 0000000000000020 00000000275b19c4
GPR08: 000000000000000c 46494c4500000000 5347495f41434c5f c0000000026073a0
GPR12: 0000000000000000 c0000000027a0000 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: c00000062ea70020 c00000062c0c38d0 0000000000000002 0000000000000002
GPR24: c00000062ac3ffe8 00000000275b19c4 0000000000000001 c00000062ac30000
GPR28: c00000062c0c38d0 c00000062ac30050 c00000062ac30058 0000000000000000
[ 417.063563] NIP [c000000000092240] memcmp+0x120/0x690
[ 417.063635] LR [c00000000066a55c] xfs_attr3_leaf_lookup_int+0x53c/0x5b0
[ 417.063709] Call Trace:
[ 417.063744] [c00000062c0c36c0] [c00000000066a098] xfs_attr3_leaf_lookup_int+0x78/0x5b0 (unreliable)
[ 417.063851] [c00000062c0c3760] [c000000000693f8c] xfs_da3_node_lookup_int+0x32c/0x5a0
[ 417.063944] [c00000062c0c3820] [c0000000006634a0] xfs_attr_node_addname+0x170/0x6b0
[ 417.064034] [c00000062c0c38b0] [c000000000664ffc] xfs_attr_set+0x2ac/0x340
[ 417.064118] [c00000062c0c39a0] [c000000000758d40] __xfs_set_acl+0xf0/0x230
[ 417.064190] [c00000062c0c3a00] [c000000000758f50] xfs_set_acl+0xd0/0x160
[ 417.064268] [c00000062c0c3aa0] [c0000000004b69b0] set_posix_acl+0xc0/0x130
[ 417.064339] [c00000062c0c3ae0] [c0000000004b6a88] posix_acl_xattr_set+0x68/0x110
[ 417.064412] [c00000062c0c3b20] [c0000000004532d4] __vfs_setxattr+0xa4/0x110
[ 417.064485] [c00000062c0c3b80] [c000000000454c2c] __vfs_setxattr_noperm+0xac/0x240
[ 417.064566] [c00000062c0c3bd0] [c000000000454ee8] vfs_setxattr+0x128/0x130
[ 417.064638] [c00000062c0c3c30] [c000000000455138] setxattr+0x248/0x600
[ 417.064710] [c00000062c0c3d90] [c000000000455738] path_setxattr+0x108/0x120
[ 417.064785] [c00000062c0c3e00] [c000000000455778] sys_setxattr+0x28/0x40
[ 417.064858] [c00000062c0c3e20] [c00000000000bae4] system_call+0x5c/0x70
[ 417.064930] Instruction dump:
[ 417.064964] 7d201c28 7d402428 7c295040 38630008 38840008 408201f0 4200ffe8 2c050000
[ 417.065051] 4182ff6c 20c50008 54c61838 7d201c28 <7d402428> 7d293436 7d4a3436 7c295040
[ 417.065150] ---[ end trace 0d060411b5e3741b ]---
Both the memory locations passed to memcmp() had "SGI_ACL_FILE" and len
argument of memcmp() was set to 12. s1 argument of memcmp() had the value
0x00000000f4af0485, while s2 argument had the value 0x00000000ce9e316f.
The following is the code path within memcmp() that gets executed for the
above mentioned values,
- Since len (i.e. 12) is greater than 7, we branch to .Lno_short.
- We then prefetch the contents of r3 & r4 and branch to
.Ldiffoffset_8bytes_make_align_start.
- Under .Ldiffoffset_novmx_cmp, Since r3 is unaligned we end up comparing
"SGI" part of the string. r3's value is then aligned. r4's value is
incremented by 3. For comparing the remaining 9 bytes, we jump to
.Lcmp_lt32bytes.
- Here, 8 bytes of the remaining 9 bytes are compared and execution moves to
.Lcmp_rest_lt8bytes.
- Here we execute "LD rB,0,r4". In the case of this bug, r4 has an unaligned
value and hence ends up accessing the "next" double word. The "next" double
word happens to occur after the last page mapped into the kernel's address
space and hence this leads to the previously listed oops.
--
chandan
^ permalink raw reply
* Re: [RFC 1/6] powerpc:/drc Define interface to acquire arch-specific drc info
From: Tyrel Datwyler @ 2019-01-25 0:04 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev, linux-kernel
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <20181214204957.16435.29255.stgit@powerkvm6.aus.stglabs.ibm.com>
On 12/14/2018 12:50 PM, Michael Bringmann wrote:
> Define interface to acquire arch-specific drc info to match against
> hotpluggable devices. The current implementation exposes several
> pseries-specific dynamic memory properties in generic kernel code.
> This patch set provides an interface to pull that code out of the
> generic kernel.
>
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> include/linux/topology.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index cb0775e..df97f5f 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -44,6 +44,15 @@
As far as I know pseries is the only platform that uses DR connectors, and I
highly doubt that any other powerpc platform or arch ever will. So, I'm not sure
that this is really generic enough to belong in topology.h. If anything I would
suggest putting this in an include in arch/powerpc/include/ named something like
drcinfo.h or pseries-drc.h. That will make it visible to modules like rpaphp
that want/need to use this functionality.
-Tyrel
>
> int arch_update_cpu_topology(void);
>
> +int arch_find_drc_match(struct device_node *dn,
> + bool (*usercb)(struct device_node *dn,
> + u32 drc_index, char *drc_name,
> + char *drc_type, u32 drc_power_domain,
> + void *data),
> + char *opt_drc_type, char *opt_drc_name,
> + bool match_drc_index, bool ck_php_type,
> + void *data);
> +
> /* Conform to ACPI 2.0 SLIT distance definitions */
> #define LOCAL_DISTANCE 10
> #define REMOTE_DISTANCE 20
>
^ permalink raw reply
* Re: [RFC 3/6] pseries/drcinfo: Pseries impl of arch_find_drc_info
From: Tyrel Datwyler @ 2019-01-25 0:04 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev, linux-kernel
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <20181214205047.16435.19045.stgit@powerkvm6.aus.stglabs.ibm.com>
On 12/14/2018 12:51 PM, Michael Bringmann wrote:
> This patch provides a common interface to parse ibm,drc-indexes,
> ibm,drc-names, ibm,drc-types, ibm,drc-power-domains, or ibm,drc-info.
> The generic interface arch_find_drc_match is provided which accepts
> callback functions that may be applied to examine the data for each
> entry.
>
The title of your patch is "pseries/drcinfo: Pseries impl of arch_find_drc_info"
but the name of the function you are ultimately implementing is
arch_find_drc_match if I'm not mistaken.
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/prom.h | 3
> arch/powerpc/platforms/pseries/of_helpers.c | 299 +++++++++++++++++++++++++++
> include/linux/topology.h | 2
> 3 files changed, 298 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index b04c5ce..910d1dc 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -91,9 +91,6 @@ struct of_drc_info {
> u32 last_drc_index;
> };
>
> -extern int of_read_drc_info_cell(struct property **prop,
> - const __be32 **curval, struct of_drc_info *data);
> -
>
> /*
> * There are two methods for telling firmware what our capabilities are.
> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
> index 0185e50..11c90cd 100644
> --- a/arch/powerpc/platforms/pseries/of_helpers.c
> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
> @@ -1,5 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
>
> +#define pr_fmt(fmt) "drc: " fmt
> +
> #include <linux/string.h>
> #include <linux/err.h>
> #include <linux/slab.h>
> @@ -11,6 +13,12 @@
>
> #define MAX_DRC_NAME_LEN 64
>
> +static int drc_debug;
> +#define dbg(args...) if (drc_debug) { printk(KERN_DEBUG args); }
> +#define err(arg...) printk(KERN_ERR args);
> +#define info(arg...) printk(KERN_INFO args);
> +#define warn(arg...) printk(KERN_WARNING args);
Its pretty standard these days to use the pr_debug, pr_err, pr_info, pr_warn
variations over printk(LEVEL args).
> +
> /**
> * pseries_of_derive_parent - basically like dirname(1)
> * @path: the full_name of a node to be added to the tree
> @@ -46,7 +54,8 @@ struct device_node *pseries_of_derive_parent(const char *path)
>
> /* Helper Routines to convert between drc_index to cpu numbers */
>
> -int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
> +static int of_read_drc_info_cell(struct property **prop,
> + const __be32 **curval,
> struct of_drc_info *data)
> {
> const char *p;
> @@ -90,4 +99,290 @@ int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
>
> return 0;
> }
> -EXPORT_SYMBOL(of_read_drc_info_cell);
> +
> +static int walk_drc_info(struct device_node *dn,
> + bool (*usercb)(struct of_drc_info *drc,
> + void *data,
> + int *ret_code),
> + char *opt_drc_type,
> + void *data)
> +{
> + struct property *info;
> + unsigned int entries;
> + struct of_drc_info drc;
> + const __be32 *value;
> + int j, ret_code = -EINVAL;
> + bool done = false;
> +
> + info = of_find_property(dn, "ibm,drc-info", NULL);
> + if (info == NULL)
> + return -EINVAL;
> +
> + value = info->value;
> + entries = of_read_number(value++, 1);
> +
> + for (j = 0, done = 0; (j < entries) && (!done); j++) {
> + of_read_drc_info_cell(&info, &value, &drc);
> +
> + if (opt_drc_type && strcmp(opt_drc_type, drc.drc_type))
> + continue;
> +
> + done = usercb(&drc, data, &ret_code);
> + }
> +
> + return ret_code;
> +}
> +
> +static int get_children_props(struct device_node *dn, const int **drc_indexes,
> + const int **drc_names, const int **drc_types,
> + const int **drc_power_domains)
> +{
> + const int *indexes, *names, *types, *domains;
> +
> + indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
> + names = of_get_property(dn, "ibm,drc-names", NULL);
> + types = of_get_property(dn, "ibm,drc-types", NULL);
> + domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
> +
> + if (!indexes || !names || !types || !domains) {
> + /* Slot does not have dynamically-removable children */
> + return -EINVAL;
> + }
> + if (drc_indexes)
> + *drc_indexes = indexes;
> + if (drc_names)
> + /* &drc_names[1] contains NULL terminated slot names */
> + *drc_names = names;
> + if (drc_types)
> + /* &drc_types[1] contains NULL terminated slot types */
> + *drc_types = types;
> + if (drc_power_domains)
> + *drc_power_domains = domains;
> +
> + return 0;
> +}
> +
> +static int is_php_type(char *drc_type)
> +{
> + unsigned long value;
> + char *endptr;
> +
> + /* PCI Hotplug nodes have an integer for drc_type */
> + value = simple_strtoul(drc_type, &endptr, 10);
> + if (endptr == drc_type)
> + return 0;
> +
> + return 1;
> +}
> +
> +/**
> + * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
> + * @dn: target &device_node
> + * @indexes: passed to get_children_props()
> + * @names: passed to get_children_props()
> + * @types: returned from get_children_props()
> + * @power_domains:
> + *
> + * This routine will return true only if the device node is
> + * a hotpluggable slot. This routine will return false
> + * for built-in pci slots (even when the built-in slots are
> + * dlparable.)
> + */
> +static int is_php_dn(struct device_node *dn, const int **indexes,
> + const int **names, const int **types,
> + const int **power_domains)
> +{
> + const int *drc_types;
> + int rc;
> +
> + rc = get_children_props(dn, indexes, names, &drc_types,
> + power_domains);
> + if (rc < 0)
> + return 0;
> +
> + if (!is_php_type((char *) &drc_types[1]))
> + return 0;
> +
> + *types = drc_types;
> + return 1;
> +}
> +
> +struct find_drc_match_cb_struct {
> + struct device_node *dn;
> + bool (*usercb)(struct device_node *dn,
> + u32 drc_index, char *drc_name,
> + char *drc_type, u32 drc_power_domain,
> + void *data);
> + char *drc_type;
> + char *drc_name;
> + u32 drc_index;
> + bool match_drc_index;
> + bool add_slot;
> + void *data;
> +};
> +
> +static int find_drc_match_v1(struct device_node *dn, void *data)
> +{
> + struct find_drc_match_cb_struct *cdata = data;
> + int i, retval = 0;
> + const int *indexes, *names, *types, *domains;
> + char *name, *type;
> + struct device_node *root = dn;
> +
> + if (cdata->match_drc_index)
> + root = dn->parent;
> +
> + if (cdata->add_slot) {
> + /* If this is not a hotplug slot, return without doing
> + * anything.
> + */
> + if (!is_php_dn(root, &indexes, &names, &types, &domains))
> + return 0;
> + } else {
> + if (get_children_props(root, &indexes, &names, &types,
> + &domains) < 0)
> + return 0;
> + }
> +
> + dbg("Entry %s: dn=%pOF\n", __func__, dn);
> +
> + name = (char *) &names[1];
> + type = (char *) &types[1];
> + for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> +
> + if (cdata->match_drc_index &&
> + ((unsigned int) indexes[i + 1] != cdata->drc_index)) {
> + name += strlen(name) + 1;
> + type += strlen(type) + 1;
> + continue;
> + }
> +
> + if (((cdata->drc_name == NULL) ||
> + (cdata->drc_name && !strcmp(cdata->drc_name, name))) &&
> + ((cdata->drc_type == NULL) ||
> + (cdata->drc_type && !strcmp(cdata->drc_type, type)))) {
> +
> + if (cdata->usercb) {
> + retval = cdata->usercb(dn,
> + be32_to_cpu(indexes[i + 1]),
> + name, type,
> + be32_to_cpu(domains[i + 1]),
> + cdata->data);
> + if (!retval)
> + return retval;
> + } else {
> + return 0;
> + }
> + }
> +
> + name += strlen(name) + 1;
> + type += strlen(type) + 1;
> + }
> +
> + dbg("%s - Exit: rc[%d]\n", __func__, retval);
> +
> + /* XXX FIXME: reports a failure only if last entry in loop failed */
> + return retval;
> +}
> +
> +static bool find_drc_match_v2_cb(struct of_drc_info *drc, void *data,
> + int *ret_code)
> +{
> + struct find_drc_match_cb_struct *cdata = data;
> + u32 drc_index;
> + char drc_name[MAX_DRC_NAME_LEN];
> + int i, retval;
> +
> + (*ret_code) = -EINVAL;
> +
> + /* This set not a PHP type? */
> + if (cdata->add_slot) {
> + if (!is_php_type((char *) drc->drc_type)) {
> + return false;
> + }
> + }
> +
> + /* Anything to use from this set? */
> + if (cdata->match_drc_index && (cdata->drc_index > drc->last_drc_index))
> + return false;
> + if ((cdata->drc_type && strcmp(cdata->drc_type, drc->drc_type))
> + return false;
> +
> + /* Check the drc-index entries of this set */
> + for (i = 0, drc_index = drc->drc_index_start;
> + i < drc->num_sequential_elems; i++, drc_index++) {
> +
> + if (cdata->match_drc_index && (cdata->drc_index != drc_index))
> + continue;
> +
> + sprintf(drc_name, "%s%d", drc->drc_name_prefix,
> + drc_index - drc->drc_index_start +
> + drc->drc_name_suffix_start);
> +
> + if ((cdata->drc_name == NULL) ||
> + (cdata->drc_name && !strcmp(cdata->drc_name, drc_name))) {
> +
> + if (cdata->usercb) {
> + retval = cdata->usercb(cdata->dn,
> + drc_index, drc_name,
> + drc->drc_type,
> + drc->drc_power_domain,
> + cdata->data);
> + if (!retval) {
> + (*ret_code) = retval;
> + return true;
> + }
> + } else {
> + (*ret_code) = 0;
> + return true;
> + }
> + }
> + }
> +
> + (*ret_code) = retval;
> + return false;
> +}
> +
> +static int find_drc_match_v2(struct device_node *dn, void *data)
> +{
> + struct find_drc_match_cb_struct *cdata = data;
> + struct device_node *root = cdata->dn;
> +
> + if (!cdata->add_slot) {
> + if (!cdata->drc_type ||
> + (cdata->drc_type && strcmp(cdata->drc_type, "SLOT")))
> + root = dn->parent;
> + }
> +
> + return walk_drc_info(root, find_drc_match_v2_cb, NULL, data);
> +}
> +
> +int arch_find_drc_match(struct device_node *dn,
> + bool (*usercb)(struct device_node *dn,
> + u32 drc_index, char *drc_name,
> + char *drc_type, u32 drc_power_domain,
> + void *data),
> + char *opt_drc_type, char *opt_drc_name,
> + bool match_drc_index, bool add_slot,
> + void *data)
> +{
> + struct find_drc_match_cb_struct cdata = { dn, usercb,
> + opt_drc_type, opt_drc_name, 0,
> + match_drc_index, add_slot, data };
> +
> + if (match_drc_index) {
> + const int *my_index =
> + of_get_property(dn, "ibm,my-drc-index", NULL);
> + if (!my_index) {
> + /* Node isn't DLPAR/hotplug capable */
> + return -EINVAL;
> + }
> + cdata.drc_index = *my_index;
> + }
> +
> + if (firmware_has_feature(FW_FEATURE_DRC_INFO))
> + return find_drc_match_v2(dn, &cdata);
> + else
> + return find_drc_match_v1(dn, &cdata);
> +}
> +EXPORT_SYMBOL(arch_find_drc_match);
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index df97f5f..c3dfa53 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -50,7 +50,7 @@ int arch_find_drc_match(struct device_node *dn,
> char *drc_type, u32 drc_power_domain,
> void *data),
> char *opt_drc_type, char *opt_drc_name,
> - bool match_drc_index, bool ck_php_type,
> + bool match_drc_index, bool add_slot,
> void *data);
Why are you making a change to the prototype here? You should just define it in
your first patch instead of touching it again here.
-Tyrel
>
> /* Conform to ACPI 2.0 SLIT distance definitions */
>
^ permalink raw reply
* Re: [RFC 1/6] powerpc:/drc Define interface to acquire arch-specific drc info
From: Tyrel Datwyler @ 2019-01-25 0:10 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev, linux-kernel
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <20181214204957.16435.29255.stgit@powerkvm6.aus.stglabs.ibm.com>
On 12/14/2018 12:50 PM, Michael Bringmann wrote:
> Define interface to acquire arch-specific drc info to match against
> hotpluggable devices. The current implementation exposes several
> pseries-specific dynamic memory properties in generic kernel code.
> This patch set provides an interface to pull that code out of the
> generic kernel.
>
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> include/linux/topology.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index cb0775e..df97f5f 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -44,6 +44,15 @@
>
> int arch_update_cpu_topology(void);
On another note a kern doc comment for this function would also be nice.
-Tyrel
>
> +int arch_find_drc_match(struct device_node *dn,
> + bool (*usercb)(struct device_node *dn,
> + u32 drc_index, char *drc_name,
> + char *drc_type, u32 drc_power_domain,
> + void *data),
> + char *opt_drc_type, char *opt_drc_name,
> + bool match_drc_index, bool ck_php_type,
> + void *data);
> +
> /* Conform to ACPI 2.0 SLIT distance definitions */
> #define LOCAL_DISTANCE 10
> #define REMOTE_DISTANCE 20
>
^ permalink raw reply
* Re: [RFC 5/6] powerpc/pci/hotplug: Use common drcinfo parsing
From: Tyrel Datwyler @ 2019-01-25 0:29 UTC (permalink / raw)
To: Bjorn Helgaas, Michael Bringmann
Cc: Thomas Falcon, linux-pci, linux-kernel, Juliet Kim,
Paul Mackerras, Tyrel Datwyler, linuxppc-dev
In-Reply-To: <20190115002857.GG33971@google.com>
On 01/14/2019 04:28 PM, Bjorn Helgaas wrote:
> On Fri, Dec 14, 2018 at 02:51:31PM -0600, Michael Bringmann wrote:
>> The implementation of the pseries-specific drc info properties
>> is currently implemented in pseries-specific and non-pseries-specific
>> files. This patch set uses a new implementation of the device-tree
>> parsing code for the properties.
>>
>> This patch refactors parsing of the pseries-specific drc-info properties
>> out of rpaphp_core.c to use the common parser. In the case where an
>> architecture does not use these properties, an __weak copy of the
>> function is provided with dummy return values. Changes include creating
>> appropriate callback functions and passing callback-specific data
>> blocks into arch_find_drc_match. All functions that were used just
>> to support the previous parsing implementation have been moved.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>
> This is fine with me. Any rpaphp_core.c maintainers want to comment?
> Tyrel?
It greatly simplifies the code in rpaphp_core.c, and as far as I can tell the
refactoring maintains the existing functionality.
Acked-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
>
> $ ./scripts/get_maintainer.pl -f drivers/pci/hotplug/rpaphp_core.c
> Tyrel Datwyler <tyreld@linux.vnet.ibm.com> (supporter:IBM Power PCI Hotplug Driver for RPA-compliant...)
> Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Michael Ellerman <mpe@ellerman.id.au> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
>
>> ---
>> drivers/pci/hotplug/rpaphp_core.c | 232 ++++---------------------------------
>> 1 file changed, 28 insertions(+), 204 deletions(-)
>>
>> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
>> index bcd5d35..9ad7384 100644
>> --- a/drivers/pci/hotplug/rpaphp_core.c
>> +++ b/drivers/pci/hotplug/rpaphp_core.c
>> @@ -154,182 +154,18 @@ static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
>> return speed;
>> }
>>
>> -static int get_children_props(struct device_node *dn, const int **drc_indexes,
>> - const int **drc_names, const int **drc_types,
>> - const int **drc_power_domains)
>> -{
>> - const int *indexes, *names, *types, *domains;
>> -
>> - indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>> - names = of_get_property(dn, "ibm,drc-names", NULL);
>> - types = of_get_property(dn, "ibm,drc-types", NULL);
>> - domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
>> -
>> - if (!indexes || !names || !types || !domains) {
>> - /* Slot does not have dynamically-removable children */
>> - return -EINVAL;
>> - }
>> - if (drc_indexes)
>> - *drc_indexes = indexes;
>> - if (drc_names)
>> - /* &drc_names[1] contains NULL terminated slot names */
>> - *drc_names = names;
>> - if (drc_types)
>> - /* &drc_types[1] contains NULL terminated slot types */
>> - *drc_types = types;
>> - if (drc_power_domains)
>> - *drc_power_domains = domains;
>> -
>> - return 0;
>> -}
>> -
>> -
>> /* Verify the existence of 'drc_name' and/or 'drc_type' within the
>> - * current node. First obtain it's my-drc-index property. Next,
>> - * obtain the DRC info from it's parent. Use the my-drc-index for
>> - * correlation, and obtain/validate the requested properties.
>> + * current node.
>> */
>>
>> -static int rpaphp_check_drc_props_v1(struct device_node *dn, char *drc_name,
>> - char *drc_type, unsigned int my_index)
>> -{
>> - char *name_tmp, *type_tmp;
>> - const int *indexes, *names;
>> - const int *types, *domains;
>> - int i, rc;
>> -
>> - rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
>> - if (rc < 0) {
>> - return -EINVAL;
>> - }
>> -
>> - name_tmp = (char *) &names[1];
>> - type_tmp = (char *) &types[1];
>> -
>> - /* Iterate through parent properties, looking for my-drc-index */
>> - for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>> - if ((unsigned int) indexes[i + 1] == my_index)
>> - break;
>> -
>> - name_tmp += (strlen(name_tmp) + 1);
>> - type_tmp += (strlen(type_tmp) + 1);
>> - }
>> -
>> - if (((drc_name == NULL) || (drc_name && !strcmp(drc_name, name_tmp))) &&
>> - ((drc_type == NULL) || (drc_type && !strcmp(drc_type, type_tmp))))
>> - return 0;
>> -
>> - return -EINVAL;
>> -}
>> -
>> -static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
>> - char *drc_type, unsigned int my_index)
>> -{
>> - struct property *info;
>> - unsigned int entries;
>> - struct of_drc_info drc;
>> - const __be32 *value;
>> - char cell_drc_name[MAX_DRC_NAME_LEN];
>> - int j, fndit;
>> -
>> - info = of_find_property(dn->parent, "ibm,drc-info", NULL);
>> - if (info == NULL)
>> - return -EINVAL;
>> -
>> - value = of_prop_next_u32(info, NULL, &entries);
>> - if (!value)
>> - return -EINVAL;
>> -
>> - for (j = 0; j < entries; j++) {
>> - of_read_drc_info_cell(&info, &value, &drc);
>> -
>> - /* Should now know end of current entry */
>> -
>> - if (my_index > drc.last_drc_index)
>> - continue;
>> -
>> - fndit = 1;
>> - break;
>> - }
>> - /* Found it */
>> -
>> - if (fndit)
>> - sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
>> - my_index);
>> -
>> - if (((drc_name == NULL) ||
>> - (drc_name && !strcmp(drc_name, cell_drc_name))) &&
>> - ((drc_type == NULL) ||
>> - (drc_type && !strcmp(drc_type, drc.drc_type))))
>> - return 0;
>> -
>> - return -EINVAL;
>> -}
>> -
>> int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
>> char *drc_type)
>> {
>> - const unsigned int *my_index;
>> -
>> - my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
>> - if (!my_index) {
>> - /* Node isn't DLPAR/hotplug capable */
>> - return -EINVAL;
>> - }
>> -
>> - if (firmware_has_feature(FW_FEATURE_DRC_INFO))
>> - return rpaphp_check_drc_props_v2(dn, drc_name, drc_type,
>> - *my_index);
>> - else
>> - return rpaphp_check_drc_props_v1(dn, drc_name, drc_type,
>> - *my_index);
>> + return arch_find_drc_match(dn, NULL, drc_type, drc_name,
>> + true, false, NULL);
>> }
>> EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
>>
>> -
>> -static int is_php_type(char *drc_type)
>> -{
>> - unsigned long value;
>> - char *endptr;
>> -
>> - /* PCI Hotplug nodes have an integer for drc_type */
>> - value = simple_strtoul(drc_type, &endptr, 10);
>> - if (endptr == drc_type)
>> - return 0;
>> -
>> - return 1;
>> -}
>> -
>> -/**
>> - * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
>> - * @dn: target &device_node
>> - * @indexes: passed to get_children_props()
>> - * @names: passed to get_children_props()
>> - * @types: returned from get_children_props()
>> - * @power_domains:
>> - *
>> - * This routine will return true only if the device node is
>> - * a hotpluggable slot. This routine will return false
>> - * for built-in pci slots (even when the built-in slots are
>> - * dlparable.)
>> - */
>> -static int is_php_dn(struct device_node *dn, const int **indexes,
>> - const int **names, const int **types, const int **power_domains)
>> -{
>> - const int *drc_types;
>> - int rc;
>> -
>> - rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
>> - if (rc < 0)
>> - return 0;
>> -
>> - if (!is_php_type((char *) &drc_types[1]))
>> - return 0;
>> -
>> - *types = drc_types;
>> - return 1;
>> -}
>> -
>> /**
>> * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
>> * @dn: device node of slot
>> @@ -346,54 +182,42 @@ static int is_php_dn(struct device_node *dn, const int **indexes,
>> *
>> * To remove a slot, it suffices to call rpaphp_deregister_slot().
>> */
>> -int rpaphp_add_slot(struct device_node *dn)
>> +
>> +static int rpaphp_add_slot_cb(struct device_node *dn,
>> + u32 drc_index, char *drc_name, char *drc_type,
>> + u32 drc_power_domain, void *data)
>> {
>> struct slot *slot;
>> int retval = 0;
>> - int i;
>> - const int *indexes, *names, *types, *power_domains;
>> - char *name, *type;
>> -
>> - if (!dn->name || strcmp(dn->name, "pci"))
>> - return 0;
>> -
>> - /* If this is not a hotplug slot, return without doing anything. */
>> - if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
>> - return 0;
>> -
>> - dbg("Entry %s: dn=%pOF\n", __func__, dn);
>>
>> - /* register PCI devices */
>> - name = (char *) &names[1];
>> - type = (char *) &types[1];
>> - for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>> - int index;
>> + slot = alloc_slot_struct(dn, drc_index, drc_name, drc_power_domain);
>> + if (!slot)
>> + return -ENOMEM;
>>
>> - index = be32_to_cpu(indexes[i + 1]);
>> - slot = alloc_slot_struct(dn, index, name,
>> - be32_to_cpu(power_domains[i + 1]));
>> - if (!slot)
>> - return -ENOMEM;
>> + slot->type = simple_strtoul(drc_type, NULL, 10);
>> + if (retval)
>> + return -EINVAL;
>>
>> - slot->type = simple_strtoul(type, NULL, 10);
>> + dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
>> + drc_index, drc_name, drc_type);
>>
>> - dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
>> - index, name, type);
>> + retval = rpaphp_enable_slot(slot);
>> + if (!retval)
>> + retval = rpaphp_register_slot(slot);
>>
>> - retval = rpaphp_enable_slot(slot);
>> - if (!retval)
>> - retval = rpaphp_register_slot(slot);
>> + if (retval)
>> + dealloc_slot_struct(slot);
>>
>> - if (retval)
>> - dealloc_slot_struct(slot);
>> + return retval;
>> +}
>>
>> - name += strlen(name) + 1;
>> - type += strlen(type) + 1;
>> - }
>> - dbg("%s - Exit: rc[%d]\n", __func__, retval);
>> +int rpaphp_add_slot(struct device_node *dn)
>> +{
>> + if (!dn->name || strcmp(dn->name, "pci"))
>> + return 0;
>>
>> - /* XXX FIXME: reports a failure only if last entry in loop failed */
>> - return retval;
>> + return arch_find_drc_match(dn, rpaphp_add_slot_cb,
>> + NULL, NULL, false, true, NULL);
>> }
>> EXPORT_SYMBOL_GPL(rpaphp_add_slot);
>>
>>
^ permalink raw reply
* Re: BUG: memcmp(): Accessing invalid memory location
From: Benjamin Herrenschmidt @ 2019-01-25 0:55 UTC (permalink / raw)
To: Chandan Rajendra, linuxppc-dev
Cc: Anton Blanchard, Nick Piggin, Aneesh Kumar K.V
In-Reply-To: <17042269.pB4heZKTbK@localhost.localdomain>
On Thu, 2019-01-24 at 19:48 +0530, Chandan Rajendra wrote:
> - Here we execute "LD rB,0,r4". In the case of this bug, r4 has an unaligned
> value and hence ends up accessing the "next" double word. The "next" double
> word happens to occur after the last page mapped into the kernel's address
> space and hence this leads to the previously listed oops.
>
This is interesting ... should we mark the last page of any piece of
mapped linear mapping as reserved to avoid that sort of issue ?
Nick ? Aneesh ?
Cheers,
Ben.
^ permalink raw reply
* [PATCH] cxl: Wrap iterations over afu slices inside 'afu_list_lock'
From: Vaibhav Jain @ 2019-01-25 4:40 UTC (permalink / raw)
To: linuxppc-dev, Frederic Barrat
Cc: Philippe Bergheaud, Vaibhav Jain, Alastair D'Silva,
Christophe Lombard, Andrew Donnellan
Within cxl module, iteration over array 'adapter->slices' may be racy
at few points as it might be simultaneously read during an EEH and its
contents being set to NULL while driver is being unloaded or unbound
from the adapter. This might result in a NULL pointer to 'struct afu'
being de-referenced during an EEH thereby causing a kernel oops.
This patch fixes this by making sure that all access to the array
'adapter->slices' is wrapped within the context of spin-lock
'adapter->afu_list_lock'.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
drivers/misc/cxl/guest.c | 2 ++
drivers/misc/cxl/pci.c | 38 ++++++++++++++++++++++++++++----------
2 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
index 5d28d9e454f5..08f4a512afad 100644
--- a/drivers/misc/cxl/guest.c
+++ b/drivers/misc/cxl/guest.c
@@ -267,6 +267,7 @@ static int guest_reset(struct cxl *adapter)
int i, rc;
pr_devel("Adapter reset request\n");
+ spin_lock(&adapter->afu_list_lock);
for (i = 0; i < adapter->slices; i++) {
if ((afu = adapter->afu[i])) {
pci_error_handlers(afu, CXL_ERROR_DETECTED_EVENT,
@@ -283,6 +284,7 @@ static int guest_reset(struct cxl *adapter)
pci_error_handlers(afu, CXL_RESUME_EVENT, 0);
}
}
+ spin_unlock(&adapter->afu_list_lock);
return rc;
}
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index c79ba1c699ad..28c28bceb063 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -1805,7 +1805,7 @@ static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
/* There should only be one entry, but go through the list
* anyway
*/
- if (afu->phb == NULL)
+ if (afu == NULL || afu->phb == NULL)
return result;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
@@ -1843,6 +1843,8 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
/* If we're permanently dead, give up. */
if (state == pci_channel_io_perm_failure) {
+ /* Stop the slice traces */
+ spin_lock(&adapter->afu_list_lock);
for (i = 0; i < adapter->slices; i++) {
afu = adapter->afu[i];
/*
@@ -1851,6 +1853,7 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
*/
cxl_vphb_error_detected(afu, state);
}
+ spin_unlock(&adapter->afu_list_lock);
return PCI_ERS_RESULT_DISCONNECT;
}
@@ -1932,14 +1935,20 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
* * In slot_reset, free the old resources and allocate new ones.
* * In resume, clear the flag to allow things to start.
*/
+
+ /* Make sure no one else changes the afu list */
+ spin_lock(&adapter->afu_list_lock);
+
for (i = 0; i < adapter->slices; i++) {
afu = adapter->afu[i];
afu_result = cxl_vphb_error_detected(afu, state);
- cxl_context_detach_all(afu);
- cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
- pci_deconfigure_afu(afu);
+ if (afu != NULL) {
+ cxl_context_detach_all(afu);
+ cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
+ pci_deconfigure_afu(afu);
+ }
/* Disconnect trumps all, NONE trumps NEED_RESET */
if (afu_result == PCI_ERS_RESULT_DISCONNECT)
@@ -1948,6 +1957,7 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
(result == PCI_ERS_RESULT_NEED_RESET))
result = PCI_ERS_RESULT_NONE;
}
+ spin_unlock(&adapter->afu_list_lock);
/* should take the context lock here */
if (cxl_adapter_context_lock(adapter) != 0)
@@ -1980,14 +1990,15 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
*/
cxl_adapter_context_unlock(adapter);
+ spin_lock(&adapter->afu_list_lock);
for (i = 0; i < adapter->slices; i++) {
afu = adapter->afu[i];
if (pci_configure_afu(afu, adapter, pdev))
- goto err;
+ goto err_unlock;
if (cxl_afu_select_best_mode(afu))
- goto err;
+ goto err_unlock;
if (afu->phb == NULL)
continue;
@@ -1999,16 +2010,16 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
ctx = cxl_get_context(afu_dev);
if (ctx && cxl_release_context(ctx))
- goto err;
+ goto err_unlock;
ctx = cxl_dev_context_init(afu_dev);
if (IS_ERR(ctx))
- goto err;
+ goto err_unlock;
afu_dev->dev.archdata.cxl_ctx = ctx;
if (cxl_ops->afu_check_and_enable(afu))
- goto err;
+ goto err_unlock;
afu_dev->error_state = pci_channel_io_normal;
@@ -2029,8 +2040,13 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
result = PCI_ERS_RESULT_DISCONNECT;
}
}
+
+ spin_unlock(&adapter->afu_list_lock);
return result;
+err_unlock:
+ spin_unlock(&adapter->afu_list_lock);
+
err:
/* All the bits that happen in both error_detected and cxl_remove
* should be idempotent, so we don't need to worry about leaving a mix
@@ -2051,10 +2067,11 @@ static void cxl_pci_resume(struct pci_dev *pdev)
* This is not the place to be checking if everything came back up
* properly, because there's no return value: do that in slot_reset.
*/
+ spin_lock(&adapter->afu_list_lock);
for (i = 0; i < adapter->slices; i++) {
afu = adapter->afu[i];
- if (afu->phb == NULL)
+ if (afu || afu->phb == NULL)
continue;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
@@ -2063,6 +2080,7 @@ static void cxl_pci_resume(struct pci_dev *pdev)
afu_dev->driver->err_handler->resume(afu_dev);
}
}
+ spin_unlock(&adapter->afu_list_lock);
}
static const struct pci_error_handlers cxl_err_handler = {
--
2.20.1
^ permalink raw reply related
* [RFC PATCH 0/2] cxl: Add support for disabling CAPP when unloading CXL
From: Vaibhav Jain @ 2019-01-25 5:11 UTC (permalink / raw)
To: linuxppc-dev, Frederic Barrat
Cc: Philippe Bergheaud, Vaibhav Jain, Alastair D'Silva,
Christophe Lombard, Andrew Donnellan
Recent updates to OPAL have implemented necessary infrastructure [1] to disable
CAPP and switch PHB back to PCIE mode during fast reset. This small patch-set
uses the same OPAL infrastructure to force disable of CAPP when CXL module is
unloaded via rmmod.
References:
[1]: https://lists.ozlabs.org/pipermail/skiboot/2019-January/013063.html
Vaibhav Jain (2):
powerpc/powernv: Add support for CXL mode switch that need PHB reset
cxl: Force a CAPP reset when unloading CXL module
arch/powerpc/platforms/powernv/pci-cxl.c | 71 +++++++++++++++++++++---
drivers/misc/cxl/cxl.h | 1 +
drivers/misc/cxl/main.c | 3 +
drivers/misc/cxl/pci.c | 25 ++++++++-
4 files changed, 91 insertions(+), 9 deletions(-)
--
2.20.1
^ permalink raw reply
* [RFC PATCH 1/2] powerpc/powernv: Add support for CXL mode switch that need PHB reset
From: Vaibhav Jain @ 2019-01-25 5:11 UTC (permalink / raw)
To: linuxppc-dev, Frederic Barrat
Cc: Philippe Bergheaud, Vaibhav Jain, Alastair D'Silva,
Christophe Lombard, Andrew Donnellan
In-Reply-To: <20190125051131.29351-1-vaibhav@linux.ibm.com>
Recent updates to OPAL [1] have provided support for new CXL modes on
PHB that need to force a cold reset on the bridge (CRESET). However
PHB CRESET is a multi step process and cannot be completed
synchronously as expected by current kernel implementation that issues
opal call opal_pci_set_phb_cxl_mode().
Hence this patch updates pnv_phb_to_cxl_mode() to implement a polling
loop that handles specific error codes (OPAL_BUSY) returned from
opal_pci_set_phb_cxl_mode() and drive the OPAL pci-state machine, if the
requested CXL mode needs a CRESET.
The patch also updates pnv_phb_to_cxl_mode() to convert and return
OPAL error codes into kernel error codes. This removes a previous
issue where callers to this function would have to include
'opal-api.h' to check for specific OPAL error codes.
References:
[1]: https://lists.ozlabs.org/pipermail/skiboot/2019-January/013063.html
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
arch/powerpc/platforms/powernv/pci-cxl.c | 71 +++++++++++++++++++++---
1 file changed, 63 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-cxl.c b/arch/powerpc/platforms/powernv/pci-cxl.c
index 1b18111453d7..d33d662c6212 100644
--- a/arch/powerpc/platforms/powernv/pci-cxl.c
+++ b/arch/powerpc/platforms/powernv/pci-cxl.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <asm/pnv-pci.h>
#include <asm/opal.h>
+#include <linux/delay.h>
#include "pci.h"
@@ -18,21 +19,75 @@ int pnv_phb_to_cxl_mode(struct pci_dev *dev, uint64_t mode)
struct pci_controller *hose = pci_bus_to_host(dev->bus);
struct pnv_phb *phb = hose->private_data;
struct pnv_ioda_pe *pe;
+ unsigned long starttime, endtime;
int rc;
pe = pnv_ioda_get_pe(dev);
if (!pe)
- return -ENODEV;
+ return -ENOENT;
+
+ pe_info(pe, "Switching PHB to CXL mode=%d\n", mode);
+
+ /*
+ * Use a 15 second timeout for mode switch. Value arrived after
+ * limited testing and may need more tweaking.
+ */
+ starttime = jiffies;
+ endtime = starttime + HZ * 15;
+
+ do {
+ rc = opal_pci_set_phb_cxl_mode(phb->opal_id, mode,
+ pe->pe_number);
+
+ /* Wait until mode transistion done */
+ if (rc != OPAL_BUSY && rc != OPAL_BUSY_EVENT)
+ break;
+
+ /* Check if we timedout */
+ if (time_after(jiffies, endtime)) {
+ rc = OPAL_TIMEOUT;
+ break;
+ }
- pe_info(pe, "Switching PHB to CXL\n");
+ /* Opal Busy with mode switch. Run pci state-machine */
+ rc = opal_pci_poll(phb->opal_id);
+ if (rc >= 0) {
+ /* wait for some time */
+ if (rc > 0)
+ msleep(rc);
+ opal_poll_events(NULL);
+ rc = OPAL_BUSY;
+ /* Continue with the mode switch */
+ }
+ } while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT);
+
+ pe_level_printk(pe, KERN_DEBUG, "CXL mode switch finished in %u-msecs.",
+ jiffies_to_msecs(jiffies - starttime));
- rc = opal_pci_set_phb_cxl_mode(phb->opal_id, mode, pe->pe_number);
- if (rc == OPAL_UNSUPPORTED)
- dev_err(&dev->dev, "Required cxl mode not supported by firmware - update skiboot\n");
- else if (rc)
- dev_err(&dev->dev, "opal_pci_set_phb_cxl_mode failed: %i\n", rc);
+ /* Check OPAL errors and convert them to kernel error codes */
+ switch (rc) {
+ case OPAL_SUCCESS:
+ return 0;
- return rc;
+ case OPAL_PARAMETER:
+ dev_err(&dev->dev, "CXL not supported on this PHB\n");
+ return -ENOENT;
+
+ case OPAL_UNSUPPORTED:
+ dev_err(&dev->dev,
+ "Required cxl mode not supported by firmware"
+ " - update skiboot\n");
+ return -ENODEV;
+
+ case OPAL_TIMEOUT:
+ dev_err(&dev->dev, "opal_pci_set_phb_cxl_mode Timedout\n");
+ return -ETIME;
+
+ default:
+ dev_err(&dev->dev,
+ "opal_pci_set_phb_cxl_mode failed: %i\n", rc);
+ return -EIO;
+ };
}
EXPORT_SYMBOL(pnv_phb_to_cxl_mode);
--
2.20.1
^ permalink raw reply related
* [RFC PATCH 2/2] cxl: Force a CAPP reset when unloading CXL module
From: Vaibhav Jain @ 2019-01-25 5:11 UTC (permalink / raw)
To: linuxppc-dev, Frederic Barrat
Cc: Philippe Bergheaud, Vaibhav Jain, Alastair D'Silva,
Christophe Lombard, Andrew Donnellan
In-Reply-To: <20190125051131.29351-1-vaibhav@linux.ibm.com>
This patch forces shutdown of CAPP when CXL module is unloaded. This
is accomplished via a call to pnv_phb_to_cxl_mode() with mode ==
OPAL_PHB_CAPI_MODE_PCIE.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
drivers/misc/cxl/cxl.h | 1 +
drivers/misc/cxl/main.c | 3 +++
drivers/misc/cxl/pci.c | 25 ++++++++++++++++++++++++-
3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index d1d927ccb589..e545c2b81faf 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -1136,4 +1136,5 @@ void cxl_context_mm_count_get(struct cxl_context *ctx);
/* Decrements the reference count to "struct mm_struct" */
void cxl_context_mm_count_put(struct cxl_context *ctx);
+void cxl_pci_shutdown_capp(void);
#endif
diff --git a/drivers/misc/cxl/main.c b/drivers/misc/cxl/main.c
index f35406be465a..f14ff0dcf231 100644
--- a/drivers/misc/cxl/main.c
+++ b/drivers/misc/cxl/main.c
@@ -372,6 +372,9 @@ static void exit_cxl(void)
if (cxl_is_power8())
unregister_cxl_calls(&cxl_calls);
idr_destroy(&cxl_adapter_idr);
+
+ if (cpu_has_feature(CPU_FTR_HVMODE))
+ cxl_pci_shutdown_capp();
}
module_init(init_cxl);
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index c79ba1c699ad..01be2e2d1069 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -25,7 +25,7 @@
#include "cxl.h"
#include <misc/cxl.h>
-
+#include <misc/cxllib.h>
#define CXL_PCI_VSEC_ID 0x1280
#define CXL_VSEC_MIN_SIZE 0x80
@@ -2065,6 +2065,29 @@ static void cxl_pci_resume(struct pci_dev *pdev)
}
}
+void cxl_pci_shutdown_capp(void)
+{
+ struct pci_dev *pdev;
+ struct pci_bus *root_bus;
+ int rc;
+
+ /* Iterate over all CAPP supported PHB's and force them to PCI mode */
+ list_for_each_entry(root_bus, &pci_root_buses, node) {
+ for_each_pci_bridge(pdev, root_bus) {
+
+ if (!cxllib_slot_is_supported(pdev, 0))
+ continue;
+
+ rc = pnv_phb_to_cxl_mode(pdev,
+ OPAL_PHB_CAPI_MODE_PCIE);
+ if (rc)
+ dev_err(&pdev->dev,
+ "cxl: Error resetting CAPP. Err=%d\n",
+ rc);
+ }
+ }
+}
+
static const struct pci_error_handlers cxl_err_handler = {
.error_detected = cxl_pci_error_detected,
.slot_reset = cxl_pci_slot_reset,
--
2.20.1
^ permalink raw reply related
* RE: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for powerpc64
From: Peng Ma @ 2019-01-25 5:54 UTC (permalink / raw)
To: Vinod Koul
Cc: Wen He, Leo Li, Scott Wood, Zhang Wei, dmaengine@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20190119125841.GE4635@vkoul-mobl>
Hi Vinod,
Sorry to replay late.
1:This patch has already send to the patchwork.
Please see the patch link: https://patchwork.kernel.org/patch/10741521/
2:I have already compile the fsl patches on arm and powerpc after patched https://patchwork.kernel.org/patch/10741521/
The compile will successful, please let me know the reported regression results, thanks very much.
Best Regards,
Peng
>-----Original Message-----
>From: Vinod Koul <vkoul@kernel.org>
>Sent: 2019年1月19日 20:59
>To: Peng Ma <peng.ma@nxp.com>
>Cc: Scott Wood <oss@buserror.net>; Leo Li <leoyang.li@nxp.com>; Zhang Wei
><zw@zh-kernel.org>; linuxppc-dev@lists.ozlabs.org;
>dmaengine@vger.kernel.org; Wen He <wen.he_1@nxp.com>
>Subject: Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for
>powerpc64
>
>On 24-12-18, 05:29, Peng Ma wrote:
>> Hi Scott,
>>
>> Oh, I did not see the in_XX64/out_XX64 supported only __powerpc64__ just
>now.
>> Thanks for your reminder.
>
>Can you send the formal patch for this...
>
>FWIW, fsl patches were not merged last cycle because of reported regression...
>
>>
>> #ifdef __powerpc64__
>>
>> #ifdef __BIG_ENDIAN__
>> DEF_MMIO_OUT_D(out_be64, 64, std);
>> DEF_MMIO_IN_D(in_be64, 64, ld);
>>
>> /* There is no asm instructions for 64 bits reverse loads and stores
>> */ static inline u64 in_le64(const volatile u64 __iomem *addr) {
>> return swab64(in_be64(addr));
>> }
>>
>> static inline void out_le64(volatile u64 __iomem *addr, u64 val) {
>> out_be64(addr, swab64(val));
>> }
>> #else
>> DEF_MMIO_OUT_D(out_le64, 64, std);
>> DEF_MMIO_IN_D(in_le64, 64, ld);
>>
>> /* There is no asm instructions for 64 bits reverse loads and stores
>> */ static inline u64 in_be64(const volatile u64 __iomem *addr) {
>> return swab64(in_le64(addr));
>> }
>>
>> static inline void out_be64(volatile u64 __iomem *addr, u64 val) {
>> out_le64(addr, swab64(val));
>> }
>>
>> #endif
>> #endif /* __powerpc64__ */
>>
>> Best Regards,
>> Peng
>> >-----Original Message-----
>> >From: Scott Wood <oss@buserror.net>
>> >Sent: 2018年12月24日 12:46
>> >To: Peng Ma <peng.ma@nxp.com>; Leo Li <leoyang.li@nxp.com>; Zhang
>Wei
>> ><zw@zh-kernel.org>
>> >Cc: linuxppc-dev@lists.ozlabs.org; dmaengine@vger.kernel.org; Wen He
>> ><wen.he_1@nxp.com>
>> >Subject: Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for
>> >powerpc64
>> >
>> >On Mon, 2018-12-24 at 03:42 +0000, Peng Ma wrote:
>> >> Hi Scott,
>> >>
>> >> You are right, we should support powerpc64, so could I changed it
>> >> as
>> >> fallows:
>> >>
>> >> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h index
>> >> 88db939..057babf 100644
>> >> --- a/drivers/dma/fsldma.h
>> >> +++ b/drivers/dma/fsldma.h
>> >> @@ -202,35 +202,10 @@ struct fsldma_chan {
>> >> #define fsl_iowrite32(v, p) out_le32(p, v)
>> >> #define fsl_iowrite32be(v, p) out_be32(p, v)
>> >>
>> >> -#ifndef __powerpc64__
>> >> -static u64 fsl_ioread64(const u64 __iomem *addr) -{
>> >> - u32 fsl_addr = lower_32_bits(addr);
>> >> - u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32;
>> >> -
>> >> - return fsl_addr_hi | in_le32((u32 *)fsl_addr);
>> >> -}
>> >> -
>> >> -static void fsl_iowrite64(u64 val, u64 __iomem *addr) -{
>> >> - out_le32((u32 __iomem *)addr + 1, val >> 32);
>> >> - out_le32((u32 __iomem *)addr, (u32)val);
>> >> -}
>> >> -
>> >> -static u64 fsl_ioread64be(const u64 __iomem *addr) -{
>> >> - u32 fsl_addr = lower_32_bits(addr);
>> >> - u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32;
>> >> -
>> >> - return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1));
>> >> -}
>> >> -
>> >> -static void fsl_iowrite64be(u64 val, u64 __iomem *addr) -{
>> >> - out_be32((u32 __iomem *)addr, val >> 32);
>> >> - out_be32((u32 __iomem *)addr + 1, (u32)val);
>> >> -}
>> >> -#endif
>> >> +#define fsl_ioread64(p) in_le64(p)
>> >> +#define fsl_ioread64be(p) in_be64(p)
>> >> +#define fsl_iowrite64(v, p) out_le64(p, v)
>> >> +#define fsl_iowrite64be(v, p) out_be64(p, v)
>> >> #endif
>> >
>> >Then you'll break 32-bit, assuming those
>> >fake-it-with-two-32-bit-accesses were actually needed.
>> >
>> >-Scott
>> >
>>
>
>--
>~Vinod
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox