mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

hw: replace most use of qemu_chr_fe_write with qemu_chr_fe_write_all

Browse Source 
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 <berrange@redhat.com>
Message-Id: <1473170165-540-4-git-send-email-berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Daniel P. Berrange 2016-09-06 14:56:04 +01:00 committed by Paolo Bonzini
parent 7983e82933
commit 6ab3fc32ea
29 changed files with 104 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
backends/rng-egd.c
View File

@ -41,7 +41,9 @@ static void rng_egd_request_entropy(RngBackend *b, RngRequest *req)
header[0] = 0x02;
header[1] = len;
qemu_chr_fe_write(s->chr, header, sizeof(header));
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
qemu_chr_fe_write_all(s->chr, header, sizeof(header));
size -= len;
}

4
gdbstub.c
View File

@ -402,7 +402,9 @@ static void put_buffer(GDBState *s, const uint8_t *buf, int len)
}
}
#else
qemu_chr_fe_write(s->chr, buf, len);
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
qemu_chr_fe_write_all(s->chr, buf, len);
#endif
}

8
hw/arm/omap2.c
View File

@ -769,14 +769,16 @@ static void omap_sti_fifo_write(void *opaque, hwaddr addr,
if (ch == STI_TRACE_CONTROL_CHANNEL) {
/* Flush channel <i>value</i>. */
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 <i>ch</i>. */
} 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);
}
}

4
hw/arm/pxa2xx.c
View File

@ -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);

4
hw/arm/strongarm.c
View File

@ -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;

4
hw/char/bcm2835_aux.c
View File

