Re: [PATCH v4 2/2] hw/char: sifive_uart: Print uart characters async

[Philippe Mathieu-Daudé]

On 10/9/24 06:54, Alistair Francis wrote:
> The current approach of using qemu_chr_fe_write() and ignoring the
> return values results in dropped characters [1].
>
> Let's update the SiFive UART to use a async sifive_uart_xmit() function
> to transmit the characters and apply back pressure to the guest with
> the SIFIVE_UART_TXFIFO_FULL status.
>
> This should avoid dropped characters and more realisticly model the
> hardware.
>
> 1: https://gitlab.com/qemu-project/qemu/-/issues/2114
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Tested-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/hw/char/sifive_uart.h | 16 +++++-
>   hw/char/sifive_uart.c         | 94 ++++++++++++++++++++++++++++++++---
>   2 files changed, 102 insertions(+), 8 deletions(-)


> +static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
> +                                      int size)
> +{
> +    uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +
> +    if (size > fifo8_num_free(&s->tx_fifo)) {
> +        size = fifo8_num_free(&s->tx_fifo);
> +        qemu_log_mask(LOG_GUEST_ERROR, "sifive_uart: TX FIFO overflow");
> +    }
> +
> +    fifo8_push_all(&s->tx_fifo, buf, size);
> +
> +    if (fifo8_is_full(&s->tx_fifo)) {
> +        s->txfifo |= SIFIVE_UART_TXFIFO_FULL;
> +    }
> +
> +    timer_mod(s->fifo_trigger_handle, current_time + 100);

Preferably using a #define instead of this magic '100' value (no
need to repost):

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +}