Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ROM from platform #773

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ceno_emul/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
27 changes: 5 additions & 22 deletions ceno_emul/src/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use crate::addr::{Addr, RegIdx};
/// - codes of environment calls.
#[derive(Clone, Debug)]
pub struct Platform {
pub rom: Range<Addr>,
pub prog_data: BTreeSet<Addr>,
pub stack: Range<Addr>,
pub heap: Range<Addr>,
Expand All @@ -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,
Expand All @@ -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))
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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));
}
}
Expand Down
15 changes: 5 additions & 10 deletions ceno_emul/tests/test_vm_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
);
Expand All @@ -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")),);
Expand Down
8 changes: 4 additions & 4 deletions ceno_zkvm/examples/riscv_opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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(),
);
Expand Down
2 changes: 0 additions & 2 deletions ceno_zkvm/src/e2e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 4 additions & 16 deletions ceno_zkvm/src/scheme/mock_prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)]
Expand Down Expand Up @@ -427,8 +415,8 @@ impl<'a, E: ExtensionField + Hash> MockProver<E> {
lkm: Option<LkMultiplicity>,
) -> Result<(), Vec<MockProverError<E>>> {
let program = Program::new(
CENO_PLATFORM.pc_base(),
CENO_PLATFORM.pc_base(),
MOCK_ENTRY_POINT,
MOCK_BASE,
input_programs.to_vec(),
Default::default(),
);
Expand Down
6 changes: 3 additions & 3 deletions ceno_zkvm/src/scheme/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
);
Expand Down