@ -169,7 +169,9 @@ static void bcm2835_aux_write(void *opaque, hwaddr offset, uint64_t value,
/* "DLAB bit set means access baudrate register" is NYI */
ch = value;
if (s->chr) {
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;

4
hw/char/debugcon.c
View File

@ -60,7 +60,9 @@ static void debugcon_ioport_write(void *opaque, hwaddr addr, uint64_t val,
printf(" [debugcon: write addr=0x%04" HWADDR_PRIx " val=0x%02" PRIx64 "]\n", addr, val);
#endif
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);
}

2
hw/char/digic-uart.c
View File

@ -77,6 +77,8 @@ static void digic_uart_write(void *opaque, hwaddr addr, uint64_t value,
switch (addr) {
case R_TX:
if (s->chr) {
/* 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;

4
hw/char/escc.c
View File

@ -557,7 +557,9 @@ static void escc_mem_write(void *opaque, hwaddr addr,
s->tx = val;
if (s->wregs[W_TXCTRL2] & TXCTRL2_TXEN) { // tx enabled
if (s->chr)
qemu_chr_fe_write(s->chr, &s->tx, 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, 1);
else if (s->type == kbd && !s->disabled) {
handle_kbd_command(s, val);
}

4
hw/char/etraxfs_ser.c
View File

@ -126,7 +126,9 @@ ser_write(void *opaque, hwaddr addr,
switch (addr)
{
case RW_DOUT:
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);
s->regs[R_INTR] |= 3;
s->pending_tx = 1;
s->regs[addr] = value;

4
hw/char/exynos4210_uart.c
View File

@ -387,7 +387,9 @@ static void exynos4210_uart_write(void *opaque, hwaddr offset,
s->reg[I_(UTRSTAT)] &= ~(UTRSTAT_TRANSMITTER_EMPTY |
UTRSTAT_Tx_BUFFER_EMPTY);
ch = (uint8_t)val;
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);
#if DEBUG_Tx_DATA
fprintf(stderr, "%c", ch);
#endif

4
hw/char/grlib_apbuart.c
View File

@ -203,7 +203,9 @@ static void grlib_apbuart_write(void *opaque, hwaddr addr,
/* Transmit when character device available and transmitter enabled */
if ((uart->chr) && (uart->control & UART_TRANSMIT_ENABLE)) {
c = value & 0xFF;
qemu_chr_fe_write(uart->chr, &c, 1);
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
qemu_chr_fe_write_all(uart->chr, &c, 1);
/* Generate interrupt */
if (uart->control & UART_TRANSMIT_INTERRUPT) {
qemu_irq_pulse(uart->irq);

4
hw/char/imx_serial.c
View File

@ -182,7 +182,9 @@ static void imx_serial_write(void *opaque, hwaddr offset,
ch = value;
if (s->ucr2 & UCR2_TXEN) {
if (s->chr) {
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);
}
s->usr1 &= ~USR1_TRDY;
imx_update(s);

4
hw/char/ipoctal232.c
View File

@ -360,7 +360,9 @@ static void io_write(IPackDevice *ip, uint8_t addr, uint16_t val)
DPRINTF("Write THR%c (0x%x)\n", channel + 'a', reg);
if (ch->dev) {
uint8_t thr = reg;
qemu_chr_fe_write(ch->dev, &thr, 1);
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
qemu_chr_fe_write_all(ch->dev, &thr, 1);
}
} else {
DPRINTF("Write THR%c (0x%x), Tx disabled\n", channel + 'a', reg);

2
hw/char/lm32_juart.c
View File

@ -76,6 +76,8 @@ void lm32_juart_set_jtx(DeviceState *d, uint32_t jtx)
s->jtx = jtx;
if (s->chr) {
/* 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);
}
}

2
hw/char/lm32_uart.c
View File

@ -178,6 +178,8 @@ static void uart_write(void *opaque, hwaddr addr,
switch (addr) {
case R_RXTX:
if (s->chr) {
/* 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;

4
hw/char/mcf_uart.c
View File

@ -114,7 +114,9 @@ static void mcf_uart_do_tx(mcf_uart_state *s)
{
if (s->tx_enabled && (s->sr & MCF_UART_TxEMP) == 0) {
if (s->chr)
qemu_chr_fe_write(s->chr, (unsigned char *)&s->tb, 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, (unsigned char *)&s->tb, 1);
s->sr |= MCF_UART_TxEMP;
}
if (s->tx_enabled) {

4
hw/char/parallel.c
View File

@ -129,7 +129,9 @@ parallel_ioport_write_sw(void *opaque, uint32_t addr, uint32_t val)
if (val & PARA_CTR_STROBE) {
s->status &= ~PARA_STS_BUSY;
if ((s->control & PARA_CTR_STROBE) == 0)
qemu_chr_fe_write(s->chr, &s->dataw, 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->dataw, 1);
} else {
if (s->control & PARA_CTR_INTEN) {
s->irq_pending = 1;

4
hw/char/pl011.c
View File

@ -146,7 +146,9 @@ static void pl011_write(void *opaque, hwaddr offset,
/* ??? Check if transmitter is enabled. */
ch = value;
if (s->chr)
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);
s->int_level |= PL011_INT_TX;
pl011_update(s);
break;

4
hw/char/sclpconsole-lm.c
View File

@ -89,7 +89,9 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
scon->buf[scon->length] = *buf;
scon->length += 1;
if (scon->echo) {
qemu_chr_fe_write(scon->chr, buf, size);
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
qemu_chr_fe_write_all(scon->chr, buf, size);
}
}

2
hw/char/sclpconsole.c
View File

@ -168,6 +168,8 @@ static ssize_t write_console_data(SCLPEvent *event, const uint8_t *buf,
return len;
}
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
return qemu_chr_fe_write_all(scon->chr, buf, len);
}

4
hw/char/sh_serial.c
View File

@ -111,7 +111,9 @@ static void sh_serial_write(void *opaque, hwaddr offs,
case 0x0c: /* FTDR / TDR */
if (s->chr) {
ch = val;
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);
}
s->dr = val;
s->flags &= ~SH_SERIAL_FLAG_TDE;

5
hw/char/spapr_vty.c
View File

@ -60,8 +60,9 @@ void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, int len)
{
VIOsPAPRVTYDevice *dev = VIO_SPAPR_VTY_DEVICE(sdev);
/* FIXME: should check the qemu_chr_fe_write() return value */
qemu_chr_fe_write(dev->chardev, buf, len);
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
qemu_chr_fe_write_all(dev->chardev, buf, len);
}
static void spapr_vty_realize(VIOsPAPRDevice *sdev, Error **errp)

2
hw/char/stm32f2xx_usart.c
View File

@ -153,6 +153,8 @@ static void stm32f2xx_usart_write(void *opaque, hwaddr addr,
if (value < 0xF000) {
ch = value;
if (s->chr) {
/* 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);
}
s->usart_sr |= USART_SR_TC;

21
hw/char/virtio-console.c
View File

@ -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.
*/
if (!k->is_console) {
virtio_serial_throttle_port(port, true);
if (!vcon->watch) {

4
hw/char/xilinx_uartlite.c
View File

@ -144,7 +144,9 @@ uart_write(void *opaque, hwaddr addr,
case R_TX:
if (s->chr)
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);
s->regs[addr] = value;

7
hw/usb/ccid-card-passthru.c
View File

@ -75,8 +75,11 @@ static void ccid_card_vscard_send_msg(PassthruState *s,
scr_msg_header.type = htonl(type);
scr_msg_header.reader_id = htonl(reader_id);
scr_msg_header.length = htonl(length);
qemu_chr_fe_write(s->cs, (uint8_t *)&scr_msg_header, sizeof(VSCMsgHeader));
qemu_chr_fe_write(s->cs, payload, length);
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
qemu_chr_fe_write_all(s->cs, (uint8_t *)&scr_msg_header,
sizeof(VSCMsgHeader));
qemu_chr_fe_write_all(s->cs, payload, length);
}
static void ccid_card_vscard_send_apdu(PassthruState *s,

4
hw/usb/dev-serial.c
View File

@ -366,7 +366,9 @@ static void usb_serial_handle_data(USBDevice *dev, USBPacket *p)
goto fail;
for (i = 0; i < p->iov.niov; i++) {
iov = p->iov.iov + i;
qemu_chr_fe_write(s->cs, iov->iov_base, iov->iov_len);
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
qemu_chr_fe_write_all(s->cs, iov->iov_base, iov->iov_len);
}
p->actual_length = p->iov.size;
break;

4
slirp/slirp.c
View File

@ -1072,7 +1072,9 @@ int slirp_add_exec(Slirp *slirp, int do_pty, const void *args,
ssize_t slirp_send(struct socket *so, const void *buf, size_t len, int flags)
{
if (so->s == -1 && so->extra) {
qemu_chr_fe_write(so->extra, buf, len);
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
qemu_chr_fe_write_all(so->extra, buf, len);
return len;
}