Re: [PATCH v6 0/7] hw/char/pl011: Implement TX (async) FIFO to avoid blocking the main loop

[Peter Maydell <peter.maydell@linaro.org>]

On Thu, 20 Feb 2025 at 10:52, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 20 Feb 2025 at 10:43, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Tue, 18 Feb 2025 at 13:54, Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > On Mon, 17 Feb 2025 at 14:55, Peter Maydell <peter.maydell@linaro.org> wrote:
> > > >
> > > > On Sat, 8 Feb 2025 at 16:39, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > This series add support for (async) FIFO on the transmit path
> > > > > of the PL011 UART.
> > > > >
> > > >
> > > > Applied to target-arm.next, thanks (with a couple of minor
> > > > tweaks to two of the patches).
> > >
> > > Unfortunately I seem to get failures in 'make check-functional'
> > > with the last patch of this series applied.
> >
> > I had a look at this this morning because I wondered if it
> > was a mistake in the style fixups I'd applied to the patches
> > on my end, and I found the bug fairly quickly. The problem is
> > that pl011_xmit() doesn't update the TXFE and TXFF FIFO empty/full
> > status flag bits when it removes characters from the FIFO.
> > So the guest kernel spins forever because TXFF is never unset.
> >
> > The following patch fixes this for me (and also makes us not
> > set INT_TX for the case where we couldn't send any bytes to
> > the chardev, which I noticed reading the code rather than
> > because it had any visible bad effects):
>
> Hmm, but that's clearly not the only problem -- it fixed the
> "no output at all issue", but now I see a test failure because
> of garbled console output:

> I also noticed that pl011_write_txdata() doesn't clear TXFE
> when it puts a byte into the fifo -- I'm testing to see if
> fixing that helps.

Yes, with this patch also:

--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -330,6 +330,7 @@ static void pl011_write_txdata(PL011State *s, uint8_t data)
     if (pl011_is_tx_fifo_full(s)) {
         s->flags |= PL011_FLAG_TXFF;
     }
+    s->flags &= ~PL011_FLAG_TXFE;

     pl011_xmit(NULL, G_IO_OUT, s);
 }

this remaining failure is fixed and I get a clean pass (other than
the gpu test failure, but that's not related to the pl011).

-- PMM