From 4b81f482fbdfc7521d1fa4e0dfda6947a33b6611 Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Wed, 18 Dec 2024 17:41:06 +0800 Subject: [PATCH] Remove ROM from platform Previously we had machinery that makes sure that executable code and data memory do not overlap. But in a Harvard Archicture where code and data live in different address spaces anyway, that is not necessary. (Our VM implements a Harvard Architecture. Our circuits have always implemented that archicture, and the emulator has recently followed suit.) Note how this PR does not remove any checks or constraints from the circuits. Ie a sufficiently motivated and independent minded prover could have always done what this PR makes easier. --- ceno_emul/src/lib.rs | 2 +- ceno_emul/src/platform.rs | 27 +++++---------------------- ceno_emul/tests/test_vm_trace.rs | 15 +++++---------- ceno_zkvm/examples/riscv_opcodes.rs | 8 ++++---- ceno_zkvm/src/e2e.rs | 2 -- ceno_zkvm/src/scheme/mock_prover.rs | 20 ++++---------------- ceno_zkvm/src/scheme/tests.rs | 6 +++--- 7 files changed, 22 insertions(+), 58 deletions(-) diff --git a/ceno_emul/src/lib.rs b/ceno_emul/src/lib.rs index ba4195815..d86784d90 100644 --- a/ceno_emul/src/lib.rs +++ b/ceno_emul/src/lib.rs @@ -4,7 +4,7 @@ mod addr; pub use addr::*; mod platform; -pub use platform::{CENO_PLATFORM, Platform}; +pub use platform::{CENO_PLATFORM, MOCK_BASE, MOCK_ENTRY_POINT, Platform}; mod tracer; pub use tracer::{Change, MemOp, ReadOp, StepRecord, Tracer, WriteOp}; diff --git a/ceno_emul/src/platform.rs b/ceno_emul/src/platform.rs index e8b06721c..e37c56770 100644 --- a/ceno_emul/src/platform.rs +++ b/ceno_emul/src/platform.rs @@ -9,7 +9,6 @@ use crate::addr::{Addr, RegIdx}; /// - codes of environment calls. #[derive(Clone, Debug)] pub struct Platform { - pub rom: Range, pub prog_data: BTreeSet, pub stack: Range, pub heap: Range, @@ -20,7 +19,6 @@ pub struct Platform { } pub const CENO_PLATFORM: Platform = Platform { - rom: 0x2000_0000..0x3000_0000, prog_data: BTreeSet::new(), stack: 0xB0000000..0xC0000000, heap: 0x8000_0000..0xFFFF_0000, @@ -29,13 +27,11 @@ pub const CENO_PLATFORM: Platform = Platform { unsafe_ecall_nop: false, }; +pub const MOCK_ENTRY_POINT: Addr = 0x2000_0000; +pub const MOCK_BASE: Addr = 0x2000_0000; + impl Platform { // Virtual memory layout. - - pub fn is_rom(&self, addr: Addr) -> bool { - self.rom.contains(&addr) - } - pub fn is_prog_data(&self, addr: Addr) -> bool { self.prog_data.contains(&(addr & !0x3)) } @@ -63,14 +59,7 @@ impl Platform { (vma >> 8) as RegIdx } - // Startup. - - pub const fn pc_base(&self) -> Addr { - self.rom.start - } - // Permissions. - pub fn can_read(&self, addr: Addr) -> bool { self.can_write(addr) } @@ -110,22 +99,16 @@ impl Platform { #[cfg(test)] mod tests { use super::*; - use crate::{VMState, WORD_SIZE}; + use crate::VMState; #[test] fn test_no_overlap() { let p = CENO_PLATFORM; - // ROM and RAM do not overlap. - assert!(!p.is_rom(p.heap.start)); - assert!(!p.is_rom(p.heap.end - WORD_SIZE as Addr)); - assert!(!p.is_ram(p.rom.start)); - assert!(!p.is_ram(p.rom.end - WORD_SIZE as Addr)); - // Registers do not overlap with ROM or RAM. + // Registers do not overlap with RAM. for reg in [ Platform::register_vma(0), Platform::register_vma(VMState::REG_COUNT - 1), ] { - assert!(!p.is_rom(reg)); assert!(!p.is_ram(reg)); } } diff --git a/ceno_emul/tests/test_vm_trace.rs b/ceno_emul/tests/test_vm_trace.rs index ef3b8820e..0a5391c12 100644 --- a/ceno_emul/tests/test_vm_trace.rs +++ b/ceno_emul/tests/test_vm_trace.rs @@ -6,15 +6,15 @@ use std::{ }; use ceno_emul::{ - CENO_PLATFORM, Cycle, EmuContext, InsnKind, Instruction, Platform, Program, StepRecord, Tracer, - VMState, WordAddr, encode_rv32, + CENO_PLATFORM, Cycle, EmuContext, InsnKind, Instruction, MOCK_BASE, MOCK_ENTRY_POINT, Platform, + Program, StepRecord, Tracer, VMState, WordAddr, encode_rv32, }; #[test] fn test_vm_trace() -> Result<()> { let program = Program::new( - CENO_PLATFORM.pc_base(), - CENO_PLATFORM.pc_base(), + MOCK_ENTRY_POINT, + MOCK_BASE, program_fibonacci_20(), Default::default(), ); @@ -40,12 +40,7 @@ fn test_vm_trace() -> Result<()> { #[test] fn test_empty_program() -> Result<()> { - let empty_program = Program::new( - CENO_PLATFORM.pc_base(), - CENO_PLATFORM.pc_base(), - vec![], - BTreeMap::new(), - ); + let empty_program = Program::new(MOCK_ENTRY_POINT, MOCK_BASE, vec![], BTreeMap::new()); let mut ctx = VMState::new(CENO_PLATFORM, Arc::new(empty_program)); let res = run(&mut ctx); assert!(matches!(res, Err(e) if e.to_string().contains("InstructionAccessFault")),); diff --git a/ceno_zkvm/examples/riscv_opcodes.rs b/ceno_zkvm/examples/riscv_opcodes.rs index 54813659e..76281157f 100644 --- a/ceno_zkvm/examples/riscv_opcodes.rs +++ b/ceno_zkvm/examples/riscv_opcodes.rs @@ -13,8 +13,8 @@ use clap::Parser; use ceno_emul::{ CENO_PLATFORM, EmuContext, InsnKind::{ADD, ADDI, BLTU, ECALL, LW}, - Instruction, Platform, Program, StepRecord, Tracer, VMState, Word, WordAddr, encode_rv32, - encode_rv32u, + Instruction, MOCK_BASE, MOCK_ENTRY_POINT, Platform, Program, StepRecord, Tracer, VMState, Word, + WordAddr, encode_rv32, encode_rv32u, }; use ceno_zkvm::{ scheme::{PublicValues, constants::MAX_NUM_VARIABLES, verifier::ZKVMVerifier}, @@ -71,8 +71,8 @@ fn main() { let program_size = program_code.len(); let program = Program::new( - CENO_PLATFORM.pc_base(), - CENO_PLATFORM.pc_base(), + MOCK_ENTRY_POINT, + MOCK_BASE, program_code, Default::default(), ); diff --git a/ceno_zkvm/src/e2e.rs b/ceno_zkvm/src/e2e.rs index 0105e2344..0e60f32e2 100644 --- a/ceno_zkvm/src/e2e.rs +++ b/ceno_zkvm/src/e2e.rs @@ -201,8 +201,6 @@ pub fn setup_platform( }; Platform { - rom: program.base_address - ..program.base_address + (program.instructions.len() * WORD_SIZE) as u32, prog_data, stack, heap, diff --git a/ceno_zkvm/src/scheme/mock_prover.rs b/ceno_zkvm/src/scheme/mock_prover.rs index 037f8a130..e706c2880 100644 --- a/ceno_zkvm/src/scheme/mock_prover.rs +++ b/ceno_zkvm/src/scheme/mock_prover.rs @@ -17,7 +17,7 @@ use crate::{ }; use ark_std::test_rng; use base64::{Engine, engine::general_purpose::STANDARD_NO_PAD}; -use ceno_emul::{ByteAddr, CENO_PLATFORM, Platform, Program}; +use ceno_emul::{ByteAddr, CENO_PLATFORM, MOCK_BASE, MOCK_ENTRY_POINT, Program}; use ff::Field; use ff_ext::ExtensionField; use generic_static::StaticTypeMap; @@ -38,19 +38,7 @@ use strum::IntoEnumIterator; const MAX_CONSTRAINT_DEGREE: usize = 2; const MOCK_PROGRAM_SIZE: usize = 32; -pub const MOCK_PC_START: ByteAddr = ByteAddr({ - // This needs to be a static, because otherwise the compiler complains - // that 'the destructor for [Platform] cannot be evaluated in constants' - // The `static` keyword means that we keep exactly one copy of the variable - // around per process, and never deallocate it. Thus never having to call - // the destructor. - // - // At least conceptually. In practice with anything beyond -O0, the optimizer - // will inline and fold constants and replace `MOCK_PC_START` with - // a simple number. - static CENO_PLATFORM: Platform = ceno_emul::CENO_PLATFORM; - CENO_PLATFORM.pc_base() -}); +pub const MOCK_PC_START: ByteAddr = ByteAddr(MOCK_BASE); #[allow(clippy::enum_variant_names)] #[derive(Debug, Clone)] @@ -427,8 +415,8 @@ impl<'a, E: ExtensionField + Hash> MockProver { lkm: Option, ) -> Result<(), Vec>> { let program = Program::new( - CENO_PLATFORM.pc_base(), - CENO_PLATFORM.pc_base(), + MOCK_ENTRY_POINT, + MOCK_BASE, input_programs.to_vec(), Default::default(), ); diff --git a/ceno_zkvm/src/scheme/tests.rs b/ceno_zkvm/src/scheme/tests.rs index 8a97ff437..d58141dee 100644 --- a/ceno_zkvm/src/scheme/tests.rs +++ b/ceno_zkvm/src/scheme/tests.rs @@ -4,7 +4,7 @@ use ark_std::test_rng; use ceno_emul::{ CENO_PLATFORM, InsnKind::{ADD, ECALL}, - Platform, Program, StepRecord, VMState, encode_rv32, + MOCK_BASE, MOCK_ENTRY_POINT, Platform, Program, StepRecord, VMState, encode_rv32, }; use ff::Field; use ff_ext::ExtensionField; @@ -201,8 +201,8 @@ fn test_single_add_instance_e2e() { // set up program let program = Program::new( - CENO_PLATFORM.pc_base(), - CENO_PLATFORM.pc_base(), + MOCK_ENTRY_POINT, + MOCK_BASE, PROGRAM_CODE.to_vec(), Default::default(), );