From 7c3cc70ab108f59c044af85a16dfbb2d7639c805 Mon Sep 17 00:00:00 2001 From: Matt Harvey Date: Sun, 15 Aug 2021 14:06:55 -0700 Subject: [PATCH] Splits UART tx/rx into separate CAmkES procedures This ends the behavior where log messages would block on the prompt. This change does not fix the potential race on dataports if kata-uart-client read or write has multiple concurrent callers. A later change will protect those using CAmkES mutexes, although the alternative of having DebugConsole *own* the UART should also be considered. Change-Id: I8d5d8336cd58b9f22cca81ae6aca13b4ed57e7e4 GitOrigin-RevId: e781fd8454d22e0f829d788fe602e431551e259a --- .../DebugConsole/DebugConsole.camkes | 5 +- .../DebugConsole/kata-uart-client/src/lib.rs | 8 +-- .../components/UartDriver/UartDriver.camkes | 7 +-- .../system/components/UartDriver/src/driver.c | 8 +-- .../{uart.idl4 => dataport_io.idl4} | 5 +- apps/system/system.camkes | 57 +++++++++++-------- 6 files changed, 49 insertions(+), 41 deletions(-) rename apps/system/interfaces/{uart.idl4 => dataport_io.idl4} (82%) diff --git a/apps/system/components/DebugConsole/DebugConsole.camkes b/apps/system/components/DebugConsole/DebugConsole.camkes index 3a9afe4..2ff735e 100644 --- a/apps/system/components/DebugConsole/DebugConsole.camkes +++ b/apps/system/components/DebugConsole/DebugConsole.camkes @@ -6,9 +6,12 @@ import ; component DebugConsole { control; - uses uart_inf uart; + dataport Buf tx_dataport; + uses dataport_io_inf uart_tx; + dataport Buf rx_dataport; + uses dataport_io_inf uart_rx; provides LoggerInterface logger; uses ProcessControlInterface proc_ctrl; diff --git a/apps/system/components/DebugConsole/kata-uart-client/src/lib.rs b/apps/system/components/DebugConsole/kata-uart-client/src/lib.rs index a19db0f..2185a76 100644 --- a/apps/system/components/DebugConsole/kata-uart-client/src/lib.rs +++ b/apps/system/components/DebugConsole/kata-uart-client/src/lib.rs @@ -8,8 +8,8 @@ use kata_io as io; extern "C" { static rx_dataport: *mut cty::c_uchar; static tx_dataport: *mut cty::c_uchar; - fn uart_rx(n: cty::size_t); - fn uart_tx(n: cty::size_t); + fn uart_rx_update(n: cty::size_t); + fn uart_tx_update(n: cty::size_t); } // Console logging interface. @@ -27,7 +27,7 @@ pub struct Rx {} impl io::Read for Rx { fn read(&mut self, buf: &mut [u8]) -> Result { unsafe { - uart_rx(buf.len()); + uart_rx_update(buf.len()); let port = core::slice::from_raw_parts(rx_dataport, buf.len()); buf.copy_from_slice(&port); } @@ -42,7 +42,7 @@ impl io::Write for Tx { unsafe { let port = core::slice::from_raw_parts_mut(tx_dataport, buf.len()); port.copy_from_slice(buf); - uart_tx(buf.len()); + uart_tx_update(buf.len()); } Ok(buf.len()) } diff --git a/apps/system/components/UartDriver/UartDriver.camkes b/apps/system/components/UartDriver/UartDriver.camkes index 29b7d6a..c612672 100644 --- a/apps/system/components/UartDriver/UartDriver.camkes +++ b/apps/system/components/UartDriver/UartDriver.camkes @@ -11,11 +11,10 @@ */ component UartDriver { - dataport Buf mem; + dataport Buf mmio_region; dataport Buf tx_dataport; dataport Buf rx_dataport; - // consumes Interrupt interrupt; - - provides uart_inf uart; + provides dataport_io_inf uart_rx; + provides dataport_io_inf uart_tx; } diff --git a/apps/system/components/UartDriver/src/driver.c b/apps/system/components/UartDriver/src/driver.c index 17f4a87..871dc76 100644 --- a/apps/system/components/UartDriver/src/driver.c +++ b/apps/system/components/UartDriver/src/driver.c @@ -11,14 +11,14 @@ #include "opentitan/uart.h" // Referenced by macros in the generated file opentitan/uart.h. -#define UART0_BASE_ADDR (void *)mem +#define UART0_BASE_ADDR (void *)mmio_region // Frequency of the primary clock clk_i. #define CLK_FIXED_FREQ_HZ (24ull * 1000 * 1000) #define REG32(addr) *((volatile uint32_t *)(addr)) -void uart__init() { +void pre_init() { // Computes NCO value corresponding to baud rate. // nco = 2^20 * baud / fclk (assuming NCO width is 16-bit) uint64_t baud = 115200ull; @@ -75,7 +75,7 @@ static int uart_tx_ready() { */ } -void uart_rx(size_t n) { +void uart_rx_update(size_t n) { char *c = (char *)rx_dataport; // TODO(mattharvey): Error return value for n > PAGE_SIZE for (size_t i = 0; i < n && i < PAGE_SIZE; ++i) { @@ -87,7 +87,7 @@ void uart_rx(size_t n) { } } -void uart_tx(size_t n) { +void uart_tx_update(size_t n) { char *c = (char *)tx_dataport; // TODO(mattharvey): Error return value for n > PAGE_SIZE for (size_t i = 0; i < n && i < PAGE_SIZE; ++i) { diff --git a/apps/system/interfaces/uart.idl4 b/apps/system/interfaces/dataport_io.idl4 similarity index 82% rename from apps/system/interfaces/uart.idl4 rename to apps/system/interfaces/dataport_io.idl4 index acfa770..4f5c659 100644 --- a/apps/system/interfaces/uart.idl4 +++ b/apps/system/interfaces/dataport_io.idl4 @@ -10,7 +10,6 @@ * @TAG(DATA61_BSD) */ -procedure uart_inf { - void rx(in size_t n); - void tx(in size_t n); +procedure dataport_io_inf { + void update(in size_t n); }; diff --git a/apps/system/system.camkes b/apps/system/system.camkes index ce0d9b1..103bba8 100644 --- a/apps/system/system.camkes +++ b/apps/system/system.camkes @@ -12,7 +12,7 @@ import ; -import "interfaces/uart.idl4"; +import "interfaces/dataport_io.idl4"; import "interfaces/VectorCoreInterface.camkes"; import "components/UartDriver/UartDriver.camkes"; import "components/DebugConsole/DebugConsole.camkes"; @@ -23,9 +23,9 @@ import "components/VectorCoreDriver/VectorCoreDriver.camkes"; component UART { hardware; - dataport Buf mem; - // TODO(mattharvey): Make receives wait on interrupt. - // emits Interrupt interrupt; + dataport Buf mmio_region; + + emits Interrupt rx_watermark; } component VectorCoreHw { @@ -35,22 +35,27 @@ component VectorCoreHw { assembly { composition { - component UART uart; - component UartDriver drv; - component DebugConsole debug_console; - component ProcessManager process_manager; component SeL4Debug sel4debug; + component VectorCoreHw vctop; component VectorCoreDriver vc_drv; + + component UART uart; + component UartDriver uart_driver; + + component ProcessManager process_manager; component MlCoordinator ml_coordinator; + component DebugConsole debug_console; - connection seL4HardwareMMIO uart_mem(from drv.mem, to uart.mem); - connection seL4HardwareMMIO vc_csr(from vc_drv.csr, to vctop.csr); - + // UartDriver + connection seL4HardwareMMIO uart_mem(from uart_driver.mmio_region, + to uart.mmio_region); // TODO(mattharvey): Make receives wait on interrupt. - // connection seL4HardwareInterrupt uart_interrupt( - // from uart.interrupt, to drv.interrupt); - connection seL4RPCCall uart_inf(from debug_console.uart, to drv.uart); + + // VectorCoreDriver + connection seL4HardwareMMIO vc_csr(from vc_drv.csr, to vctop.csr); + connection seL4RPCCall VectorCoreInterface(from ml_coordinator.vctop, + to vc_drv.vctop); // Hookup ProcessManager to DebugConsole for shell commands. connection seL4RPCCall ProcessControlInterface(from debug_console.proc_ctrl, @@ -58,6 +63,16 @@ assembly { connection seL4RPCCall PackageManagerInterface(from debug_console.pkg_mgmt, to process_manager.pkg_mgmt); + // Connect the DebugConsole to the UartDriver. + connection seL4SharedData tx_channel( + from debug_console.tx_dataport, to uart_driver.tx_dataport); + connection seL4RPCCall tx_call( + from debug_console.uart_tx, to uart_driver.uart_tx); + connection seL4SharedData rx_channel( + from debug_console.rx_dataport, to uart_driver.rx_dataport); + connection seL4RPCCall rx_call( + from debug_console.uart_rx, to uart_driver.uart_rx); + // Connect the LoggerInterface to each component that needs to log // to the console. connection seL4RPCCall LoggerInterface(from process_manager.logger, @@ -72,25 +87,17 @@ assembly { connection seL4RPCCall MlCoordinatorInterface(from debug_console.mlcoord, to ml_coordinator.mlcoord); - connection seL4RPCCall VectorCoreInterface(from ml_coordinator.vctop, to vc_drv.vctop); - - connection seL4SharedData tx_channel( - from debug_console.tx_dataport, to drv.tx_dataport); - connection seL4SharedData rx_channel( - from debug_console.rx_dataport, to drv.rx_dataport); } configuration { - uart.mem_paddr = 0x40031000; - uart.mem_size = 0x1000; - // seL4 claims 10 is bigger than irqMax if this is uncommented. - // uart.interrupt_irq_number = 10; + uart.mmio_region_paddr = 0x40031000; + uart.mmio_region_size = 0x1000; vctop.csr_paddr = 0x48000000; vctop.csr_size = 0x1000; random.ID = 1; - uart.integrity_label = "drv"; + uart.integrity_label = "uart_driver"; } }