From b433dfa308a5a3a8e2b5024203e9179396db4119 Mon Sep 17 00:00:00 2001 From: Kit Rhett Aultman Date: Sun, 29 Sep 2024 14:58:30 -0400 Subject: [PATCH] uart/trap/main: remove global read_uart fn The previous implementation of the UART interrupt handler relied on a public global function, read_uart, in order to get data waiting in the UART's registers. This is an artifact of the Uart code which focuses on having a single global Uart, largely so the println!() macros can work with it. It seems a bit awkward going forward having all possible read/write points of the UART going through global functions; indeed, future code would like to work simply by writing bytes or strings into interfaces and not worrying about where it goes, so as a stepping stone on that way, this commit refactors out the global read_uart( ) function and replaces it with a scheme where the interrupt handler has its own interface to the UART. Having multiple Uart objects is fine for now but really only because we know we're single-threading. More ideal would be, in the future, for each Uart to be a reference to a single Uart struct, since the Uart's underlying implementation already has some amount of concurrency control. At the end of the day, there's one single UART on the system, and we really want to create a situation where code writes into, say, a Console interface instead of the Uart, and there's a "console manager" deciding what's going to the UART and what isn't. Baby steps, though. --- src/main.rs | 16 ++++++++++++---- src/trap.rs | 16 +++++++++------- src/uart.rs | 19 +++++-------------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/main.rs b/src/main.rs index b7d72ac..a12d09b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -14,6 +14,13 @@ use core::arch::asm; use core::arch::global_asm; use core::panic::PanicInfo; +extern crate alloc; +use alloc::boxed::Box; + +use trap::TrapFrame; +use uart::Uart; +use uart::SERIAL_BASE; + global_asm!(include_str!("trap.S")); // cfg not test is set here because the analyzer and the test system @@ -100,10 +107,11 @@ extern "C" fn entry() -> ! { // Safety: initialize once (and that's done now) unsafe { heap::init() }; - // Load the trap frame for interrupt handling - // This must be done here rather than in the _start function because the _start function is naked and - // we can't use keywords other than const and sym unsafe { + // Load the trap frame for interrupt handling + // This must be done here rather than in the _start function because the _start function is naked and + // we can't use keywords other than const and sym + let trap_frame = Box::new(TrapFrame::new(Uart::new(SERIAL_BASE))); asm!( // Turn on floating point support "li t0, 1 << 13", @@ -112,7 +120,7 @@ extern "C" fn entry() -> ! { "csrw mscratch, {trap_frame}", "lla t2, m_trap_vector", "csrw mtvec, t2", - trap_frame = in(reg)(&raw mut trap::TRAP_FRAME as usize) + trap_frame = in(reg)(Box::into_raw(trap_frame) as usize) ); // We're now good to take interrupts diff --git a/src/trap.rs b/src/trap.rs index b40787f..b40cb03 100644 --- a/src/trap.rs +++ b/src/trap.rs @@ -1,9 +1,10 @@ use crate::uart::uart_read; +use crate::uart::Uart; use core::arch::asm; use plic::Plic; #[repr(C)] -#[derive(Clone, Copy)] +//#[derive(Clone, Copy)] pub struct TrapFrame { pub regs: [usize; 32], // 8 bytes per register; 32 gp regs (0 - 255) pub fregs: [usize; 32], // 8 bytes per register; 32 fp regs (256 - 511) @@ -17,19 +18,20 @@ pub struct TrapFrame { // We're also skipping this; it's not needed when we've got only one hart and besides // that we're in machine mode. //pub hartid: usize, // 528 + pub uart: Uart, } + impl TrapFrame { - pub const fn new() -> Self { + pub fn new(u: Uart) -> Self { TrapFrame { regs: [0; 32], fregs: [0; 32], + uart: u } } } -pub static mut TRAP_FRAME: TrapFrame = TrapFrame::new(); - // unsafe: call only once! pub unsafe fn init_interrupts() { asm!("csrsi mstatus, 0b1000", "li t2, 0xaaa", "csrw mie, t2"); @@ -52,7 +54,7 @@ extern "C" fn m_trap( cause: usize, hart: usize, _status: usize, - _frame: &mut TrapFrame, + frame: &mut TrapFrame, ) -> usize { // We're going to handle all traps in machine mode. RISC-V lets // us delegate to supervisor mode, but switching out SATP (virtual memory) @@ -90,8 +92,8 @@ extern "C" fn m_trap( let plic = get_plic(); match plic.claim(0) { Some(int) => { - let data = uart_read(); - print!("{}", data as char); + let data = frame.uart.read(); + frame.uart.write(data); plic.complete(0, int); } _ => { diff --git a/src/uart.rs b/src/uart.rs index 73bbc41..29b50fa 100644 --- a/src/uart.rs +++ b/src/uart.rs @@ -28,6 +28,10 @@ impl Uart { pub fn write(&mut self, data: u8) { self.serial_port.send(data); } + + pub fn read(&mut self) -> u8 { + return self.serial_port.receive(); + } } // Write trait is used in the macros below; this basicallt @@ -40,7 +44,7 @@ impl core::fmt::Write for Uart { } } -static SERIAL_BASE: usize = 0x1000_0000; +pub static SERIAL_BASE: usize = 0x1000_0000; // Spinlock provides us a means to have a global console that is initialized // at runtime. Note below that locking outside of this module seems to have @@ -56,19 +60,6 @@ pub fn init_uart() { *uart = Some(unsafe { Uart::new(SERIAL_BASE) }) } -// Read a byte from the underlying UART, clearing any asserted interrupt in the process. -// It's not entirely clear why, but locking CONSOLE outside of this module, or at least doing -// so while in interrupt context, seems to never return, but providing a public function -// to do the work gets around this problem. -pub fn uart_read() -> u8 { - return UART - .lock() - .as_mut() - .unwrap_or_else(|| panic!("Failed to acquire console for reading!")) - .serial_port - .receive(); -} - // These, like a lot of the code in here, came courtesy of Meyer Zinn's // baremetal Rust tutorials: https://meyerzinn.tech/posts/2023/03/08/p1-printing-and-allocating/ // I don't know how to use macros that well yet! -- 2.34.1