From b7727a08238876d7f78e41e43bd6ce4ce6eea56b Mon Sep 17 00:00:00 2001 From: Matt Harvey Date: Thu, 19 Aug 2021 23:17:14 +0000 Subject: [PATCH] Merge "Makes UART transmit interrupt-driven" GitOrigin-RevId: 12484d6028b4885eb42e3074c75114454135e270 --- .../OpenTitanUARTDriver.camkes | 12 +- .../OpenTitanUARTDriver/src/circular_buffer.c | 3 - .../OpenTitanUARTDriver/src/driver.c | 221 +++++++++++++----- apps/system/system.camkes | 8 + 4 files changed, 183 insertions(+), 61 deletions(-) diff --git a/apps/system/components/OpenTitanUARTDriver/OpenTitanUARTDriver.camkes b/apps/system/components/OpenTitanUARTDriver/OpenTitanUARTDriver.camkes index 8545eed..fcc306d 100644 --- a/apps/system/components/OpenTitanUARTDriver/OpenTitanUARTDriver.camkes +++ b/apps/system/components/OpenTitanUARTDriver/OpenTitanUARTDriver.camkes @@ -12,13 +12,17 @@ component OpenTitanUARTDriver { dataport Buf mmio_region; + dataport Buf tx_dataport; - dataport Buf rx_dataport; - - provides dataport_io_inf rx; provides dataport_io_inf tx; - consumes Interrupt rx_watermark; + consumes Interrupt tx_watermark; + consumes Interrupt tx_empty; + has semaphore tx_semaphore; + has mutex tx_mutex; + dataport Buf rx_dataport; + provides dataport_io_inf rx; + consumes Interrupt rx_watermark; has semaphore rx_semaphore; has mutex rx_mutex; } diff --git a/apps/system/components/OpenTitanUARTDriver/src/circular_buffer.c b/apps/system/components/OpenTitanUARTDriver/src/circular_buffer.c index 6b6a3b7..1b59384 100644 --- a/apps/system/components/OpenTitanUARTDriver/src/circular_buffer.c +++ b/apps/system/components/OpenTitanUARTDriver/src/circular_buffer.c @@ -7,9 +7,6 @@ */ #include "circular_buffer.h" -// TODO(mattharvey): Determine how to do unit testing within the kata build -// system and add unit tests covering circular_buffer. - // Advances one of the begin/end pointers, wrapping around the end of the // data array when necessary. static void *circular_buffer_advance(circular_buffer *buf, char **p) { diff --git a/apps/system/components/OpenTitanUARTDriver/src/driver.c b/apps/system/components/OpenTitanUARTDriver/src/driver.c index 842dc99..ff114b6 100644 --- a/apps/system/components/OpenTitanUARTDriver/src/driver.c +++ b/apps/system/components/OpenTitanUARTDriver/src/driver.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "circular_buffer.h" #include "opentitan/uart.h" @@ -18,6 +19,9 @@ // Referenced by macros in the generated file opentitan/uart.h. #define UART0_BASE_ADDR (void *)mmio_region +// The TX/RX Fifo capacity mentioned in the programming guide. +#define UART_FIFO_CAPACITY 32ul + // This is the default in CAmkES 2 and the configurable default in CAmkES 3. #define TX_RX_DATAPORT_CAPACITY PAGE_SIZE @@ -29,21 +33,35 @@ // https://docs.opentitan.org/hw/ip/clkmgr/doc/ #define CLK_FIXED_FREQ_HZ (48ull * 1000 * 1000) -// Read/write access to a single 32-bit register. -#define REG32(addr) *((volatile uint32_t *)(addr)) +// Read/write access to a 32-bit register of UART0, using substrings of the +// #define names in opentitan/uart.h. (The literal 0 is the value of ##id## +// substitutions in uart.h.) +#define REG(name) *((volatile uint32_t *)(UART_##name(0))) + +#define SHIFT_DOWN_AND_MASK(regval, regname, subfield) \ + ((regval >> UART_##regname##_##subfield##_OFFSET) & \ + UART_##regname##_##subfield##_MASK) + +#define MASK_AND_SHIFT_UP(value, regname, subfield) \ + ((value & UART_##regname##_##subfield##_MASK) \ + << UART_##regname##_##subfield##_OFFSET) // Driver-owned buffer to receive more than the FIFO size before the received // data is consumed by rx_update. static circular_buffer rx_buf; // guarded by rx_mutex -// Returns whether the hardware says the receive FIFO is empty. -static bool rx_empty() { - return (REG32(UART_STATUS(0)) & (1 << UART_STATUS_RXEMPTY)) != 0; +// Driver-owned buffer to buffer more transmitted bytes than can fit in the +// transmit FIFO. +static circular_buffer tx_buf; // guarded by tx_mutex + +// Gets the number of unsent bytes in the TX FIFO from hardware MMIO. +static uint32_t tx_fifo_level() { + return SHIFT_DOWN_AND_MASK(REG(FIFO_STATUS), FIFO_STATUS, TXLVL); } -// Returns whether the hardware is ready to take another byte for transmit. -static bool tx_ready() { - return (REG32(UART_STATUS(0)) & (1 << UART_STATUS_TXFULL)) == 0; +// Gets the number of pending bytes in the RX FIFO from hardware MMIO. +static uint32_t rx_fifo_level() { + return SHIFT_DOWN_AND_MASK(REG(FIFO_STATUS), FIFO_STATUS, RXLVL); } // Reads one byte from the hardware read data register. @@ -51,13 +69,45 @@ static bool tx_ready() { // Callers should first ensure the receive FIFO is not empty rather than rely on // any particular magic value to indicate that. static char uart_getchar() { - return REG32(UART_RDATA(0)) & UART_RDATA_RDATA_MASK; + return SHIFT_DOWN_AND_MASK(REG(RDATA), RDATA, RDATA); } // Writes one byte to the hardware write data register. // // The byte will be dropped if the transmit FIFO is empty. -static void uart_putchar(char c) { REG32(UART_WDATA(0)) = c; } +static void uart_putchar(char c) { + REG(WDATA) = MASK_AND_SHIFT_UP(c, WDATA, WDATA); +} + +// Writes just enough of tx_buf to fill the transmit FIFO. +// +// This is called from all of tx_update, tx_watermark_handle, and +// tx_empty_handle. If tx_update has filled tx_buf to a size larger than the +// FIFO, interrupts will trigger and repeatedly until tx_buf is completely sent. +static void fill_tx_fifo() { + // Caps the number of bytes sent to a fixed constant, since otherwise an + // emulation reporting a very largo FIFO level could cause a long time to be + // spent in an interrupt handler. + uint32_t max_to_send = UART_FIFO_CAPACITY - tx_fifo_level(); + if (max_to_send > UART_FIFO_CAPACITY) { + max_to_send = UART_FIFO_CAPACITY; + } + + seL4_Assert(tx_mutex_lock() == 0); + uint32_t capacity = circular_buffer_size(&tx_buf); + for (uint32_t num_sent = 0; num_sent < max_to_send; ++num_sent) { + char c; + if (!circular_buffer_pop_front(&tx_buf, &c)) { + // The buffer is empty. + break; + } + uart_putchar(c); + } + if (circular_buffer_remaining(&tx_buf) > 0) { + seL4_Assert(tx_semaphore_post() == 0); + } + seL4_Assert(tx_mutex_unlock() == 0); +} // CAmkES initialization hook. // @@ -65,6 +115,10 @@ static void uart_putchar(char c) { REG32(UART_WDATA(0)) = c; } // // In short, sets 115200bps, TX and RX on, and TX watermark to 1. void pre_init() { + // Clears the driver-owned buffers. + circular_buffer_init(&tx_buf); + circular_buffer_init(&rx_buf); + // 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); @@ -73,16 +127,21 @@ void pre_init() { seL4_Assert(ctrl_nco < 0xffff); // Sets baud rate and enables TX and RX. - REG32(UART_CTRL(0)) = - ((ctrl_nco & UART_CTRL_NCO_MASK) << UART_CTRL_NCO_OFFSET) | - (1 << UART_CTRL_TX) | (1 << UART_CTRL_RX); + REG(CTRL) = MASK_AND_SHIFT_UP(ctrl_nco, CTRL, NCO) | BIT(UART_CTRL_TX) | + BIT(UART_CTRL_RX); // Resets TX and RX FIFOs. - uint32_t fifo_ctrl = REG32(UART_FIFO_CTRL(0)); - REG32(UART_FIFO_CTRL(0)) = - fifo_ctrl | UART_FIFO_CTRL_RXRST | UART_FIFO_CTRL_TXRST; + uint32_t fifo_ctrl = REG(FIFO_CTRL); + REG(FIFO_CTRL) = + fifo_ctrl | BIT(UART_FIFO_CTRL_RXRST) | BIT(UART_FIFO_CTRL_TXRST); - // Sets RX watermark to 1. + // Sets FIFO watermarks. + fifo_ctrl = REG(FIFO_CTRL); + // Clears old values of both watermarks. + fifo_ctrl = fifo_ctrl & + (~(UART_FIFO_CTRL_RXILVL_MASK << UART_FIFO_CTRL_RXILVL_OFFSET)) & + (~(UART_FIFO_CTRL_TXILVL_MASK << UART_FIFO_CTRL_TXILVL_OFFSET)); + // 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 @@ -95,45 +154,47 @@ void pre_init() { // Although a higher watermark in combination with rx_timeout might be // preferable, Renode simulation does not yet support the rx_timeout // interrupt. - 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; + fifo_ctrl = fifo_ctrl | MASK_AND_SHIFT_UP(UART_FIFO_CTRL_RXILVL_VALUE_RXLVL1, + FIFO_CTRL, RXILVL); + // TX watermark to 16 (half full). + fifo_ctrl = fifo_ctrl | MASK_AND_SHIFT_UP(UART_FIFO_CTRL_TXILVL_VALUE_TXLVL16, + FIFO_CTRL, TXILVL); + REG(FIFO_CTRL) = fifo_ctrl; // Enables interrupts. - REG32(UART_INTR_ENABLE(0)) = (1 << UART_INTR_COMMON_RX_WATERMARK); - - // TODO (mattharvey): Add tx_buf and tx_watermark_handle so that tx_update - // calls only need to block if tx_buf is full. - - circular_buffer_clear(&rx_buf); + REG(INTR_ENABLE) = ( + // TODO(mattharvey): Enable tx_watermark. Currently the handler fires + // repeatedly in a loop when doing this, even while the TX FIFO remains + // empty, contrary to the "edge triggered events" working in the OpenTitan + // docs. Check that Renode is doing the right thing and if so debug after + // re-enabling this. + // BIT(UART_INTR_COMMON_TX_WATERMARK) | + BIT(UART_INTR_COMMON_RX_WATERMARK) | BIT(UART_INTR_COMMON_TX_EMPTY)); } // Implements the update method of the CAmkES dataport_inf rx. // // Reads a given number of bytes from rx_buf into the CAmkES rx_dataport, // blocking the RPC until the entire requested byte count has been read. -void rx_update(size_t num_to_read) { - char *dataport_cursor = (char *)rx_dataport; +void rx_update(uint32_t num_to_read) { // TODO(mattharvey): Error return value for num_to_read > // TX_RX_DATAPORT_CAPACITY. seL4_Assert(num_to_read <= TX_RX_DATAPORT_CAPACITY); - size_t num_read = 0; - while (num_read < num_to_read) { - while (circular_buffer_empty(&rx_buf)) { - seL4_Assert(rx_semaphore_wait() == 0); - } + char *dataport_cursor = (char *)rx_dataport; + char *const dataport_end = dataport_cursor + num_to_read; + while (dataport_cursor < dataport_end) { seL4_Assert(rx_mutex_lock() == 0); - while (num_read < num_to_read && !circular_buffer_empty(&rx_buf)) { - char c; - if (!circular_buffer_pop_front(&rx_buf, &c)) { + while (circular_buffer_empty(&rx_buf)) { + seL4_Assert(rx_mutex_unlock() == 0); + seL4_Assert(rx_semaphore_wait() == 0); + seL4_Assert(rx_mutex_lock() == 0); + } + for (; dataport_cursor < dataport_end; ++dataport_cursor) { + if (!circular_buffer_pop_front(&rx_buf, dataport_cursor)) { // The buffer is empty. break; } - *(dataport_cursor++) = c; - ++num_read; } seL4_Assert(rx_mutex_unlock() == 0); } @@ -143,32 +204,70 @@ void rx_update(size_t num_to_read) { // // Writes the contents of the CAmkES tx_dataport to the UART, one at a time, // blocking the RPC until the entire requested number of bytes has been written. -void tx_update(size_t num_valid_dataport_bytes) { - char *c = (char *)tx_dataport; +void tx_update(uint32_t num_valid_dataport_bytes) { // TODO(mattharvey): Error return value for num_valid_dataport_bytes > // TX_RX_DATAPORT_CAPACITY. seL4_Assert(num_valid_dataport_bytes <= TX_RX_DATAPORT_CAPACITY); - for (size_t i = 0; i < num_valid_dataport_bytes; ++i) { - while (!tx_ready()) { - seL4_Yield(); + const char *dataport_cursor = (const char *)tx_dataport; + const char *const dataport_end = dataport_cursor + num_valid_dataport_bytes; + while (dataport_cursor < dataport_end) { + seL4_Assert(tx_mutex_lock() == 0); + while (circular_buffer_remaining(&tx_buf) == 0) { + seL4_Assert(tx_mutex_unlock() == 0); + seL4_Assert(tx_semaphore_wait() == 0); + seL4_Assert(tx_mutex_lock() == 0); } - uart_putchar(*c); - ++c; + for (; dataport_cursor < dataport_end; ++dataport_cursor) { + if (!circular_buffer_push_back(&tx_buf, *dataport_cursor)) { + // The buffer is full. + break; + } + } + seL4_Assert(tx_mutex_unlock() == 0); } + + if (tx_fifo_level() == 0) { + // If the FIFO is already empty, there is no interrupt coming, so we trigger + // the first transmission manually. + fill_tx_fifo(); + } +} + +// Handles a tx_watermark interrupt. +// +// These happen when the transmit FIFO is half-empty. This refills the FIFO to +// prevent stalling, stopping early if tx_buf becomes empty, and then signals +// any tx_update that might be waiting for tx_buf to not be full. +// +// Currently this is not actually enabled. See the TODO in the implementation of +// pre_init. +void tx_watermark_handle(void) { + fill_tx_fifo(); + + // Clears INTR_STATE for tx_watermark. (INTR_STATE is write-1-to-clear.) + REG(INTR_STATE) = BIT(UART_INTR_STATE_TX_WATERMARK); + + seL4_Assert(tx_watermark_acknowledge() == 0); } // Handles an rx_watermark interrupt. // -// Reads from the receive FIFO into rx_buf until rx_buf is full or the FIFO is -// empty, whichever happens first, and then signals any call to tx_update that -// may be waiting on the condition that rx_buf not be empty. +// Reads any bytes currently pending in the receive FIFO into rx_buf, stopping +// early if rx_buf becomes full and then signals any call to rx_update that may +// be waiting on the condition that rx_buf not be empty. void rx_watermark_handle(void) { - size_t num_read = 0; + // Set a constant cap on the number of bytes read to ensure the interrupt + // handler returns promptly, even on emulations that report an unusually large + // FIFO level. + uint32_t num_to_read = rx_fifo_level(); + if (num_to_read > UART_FIFO_CAPACITY) { + num_to_read = UART_FIFO_CAPACITY; + } + uint32_t num_read = 0; seL4_Assert(rx_mutex_lock() == 0); - size_t buf_remaining_size = circular_buffer_remaining(&rx_buf); - while (!rx_empty() && num_read < buf_remaining_size) { + while (num_read < num_to_read) { if (!circular_buffer_push_back(&rx_buf, uart_getchar())) { // The buffer is full. break; @@ -182,7 +281,21 @@ void rx_watermark_handle(void) { } // Clears INTR_STATE for rx_watermark. (INTR_STATE is write-1-to-clear.) - REG32(UART_INTR_STATE(0)) = (1 << UART_INTR_STATE_RX_WATERMARK); + REG(INTR_STATE) = BIT(UART_INTR_STATE_RX_WATERMARK); seL4_Assert(rx_watermark_acknowledge() == 0); } + +// Handles a tx_empty interrupt. +// +// This copies tx_buf into the hardware transmit FIFO, stopping early if tx_buf +// becomes empty, and then signals any tx_update that might be waiting for +// tx_buf to not be full. +void tx_empty_handle(void) { + fill_tx_fifo(); + + // Clears INTR_STATE for tx_empty. (INTR_STATE is write-1-to-clear.) + REG(INTR_STATE) = BIT(UART_INTR_STATE_TX_EMPTY); + + seL4_Assert(tx_empty_acknowledge() == 0); +} diff --git a/apps/system/system.camkes b/apps/system/system.camkes index 9754ef3..f3fa024 100644 --- a/apps/system/system.camkes +++ b/apps/system/system.camkes @@ -25,7 +25,9 @@ component OpenTitanUART { hardware; dataport Buf mmio_region; + emits Interrupt tx_watermark; emits Interrupt rx_watermark; + emits Interrupt tx_empty; } component VectorCoreHw { @@ -50,8 +52,12 @@ assembly { // OpenTitanUARTDriver connection seL4HardwareMMIO uart_mem(from uart_driver.mmio_region, to uart.mmio_region); + connection seL4HardwareInterrupt uart_tx_watermark(from uart.tx_watermark, + to uart_driver.tx_watermark); connection seL4HardwareInterrupt uart_rx_watermark(from uart.rx_watermark, to uart_driver.rx_watermark); + connection seL4HardwareInterrupt uart_tx_empty(from uart.tx_empty, + to uart_driver.tx_empty); // VectorCoreDriver connection seL4HardwareMMIO vc_csr(from vc_drv.csr, to vctop.csr); @@ -93,7 +99,9 @@ assembly { configuration { uart.mmio_region_paddr = 0x40030000; uart.mmio_region_size = 0x1000; + uart.tx_watermark_irq_number = 25; uart.rx_watermark_irq_number = 26; + uart.tx_empty_irq_number = 27; vctop.csr_paddr = 0x48000000; vctop.csr_size = 0x1000;