Re: [PATCH] hw/char: disable ibex uart receive if the buffer is full

[Alexander Wagner]

ping

https://patchew.org/QEMU/20210215231528.2718086-1-alexander.wagner@ulal.de/

On 16.02.21 00:15, Alexander Wagner wrote:
> Not disabling the UART leads to QEMU overwriting the UART receive buffer with
> the newest received byte. The rx_level variable is added to allow the use of
> the existing OpenTitan driver libraries.
>
> Signed-off-by: Alexander Wagner <alexander.wagner@ulal.de>
> ---
>   hw/char/ibex_uart.c         | 20 +++++++++++++++-----
>   include/hw/char/ibex_uart.h |  4 ++++
>   2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
> index 89f1182c9b..dac09d53d6 100644
> --- a/hw/char/ibex_uart.c
> +++ b/hw/char/ibex_uart.c
> @@ -66,7 +66,8 @@ static int ibex_uart_can_receive(void *opaque)
>   {
>       IbexUartState *s = opaque;
>   
> -    if (s->uart_ctrl & R_CTRL_RX_ENABLE_MASK) {
> +    if ((s->uart_ctrl & R_CTRL_RX_ENABLE_MASK)
> +           && !(s->uart_status & R_STATUS_RXFULL_MASK)) {
>           return 1;
>       }
>   
> @@ -83,6 +84,8 @@ static void ibex_uart_receive(void *opaque, const uint8_t *buf, int size)
>   
>       s->uart_status &= ~R_STATUS_RXIDLE_MASK;
>       s->uart_status &= ~R_STATUS_RXEMPTY_MASK;
> +    s->uart_status |= R_STATUS_RXFULL_MASK;
> +    s->rx_level += 1;
>   
>       if (size > rx_fifo_level) {
>           s->uart_intr_state |= R_INTR_STATE_RX_WATERMARK_MASK;
> @@ -199,6 +202,7 @@ static void ibex_uart_reset(DeviceState *dev)
>       s->uart_timeout_ctrl = 0x00000000;
>   
>       s->tx_level = 0;
> +    s->rx_level = 0;
>   
>       s->char_tx_time = (NANOSECONDS_PER_SECOND / 230400) * 10;
>   
> @@ -243,11 +247,15 @@ static uint64_t ibex_uart_read(void *opaque, hwaddr addr,
>   
>       case R_RDATA:
>           retvalue = s->uart_rdata;
> -        if (s->uart_ctrl & R_CTRL_RX_ENABLE_MASK) {
> +        if ((s->uart_ctrl & R_CTRL_RX_ENABLE_MASK) && (s->rx_level > 0)) {
>               qemu_chr_fe_accept_input(&s->chr);
>   
> -            s->uart_status |= R_STATUS_RXIDLE_MASK;
> -            s->uart_status |= R_STATUS_RXEMPTY_MASK;
> +            s->rx_level -= 1;
> +            s->uart_status &= ~R_STATUS_RXFULL_MASK;
> +            if (s->rx_level == 0) {
> +                s->uart_status |= R_STATUS_RXIDLE_MASK;
> +                s->uart_status |= R_STATUS_RXEMPTY_MASK;
> +            }
>           }
>           break;
>       case R_WDATA:
> @@ -261,7 +269,8 @@ static uint64_t ibex_uart_read(void *opaque, hwaddr addr,
>       case R_FIFO_STATUS:
>           retvalue = s->uart_fifo_status;
>   
> -        retvalue |= s->tx_level & 0x1F;
> +        retvalue |= (s->rx_level & 0x1F) << R_FIFO_STATUS_RXLVL_SHIFT;
> +        retvalue |= (s->tx_level & 0x1F) << R_FIFO_STATUS_TXLVL_SHIFT;
>   
>           qemu_log_mask(LOG_UNIMP,
>                         "%s: RX fifos are not supported\n", __func__);
> @@ -364,6 +373,7 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
>           s->uart_fifo_ctrl = value;
>   
>           if (value & R_FIFO_CTRL_RXRST_MASK) {
> +            s->rx_level = 0;
>               qemu_log_mask(LOG_UNIMP,
>                             "%s: RX fifos are not supported\n", __func__);
>           }
> diff --git a/include/hw/char/ibex_uart.h b/include/hw/char/ibex_uart.h
> index 03d19e3f6f..546f958eb8 100644
> --- a/include/hw/char/ibex_uart.h
> +++ b/include/hw/char/ibex_uart.h
> @@ -62,6 +62,8 @@ REG32(FIFO_CTRL, 0x1c)
>       FIELD(FIFO_CTRL, RXILVL, 2, 3)
>       FIELD(FIFO_CTRL, TXILVL, 5, 2)
>   REG32(FIFO_STATUS, 0x20)
> +    FIELD(FIFO_STATUS, TXLVL, 0, 5)
> +    FIELD(FIFO_STATUS, RXLVL, 16, 5)
>   REG32(OVRD, 0x24)
>   REG32(VAL, 0x28)
>   REG32(TIMEOUT_CTRL, 0x2c)
> @@ -82,6 +84,8 @@ struct IbexUartState {
>       uint8_t tx_fifo[IBEX_UART_TX_FIFO_SIZE];
>       uint32_t tx_level;
>   
> +    uint32_t rx_level;
> +
>       QEMUTimer *fifo_trigger_handle;
>       uint64_t char_tx_time;
>