Merge pull request #1473 from hermit-os/serial

feat: rework console and resolve deadlock when panicking while printing

src/arch/aarch64/kernel/mod.rs

@@ -16,7 +16,6 @@ use core::arch::global_asm;
 use core::str;
 use core::sync::atomic::{AtomicU32, AtomicU64, Ordering};
 
-use hermit_sync::SpinMutex;
 use memory_addresses::arch::aarch64::{PhysAddr, VirtAddr};
 
 use crate::arch::aarch64::kernel::core_local::*;
@@ -25,7 +24,41 @@ use crate::env;
 
 const SERIAL_PORT_BAUDRATE: u32 = 115_200;
 
-static COM1: SpinMutex<SerialPort> = SpinMutex::new(SerialPort::new(0x800));
+pub struct Console {
+	serial_port: SerialPort,
+}
+
+impl Console {
+	pub fn new() -> Self {
+		CoreLocal::install();
+
+		let base = env::boot_info()
+			.hardware_info
+			.serial_port_base
+			.map(|uartport| uartport.get())
+			.unwrap_or_default()
+			.try_into()
+			.unwrap();
+
+		let serial_port = SerialPort::new(base);
+
+		serial_port.init(SERIAL_PORT_BAUDRATE);
+
+		Self { serial_port }
+	}
+
+	pub fn write(&mut self, buf: &[u8]) {
+		for byte in buf {
+			self.serial_port.write_byte(*byte);
+		}
+	}
+}
+
+impl Default for Console {
+	fn default() -> Self {
+		Self::new()
+	}
+}
 
 /// `CPU_ONLINE` is the count of CPUs that finished initialization.
 ///
@@ -77,36 +110,6 @@ pub fn args() -> Option<&'static str> {
 	None
 }
 
-/// Earliest initialization function called by the Boot Processor.
-pub fn message_output_init() {
-	CoreLocal::install();
-
-	let mut com1 = COM1.lock();
-
-	com1.port_address = env::boot_info()
-		.hardware_info
-		.serial_port_base
-		.map(|uartport| uartport.get())
-		.unwrap_or_default()
-		.try_into()
-		.unwrap();
-
-	// We can only initialize the serial port here, because VGA requires processor
-	// configuration first.
-	com1.init(SERIAL_PORT_BAUDRATE);
-}
-
-pub fn output_message_byte(byte: u8) {
-	// Output messages to the serial port.
-	COM1.lock().write_byte(byte);
-}
-
-pub fn output_message_buf(buf: &[u8]) {
-	for byte in buf {
-		output_message_byte(*byte);
-	}
-}
-
 /// Real Boot Processor initialization as soon as we have put the first Welcome message on the screen.
 #[cfg(target_os = "none")]
 pub fn boot_processor_init() {

src/arch/aarch64/kernel/serial.rs

@@ -34,11 +34,6 @@ impl SerialPort {
 		}
 	}
 
-	#[allow(dead_code)]
-	pub fn read(&mut self) -> Option<u8> {
-		None
-	}
-
 	pub fn init(&self, _baudrate: u32) {
 		// We don't do anything here (yet).
 	}

src/arch/mod.rs

@@ -20,8 +20,6 @@ cfg_if::cfg_if! {
 		pub(crate) use self::aarch64::kernel::application_processor_init;
 		pub(crate) use self::aarch64::kernel::{
 			get_processor_count,
-			message_output_init,
-			output_message_buf,
 		};
 		pub use self::aarch64::mm::paging::{BasePageSize, PageSize};
 	} else if #[cfg(target_arch = "x86_64")] {
