On Thu, 20 Feb 2020 at 09:10, Marc Zyngier <m...@kernel.org> wrote: > > On 2020-02-20 06:01, Gavin Shan wrote: > > This fixes the issue by using newly added API > > qemu_chr_fe_try_write_all(), > > which provides another type of service (best-effort). It's different > > from > > qemu_chr_fe_write_all() as the data will be dropped if the backend has > > been running into so-called broken state or 50 attempts of > > transmissions. > > The broken state is cleared if the data is transmitted at once. > > I don't think dropping the serial port output is an acceptable outcome.
Agreed. The correct fix for this is the one cryptically described in the XXX comment this patch deletes: - /* XXX this blocks entire thread. Rewrite to use - * qemu_chr_fe_write and background I/O callbacks */ The idea is that essentially we end up emulating the real hardware's transmit FIFO: * as data arrives from the guest we put it in the FIFO * we try to send the data with qemu_chr_fe_write(), which does not block * if qemu_chr_fe_write() tells us it did not send all the data, we use qemu_chr_fe_add_watch() to set up an I/O callback which will get called when the output chardev has drained enough that we can try again * we make sure all the guest visible registers and mechanisms for tracking tx fifo level (status bits, interrupts, etc) are correctly wired up Then we don't lose data or block QEMU if the guest sends faster than the chardev backend can handle, assuming the guest is well-behaved -- just as with a real hardware slow serial port, the guest will fill the tx fifo and then either poll or wait for an interrupt telling it that the fifo has drained before it tries to send more data. There is an example of this in hw/char/cadence_uart.c (and an example of how it works for a UART with no tx fifo in hw/char-cmsdk-apb-uart.c, which is basically the same except the 'fifo' is just one byte.) You will also find an awful lot of XXX comments like the above one in various UART models in hw/char, because converting an old-style simple blocking UART implementation to a non-blocking one is a bit fiddly and needs knowledge of the specifics of the UART behaviour. The other approach here would be that we could add options to relevant chardev backends so the user could say "if you couldn't connect to the tcp server I specified, throw away data rather than waiting", where we don't have suitable options already. If the user specifically tells us they're ok to throw away the serial data, then it's fine to throw away the serial data :-) thanks -- PMM
