Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for the Ziren (zkMIPS) architecture as a new zkVM target to the state transition runtime. Ziren is a MIPS32-based zero-knowledge virtual machine that uses MIPS syscalls for I/O and system operations. The implementation follows the established pattern of other zkVM targets in the codebase (risc0, zisk, openvm, powdr) by providing the required interface functions: halt, get_input, free_input, get_allocator, and an io module.
Changes:
- Added Ziren MIPS32 target with linker script, assembly startup, syscall definitions, and runtime library
- Configured build system to compile for mipsel-freestanding-none with mips32r2 CPU features
- Implemented syscall wrapper functions for MIPS syscall ABI using inline assembly
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkgs/state-transition-runtime/src/ziren/ziren.ld | MIPS32 linker script defining memory layout, sections, and entry point at 0x00400000 with heap and stack regions |
| pkgs/state-transition-runtime/src/ziren/syscalls.zig | MIPS syscall definitions and inline assembly wrappers for 0-3 argument syscalls using MIPS register conventions |
| pkgs/state-transition-runtime/src/ziren/start.s | MIPS assembly startup code that initializes global pointer and stack pointer before jumping to main |
| pkgs/state-transition-runtime/src/ziren/lib.zig | Runtime library providing halt, get_input, free_input, and get_allocator functions for Ziren zkVM |
| pkgs/state-transition-runtime/src/ziren/io.zig | I/O functions for printing, writing, and reading hint data using Ziren syscalls |
| build.zig | Added Ziren to zkvm_targets array with MIPS little-endian target configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn get_input(allocator: std.mem.Allocator) []const u8 { | ||
| const input_len = io.hint_len(); | ||
|
|
||
| // Sanity check: limit to 10MB to prevent excessive allocation | ||
| if (input_len > 10 * 1024 * 1024) { | ||
| @panic("input size exceeds maximum allowed (10MB)"); | ||
| } | ||
|
|
||
| var buffer: []u8 = allocator.alloc(u8, input_len) catch @panic("could not allocate space for the input slice"); | ||
| const bytes_read = io.hint_read(buffer); | ||
| if (bytes_read != input_len) { | ||
| @panic("input size mismatch"); | ||
| } | ||
|
|
||
| return buffer[0..bytes_read]; | ||
| } | ||
|
|
||
| pub fn free_input(_: std.mem.Allocator) void {} |
There was a problem hiding this comment.
Memory leak: get_input allocates memory (line 17) but free_input does not free it. This will cause the FixedBufferAllocator to run out of memory if get_input is called multiple times.
The current implementation follows zisk's free_input signature (taking only allocator) but allocates memory like risc0/openvm do. This is inconsistent. Since the signature matches zisk's pattern, consider changing get_input to NOT allocate like zisk does - return a slice to pre-existing hint memory instead of allocating a new buffer. This would make the implementation consistent with the free_input signature and avoid the memory leak.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const rocksdb = b.dependency("rocksdb", .{ | ||
| .target = target, | ||
| .optimize = .ReleaseFast, | ||
| .optimize = optimize, |
There was a problem hiding this comment.
rocksdb dependency was previously forced to an optimized build (likely to avoid huge debug artifacts). Switching .optimize to the user-selected optimize can significantly increase build time and disk usage for Debug/ReleaseSafe builds. If the intent is still to keep RocksDB lightweight, keep .ReleaseFast here (or gate the behavior behind a build option).
| .optimize = optimize, | |
| .optimize = .ReleaseFast, |
| return asm volatile ("syscall" | ||
| : [ret] "={$2}" (-> u32), | ||
| : [num] "{$2}" (num), | ||
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); | ||
| } | ||
|
|
||
| pub inline fn syscall_1(num: u32, a0: u32) u32 { | ||
| return asm volatile ("syscall" | ||
| : [ret] "={$2}" (-> u32), | ||
| : [num] "{$2}" (num), | ||
| [a0] "{$4}" (a0), | ||
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); | ||
| } | ||
|
|
||
| pub inline fn syscall_2(num: u32, a0: u32, a1: u32) u32 { | ||
| return asm volatile ("syscall" | ||
| : [ret] "={$2}" (-> u32), | ||
| : [num] "{$2}" (num), | ||
| [a0] "{$4}" (a0), | ||
| [a1] "{$5}" (a1), | ||
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); | ||
| } | ||
|
|
||
| pub inline fn syscall_3(num: u32, a0: u32, a1: u32, a2: u32) u32 { | ||
| return asm volatile ("syscall" | ||
| : [ret] "={$2}" (-> u32), | ||
| : [num] "{$2}" (num), | ||
| [a0] "{$4}" (a0), | ||
| [a1] "{$5}" (a1), | ||
| [a2] "{$6}" (a2), | ||
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); |
There was a problem hiding this comment.
The inline asm ties both the input syscall number and the output return value to $v0/$2, but it models them as separate operands. For LLVM inline asm this typically needs to be expressed as a single in/out operand (so the compiler knows the register is both read and written), otherwise it can miscompile or be rejected. Consider using an explicit inout operand for $2 and apply the same pattern across syscall_0..syscall_3.
| return asm volatile ("syscall" | |
| : [ret] "={$2}" (-> u32), | |
| : [num] "{$2}" (num), | |
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); | |
| } | |
| pub inline fn syscall_1(num: u32, a0: u32) u32 { | |
| return asm volatile ("syscall" | |
| : [ret] "={$2}" (-> u32), | |
| : [num] "{$2}" (num), | |
| [a0] "{$4}" (a0), | |
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); | |
| } | |
| pub inline fn syscall_2(num: u32, a0: u32, a1: u32) u32 { | |
| return asm volatile ("syscall" | |
| : [ret] "={$2}" (-> u32), | |
| : [num] "{$2}" (num), | |
| [a0] "{$4}" (a0), | |
| [a1] "{$5}" (a1), | |
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); | |
| } | |
| pub inline fn syscall_3(num: u32, a0: u32, a1: u32, a2: u32) u32 { | |
| return asm volatile ("syscall" | |
| : [ret] "={$2}" (-> u32), | |
| : [num] "{$2}" (num), | |
| [a0] "{$4}" (a0), | |
| [a1] "{$5}" (a1), | |
| [a2] "{$6}" (a2), | |
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); | |
| var v0: u32 = num; | |
| asm volatile ("syscall" | |
| : [v0_rw] "+{$2}" (v0), | |
| : | |
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); | |
| return v0; | |
| } | |
| pub inline fn syscall_1(num: u32, a0: u32) u32 { | |
| var v0: u32 = num; | |
| asm volatile ("syscall" | |
| : [v0_rw] "+{$2}" (v0), | |
| : [a0] "{$4}" (a0), | |
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); | |
| return v0; | |
| } | |
| pub inline fn syscall_2(num: u32, a0: u32, a1: u32) u32 { | |
| var v0: u32 = num; | |
| asm volatile ("syscall" | |
| : [v0_rw] "+{$2}" (v0), | |
| : [a0] "{$4}" (a0), | |
| [a1] "{$5}" (a1), | |
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); | |
| return v0; | |
| } | |
| pub inline fn syscall_3(num: u32, a0: u32, a1: u32, a2: u32) u32 { | |
| var v0: u32 = num; | |
| asm volatile ("syscall" | |
| : [v0_rw] "+{$2}" (v0), | |
| : [a0] "{$4}" (a0), | |
| [a1] "{$5}" (a1), | |
| [a2] "{$6}" (a2), | |
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); | |
| return v0; |
| pub inline fn syscall_3(num: u32, a0: u32, a1: u32, a2: u32) u32 { | ||
| return asm volatile ("syscall" | ||
| : [ret] "={$2}" (-> u32), | ||
| : [num] "{$2}" (num), | ||
| [a0] "{$4}" (a0), | ||
| [a1] "{$5}" (a1), | ||
| [a2] "{$6}" (a2), | ||
| : .{ .r1 = true, .r3 = true, .r7 = true, .r8 = true, .r9 = true, .r10 = true, .r11 = true, .r12 = true, .r13 = true, .r14 = true, .r15 = true, .r24 = true, .r25 = true, .memory = true }); |
There was a problem hiding this comment.
The clobber list does not include $a0-$a2 ($4-$6) even though those registers are used to pass arguments and may be modified by the syscall handler. Without marking them clobbered (or modeling them as outputs), the compiler may assume their values are preserved across the asm block and reuse stale values. Please update clobbers consistently for the syscall wrappers that take arguments.
| const mem_start: [*]u8 = @ptrCast(&_end); | ||
| const mem_end: [*]u8 = @ptrFromInt(0x7E000000); // _heap_end from linker script | ||
| const mem_size: usize = @intFromPtr(mem_end) - @intFromPtr(mem_start); |
There was a problem hiding this comment.
get_allocator hard-codes 0x7E000000 as the heap end while the linker script also defines _heap_end with the same value. Keeping this duplicated constant risks drift if the memory map changes. Consider declaring _heap_end as an extern symbol and deriving mem_end from it, or remove the _heap_end symbol from the linker script if it’s intentionally unused.
No description provided.