@@ -46,8 +44,6 @@ cfg_if::cfg_if! {
 		pub(crate) use self::x86_64::kernel::boot_processor_init;
 		pub(crate) use self::x86_64::kernel::{
 			get_processor_count,
-			message_output_init,
-			output_message_buf,
 		};
 		pub use self::x86_64::mm::paging::{BasePageSize, PageSize};
 		#[cfg(feature = "common-os")]
@@ -68,8 +64,6 @@ cfg_if::cfg_if! {
 			core_local,
 			get_processor_count,
 			interrupts,
-			message_output_init,
-			output_message_buf,
 			scheduler,
 			switch,
 		};

src/arch/riscv64/kernel/mod.rs

@@ -27,6 +27,28 @@ use crate::config::KERNEL_STACK_SIZE;
 use crate::env;
 use crate::init_cell::InitCell;
 
+pub struct Console {}
+
+impl Console {
+	pub fn new() -> Self {
+		CoreLocal::install();
+
+		Self {}
+	}
+
+	pub fn write(&mut self, buf: &[u8]) {
+		for byte in buf {
+			sbi_rt::console_write_byte(*byte);
+		}
+	}
+}
+
+impl Default for Console {
+	fn default() -> Self {
+		Self::new()
+	}
+}
+
 // Used to store information about available harts. The index of the hart in the vector
 // represents its CpuId and does not need to match its hart_id
 pub(crate) static HARTS_AVAILABLE: InitCell<Vec<usize>> = InitCell::new(Vec::new());
@@ -109,17 +131,6 @@ pub fn get_current_boot_id() -> u32 {
 	CURRENT_BOOT_ID.load(Ordering::Relaxed)
 }
 
-/// Earliest initialization function called by the Boot Processor.
-pub fn message_output_init() {
-	CoreLocal::install();
-}
-
-pub fn output_message_buf(buf: &[u8]) {
-	for byte in buf {
-		sbi_rt::console_write_byte(*byte);
-	}
-}
-
 /// Real Boot Processor initialization as soon as we have put the first Welcome message on the screen.
 pub fn boot_processor_init() {
 	devicetree::init();

src/arch/x86_64/kernel/interrupts.rs

@@ -373,17 +373,17 @@ impl IrqStatistics {
 }
 
 pub(crate) fn print_statistics() {
-	println!("Number of interrupts");
+	panic_println!("Number of interrupts");
 	for (core_id, irg_statistics) in IRQ_COUNTERS.lock().iter() {
 		for (i, counter) in irg_statistics.counters.iter().enumerate() {
 			let counter = counter.load(Ordering::Relaxed);
 			if counter > 0 {
 				match get_irq_name(i.try_into().unwrap()) {
 					Some(name) => {
-						println!("[{core_id}][{name}]: {counter}");
+						panic_println!("[{core_id}][{name}]: {counter}");
 					}
 					_ => {
-						println!("[{core_id}][{i}]: {counter}");
+						panic_println!("[{core_id}][{i}]: {counter}");
 					}
 				}
 			}

src/arch/x86_64/kernel/mod.rs

@@ -5,7 +5,6 @@ use core::ptr;
 use core::sync::atomic::{AtomicPtr, AtomicU32, Ordering};
 
 use hermit_entry::boot_info::{PlatformInfo, RawBootInfo};
-use hermit_sync::InterruptSpinMutex;
 use memory_addresses::{PhysAddr, VirtAddr};
 use x86::controlregs::{cr0, cr0_write, cr4, Cr0};
 
@@ -37,8 +36,49 @@ pub(crate) mod systemtime;
 #[cfg(feature = "vga")]
 mod vga;
 
-/// Serial port to print kernel messages
-pub(crate) static COM1: InterruptSpinMutex<Option<SerialPort>> = InterruptSpinMutex::new(None);
+pub struct Console {
+	serial_port: SerialPort,
+}
+
+impl Console {
+	pub fn new() -> Self {
+		CoreLocal::install();
+
+		let base = env::boot_info()
+			.hardware_info
+			.serial_port_base
+			.unwrap()
+			.get();
+		let serial_port = unsafe { SerialPort::new(base) };
+		Self { serial_port }
+	}
+
+	pub fn write(&mut self, buf: &[u8]) {
+		self.serial_port.send(buf);
+
+		#[cfg(feature = "vga")]
+		for &byte in buf {
+			// vga::write_byte() checks if VGA support has been initialized,
+			// so we don't need any additional if clause around it.
+			vga::write_byte(byte);
+		}
+	}
+
+	pub fn buffer_input(&mut self) {
+		self.serial_port.buffer_input();
+	}
+
+	#[cfg(feature = "shell")]
+	pub fn read(&mut self) -> Option<u8> {
+		self.serial_port.read()
+	}
+}
+
+impl Default for Console {
+	fn default() -> Self {
+		Self::new()
+	}
+}
 
 pub fn get_ram_address() -> PhysAddr {
 	PhysAddr::new(env::boot_info().hardware_info.phys_addr_range.start)
@@ -106,33 +146,6 @@ pub fn args() -> Option<&'static str> {
 	}
 }
 
-// We can only initialize the serial port here, because VGA requires processor
-// configuration first.
-/// Earliest initialization function called by the Boot Processor.
-pub fn message_output_init() {
-	CoreLocal::install();
-
-	let base = env::boot_info()
-		.hardware_info
-		.serial_port_base
-		.unwrap()
-		.get();
-	let serial_port = unsafe { SerialPort::new(base) };
-	*COM1.lock() = Some(serial_port);
-}
-
-pub fn output_message_buf(buf: &[u8]) {
-	// Output messages to the serial port and VGA screen in unikernel mode.
-	COM1.lock().as_mut().unwrap().send(buf);
-
-	#[cfg(feature = "vga")]
-	for &byte in buf {
-		// vga::write_byte() checks if VGA support has been initialized,
-		// so we don't need any additional if clause around it.
-		vga::write_byte(byte);
-	}
-}
-
 /// Real Boot Processor initialization as soon as we have put the first Welcome message on the screen.
 #[cfg(target_os = "none")]
 pub fn boot_processor_init() {

src/arch/x86_64/kernel/serial.rs

@@ -3,9 +3,9 @@ use alloc::collections::VecDeque;
 
 use x86_64::instructions::port::Port;
 
+use crate::arch::x86_64::kernel::apic;
 use crate::arch::x86_64::kernel::core_local::increment_irq_counter;
 use crate::arch::x86_64::kernel::interrupts::{self, IDT};
-use crate::arch::x86_64::kernel::{apic, COM1};
 
 const SERIAL_IRQ: u8 = 36;
 
@@ -52,18 +52,11 @@ impl SerialPort {
 		}
 	}
 
-	#[allow(dead_code)]
 	#[cfg(feature = "shell")]
 	pub fn read(&mut self) -> Option<u8> {
 		self.buffer.pop_front()
 	}
 
-	#[allow(dead_code)]
-	#[cfg(not(feature = "shell"))]
-	pub fn read(&mut self) -> Option<u8> {
-		None
-	}
-
 	pub fn send(&mut self, buf: &[u8]) {
 		match &mut self.inner {
 			SerialInner::Uhyve(s) => {
@@ -83,7 +76,7 @@ impl SerialPort {
 }
 
 extern "x86-interrupt" fn serial_interrupt(_stack_frame: crate::interrupts::ExceptionStackFrame) {
-	COM1.lock().as_mut().unwrap().buffer_input();
+	crate::console::CONSOLE.lock().0.buffer_input();
 	increment_irq_counter(SERIAL_IRQ);
 
 	apic::eoi();

src/console.rs

@@ -1,10 +1,25 @@
-use core::fmt;
+use core::{fmt, mem};
 
-use hermit_sync::InterruptTicketMutex;
+use hermit_sync::{InterruptTicketMutex, Lazy};
 
 use crate::arch;
 
-pub(crate) struct Console(());
+pub struct Console(pub arch::kernel::Console);
+
+impl Console {
+	fn new() -> Self {
+		Self(arch::kernel::Console::new())
+	}
+
+	pub fn write(&mut self, buf: &[u8]) {
+		self.0.write(buf);
+	}
+
+	#[cfg(feature = "shell")]
+	pub fn read(&mut self) -> Option<u8> {
+		self.0.read()
+	}
+}
 
 /// A collection of methods that are required to format
 /// a message to Hermit's console.
@@ -13,22 +28,30 @@ impl fmt::Write for Console {
 	#[inline]
 	fn write_str(&mut self, s: &str) -> fmt::Result {
 		if !s.is_empty() {
-			let buf = s.as_bytes();
-			arch::output_message_buf(buf);
+			self.write(s.as_bytes());
 		}
 
 		Ok(())
 	}
 }
 
-static CONSOLE: InterruptTicketMutex<Console> = InterruptTicketMutex::new(Console(()));
+pub static CONSOLE: Lazy<InterruptTicketMutex<Console>> =
+	Lazy::new(|| InterruptTicketMutex::new(Console::new()));
 
 #[doc(hidden)]
 pub fn _print(args: fmt::Arguments<'_>) {
 	use fmt::Write;
 	CONSOLE.lock().write_fmt(args).unwrap();
 }
 
+#[doc(hidden)]
+pub fn _panic_print(args: fmt::Arguments<'_>) {
+	use fmt::Write;
+	let mut console = unsafe { CONSOLE.make_guard_unchecked() };
+	console.write_fmt(args).ok();
+	mem::forget(console);
+}
+
 #[cfg(all(test, not(target_os = "none")))]
 mod tests {
 	use super::*;