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

[Alistair Francis]

On Wed, Sep 11, 2024 at 6:35 AM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> On 10/09/2024 05: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.
>
> Does the UART work reliably using the fifo8_*_bufptr() functions? One of the

I haven't noticed any issues.

> motivations for my recent Fifo8 series is that these functions don't handle 
> the
> wraparound correctly, unlike the fifo8_*_buf() functions in my recent Fifo8 
> series
> which do. This was the cause of Phil's async issue in
> https://mail.gnu.org/archive/html/qemu-devel/2024-07/msg05028.html.

I'm not sure if it matters here

    characters = fifo8_peek_bufptr(&s->tx_fifo,
                                   fifo8_num_used(&s->tx_fifo), &numptr);
    ret = qemu_chr_fe_write(&s->chr, characters, numptr);

    if (ret >= 0) {
        /* We wrote the data, actually pop the fifo */
        fifo8_pop_bufptr(&s->tx_fifo, ret, NULL);
    }

I don't care how many characters are returned from
fifo8_peek_bufptr(), I'll just write them and then pop that number
with fifo8_pop_bufptr()

If fifo8_is_empty() isn't empty I re-add the watcher for qemu_chr_fe_add_watch()

Alistair