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

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

On Tue, 11 Mar 2025 at 10:33, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> On 10/3/25 18:28, Peter Maydell wrote:
> > This seems to be because the pl011 code and the chardev
> > code disagree about how "couldn't write anything" is
> > reported. pl011 here is looking for "0 means wrote nothing",
> > but the chardev code reports it as "-1 and errno is EAGAIN".
> >
> > I think the chardev code is actually what we need to fix here,
> > because it makes basically no effort to guarantee that the
> > errno from the underlying write is still in 'errno' by the
> > time qemu_chr_fe_write() returns. In particular it may
> > call qemu_chr_write_log() or replay_char_write_event_save(),
> > both of which will happily trash errno if something fails
> > during their execution.
>
> IIUC when retrying qemu_chr_write_buffer(s, buf, len, ofs) could
> write less than @len (but still writing few bytes, that information
> is stored in @offset) and return -errno, discarding @offset partial
> write len.

I thought that too when I first read the code, because it's
been written in a way that didn't match what I was expecting
for a "write until you would block, then stop" loop, but on
second reading I decided it could not, at least in the case
where write_all is false. If we write any data at all on the
first cc->chr_write call, we will then break out of the loop
without trying to send any more, so there's no code path
that will then set res to a negative number. (If write_all
is true, then yeah I think we can return an errno rather
than a partial-write count if chr_write fails on the second
time through the loop, but all the callers of that version
of the function ignore errors of any kind anyway, and
certainly don't have any means to handle "only wrote half
the data", otherwise they'd be using the non-blocking
version.)

-- PMM