[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH v2] hw/char/escc: Lower irq when
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH v2] hw/char/escc: Lower irq when transmit buffer is filled |
Date: |
Thu, 18 Apr 2019 14:13:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
On 4/17/19 2:50 AM, 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.
You might want to add a line expliciting the chipset model which forced
you to do that patch (Z85C30).
> This 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 <address@hidden>
> ---
> 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);
Way better, thanks :)
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Note: the Z85C30 has a 1-byte transmit FIFO and 3-byte receive FIFO.
Our model default is the ESCC, and we use ESCC_SERIO_QUEUE_SIZE=256 byte
for rx FIFO... If needed we can add properties and use a white list of
chipset with their tx/rx FIFO size.
> s->tx = val;
> if (s->wregs[W_TXCTRL2] & TXCTRL2_TXEN) { // tx enabled
> if (qemu_chr_fe_backend_connected(&s->chr)) {
>