Skip to contentSkip to conversation

powerpc/64: Fix kernel stack 16-byte alignment

PATCH Applied Nicholas Piggin 5 participants 5 messages Nov 15, 2018 – Nov 16, 2018

Subsystems (2)
PowerPC ptrace support
lore
Share & tools
lore

Nicholas Piggin

author reviewer
#0 lore raw

PATCH 1/1 powerpc/64: Fix kernel stack 16-byte alignment

Commit 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather than thread_struct") changed sizeof(struct pt_regs) % 16 from 0 to 8, which causes the interrupt frame allocation on kernel entry to put the kernel stack out of alignment.

Add a pad field to fix alignment, and add a BUILD_BUG_ON to catch this in future.

Fixes 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather

than thread_struct")

Signed-off-by Nicholas Piggin <[email protected]>
---
 arch/powerpc/include/asm/ptrace.h | 1 +
 arch/powerpc/kernel/setup_64.c    | 2 ++
 2 files changed, 3 insertions(+)
arch/powerpc/include/asm/ptrace.h +1 -0
index f73886a1a7f5..1513292bf046 100644
@@ -54,6 +54,7 @@ struct pt_regs
 
 #ifdef CONFIG_PPC64
 	unsigned long ppr;
+	unsigned long pad;	/* Maintain 16 byte interrupt stack alignment */
 #endif
 };
 #endif
arch/powerpc/kernel/setup_64.c +2 -0
index 2a51e4cc8246..236c1151a3a7 100644
@@ -636,6 +636,8 @@ static void *__init alloc_stack(unsigned long limit, int cpu)
 {
 	unsigned long pa;
 
+	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) {
Signature · 1 line
2.18.0
Applied
#1 lore raw
Quoted 11 lines · Nicholas Piggin

Commit 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather than thread_struct") changed sizeof(struct pt_regs) % 16 from 0 to 8, which causes the interrupt frame allocation on kernel entry to put the kernel stack out of alignment.

Add a pad field to fix alignment, and add a BUILD_BUG_ON to catch this in future.

Fixes 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather

than thread_struct")

Signed-off-by Nicholas Piggin <[email protected]>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/66f93c5a02d5ba6ef17fef45914396

cheers

Tested-by on v1 1/1
#2 lore raw
In reply to Nicholas Piggin
Quoted 33 lines · Nicholas Piggin

Commit 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather than thread_struct") changed sizeof(struct pt_regs) % 16 from 0 to 8, which causes the interrupt frame allocation on kernel entry to put the kernel stack out of alignment.

Add a pad field to fix alignment, and add a BUILD_BUG_ON to catch this in future.

Fixes 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather

than thread_struct")

Signed-off-by Nicholas Piggin <[email protected]>
---
 arch/powerpc/include/asm/ptrace.h | 1 +
 arch/powerpc/kernel/setup_64.c    | 2 ++
 2 files changed, 3 insertions(+)
arch/powerpc/include/asm/ptrace.h +1 -0
index f73886a1a7f5..1513292bf046 100644
@@ -54,6 +54,7 @@ struct pt_regs
 
 #ifdef CONFIG_PPC64
 	unsigned long ppr;
+	unsigned long pad;	/* Maintain 16 byte interrupt stack alignment */
 #endif
 };
 #endif
arch/powerpc/kernel/setup_64.c +0 -0
index 2a51e4cc8246..236c1151a3a7 100644

arch/powerpc/kernel/setup_64.c static void *__init alloc_stack(unsigned long limit, int cpu) · line 643

