From d7643981831fc12248de2dc46e60e685b4c3c8b1 Mon Sep 17 00:00:00 2001 From: Emma Turner Date: Mon, 11 Nov 2024 11:54:24 +0000 Subject: [PATCH 1/3] RISC-V: require 'static bound on MainMemoryLayout This is required to make `ICall` static, as required by enriched cell --- src/riscv/lib/src/machine_state/bus/main_memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/riscv/lib/src/machine_state/bus/main_memory.rs b/src/riscv/lib/src/machine_state/bus/main_memory.rs index c04e2baf2ec6..d67c24ea9bd4 100644 --- a/src/riscv/lib/src/machine_state/bus/main_memory.rs +++ b/src/riscv/lib/src/machine_state/bus/main_memory.rs @@ -68,7 +68,7 @@ gen_memory_layout!(M4G = 4 GiB); // XXX: We can't associate these region types directly with [Sizes] because // inherent associated types are unstable. Hence we must go through a dummy // trait. -pub trait MainMemoryLayout: backend::Layout { +pub trait MainMemoryLayout: backend::Layout + 'static { type Data; const BYTES: usize; -- GitLab From 10bb98999c88836b99c4173cfd33974719a3720d Mon Sep 17 00:00:00 2001 From: Emma Turner Date: Wed, 27 Nov 2024 09:57:32 +0000 Subject: [PATCH 2/3] RISC-V: require Sized bound on ManagerBase Any manager of the state backend storage is expected to be `Sized`. If there is ever a case where this doesn't hold, the implementations of the `Manager*` traits should be over boxed variants. --- src/riscv/lib/src/state_backend.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/riscv/lib/src/state_backend.rs b/src/riscv/lib/src/state_backend.rs index 8634ea7d183b..1e7056a8c980 100644 --- a/src/riscv/lib/src/state_backend.rs +++ b/src/riscv/lib/src/state_backend.rs @@ -76,12 +76,12 @@ pub use region::*; /// This derived value does not form part of any stored state/commitments. pub trait EnrichedValue: 'static { type E: 'static; - type D: Sized; + type D; } /// Specifies that there exists a path to derive `V::D` from `&V::E`, /// for a given manager. -pub trait EnrichedValueLinked: EnrichedValue { +pub trait EnrichedValueLinked: EnrichedValue { /// Construct the derived value from the stored value, maps to /// the `From` trait by default. fn derive(v: &Self::E) -> Self::D; @@ -98,7 +98,7 @@ where } /// Manager of the state backend storage -pub trait ManagerBase { +pub trait ManagerBase: Sized { /// Region that has been allocated in the state storage type Region; -- GitLab From 0a35a2845c76409f318b162a054f626045587a76 Mon Sep 17 00:00:00 2001 From: Emma Turner Date: Mon, 11 Nov 2024 11:02:17 +0000 Subject: [PATCH 3/3] RISC-V: dispatch by opcode at block construction RISC-V: blocks --- src/riscv/lib/src/machine_state.rs | 4 +- .../lib/src/machine_state/block_cache.rs | 119 +++++++++++++----- src/riscv/lib/src/pvm/common.rs | 2 +- .../lib/src/state_backend/owned_backend.rs | 36 ++++++ .../lib/tests/expected/jstz/state_hash_final | 2 +- .../tests/expected/jstz/state_hash_initial | 2 +- 6 files changed, 131 insertions(+), 34 deletions(-) diff --git a/src/riscv/lib/src/machine_state.rs b/src/riscv/lib/src/machine_state.rs index a4ca049212be..f716aa59c2fb 100644 --- a/src/riscv/lib/src/machine_state.rs +++ b/src/riscv/lib/src/machine_state.rs @@ -223,7 +223,7 @@ impl MachineCoreStat /// Reset the machine state. pub fn reset(&mut self) where - M: backend::ManagerWrite, + M: backend::ManagerReadWrite, { self.hart.reset(main_memory::FIRST_ADDRESS); self.main_memory.reset(); @@ -268,7 +268,7 @@ impl(PhantomData); +/// Layout for an [`ICallPlaced`]. +pub struct ICallLayout { + _pd: PhantomData, +} + +impl crate::state_backend::Layout for ICallLayout { + type Allocated = EnrichedCell, M>; + + fn allocate(backend: &mut M) -> Self::Allocated { + let value = backend.allocate_enriched_cell(Instruction::DEFAULT); + EnrichedCell::bind(value) + } +} + +/// Bindings for deriving an [`ICall`] from an [`Instruction`] via the [`EnrichedCell`] mechanism. +pub struct ICallPlaced { + _pd: PhantomData, +} + +impl EnrichedValue for ICallPlaced { + type E = Instruction; + + type D = ICall; +} + +/// A function derived from an [`OpCode`] that can be directly run over the [`MachineCoreState`]. +/// +/// This allows static dispatch of this function during block construction, +/// rather than for each instruction, during each block execution. +pub struct ICall { + run_instr: fn(&Args, &mut MachineCoreState) -> Result, +} + +impl Clone for ICall { + fn clone(&self) -> Self { + *self + } +} -impl state_backend::Layout for MLCapture { - type Allocated = (); +impl Copy for ICall {} - fn allocate(_backend: &mut M) -> Self::Allocated {} +impl ICall { + #[inline(always)] + fn run( + &self, + args: &Args, + core: &mut MachineCoreState, + ) -> Result { + (self.run_instr)(args, core) + } +} + +impl<'a, ML: MainMemoryLayout, M: ManagerReadWrite> From<&'a Instruction> for ICall { + fn from(value: &'a Instruction) -> Self { + let run_instr = value.opcode.to_run::(); + Self { run_instr } + } } /// Layout for an address cell @@ -120,8 +176,7 @@ pub type CachedLayout = ( AddressCellLayout, Atom, Atom, - [Atom; CACHE_INSTR], - MLCapture, + [ICallLayout; CACHE_INSTR], ); /// Block cache entry. @@ -132,8 +187,7 @@ pub struct Cached { address: Cell, fence_counter: Cell, len_instr: Cell, - instr: [Cell; CACHE_INSTR], - _pd: PhantomData, + instr: [EnrichedCell, M>; CACHE_INSTR], } impl Cached { @@ -143,7 +197,6 @@ impl Cached { fence_counter: space.1, len_instr: space.2, instr: space.3, - _pd: PhantomData, } } @@ -157,7 +210,7 @@ impl Cached { fn reset(&mut self) where - M: ManagerWrite, + M: ManagerReadWrite, { self.address.write(!0); self.fence_counter.write(FenceCounter::INITIAL); @@ -182,7 +235,6 @@ impl Cached { { Block { instr: &mut self.instr[..self.len_instr.read() as usize], - _pd: PhantomData, } } @@ -191,8 +243,7 @@ impl Cached { self.address.struct_ref(), self.fence_counter.struct_ref(), self.len_instr.struct_ref(), - self.instr.each_ref().map(Cell::struct_ref), - (), + self.instr.each_ref().map(EnrichedCell::struct_ref), ) } } @@ -204,7 +255,6 @@ impl Clone for Cached { fence_counter: self.fence_counter.clone(), len_instr: self.len_instr.clone(), instr: self.instr.clone(), - _pd: PhantomData, } } } @@ -298,7 +348,7 @@ pub trait BlockCacheLayout: state_backend::Layout { phys_addr: Address, ) -> &mut Cached; - fn entries_reset(entries: &mut Self::Entries); + fn entries_reset(entries: &mut Self::Entries); fn struct_ref( cache: &BlockCache, @@ -357,7 +407,7 @@ impl BlockCacheLayou &mut entries[Self::Sizes::cache_index(phys_addr)] } - fn entries_reset(entries: &mut Self::Entries) { + fn entries_reset(entries: &mut Self::Entries) { entries.iter_mut().for_each(Cached::reset) } @@ -419,7 +469,7 @@ impl, ML: MainMemoryLayout, M: Mana /// Reset the underlying storage. pub fn reset(&mut self) where - M: ManagerWrite, + M: ManagerReadWrite, { self.fence_counter.write(FenceCounter::INITIAL); self.reset_to(!0); @@ -515,7 +565,7 @@ impl, ML: MainMemoryLayout, M: Mana // Need to resolve the adjacent block again because we may only keep one reference at a time // to `self.entries`. let new_block = BCL::entry(&self.entries, next_phys_addr); - let new_instr = new_block.instr[i].read(); + let new_instr = new_block.instr[i].read_stored(); // Need to resolve the target block again because we may only keep one reference at a time // to `self.entries`. let current_entry = BCL::entry_mut(&mut self.entries, block_addr); @@ -620,8 +670,8 @@ impl, ML: MainMemoryLayout, M: Mana let mut instr_pc = core.hart.pc.read(); let range = progress as usize..entry.len_instr.read() as usize; - for i in entry.instr[range].iter() { - match i.as_ref().run(core) { + for instr in entry.instr[range].iter() { + match run_instr(instr, core) { Ok(ProgramCounterUpdate::Next(width)) => { instr_pc += width as u64; core.hart.pc.write(instr_pc); @@ -681,8 +731,7 @@ impl, ML: MainMemoryLayout, M: Mana /// A block fetched from the block cache, that can be executed against the machine state. pub struct Block<'a, ML: MainMemoryLayout, M: ManagerBase> { - instr: &'a mut [Cell], - _pd: PhantomData, + instr: &'a mut [EnrichedCell, M>], } impl<'a, ML: MainMemoryLayout, M: ManagerRead> Block<'a, ML, M> { @@ -725,8 +774,8 @@ impl<'a, ML: MainMemoryLayout, M: ManagerRead> Block<'a, ML, M> { where M: ManagerReadWrite, { - for i in self.instr.iter() { - match i.as_ref().run(core) { + for instr in self.instr.iter() { + match run_instr(instr, core) { Ok(ProgramCounterUpdate::Next(width)) => { *instr_pc += width as u64; core.hart.pc.write(*instr_pc); @@ -756,10 +805,22 @@ impl<'a, ML: MainMemoryLayout, M: ManagerRead> Block<'a, ML, M> { where M: ManagerRead, { - self.instr.iter().map(|cell| cell.read()).collect() + self.instr.iter().map(|cell| cell.read_stored()).collect() } } +#[inline(always)] +fn run_instr( + instr: &EnrichedCell, M>, + core: &mut MachineCoreState, +) -> Result { + let instr = instr.read_ref(); + let args = &instr.0.args; + let icall = &instr.1; + + icall.run(args, core) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/riscv/lib/src/pvm/common.rs b/src/riscv/lib/src/pvm/common.rs index 768b8dc6e01f..9a8be605287e 100644 --- a/src/riscv/lib/src/pvm/common.rs +++ b/src/riscv/lib/src/pvm/common.rs @@ -136,7 +136,7 @@ impl< /// Reset the PVM state. pub fn reset(&mut self) where - M: state_backend::ManagerWrite, + M: state_backend::ManagerReadWrite, { self.version.write(INITIAL_VERSION); self.machine_state.reset(); diff --git a/src/riscv/lib/src/state_backend/owned_backend.rs b/src/riscv/lib/src/state_backend/owned_backend.rs index be955101c19c..5838853428f7 100644 --- a/src/riscv/lib/src/state_backend/owned_backend.rs +++ b/src/riscv/lib/src/state_backend/owned_backend.rs @@ -462,6 +462,42 @@ pub mod test_helpers { }); } + /// Ensure [`EnrichedCell`] is serialized identically to [`Cell`]. + #[test] + fn enriched_cell_serialise_match_cell() { + use crate::state_backend::hash::RootHashable; + + pub struct Enriching; + pub struct Fun; + + impl EnrichedValue for Enriching { + type E = u64; + type D = Fun; + } + + impl<'a> From<&'a u64> for Fun { + fn from(_value: &'a u64) -> Self { + Self + } + } + + proptest::proptest!(|(value: u64)| { + let mut ecell: EnrichedCell = EnrichedCell::bind((0u64, Fun::from(&0))); + let mut cell: Cell = Cell::bind([0; 1]); + ecell.write(value); + cell.write(value); + + assert_eq!(value, ecell.read_stored()); + assert_eq!(value, cell.read()); + + let ebytes = bincode::serialize(&ecell).unwrap(); + let cbytes = bincode::serialize(&cell).unwrap(); + + assert_eq!(ebytes, cbytes, "Serializing EnrichedCell and Cell should match"); + assert_eq!(ecell.hash().unwrap(), cell.hash().unwrap(), "RootHashable for EnrichedCell and Cell should match"); + }); + } + /// Ensure that [`Cell`] serialises in a way that represents the underlying element /// directly instead of wrapping it into an array (as it is an array under the hood). #[test] diff --git a/src/riscv/lib/tests/expected/jstz/state_hash_final b/src/riscv/lib/tests/expected/jstz/state_hash_final index d756caf6e769..87dd6961bc99 100644 --- a/src/riscv/lib/tests/expected/jstz/state_hash_final +++ b/src/riscv/lib/tests/expected/jstz/state_hash_final @@ -1 +1 @@ -Hash { digest: [162, 233, 67, 233, 200, 16, 124, 72, 204, 141, 78, 76, 169, 209, 24, 173, 59, 237, 176, 30, 62, 139, 165, 225, 39, 132, 110, 254, 141, 127, 179, 16] } +Hash { digest: [99, 156, 99, 147, 177, 11, 123, 91, 82, 58, 185, 119, 148, 177, 78, 170, 187, 36, 153, 68, 202, 190, 77, 57, 161, 172, 214, 210, 51, 229, 50, 130] } diff --git a/src/riscv/lib/tests/expected/jstz/state_hash_initial b/src/riscv/lib/tests/expected/jstz/state_hash_initial index fab951770a92..47b4d469fe52 100644 --- a/src/riscv/lib/tests/expected/jstz/state_hash_initial +++ b/src/riscv/lib/tests/expected/jstz/state_hash_initial @@ -1 +1 @@ -Hash { digest: [158, 84, 209, 243, 58, 177, 204, 20, 232, 32, 77, 125, 115, 233, 140, 127, 13, 207, 151, 54, 72, 125, 193, 45, 189, 26, 245, 202, 113, 186, 56, 212] } +Hash { digest: [121, 50, 230, 88, 91, 149, 241, 176, 130, 8, 84, 207, 114, 140, 195, 135, 225, 78, 89, 111, 79, 212, 0, 123, 66, 28, 3, 80, 149, 9, 119, 53] } -- GitLab