|
|
619821 |
From 1b37b298fc1f0d69e24229191e4bbe741e4d96ab Mon Sep 17 00:00:00 2001
|
|
|
b28c64 |
From: Fam Zheng <famz@redhat.com>
|
|
|
619821 |
Date: Thu, 18 May 2017 09:21:25 +0200
|
|
|
b28c64 |
Subject: [PATCH 12/18] serial: clean up THRE/TEMT handling
|
|
|
b28c64 |
|
|
|
b28c64 |
RH-Author: Fam Zheng <famz@redhat.com>
|
|
|
619821 |
Message-id: <20170518092131.16571-13-famz@redhat.com>
|
|
|
619821 |
Patchwork-id: 75303
|
|
|
619821 |
O-Subject: [RHEL-7.4 qemu-kvm PATCH v3 12/18] serial: clean up THRE/TEMT handling
|
|
|
619821 |
Bugzilla: 1451470
|
|
|
b28c64 |
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
|
|
|
619821 |
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
|
|
|
b28c64 |
RH-Acked-by: Eduardo Habkost <ehabkost@redhat.com>
|
|
|
b28c64 |
|
|
|
b28c64 |
From: Paolo Bonzini <pbonzini@redhat.com>
|
|
|
b28c64 |
|
|
|
b28c64 |
- assert TEMT is cleared before sending a character; we'll get one from
|
|
|
b28c64 |
TSR if tsr_retry > 0, from the FIFO or THR otherwise
|
|
|
b28c64 |
|
|
|
b28c64 |
- assert THRE cleared and FIFO not empty (if enabled) before fetching a
|
|
|
b28c64 |
character to send. This effectively reverts dffacd46, but the check
|
|
|
b28c64 |
makes no sense and commit f702e62 (serial: change retry logic to avoid
|
|
|
b28c64 |
concurrency, 2014-07-11) must have made it unnecessary. The commit
|
|
|
b28c64 |
message for f702e62 talks about multiple calls to qemu_chr_fe_add_watch
|
|
|
b28c64 |
triggering s->tsr_retry >= MAX_XMIT_RETRY, but other failures were
|
|
|
b28c64 |
possible. For example, if you have multiple calls, the subsequent ones
|
|
|
b28c64 |
will see s->tsr_retry == 0 and will find THRE and/or TEMT on entry.
|
|
|
b28c64 |
|
|
|
b28c64 |
- for clarity, raise THRI immediately after the code sets THRE
|
|
|
b28c64 |
|
|
|
b28c64 |
- check THRE to see if another character has to be sent. This makes
|
|
|
b28c64 |
the assertions more obvious and also means TEMT has to be set as soon as
|
|
|
b28c64 |
the loop ends. It makes the loop send both TSR and THR if flow-control
|
|
|
b28c64 |
happens in non-FIFO mode. Previously, THR would be lost.
|
|
|
b28c64 |
|
|
|
b28c64 |
- clear TEMT together with THRE even in the non-FIFO case
|
|
|
b28c64 |
|
|
|
b28c64 |
The last two items are bugfixes, but they were just found by inspection
|
|
|
b28c64 |
and do not squash known bugs.
|
|
|
b28c64 |
|
|
|
b28c64 |
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
|
|
|
b28c64 |
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
|
|
|
b28c64 |
(cherry picked from commit 0d931d706266d6ada3bf22d3afca1afdc8d12fa9)
|
|
|
b28c64 |
Signed-off-by: Fam Zheng <famz@redhat.com>
|
|
|
b28c64 |
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
|
b28c64 |
|
|
|
b28c64 |
Conflicts:
|
|
|
b28c64 |
hw/char/serial.c
|
|
|
b28c64 |
|
|
|
b28c64 |
Contextual conflict because upstream has the new timer API
|
|
|
b28c64 |
qemu_clock_get_ns, but downstream still uses qemu_get_clock_ns.
|
|
|
b28c64 |
|
|
|
b28c64 |
hw/char/serial.c | 26 ++++++++++++
|
|
|
b28c64 |
1 file changed, 12 insertions(+), 14 deletions(-)
|
|
|
b28c64 |
|
|
|
b28c64 |
diff --git a/hw/char/serial.c b/hw/char/serial.c
|
|
|
b28c64 |
index 15c628f..c2be4bd 100644
|
|
|
b28c64 |
|
|
|
b28c64 |
+++ b/hw/char/serial.c
|
|
|
b28c64 |
@@ -224,21 +224,23 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
|
|
|
b28c64 |
SerialState *s = opaque;
|
|
|
b28c64 |
|
|
|
b28c64 |
do {
|
|
|
b28c64 |
+ assert(!(s->lsr & UART_LSR_TEMT));
|
|
|
b28c64 |
if (s->tsr_retry <= 0) {
|
|
|
b28c64 |
+ assert(!(s->lsr & UART_LSR_THRE));
|
|
|
b28c64 |
+
|
|
|
b28c64 |
if (s->fcr & UART_FCR_FE) {
|
|
|
b28c64 |
- if (fifo8_is_empty(&s->xmit_fifo)) {
|
|
|
b28c64 |
- return FALSE;
|
|
|
b28c64 |
- }
|
|
|
b28c64 |
+ assert(!fifo8_is_empty(&s->xmit_fifo));
|
|
|
b28c64 |
s->tsr = fifo8_pop(&s->xmit_fifo);
|
|
|
b28c64 |
if (!s->xmit_fifo.num) {
|
|
|
b28c64 |
s->lsr |= UART_LSR_THRE;
|
|
|
b28c64 |
}
|
|
|
b28c64 |
- } else if ((s->lsr & UART_LSR_THRE)) {
|
|
|
b28c64 |
- return FALSE;
|
|
|
b28c64 |
} else {
|
|
|
b28c64 |
s->tsr = s->thr;
|
|
|
b28c64 |
s->lsr |= UART_LSR_THRE;
|
|
|
b28c64 |
- s->lsr &= ~UART_LSR_TEMT;
|
|
|
b28c64 |
+ }
|
|
|
b28c64 |
+ if ((s->lsr & UART_LSR_THRE) && !s->thr_ipending) {
|
|
|
b28c64 |
+ s->thr_ipending = 1;
|
|
|
b28c64 |
+ serial_update_irq(s);
|
|
|
b28c64 |
}
|
|
|
b28c64 |
}
|
|
|
b28c64 |
|
|
|
b28c64 |
@@ -256,17 +258,13 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
|
|
|
b28c64 |
} else {
|
|
|
b28c64 |
s->tsr_retry = 0;
|
|
|
b28c64 |
}
|
|
|
b28c64 |
+
|
|
|
b28c64 |
/* Transmit another byte if it is already available. It is only
|
|
|
b28c64 |
possible when FIFO is enabled and not empty. */
|
|
|
b28c64 |
- } while ((s->fcr & UART_FCR_FE) && !fifo8_is_empty(&s->xmit_fifo));
|
|
|
b28c64 |
+ } while (!(s->lsr & UART_LSR_THRE));
|
|
|
b28c64 |
|
|
|
b28c64 |
s->last_xmit_ts = qemu_get_clock_ns(vm_clock);
|
|
|
b28c64 |
-
|
|
|
b28c64 |
- if (s->lsr & UART_LSR_THRE) {
|
|
|
b28c64 |
- s->lsr |= UART_LSR_TEMT;
|
|
|
b28c64 |
- s->thr_ipending = 1;
|
|
|
b28c64 |
- serial_update_irq(s);
|
|
|
b28c64 |
- }
|
|
|
b28c64 |
+ s->lsr |= UART_LSR_TEMT;
|
|
|
b28c64 |
|
|
|
b28c64 |
return FALSE;
|
|
|
b28c64 |
}
|
|
|
b28c64 |
@@ -293,10 +291,10 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
|
|
|
b28c64 |
fifo8_pop(&s->xmit_fifo);
|
|
|
b28c64 |
}
|
|
|
b28c64 |
fifo8_push(&s->xmit_fifo, s->thr);
|
|
|
b28c64 |
- s->lsr &= ~UART_LSR_TEMT;
|
|
|
b28c64 |
}
|
|
|
b28c64 |
s->thr_ipending = 0;
|
|
|
b28c64 |
s->lsr &= ~UART_LSR_THRE;
|
|
|
b28c64 |
+ s->lsr &= ~UART_LSR_TEMT;
|
|
|
b28c64 |
serial_update_irq(s);
|
|
|
b28c64 |
if (s->tsr_retry <= 0) {
|
|
|
b28c64 |
serial_xmit(NULL, G_IO_OUT, s);
|
|
|
b28c64 |
--
|
|
|
b28c64 |
1.8.3.1
|
|
|
b28c64 |
|