Bug 225684 - arm64 pcpu: crash when dereferencing per cpu structure (with 1 or more cpus)
Summary: arm64 pcpu: crash when dereferencing per cpu structure (with 1 or more cpus)
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: arm (show other bugs)
Version: CURRENT
Hardware: arm64 Any
: --- Affects Some People
Assignee: freebsd-arm (Nobody)
URL: https://reviews.freebsd.org/D16145
Keywords:
: 225816 229784 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-02-05 14:38 UTC by Johannes Lundberg
Modified: 2018-07-16 18:25 UTC (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Johannes Lundberg 2018-02-05 14:38:41 UTC
Tested on RPI3 with and without options SMP. Crash on CPU #0 in both cases. 


#include <sys/types.h>
#include <sys/module.h>
#include <sys/param.h>
#include <sys/kernel.h>
#include <sys/systm.h>
#include <sys/smp.h>
#include <sys/pcpu.h>


struct mystruct {
	int id;
} __aligned(CACHE_LINE_SIZE);



static DPCPU_DEFINE(struct mystruct, mystruct);


static int hello_handler(module_t _mod, int event, void *_arg) {

	int i;

	switch(event) {
	case MOD_LOAD:
		printf("MOD_LOAD: \n");

		printf("mp_ncpus: %d. mp_maxcpus: %d\n", mp_ncpus, mp_maxcpus);
		CPU_FOREACH(i) {
			struct mystruct *m;
			m = &DPCPU_ID_GET(i, mystruct);
			printf("Iterating CPU ID: %d\n", i);
			printf("Address of mystruct pcpu pointer: %p\n", m);
			m->id = i;
			printf("Assigned %d tp mystruct->id\n", m->id);
		}	
	break;
	case MOD_UNLOAD:
		printf("MOD_UNLOAD: \n");
		break;
	default:
		break;
	}
	return 0;
}

static moduledata_t mod_data= {
	"hello",
	hello_handler,
	NULL
};


MODULE_VERSION(hello, 1);
MODULE_DEPEND(hello, pci, 1, 1, 1);
DECLARE_MODULE(hello, mod_data, SI_SUB_EXEC, SI_ORDER_ANY);


Cross build on amd64 (after calling make buildenv) (building on RPI3 give same results)

johannes@jd:~/tmp/module % make
machine -> /usr/src/sys/arm64/include
cc -target aarch64-unknown-freebsd12.0 --sysroot=/usr/obj/usr/src/arm64.aarch64/tmp -B/usr/obj/usr/src/arm64.aarch64/tmp/usr/bin  -O2 -pipe  -fno-strict-aliasing -Werror -D_KERNEL -DKLD_MODULE -nostdinc   -I. -I/usr/src/sys -fno-common  -fPIC   -MD  -MF.depend.hello.o -MThello.o -mgeneral-regs-only -ffixed-x18 -ffreestanding -fwrapv -fstack-protector -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual -Wundef -Wno-pointer-sign -D__printf__=__freebsd_kprintf__ -Wmissing-include-dirs -fdiagnostics-show-option -Wno-unknown-pragmas -Wno-error-tautological-compare -Wno-error-empty-body -Wno-error-parentheses-equality -Wno-error-unused-function -Wno-error-pointer-sign -Wno-error-shift-negative-value -Wno-error-address-of-packed-member    -std=iso9899:1999 -c hello.c -o hello.o
/usr/local/bin/ld -m aarch64elf -d -warn-common --build-id=sha1 -r -d -o hello.kld hello.o
:> export_syms
awk -f /usr/src/sys/conf/kmod_syms.awk hello.kld  export_syms | xargs -J% objcopy % hello.kld
/usr/local/bin/ld -m aarch64elf -Bshareable -znotext -d -warn-common --build-id=sha1  -o hello.ko hello.kld
/usr/local/bin/ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored.
objcopy --strip-debug hello.ko



root@rpi:~ # kldload ./hello.ko 
MOD_LOAD: 
mp_ncpus: 1. mp_maxcpus: 1
Iterating CPU ID: 0
Address of mystruct pcpu pointer: 0xffff00005317f780
Fatal data abort:
  x0: ffff000052b1d9a7
  x1:                0
  x2: ffff00005280e428
  x3:              215
  x4:                0
  x5:                0
  x6:                0
  x7:                1
  x8:  d43d561ece232bd
  x9:  d43d561ece232bd
 x10:               1e
 x11:                1
 x12:                1
 x13:                1
 x14:                0
 x15:               64
 x16:                1
 x17:               1e
 x18: ffff00005280e560
 x19:                0
 x20: ffff000052b1d96b
 x21: ffff000052b1d981
 x22: ffff000052b1d9a7
 x23: ffff00005317f780
 x24: ffff000000ada4d8
 x25:                1
 x26: ffff000000ada4e0
 x27: ffff000000ada4a0
 x28: ffff000052b2dc80
 x29: ffff00005280e5b0
  sp: ffff00005280e560
  lr: ffff000052b1d8e0
 elr: ffff000052b1d8e8
spsr:         60000345
 far: ffff00005317f780
 esr:         96000046
[ thread pid 711 tid 100072 ]
Stopped at      hello_handler+0xe0:     undefined       b90002f3



0000000000000808 <hello_handler>:
 808:	a9ba6ffc 	stp	x28, x27, [sp, #-96]!
 80c:	7100043f 	cmp	w1, #0x1
 810:	a90167fa 	stp	x26, x25, [sp, #16]
 814:	a9025ff8 	stp	x24, x23, [sp, #32]
 818:	a90357f6 	stp	x22, x21, [sp, #48]
 81c:	a9044ff4 	stp	x20, x19, [sp, #64]
 820:	a9057bfd 	stp	x29, x30, [sp, #80]
 824:	910143fd 	add	x29, sp, #0x50
 828:	540006e0 	b.eq	904 <hello_handler+0xfc>  // b.none
 82c:	35000721 	cbnz	w1, 910 <hello_handler+0x108>
 830:	90000000 	adrp	x0, 0 <.plt-0x7d8>
 834:	91250400 	add	x0, x0, #0x941
 838:	97fffff0 	bl	7f8 <printf@plt>
 83c:	90000088 	adrp	x8, 10000 <hello_handler+0xf7f8>
 840:	90000089 	adrp	x9, 10000 <hello_handler+0xf7f8>
 844:	f9458508 	ldr	x8, [x8, #2824]
 848:	f9458129 	ldr	x9, [x9, #2816]
 84c:	90000000 	adrp	x0, 0 <.plt-0x7d8>
 850:	91253400 	add	x0, x0, #0x94d
 854:	b9400101 	ldr	w1, [x8]
 858:	b9400122 	ldr	w2, [x9]
 85c:	97ffffe7 	bl	7f8 <printf@plt>
 860:	90000098 	adrp	x24, 10000 <hello_handler+0xf7f8>
 864:	9000009a 	adrp	x26, 10000 <hello_handler+0xf7f8>
 868:	9000009b 	adrp	x27, 10000 <hello_handler+0xf7f8>
 86c:	f9458f18 	ldr	x24, [x24, #2840]
 870:	f945935a 	ldr	x26, [x26, #2848]
 874:	f9458b7b 	ldr	x27, [x27, #2832]
 878:	9000009c 	adrp	x28, 10000 <hello_handler+0xf7f8>
 87c:	90000014 	adrp	x20, 0 <.plt-0x7d8>
 880:	90000015 	adrp	x21, 0 <.plt-0x7d8>
 884:	90000016 	adrp	x22, 0 <.plt-0x7d8>
 888:	2a1f03f3 	mov	w19, wzr
 88c:	320003f9 	orr	w25, wzr, #0x1
 890:	9132039c 	add	x28, x28, #0xc80
 894:	9125ae94 	add	x20, x20, #0x96b
 898:	912606b5 	add	x21, x21, #0x981
 89c:	91269ed6 	add	x22, x22, #0x9a7
 8a0:	93407e68 	sxtw	x8, w19
 8a4:	d343fd09 	lsr	x9, x8, #3
 8a8:	927de529 	and	x9, x9, #0x1ffffffffffffff8
 8ac:	f8696b09 	ldr	x9, [x24, x9]
 8b0:	9240150a 	and	x10, x8, #0x3f
 8b4:	9aca232a 	lsl	x10, x25, x10
 8b8:	ea0a013f 	tst	x9, x10
 8bc:	540001a0 	b.eq	8f0 <hello_handler+0xe8>  // b.none
 8c0:	f8687b68 	ldr	x8, [x27, x8, lsl #3]
 8c4:	aa1403e0 	mov	x0, x20
 8c8:	2a1303e1 	mov	w1, w19
 8cc:	8b1c0117 	add	x23, x8, x28
 8d0:	97ffffca 	bl	7f8 <printf@plt>
 8d4:	aa1503e0 	mov	x0, x21
 8d8:	aa1703e1 	mov	x1, x23
 8dc:	97ffffc7 	bl	7f8 <printf@plt>
 8e0:	aa1603e0 	mov	x0, x22
 8e4:	2a1303e1 	mov	w1, w19

 Instruction causing crash:
 8e8:	b90002f3 	str	w19, [x23]

 8ec:	97ffffc3 	bl	7f8 <printf@plt>
 8f0:	b9400348 	ldr	w8, [x26]
 8f4:	11000673 	add	w19, w19, #0x1
 8f8:	6b08027f 	cmp	w19, w8
 8fc:	54fffd29 	b.ls	8a0 <hello_handler+0x98>  // b.plast
 900:	14000004 	b	910 <hello_handler+0x108>
 904:	90000000 	adrp	x0, 0 <.plt-0x7d8>
 908:	91271000 	add	x0, x0, #0x9c4
 90c:	97ffffbb 	bl	7f8 <printf@plt>
 910:	a9457bfd 	ldp	x29, x30, [sp, #80]
 914:	a9444ff4 	ldp	x20, x19, [sp, #64]
 918:	a94357f6 	ldp	x22, x21, [sp, #48]
 91c:	a9425ff8 	ldp	x24, x23, [sp, #32]
 920:	a94167fa 	ldp	x26, x25, [sp, #16]
 924:	2a1f03e0 	mov	w0, wzr
 928:	a8c66ffc 	ldp	x28, x27, [sp], #96
 92c:	d65f03c0 	ret
Comment 1 Andrew Turner freebsd_committer freebsd_triage 2018-02-05 15:08:58 UTC
A static DPCPU entry in modules is known to not work on arm64. The only workaround is to make the DPCPU data global.
Comment 2 Johannes Lundberg 2018-02-05 15:12:37 UTC
(In reply to Andrew Turner from comment #1)

Do you know if it would work if the module is built into the kernel? (same code)
Comment 3 Johannes Lundberg 2018-02-05 15:18:14 UTC
Confirmed. Removing the static keyword before DPCPU_DEFINE and it works with kldload.
Comment 4 Hans Petter Selasky freebsd_committer freebsd_triage 2018-02-05 15:22:45 UTC
can you try to add a __used attribute instead of removing the static keyword?
Comment 5 Johannes Lundberg 2018-02-05 15:44:02 UTC
(In reply to Hans Petter Selasky from comment #4)

It didn't make any difference. Which is obvious when looking at the definition:

#define	DPCPU_DEFINE(t, n)	t DPCPU_NAME(n) __section(DPCPU_SETNAME) __used
Comment 6 Andrew Turner freebsd_committer freebsd_triage 2018-02-05 16:19:01 UTC
The problem is due to the runtime linking in the DPCPU code. A static value will be loaded with a PC-relative load, however the DPCPU code expects to relocate it to another address based on the CPU and offset of the data within the section.

This is also an issue with VIMAGE and for this reason that's turned off on arm64 for now.
Comment 7 Andrew Turner freebsd_committer freebsd_triage 2018-07-16 14:59:21 UTC
I have a patch for this in review D16145
Comment 8 Andrew Turner freebsd_committer freebsd_triage 2018-07-16 15:01:16 UTC
*** Bug 229784 has been marked as a duplicate of this bug. ***
Comment 9 Andrew Turner freebsd_committer freebsd_triage 2018-07-16 15:01:24 UTC
*** Bug 225816 has been marked as a duplicate of this bug. ***
Comment 10 commit-hook freebsd_committer freebsd_triage 2018-07-16 18:22:19 UTC
A commit references this bug:

Author: andrew
Date: Mon Jul 16 18:21:30 UTC 2018
New revision: 336349
URL: https://svnweb.freebsd.org/changeset/base/336349

Log:
  Don't use the static keyword with DPCPU defines in arm64 modules.

  On arm64 compiler will create PC-relative loads and stores for static data.
  This means it doesn't emit a relocation. Unfortunately the in-kernel linker
  expects there to be one for DPCPU defines so it can modify its value so the
  code will use the correct DPCPU region.

  To workaround the lack of a relocation with static data remove it when
  building modules on arm64. The kernel is unaffected as it doesn't rely on
  modifying these relocations to find the data.

  PR:		225684
  Reported by:	Johannes Lundberg <johalun0@gmail.com>
  Reported by:	Jose Luis Duran <jlduran@gmail.com>
  Reported by:	Greg V <greg@unrelenting.technology>
  Reviewed by:	bz
  Sponsored by:	ABT Systems Ltd
  Differential Revision:	https://reviews.freebsd.org/D16145

Changes:
  head/sys/sys/pcpu.h