Everywhere: Use parking_lot's RwLock instead of std

This lets us do timeouts on locking, which simplifies the panic/ctrl-c handler issue!

Author: AndrewPiroli

Cargo.lock

@@ -11,6 +11,7 @@ anyhow = { version = "1.0.70", features = ["std", "backtrace"] }
 elf = { path = "../vendor/rust-elf", package = "elf2" }
 gimli = "~0.27.2"
 ironic-core = { path = "../core" }
+parking_lot = { version = "~0.12.1", default-features = false, features = ["nightly", "hardware-lock-elision"] }
 log = { version = "0.4.17", default-features = false, features = ["std"] }
 
 [target.'cfg(windows)'.dependencies]

back/Cargo.toml

@@ -8,9 +8,11 @@ pub mod lut;
 use anyhow::anyhow;
 use gimli::{BigEndian, read::*};
 use log::{error, info};
+use parking_lot::RwLock;
 
-use std::sync::{Arc, RwLock};
+use std::sync::Arc;
 use std::fs;
+use std::time::Duration;
 
 extern crate elf;
 
@@ -117,7 +119,7 @@ impl InterpBackend {
         match self.boot_status {
             BootStatus::Boot0 => {
                 if self.cpu.read_fetch_pc() == 0xfff0_0000 {
-                    if let Ok(bus) = self.bus.try_read() { // Try to detect boot1 version
+                    if let Some(bus) = self.bus.try_read_for(Duration::new(1,0)) { // Try to detect boot1 version
                         let boot1_otp_hash =
                         [
                             bus.hlwd.otp.read(0),
@@ -194,7 +196,7 @@ impl InterpBackend {
         // Probably a limitation of their early semihosting hardware
         // We buffer that internally until we see a newline, that's our cue to print
         let mut line_buf = [0u8; 16];
-        self.bus.read().map_err(|e| anyhow!(e.to_string()))?.dma_read(paddr, &mut line_buf)?;
+        self.bus.read().dma_read(paddr, &mut line_buf)?;
 
         let s = std::str::from_utf8(&line_buf)?
             .trim_matches(char::from(0));
@@ -268,7 +270,7 @@ impl InterpBackend {
                 )?;
                 info!(target: "Other", "DBG hotpatching module entrypoint {paddr:08x}");
                 info!(target: "Other", "{:?}", self.cpu.reg);
-                self.bus.write().map_err(|e| anyhow!(e.to_string()))?.dma_write(paddr,
+                self.bus.write().dma_write(paddr,
                     &Self::THREAD_CANCEL_PATCH)?;
             }
         }
@@ -399,17 +401,17 @@ impl Backend for InterpBackend {
                 }
             }
             match load_custom_kernel_debuginfo(&kernel_elf) {
-                Ok(debuginfo) => {self.bus.write().unwrap().install_debuginfo(debuginfo)},
+                Ok(debuginfo) => {self.bus.write().install_debuginfo(debuginfo)},
                 Err(err) => {error!(target: "Custom Kernel", "Failed to load debuginfo for kernel: {err}")},
             }
 
             match load_custom_kernel_debug_frame(&kernel_elf) {
-                Ok(debug_frames) => {self.bus.write().unwrap().install_debug_frames(debug_frames)},
+                Ok(debug_frames) => {self.bus.write().install_debug_frames(debug_frames)},
                 Err(err) => {error!(target: "Custom Kernel", "Failed to load debug frames for kernel: {err}")},
             }
 
             let headers = kernel_elf.phdrs;
-            let mut bus = self.bus.write().unwrap();
+            let mut bus = self.bus.write();
             // We are relying on the mirror being available
             // Or else we would be writing to mask ROM.
             bus.rom_disabled = true;
@@ -431,7 +433,7 @@ impl Backend for InterpBackend {
         loop {
             // Take ownership of the bus to deal with any pending tasks
             {
-                let mut bus = self.bus.write().unwrap();
+                let mut bus = self.bus.write();
                 bus.step(self.cpu_cycle)?;
                 self.bus_cycle += 1;
                 bus.update_debug_location(Some(self.cpu.reg.pc), Some(self.cpu.reg.r[14]), Some(self.cpu.reg.r[13]));

back/src/interp.rs

@@ -28,7 +28,7 @@ pub fn bkpt(cpu: &mut Cpu, op: BkptBits) -> DispatchRes {
             return DispatchRes::RetireOk;
         },
         0xfb => {
-            let bus = cpu.bus.read().expect("breakpoint instruction - bus access");
+            let bus = cpu.bus.read();
             match bus.dump_memory("bkpt.bin") {
                 Ok(path) => {
                     debug!(target: "Other", "Dumped RAM to {}/*.bkpt.bin", path.to_string_lossy());

back/src/interp/arm/misc.rs

@@ -32,7 +32,7 @@ pub fn bkpt(cpu: &mut Cpu, op: MiscBits) -> DispatchRes {
             return DispatchRes::RetireOk;
         },
         0xfb => {
-            let bus = cpu.bus.read().expect("breakpoint instruction - bus access");
+            let bus = cpu.bus.read();
             match bus.dump_memory("bkpt.bin") {
                 Ok(path) => {
                     debug!(target: "Other", "Dumped RAM to {}/*.bkpt.bin", path.to_string_lossy());

back/src/interp/thumb/misc.rs

@@ -8,11 +8,12 @@ use ironic_core::dev::hlwd::irq::*;
 use crate::back::*;
 
 use log::{info, error};
+use parking_lot::RwLock;
 
 use std::env::temp_dir;
 use std::path::PathBuf;
 use std::thread;
-use std::sync::{Arc, RwLock};
+use std::sync::Arc;
 use std::net::Shutdown;
 use std::io::{Read, Write};
 use std::convert::TryInto;
@@ -145,9 +146,9 @@ impl PpcBackend {
     fn wait_for_resp(&mut self) -> u32 {
         info!(target: "PPC", "waiting for response ...");
         loop {
-            if self.bus.read().unwrap().hlwd.irq.ppc_irq_output {
+            if self.bus.read().hlwd.irq.ppc_irq_output {
                 info!(target: "PPC", "got irq");
-                let mut bus = self.bus.write().unwrap();
+                let mut bus = self.bus.write();
 
                 if bus.hlwd.ipc.state.ppc_ack {
                     info!(target: "PPC", "got extra ACK");
@@ -167,7 +168,7 @@ impl PpcBackend {
                     return armmsg;
                 }
 
-                drop(bus); // Drop bus to avoid poisoning the lock.
+                drop(bus); // Release RwLock
                 error!(target: "PPC", "Invalid IRQ state");
                 unreachable!("Invalid IRQ state. You forgot to update your IRQ lines somewhere!");
             } else {
@@ -180,9 +181,9 @@ impl PpcBackend {
     fn wait_for_ack(&mut self) {
         info!(target: "PPC", "waiting for ACK ...");
         loop {
-            if self.bus.read().unwrap().hlwd.irq.ppc_irq_output {
+            if self.bus.read().hlwd.irq.ppc_irq_output {
                 info!(target: "PPC", "got irq");
-                let mut bus = self.bus.write().unwrap();
+                let mut bus = self.bus.write();
 
                 if bus.hlwd.ipc.state.ppc_ack {
                     bus.hlwd.ipc.state.ppc_ack = false;
@@ -201,7 +202,7 @@ impl PpcBackend {
                     continue;
                 }
 
-                drop(bus); // Drop bus to avoid poisoning the lock.
+                drop(bus); // Release RwLock
                 error!(target: "PPC", "Invalid IRQ state");
                 unreachable!("Invalid IRQ state. You forgot to update your IRQ lines somewhere!")
             } else {
@@ -236,7 +237,7 @@ impl PpcBackend {
     /// Read from physical memory.
     pub fn handle_read(&mut self, client: &mut UnixStream, req: SocketReq) -> anyhow::Result<()> {
         info!(target: "PPC", "read {:x} bytes at {:08x}", req.len, req.addr);
-        self.bus.read().unwrap().dma_read(req.addr,
+        self.bus.read().dma_read(req.addr,
             &mut self.obuf[0..req.len as usize])?;
         let _ = client.write(&self.obuf[0..req.len as usize])?; // maybe FIXME: is it ok to ignore the # of bytes written here?
         Ok(())
@@ -246,15 +247,15 @@ impl PpcBackend {
     pub fn handle_write(&mut self, client: &mut UnixStream, req: SocketReq) -> anyhow::Result<()> {
         info!(target: "PPC", "write {:x} bytes at {:08x}", req.len, req.addr);
         let data = &self.ibuf[0xc..(0xc + req.len as usize)];
-        self.bus.write().unwrap().dma_write(req.addr, data)?;
+        self.bus.write().dma_write(req.addr, data)?;
         let _ = client.write("OK".as_bytes())?; // maybe FIXME: is it ok to ignore the # of bytes written here?
         Ok(())
     }
 
     /// Tell ARM-world that an IPC request is ready at the location indicated
     /// by the pointer in PPC_MSG.
     pub fn handle_message(&mut self, client: &mut UnixStream, req: SocketReq) -> anyhow::Result<()> {
-        let mut bus = self.bus.write().unwrap();
+        let mut bus = self.bus.write();
         bus.hlwd.ipc.ppc_msg = req.addr;
         bus.hlwd.ipc.state.arm_req = true;
         bus.hlwd.ipc.state.arm_ack = true;
@@ -263,7 +264,7 @@ impl PpcBackend {
     }
 
     pub fn handle_ack(&mut self, _req: SocketReq) -> anyhow::Result<()> {
-        let mut bus = self.bus.write().unwrap();
+        let mut bus = self.bus.write();
         let ppc_ctrl = bus.hlwd.ipc.read_handler(4)? & 0x3c;
         bus.hlwd.ipc.write_handler(4, ppc_ctrl | 0x8)?;
         Ok(())
@@ -275,10 +276,10 @@ impl PpcBackend {
 impl Backend for PpcBackend {
     fn run(&mut self) -> anyhow::Result<()> {
         info!(target: "PPC", "PPC backend thread started");
-        self.bus.write().unwrap().hlwd.ipc.state.ppc_ctrl_write(0x36);
+        self.bus.write().hlwd.ipc.state.ppc_ctrl_write(0x36);
 
         loop {
-            if self.bus.read().unwrap().hlwd.ppc_on {
+            if self.bus.read().hlwd.ppc_on {
                 info!(target: "PPC", "Broadway came online");
                 break;
             }
@@ -289,7 +290,7 @@ impl Backend for PpcBackend {
         self.wait_for_ack();
 
         // Send an extra ACK
-        self.bus.write().unwrap().hlwd.ipc.state.arm_ack = true;
+        self.bus.write().hlwd.ipc.state.arm_ack = true;
         thread::sleep(std::time::Duration::from_millis(100));
 
         // Try binding to the socket

back/src/ppc.rs

@@ -17,3 +17,4 @@ crc32fast = { version = "~1.3.2", default-features = false, features = ["std", "
 bincode = { version = "~2.0.0-rc.3" }
 lz4_flex = { version = "~0.11.1", default-features = false, features = ["std", "safe-encode", "safe-decode", "frame"] }
 iset = { version = "~0.2.2", default-features = false }
+parking_lot = { version = "~0.12.1", default-features = false, features = ["nightly", "hardware-lock-elision"] }

core/Cargo.toml

@@ -6,7 +6,8 @@ pub mod psr;
 pub mod mmu;
 pub mod alu;
 
-use std::sync::{Arc,RwLock};
+use std::sync::Arc;
+use parking_lot::RwLock;
 
 use crate::bus::*;
 use crate::cpu::excep::*;

core/src/cpu.rs

@@ -1,7 +1,7 @@
 //! Coprocessor register definitions and functionality.
 
-use std::{cell::RefCell, collections::HashMap, sync::{RwLock, Arc}, hash::BuildHasherDefault};
-use anyhow::anyhow;
+use std::{cell::RefCell, collections::HashMap, sync::Arc, hash::BuildHasherDefault};
+use parking_lot::RwLock;
 
 use crate::bus::Bus;
 use fxhash::FxHasher32;
@@ -244,7 +244,7 @@ impl SystemControl {
         let val = match tlb_inner.get(&addr) {
             Some(val) => *val, // TLB hit
             None => { // miss
-                let val = bus.read().map_err(|e| anyhow!(e.to_string()))?.read32(addr)?;
+                let val = bus.read().read32(addr)?;
                 tlb_inner.insert(addr, val);
                 val
             },

core/src/cpu/coproc.rs

@@ -5,37 +5,37 @@ pub mod prim;
 use crate::cpu::mmu::prim::*;
 use crate::cpu::Cpu;
 
-use anyhow::{anyhow, bail};
+use anyhow::bail;
 
 /// These are the top-level "public" functions providing read/write accesses.
 impl Cpu {
     pub fn read32(&self, addr: u32) -> anyhow::Result<u32> {
         let paddr = self.translate(TLBReq::new(addr, Access::Read))?;
-        let res = self.bus.read().unwrap().read32(paddr)?;
+        let res = self.bus.read().read32(paddr)?;
         Ok(res)
     }
     pub fn read16(&self, addr: u32) -> anyhow::Result<u16> {
         let paddr = self.translate(TLBReq::new(addr, Access::Read))?;
-        let res = self.bus.read().unwrap().read16(paddr)?;
+        let res = self.bus.read().read16(paddr)?;
         Ok(res)
     }
     pub fn read8(&self, addr: u32) -> anyhow::Result<u8> {
         let paddr = self.translate(TLBReq::new(addr, Access::Read))?;
-        let res = self.bus.read().unwrap().read8(paddr)?;
+        let res = self.bus.read().read8(paddr)?;
         Ok(res)
     }
 
     pub fn write32(&mut self, addr: u32, val: u32) -> anyhow::Result<()> {
         let paddr = self.translate(TLBReq::new(addr, Access::Write))?;
-        self.bus.write().unwrap().write32(paddr, val)
+        self.bus.write().write32(paddr, val)
     }
     pub fn write16(&mut self, addr: u32, val: u32) -> anyhow::Result<()> {
         let paddr = self.translate(TLBReq::new(addr, Access::Write))?;
-        self.bus.write().unwrap().write16(paddr, val as u16)
+        self.bus.write().write16(paddr, val as u16)
     }
     pub fn write8(&mut self, addr: u32, val: u32) -> anyhow::Result<()> {
         let paddr = self.translate(TLBReq::new(addr, Access::Write))?;
-        self.bus.write().unwrap().write8(paddr, val as u8)
+        self.bus.write().write8(paddr, val as u8)
     }
 }
 
@@ -103,7 +103,7 @@ impl Cpu {
             },
             _ => bail!("l2_fetch requires an L1::Coarse descriptor"),
         };
-        let val = self.bus.read().map_err(|e| anyhow!(e.to_string()))?.read32(addr)?;
+        let val = self.bus.read().read32(addr)?;
         Ok(L2Descriptor::from_u32(val))
     }