[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/4] serial: reset thri_pending on IER writes
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/4] serial: reset thri_pending on IER writes with THRI=0 |
Date: |
Mon, 15 Dec 2014 10:46:20 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Paolo Bonzini (address@hidden) 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: address@hidden
> Reported-by: Igor Mammedov <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> 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 / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH v3 0/4] serial fixes, including 2.2->2.1 migration, Paolo Bonzini, 2014/12/12
- [Qemu-devel] [PATCH v3 1/4] serial: reset thri_pending on IER writes with THRI=0, Paolo Bonzini, 2014/12/12
- Re: [Qemu-devel] [PATCH v3 1/4] serial: reset thri_pending on IER writes with THRI=0,
Dr. David Alan Gilbert <=
- Re: [Qemu-devel] [PATCH v3 1/4] serial: reset thri_pending on IER writes with THRI=0, Paolo Bonzini, 2014/12/15
- Re: [Qemu-devel] [PATCH v3 1/4] serial: reset thri_pending on IER writes with THRI=0, Dr. David Alan Gilbert, 2014/12/15
- Re: [Qemu-devel] [PATCH v3 1/4] serial: reset thri_pending on IER writes with THRI=0, Paolo Bonzini, 2014/12/15
- Re: [Qemu-devel] [PATCH v3 1/4] serial: reset thri_pending on IER writes with THRI=0, Dr. David Alan Gilbert, 2014/12/15
- Re: [Qemu-devel] [PATCH v3 1/4] serial: reset thri_pending on IER writes with THRI=0, Paolo Bonzini, 2014/12/15
[Qemu-devel] [PATCH v3 4/4] serial: only resample THR interrupt on rising edge of IER.THRI, Paolo Bonzini, 2014/12/12
[Qemu-devel] [PATCH v3 3/4] serial: update LSR on enabling/disabling FIFOs, Paolo Bonzini, 2014/12/12