Issue 183 riscv64 checkpoint restore - #192
Conversation
Add riscv64 support to the checkpoint/restore path: - restore_blob.rs: rearm_restartable_syscall (a0 return, a7 syscall_nr, 4-byte ecall rewind) and build_fpstate_image (raw fpregs passthrough) - restore-stub.c: full riscv64 #ifdef port with signal frame layout, syscall numbers, sc6 wrapper, rt_sigreturn, and _start entry - sandbox.rs: relax restore gate to include riscv64 - build.rs: detect riscv64 target and try cross-compilers - resume.rs: generalize stub_links_at_the_reserved_base cfg gate and add riscv64 synthetic-image restore test
…ore gate Add three riscv64 unit tests in restore_blob.rs that mirror the existing x86_64 tests: restart_sentinel_rewinds_pc_onto_the_ecall (a0=-514, a7=34, pc rewound by 4), non_restart_errno_is_left_alone (-515 ignored), and fpstate_image_passes_through_raw_fpregs (identity passthrough). Relax the runtime gate in test_restore_glibc_vdso_program_resumes from cfg!(not(x86_64)) to cfg!(not(any(x86_64, riscv64))) so the integration test runs on riscv64 as well.
congwang-mk
left a comment
There was a problem hiding this comment.
Reviewed the riscv64 port. The direction is right and the ABI research (1:1 sc_regs/ptrace ordering, no arch_prctl, no xstate framing, dup3) is accurate, but the code has not run yet: the riscv64 stub does not compile, and CI cannot tell you that because build_static downgrades a compiler failure to a cargo:warning and every riscv64 test then skips on !stub.exists(). The Rust cross-build (riscv64) job is green for the same reason.
I checked the findings below on a real riscv64 host (Sv39, 6.18.3) and against the kernel uapi headers rather than by reading alone.
Six blockers, in the order you'd hit them:
restore-stub.c—u8is never typedef'd; the riscv64 branch fails to compile.restore-stub.c—uc_mcontextis at 176, not 168;rt_sigreturnrestores a register file shifted by 8 bytes.build.rs/restore_blob.rs:45— the 3 TiBSTUB_BASEis above the Sv39 user ceiling; the stub SIGSEGVs at exec on real boards.resume.rs— the synthetic payload'slui/addiyields0x2000_0040, notCODE + 64.restore_blob.rs— the restart re-arm writes the syscall number intoa0, which is the first argument on riscv64; andorig_a0isn't reachable throughNT_PRSTATUS.build.rs— a native riscv64 build dropsccfrom the compiler list, so the stub is never built there at all.
Plus smaller things inline: the #ifdef __riscv guard also matches RV32, and the 544-byte sc_fpregs bound overshoots the kernel's 528-byte union (the clamp itself is dead code).
One doc nit outside the diff: sandbox.rs:1067 still says "x86_64 restore engine only." in the restore_interactive doc comment, three lines above the gate this PR widened.
Since none of this can be caught by the current CI shape, it's worth making the stub build failure loud on the arches we claim to support — a cargo:warning is invisible in a green build, which is how all six of these got this far.
| * signal frame's sc_regs[4]; nothing needs arch_prctl. */ | ||
| struct sigctx { | ||
| u64 gregs[32]; /* sc_regs: 32 gregs, 256 bytes */ | ||
| u8 fpregs[544]; /* sc_fpregs: union __riscv_fp_state (max Q ext) */ |
There was a problem hiding this comment.
Blocker: this branch does not compile — u8 is never declared.
The typedefs at lines 136-138 are u64, u32, i64 only; u8 was never needed before this PR. Building the stub natively on riscv64 (Debian gcc 14, mmu: sv39) with exactly the flags build.rs passes:
/tmp/stub.c:254:5: error: unknown type name 'u8'
254 | u8 fpregs[544];
/tmp/stub.c:265:5: error: unknown type name 'u8'
/tmp/stub.c:271:5: error: unknown type name 'u8'
Adding typedef unsigned char u8; next to the others makes it compile and link.
This is why CI is green while none of the riscv64 work has ever run: build_static turns a compiler failure into a cargo:warning, and every riscv64 test then early-returns on !stub.exists().
| u64 ss_size; /* 0x20 */ | ||
| u64 uc_sigmask; /* 0x28 */ | ||
| u8 __unused[120]; /* 0x30 */ | ||
| struct sigctx mc; /* 0xA8 (168) */ |
There was a problem hiding this comment.
Blocker: uc_mcontext is at offset 176 (0xB0), not 168 (0xA8).
struct sigcontext has 16-byte alignment on riscv64 — both __riscv_q_ext_state.f[64] and __riscv_extra_ext_header.__padding carry __attribute__((aligned(16))), so the union and therefore struct sigcontext are 16-aligned. 168 is not a multiple of 16, so the kernel pads to 176. Measured on a riscv64 host against <asm/ucontext.h>:
offsetof(kernel struct ucontext, uc_mcontext) = 176 (0xb0)
sizeof(struct sigcontext) = 784
struct sigctx here has alignment 8, so the compiler places mc at exactly 168 and the whole register file lands 8 bytes low. rt_sigreturn then reads pc from gregs[1] (ra), sp from gregs[2] (gp), and so on. In the new synthetic test ra is 0, so the restored process jumps to 0 and dies.
Fix: __attribute__((aligned(16))) on struct sigctx (or widen __unused to 128 bytes and keep the comment honest).
| #define STUB_BASE 0x30000000000UL | ||
| #define STUB_SPAN 0x400000UL | ||
|
|
||
| #ifdef __riscv |
There was a problem hiding this comment.
__riscv is also defined on RV32, where __riscv_xlen == 32. The u64-based struct sigctx/struct uctx mirrors, the SYS_* numbers, and the 32x8-byte sc_regs copy are all RV64-only, and the #else #error added below never fires there — so an RV32 build would compile silently wrong rather than being rejected.
Suggest #if defined(__riscv) && __riscv_xlen == 64.
| if (nregs > 32) nregs = 32; | ||
| memcpy(sf.uc.mc.gregs, gp, nregs * sizeof(u64)); | ||
| if (h->fpstate_len) { | ||
| if (h->fpstate_len > sizeof(sf.uc.mc.fpregs)) |
There was a problem hiding this comment.
Two things here:
-
This clamp is dead. Line 349 already does
if (h->fpstate_len > FP_MAX) die(3);, andFP_MAXis 544 on riscv64 — the same bound. It also mutates the header in place insidectrl_buf, which the rest of the stub treats as read-only after validation. -
If it were reachable, 544 is the wrong bound. The kernel's second
sigcontextmember isunion { union __riscv_fp_state sc_fpregs; struct __riscv_extra_ext_header sc_extdesc; }, which is 528 bytes, withsc_extdesc.reservedat offset 516 andsc_extdesc.hdrat 520 (verified:sizeof(struct sigcontext) == 784= 256 + 528). A 544-byte copy would overwritereserved, andrestore_sigcontext()returns-EINVALon a non-zeroreserved, sort_sigreturnwould fail instead of resuming.
Not reachable today (riscv NT_PRFPREG yields 264 bytes), but FP_MAX should be 516/528 rather than 544, and the comment above it calls 544 the Q-extension size, which it isn't.
| const PC: usize = 0; | ||
| if let (Some(&a0), Some(&a7)) = (regs.get(A0), regs.get(A7)) { | ||
| if matches!(a0 as i64, -512 | -513 | -514 | -516) { | ||
| regs[A0] = a7; // restore original syscall number |
There was a problem hiding this comment.
Blocker: on riscv64 a0 is the first argument, not the syscall number, so this corrupts the restarted call.
The x86_64 version works because rax carries both the number and the return value. riscv64 splits them: the number stays in a7 (the kernel never clobbers it) and a0 is overwritten by the return value. The kernel's own fixup is arch/riscv/kernel/signal.c:
case -ERESTARTNOHAND:
case -ERESTARTSYS:
case -ERESTARTNOINTR:
case -ERESTART_RESTARTBLOCK:
regs->a0 = regs->orig_a0;
regs->epc = restart_addr;
break;Concretely: a checkpoint taken while blocked in read(5, buf, n) has a0 = -ERESTARTSYS, a7 = 63. After this re-arm the ecall re-executes as read(63, buf, n) — EBADF, or silently the wrong fd.
The harder problem is that orig_a0 is not recoverable from ps.regs: riscv_gpr_get() writes only sizeof(struct user_regs_struct) (256 bytes, pc..t6), and orig_a0 sits past status/badaddr/cause in pt_regs. So the re-arm as designed cannot be implemented from the captured register file — it needs an extra capture (e.g. /proc/<pid>/syscall, which does expose the original arg0) or should be left unimplemented on riscv64 rather than writing a wrong a0.
Two related notes:
- For
-ERESTART_RESTARTBLOCKthe kernel setsa7 = __NR_restart_syscall; it does not re-run the original number. - The new unit test at line 911 asserts
regs[10] == 34, i.e. it locks in this behaviour, so it will need to change with the fix.
| || target.starts_with("riscv64gc") | ||
| { | ||
| ( | ||
| &["riscv64-linux-gnu-gcc", "riscv64-unknown-linux-gnu-gcc"][..], |
There was a problem hiding this comment.
Blocker: a native riscv64 build loses the compiler entirely.
TARGET is riscv64gc-unknown-linux-gnu for a plain cargo build on a riscv64 host too, so the candidate list becomes just these two cross-prefixed names — neither of which exists on a riscv64 machine, where the compiler is /usr/bin/cc / /usr/bin/gcc (checked on our riscv64 test host). The stub is then never built, stub_links_at_the_reserved_base, the new synthetic test, and test_restore_glibc_vdso_program_resumes all silently return early, and restore_interactive fails at runtime with "restore-stub was not built" — on exactly the platform this PR is for.
Keep cc in the list, gated on HOST so a genuine cross build doesn't silently produce a host binary:
let host = std::env::var("HOST").unwrap_or_default();
// ... riscv64 branch:
if host.starts_with("riscv64") {
&["cc", "riscv64-linux-gnu-gcc", "riscv64-unknown-linux-gnu-gcc"][..]
} else {
&["riscv64-linux-gnu-gcc", "riscv64-unknown-linux-gnu-gcc"][..]
}Minor, same hunk: || target.starts_with("riscv64gc") on line 39 is subsumed by target.starts_with("riscv64").
| @@ -47,7 +66,7 @@ fn main() { | |||
| "-fno-tree-loop-distribute-patterns", | |||
| "-Wl,-Ttext-segment=0x30000000000", | |||
There was a problem hiding this comment.
Blocker: 3 TiB is above the Sv39 user-address ceiling, so the stub cannot be exec'd on real riscv64 hardware.
restore_blob::STUB_BASE is 0x300_0000_0000 and this link address matches it, but Sv39 (what JH7110, SpacemiT K1, and most riscv64 Linux boards run) gives TASK_SIZE = 256 GiB. The PR's own new comment in resume.rs says addresses must stay below 0x40_0000_0000; this is 12x over that.
Verified on a riscv64 host (mmu: sv39, [stack] at 0x3ff1469000):
mmap STUB_BASE 0x300_0000_0000 -> ENOMEM
mmap test CODE 0x200_0000_0000 -> ENOMEM
mmap 0x2000_0000 -> ok
$ ./restore-stub # ET_EXEC with p_vaddr = 0x300_0000_0000
Segmentation fault # elf_map() fails past the point of no return
Relinking the same source at -Wl,-Ttext-segment=0x3000000000 (192 GiB, under the ceiling) makes it exec and run correctly — it reaches the blob parser and exits 3 on a short blob, as designed.
So riscv64 needs its own stub base inside the Sv39 window, chosen here and in restore_blob.rs:45 together (the stub_links_at_the_reserved_base test exists precisely to catch them drifting apart, and it will once the stub actually builds).
Aside, since it's the obvious suspicion with a 3 TiB text segment: -mcmodel is not a problem. GCC on riscv64 Linux defaults to medany, and la sp, stub_stack assembles to auipc/ld through the GOT — the link succeeds at either base.
| let mut w = 0usize; | ||
| let mut put = |bytes: &[u8]| { c[w..w + bytes.len()].copy_from_slice(bytes); w += bytes.len(); }; | ||
| put(&0x00A00513u32.to_le_bytes()); // addi a0, zero, 10 | ||
| put(&0x200005B7u32.to_le_bytes()); // lui a1, 0x20000 |
There was a problem hiding this comment.
Blocker: this pair does not materialize CODE + 64.
lui a1, 0x20000 puts 0x20000 in bits 31:12, giving a1 = 0x2000_0000; the addi makes it 0x2000_0040. But CODE + 64 is 0x200_0000_0040 — lui+addi reach 32 bits, and CODE is 2^41. The restored payload calls write(10, 0x2000_0040, 1) against an unmapped address, gets -EFAULT, and the test fails at assert_eq!(n, 1, ...).
The encoded instructions are self-consistent with CODE = 0x2000_0000, so it looks like the constant on line 588 picked up three extra zeros.
That also fixes a second problem in the same test: 0x200_0000_0000 is 2 TiB, which is 8x above the Sv39 ceiling the comment on line 585 cites, and mmap there returns ENOMEM on Sv39 hardware (measured). const CODE = 0x2000_0000 / const STACK = 0x2001_0000 satisfies both the encoding and the comment.
restore-stub.c: - add missing `typedef unsigned char u8;` - fix uc_mcontext offset: 0xA8→0xB0 (sigcontext is 16-aligned, widen __unused 120→128) - guard __riscv with __riscv_xlen==64 so RV32 is not silently miscompiled - fix FP_MAX 544→516 (last safe byte before sc_extdesc.reserved); fpregs array 544→528 (kernel union size); remove dead clamp - make STUB_BASE arch-conditional: riscv64 uses 0x3000000000 (192 GiB, below Sv39 ceiling) restore_blob.rs: - remove the broken riscv64 rearm_restartable_syscall that wrote a7 into a0 (corrupting the restarted call). orig_a0 is not recoverable from ptrace_getregs on riscv64. Fall through to the no-op fallback instead. - remove the unit tests that asserted the wrong behaviour - split STUB_BASE per-arch (x86_64 0x300_0000_0000, riscv64 0x30_0000_0000) build.rs: - when HOST is riscv64, include cc in the compiler search list so a native build finds the host compiler - pass the correct -Wl,-Ttext-segment= per architecture resume.rs: - fix CODE constant (2 TiB→0x2000_0000) and STACK constant so the synthetic-image test addresses stay within Sv39 test_restore.rs: split STUB_BASE per-arch sandbox.rs, sandbox.py: update comments from "x86_64 only" to acknowledge riscv64 support test files (restore.rs, integration.rs, test_checkpoint.py): clarify that the counter program is x86_64-only, not checkpoint/restore itself
The previous edit to build.rs lost the println! that emits RESTORE_STUB_PATH, causing CI to fail at compile time with "environment variable RESTORE_STUB_PATH not defined".
The riscv64 rearm_restartable_syscall was removed because it wrote a7 into a0, corrupting the restarted call. But the fallback no-op was gated on cfg(not(any(x86_64, riscv64))), so on riscv64 no rearm_restartable_syscall existed at all and the cross-build failed. Broaden the fallback to cfg(not(target_arch = "x86_64")) so riscv64 (and any future arch) gets the no-op.
STUB_BASE was split per-arch (x86_64=0x300_0000_0000, riscv64=0x30_0000_0000), but aarch64 builds (ubuntu-24.04-arm) had no definition. Add a 0-valued fallback for all other arches; STUB_BASE is only referenced in the reserved-window check which is dead on arches where the restore gate rejects at runtime.
STUB_BASE=0 on architectures without a restore stub created a false keep interval (0, STUB_SPAN) that overlapped real user-space mappings, causing plan_sweep to return empty sweep lists and plan() to reject legitimate checkpoint regions. - plan_sweep: only chain stub window into keep set when STUB_BASE > 0 - plan: only check stub-window overlap when STUB_BASE > 0 - Tests using STUB_BASE in data are now cfg-guarded to stub-capable archs Fixes sweep_takes_only_the_uncovered_part_of_a_partly_recorded_mapping and blob_interns_a_repeated_mapping_path_once on aarch64/ubuntu-24.04-arm.
Summary
Port the checkpoint/restore engine to riscv64, matching x86_64 functionality:
#ifdef __riscv64port with signal frame layout,syscall numbers,
sc6()ecall wrapper,rt_sigreturn, and_startentryrearm_restartable_syscall(a0 return, a7 syscall_nr,4-byte ecall rewind) and
build_fpstate_image(raw passthrough)file, a7+ecall machine code, Sv39-safe addresses
test_restore_glibc_vdso_program_resumesruns on riscv64
Files changed
restore-stub.cresume.rsrestore_blob.rsbuild.rssandbox.rstest_restore.rsKey design decisions
BLOB_MAGIC,VERSION,HEADER_LEN;register count determined by
regs_len; both arches are LE__riscv_d_ext_statedirectly insc_fpregs— no separatefp_buf, no magic framinguc_mcontextat offset 0xA8=168;sc_regs[32]is 1:1with
user_regs_struct— no remapping, noCSGSFS, noarch_prctl0x40_0000_0000