On Wed, 3 Aug 2016 17:22:39 +0100 "Daniel P. Berrange" <berra...@redhat.com> wrote:
> The qemu_chr_fe_write method will return -1 on EAGAIN if the > chardev backend write would block. Almost no callers of the > qemu_chr_fe_write() method check the return value, instead > blindly assuming data was successfully sent. In most cases > this will lead to silent data loss on interactive consoles, > but in some cases (eg RNG EGD) it'll just cause corruption > of the protocol being spoken. > > We unfortunately can't fix the virtio-console code, due to > a bug in the Linux guest drivers, which would cause the > entire Linux kernel to hang if we delay processing of the > incoming data in any way. Fixing this requires first fixing > the guest driver to not hold spinlocks while writing to the > hvc device backend. > > Fixes bug: https://bugs.launchpad.net/qemu/+bug/1586756 > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > backends/rng-egd.c | 4 +++- > gdbstub.c | 4 +++- > hw/arm/omap2.c | 8 +++++--- > hw/arm/pxa2xx.c | 4 +++- > hw/arm/strongarm.c | 4 +++- > hw/char/bcm2835_aux.c | 4 +++- > hw/char/debugcon.c | 4 +++- > hw/char/digic-uart.c | 2 ++ > hw/char/escc.c | 4 +++- > hw/char/etraxfs_ser.c | 4 +++- > hw/char/exynos4210_uart.c | 4 +++- > hw/char/grlib_apbuart.c | 4 +++- > hw/char/imx_serial.c | 4 +++- > hw/char/ipoctal232.c | 4 +++- > hw/char/lm32_juart.c | 2 ++ > hw/char/lm32_uart.c | 2 ++ > hw/char/mcf_uart.c | 4 +++- > hw/char/parallel.c | 4 +++- > hw/char/pl011.c | 4 +++- > hw/char/sclpconsole-lm.c | 4 +++- > hw/char/sclpconsole.c | 2 ++ ack for the sclp consoles > hw/char/sh_serial.c | 4 +++- > hw/char/spapr_vty.c | 5 +++-- > hw/char/stm32f2xx_usart.c | 2 ++ > hw/char/virtio-console.c | 21 +++++++++++++++++++++ > hw/char/xilinx_uartlite.c | 4 +++- > hw/usb/ccid-card-passthru.c | 7 +++++-- > hw/usb/dev-serial.c | 4 +++- > slirp/slirp.c | 4 +++- > 29 files changed, 104 insertions(+), 27 deletions(-) > > diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c > index 4f0e03d..d44c18c 100644 > --- a/hw/char/virtio-console.c > +++ b/hw/char/virtio-console.c > @@ -68,6 +68,27 @@ static ssize_t flush_buf(VirtIOSerialPort *port, > */ > if (ret < 0) > ret = 0; > + > + /* XXX we should be queuing data to send later for the > + * console devices too rather than silently dropping > + * console data on EAGAIN. The Linux virtio-console > + * hvc driver though does sends with spinlocks held, > + * so if we enable throttling that'll stall the entire > + * guest kernel, not merely the process writing to the > + * console. > + * > + * While we could queue data for later write without > + * enabling throttling, this would result in the guest > + * being able to trigger arbitrary memory usage in QEMU > + * buffering data for later writes. > + * > + * So fixing this problem likely requires fixing the > + * Linux virtio-console hvc driver to not hold spinlocks > + * while writing, and instead merely block the process > + * that's writing. QEMU would then need some way to detect > + * if the guest had the fixed driver too, before we can > + * use throttling on host side. Probably best done via a new virtio-console feature bit, as this would be OS agnostic. > + */ > if (!k->is_console) { > virtio_serial_throttle_port(port, true); > if (!vcon->watch) {
