From d651e737675b5aebbc0920fb8f34cdcb7fb3d42a Mon Sep 17 00:00:00 2001 From: Matt Harvey Date: Thu, 19 Aug 2021 23:16:39 +0000 Subject: [PATCH] Merge "Makes UART receives interrupt-driven" GitOrigin-RevId: d8fd1cba1db7257f44e971af457a0fa59b2373ce --- .../components/UartDriver/UartDriver.camkes | 4 + .../system/components/UartDriver/src/driver.c | 135 ++++++++++++------ apps/system/system.camkes | 4 +- 3 files changed, 99 insertions(+), 44 deletions(-) diff --git a/apps/system/components/UartDriver/UartDriver.camkes b/apps/system/components/UartDriver/UartDriver.camkes index c612672..06a75d4 100644 --- a/apps/system/components/UartDriver/UartDriver.camkes +++ b/apps/system/components/UartDriver/UartDriver.camkes @@ -17,4 +17,8 @@ component UartDriver { provides dataport_io_inf uart_rx; provides dataport_io_inf uart_tx; + consumes Interrupt rx_watermark; + + has semaphore rx_semaphore; + has mutex rx_mutex; } diff --git a/apps/system/components/UartDriver/src/driver.c b/apps/system/components/UartDriver/src/driver.c index 871dc76..24ec263 100644 --- a/apps/system/components/UartDriver/src/driver.c +++ b/apps/system/components/UartDriver/src/driver.c @@ -3,10 +3,10 @@ // A programming guide for the hardware can be found at // https://docs.opentitan.org/hw/ip/uart/doc/ -#include - +#include #include #include +#include #include "opentitan/uart.h" @@ -14,15 +14,22 @@ #define UART0_BASE_ADDR (void *)mmio_region // Frequency of the primary clock clk_i. -#define CLK_FIXED_FREQ_HZ (24ull * 1000 * 1000) +#define CLK_FIXED_FREQ_HZ (48ull * 1000 * 1000) #define REG32(addr) *((volatile uint32_t *)(addr)) +#define UART_BUF_SIZE 512 + +static char rx_buf[UART_BUF_SIZE]; +static char *rx_buf_end = rx_buf; // guarded by rx_mutex + void pre_init() { // Computes NCO value corresponding to baud rate. // nco = 2^20 * baud / fclk (assuming NCO width is 16-bit) + seL4_CompileTimeAssert(UART_CTRL_NCO_MASK == 0xffff); uint64_t baud = 115200ull; uint64_t uart_ctrl_nco = ((uint64_t)baud << 20) / CLK_FIXED_FREQ_HZ; + seL4_Assert(uart_ctrl_nco < 0xffff); // Sets baud rate and enables TX and RX. REG32(UART_CTRL(0)) = @@ -34,67 +41,109 @@ void pre_init() { REG32(UART_FIFO_CTRL(0)) = fifo_ctrl | UART_FIFO_CTRL_RXRST | UART_FIFO_CTRL_TXRST; - // Disables interrupts. - // TODO (mattharvey): Configure seL4 to dispatch UART interrupts to this - // driver, enable here at least TX watermark and RX watermark, and add - // handlers. For now, this driver spins to wait. - REG32(UART_INTR_ENABLE(0)) = 0ul; + // Sets RX watermark to 1. + // + // This enables calls that block on a single byte at a time, like the one the + // shell does when reading a line of input, to return immediately when that + // byte is received. + // + // Note that this high watermark is only a threshold for when to be informed + // that bytes have been received. The FIFO can still fill to its full capacity + // (32) independent of how this is set. + fifo_ctrl = REG32(UART_FIFO_CTRL(0)); + fifo_ctrl = fifo_ctrl & (~UART_FIFO_CTRL_RXILVL_MASK); + fifo_ctrl = fifo_ctrl | (UART_FIFO_CTRL_RXILVL_VALUE_RXLVL1 + << UART_FIFO_CTRL_RXILVL_OFFSET); + REG32(UART_FIFO_CTRL(0)) = fifo_ctrl; + + // Enables interrupts. + REG32(UART_INTR_ENABLE(0)) = (1 << UART_INTR_COMMON_RX_WATERMARK); + + // TODO (mattharvey): Add tx_watermark_handle and make uart_tx have a buffer + // and be interrupt-driven. + + rx_buf_end = rx_buf; +} + +static int uart_get_rx_level() { + return (REG32(UART_FIFO_STATUS(0)) & + (UART_FIFO_STATUS_RXLVL_MASK << UART_FIFO_STATUS_RXLVL_OFFSET)) >> + UART_FIFO_STATUS_RXLVL_OFFSET; } static int uart_rx_empty() { return (REG32(UART_STATUS(0)) & (1 << UART_STATUS_RXEMPTY)) != 0; - /* - * A more direct adapation of the example from the Programmers Guide in - * - * https://docs.opentitan.org/hw/ip/uart/doc/ - * - * would look like the below. (The example does not compile verbatim.) Using - * the UART_STATUS register is simpler than this expression and seems - * equivalent, according to the wording of the doc. - * - * Still, we'll keep this commented implementation until we gain some - * confidence the simulation is happy with the simpler approach. - * - return (REG32(UART_FIFO_STATUS(0)) & - (UART_FIFO_STATUS_RXLVL_MASK << UART_FIFO_STATUS_RXLVL_OFFSET)) >> - UART_FIFO_STATUS_RXLVL_OFFSET == - 0; - */ } static int uart_tx_ready() { return (REG32(UART_STATUS(0)) & (1 << UART_STATUS_TXFULL)) == 0; - /* - * See similar comment in uart_rx_empty. - * - int32_t tx_fifo_capacity = 32; // uart.h provides no define for this. - return ((REG32(UART_FIFO_STATUS(0)) & UART_FIFO_STATUS_TXLVL_MASK) == - tx_fifo_capacity) - ? 0 - : 1; - */ } +static char uart_getchar() { + return REG32(UART_RDATA(0)) & UART_RDATA_RDATA_MASK; +} + +static void uart_putchar(char c) { REG32(UART_WDATA(0)) = c; } + void uart_rx_update(size_t n) { - char *c = (char *)rx_dataport; + char *dataport_cursor = (char *)rx_dataport; // TODO(mattharvey): Error return value for n > PAGE_SIZE - for (size_t i = 0; i < n && i < PAGE_SIZE; ++i) { - while (uart_rx_empty()) { - seL4_Yield(); // TODO(mattharvey): remove when interrupt-driven + seL4_Assert(n <= PAGE_SIZE); + + size_t num_read = 0; + while (num_read < n) { + while (rx_buf_end == rx_buf) { + seL4_Assert(rx_semaphore_wait() == 0); } - *c = REG32(UART_RDATA(0)) & UART_RDATA_RDATA_MASK; - ++c; + + char *read_buf_cursor = rx_buf; + while (num_read < n && read_buf_cursor < rx_buf_end) { + *(dataport_cursor++) = *(read_buf_cursor++); + ++num_read; + } + + // Shifts remainder of rx_buf to the beginning. + seL4_Assert(rx_mutex_lock() == 0); + char *write_buf_cursor = rx_buf; + while (read_buf_cursor < rx_buf_end) { + *(write_buf_cursor++) = *(read_buf_cursor++); + } + rx_buf_end = write_buf_cursor; + seL4_Assert(rx_mutex_unlock() == 0); } } 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) { + seL4_Assert(n <= PAGE_SIZE); + + for (size_t i = 0; i < n; ++i) { while (!uart_tx_ready()) { - seL4_Yield(); // TODO(mattharvey): remove when interrupt-driven + seL4_Yield(); } - REG32(UART_WDATA(0)) = *c; + uart_putchar(*c); ++c; } } + +void rx_watermark_handle(void) { + size_t buf_remaining_size = sizeof(rx_buf) - (rx_buf_end - rx_buf); + size_t num_read = 0; + + seL4_Assert(rx_mutex_lock() == 0); + while (!uart_rx_empty() && num_read < buf_remaining_size) { + *(rx_buf_end++) = uart_getchar(); + ++num_read; + } + seL4_Assert(rx_mutex_unlock() == 0); + + if (num_read > 0) { + seL4_Assert(rx_semaphore_post() == 0); + } + + // Clears INTR_STATE for rx_watermark. + REG32(UART_INTR_STATE(0)) = (1 << UART_INTR_STATE_RX_WATERMARK); + + seL4_Assert(rx_watermark_acknowledge() == 0); +} diff --git a/apps/system/system.camkes b/apps/system/system.camkes index f4e1cd5..303fac9 100644 --- a/apps/system/system.camkes +++ b/apps/system/system.camkes @@ -50,7 +50,8 @@ assembly { // UartDriver connection seL4HardwareMMIO uart_mem(from uart_driver.mmio_region, to uart.mmio_region); - // TODO(mattharvey): Make receives wait on interrupt. + connection seL4HardwareInterrupt uart_rx_watermark(from uart.rx_watermark, + to uart_driver.rx_watermark); // VectorCoreDriver connection seL4HardwareMMIO vc_csr(from vc_drv.csr, to vctop.csr); @@ -92,6 +93,7 @@ assembly { configuration { uart.mmio_region_paddr = 0x40030000; uart.mmio_region_size = 0x1000; + uart.rx_watermark_irq_number = 26; vctop.csr_paddr = 0x48000000; vctop.csr_size = 0x1000;