Skip to content

[fix]: volatile store the intr-critical regs in uart, now uart input … #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
chyyuu wants to merge 1 commit into from
Closed

[fix]: volatile store the intr-critical regs in uart, now uart input … #33

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion components/axplat_crates/platforms/axplat-riscv64-qemu-virt/src/console.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,19 @@ use crate::config::{devices::UART_PADDR, plat::PHYS_VIRT_OFFSET};
static UART: LazyInit<SpinNoIrq<MmioSerialPort>> = LazyInit::new();

pub(crate) fn init_early() {
let base = UART_PADDR + PHYS_VIRT_OFFSET;
UART.init_once({
let mut uart = unsafe { MmioSerialPort::new(UART_PADDR + PHYS_VIRT_OFFSET) };
let mut uart = unsafe { MmioSerialPort::new(base) };
uart.init();
// `uart_16550` uses non-volatile ptr::write() which the compiler may
// optimise away. Re-write the interrupt-critical registers with
// volatile stores — in particular MCR.OUT2 (bit 3) gates the 16550
// interrupt output; without it the UART never signals the PLIC.
unsafe {
core::ptr::write_volatile((base + 2) as *mut u8, 0x01); // FCR: FIFO enable, 1-byte trigger
core::ptr::write_volatile((base + 4) as *mut u8, 0x0B); // MCR: DTR+RTS+OUT2
core::ptr::write_volatile((base + 1) as *mut u8, 0x01); // IER: RX data available
Comment on lines +20 to +22
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The register offsets and values here (e.g., base + 2, 0x0B, 0x01) are hardware-specific magic numbers. Consider introducing local const offsets/bitmasks (FCR/MCR/IER and OUT2) to make the intent clearer and reduce the chance of accidental drift if these settings change.

Copilot uses AI. Check for mistakes.
}
Comment on lines +21 to +23
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The volatile writes to MCR/IER enable UART interrupts unconditionally (MCR.OUT2 + IER=0x01). Since irq is a feature flag on this platform, consider gating the interrupt-enabling writes with #[cfg(feature = "irq")] (or otherwise ensuring they can’t fire when IRQ support/handlers aren’t enabled) to avoid behavior changes in non-IRQ builds.

Suggested change
core::ptr::write_volatile((base + 4) as *mut u8, 0x0B); // MCR: DTR+RTS+OUT2
core::ptr::write_volatile((base + 1) as *mut u8, 0x01); // IER: RX data available
}
}
#[cfg(feature = "irq")]
unsafe {
core::ptr::write_volatile((base + 4) as *mut u8, 0x0B); // MCR: DTR+RTS+OUT2 (enables UART IRQ output)
core::ptr::write_volatile((base + 1) as *mut u8, 0x01); // IER: RX data available (UART IRQ enable)
}

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +23
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new unsafe MMIO block would benefit from an explicit // SAFETY: note explaining why (base + offset) as *mut u8 is valid and correctly mapped as UART device memory (and why 8-bit accesses are correct for this platform). This makes the unsafety audit-friendly and consistent with other parts of the repo that document unsafe invariants.

Copilot uses AI. Check for mistakes.
SpinNoIrq::new(uart)
});
}
Expand Down
Loading