[Qemu-devel] [PATCH] serial: do not trigger THR interrupt after writing to IER

[Qemu-devel] [PATCH] serial: do not trigger THR interrupt after writing to IER

[Paolo Bonzini]

This is responsible for failure of migration from 2.2 to 2.1, because
thr_ipending is always one in practice.  Calling serial_update_irq is
the right thing to do indeed, because writing to IER could cause an
interrupt to appear.  However, there is no reason to set thr_ipending
again.

This was already reported in 2010.  See this quote from
https://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01914.html:

> The commit in r1049 (serial interrupt fix (Hampa Hug)) prevents
> booting Digital Research DOSPlus. Following patch partially reverts
> that commit and makes DOSPlus booting in QEMU again.

Bochs does not check LSR_THRE in IER, and the log message in r1049 doesn't
explain why the change was made in the first place.

This does not change the migration format, so 2.2.0 -> 2.1 will remain
broken but we can fix 2.2.1 -> 2.1 without breaking 2.2.1 <-> 2.2.0.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/char/serial.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index ebcacdc..cf8e4e3 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -350,10 +350,7 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
                      s->poll_msl = 0;
                 }
             }
-            if (s->lsr & UART_LSR_THRE) {
-                s->thr_ipending = 1;
-                serial_update_irq(s);
-            }
+            serial_update_irq(s);
         }
         break;
     case 2:
-- 
2.1.0

Re: [Qemu-devel] [PATCH] serial: do not trigger THR interrupt after writing to IER

[Paolo Bonzini]

On 10/12/2014 17:34, Paolo Bonzini wrote:
> This is responsible for failure of migration from 2.2 to 2.1, because
> thr_ipending is always one in practice.  Calling serial_update_irq is
> the right thing to do indeed, because writing to IER could cause an
> interrupt to appear.  However, there is no reason to set thr_ipending
> again.
> 
> This was already reported in 2010.  See this quote from
> https://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01914.html:
> 
>> The commit in r1049 (serial interrupt fix (Hampa Hug)) prevents
>> booting Digital Research DOSPlus. Following patch partially reverts
>> that commit and makes DOSPlus booting in QEMU again.
> 
> Bochs does not check LSR_THRE in IER, and the log message in r1049 doesn't
> explain why the change was made in the first place.
> 
> This does not change the migration format, so 2.2.0 -> 2.1 will remain
> broken but we can fix 2.2.1 -> 2.1 without breaking 2.2.1 <-> 2.2.0.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/char/serial.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index ebcacdc..cf8e4e3 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -350,10 +350,7 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
>                       s->poll_msl = 0;
>                  }
>              }
> -            if (s->lsr & UART_LSR_THRE) {
> -                s->thr_ipending = 1;
> -                serial_update_irq(s);
> -            }
> +            serial_update_irq(s);
>          }
>          break;
>      case 2:
> 

Nope, this breaks the Windows UART driver.

Paolo