* Paolo Bonzini (pbonz...@redhat.com) wrote:
> This is responsible for failure of migration from 2.2 to 2.1, because
> thr_ipending is always one in practice.
>
> serial.c is setting thr_ipending unconditionally. However, thr_ipending
> is not used at all if THRI=0, and it will be overwritten again the next
> time THRE or THRI changes. For that reason, we can set thr_ipending to
> zero every time THRI is reset.
>
> This has no semantic change and is enough to fix migration. It 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.
I can see this causes the thr_ipending to be 0 more of the time, but I don't
see why it's sufficient.
If the transmitter has just transmitted it's last byte, then thr_ipending is set
true by serial_xmit, however if there is a higher priority interrupt then
serial_update_irq would set tmp_iir to something other than THRI,
so I think serial_thr_ipending_needed would return true and write the
subsection.
Dave
> There is disagreement on whether LSR.THRE should be resampled when IER.THRI
> goes from 1 to 1. Do not touch the code for now.
>
> Cc: qemu-sta...@nongnu.org
> Reported-by: Igor Mammedov <imamm...@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> hw/char/serial.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index ebcacdc..8c42d03 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -350,10 +350,24 @@ static void serial_ioport_write(void *opaque, hwaddr
> addr, uint64_t val,
> s->poll_msl = 0;
> }
> }
> - if (s->lsr & UART_LSR_THRE) {
> +
> + /* Turning on the THRE interrupt on IER can trigger the interrupt
> + * if LSR.THRE=1, even if it had been masked before by reading
> IIR.
> + * This is not in the datasheet, but Windows relies on it. It is
> + * unclear if THRE has to be resampled every time THRI becomes
> + * 1, or only on the rising edge. Bochs does the latter, and
> Windows
> + * always toggles IER to all zeroes and back to all ones. But
> for
> + * now leave it as it has always been in QEMU.
> + *
> + * If IER.THRI is zero, thr_ipending is not used. Set it to zero
> + * so that the thr_ipending subsection is not migrated.
> + */
> + if ((s->ier & UART_IER_THRI) && (s->lsr & UART_LSR_THRE)) {
> s->thr_ipending = 1;
> - serial_update_irq(s);
> + } else {
> + s->thr_ipending = 0;
> }
> + serial_update_irq(s);
> }
> break;
> case 2:
> --
> 1.8.3.1
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
