NetBSD Problem Report #54720

From ryo@nerv.org  Mon Nov 25 19:24:06 2019
Return-Path: <ryo@nerv.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 84F617A154
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 25 Nov 2019 19:24:06 +0000 (UTC)
Message-Id: <20191125192402.C62721CC075@yaml.nerv.org>
Date: Tue, 26 Nov 2019 04:24:02 +0900 (JST)
From: ryo@nerv.org
Reply-To: ryo@nerv.org
To: gnats-bugs@NetBSD.org
Subject: pagein problem by prefetching Thumb 32bit instruction on a page boundary
X-Send-Pr-Version: 3.95

>Number:         54720
>Category:       port-arm
>Synopsis:       cannot pagein by prefetching Thumb 32bit instruction on a page boundary
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-arm-maintainer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Nov 25 19:25:00 +0000 2019
>Closed-Date:    Tue Dec 10 02:56:03 +0000 2019
>Last-Modified:  Tue Dec 10 02:56:03 +0000 2019
>Originator:     Ryo Shimizu
>Release:        NetBSD 9.99.18
>Organization:
>Environment:
System: NetBSD max 9.99.18 NetBSD 9.99.18 (NITROGEN6MAX) #24: Mon Nov 25 20:31:25 JST 2019 ryo@phenomena:/src/cvs/NetBSD/sys/arch/evbarm/compile/NITROGEN6MAX evbarm
Architecture: earmv7hf
Machine: evbarm
>Description:
A 32bit Thumb instruction is located on the page boundaries, and the second page is unmap,
prefetch-abort is endlessly occured, and the process goes into a busyloop.

>How-To-Repeat:

execute below test program (shar)

	% cc -Wall -mthumb -c -O -fomit-frame-pointer thumb.c
	% cc -Wall -static -o pagein_test -O main.c thumb.o
	% ./pagein_test
	pagesize=8192
	thumb_nop = 0x14000

	execute thumb "nop.w" code on page boundary
	ok

	% ./pagein_test pageout
	pagesize=8192
	thumb_nop = 0x14000

	0x15fe0: 00 bf 00 bf 00 bf 00 bf 00 bf 00 bf 00 bf 00 bf
	0x15ff0: 00 bf 00 bf 00 bf 00 bf 00 bf 00 bf 00 bf af f3
	0x16000: 00 80 00 bf 00 bf 00 bf 00 bf 00 bf 00 bf 00 bf
	0x16010: 00 bf 00 bf 00 bf 00 bf 00 bf 00 bf 00 bf 00 bf

	pageout 0x16000-0x17fff

	execute thumb "nop.w" code on page boundary
	[ 21620.9678153] load: 0.27  cmd: pagein_test 1588 [0x15ffe/0] 0.15u 2.97s 13% 744k
	[ 21623.0878012] load: 0.33  cmd: pagein_test 1588 [0x15ffe/0] 0.22u 5.02s 21% 744k
	[ 21624.5777914] load: 0.33  cmd: pagein_test 1588 [0x15ffe/0] 0.32u 6.41s 25% 744k
	[ 21625.0277890] load: 0.33  cmd: pagein_test 1588 [0x15ffe/0] 0.34u 6.84s 28% 744k
	^C

- page 0x14000-15fff is mapped
- page 0x16000-17fff is not mapped (paged out)
- "00 bf" is Thumb-16bit nop
- "af f3 00 80" is Thumb-32bit nop.w (at 0x15ffe).


# This is a shell archive.  Save it in a file, remove anything before
# this line, and then unpack it by entering "sh file".  Note, it may
# create directories; files and directories will be owned by you and
# have default permissions.
#
# This archive contains:
#
#	main.c
#	thumb.c
#	Makefile
#
echo x - main.c
sed 's/^X//' >main.c << 'END-of-main.c'
X#include <stdio.h>
X#include <stdlib.h>
X#include <string.h>
X#include <stdbool.h>
X#include <sys/mman.h>
X
Xvoid thumb_nop(void);
X
Xstatic void
Xdumpstr(const unsigned char *data, int len)
X{
X	for (int i = 0; i < len; i++) {
X		if ((i & 15) == 0)
X			printf("%p:", data);
X		printf(" %02x", *data++);
X		if ((i & 15) == 15)
X			printf("\n");
X	}
X}
X
Xint
Xmain(int argc, char *argv[])
X{
X	int pagesize = 8192;	// XXX
X	void *thumb_nop_addr;
X	bool do_pageout = false;
X
X	if (argc == 2 && strcmp(argv[1], "pageout") == 0)
X		do_pageout = true;
X	else if (argc != 1) {
X		fprintf(stderr, "usage: pagein_test [pageout]\n");
X		exit(1);
X	}
X
X
X	setbuf(stdout, NULL);
X	printf("pagesize=%d\n", pagesize);
X
X	thumb_nop_addr = (void *)((uintptr_t)thumb_nop & -2);
X	printf("thumb_nop = %p\n\n", thumb_nop_addr);
X
X	if (do_pageout) {
X		/* dump around page boundary */
X		dumpstr(thumb_nop_addr + pagesize - 32, 64);
X		printf("\n");
X
X		/* pageout last half of thumb_nop()) */
X		printf("pageout %p-%p\n", thumb_nop_addr + pagesize, thumb_nop_addr + pagesize * 2 - 1);
X		madvise(thumb_nop_addr + pagesize, pagesize, MADV_DONTNEED);
X		madvise(thumb_nop_addr + pagesize, pagesize, MADV_FREE);
X		printf("\n");
X	}
X
X	printf("execute thumb \"nop.w\" code on page boundary\n");
X	thumb_nop();
X	printf("ok\n");
X}
END-of-main.c
echo x - thumb.c
sed 's/^X//' >thumb.c << 'END-of-thumb.c'
X#include <sys/cdefs.h>
X
X __aligned(8192)	/* 0x2000 */
Xvoid
Xthumb_nop(void)
X{
X	// at +0x00000
X	asm(".rept 4095; nop; .endr");
X
X	// at +0x01ffe
X	asm("nop.w");
X
X	// at +0x02002
X	asm(".rept 4095; nop; .endr");
X}
END-of-thumb.c
echo x - Makefile
sed 's/^X//' >Makefile << 'END-of-Makefile'
X
Xmcr_test: main.c thumb.c
X	cc -Wall -mthumb -c -O -fomit-frame-pointer thumb.c
X	cc -Wall -static -o pagein_test -O main.c thumb.o
X
Xclean:
X	rm -f pagein_test thumb.o
END-of-Makefile
exit



