qemu-trivial
[Top][All Lists]
Advanced

[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: Laurent Vivier
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH v3] hw/char/escc: Lower irq when transmit buffer is filled
Date: Fri, 3 May 2019 09:21:16 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0

On 03/05/2019 09:14, Mark Cave-Ayland wrote:
> 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.

Hi Mark,

I've already included a leon3 patch ("hw/sparc/leon3: Allow load of
uImage firmwares") in the PR I sent yesterday for the trivial branch.
I've removed from my PR this patch about the ESCC, so you can take it.

Thanks,
Laurent



reply via email to

[Prev in Thread] Current Thread [Next in Thread]