[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH v3] hw/char/escc: Lower irq when
From: |
Mark Cave-Ayland |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH v3] hw/char/escc: Lower irq when transmit buffer is filled |
Date: |
Fri, 3 May 2019 08:14:11 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
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 <address@hidden>
>>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>>> ---
>>>
>>> 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.