On 02/05/2019 13:11, Philippe Mathieu-Daudé wrote: > On 5/2/19 11:04 AM, Laurent Vivier wrote: >> On 19/04/2019 17:40, Stephen Checkoway wrote: >>> The SCC/ESCC will briefly stop asserting an interrupt when the >>> transmit FIFO is filled. >>> >>> This code doesn't model the transmit FIFO/shift register so the >>> pending transmit interrupt is never deasserted which means that an >>> edge-triggered interrupt controller will never see the low-to-high >>> transition it needs to raise another interrupt. The practical >>> consequence of this is that guest firmware with an interrupt service >>> routine for the ESCC that does not send all of the data it has >>> immediately will stop sending data if the following sequence of >>> events occurs: >>> 1. Disable processor interrupts >>> 2. Write a character to the ESCC >>> 3. Add additional characters to a buffer which is drained by the ISR >>> 4. Enable processor interrupts >>> >>> In this case, the first character will be sent, the interrupt will >>> fire and the ISR will output the second character. Since the pending >>> transmit interrupt remains asserted, no additional interrupts will >>> ever fire. >>> >>> This behavior was triggered by firmware for an embedded system with a >>> Z85C30 which necessitated this patch. >>> >>> This patch fixes that situation by explicitly lowering the IRQ when a >>> character is written to the buffer and no other interrupts are currently >>> pending. >>> >>> Signed-off-by: Stephen Checkoway <stephen.checko...@oberlin.edu> >>> Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com> >>> --- >>> >>> I added a sentence about the Z85C30 necessitating this to the commit >>> message. >>> >>> hw/char/escc.c | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/hw/char/escc.c b/hw/char/escc.c >>> index 628f5f81f7..c5b05a63f1 100644 >>> --- a/hw/char/escc.c >>> +++ b/hw/char/escc.c >>> @@ -509,6 +509,13 @@ static void escc_mem_write(void *opaque, hwaddr addr, >>> break; >>> case SERIAL_DATA: >>> trace_escc_mem_writeb_data(CHN_C(s), val); >>> + /* >>> + * Lower the irq when data is written to the Tx buffer and no other >>> + * interrupts are currently pending. The irq will be raised again >>> once >>> + * the Tx buffer becomes empty below. >>> + */ >>> + s->txint = 0; >>> + escc_update_irq(s); >>> s->tx = val; >>> if (s->wregs[W_TXCTRL2] & TXCTRL2_TXEN) { // tx enabled >>> if (qemu_chr_fe_backend_connected(&s->chr)) { >>> >> >> >> Applied to my trivial-patches branch. > > Mark, Artyom, are you OK with this patch?
I started testing this with my OpenBIOS test images at the start of the week, but unfortunately got distracted by real life :) I've now finished and confirmed there are no regressions in my local tests, so I'll include this in the PR I am planning to send shortly containing the leon3 updates. ATB, Mark.