Timer wraparound fix; app RAM address calculation (#47)

Fix several bugs related to alarms not firing in applications:

* Emulated internal timers were not handling wraparound correctly when
  checking how long to wait for the next interrupt. If the time had
  already passed for the next interrupt, then the function would
  underflow and return a value near u32::MAX, so the interrupt would be
  scheduled for the distant future instead of firing immediately.
* Fix alarm example code to sleep the correct amount of time.
* Application RAM was not being put in the correct location in the
  linker script. This caused issues with sharing memory with the kernel,
  as it was in the wrong region of RAM, which manifested as errors when
  printing to the console.
* Remove weird BSS workaround in the runtime linking, as we would only
  need it for certain ARM platforms.
* Add low-level debug capsule to the kernel so that application panics
  are handled correctly

emulator/cpu/src/internal_timers.rs

@@ -96,8 +96,8 @@ impl InternalTimer {
         self.read_time(now) >= self.bound
     }
 
-    fn ticks_to_interrupt(&self, now: u64) -> u32 {
-        self.bound.wrapping_sub(self.read_time(now))
+    pub(crate) fn ticks_to_interrupt(&self, now: u64) -> u32 {
+        self.bound.saturating_sub(self.read_time(now)).max(1)
     }
 
     fn arm(&mut self, timer: &Timer, now: u64) {
@@ -195,3 +195,29 @@ impl InternalTimers {
         )
     }
 }
+
+#[cfg(test)]
+mod test {
+    use std::rc::Rc;
+
+    use super::InternalTimer;
+    use emulator_bus::Clock;
+
+    #[test]
+    fn test_ticks_to_interrupt() {
+        let clock = Rc::new(Clock::new());
+        let t = clock.timer();
+        let mut itimer = InternalTimer::new(0);
+        itimer.write_bound(300, &t, 100);
+        assert_eq!(200, itimer.ticks_to_interrupt(100));
+    }
+
+    #[test]
+    fn test_ticks_to_interrupt_underflow() {
+        let clock = Rc::new(Clock::new());
+        let t = clock.timer();
+        let mut itimer = InternalTimer::new(0);
+        itimer.write_bound(300, &t, 100);
+        assert_eq!(1, itimer.ticks_to_interrupt(400));
+    }
+}

runtime/apps/pldm/src/riscv.rs

@@ -71,6 +71,12 @@ fn init(spawner: Spawner) {
 async fn async_main() {
     let mut console_writer = Console::writer();
     writeln!(console_writer, "Hello async world!").unwrap();
+    writeln!(
+        console_writer,
+        "Timer frequency: {}",
+        AsyncAlarm::<TockSyscalls>::get_frequency().unwrap().0
+    )
+    .unwrap();
 
     match AsyncAlarm::<TockSyscalls>::exists() {
         Ok(()) => {}
@@ -86,8 +92,8 @@ async fn async_main() {
     };
 
     for _ in 0..5 {
-        writeln!(console_writer, "Sleeping for 10 millisecond").unwrap();
-        sleep(Milliseconds(10)).await;
+        writeln!(console_writer, "Sleeping for 1 millisecond").unwrap();
+        sleep(Milliseconds(1)).await;
         writeln!(console_writer, "async sleeper woke").unwrap();
     }
     writeln!(console_writer, "app finished").unwrap();
@@ -150,12 +156,12 @@ impl<S: Syscalls, C: platform::subscribe::Config> AsyncAlarm<S, C> {
         Ok(ticks.saturating_div(freq / 1000))
     }
 
-    pub async fn sleep_for<T: Convert>(_time: T) -> Result<(), ErrorCode> {
-        // TODO: this seems to never return, so we just sleep for 1 tick
-        // let freq = Self::get_frequency()?;
-        // let ticks = time.to_ticks(freq);
+    pub async fn sleep_for<T: Convert>(time: T) -> Result<(), ErrorCode> {
+        let freq = Self::get_frequency()?;
+        let ticks = time.to_ticks(freq).0;
+        writeln!(Console::writer(), "Sleeping for {} ticks", ticks).unwrap();
         let sub = TockSubscribe::subscribe::<S>(DRIVER_NUM, 0);
-        S::command(DRIVER_NUM, command::SET_RELATIVE, 1, 0)
+        S::command(DRIVER_NUM, command::SET_RELATIVE, ticks, 0)
             .to_result()
             .map(|_when: u32| ())?;
         sub.await.map(|_| ())

runtime/kernel_layout.ld

@@ -235,18 +235,6 @@ SECTIONS
         . = ALIGN(4);
         _sapps = .;
 
-        /* Include placeholder bytes in this section so that the linker
-         * includes a segment for it. Otherwise the section will be empty and
-         * the linker will ignore it when defining the segments.
-         * If less then 4 bytes, some linkers set this section to size 0
-         * and openocd fails to write it.
-         *
-         * An issue has been submitted https://github.com/raspberrypi/openocd/issues/25
-         */
-        BYTE(0xFF)
-        BYTE(0xFF)
-        BYTE(0xFF)
-        BYTE(0xFF)
     } > prog
     /* _eapps symbol used by tock to calculate the length of app flash */
     _eapps = _sapps + LENGTH(prog);
@@ -332,6 +320,7 @@ SECTIONS
          * future enhancement may allow the kernel to parcel this memory space
          * dynamically, requiring changes to this section.
          */
+         . = ALIGN(4096); /* align to page boundary due to lld limitation */
         _sappmem = .;
         *(.app_memory)
     } > ram
@@ -423,5 +412,5 @@ _erom = ORIGIN(rom) + LENGTH(rom);
 
 /* This assert works out because even though some of the relative positions are
  * off, the sizes are sane in each pass. */
-ASSERT((_etext - _stext) + (_erelocate - _srelocate) + (_eattributes - _sattributes) < LENGTH(rom),
-"Text plus relocations plus attributes exceeds the available ROM space.");
+/*ASSERT((_etext - _stext) + (_erelocate - _srelocate) + (_eattributes - _sattributes) < LENGTH(rom),
+"Text plus relocations plus attributes exceeds the available ROM space.");*/

runtime/src/board.rs

@@ -48,6 +48,10 @@ struct VeeR {
         VirtualMuxAlarm<'static, InternalTimers<'static>>,
     >,
     console: &'static capsules_core::console::Console<'static>,
+    lldb: &'static capsules_core::low_level_debug::LowLevelDebug<
+        'static,
+        capsules_core::virtualizers::virtual_uart::UartDevice<'static>,
+    >,
     scheduler: &'static CooperativeSched<'static>,
     scheduler_timer:
         &'static VirtualSchedulerTimer<VirtualMuxAlarm<'static, InternalTimers<'static>>>,
@@ -62,6 +66,7 @@ impl SyscallDriverLookup for VeeR {
         match driver_num {
             capsules_core::alarm::DRIVER_NUM => f(Some(self.alarm)),
             capsules_core::console::DRIVER_NUM => f(Some(self.console)),
+            capsules_core::low_level_debug::DRIVER_NUM => f(Some(self.lldb)),
             _ => f(None),
         }
     }
@@ -162,6 +167,13 @@ pub unsafe fn main() {
     components::debug_writer::DebugWriterComponent::new(uart_mux)
         .finalize(components::debug_writer_component_static!());
 
+    let lldb = components::lldb::LowLevelDebugComponent::new(
+        board_kernel,
+        capsules_core::low_level_debug::DRIVER_NUM,
+        uart_mux,
+    )
+    .finalize(components::low_level_debug_component_static!());
+
     // Setup the console.
     let console = components::console::ConsoleComponent::new(
         board_kernel,
@@ -226,6 +238,7 @@ pub unsafe fn main() {
     let veer = VeeR {
         alarm,
         console,
+        lldb,
         scheduler,
         scheduler_timer,
     };

runtime/src/timers.rs

@@ -6,7 +6,7 @@
 //! Create a timer using the VeeR EL2 Internal Timer registers.
 
 use core::cell::Cell;
-use kernel::hil::time::{self, Alarm, ConvertTicks, Freq32KHz, Frequency, Ticks, Ticks64, Time};
+use kernel::hil::time::{self, Alarm, ConvertTicks, Freq1MHz, Frequency, Ticks, Ticks64, Time};
 use kernel::utilities::cells::OptionalCell;
 use kernel::utilities::registers::interfaces::{ReadWriteable, Readable, Writeable};
 use kernel::utilities::registers::register_bitfields;
@@ -134,7 +134,8 @@ impl<'a> InternalTimers<'a> {
 
 impl Time for InternalTimers<'_> {
     // TODO: replace with real VeeR frequency
-    type Frequency = Freq32KHz;
+    // This is roughly okay for the emulator though.
+    type Frequency = Freq1MHz;
     type Ticks = Ticks64;
 
     fn now(&self) -> Ticks64 {

xtask/Cargo.toml

@@ -11,6 +11,7 @@ tempfile = "3.14.0"
 walkdir = "2.5.0"
 proc-macro2.workspace = true
 clap.workspace = true
+elf.workspace = true
 registers-generator.workspace = true
 registers-systemrdl.workspace = true
 quote.workspace = true

xtask/src/apps_build.rs

@@ -29,22 +29,29 @@ pub const BASE_PERMISSIONS: &[(u32, u32)] = &[
     (1, 1),
     (1, 2),
     (1, 3),
+    (8, 0), // Low-level debug
+    (8, 1), // Low-level debug
+    (8, 2), // Low-level debug
+    (8, 3), // Low-level debug
 ];
 
 // creates a single flat binary with all the apps built with TBF headers
-pub fn apps_build_flat_tbf(start: usize) -> Result<Vec<u8>, DynError> {
+pub fn apps_build_flat_tbf(start: usize, ram_start: usize) -> Result<Vec<u8>, DynError> {
     let mut bin = vec![];
     let mut offset = start;
+    let mut ram_start = ram_start;
     for app in APPS.iter() {
-        let app_bin = app_build_tbf(app, offset)?;
+        let app_bin = app_build_tbf(app, offset, ram_start)?;
         bin.extend_from_slice(&app_bin);
         offset += app_bin.len();
+        // TODO: support different amount of RAM per app
+        ram_start += 0x4000;
     }
     Ok(bin)
 }
 
 // creates a flat binary of the app with the TBF header
-fn app_build_tbf(app: &App, start: usize) -> Result<Vec<u8>, DynError> {
+fn app_build_tbf(app: &App, start: usize, ram_start: usize) -> Result<Vec<u8>, DynError> {
     // start the TBF header
     let mut tbf = TbfHeader::new();
     let mut permissions = BASE_PERMISSIONS.to_vec();
@@ -63,7 +70,7 @@ fn app_build_tbf(app: &App, start: usize) -> Result<Vec<u8>, DynError> {
     tbf.set_binary_end_offset(0); // temporary just to get the size of the header
     let tbf_header_size = tbf.generate()?.get_ref().len();
 
-    app_build(app.name, start, tbf_header_size)?;
+    app_build(app.name, start, ram_start, tbf_header_size)?;
     let objcopy = objcopy()?;
 
     let app_bin = target_binary(&format!("{}.bin", app.name));
@@ -95,7 +102,12 @@ fn app_build_tbf(app: &App, start: usize) -> Result<Vec<u8>, DynError> {
 }
 
 // creates an ELF of the app
-fn app_build(app_name: &str, offset: usize, tbf_header_size: usize) -> Result<(), DynError> {
+fn app_build(
+    app_name: &str,
+    offset: usize,
+    ram_start: usize,
+    tbf_header_size: usize,
+) -> Result<(), DynError> {
     let layout_ld = &PROJECT_ROOT.join("runtime").join("apps").join("layout.ld");
 
     // TODO: do we need to fix the RAM start and length?
@@ -107,10 +119,10 @@ fn app_build(app_name: &str, offset: usize, tbf_header_size: usize) -> Result<()
 TBF_HEADER_SIZE = 0x{:x};
 FLASH_START = 0x{:x};
 FLASH_LENGTH = 0x10000;
-RAM_START = 0x50000000;
-RAM_LENGTH = 0x10000;
+RAM_START = 0x{:x};
+RAM_LENGTH = 0x4000;
 INCLUDE runtime/apps/app_layout.ld",
-            tbf_header_size, offset,
+            tbf_header_size, offset, ram_start
         ),
     )?;
 
@@ -120,7 +132,7 @@ INCLUDE runtime/apps/app_layout.ld",
         .current_dir(&*PROJECT_ROOT)
         .env("LIBTOCK_LINKER_FLASH", format!("0x{:x}", offset))
         .env("LIBTOCK_LINKER_FLASH_LENGTH", "128K")
-        .env("LIBTOCK_LINKER_RAM", "0x50000000")
+        .env("LIBTOCK_LINKER_RAM", format!("0x{:x}", ram_start))
         .env("LIBTOCK_LINKER_RAM_LENGTH", "128K")
         .args([
             "rustc",