Quoted 8 lines
arch/powerpc/kernel/setup_64.c +2 -0
@@ -636,6 +636,8 @@ static void *__init alloc_stack(unsigned long limit, int cpu)
 {
 	unsigned long pa;
 
+	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) {
Tested-by Abdul Haleem <[email protected]>

This patch fixes hard lockup warnings at opal_interrupt on our P8 system seen when bnx2x module load unload.

watchdog: CPU 80 self-detected hard LOCKUP @ opal_interrupt+0x34/0x90
watchdog: CPU 80 TB:2355493195232, last heartbeat TB:2350368214474 (10009ms ago)

Modules linked in: iptable_mangle ipt_MASQUERADE iptable_nat nf_nat_ipv4

nf_nat xt_conntrack nf_conntrack nf_defrag_ipv4 ipt_REJECT nf_reject_ipv4 xt_tcpudp tun bridge stp llc iptable_filter dm_mirror dm_region_hash dm_log dm_service_time vmx_crypto powernv_rng rng_core dm_multipath kvm_hv kvm nfsd binfmt_misc ip_tables x_tables autofs4 xfs lpfc bnx2x crc_t10dif crct10dif_generic nvme_fc mdio nvme libcrc32c nvme_fabrics nvme_core crct10dif_common

CPU: 80 PID: 411 Comm: ksoftirqd/80 Not tainted 4.20.0-rc2-next-20181114-autotest-autotest #1
NIP:  c00000000009ffc4 LR: c000000000095d28 CTR: 0000000030032a00
REGS: c000003fff48fd78 TRAP: 0900   Not tainted  (4.20.0-rc2-next-20181114-autotest-autotest)
MSR:  9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 48000022  XER: 00000000

CFAR: c000000000095d14 IRQMASK: 1
GPR00: 0000000030005128 c000003fff70bd70 c0000000010dcd00 0000000000000000
GPR04: 0000000048000022 c00000000009ffc4 9000000000009033 0000000000000090
GPR08: 0000000000000000 0000000000000000 c000000000095d3c 9000000000001003
GPR12: c000000000095d14 c000003fff7ff300 c000003c95080c80 0000000000010000
GPR16: 0000000000000000 000000000000003c c000003c95080800 c000003c95030e00
GPR20: 0000000000000001 0000000000000001 0000000000000002 0000000000000014
GPR24: c000003ca4981000 c000003fff70be84 0000000000000000 0000000000000000
GPR28: 0000000000000000 0000000000000014 c000003ca4981000 c000003ca4970200 NIP [c00000000009ffc4] opal_interrupt+0x34/0x90
LR [c000000000095d28] opal_return+0x14/0x48

Call Trace:
[c000003fff70bd70] [c00000000009ffc4] opal_interrupt+0x34/0x90 (unreliable)
[c000003fff70bda0] [c0000000001756d0] __handle_irq_event_percpu+0x90/0x2d0
[c000003fff70be60] [c000000000175948] handle_irq_event_percpu+0x38/0xb0
[c000003fff70bea0] [c000000000175a20] handle_irq_event+0x60/0xc0
[c000003fff70bed0] [c00000000017aedc] handle_fasteoi_irq+0xbc/0x1f0
[c000003fff70bf00] [c000000000173ec4] generic_handle_irq+0x44/0x70
[c000003fff70bf20] [c000000000019eac] __do_irq+0x8c/0x200
[c000003fff70bf90] [c00000000002c744] call_do_irq+0x14/0x24
[c000003ca96f7320] [c00000000001a0b4] do_IRQ+0x94/0x110
[c000003ca96f7370] [c000000000008d54] hardware_interrupt_common+0x154/0x160
--- interrupt: 501 at ip_route_input_rcu+0x510/0x950
    LR = ip_route_input_rcu+0x220/0x950
[c000003ca96f7768] [c00000000092a69c] ip_route_input_noref+0x2c/0x60
[c000003ca96f77b8] [c0000000009735b0] arp_process.constprop.14+0x3f0/0x8c0
[c000003ca96f78d8] [c0000000008c7508] __netif_receive_skb_one_core+0x68/0xa0
[c000003ca96f7918] [c0000000008d09bc] netif_receive_skb_internal+0x3c/0x130
[c000003ca96f7968] [c0000000008d153c] napi_gro_receive+0x11c/0x1c0
[c000003ca96f79a8] [c0000000007232bc] tg3_poll_work+0x60c/0x10a0
[c000003ca96f7ab8] [c000000000723da4] tg3_poll_msix+0x54/0x210
[c000003ca96f7b08] [c0000000008d1f0c] net_rx_action+0x35c/0x4c0
[c000003ca96f7c28] [c000000000a2d8ac] __do_softirq+0x16c/0x3e0
[c000003ca96f7d28] [c000000000102df4] run_ksoftirqd+0x54/0x80
[c000003ca96f7d48] [c00000000012d830] smpboot_thread_fn+0x2b0/0x2c0
[c000003ca96f7db8] [c000000000127d9c] kthread+0x15c/0x1a0
[c000003ca96f7e28] [c00000000000bdd0] ret_from_kernel_thread+0x5c/0x6c
Signature · 4 lines
Regard's

Abdul Haleem
IBM Linux Technology Centre
#3 lore raw
In reply to Nicholas Piggin
Quoted 5 lines · Nicholas Piggin

Commit 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather than thread_struct") changed sizeof(struct pt_regs) % 16 from 0 to 8, which causes the interrupt frame allocation on kernel entry to put the kernel stack out of alignment.

Quoted 7 lines

Add a pad field to fix alignment, and add a BUILD_BUG_ON to catch this in future.

Fixes 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather

than thread_struct")

Signed-off-by Nicholas Piggin <[email protected]>
[...]

Thanks for fixing this. Commit 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather than thread_struct") was also leading to incorrect kernel stack traces.

E.g.
If you are using perf record -g and expect to see a stack trace like this:

        c0000000002b19a0 bpf_check+0x1910 (/usr/lib/debug/lib/modules/4.18.17-200.fc28.ppc64le/vmlinux)
        c0000000002a5554 bpf_prog_load+0x684 (/usr/lib/debug/lib/modules/4.18.17-200.fc28.ppc64le/vmlinux)
        c0000000002a6938 sys_bpf+0xaf8 (/usr/lib/debug/lib/modules/4.18.17-200.fc28.ppc64le/vmlinux)
        c00000000000b9e4 system_call+0x5c (/usr/lib/debug/lib/modules/4.18.17-200.fc28.ppc64le/vmlinux)
            7fff815bca90 syscall+0x50 (/usr/lib64/libc-2.27.so)
            7fff7d96ee0c bpf_prog_load+0x16c (/usr/lib64/libbcc.so.0.7.0)

[...]

you would instead see something like this:

        c0000000002bdb88 bpf_check+0xb88 (/lib/modules/4.20.0-rc1+/build/vmlinux)
        c0000000002bdb60 bpf_check+0xb60 (/lib/modules/4.20.0-rc1+/build/vmlinux)
            3fff8f350a90 syscall+0x50 (/usr/lib64/libc-2.27.so)
            3fff8b788e0c bpf_prog_load+0x16c (/usr/lib64/libbcc.so.0.7.0)

[...]

--
With Regards,
Sandipan

reviewer
#4 lore raw
Quoted 9 lines · Sandipan Das
Quoted 8 lines · Nicholas Piggin

Commit 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather than thread_struct") changed sizeof(struct pt_regs) % 16 from 0 to 8, which causes the interrupt frame allocation on kernel entry to put the kernel stack out of alignment.

Add a pad field to fix alignment, and add a BUILD_BUG_ON to catch this in future.

Quoted 8 lines
Quoted 4 lines
Fixes 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather

than thread_struct")

Signed-off-by Nicholas Piggin <[email protected]>
[...]

Thanks for fixing this. Commit 4c2de74cc869 ("powerpc/64: Interrupts save PPR on stack rather than thread_struct") was also leading to incorrect kernel stack traces.

OK thanks.

In future if you see something weird like that please report it to the list.

cheers

Quoted 22 lines

E.g.
If you are using perf record -g and expect to see a stack trace like this:

        c0000000002b19a0 bpf_check+0x1910 (/usr/lib/debug/lib/modules/4.18.17-200.fc28.ppc64le/vmlinux)
        c0000000002a5554 bpf_prog_load+0x684 (/usr/lib/debug/lib/modules/4.18.17-200.fc28.ppc64le/vmlinux)
        c0000000002a6938 sys_bpf+0xaf8 (/usr/lib/debug/lib/modules/4.18.17-200.fc28.ppc64le/vmlinux)
        c00000000000b9e4 system_call+0x5c (/usr/lib/debug/lib/modules/4.18.17-200.fc28.ppc64le/vmlinux)
            7fff815bca90 syscall+0x50 (/usr/lib64/libc-2.27.so)
            7fff7d96ee0c bpf_prog_load+0x16c (/usr/lib64/libbcc.so.0.7.0)

[...]

you would instead see something like this:

        c0000000002bdb88 bpf_check+0xb88 (/lib/modules/4.20.0-rc1+/build/vmlinux)
        c0000000002bdb60 bpf_check+0xb60 (/lib/modules/4.20.0-rc1+/build/vmlinux)
            3fff8f350a90 syscall+0x50 (/usr/lib64/libc-2.27.so)
            3fff8b788e0c bpf_prog_load+0x16c (/usr/lib64/libbcc.so.0.7.0)

[...]

--
With Regards,
Sandipan