From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36460) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0TAk-0002ES-4b for qemu-devel@nongnu.org; Mon, 15 Dec 2014 05:46:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y0TAe-0007Fq-Ra for qemu-devel@nongnu.org; Mon, 15 Dec 2014 05:46:38 -0500 Date: Mon, 15 Dec 2014 10:46:20 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20141215104620.GB5502@work-vm> References: <1418388243-1886-1-git-send-email-pbonzini@redhat.com> <1418388243-1886-2-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1418388243-1886-2-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 1/4] serial: reset thri_pending on IER writes with THRI=0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: imammedo@redhat.com, qemu-stable@nongnu.org, andrey@xdel.ru, qemu-devel@nongnu.org, batuzovk@ispras.ru * Paolo Bonzini (pbonzini@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-stable@nongnu.org > Reported-by: Igor Mammedov > Signed-off-by: Paolo Bonzini > --- > 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 / dgilbert@redhat.com / Manchester, UK