port to riscv64

[ancientmodern]

Add riscv64 support to CRIU. Hopefully solve issue #1702

Present status of zdtm tests:

################### 6 TEST(S) FAILED (TOTAL 454/SKIPPED 46) ####################
 * zdtm/static/apparmor_stacking(h)
 * zdtm/static/netns_lock_iptables(h)
 * zdtm/static/socket-tcp-closed-last-ack(uns)
 * zdtm/static/socket-tcp-nfconntrack(h)
 * zdtm/static/socket-tcp-syn-sent(uns)
 * zdtm/transition/maps007(unknown)
##################################### FAIL #####################################

@mihalicyn @felicitia for you reference

WIP: I'll enrich this pull request with more details by tomorrow. And some sections might have been affected when I removed debug info from our code :)

[mihalicyn]

If you did this work together with Yixue, you can add Co-authored-by to the commit messages.
https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors

Great work! I'll take a look and review this.

[ancientmodern]

If you did this work together with Yixue, you can add Co-authored-by to the commit messages. https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors

Great work! I'll take a look and review this.

Thank you Alex! I will make the updates with Yixue tomorrow. If you'd like to test it locally, this branch should be functional with the kernel patch below, though with tons of messy commits and debugging stuff 😂

Patch:

---
 arch/riscv/kernel/signal.c | 71 +++++++++++++++++++++++++++++++-------
 1 file changed, 58 insertions(+), 13 deletions(-)

diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 9aff9d720590..01ccb949d26e 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -16,6 +16,7 @@
 
 #include <asm/ucontext.h>
 #include <asm/vdso.h>
+#include <asm/syscall.h>
 #include <asm/signal.h>
 #include <asm/signal32.h>
 #include <asm/switch_to.h>