>Fix:
Index: sys/arch/arm/arm32/fault.c
===================================================================
RCS file: /src/cvs/cvsroot-netbsd/src/sys/arch/arm/arm32/fault.c,v
retrieving revision 1.108
diff -u -r1.108 fault.c
--- sys/arch/arm/arm32/fault.c	6 Apr 2019 03:06:25 -0000	1.108
+++ sys/arch/arm/arm32/fault.c	25 Nov 2019 17:58:04 -0000
@@ -838,6 +838,9 @@
 	UVMHIST_LOG(maphist, " (pc=0x%jx, l=0x%#jx, tf=0x%#jx)",
 	    fault_pc, (uintptr_t)l, (uintptr_t)tf, 0);

+#ifdef THUMB_CODE
+ recheck:
+#endif
 	/* Ok validate the address, can only execute in USER space */
 	if (__predict_false(fault_pc >= VM_MAXUSER_ADDRESS ||
 	    (fault_pc < VM_MIN_ADDRESS && vector_page == ARM_VECTORS_LOW))) {
@@ -897,6 +900,18 @@
 	call_trapsignal(l, tf, &ksi);

 out:
+
+#ifdef THUMB_CODE
+#define THUMB_32BIT(hi) (((hi) & 0xe000) == 0xe000 && ((hi) & 0x1800))
+	/* thumb-32 instruction was located on page boundary? */
+	if ((tf->tf_spsr & PSR_T_bit) &&
+	    ((fault_pc & PAGE_MASK) == (PAGE_SIZE - THUMB_INSN_SIZE)) &&
+	    THUMB_32BIT(*(uint16_t *)tf->tf_pc)) {
+		fault_pc = tf->tf_pc + THUMB_INSN_SIZE;
+		goto recheck;
+	}
+#endif /* THUMB_CODE */
+
 	KASSERT(!TRAP_USERMODE(tf) || VALID_R15_PSR(tf->tf_pc, tf->tf_spsr));
 	userret(l);
 }

>Release-Note:

>Audit-Trail:
From: Ryo Shimizu <ryo@nerv.org>
To: gnats-bugs@netbsd.org
Cc: port-arm-maintainer@netbsd.org, gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org
Subject: Re: port-arm/54720: pagein problem by prefetching Thumb 32bit instruction on a page boundary
Date: Thu, 28 Nov 2019 14:27:24 +0900

 Additional information.
 There is another way to resolve this:

 #if 1
 	__asm __volatile ("mrc p15, 0, %0, c6, c0, 2" : "=r"(fault_pc));	/* ifar */
 #else
 	fault_pc = tf->tf_pc;
 #endif

 However, the ifar (instruction fault address register) is an optional implementation in ARMv6.
 So I think the first patch is reasonable.

 -- 
 ryo shimizu

From: "Ryo Shimizu" <ryo@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54720 CVS commit: src/sys/arch/arm/arm32
Date: Fri, 29 Nov 2019 17:33:43 +0000

 Module Name:	src
 Committed By:	ryo
 Date:		Fri Nov 29 17:33:43 UTC 2019

 Modified Files:
 	src/sys/arch/arm/arm32: fault.c

 Log Message:
 if Thumb-32 bit instruction located on a page boundariy, also need to consider the pc + 2 address.

 Fix PR/54720. more detail and PoC are descrived in the PR.


 To generate a diff of this commit:
 cvs rdiff -u -r1.108 -r1.109 src/sys/arch/arm/arm32/fault.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->needs-pullups
State-Changed-By: ryo@NetBSD.org
State-Changed-When: Fri, 29 Nov 2019 18:21:12 +0000
State-Changed-Why:
fixed


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54720 CVS commit: [netbsd-9] src/sys/arch/arm/arm32
Date: Sun, 8 Dec 2019 14:31:58 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Sun Dec  8 14:31:58 UTC 2019

 Modified Files:
 	src/sys/arch/arm/arm32 [netbsd-9]: fault.c

 Log Message:
 Pull up following revision(s) (requested by ryo in ticket #510):

 	sys/arch/arm/arm32/fault.c: revision 1.109

 if Thumb-32 bit instruction located on a page boundariy, also need to consider the pc + 2 address.
 Fix PR/54720. more detail and PoC are descrived in the PR.


 To generate a diff of this commit:
 cvs rdiff -u -r1.108 -r1.108.4.1 src/sys/arch/arm/arm32/fault.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: needs-pullups->closed
State-Changed-By: ryo@NetBSD.org
State-Changed-When: Tue, 10 Dec 2019 02:56:03 +0000
State-Changed-Why:
pulluped


>Unformatted:

Home	PR Database Search