From 6ab3fc32ea640026726bc5f9f4db622d0954fb8a Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 6 Sep 2016 14:56:04 +0100 Subject: hw: replace most use of qemu_chr_fe_write with qemu_chr_fe_write_all 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 Message-Id: <1473170165-540-4-git-send-email-berrange@redhat.com> Signed-off-by: Paolo Bonzini --- hw/arm/omap2.c | 8 +++++--- hw/arm/pxa2xx.c | 4 +++- hw/arm/strongarm.c | 4 +++- 3 files changed, 11 insertions(+), 5 deletions(-) (limited to 'hw/arm') diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c index 3a0d77714a..7e11c65cba 100644 --- a/hw/arm/omap2.c +++ b/hw/arm/omap2.c @@ -769,14 +769,16 @@ static void omap_sti_fifo_write(void *opaque, hwaddr addr, if (ch == STI_TRACE_CONTROL_CHANNEL) { /* Flush channel value. */ - qemu_chr_fe_write(s->chr, (const uint8_t *) "\r", 1); + /* XXX this blocks entire thread. Rewrite to use + * qemu_chr_fe_write and background I/O callbacks */ + qemu_chr_fe_write_all(s->chr, (const uint8_t *) "\r", 1); } else if (ch == STI_TRACE_CONSOLE_CHANNEL || 1) { if (value == 0xc0 || value == 0xc3) { /* Open channel ch. */ } else if (value == 0x00) - qemu_chr_fe_write(s->chr, (const uint8_t *) "\n", 1); + qemu_chr_fe_write_all(s->chr, (const uint8_t *) "\n", 1); else - qemu_chr_fe_write(s->chr, &byte, 1); + qemu_chr_fe_write_all(s->chr, &byte, 1); } } diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c index cb55704687..0241e07d84 100644 --- a/hw/arm/pxa2xx.c +++ b/hw/arm/pxa2xx.c @@ -1903,7 +1903,9 @@ static void pxa2xx_fir_write(void *opaque, hwaddr addr, else ch = ~value; if (s->chr && s->enable && (s->control[0] & (1 << 3))) /* TXE */ - qemu_chr_fe_write(s->chr, &ch, 1); + /* XXX this blocks entire thread. Rewrite to use + * qemu_chr_fe_write and background I/O callbacks */ + qemu_chr_fe_write_all(s->chr, &ch, 1); break; case ICSR0: s->status[0] &= ~(value & 0x66); diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c index f1b2c6c966..021cbf9a0f 100644 --- a/hw/arm/strongarm.c +++ b/hw/arm/strongarm.c @@ -1108,7 +1108,9 @@ static void strongarm_uart_tx(void *opaque) if (s->utcr3 & UTCR3_LBM) /* loopback */ { strongarm_uart_receive(s, &s->tx_fifo[s->tx_start], 1); } else if (s->chr) { - qemu_chr_fe_write(s->chr, &s->tx_fifo[s->tx_start], 1); + /* XXX this blocks entire thread. Rewrite to use + * qemu_chr_fe_write and background I/O callbacks */ + qemu_chr_fe_write_all(s->chr, &s->tx_fifo[s->tx_start], 1); } s->tx_start = (s->tx_start + 1) % 8; -- cgit v1.2.3