From 807464d8a7326e1025a2f392bf41636b0809d6da Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 14 Jun 2016 14:17:16 +0200 Subject: serial: make tsr_retry unsigned It can never become negative; reflect this in the type of the field and simplify the conditions. Tested-by: Bret Ketchum Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Paolo Bonzini --- hw/char/serial.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'hw/char') diff --git a/hw/char/serial.c b/hw/char/serial.c index 6d815b5c69..e65e9e0b4c 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -229,7 +229,7 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque) do { assert(!(s->lsr & UART_LSR_TEMT)); - if (s->tsr_retry <= 0) { + if (s->tsr_retry == 0) { assert(!(s->lsr & UART_LSR_THRE)); if (s->fcr & UART_FCR_FE) { @@ -252,7 +252,7 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque) /* in loopback mode, say that we just received a char */ serial_receive1(s, &s->tsr, 1); } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) { - if (s->tsr_retry >= 0 && s->tsr_retry < MAX_XMIT_RETRY && + if (s->tsr_retry < MAX_XMIT_RETRY && qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, serial_xmit, s) > 0) { s->tsr_retry++; @@ -330,7 +330,7 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val, s->lsr &= ~UART_LSR_THRE; s->lsr &= ~UART_LSR_TEMT; serial_update_irq(s); - if (s->tsr_retry <= 0) { + if (s->tsr_retry == 0) { serial_xmit(NULL, G_IO_OUT, s); } } @@ -639,6 +639,10 @@ static int serial_post_load(void *opaque, int version_id) if (s->thr_ipending == -1) { s->thr_ipending = ((s->iir & UART_IIR_ID) == UART_IIR_THRI); } + if (s->tsr_retry > MAX_XMIT_RETRY) { + s->tsr_retry = MAX_XMIT_RETRY; + } + s->last_break_enable = (s->lcr >> 6) & 1; /* Initialize fcr via setter to perform essential side-effects */ serial_write_fcr(s, s->fcr_vmstate); @@ -685,7 +689,7 @@ static const VMStateDescription vmstate_serial_tsr = { .minimum_version_id = 1, .needed = serial_tsr_needed, .fields = (VMStateField[]) { - VMSTATE_INT32(tsr_retry, SerialState), + VMSTATE_UINT32(tsr_retry, SerialState), VMSTATE_UINT8(thr, SerialState), VMSTATE_UINT8(tsr, SerialState), VMSTATE_END_OF_LIST() -- cgit v1.2.3-55-g7522 From bce933b85a34514fe34fa559be1d8ccd1f39f954 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 14 Jun 2016 14:20:50 +0200 Subject: serial: simplify tsr_retry reset Move common code outside the if, and reset tsr_retry even in loopback mode. Right now it cannot become non-zero, but it will be possible as soon as we start respecting the baud rate. Tested-by: Bret Ketchum Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Paolo Bonzini --- hw/char/serial.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'hw/char') diff --git a/hw/char/serial.c b/hw/char/serial.c index e65e9e0b4c..904b218c21 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -258,10 +258,8 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque) s->tsr_retry++; return FALSE; } - s->tsr_retry = 0; - } else { - s->tsr_retry = 0; } + s->tsr_retry = 0; /* Transmit another byte if it is already available. It is only possible when FIFO is enabled and not empty. */ -- cgit v1.2.3-55-g7522 From b0585e7e07982daa578c3bfef7f6843c89f110a8 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 20 Jun 2016 15:08:20 +0200 Subject: serial: separate serial_xmit and serial_watch_cb serial_xmit starts transmission of whatever is in the transmitter register, THR or FIFO; serial_watch_cb is a wrapper around it and is only used as a qemu_chr_fe_add_watch callback. Tested-by: Bret Ketchum Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Paolo Bonzini --- hw/char/serial.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'hw/char') diff --git a/hw/char/serial.c b/hw/char/serial.c index 904b218c21..0b09094c0c 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -106,6 +106,7 @@ do {} while (0) #endif static void serial_receive1(void *opaque, const uint8_t *buf, int size); +static void serial_xmit(SerialState *s); static inline void recv_fifo_put(SerialState *s, uint8_t chr) { @@ -223,10 +224,16 @@ static void serial_update_msl(SerialState *s) } } -static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque) +static gboolean serial_watch_cb(GIOChannel *chan, GIOCondition cond, + void *opaque) { SerialState *s = opaque; + serial_xmit(s); + return FALSE; +} +static void serial_xmit(SerialState *s) +{ do { assert(!(s->lsr & UART_LSR_TEMT)); if (s->tsr_retry == 0) { @@ -254,9 +261,9 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque) } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) { if (s->tsr_retry < MAX_XMIT_RETRY && qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, - serial_xmit, s) > 0) { + serial_watch_cb, s) > 0) { s->tsr_retry++; - return FALSE; + return; } } s->tsr_retry = 0; @@ -267,11 +274,8 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque) s->last_xmit_ts = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); s->lsr |= UART_LSR_TEMT; - - return FALSE; } - /* Setter for FCR. is_load flag means, that value is set while loading VM state and interrupt should not be invoked */ @@ -329,7 +333,7 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val, s->lsr &= ~UART_LSR_TEMT; serial_update_irq(s); if (s->tsr_retry == 0) { - serial_xmit(NULL, G_IO_OUT, s); + serial_xmit(s); } } break; -- cgit v1.2.3-55-g7522 From 6f1de6b70d857d5e316ae6fd908f52818b827b08 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 20 Jun 2016 15:02:40 +0200 Subject: char: change qemu_chr_fe_add_watch to return unsigned g_source_attach can return any value between 1 and UINT_MAX if you let QEMU run long enough. However, qemu_chr_fe_add_watch can also return a negative errno value when the device is disconnected or does not support chr_add_watch. Change it to return zero to avoid overloading these values. Fix the cadence_uart which asserts in this case (easily obtained with "-serial pty"). Tested-by: Bret Ketchum Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Paolo Bonzini --- hw/char/cadence_uart.c | 9 ++++++--- include/sysemu/char.h | 16 ++++++++++++++-- net/vhost-user.c | 2 +- qemu-char.c | 8 ++++---- 4 files changed, 25 insertions(+), 10 deletions(-) (limited to 'hw/char') diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c index c856fc30b2..65179fa500 100644 --- a/hw/char/cadence_uart.c +++ b/hw/char/cadence_uart.c @@ -292,9 +292,12 @@ static gboolean cadence_uart_xmit(GIOChannel *chan, GIOCondition cond, memmove(s->tx_fifo, s->tx_fifo + ret, s->tx_count); if (s->tx_count) { - int r = qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, - cadence_uart_xmit, s); - assert(r); + guint r = qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, + cadence_uart_xmit, s); + if (!r) { + s->tx_count = 0; + return FALSE; + } } uart_update_status(s); diff --git a/include/sysemu/char.h b/include/sysemu/char.h index 1eb2d0f309..57df10aa00 100644 --- a/include/sysemu/char.h +++ b/include/sysemu/char.h @@ -221,8 +221,20 @@ void qemu_chr_fe_event(CharDriverState *s, int event); void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...) GCC_FMT_ATTR(2, 3); -int qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond, - GIOFunc func, void *user_data); +/** + * @qemu_chr_fe_add_watch: + * + * If the backend is connected, create and add a #GSource that fires + * when the given condition (typically G_IO_OUT|G_IO_HUP or G_IO_HUP) + * is active; return the #GSource's tag. If it is disconnected, + * return 0. + * + * @cond the condition to poll for + * @func the function to call when the condition happens + * @user_data the opaque pointer to pass to @func + */ +guint qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond, + GIOFunc func, void *user_data); /** * @qemu_chr_fe_write: diff --git a/net/vhost-user.c b/net/vhost-user.c index d72ce9b490..636899a877 100644 --- a/net/vhost-user.c +++ b/net/vhost-user.c @@ -22,7 +22,7 @@ typedef struct VhostUserState { NetClientState nc; CharDriverState *chr; VHostNetState *vhost_net; - int watch; + guint watch; uint64_t acked_features; } VhostUserState; diff --git a/qemu-char.c b/qemu-char.c index 84f49acbac..8575c54c98 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -3966,19 +3966,19 @@ void qemu_chr_fe_event(struct CharDriverState *chr, int event) } } -int qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond, - GIOFunc func, void *user_data) +guint qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond, + GIOFunc func, void *user_data) { GSource *src; guint tag; if (s->chr_add_watch == NULL) { - return -ENOSYS; + return 0; } src = s->chr_add_watch(s, cond); if (!src) { - return -EINVAL; + return 0; } g_source_set_callback(src, (GSourceFunc)func, user_data, NULL); -- cgit v1.2.3-55-g7522 From a1df76da57aa8772a75e7c49f8e3829d07b4c46c Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 14 Jun 2016 14:35:20 +0200 Subject: serial: remove watch on reset Otherwise, this can cause serial_xmit to be entered with LSR.TEMT=0, which is invalid and causes an assertion failure. Reported-by: Bret Ketchum Tested-by: Bret Ketchum Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Paolo Bonzini --- hw/char/serial.c | 16 ++++++++++++---- include/hw/char/serial.h | 1 + 2 files changed, 13 insertions(+), 4 deletions(-) (limited to 'hw/char') diff --git a/hw/char/serial.c b/hw/char/serial.c index 0b09094c0c..af39e8f867 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -228,6 +228,7 @@ static gboolean serial_watch_cb(GIOChannel *chan, GIOCondition cond, void *opaque) { SerialState *s = opaque; + s->watch_tag = 0; serial_xmit(s); return FALSE; } @@ -258,10 +259,12 @@ static void serial_xmit(SerialState *s) if (s->mcr & UART_MCR_LOOP) { /* in loopback mode, say that we just received a char */ serial_receive1(s, &s->tsr, 1); - } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) { - if (s->tsr_retry < MAX_XMIT_RETRY && - qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, - serial_watch_cb, s) > 0) { + } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1 && + s->tsr_retry < MAX_XMIT_RETRY) { + assert(s->watch_tag == 0); + s->watch_tag = qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, + serial_watch_cb, s); + if (s->watch_tag > 0) { s->tsr_retry++; return; } @@ -821,6 +824,11 @@ static void serial_reset(void *opaque) { SerialState *s = opaque; + if (s->watch_tag > 0) { + g_source_remove(s->watch_tag); + s->watch_tag = 0; + } + s->rbr = 0; s->ier = 0; s->iir = UART_IIR_NO_INT; diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h index 6a322eb22e..9feddc613c 100644 --- a/include/hw/char/serial.h +++ b/include/hw/char/serial.h @@ -56,6 +56,7 @@ struct SerialState { int it_shift; int baudbase; uint32_t tsr_retry; + guint watch_tag; uint32_t wakeup; /* Time when the last byte was successfully sent out of the tsr */ -- cgit v1.2.3-55-g7522 From 9f34a35e0020b0b2b2e21c086a486d7dfd18df4f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 14 Jun 2016 14:46:51 +0200 Subject: serial: reinstate watch after migration Otherwise, a serial port can get stuck if it is migrated while flow control is in effect. Tested-by: Bret Ketchum Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Paolo Bonzini --- hw/char/serial.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) (limited to 'hw/char') diff --git a/hw/char/serial.c b/hw/char/serial.c index af39e8f867..3442f47d36 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -644,8 +644,29 @@ static int serial_post_load(void *opaque, int version_id) if (s->thr_ipending == -1) { s->thr_ipending = ((s->iir & UART_IIR_ID) == UART_IIR_THRI); } - if (s->tsr_retry > MAX_XMIT_RETRY) { - s->tsr_retry = MAX_XMIT_RETRY; + + if (s->tsr_retry > 0) { + /* tsr_retry > 0 implies LSR.TEMT = 0 (transmitter not empty). */ + if (s->lsr & UART_LSR_TEMT) { + error_report("inconsistent state in serial device " + "(tsr empty, tsr_retry=%d", s->tsr_retry); + return -1; + } + + if (s->tsr_retry > MAX_XMIT_RETRY) { + s->tsr_retry = MAX_XMIT_RETRY; + } + + assert(s->watch_tag == 0); + s->watch_tag = qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, + serial_watch_cb, s); + } else { + /* tsr_retry == 0 implies LSR.TEMT = 1 (transmitter empty). */ + if (!(s->lsr & UART_LSR_TEMT)) { + error_report("inconsistent state in serial device " + "(tsr not empty, tsr_retry=0"); + return -1; + } } s->last_break_enable = (s->lcr >> 6) & 1; -- cgit v1.2.3-55-g7522