On Fri, Jul 21, 2023 at 08:41:32PM +0200, Jeremie Courreges-Anglas wrote:
>
> Spotted while testing a diff from cheloha@, option GPROF doesn't build
> on riscv64 because MCOUNT_ENTER/MCOUNT_EXIT from
> riscv64/include/profile.h haven't been adapted for riscv64.
>
> riscv64 /sys/arch/riscv64/compile/GPROF.MP$ doas -u build make
> cc -g -Werror -Wall -Wimplicit-function-declaration -Wno-pointer-sign
> -Wno-constant-conversion -Wno-address-of-packed-member
> -Wno-unused-but-set-variable -Wno-gnu-folding-constant
> -Wframe-larger-than=2047 -Wno-deprecated-non-prototype
> -Wno-unknown-warning-option -march=rv64gc -mcmodel=medany -mno-relax
> -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -ffreestanding -fno-pie
> -O2 -pipe -nostdinc -I/sys -I/sys/arch/riscv64/compile/GPROF.MP/obj
> -I/sys/arch -I/sys/dev/pci/drm/include -I/sys/dev/pci/drm/include/uapi
> -DDDB -DDIAGNOSTIC -DKTRACE -DACCOUNTING -DKMEMSTATS -DPTRACE -DPOOL_DEBUG
> -DCRYPTO -DSYSVMSG -DSYSVSEM -DSYSVSHM -DUVM_SWAP_ENCRYPT -DFFS -DFFS2
> -DFFS_SOFTUPDATES -DUFS_DIRHASH -DQUOTA -DEXT2FS -DMFS -DNFSCLIENT
> -DNFSSERVER -DCD9660 -DUDF -DMSDOSFS -DFIFO -DFUSE -DSOCKET_SPLICE -DTCP_ECN
> -DTCP_SIGNATURE -DINET6 -DIPSEC -DPPP_BSDCOMP -DPPP_DEFLATE -DPIPEX
> -DMROUTING -DMPLS -DBOOT_CONFIG -DPCIVERBOSE -DUSER_PCICONF
> -DWSDISPLAY_COMPAT_USL -DWSDISPLAY_COMPAT_RAWKBD
> -DWSDISPLAY_DEFAULTSCREENS="6" -DMULTIPROCESSOR -DGPROF -DMAXUSERS=80
> -D_KERNEL -D__riscv64__ -MD -MP -pg -c /sys/kern/subr_prof.c
> cc -g -Werror -Wall -Wimplicit-function-declaration -Wno-pointer-sign
> -Wno-constant-conversion -Wno-address-of-packed-member
> -Wno-unused-but-set-variable -Wno-gnu-folding-constant
> -Wframe-larger-than=2047 -Wno-deprecated-non-prototype
> -Wno-unknown-warning-option -march=rv64gc -mcmodel=medany -mno-relax
> -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -ffreestanding -fno-pie
> -O2 -pipe -nostdinc -I/sys -I/sys/arch/riscv64/compile/GPROF.MP/obj
> -I/sys/arch -I/sys/dev/pci/drm/include -I/sys/dev/pci/drm/include/uapi
> -DDDB -DDIAGNOSTIC -DKTRACE -DACCOUNTING -DKMEMSTATS -DPTRACE -DPOOL_DEBUG
> -DCRYPTO -DSYSVMSG -DSYSVSEM -DSYSVSHM -DUVM_SWAP_ENCRYPT -DFFS -DFFS2
> -DFFS_SOFTUPDATES -DUFS_DIRHASH -DQUOTA -DEXT2FS -DMFS -DNFSCLIENT
> -DNFSSERVER -DCD9660 -DUDF -DMSDOSFS -DFIFO -DFUSE -DSOCKET_SPLICE -DTCP_ECN
> -DTCP_SIGNATURE -DINET6 -DIPSEC -DPPP_BSDCOMP -DPPP_DEFLATE -DPIPEX
> -DMROUTING -DMPLS -DBOOT_CONFIG -DPCIVERBOSE -DUSER_PCICONF
> -DWSDISPLAY_COMPAT_USL -DWSDISPLAY_COMPAT_RAWKBD
> -DWSDISPLAY_DEFAULTSCREENS="6" -DMULTIPROCESSOR -DGPROF -DMAXUSERS=80
> -D_KERNEL -D__riscv64__ -MD -MP -fno-ret-protector -c
> /sys/lib/libkern/mcount.c
> /sys/lib/libkern/mcount.c:79:2: error: invalid operand in inline asm: 'mrs
> ${0:x},daif; msr daifset, #0x2'
> MCOUNT_ENTER;
> ^
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:88:10: note:
> expanded from macro 'MCOUNT_ENTER'
> __asm__ ("mrs %x0,daif; msr daifset, #0x2": "=r"(s));
> ^
> /sys/lib/libkern/mcount.c:79:2: error: unknown operand
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:88:10: note:
> expanded from macro 'MCOUNT_ENTER'
> __asm__ ("mrs %x0,daif; msr daifset, #0x2": "=r"(s));
> ^
> <inline asm>:1:6: note: instantiated into assembly here
> mrs ,daif; msr daifset, #0x2
> ^
> /sys/lib/libkern/mcount.c:79:2: error: unknown operand
> MCOUNT_ENTER;
> ^
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:88:10: note:
> expanded from macro 'MCOUNT_ENTER'
> __asm__ ("mrs %x0,daif; msr daifset, #0x2": "=r"(s));
> ^
> <inline asm>:1:26: note: instantiated into assembly here
> mrs ,daif; msr daifset, #0x2
> ^
> /sys/lib/libkern/mcount.c:171:2: error: invalid operand in inline asm: 'msr
> daif, ${0:x}'
> MCOUNT_EXIT;
> ^
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note:
> expanded from macro 'MCOUNT_EXIT'
> __asm__ ("msr daif, %x0":: "r"(s));
> ^
> /sys/lib/libkern/mcount.c:171:2: error: unknown operand
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note:
> expanded from macro 'MCOUNT_EXIT'
> __asm__ ("msr daif, %x0":: "r"(s));
> ^
> <inline asm>:1:12: note: instantiated into assembly here
> msr daif,
> ^
> /sys/lib/libkern/mcount.c:171:2: error: invalid operand in inline asm: 'msr
> daif, ${0:x}'
> MCOUNT_EXIT;
> ^
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note:
> expanded from macro 'MCOUNT_EXIT'
> __asm__ ("msr daif, %x0":: "r"(s));
> ^
> /sys/lib/libkern/mcount.c:171:2: error: unknown operand
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note:
> expanded from macro 'MCOUNT_EXIT'
> __asm__ ("msr daif, %x0":: "r"(s));
> ^
> <inline asm>:1:12: note: instantiated into assembly here
> msr daif,
> ^
> /sys/lib/libkern/mcount.c:171:2: error: invalid operand in inline asm: 'msr
> daif, ${0:x}'
> MCOUNT_EXIT;
> ^
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note:
> expanded from macro 'MCOUNT_EXIT'
> __asm__ ("msr daif, %x0":: "r"(s));
> ^
> /sys/lib/libkern/mcount.c:171:2: error: unknown operand
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note:
> expanded from macro 'MCOUNT_EXIT'
> __asm__ ("msr daif, %x0":: "r"(s));
> ^
> <inline asm>:1:12: note: instantiated into assembly here
> msr daif,
> ^
> /sys/lib/libkern/mcount.c:171:2: error: invalid operand in inline asm: 'msr
> daif, ${0:x}'
> MCOUNT_EXIT;
> ^
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note:
> expanded from macro 'MCOUNT_EXIT'
> __asm__ ("msr daif, %x0":: "r"(s));
> ^
> /sys/lib/libkern/mcount.c:171:2: error: unknown operand
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note:
> expanded from macro 'MCOUNT_EXIT'
> __asm__ ("msr daif, %x0":: "r"(s));
> ^
> <inline asm>:1:12: note: instantiated into assembly here
> msr daif,
> ^
> /sys/lib/libkern/mcount.c:179:2: error: invalid operand in inline asm: 'msr
> daif, ${0:x}'
> MCOUNT_EXIT;
> ^
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note:
> expanded from macro 'MCOUNT_EXIT'
> __asm__ ("msr daif, %x0":: "r"(s));
> ^
> /sys/lib/libkern/mcount.c:179:2: error: unknown operand
> /sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note:
> expanded from macro 'MCOUNT_EXIT'
> __asm__ ("msr daif, %x0":: "r"(s));
> ^
> <inline asm>:1:12: note: instantiated into assembly here
> msr daif,
> ^
> 13 errors generated.
> *** Error 1 in /sys/arch/riscv64/compile/GPROF.MP (Makefile:765 'mcount.o')
>
> Here's a tentative fix inspired by amd64. The resulting kernel
> seemingly hangs when reaching userland, and ends up panicking, probably
> because the intr_disable()/restore dance:
>
> [...]
> nvme0 at pci6 dev 0 function 0 "Samsung PM991 NVMe" rev 0x00: intx, NVMe 1.4
> nvme0: Samsung SSD 980 500GB, firmware 1B4QFXO7, serial S64DNF0R618716X
> scsibus0 at nvme0: 2 targets, initiator 0
> sd0 at scsibus0 targ 1 lun 0: <NVMe, Samsung SSD 980, 1B4Q>
> sd0: 476940MB, 512 bytes/sector, 976773168 sectors
> ppb6 at pci2 dev 8 function 0 "ASMedia ASM2824" rev 0x01: intx
> pci7 at ppb6 bus 7
> "hfclk" at mainbus0 not configured
> "rtcclk" at mainbus0 not configured
> "gpio-poweroff" at mainbus0 not configured
> Profiling kernel, textsize=7925768 [ffffffc000000000..ffffffc00078f008]
> uhub1 at uhub0 port 2 configuration 1 interface 0 "ASMedia AS2107" rev
> 3.00/0.01 addr 2
> uhub2 at uhub0 port 4 configuration 1 interface 0 "ASMedia AS2107" rev
> 2.10/0.01 addr 3
> vscsi0 at root
> scsibus1 at vscsi0: 256 targets
> softraid0 at root
> scsibus2 at softraid0: 256 targets
> root on sd0a (c92bac01c037a788.a) swap on sd0b dump on sd0b
> <seemingly hangs for a few minutes>
> panic: kernel diagnostic assertion "(csr_read(sstatus) & (SSTATUS_SPP |
> SSTATUS
> _SIE)) == 0" failed: file "/sys/arch/riscv64/riscv64/trap.c", line 123 Came
> fro
> m U mode with interrupts enabled
> Stopped at db_enter+0x10: c.ebreak
> TID PID UID PRFLAGS PFLAGS CPU COMMAND
> [...]
>
> Note: intr_disable() returns an unsigned long while mcount.c provides "int
> s;", but that shouldn't be a problem on riscv64, only SIE_SSIE (1 << 1)
> matters.
Right. It would be nice to "fix" this separately, but it shouldn't
matter here.
> Input welcome.
Is there any difference if you duplicate the contents of
intr_disable/intr_restore in MCOUNT_ENTER/MCOUNT_EXIT directly?
Unlikely, but maybe the compiler is shoving an _mcount() call into
intr_disable/intr_restore, causing infinite recursion.
Index: profile.h
===================================================================
RCS file: /cvs/src/sys/arch/riscv64/include/profile.h,v
retrieving revision 1.4
diff -u -p -r1.4 profile.h
--- profile.h 21 May 2021 16:49:57 -0000 1.4
+++ profile.h 23 Jul 2023 00:43:00 -0000
@@ -83,10 +83,12 @@ __asm__ (".text
\n;" \
" jr ra \n");
#ifdef _KERNEL
-// Change this to dair read/set, then restore.
-#define MCOUNT_ENTER \
-__asm__ ("mrs %x0,daif; msr daifset, #0x2": "=r"(s));
-#define MCOUNT_EXIT \
-__asm__ ("msr daif, %x0":: "r"(s));
-
+#define MCOUNT_ENTER do { \
+ __asm volatile( \
+ "csrrci %0, sstatus, %1" \
+ : "=&r" (s) : "i" (SSTATUS_SIE) \
+ ); \
+ s &= SSTATUS_SIE; \
+} while (0)
+#define MCOUNT_EXIT __asm volatile("csrs sstatus, %0" :: "r" (s))
#endif
