From 5ee26a5dd1af3e172610b94b2657af709b319dad Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 5 Jun 2023 22:28:14 +0200 Subject: [PATCH 1/6] rp/dma: fix use-after-free read. --- embassy-rp/src/dma.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 74f4e6998..1cbb4651a 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -75,16 +75,17 @@ pub unsafe fn write<'a, C: Channel, W: Word>( ) } +static DUMMY: u32 = 0; + pub unsafe fn write_repeated<'a, C: Channel, W: Word>( ch: impl Peripheral

+ 'a, to: *mut W, len: usize, dreq: u8, ) -> Transfer<'a, C> { - let dummy: u32 = 0; copy_inner( ch, - &dummy as *const u32, + &DUMMY as *const u32, to as *mut u32, len, W::size(), From adf053a935d03711b48859e3243e86d3038fa7c4 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 5 Jun 2023 22:54:25 +0200 Subject: [PATCH 2/6] rp/flash: unify FLASH_BASE const. --- embassy-rp/src/flash.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/embassy-rp/src/flash.rs b/embassy-rp/src/flash.rs index 0410429e0..838b987dc 100644 --- a/embassy-rp/src/flash.rs +++ b/embassy-rp/src/flash.rs @@ -9,7 +9,7 @@ use embedded_storage::nor_flash::{ use crate::pac; use crate::peripherals::FLASH; -pub const FLASH_BASE: usize = 0x10000000; +pub const FLASH_BASE: *const u32 = 0x10000000 as _; // **NOTE**: // @@ -63,8 +63,8 @@ impl<'d, T: Instance, const FLASH_SIZE: usize> Flash<'d, T, FLASH_SIZE> { pub fn read(&mut self, offset: u32, bytes: &mut [u8]) -> Result<(), Error> { trace!( "Reading from 0x{:x} to 0x{:x}", - FLASH_BASE + offset as usize, - FLASH_BASE + offset as usize + bytes.len() + FLASH_BASE as u32 + offset, + FLASH_BASE as u32 + offset + bytes.len() as u32 ); check_read(self, offset, bytes.len())?; @@ -242,6 +242,7 @@ impl<'d, T: Instance, const FLASH_SIZE: usize> NorFlash for Flash<'d, T, FLASH_S mod ram_helpers { use core::marker::PhantomData; + use super::*; use crate::rom_data; #[repr(C)] @@ -321,7 +322,7 @@ mod ram_helpers { pub unsafe fn flash_range_erase(addr: u32, len: u32, use_boot2: bool) { let mut boot2 = [0u32; 256 / 4]; let ptrs = if use_boot2 { - rom_data::memcpy44(&mut boot2 as *mut _, super::FLASH_BASE as *const _, 256); + rom_data::memcpy44(&mut boot2 as *mut _, FLASH_BASE, 256); flash_function_pointers_with_boot2(true, false, &boot2) } else { flash_function_pointers(true, false) @@ -351,7 +352,7 @@ mod ram_helpers { pub unsafe fn flash_range_erase_and_program(addr: u32, data: &[u8], use_boot2: bool) { let mut boot2 = [0u32; 256 / 4]; let ptrs = if use_boot2 { - rom_data::memcpy44(&mut boot2 as *mut _, super::FLASH_BASE as *const _, 256); + rom_data::memcpy44(&mut boot2 as *mut _, FLASH_BASE, 256); flash_function_pointers_with_boot2(true, true, &boot2) } else { flash_function_pointers(true, true) @@ -386,7 +387,7 @@ mod ram_helpers { pub unsafe fn flash_range_program(addr: u32, data: &[u8], use_boot2: bool) { let mut boot2 = [0u32; 256 / 4]; let ptrs = if use_boot2 { - rom_data::memcpy44(&mut boot2 as *mut _, super::FLASH_BASE as *const _, 256); + rom_data::memcpy44(&mut boot2 as *mut _, FLASH_BASE, 256); flash_function_pointers_with_boot2(false, true, &boot2) } else { flash_function_pointers(false, true) @@ -511,7 +512,7 @@ mod ram_helpers { pub unsafe fn flash_unique_id(out: &mut [u8], use_boot2: bool) { let mut boot2 = [0u32; 256 / 4]; let ptrs = if use_boot2 { - rom_data::memcpy44(&mut boot2 as *mut _, 0x10000000 as *const _, 256); + rom_data::memcpy44(&mut boot2 as *mut _, FLASH_BASE, 256); flash_function_pointers_with_boot2(false, false, &boot2) } else { flash_function_pointers(false, false) @@ -539,7 +540,7 @@ mod ram_helpers { pub unsafe fn flash_jedec_id(use_boot2: bool) -> u32 { let mut boot2 = [0u32; 256 / 4]; let ptrs = if use_boot2 { - rom_data::memcpy44(&mut boot2 as *mut _, 0x10000000 as *const _, 256); + rom_data::memcpy44(&mut boot2 as *mut _, FLASH_BASE, 256); flash_function_pointers_with_boot2(false, false, &boot2) } else { flash_function_pointers(false, false) From 70e1b976d87c5bde4b07e7e0b0fcf1499e4d98ef Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 5 Jun 2023 23:40:34 +0200 Subject: [PATCH 3/6] rp/flash: fix missing clobbers, do not clobber frame pointer (r7). --- embassy-rp/src/flash.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/embassy-rp/src/flash.rs b/embassy-rp/src/flash.rs index 838b987dc..cd34e605e 100644 --- a/embassy-rp/src/flash.rs +++ b/embassy-rp/src/flash.rs @@ -587,7 +587,6 @@ mod ram_helpers { "ldr r4, [r5, #4]", "blx r4", // flash_exit_xip() - "mov r7, r10", // cmd "movs r4, #0x18", "lsls r4, r4, #24", // 0x18000000, SSI, RP2040 datasheet 4.10.13 @@ -604,8 +603,9 @@ mod ram_helpers { "str r1, [r4, #0]", // Write ctrlr1 with len-1 - "ldr r0, [r7, #8]", // dummy_len - "ldr r1, [r7, #16]", // data_len + "mov r3, r10", // cmd + "ldr r0, [r3, #8]", // dummy_len + "ldr r1, [r3, #16]", // data_len "add r0, r1", "subs r0, #1", "str r0, [r4, #0x04]", // CTRLR1 @@ -617,8 +617,8 @@ mod ram_helpers { // Write cmd/addr phase to DR "mov r2, r4", "adds r2, 0x60", // &DR - "ldr r0, [r7, #0]", // cmd_addr - "ldr r1, [r7, #4]", // cmd_addr_len + "ldr r0, [r3, #0]", // cmd_addr + "ldr r1, [r3, #4]", // cmd_addr_len "10:", "ldrb r3, [r0]", "strb r3, [r2]", // DR @@ -627,7 +627,8 @@ mod ram_helpers { "bne 10b", // Skip any dummy cycles - "ldr r1, [r7, #8]", // dummy_len + "mov r3, r10", // cmd + "ldr r1, [r3, #8]", // dummy_len "cmp r1, #0", "beq 9f", "4:", @@ -644,8 +645,9 @@ mod ram_helpers { // Read RX fifo "9:", - "ldr r0, [r7, #12]", // data - "ldr r1, [r7, #16]", // data_len + "mov r2, r10", // cmd + "ldr r0, [r2, #12]", // data + "ldr r1, [r2, #16]", // data_len "2:", "ldr r3, [r4, #0x28]", // SR @@ -679,13 +681,12 @@ mod ram_helpers { out("r2") _, out("r3") _, out("r4") _, + out("r5") _, // Registers r8-r10 are used to store values // from r0-r2 in registers not clobbered by // function calls. // The values can't be passed in using r8-r10 directly // due to https://github.com/rust-lang/rust/issues/99071 - out("r8") _, - out("r9") _, out("r10") _, clobber_abi("C"), ); From 162d48530436be9cfa6a0d4aba88d97f52a5ad4a Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 5 Jun 2023 23:41:26 +0200 Subject: [PATCH 4/6] rp/flash: centralize `USE_BOOT2` in a single const. --- embassy-rp/src/flash.rs | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/embassy-rp/src/flash.rs b/embassy-rp/src/flash.rs index cd34e605e..0372afb1e 100644 --- a/embassy-rp/src/flash.rs +++ b/embassy-rp/src/flash.rs @@ -10,6 +10,7 @@ use crate::pac; use crate::peripherals::FLASH; pub const FLASH_BASE: *const u32 = 0x10000000 as _; +pub const USE_BOOT2: bool = true; // **NOTE**: // @@ -89,7 +90,7 @@ impl<'d, T: Instance, const FLASH_SIZE: usize> Flash<'d, T, FLASH_SIZE> { let len = to - from; - unsafe { self.in_ram(|| ram_helpers::flash_range_erase(from, len, true))? }; + unsafe { self.in_ram(|| ram_helpers::flash_range_erase(from, len))? }; Ok(()) } @@ -114,7 +115,7 @@ impl<'d, T: Instance, const FLASH_SIZE: usize> Flash<'d, T, FLASH_SIZE> { let unaligned_offset = offset as usize - start; - unsafe { self.in_ram(|| ram_helpers::flash_range_program(unaligned_offset as u32, &pad_buf, true))? } + unsafe { self.in_ram(|| ram_helpers::flash_range_program(unaligned_offset as u32, &pad_buf))? } } let remaining_len = bytes.len() - start_padding; @@ -132,12 +133,12 @@ impl<'d, T: Instance, const FLASH_SIZE: usize> Flash<'d, T, FLASH_SIZE> { if bytes.as_ptr() as usize >= 0x2000_0000 { let aligned_data = &bytes[start_padding..end_padding]; - unsafe { self.in_ram(|| ram_helpers::flash_range_program(aligned_offset as u32, aligned_data, true))? } + unsafe { self.in_ram(|| ram_helpers::flash_range_program(aligned_offset as u32, aligned_data))? } } else { for chunk in bytes[start_padding..end_padding].chunks_exact(PAGE_SIZE) { let mut ram_buf = [0xFF_u8; PAGE_SIZE]; ram_buf.copy_from_slice(chunk); - unsafe { self.in_ram(|| ram_helpers::flash_range_program(aligned_offset as u32, &ram_buf, true))? } + unsafe { self.in_ram(|| ram_helpers::flash_range_program(aligned_offset as u32, &ram_buf))? } aligned_offset += PAGE_SIZE; } } @@ -152,7 +153,7 @@ impl<'d, T: Instance, const FLASH_SIZE: usize> Flash<'d, T, FLASH_SIZE> { let unaligned_offset = end_offset - (PAGE_SIZE - rem_offset); - unsafe { self.in_ram(|| ram_helpers::flash_range_program(unaligned_offset as u32, &pad_buf, true))? } + unsafe { self.in_ram(|| ram_helpers::flash_range_program(unaligned_offset as u32, &pad_buf))? } } Ok(()) @@ -190,7 +191,7 @@ impl<'d, T: Instance, const FLASH_SIZE: usize> Flash<'d, T, FLASH_SIZE> { /// Read SPI flash unique ID pub fn unique_id(&mut self, uid: &mut [u8]) -> Result<(), Error> { - unsafe { self.in_ram(|| ram_helpers::flash_unique_id(uid, true))? }; + unsafe { self.in_ram(|| ram_helpers::flash_unique_id(uid))? }; Ok(()) } @@ -199,7 +200,7 @@ impl<'d, T: Instance, const FLASH_SIZE: usize> Flash<'d, T, FLASH_SIZE> { let mut jedec = None; unsafe { self.in_ram(|| { - jedec.replace(ram_helpers::flash_jedec_id(true)); + jedec.replace(ram_helpers::flash_jedec_id()); })?; }; Ok(jedec.unwrap()) @@ -307,7 +308,7 @@ mod ram_helpers { /// /// `addr` and `len` must be multiples of 4096 /// - /// If `use_boot2` is `true`, a copy of the 2nd stage boot loader + /// If `USE_BOOT2` is `true`, a copy of the 2nd stage boot loader /// is used to re-initialize the XIP engine after flashing. /// /// # Safety @@ -319,9 +320,9 @@ mod ram_helpers { /// - DMA must not access flash memory /// /// `addr` and `len` parameters must be valid and are not checked. - pub unsafe fn flash_range_erase(addr: u32, len: u32, use_boot2: bool) { + pub unsafe fn flash_range_erase(addr: u32, len: u32) { let mut boot2 = [0u32; 256 / 4]; - let ptrs = if use_boot2 { + let ptrs = if USE_BOOT2 { rom_data::memcpy44(&mut boot2 as *mut _, FLASH_BASE, 256); flash_function_pointers_with_boot2(true, false, &boot2) } else { @@ -337,7 +338,7 @@ mod ram_helpers { /// /// `addr` and `data.len()` must be multiples of 4096 /// - /// If `use_boot2` is `true`, a copy of the 2nd stage boot loader + /// If `USE_BOOT2` is `true`, a copy of the 2nd stage boot loader /// is used to re-initialize the XIP engine after flashing. /// /// # Safety @@ -349,9 +350,9 @@ mod ram_helpers { /// - DMA must not access flash memory /// /// `addr` and `len` parameters must be valid and are not checked. - pub unsafe fn flash_range_erase_and_program(addr: u32, data: &[u8], use_boot2: bool) { + pub unsafe fn flash_range_erase_and_program(addr: u32, data: &[u8]) { let mut boot2 = [0u32; 256 / 4]; - let ptrs = if use_boot2 { + let ptrs = if USE_BOOT2 { rom_data::memcpy44(&mut boot2 as *mut _, FLASH_BASE, 256); flash_function_pointers_with_boot2(true, true, &boot2) } else { @@ -372,7 +373,7 @@ mod ram_helpers { /// /// `addr` and `data.len()` must be multiples of 256 /// - /// If `use_boot2` is `true`, a copy of the 2nd stage boot loader + /// If `USE_BOOT2` is `true`, a copy of the 2nd stage boot loader /// is used to re-initialize the XIP engine after flashing. /// /// # Safety @@ -384,9 +385,9 @@ mod ram_helpers { /// - DMA must not access flash memory /// /// `addr` and `len` parameters must be valid and are not checked. - pub unsafe fn flash_range_program(addr: u32, data: &[u8], use_boot2: bool) { + pub unsafe fn flash_range_program(addr: u32, data: &[u8]) { let mut boot2 = [0u32; 256 / 4]; - let ptrs = if use_boot2 { + let ptrs = if USE_BOOT2 { rom_data::memcpy44(&mut boot2 as *mut _, FLASH_BASE, 256); flash_function_pointers_with_boot2(false, true, &boot2) } else { @@ -509,9 +510,9 @@ mod ram_helpers { /// - DMA must not access flash memory /// /// Credit: taken from `rp2040-flash` (also licensed Apache+MIT) - pub unsafe fn flash_unique_id(out: &mut [u8], use_boot2: bool) { + pub unsafe fn flash_unique_id(out: &mut [u8]) { let mut boot2 = [0u32; 256 / 4]; - let ptrs = if use_boot2 { + let ptrs = if USE_BOOT2 { rom_data::memcpy44(&mut boot2 as *mut _, FLASH_BASE, 256); flash_function_pointers_with_boot2(false, false, &boot2) } else { @@ -537,9 +538,9 @@ mod ram_helpers { /// - DMA must not access flash memory /// /// Credit: taken from `rp2040-flash` (also licensed Apache+MIT) - pub unsafe fn flash_jedec_id(use_boot2: bool) -> u32 { + pub unsafe fn flash_jedec_id() -> u32 { let mut boot2 = [0u32; 256 / 4]; - let ptrs = if use_boot2 { + let ptrs = if USE_BOOT2 { rom_data::memcpy44(&mut boot2 as *mut _, FLASH_BASE, 256); flash_function_pointers_with_boot2(false, false, &boot2) } else { From 4f03dff577f08bb9af1ea3fc118857973baa686e Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 6 Jun 2023 00:06:32 +0200 Subject: [PATCH 5/6] rp: add run-from-ram feature. --- embassy-rp/Cargo.toml | 5 +++++ embassy-rp/src/flash.rs | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/embassy-rp/Cargo.toml b/embassy-rp/Cargo.toml index 5f08c7f33..ddada655b 100644 --- a/embassy-rp/Cargo.toml +++ b/embassy-rp/Cargo.toml @@ -41,6 +41,11 @@ boot2-ram-memcpy = [] boot2-w25q080 = [] boot2-w25x10cl = [] +# Indicate code is running from RAM. +# Set this if all code is in RAM, and the cores never access memory-mapped flash memory through XIP. +# This allows the flash driver to not force pausing execution on both cores when doing flash operations. +run-from-ram = [] + # Enable nightly-only features nightly = ["embassy-executor/nightly", "embedded-hal-1", "embedded-hal-async", "embassy-embedded-hal/nightly", "dep:embassy-usb-driver", "dep:embedded-io"] diff --git a/embassy-rp/src/flash.rs b/embassy-rp/src/flash.rs index 0372afb1e..5d928abad 100644 --- a/embassy-rp/src/flash.rs +++ b/embassy-rp/src/flash.rs @@ -10,7 +10,10 @@ use crate::pac; use crate::peripherals::FLASH; pub const FLASH_BASE: *const u32 = 0x10000000 as _; -pub const USE_BOOT2: bool = true; + +// If running from RAM, we might have no boot2. Use bootrom `flash_enter_cmd_xip` instead. +// TODO: when run-from-ram is set, completely skip the "pause cores and jumpp to RAM" dance. +pub const USE_BOOT2: bool = !cfg!(feature = "run-from-ram"); // **NOTE**: // From 593fc78dd892338386a2a81400e0621372264053 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 6 Jun 2023 00:07:03 +0200 Subject: [PATCH 6/6] tests/rp: enable run-from-ram. Otherwise the flash test is flaky because it attempts to use boot2. --- tests/rp/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rp/Cargo.toml b/tests/rp/Cargo.toml index 1786baee3..6bcac373c 100644 --- a/tests/rp/Cargo.toml +++ b/tests/rp/Cargo.toml @@ -10,7 +10,7 @@ teleprobe-meta = "1" embassy-sync = { version = "0.2.0", path = "../../embassy-sync", features = ["defmt"] } embassy-executor = { version = "0.2.0", path = "../../embassy-executor", features = ["arch-cortex-m", "executor-thread", "defmt", "integrated-timers"] } embassy-time = { version = "0.1.0", path = "../../embassy-time", features = ["defmt"] } -embassy-rp = { version = "0.1.0", path = "../../embassy-rp", features = ["nightly", "defmt", "unstable-pac", "unstable-traits", "time-driver", "critical-section-impl", "intrinsics", "rom-v2-intrinsics"] } +embassy-rp = { version = "0.1.0", path = "../../embassy-rp", features = ["nightly", "defmt", "unstable-pac", "unstable-traits", "time-driver", "critical-section-impl", "intrinsics", "rom-v2-intrinsics", "run-from-ram"] } embassy-futures = { version = "0.1.0", path = "../../embassy-futures" } defmt = "0.3.0"