@@ -284,35 +285,79 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 
 void arch_do_signal_or_restart(struct pt_regs *regs)
 {
+	unsigned long continue_addr = 0, restart_addr = 0;
+	int retval = 0;
 	struct ksignal ksig;
-
-	if (get_signal(&ksig)) {
-		/* Actually deliver the signal */
-		handle_signal(&ksig, regs);
-		return;
-	}
+	bool syscall = (regs->cause == EXC_SYSCALL);
 
 	/* Did we come from a system call? */
-	if (regs->cause == EXC_SYSCALL) {
+	if (syscall) {
+		continue_addr = regs->epc;
+		restart_addr = continue_addr - 4;
+		retval = regs->a0;
+
 		/* Avoid additional syscall restarting via ret_from_exception */
 		regs->cause = -1UL;
 
-		/* Restart the system call - no handlers present */
-		switch (regs->a0) {
+		/*
+		 * Prepare for system call restart. We do this here so that a
+		 * debugger will see the already changed PC.
+		 */
+		switch (retval) {
 		case -ERESTARTNOHAND:
 		case -ERESTARTSYS:
 		case -ERESTARTNOINTR:
-                        regs->a0 = regs->orig_a0;
-			regs->epc -= 0x4;
+			regs->a0 = regs->orig_a0;
+			regs->epc = restart_addr;
 			break;
 		case -ERESTART_RESTARTBLOCK:
-                        regs->a0 = regs->orig_a0;
+			regs->a0 = regs->orig_a0;
 			regs->a7 = __NR_restart_syscall;
-			regs->epc -= 0x4;
+			regs->epc = restart_addr;
 			break;
 		}
 	}
 
+	// printk("~RISCV~ Before get_signal: a7 = %ld, a0 = %ld, orig_a0 = %ld, cause = %ld\n",
+	//        regs->a7, regs->a0, regs->orig_a0, regs->cause);
+
+	/*
+	 * Get the signal to deliver. When running under ptrace, at this point
+	 * the debugger may change all of our registers.
+	 */
+	if (get_signal(&ksig)) {
+		/*
+		 * Depending on the signal settings, we may need to revert the
+		 * decision to restart the system call, but skip this if a
+		 * debugger has chosen to restart at a different PC.
+		 */
+		if (regs->epc == restart_addr &&
+		    (retval == -ERESTARTNOHAND ||
+		     retval == -ERESTART_RESTARTBLOCK ||
+		     (retval == -ERESTARTSYS &&
+		      !(ksig.ka.sa.sa_flags & SA_RESTART)))) {
+			syscall_set_return_value(current, regs, -EINTR, 0);
+			regs->epc = continue_addr;
+		}
+
+		/* Actually deliver the signal */
+		handle_signal(&ksig, regs);
+		return;
+	}
+
+	// printk("~RISCV~ After get_signal: a7 = %ld, a0 = %ld, orig_a0 = %ld, cause = %ld\n",
+	//        regs->a7, regs->a0, regs->orig_a0, regs->cause);
+
+	/*
+	 * Handle restarting a different system call. As above, if a debugger
+	 * has chosen to restart at a different PC, ignore the restart.
+	 */
+	if (syscall && regs->epc == restart_addr) {
+		if (retval == -ERESTART_RESTARTBLOCK)
+			regs->a7 = __NR_restart_syscall;
+		// user_rewind_single_step(current);
+	}
+
 	/*
 	 * If there is no signal to deliver, we just put the saved
 	 * sigmask back.

Copied from my gitter msg:

Sorry for this very late reply as I forgot to turn on gitter notification :( But I wanted to share an exciting update about the development of the RISC-V version of CRIU. It now only fails 7 out of 454 zdtm tests! Here are the final results of ./zdtm.py run -a --keep-going:

################### 7 TEST(S) FAILED (TOTAL 454/SKIPPED 46) ####################
 * zdtm/static/apparmor_stacking(h)
 * zdtm/static/cmdlinenv00(ns)
 * zdtm/static/netns_lock_iptables(h)
 * zdtm/static/socket-tcp-closed-last-ack(uns)
 * zdtm/static/socket-tcp-nfconntrack(h)
 * zdtm/static/socket-tcp-syn-sent(uns)
 * zdtm/transition/maps007(unknown)
##################################### FAIL #####################################

I think failed tests related to iptables and sockets might be due to some disabled kernel configs. I haven't confirmed this yet, as compiling an Ubuntu kernel locally takes quite a bit of time on my side :(
I've also set up the cross-compile CI for RISC-V. As riscv64 is not an official stable arch in Debian yet (but it will be soon), I had to take a strange approach using Ubuntu ports for riscv64 cross-compiling. It now works with some dirty work in the Dockerfile.
The major problem right now is a critical bug related to ptrace and syscalls in riscv kernel. You can find more details here.
I'm going to finalize and send the patch (changing arch_do_signal_or_restart() to arm64 style) to LKML, but I'm not sure if this is a proper solution and how long it will take. Unfortunately, I cannot think of any workarounds (the orig_a0 approach also relies on kernel change, and it seems not reasonable according to the root cause). As it stands, without this fix, kernels <= 6.4 might only handle naive jobs, such as simple loops.
I'm planning to create a pull request soon, and I'll focus on organizing the commits and changes and fixing any potential issues (including the failed tests and CI setup). I'm really looking forward to your feedback and suggestions!

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.23%. Comparing base (cb39c62) to head (5f79f37).

❗ Current head 5f79f37 differs from pull request most recent head 91d3cfd. Consider uploading reports for the commit 91d3cfd to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##           criu-dev    #2234      +/-   ##
============================================
+ Coverage     70.21%   70.23%   +0.02%     
============================================
  Files           132      131       -1     
  Lines         34372    34370       -2     
============================================
+ Hits          24134    24141       +7     
+ Misses        10238    10229       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ancientmodern]

Update: The zdtm/static/cmdlinenv00 test now passes after syncing AT_VECTOR_SIZE to the correct value (64) in the riscv Linux kernel, and zdtm/static/apparmor_stacking should be fixed with #2235 :)

@mihalicyn, I have a small question about running make lint locally. I'm experiencing many more errors with flake8 on my side, even after reverting to the version used by CI (flake8 == 5.0.3). Do you have any idea what might be causing this difference? Thanks!

haorong at haorong:/criu [riscv64-upstream]$ make lint
flake8 --version
5.0.3 (mccabe: 0.7.0, pycodestyle: 2.9.1, pyflakes: 2.5.0, tryceratops: 2.3.2) CPython 3.10.4 on Linux
flake8 --config=scripts/flake8.cfg test/zdtm.py
test/zdtm.py:492:13: TRY200 Use 'raise from' to specify exception cause
test/zdtm.py:550:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:590:17: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:648:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:979:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:981:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:983:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:998:21: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:1276:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:1489:17: TRY300 Consider moving this statement to an 'else' block
test/zdtm.py:1664:17: TRY201 Simply use 'raise' without specifying exception object again
test/zdtm.py:1687:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:1700:17: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:1713:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:1722:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:1937:21: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:2011:17: TRY002 Create your own exception
test/zdtm.py:2040:13: TRY002 Create your own exception
test/zdtm.py:2113:17: TRY201 Simply use 'raise' without specifying exception object again
test/zdtm.py:2230:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:2363:13: TRY002 Create your own exception
test/zdtm.py:2363:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:2368:13: TRY002 Create your own exception
test/zdtm.py:2368:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:2374:13: TRY002 Create your own exception
test/zdtm.py:2374:13: TRY003 Avoid specifying long messages outside the exception class
test/zdtm.py:2628:9: TRY300 Consider moving this statement to an 'else' block
make: *** [Makefile:439: lint] Error 1

[github-actions]

A friendly reminder that this PR had no activity for 30 days.

[ancientmodern]

Really sorry for the delay. The past two months turned out to be more "fast-paced" than I had anticipated, and the riscv ubuntu VM I've been using for development got messed up after I installed a buggy kernel :( This led me to somewhat negatively sideline this PR and the corresponding kernel patch. Fortunately, I dug out an old VM snapshot from deep in my hard drive and successfully restored it to a dev-ready state today. I'll return to the PR as soon as my work schedule lightens up. Thanks for your patience and continued support! 🙏

[Cryolitia]

Hello, sorry for bother. Is there any progress?

I have rebased and fixed this PR on v4.0, in my repo: https://github.com/Cryolitia-Forks/criu/tree/ricv64

Hope this PR can be merged eventually.

best wishes.

[avagin]

@Cryolitia I will find time this week to review this series.

[mihalicyn]

Hey @Cryolitia,

thanks for your work on this!

I see that in your branch you also have some fixes on top and support for membarrier() C/R. Let's land this as a separate PR, just to make review a bit easier (or I can put your commits in this PR if you don't mind).

I'll update this PR myself (rebase and make small adjustments to make linter happy) as it's easier.

[Cryolitia]

or I can put your commits in this PR if you don't mind

Of course sure you could. Feel free to use it just keep me as co-author. ^_^ I'm afraid of that these fix are needed for criu to successfully build, at least on Arch Linix RISC-V.

[mihalicyn]

Of course sure you could. Feel free to use it just keep me as co-author. ^_^ I'm afraid of that these fix are needed for criu to successfully build, at least on Arch Linix RISC-V.

no doubt. All your authorship on the commits will be kept. ;-)

[mihalicyn]

Hey @Cryolitia,

please, can you add Signed-off-by tags to the commits in your branch https://github.com/Cryolitia-Forks/criu/commits/ricv64/ so I can import them safely ;-)

[mihalicyn]

It's worth mentioning that we have a signal issue (described in #2234 (comment)) fix in the kernel torvalds/linux@ce4f78f thanks to Haorong Lu (@ancientmodern).

[Cryolitia]

please, can you add Signed-off-by tags to the commits in your branch https://github.com/Cryolitia-Forks/criu/commits/ricv64/ so I can import them safely ;-)

sure, all done

[mihalicyn]

I'm currently playing with this one on a real hardware:

[    0.000000] Linux version 6.6.20-starfive (root@leo-virtual-machine) (riscv64-unknown-linux-gnu-gcc (g2ee5e430018) 12.2.0, GNU ld (GNU Binutils) 2.40.0.20230214) #41SF SMP Fri Sep 20 17:48:26 CST 2024
[    0.000000] Machine model: StarFive VisionFive 2 v1.3B

Getting the following error:

root@starfive:/mnt/mnt/criu# ./test/zdtm.py run --ignore-taint -t zdtm/static/cwd00 -f h
userns is supported
The kernel is tainted: '135168'
=== Run 1/1 ================ zdtm/static/cwd00
========================== Run zdtm/static/cwd00 in h ==========================
Start test
./cwd00 --pidfile=cwd00.pid --outfile=cwd00.out --dirname=cwd00.test
Run criu dump
Run criu restore
=[log]=> dump/zdtm/static/cwd00/54/1/restore.log
------------------------ grep Error ------------------------
b'(00.007038) pie: 54: Switched to the restorer 54'
b'(00.007044) pie: 54: vdso: Remap rt-vdso 0x3f7ff94000 -> 0x28000'
b'(00.007061) pie: 54: vdso: Remap rt-vvar 0x3f7ff92000 -> 0x26000'
b'(00.007076) pie: 54: vdso: Using gettimeofday() on vdso at 0x28a6c'
b'(00.007207) pie: 54: Error (criu/pie/restorer.c:1349): Unable to unmap (0x3f7f610000-0x800000000000): -22'
b'(00.007229) pie: 54: Error (criu/pie/restorer.c:2240): Restorer fail 54'
b'(00.007263) Error (criu/cr-restore.c:2314): Restoring FAILED.'
------------------------ ERROR OVER ------------------------
################# Test zdtm/static/cwd00 FAIL at CRIU restore ##################

I'll sort this stuff out.

[mihalicyn]

Ok, I know what's wrong with this munmap.

# cat /proc/cpuinfo 
processor	: 0
hart		: 1
isa		: rv64imafdc_zicntr_zicsr_zifencei_zihpm_zba_zbb
mmu		: sv39
uarch		: sifive,u74-mc
mvendorid	: 0x489
marchid		: 0x8000000000000007
mimpid		: 0x4210427

processor	: 1
hart		: 2
isa		: rv64imafdc_zicntr_zicsr_zifencei_zihpm_zba_zbb
mmu		: sv39
uarch		: sifive,u74-mc
mvendorid	: 0x489
marchid		: 0x8000000000000007
mimpid		: 0x4210427

processor	: 2
hart		: 3
isa		: rv64imafdc_zicntr_zicsr_zifencei_zihpm_zba_zbb
mmu		: sv39
uarch		: sifive,u74-mc
mvendorid	: 0x489
marchid		: 0x8000000000000007
mimpid		: 0x4210427

processor	: 3
hart		: 4
isa		: rv64imafdc_zicntr_zicsr_zifencei_zihpm_zba_zbb
mmu		: sv39
uarch		: sifive,u74-mc
mvendorid	: 0x489
marchid		: 0x8000000000000007
mimpid		: 0x4210427

We have hardcoded TASK_SIZE for RISC-V here https://github.com/checkpoint-restore/criu/pull/2234/files#diff-4b0376104d8abb871cafe463f9c4a9a786b3a09cad4352259b7261731cdba60eR193:

#define TASK_SIZE 0x800000000000UL // hardcoded for SV48 MMU

but my MMU is SV39. I should improve this logic to detect MMU and choose a proper TASK_SIZE value.
After re-hardcoding it to 0x4000000000 (which is suitable for my MMU, see also https://github.com/torvalds/linux/blob/master/arch/riscv/include/asm/pgtable.h#L908) I can't see munmap error anymore as expected, but process crashes on a later stages (probably on sigreturn). Need to dive deeper.

[mihalicyn]

and... this:

root@starfive:/mnt/mnt/criu/compel/test/infect# ./spy 
Checking the victim alive
1, want 1
42, want 42
Infecting the victim
Stopping task
Preparing parasite ctl
	LC4: Preparing seqsk for 32765
Configuring contexts
Infecting
	LC3: Set up parasite blob using memfd
	LC3: Putting parasite blob into 0x3f8ccb8000->0x3fac5b2000
	LC3: Dumping FPU registers for 32765
	LC3: Putting tsock into pid 32765
	LC3: Wait for parasite being daemonized...
pie: 32765: Running daemon thread leader
	LC4: Wait for ack 2 on daemon socket
pie: 32765: __sent ack msg: 2 2 0
	LC4: Fetched ack: 2 2 0
	LC3: Parasite 32765 has been switched to daemon mode
Running cmd 1
	LC4: Sent msg to daemon 64 0 0
pie: 32765: Daemon waits for command
	LC4: Wait for ack 64 on daemon socket
pie: 32765: __fetched msg: 64 0 0
pie: 32765: __sent ack msg: 64 64 0
pie: 32765: Daemon waits for command
	LC4: Fetched ack: 64 64 0
Running cmd 2
	LC4: Sent msg to daemon 65 0 0
pie: 32765: __fetched msg: 65 0 0
	LC4: Wait for ack 65 on daemon socket
	LC4: Fetched ack: 65 65 0
Curing
	LC4: Waiting for 32765 to trap
	LC4: Daemon 32765 exited trapping
	LC4: Sent msg to daemon 3 0 0
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 178, required is 139
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 64, required is 139
pie: 32765: __sent ack msg: 65 65 0
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 178, required is 139
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 64, required is 139
pie: 32765: Daemon waits for command
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 207, required is 139
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 178, required is 139
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 64, required is 139
pie: 32765: __fetched msg: 3 0 0
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 178, required is 139
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 178, required is 139
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 64, required is 139
pie: 32765: 32765: new_sp=0x3fac5b3e00 ip 0x3fac5235ce
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 57, required is 139
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 57, required is 139
	LC4: 32765 was trapped
	LC4: `- Expecting exit
	LC4: 32765 was trapped
	LC4: 32765 (native) is going to execute the syscall 139, required is 139
	LC4: 32765 was stopped
	LC4: 	Unseizing 32765 into 1
Done
Closing victim stdin
Waiting for victim to die
Checking the result
138, want 138
403, want 403
Something went WRONG

should be fixed before.

[mihalicyn]

In general, I think we can merge this as it is for now (with only support for SV48 MMU). But I'll have to do some improvements on top to support cheaper SV39 MMU and more modern SV57. But we are pretty limited in access to hardware now and sometimes we have to move blindly...

[Cryolitia]

In general, I think we can merge this as it is for now (with only support for SV48 MMU). But I'll have to do some improvements on top to support cheaper SV39 MMU and more modern SV57. But we are pretty limited in access to hardware now and sometimes we have to move blindly...

I'm sorry that I don't have much developing experience on kernel, but if you need more riscv64 hardware for simple testing. We (PLCT Lab) have A truckload of various riscv64 hardware from HiFive Unmatched, LicheePi 4A to SG2042. Feel free to tell me if it's needed.

I pay my highest respects to your work.

[avagin]

Thanks to all involved in this work. This is a great starting point.