* [Qemu-devel] qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) @ 2009-09-11 21:35 Juergen Lock 2009-09-12 11:20 ` [Qemu-devel] " Olivier Cochard-Labbé 2009-09-12 12:26 ` Jan Kiszka 0 siblings, 2 replies; 10+ messages in thread From: Juergen Lock @ 2009-09-11 21:35 UTC (permalink / raw) To: qemu-devel; +Cc: Olivier =?iso-8859-1?Q?Cochard-Labb=E9?=, freebsd-current Hi! I got a report of FreeBSD guest's new uart(4) driver misbehaving in qemu again(?) (output stopping for no apparent reason), and now found out the problem is tx irqs (UART_IIR_THRI) are getting lost because serial_update_irq() checks for the rx condtion, ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) first before checking for the tx irq condition, ... if ((s->ier & UART_IER_THRI) && s->thr_ipending) which at least in this case (FreeBSD 8 guest after doing set console="comconsole" at the loader prompt or when simply echo'ing text to /dev/ttyu0 or typing to the serial port from cu(1) on a `regular' vga console) causes the second condition (.. && s->thr_ipending) to be never reached anymore, or only after a very long delay. Moving that condition up so it is checked first like this, Index: qemu/hw/serial.c @@ -189,7 +188,9 @@ static void serial_update_irq(SerialStat { uint8_t tmp_iir = UART_IIR_NO_INT; - if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { + if ((s->ier & UART_IER_THRI) && s->thr_ipending) { + tmp_iir = UART_IIR_THRI; + } else if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { tmp_iir = UART_IIR_RLSI; } else if ((s->ier & UART_IER_RDI) && s->timeout_ipending) { /* Note that(s->ier & UART_IER_RDI) can mask this interrupt, @@ -202,8 +203,6 @@ static void serial_update_irq(SerialStat } else if (s->recv_fifo.count >= s->recv_fifo.itl) { tmp_iir = UART_IIR_RDI; } - } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { - tmp_iir = UART_IIR_THRI; } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { tmp_iir = UART_IIR_MSI; } ...fixes the issue for me, but I'm not 100% sure if this might cause rx irqs to come (too?) late when a guest keeps sending while its receiving at the same time. Anyone care to comment? :) Thanx, Juergen ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) 2009-09-11 21:35 [Qemu-devel] qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) Juergen Lock @ 2009-09-12 11:20 ` Olivier Cochard-Labbé 2009-09-12 12:26 ` Jan Kiszka 1 sibling, 0 replies; 10+ messages in thread From: Olivier Cochard-Labbé @ 2009-09-12 11:20 UTC (permalink / raw) To: Juergen Lock; +Cc: freebsd-current, qemu-devel Hi, On Fri, Sep 11, 2009 at 11:35 PM, Juergen Lock <nox@jelal.kn-bremen.de> wrote: > ...fixes the issue for me, but I'm not 100% sure if this might cause > rx irqs to come (too?) late when a guest keeps sending while its > receiving at the same time. Anyone care to comment? :) > Your patch fix the issue for me too. Thanks a lot's Juergen ! Regards, Olivier ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) 2009-09-11 21:35 [Qemu-devel] qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) Juergen Lock 2009-09-12 11:20 ` [Qemu-devel] " Olivier Cochard-Labbé @ 2009-09-12 12:26 ` Jan Kiszka 2009-09-12 16:52 ` Juergen Lock 1 sibling, 1 reply; 10+ messages in thread From: Jan Kiszka @ 2009-09-12 12:26 UTC (permalink / raw) To: Juergen Lock; +Cc: Olivier Cochard-Labbé, freebsd-current, qemu-devel [-- Attachment #1: Type: text/plain, Size: 2256 bytes --] Juergen Lock wrote: > Hi! > > I got a report of FreeBSD guest's new uart(4) driver misbehaving in > qemu again(?) (output stopping for no apparent reason), and now found > out the problem is tx irqs (UART_IIR_THRI) are getting lost because > serial_update_irq() checks for the rx condtion, > ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > first before checking for the tx irq condition, > ... if ((s->ier & UART_IER_THRI) && s->thr_ipending) > which at least in this case (FreeBSD 8 guest after doing > set console="comconsole" > at the loader prompt or when simply echo'ing text to /dev/ttyu0 > or typing to the serial port from cu(1) on a `regular' vga console) > causes the second condition (.. && s->thr_ipending) to be never > reached anymore, or only after a very long delay. Moving that > condition up so it is checked first like this, > > Index: qemu/hw/serial.c > @@ -189,7 +188,9 @@ static void serial_update_irq(SerialStat > { > uint8_t tmp_iir = UART_IIR_NO_INT; > > - if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > + if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > + tmp_iir = UART_IIR_THRI; > + } else if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > tmp_iir = UART_IIR_RLSI; > } else if ((s->ier & UART_IER_RDI) && s->timeout_ipending) { > /* Note that(s->ier & UART_IER_RDI) can mask this interrupt, > @@ -202,8 +203,6 @@ static void serial_update_irq(SerialStat > } else if (s->recv_fifo.count >= s->recv_fifo.itl) { > tmp_iir = UART_IIR_RDI; > } > - } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > - tmp_iir = UART_IIR_THRI; > } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { > tmp_iir = UART_IIR_MSI; > } > > ...fixes the issue for me, but I'm not 100% sure if this might cause > rx irqs to come (too?) late when a guest keeps sending while its > receiving at the same time. Anyone care to comment? :) The reordering violates the 16550A spec in that RX event overrules TX in the IRQ status register. Maybe something else is wrong but it's not the ordering in serial_update_irq. Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) 2009-09-12 12:26 ` Jan Kiszka @ 2009-09-12 16:52 ` Juergen Lock 2009-09-12 17:00 ` Jan Kiszka ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Juergen Lock @ 2009-09-12 16:52 UTC (permalink / raw) To: Jan Kiszka Cc: Olivier Cochard-Labbé, freebsd-current, Juergen Lock, qemu-devel On Sat, Sep 12, 2009 at 02:26:51PM +0200, Jan Kiszka wrote: > Juergen Lock wrote: > > Hi! > > > > I got a report of FreeBSD guest's new uart(4) driver misbehaving in > > qemu again(?) (output stopping for no apparent reason), and now found > > out the problem is tx irqs (UART_IIR_THRI) are getting lost because > > serial_update_irq() checks for the rx condtion, > > ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > > first before checking for the tx irq condition, > > ... if ((s->ier & UART_IER_THRI) && s->thr_ipending) > > which at least in this case (FreeBSD 8 guest after doing > > set console="comconsole" > > at the loader prompt or when simply echo'ing text to /dev/ttyu0 > > or typing to the serial port from cu(1) on a `regular' vga console) > > causes the second condition (.. && s->thr_ipending) to be never > > reached anymore, or only after a very long delay. Moving that > > condition up so it is checked first like this, > > > > Index: qemu/hw/serial.c > > @@ -189,7 +188,9 @@ static void serial_update_irq(SerialStat > > { > > uint8_t tmp_iir = UART_IIR_NO_INT; > > > > - if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > > + if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > > + tmp_iir = UART_IIR_THRI; > > + } else if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > > tmp_iir = UART_IIR_RLSI; > > } else if ((s->ier & UART_IER_RDI) && s->timeout_ipending) { > > /* Note that(s->ier & UART_IER_RDI) can mask this interrupt, > > @@ -202,8 +203,6 @@ static void serial_update_irq(SerialStat > > } else if (s->recv_fifo.count >= s->recv_fifo.itl) { > > tmp_iir = UART_IIR_RDI; > > } > > - } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > > - tmp_iir = UART_IIR_THRI; > > } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { > > tmp_iir = UART_IIR_MSI; > > } > > > > ...fixes the issue for me, but I'm not 100% sure if this might cause > > rx irqs to come (too?) late when a guest keeps sending while its > > receiving at the same time. Anyone care to comment? :) > > The reordering violates the 16550A spec in that RX event overrules TX in > the IRQ status register. Maybe something else is wrong but it's not the > ordering in serial_update_irq. Well one problem seems to be the rx condition, ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) is not enough to trigger an irq, yet still causes the following conditions not to be checked anymore at all. And ideed, fixing that seems to get my FreeBSD 8 guest back to working order as well: Index: qemu/hw/serial.c @@ -196,12 +195,10 @@ static void serial_update_irq(SerialStat * this is not in the specification but is observed on existing * hardware. */ tmp_iir = UART_IIR_CTI; - } else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) { - if (!(s->fcr & UART_FCR_FE)) { - tmp_iir = UART_IIR_RDI; - } else if (s->recv_fifo.count >= s->recv_fifo.itl) { - tmp_iir = UART_IIR_RDI; - } + } else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR) && + (!(s->fcr & UART_FCR_FE) || + s->recv_fifo.count >= s->recv_fifo.itl)) { + tmp_iir = UART_IIR_RDI; } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { tmp_iir = UART_IIR_THRI; } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) 2009-09-12 16:52 ` Juergen Lock @ 2009-09-12 17:00 ` Jan Kiszka 2009-09-14 16:59 ` Stefano Stabellini 2009-09-16 19:01 ` Aurelien Jarno 2 siblings, 0 replies; 10+ messages in thread From: Jan Kiszka @ 2009-09-12 17:00 UTC (permalink / raw) To: Juergen Lock Cc: Olivier Cochard-Labbé, freebsd-current, qemu-devel, Stefano Stabellini [-- Attachment #1: Type: text/plain, Size: 3865 bytes --] Juergen Lock wrote: > On Sat, Sep 12, 2009 at 02:26:51PM +0200, Jan Kiszka wrote: >> Juergen Lock wrote: >>> Hi! >>> >>> I got a report of FreeBSD guest's new uart(4) driver misbehaving in >>> qemu again(?) (output stopping for no apparent reason), and now found >>> out the problem is tx irqs (UART_IIR_THRI) are getting lost because >>> serial_update_irq() checks for the rx condtion, >>> ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) >>> first before checking for the tx irq condition, >>> ... if ((s->ier & UART_IER_THRI) && s->thr_ipending) >>> which at least in this case (FreeBSD 8 guest after doing >>> set console="comconsole" >>> at the loader prompt or when simply echo'ing text to /dev/ttyu0 >>> or typing to the serial port from cu(1) on a `regular' vga console) >>> causes the second condition (.. && s->thr_ipending) to be never >>> reached anymore, or only after a very long delay. Moving that >>> condition up so it is checked first like this, >>> >>> Index: qemu/hw/serial.c >>> @@ -189,7 +188,9 @@ static void serial_update_irq(SerialStat >>> { >>> uint8_t tmp_iir = UART_IIR_NO_INT; >>> >>> - if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { >>> + if ((s->ier & UART_IER_THRI) && s->thr_ipending) { >>> + tmp_iir = UART_IIR_THRI; >>> + } else if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { >>> tmp_iir = UART_IIR_RLSI; >>> } else if ((s->ier & UART_IER_RDI) && s->timeout_ipending) { >>> /* Note that(s->ier & UART_IER_RDI) can mask this interrupt, >>> @@ -202,8 +203,6 @@ static void serial_update_irq(SerialStat >>> } else if (s->recv_fifo.count >= s->recv_fifo.itl) { >>> tmp_iir = UART_IIR_RDI; >>> } >>> - } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { >>> - tmp_iir = UART_IIR_THRI; >>> } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { >>> tmp_iir = UART_IIR_MSI; >>> } >>> >>> ...fixes the issue for me, but I'm not 100% sure if this might cause >>> rx irqs to come (too?) late when a guest keeps sending while its >>> receiving at the same time. Anyone care to comment? :) >> The reordering violates the 16550A spec in that RX event overrules TX in >> the IRQ status register. Maybe something else is wrong but it's not the >> ordering in serial_update_irq. > > Well one problem seems to be the rx condition, > ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > is not enough to trigger an irq, yet still causes the following > conditions not to be checked anymore at all. And ideed, fixing that > seems to get my FreeBSD 8 guest back to working order as well: > > Index: qemu/hw/serial.c > @@ -196,12 +195,10 @@ static void serial_update_irq(SerialStat > * this is not in the specification but is observed on existing > * hardware. */ > tmp_iir = UART_IIR_CTI; > - } else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) { > - if (!(s->fcr & UART_FCR_FE)) { > - tmp_iir = UART_IIR_RDI; > - } else if (s->recv_fifo.count >= s->recv_fifo.itl) { > - tmp_iir = UART_IIR_RDI; > - } > + } else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR) && > + (!(s->fcr & UART_FCR_FE) || > + s->recv_fifo.count >= s->recv_fifo.itl)) { > + tmp_iir = UART_IIR_RDI; > } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > tmp_iir = UART_IIR_THRI; > } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { > > Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de> Yep, that does make sense! Acked-by: Jan Kiszka <jan.kiszka@web.de> But I also but Stefano on CC as he introduced the logic above. Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) 2009-09-12 16:52 ` Juergen Lock 2009-09-12 17:00 ` Jan Kiszka @ 2009-09-14 16:59 ` Stefano Stabellini 2009-09-16 19:01 ` Aurelien Jarno 2 siblings, 0 replies; 10+ messages in thread From: Stefano Stabellini @ 2009-09-14 16:59 UTC (permalink / raw) To: Juergen Lock Cc: Olivier Cochard-Labbé, freebsd-current, Jan Kiszka, qemu-devel On Sat, 12 Sep 2009, Juergen Lock wrote: > Well one problem seems to be the rx condition, > ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > is not enough to trigger an irq, yet still causes the following > conditions not to be checked anymore at all. And ideed, fixing that > seems to get my FreeBSD 8 guest back to working order as well: Good spot! The fix also seems correct to me. Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Index: qemu/hw/serial.c > @@ -196,12 +195,10 @@ static void serial_update_irq(SerialStat > * this is not in the specification but is observed on existing > * hardware. */ > tmp_iir = UART_IIR_CTI; > - } else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) { > - if (!(s->fcr & UART_FCR_FE)) { > - tmp_iir = UART_IIR_RDI; > - } else if (s->recv_fifo.count >= s->recv_fifo.itl) { > - tmp_iir = UART_IIR_RDI; > - } > + } else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR) && > + (!(s->fcr & UART_FCR_FE) || > + s->recv_fifo.count >= s->recv_fifo.itl)) { > + tmp_iir = UART_IIR_RDI; > } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > tmp_iir = UART_IIR_THRI; > } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { > > Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de> > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) 2009-09-12 16:52 ` Juergen Lock 2009-09-12 17:00 ` Jan Kiszka 2009-09-14 16:59 ` Stefano Stabellini @ 2009-09-16 19:01 ` Aurelien Jarno 2009-09-23 18:47 ` Juergen Lock 2 siblings, 1 reply; 10+ messages in thread From: Aurelien Jarno @ 2009-09-16 19:01 UTC (permalink / raw) To: Juergen Lock Cc: Olivier Cochard-Labbé, freebsd-current, Jan Kiszka, qemu-devel On Sat, Sep 12, 2009 at 06:52:22PM +0200, Juergen Lock wrote: > On Sat, Sep 12, 2009 at 02:26:51PM +0200, Jan Kiszka wrote: > > Juergen Lock wrote: > > > Hi! > > > > > > I got a report of FreeBSD guest's new uart(4) driver misbehaving in > > > qemu again(?) (output stopping for no apparent reason), and now found > > > out the problem is tx irqs (UART_IIR_THRI) are getting lost because > > > serial_update_irq() checks for the rx condtion, > > > ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > > > first before checking for the tx irq condition, > > > ... if ((s->ier & UART_IER_THRI) && s->thr_ipending) > > > which at least in this case (FreeBSD 8 guest after doing > > > set console="comconsole" > > > at the loader prompt or when simply echo'ing text to /dev/ttyu0 > > > or typing to the serial port from cu(1) on a `regular' vga console) > > > causes the second condition (.. && s->thr_ipending) to be never > > > reached anymore, or only after a very long delay. Moving that > > > condition up so it is checked first like this, > > > > > > Index: qemu/hw/serial.c > > > @@ -189,7 +188,9 @@ static void serial_update_irq(SerialStat > > > { > > > uint8_t tmp_iir = UART_IIR_NO_INT; > > > > > > - if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > > > + if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > > > + tmp_iir = UART_IIR_THRI; > > > + } else if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > > > tmp_iir = UART_IIR_RLSI; > > > } else if ((s->ier & UART_IER_RDI) && s->timeout_ipending) { > > > /* Note that(s->ier & UART_IER_RDI) can mask this interrupt, > > > @@ -202,8 +203,6 @@ static void serial_update_irq(SerialStat > > > } else if (s->recv_fifo.count >= s->recv_fifo.itl) { > > > tmp_iir = UART_IIR_RDI; > > > } > > > - } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > > > - tmp_iir = UART_IIR_THRI; > > > } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { > > > tmp_iir = UART_IIR_MSI; > > > } > > > > > > ...fixes the issue for me, but I'm not 100% sure if this might cause > > > rx irqs to come (too?) late when a guest keeps sending while its > > > receiving at the same time. Anyone care to comment? :) > > > > The reordering violates the 16550A spec in that RX event overrules TX in > > the IRQ status register. Maybe something else is wrong but it's not the > > ordering in serial_update_irq. > > Well one problem seems to be the rx condition, > ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > is not enough to trigger an irq, yet still causes the following > conditions not to be checked anymore at all. And ideed, fixing that > seems to get my FreeBSD 8 guest back to working order as well: Applied. In the future, could you please make sure to send patches with a correct unified headers? > Index: qemu/hw/serial.c > @@ -196,12 +195,10 @@ static void serial_update_irq(SerialStat > * this is not in the specification but is observed on existing > * hardware. */ > tmp_iir = UART_IIR_CTI; > - } else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) { > - if (!(s->fcr & UART_FCR_FE)) { > - tmp_iir = UART_IIR_RDI; > - } else if (s->recv_fifo.count >= s->recv_fifo.itl) { > - tmp_iir = UART_IIR_RDI; > - } > + } else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR) && > + (!(s->fcr & UART_FCR_FE) || > + s->recv_fifo.count >= s->recv_fifo.itl)) { > + tmp_iir = UART_IIR_RDI; > } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > tmp_iir = UART_IIR_THRI; > } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { > > Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de> > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) 2009-09-16 19:01 ` Aurelien Jarno @ 2009-09-23 18:47 ` Juergen Lock 2009-09-24 16:20 ` Aurelien Jarno 0 siblings, 1 reply; 10+ messages in thread From: Juergen Lock @ 2009-09-23 18:47 UTC (permalink / raw) To: aurelien Cc: Olivier Cochard-Labbé, freebsd-current, Jan Kiszka, qemu-devel In article <20090916190142.GC770@volta.aurel32.net> you write: >On Sat, Sep 12, 2009 at 06:52:22PM +0200, Juergen Lock wrote: >> On Sat, Sep 12, 2009 at 02:26:51PM +0200, Jan Kiszka wrote: >> > Juergen Lock wrote: >> > > Hi! >> > > >> > > I got a report of FreeBSD guest's new uart(4) driver misbehaving in >> > > qemu again(?) (output stopping for no apparent reason), and now found >> > > out the problem is tx irqs (UART_IIR_THRI) are getting lost because >> > > serial_update_irq() checks for the rx condtion, >> > > ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) >> > > first before checking for the tx irq condition, >> > > ... if ((s->ier & UART_IER_THRI) && s->thr_ipending) >> > > which at least in this case (FreeBSD 8 guest after doing >> > > set console="comconsole" >> > > at the loader prompt or when simply echo'ing text to /dev/ttyu0 >> > > or typing to the serial port from cu(1) on a `regular' vga console) >> > > causes the second condition (.. && s->thr_ipending) to be never >> > > reached anymore, or only after a very long delay. Moving that >> > > condition up so it is checked first like this, >> > > >> > > Index: qemu/hw/serial.c >> > > @@ -189,7 +188,9 @@ static void serial_update_irq(SerialStat >> > > { >> > > uint8_t tmp_iir = UART_IIR_NO_INT; >> > > >> > > - if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { >> > > + if ((s->ier & UART_IER_THRI) && s->thr_ipending) { >> > > + tmp_iir = UART_IIR_THRI; >> > > + } else if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { >> > > tmp_iir = UART_IIR_RLSI; >> > > } else if ((s->ier & UART_IER_RDI) && s->timeout_ipending) { >> > > /* Note that(s->ier & UART_IER_RDI) can mask this interrupt, >> > > @@ -202,8 +203,6 @@ static void serial_update_irq(SerialStat >> > > } else if (s->recv_fifo.count >= s->recv_fifo.itl) { >> > > tmp_iir = UART_IIR_RDI; >> > > } >> > > - } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { >> > > - tmp_iir = UART_IIR_THRI; >> > > } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { >> > > tmp_iir = UART_IIR_MSI; >> > > } >> > > >> > > ...fixes the issue for me, but I'm not 100% sure if this might cause >> > > rx irqs to come (too?) late when a guest keeps sending while its >> > > receiving at the same time. Anyone care to comment? :) >> > >> > The reordering violates the 16550A spec in that RX event overrules TX in >> > the IRQ status register. Maybe something else is wrong but it's not the >> > ordering in serial_update_irq. >> >> Well one problem seems to be the rx condition, >> ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) >> is not enough to trigger an irq, yet still causes the following >> conditions not to be checked anymore at all. And ideed, fixing that >> seems to get my FreeBSD 8 guest back to working order as well: > >Applied. In the future, could you please make sure to send patches with >a correct unified headers? Alright, if thats is what you guys prefer... (I just didn't want to break the thread.) Anyway, I guess this is also material for the stable branch(es)? (I just saw 0.11.0 has already been tagged but not announced yet, and another patch merged to the same branch after that, maybe the tag can still be slided if this is possible with git?) Thanx, Juergen ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) 2009-09-23 18:47 ` Juergen Lock @ 2009-09-24 16:20 ` Aurelien Jarno 2009-09-24 21:26 ` Juergen Lock 0 siblings, 1 reply; 10+ messages in thread From: Aurelien Jarno @ 2009-09-24 16:20 UTC (permalink / raw) To: Juergen Lock Cc: Olivier Cochard-Labbé, freebsd-current, Jan Kiszka, qemu-devel On Wed, Sep 23, 2009 at 08:47:23PM +0200, Juergen Lock wrote: > In article <20090916190142.GC770@volta.aurel32.net> you write: > >On Sat, Sep 12, 2009 at 06:52:22PM +0200, Juergen Lock wrote: > >> On Sat, Sep 12, 2009 at 02:26:51PM +0200, Jan Kiszka wrote: > >> > Juergen Lock wrote: > >> > > Hi! > >> > > > >> > > I got a report of FreeBSD guest's new uart(4) driver misbehaving in > >> > > qemu again(?) (output stopping for no apparent reason), and now found > >> > > out the problem is tx irqs (UART_IIR_THRI) are getting lost because > >> > > serial_update_irq() checks for the rx condtion, > >> > > ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > >> > > first before checking for the tx irq condition, > >> > > ... if ((s->ier & UART_IER_THRI) && s->thr_ipending) > >> > > which at least in this case (FreeBSD 8 guest after doing > >> > > set console="comconsole" > >> > > at the loader prompt or when simply echo'ing text to /dev/ttyu0 > >> > > or typing to the serial port from cu(1) on a `regular' vga console) > >> > > causes the second condition (.. && s->thr_ipending) to be never > >> > > reached anymore, or only after a very long delay. Moving that > >> > > condition up so it is checked first like this, > >> > > > >> > > Index: qemu/hw/serial.c > >> > > @@ -189,7 +188,9 @@ static void serial_update_irq(SerialStat > >> > > { > >> > > uint8_t tmp_iir = UART_IIR_NO_INT; > >> > > > >> > > - if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > >> > > + if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > >> > > + tmp_iir = UART_IIR_THRI; > >> > > + } else if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > >> > > tmp_iir = UART_IIR_RLSI; > >> > > } else if ((s->ier & UART_IER_RDI) && s->timeout_ipending) { > >> > > /* Note that(s->ier & UART_IER_RDI) can mask this interrupt, > >> > > @@ -202,8 +203,6 @@ static void serial_update_irq(SerialStat > >> > > } else if (s->recv_fifo.count >= s->recv_fifo.itl) { > >> > > tmp_iir = UART_IIR_RDI; > >> > > } > >> > > - } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > >> > > - tmp_iir = UART_IIR_THRI; > >> > > } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { > >> > > tmp_iir = UART_IIR_MSI; > >> > > } > >> > > > >> > > ...fixes the issue for me, but I'm not 100% sure if this might cause > >> > > rx irqs to come (too?) late when a guest keeps sending while its > >> > > receiving at the same time. Anyone care to comment? :) > >> > > >> > The reordering violates the 16550A spec in that RX event overrules TX in > >> > the IRQ status register. Maybe something else is wrong but it's not the > >> > ordering in serial_update_irq. > >> > >> Well one problem seems to be the rx condition, > >> ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > >> is not enough to trigger an irq, yet still causes the following > >> conditions not to be checked anymore at all. And ideed, fixing that > >> seems to get my FreeBSD 8 guest back to working order as well: > > > >Applied. In the future, could you please make sure to send patches with > >a correct unified headers? > > Alright, if thats is what you guys prefer... (I just didn't want to > break the thread.) Don't need to break the thread, just use | --- a/hw/serial.c | +++ a/hw/serial.c instead of simply | Index: qemu/hw/serial.c ie the output of diff -u > Anyway, I guess this is also material for the stable branch(es)? > (I just saw 0.11.0 has already been tagged but not announced yet, and > another patch merged to the same branch after that, maybe the tag can > still be slided if this is possible with git?) > Pushed to the branch. For the details about the release, I let Anthony handling that. Worst case scenario, it will be in 0.11.1. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) 2009-09-24 16:20 ` Aurelien Jarno @ 2009-09-24 21:26 ` Juergen Lock 0 siblings, 0 replies; 10+ messages in thread From: Juergen Lock @ 2009-09-24 21:26 UTC (permalink / raw) To: Aurelien Jarno Cc: Olivier Cochard-Labbé, freebsd-current, Jan Kiszka, Juergen Lock, qemu-devel On Thu, Sep 24, 2009 at 06:20:00PM +0200, Aurelien Jarno wrote: > On Wed, Sep 23, 2009 at 08:47:23PM +0200, Juergen Lock wrote: > > In article <20090916190142.GC770@volta.aurel32.net> you write: > > >On Sat, Sep 12, 2009 at 06:52:22PM +0200, Juergen Lock wrote: > > >> On Sat, Sep 12, 2009 at 02:26:51PM +0200, Jan Kiszka wrote: > > >> > Juergen Lock wrote: > > >> > > Hi! > > >> > > > > >> > > I got a report of FreeBSD guest's new uart(4) driver misbehaving in > > >> > > qemu again(?) (output stopping for no apparent reason), and now found > > >> > > out the problem is tx irqs (UART_IIR_THRI) are getting lost because > > >> > > serial_update_irq() checks for the rx condtion, > > >> > > ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > > >> > > first before checking for the tx irq condition, > > >> > > ... if ((s->ier & UART_IER_THRI) && s->thr_ipending) > > >> > > which at least in this case (FreeBSD 8 guest after doing > > >> > > set console="comconsole" > > >> > > at the loader prompt or when simply echo'ing text to /dev/ttyu0 > > >> > > or typing to the serial port from cu(1) on a `regular' vga console) > > >> > > causes the second condition (.. && s->thr_ipending) to be never > > >> > > reached anymore, or only after a very long delay. Moving that > > >> > > condition up so it is checked first like this, > > >> > > > > >> > > Index: qemu/hw/serial.c > > >> > > @@ -189,7 +188,9 @@ static void serial_update_irq(SerialStat > > >> > > { > > >> > > uint8_t tmp_iir = UART_IIR_NO_INT; > > >> > > > > >> > > - if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > > >> > > + if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > > >> > > + tmp_iir = UART_IIR_THRI; > > >> > > + } else if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > > >> > > tmp_iir = UART_IIR_RLSI; > > >> > > } else if ((s->ier & UART_IER_RDI) && s->timeout_ipending) { > > >> > > /* Note that(s->ier & UART_IER_RDI) can mask this interrupt, > > >> > > @@ -202,8 +203,6 @@ static void serial_update_irq(SerialStat > > >> > > } else if (s->recv_fifo.count >= s->recv_fifo.itl) { > > >> > > tmp_iir = UART_IIR_RDI; > > >> > > } > > >> > > - } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > > >> > > - tmp_iir = UART_IIR_THRI; > > >> > > } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { > > >> > > tmp_iir = UART_IIR_MSI; > > >> > > } > > >> > > > > >> > > ...fixes the issue for me, but I'm not 100% sure if this might cause > > >> > > rx irqs to come (too?) late when a guest keeps sending while its > > >> > > receiving at the same time. Anyone care to comment? :) > > >> > > > >> > The reordering violates the 16550A spec in that RX event overrules TX in > > >> > the IRQ status register. Maybe something else is wrong but it's not the > > >> > ordering in serial_update_irq. > > >> > > >> Well one problem seems to be the rx condition, > > >> ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > > >> is not enough to trigger an irq, yet still causes the following > > >> conditions not to be checked anymore at all. And ideed, fixing that > > >> seems to get my FreeBSD 8 guest back to working order as well: > > > > > >Applied. In the future, could you please make sure to send patches with > > >a correct unified headers? > > > > Alright, if thats is what you guys prefer... (I just didn't want to > > break the thread.) > > Don't need to break the thread, just use > > | --- a/hw/serial.c > | +++ a/hw/serial.c > > instead of simply > > | Index: qemu/hw/serial.c > > ie the output of diff -u > Oh ok, then I completely misunderstood. (And I had no idea this makes a difference... :) Anyway, I'll try to remember. > > Anyway, I guess this is also material for the stable branch(es)? > > (I just saw 0.11.0 has already been tagged but not announced yet, and > > another patch merged to the same branch after that, maybe the tag can > > still be slided if this is possible with git?) > > > > Pushed to the branch. For the details about the release, I let Anthony > handling that. Worst case scenario, it will be in 0.11.1. > Yeah, looks like it... Thanx, Juergen ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-09-24 21:39 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-09-11 21:35 [Qemu-devel] qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) Juergen Lock 2009-09-12 11:20 ` [Qemu-devel] " Olivier Cochard-Labbé 2009-09-12 12:26 ` Jan Kiszka 2009-09-12 16:52 ` Juergen Lock 2009-09-12 17:00 ` Jan Kiszka 2009-09-14 16:59 ` Stefano Stabellini 2009-09-16 19:01 ` Aurelien Jarno 2009-09-23 18:47 ` Juergen Lock 2009-09-24 16:20 ` Aurelien Jarno 2009-09-24 21:26 ` Juergen Lock
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.