| Message ID | 20171024170922.17207-3-andre.przywara@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix SBSA UART emulation | expand |
On Tue, 24 Oct 2017, Andre Przywara wrote: > From: Bhupinder Thakur <bhupinder.thakur@linaro.org> > > With the current SBSA UART emulation, streaming larger amounts of data > (as caused by "find /", for instance) can lead to character loses. ^ losses > This is due to the OUT ring buffer getting full, because we change the > TX interrupt bit only when the FIFO is actually full, and not already > when it's half-way filled, as the Linux driver expects. > The SBSA spec does not explicitly state this, but we assume that an > SBSA compliant UART uses the PL011 default "interrupt FIFO level select > register" value of "1/2 way". The Linux driver certainly makes this > assumption, so it expect to be able to write a number of characters > after the TX interrupt line has been asserted. > On a similar issue we have the same wrong behaviour on the receive side. > However changing the RX interrupt to trigger on reaching half of the FIFO > level will lead to lag, because the guest would not be notified of incoming > characters until the FIFO is half way filled. This leads to inacceptible > lags when typing on a terminal. > Real hardware solves this issue by using the "receive timeout > interrupt" (RTI), which is triggered when character reception stops for > 32 baud cycles. As we cannot and do not want to emulate any timing here, > we slightly abuse the timeout interrupt to notify the guest of new > characters: when a new character comes in, the RTI is asserted, when > the FIFO is cleared, the interrupt gets cleared. > > So this patch changes the emulated interrupt trigger behaviour to come > as close to real hardware as possible: the RX and TX interrupt trigger > when the FIFO gets half full / half empty, and the RTI interrupt signals > new incoming characters. > > [Andre: reword commit message, introduce receive timeout interrupt, add > comments] > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> > Reviewed-by: Andre Przywara <andre.przywara@linaro.org> > Signed-off-by: Andre Przywara <andre.przywara@linaro.org> This is good, only minor cosmetic comments. Acked-by: Stefano Stabellini <sstabellini@kernel.org> Julien, can we have the release-ack? > --- > xen/arch/arm/vpl011.c | 131 ++++++++++++++++++++++++++++++------------- > xen/include/asm-arm/vpl011.h | 2 + > 2 files changed, 94 insertions(+), 39 deletions(-) > > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c > index 0b0743679f..6d02406acf 100644 > --- a/xen/arch/arm/vpl011.c > +++ b/xen/arch/arm/vpl011.c > @@ -18,6 +18,9 @@ > > #define XEN_WANT_FLEX_CONSOLE_RING 1 > > +/* We assume the PL011 default of "1/2 way" for the FIFO trigger level. */ > +#define SBSA_UART_FIFO_LEVEL (SBSA_UART_FIFO_SIZE / 2) > + > #include <xen/errno.h> > #include <xen/event.h> > #include <xen/guest_access.h> > @@ -93,24 +96,37 @@ static uint8_t vpl011_read_data(struct domain *d) > */ > if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 ) > { > + unsigned int fifo_level; > + > data = intf->in[xencons_mask(in_cons, sizeof(intf->in))]; > in_cons += 1; > smp_mb(); > intf->in_cons = in_cons; > + > + fifo_level = xencons_queued(in_prod, in_cons, sizeof(intf->in)); This is only cosmetics but can we call xencons_queued only once? See the `if' just above. > + /* If the FIFO is now empty, we clear the receive timeout interrupt. */ > + if ( fifo_level == 0 ) > + { > + vpl011->uartfr |= RXFE; > + vpl011->uartris &= ~RTI; > + } > + > + /* If the FIFO is more than half empty, we clear the RX interrupt. */ > + if ( fifo_level < sizeof(intf->in) - SBSA_UART_FIFO_LEVEL ) > + vpl011->uartris &= ~RXI; > + > + vpl011_update_interrupt_status(d); > } > else > gprintk(XENLOG_ERR, "vpl011: Unexpected IN ring buffer empty\n"); > > - if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) == 0 ) > - { > - vpl011->uartfr |= RXFE; > - vpl011->uartris &= ~RXI; > - } > - > + /* > + * We have consumed a character or the FIFO was empty, so clear the > + * "FIFO full" bit. > + */ > vpl011->uartfr &= ~RXFF; > > - vpl011_update_interrupt_status(d); > - > VPL011_UNLOCK(d, flags); > > /* > @@ -122,6 +138,24 @@ static uint8_t vpl011_read_data(struct domain *d) > return data; > } > > +static void vpl011_update_tx_fifo_status(struct vpl011 *vpl011, > + unsigned int fifo_level) > +{ > + struct xencons_interface *intf = vpl011->ring_buf; > + unsigned int fifo_threshold = sizeof(intf->out) - SBSA_UART_FIFO_LEVEL; > + > + BUILD_BUG_ON(sizeof (intf->out) < SBSA_UART_FIFO_SIZE); > + > + /* > + * Set the TXI bit only when there is space for fifo_size/2 bytes which > + * is the trigger level for asserting/de-assterting the TX interrupt. > + */ > + if ( fifo_level <= fifo_threshold ) > + vpl011->uartris |= TXI; > + else > + vpl011->uartris &= ~TXI; > +} > + > static void vpl011_write_data(struct domain *d, uint8_t data) > { > unsigned long flags; > @@ -146,33 +180,37 @@ static void vpl011_write_data(struct domain *d, uint8_t data) > if ( xencons_queued(out_prod, out_cons, sizeof(intf->out)) != > sizeof (intf->out) ) > { > + unsigned int fifo_level; > + > intf->out[xencons_mask(out_prod, sizeof(intf->out))] = data; > out_prod += 1; > smp_wmb(); > intf->out_prod = out_prod; > - } > - else > - gprintk(XENLOG_ERR, "vpl011: Unexpected OUT ring buffer full\n"); > > - if ( xencons_queued(out_prod, out_cons, sizeof(intf->out)) == > - sizeof (intf->out) ) > - { > - vpl011->uartfr |= TXFF; > - vpl011->uartris &= ~TXI; > + fifo_level = xencons_queued(out_prod, out_cons, sizeof(intf->out)); Same here > - /* > - * This bit is set only when FIFO becomes full. This ensures that > - * the SBSA UART driver can write the early console data as fast as > - * possible, without waiting for the BUSY bit to get cleared before > - * writing each byte. > - */ > - vpl011->uartfr |= BUSY; > + if ( fifo_level == sizeof (intf->out) ) code style > + { > + vpl011->uartfr |= TXFF; > + > + /* > + * This bit is set only when FIFO becomes full. This ensures that > + * the SBSA UART driver can write the early console data as fast as > + * possible, without waiting for the BUSY bit to get cleared before > + * writing each byte. > + */ > + vpl011->uartfr |= BUSY; > + } > + > + vpl011_update_tx_fifo_status(vpl011, fifo_level); > + > + vpl011_update_interrupt_status(d); > } > + else > + gprintk(XENLOG_ERR, "vpl011: Unexpected OUT ring buffer full\n"); > > vpl011->uartfr &= ~TXFE; > > - vpl011_update_interrupt_status(d); > - > VPL011_UNLOCK(d, flags); > > /* > @@ -344,7 +382,7 @@ static void vpl011_data_avail(struct domain *d) > struct vpl011 *vpl011 = &d->arch.vpl011; > struct xencons_interface *intf = vpl011->ring_buf; > XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod; > - XENCONS_RING_IDX in_ring_qsize, out_ring_qsize; > + XENCONS_RING_IDX in_fifo_level, out_fifo_level; > > VPL011_LOCK(d, flags); > > @@ -355,28 +393,41 @@ static void vpl011_data_avail(struct domain *d) > > smp_rmb(); > > - in_ring_qsize = xencons_queued(in_prod, > + in_fifo_level = xencons_queued(in_prod, > in_cons, > sizeof(intf->in)); > > - out_ring_qsize = xencons_queued(out_prod, > + out_fifo_level = xencons_queued(out_prod, > out_cons, > sizeof(intf->out)); > > - /* Update the uart rx state if the buffer is not empty. */ > - if ( in_ring_qsize != 0 ) > - { > + /**** Update the UART RX state ****/ > + > + /* Clear the FIFO_EMPTY bit if the FIFO holds at least one character. */ > + if ( in_fifo_level > 0 ) > vpl011->uartfr &= ~RXFE; > - if ( in_ring_qsize == sizeof(intf->in) ) > - vpl011->uartfr |= RXFF; > + > + /* Set the FIFO_FULL bit if the Xen buffer is full. */ > + if ( in_fifo_level == sizeof(intf->in) ) > + vpl011->uartfr |= RXFF; > + > + /* Assert the RX interrupt if the FIFO is more than half way filled. */ > + if ( in_fifo_level >= sizeof(intf->in) - SBSA_UART_FIFO_LEVEL ) > vpl011->uartris |= RXI; > - } > > - /* Update the uart tx state if the buffer is not full. */ > - if ( out_ring_qsize != sizeof(intf->out) ) > + /* > + * If the input queue is not empty, we assert the receive timeout interrupt. > + * As we don't emulate any timing here, so we ignore the actual timeout > + * of 32 baud cycles. > + */ > + if ( in_fifo_level > 0 ) > + vpl011->uartris |= RTI; > + > + /**** Update the UART TX state ****/ > + > + if ( out_fifo_level != sizeof(intf->out) ) > { > vpl011->uartfr &= ~TXFF; > - vpl011->uartris |= TXI; > > /* > * Clear the BUSY bit as soon as space becomes available > @@ -385,12 +436,14 @@ static void vpl011_data_avail(struct domain *d) > */ > vpl011->uartfr &= ~BUSY; > > - if ( out_ring_qsize == 0 ) > - vpl011->uartfr |= TXFE; > + vpl011_update_tx_fifo_status(vpl011, out_fifo_level); > } > > vpl011_update_interrupt_status(d); > > + if ( out_fifo_level == 0 ) > + vpl011->uartfr |= TXFE; > + > VPL011_UNLOCK(d, flags); > } > > diff --git a/xen/include/asm-arm/vpl011.h b/xen/include/asm-arm/vpl011.h > index 1b583dac3c..db95ff822f 100644 > --- a/xen/include/asm-arm/vpl011.h > +++ b/xen/include/asm-arm/vpl011.h > @@ -28,6 +28,8 @@ > #define VPL011_LOCK(d,flags) spin_lock_irqsave(&(d)->arch.vpl011.lock, flags) > #define VPL011_UNLOCK(d,flags) spin_unlock_irqrestore(&(d)->arch.vpl011.lock, flags) > > +#define SBSA_UART_FIFO_SIZE 32 > + > struct vpl011 { > void *ring_buf; > struct page_info *ring_page; > -- > 2.14.1 >
Hi Stefano, On 26/10/17 00:50, Stefano Stabellini wrote: > On Tue, 24 Oct 2017, Andre Przywara wrote: >> From: Bhupinder Thakur <bhupinder.thakur@linaro.org> >> >> With the current SBSA UART emulation, streaming larger amounts of data >> (as caused by "find /", for instance) can lead to character loses. > ^ losses > > >> This is due to the OUT ring buffer getting full, because we change the >> TX interrupt bit only when the FIFO is actually full, and not already >> when it's half-way filled, as the Linux driver expects. >> The SBSA spec does not explicitly state this, but we assume that an >> SBSA compliant UART uses the PL011 default "interrupt FIFO level select >> register" value of "1/2 way". The Linux driver certainly makes this >> assumption, so it expect to be able to write a number of characters >> after the TX interrupt line has been asserted. >> On a similar issue we have the same wrong behaviour on the receive side. >> However changing the RX interrupt to trigger on reaching half of the FIFO >> level will lead to lag, because the guest would not be notified of incoming >> characters until the FIFO is half way filled. This leads to inacceptible >> lags when typing on a terminal. >> Real hardware solves this issue by using the "receive timeout >> interrupt" (RTI), which is triggered when character reception stops for >> 32 baud cycles. As we cannot and do not want to emulate any timing here, >> we slightly abuse the timeout interrupt to notify the guest of new >> characters: when a new character comes in, the RTI is asserted, when >> the FIFO is cleared, the interrupt gets cleared. >> >> So this patch changes the emulated interrupt trigger behaviour to come >> as close to real hardware as possible: the RX and TX interrupt trigger >> when the FIFO gets half full / half empty, and the RTI interrupt signals >> new incoming characters. >> >> [Andre: reword commit message, introduce receive timeout interrupt, add >> comments] >> >> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> >> Reviewed-by: Andre Przywara <andre.przywara@linaro.org> >> Signed-off-by: Andre Przywara <andre.przywara@linaro.org> > > This is good, only minor cosmetic comments. > > Acked-by: Stefano Stabellini <sstabellini@kernel.org> > > > Julien, can we have the release-ack? Sure. For the 2 patches: Release-acked-by: Julien Grall <julien.gral@linaro.org> Cheers, > > > >> --- >> xen/arch/arm/vpl011.c | 131 ++++++++++++++++++++++++++++++------------- >> xen/include/asm-arm/vpl011.h | 2 + >> 2 files changed, 94 insertions(+), 39 deletions(-) >> >> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c >> index 0b0743679f..6d02406acf 100644 >> --- a/xen/arch/arm/vpl011.c >> +++ b/xen/arch/arm/vpl011.c >> @@ -18,6 +18,9 @@ >> >> #define XEN_WANT_FLEX_CONSOLE_RING 1 >> >> +/* We assume the PL011 default of "1/2 way" for the FIFO trigger level. */ >> +#define SBSA_UART_FIFO_LEVEL (SBSA_UART_FIFO_SIZE / 2) >> + >> #include <xen/errno.h> >> #include <xen/event.h> >> #include <xen/guest_access.h> >> @@ -93,24 +96,37 @@ static uint8_t vpl011_read_data(struct domain *d) >> */ >> if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 ) >> { >> + unsigned int fifo_level; >> + >> data = intf->in[xencons_mask(in_cons, sizeof(intf->in))]; >> in_cons += 1; >> smp_mb(); >> intf->in_cons = in_cons; >> + >> + fifo_level = xencons_queued(in_prod, in_cons, sizeof(intf->in)); > > This is only cosmetics but can we call xencons_queued only once? See the `if' just above. > > >> + /* If the FIFO is now empty, we clear the receive timeout interrupt. */ >> + if ( fifo_level == 0 ) >> + { >> + vpl011->uartfr |= RXFE; >> + vpl011->uartris &= ~RTI; >> + } >> + >> + /* If the FIFO is more than half empty, we clear the RX interrupt. */ >> + if ( fifo_level < sizeof(intf->in) - SBSA_UART_FIFO_LEVEL ) >> + vpl011->uartris &= ~RXI; >> + >> + vpl011_update_interrupt_status(d); >> } >> else >> gprintk(XENLOG_ERR, "vpl011: Unexpected IN ring buffer empty\n"); >> >> - if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) == 0 ) >> - { >> - vpl011->uartfr |= RXFE; >> - vpl011->uartris &= ~RXI; >> - } >> - >> + /* >> + * We have consumed a character or the FIFO was empty, so clear the >> + * "FIFO full" bit. >> + */ >> vpl011->uartfr &= ~RXFF; >> >> - vpl011_update_interrupt_status(d); >> - >> VPL011_UNLOCK(d, flags); >> >> /* >> @@ -122,6 +138,24 @@ static uint8_t vpl011_read_data(struct domain *d) >> return data; >> } >> >> +static void vpl011_update_tx_fifo_status(struct vpl011 *vpl011, >> + unsigned int fifo_level) >> +{ >> + struct xencons_interface *intf = vpl011->ring_buf; >> + unsigned int fifo_threshold = sizeof(intf->out) - SBSA_UART_FIFO_LEVEL; >> + >> + BUILD_BUG_ON(sizeof (intf->out) < SBSA_UART_FIFO_SIZE); >> + >> + /* >> + * Set the TXI bit only when there is space for fifo_size/2 bytes which >> + * is the trigger level for asserting/de-assterting the TX interrupt. >> + */ >> + if ( fifo_level <= fifo_threshold ) >> + vpl011->uartris |= TXI; >> + else >> + vpl011->uartris &= ~TXI; >> +} >> + >> static void vpl011_write_data(struct domain *d, uint8_t data) >> { >> unsigned long flags; >> @@ -146,33 +180,37 @@ static void vpl011_write_data(struct domain *d, uint8_t data) >> if ( xencons_queued(out_prod, out_cons, sizeof(intf->out)) != >> sizeof (intf->out) ) >> { >> + unsigned int fifo_level; >> + >> intf->out[xencons_mask(out_prod, sizeof(intf->out))] = data; >> out_prod += 1; >> smp_wmb(); >> intf->out_prod = out_prod; >> - } >> - else >> - gprintk(XENLOG_ERR, "vpl011: Unexpected OUT ring buffer full\n"); >> >> - if ( xencons_queued(out_prod, out_cons, sizeof(intf->out)) == >> - sizeof (intf->out) ) >> - { >> - vpl011->uartfr |= TXFF; >> - vpl011->uartris &= ~TXI; >> + fifo_level = xencons_queued(out_prod, out_cons, sizeof(intf->out)); > > Same here > > >> - /* >> - * This bit is set only when FIFO becomes full. This ensures that >> - * the SBSA UART driver can write the early console data as fast as >> - * possible, without waiting for the BUSY bit to get cleared before >> - * writing each byte. >> - */ >> - vpl011->uartfr |= BUSY; >> + if ( fifo_level == sizeof (intf->out) ) > > code style > > >> + { >> + vpl011->uartfr |= TXFF; >> + >> + /* >> + * This bit is set only when FIFO becomes full. This ensures that >> + * the SBSA UART driver can write the early console data as fast as >> + * possible, without waiting for the BUSY bit to get cleared before >> + * writing each byte. >> + */ >> + vpl011->uartfr |= BUSY; >> + } >> + >> + vpl011_update_tx_fifo_status(vpl011, fifo_level); >> + >> + vpl011_update_interrupt_status(d); >> } >> + else >> + gprintk(XENLOG_ERR, "vpl011: Unexpected OUT ring buffer full\n"); >> >> vpl011->uartfr &= ~TXFE; >> >> - vpl011_update_interrupt_status(d); >> - >> VPL011_UNLOCK(d, flags); >> >> /* >> @@ -344,7 +382,7 @@ static void vpl011_data_avail(struct domain *d) >> struct vpl011 *vpl011 = &d->arch.vpl011; >> struct xencons_interface *intf = vpl011->ring_buf; >> XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod; >> - XENCONS_RING_IDX in_ring_qsize, out_ring_qsize; >> + XENCONS_RING_IDX in_fifo_level, out_fifo_level; >> >> VPL011_LOCK(d, flags); >> >> @@ -355,28 +393,41 @@ static void vpl011_data_avail(struct domain *d) >> >> smp_rmb(); >> >> - in_ring_qsize = xencons_queued(in_prod, >> + in_fifo_level = xencons_queued(in_prod, >> in_cons, >> sizeof(intf->in)); >> >> - out_ring_qsize = xencons_queued(out_prod, >> + out_fifo_level = xencons_queued(out_prod, >> out_cons, >> sizeof(intf->out)); >> >> - /* Update the uart rx state if the buffer is not empty. */ >> - if ( in_ring_qsize != 0 ) >> - { >> + /**** Update the UART RX state ****/ >> + >> + /* Clear the FIFO_EMPTY bit if the FIFO holds at least one character. */ >> + if ( in_fifo_level > 0 ) >> vpl011->uartfr &= ~RXFE; >> - if ( in_ring_qsize == sizeof(intf->in) ) >> - vpl011->uartfr |= RXFF; >> + >> + /* Set the FIFO_FULL bit if the Xen buffer is full. */ >> + if ( in_fifo_level == sizeof(intf->in) ) >> + vpl011->uartfr |= RXFF; >> + >> + /* Assert the RX interrupt if the FIFO is more than half way filled. */ >> + if ( in_fifo_level >= sizeof(intf->in) - SBSA_UART_FIFO_LEVEL ) >> vpl011->uartris |= RXI; >> - } >> >> - /* Update the uart tx state if the buffer is not full. */ >> - if ( out_ring_qsize != sizeof(intf->out) ) >> + /* >> + * If the input queue is not empty, we assert the receive timeout interrupt. >> + * As we don't emulate any timing here, so we ignore the actual timeout >> + * of 32 baud cycles. >> + */ >> + if ( in_fifo_level > 0 ) >> + vpl011->uartris |= RTI; >> + >> + /**** Update the UART TX state ****/ >> + >> + if ( out_fifo_level != sizeof(intf->out) ) >> { >> vpl011->uartfr &= ~TXFF; >> - vpl011->uartris |= TXI; >> >> /* >> * Clear the BUSY bit as soon as space becomes available >> @@ -385,12 +436,14 @@ static void vpl011_data_avail(struct domain *d) >> */ >> vpl011->uartfr &= ~BUSY; >> >> - if ( out_ring_qsize == 0 ) >> - vpl011->uartfr |= TXFE; >> + vpl011_update_tx_fifo_status(vpl011, out_fifo_level); >> } >> >> vpl011_update_interrupt_status(d); >> >> + if ( out_fifo_level == 0 ) >> + vpl011->uartfr |= TXFE; >> + >> VPL011_UNLOCK(d, flags); >> } >> >> diff --git a/xen/include/asm-arm/vpl011.h b/xen/include/asm-arm/vpl011.h >> index 1b583dac3c..db95ff822f 100644 >> --- a/xen/include/asm-arm/vpl011.h >> +++ b/xen/include/asm-arm/vpl011.h >> @@ -28,6 +28,8 @@ >> #define VPL011_LOCK(d,flags) spin_lock_irqsave(&(d)->arch.vpl011.lock, flags) >> #define VPL011_UNLOCK(d,flags) spin_unlock_irqrestore(&(d)->arch.vpl011.lock, flags) >> >> +#define SBSA_UART_FIFO_SIZE 32 >> + >> struct vpl011 { >> void *ring_buf; >> struct page_info *ring_page; >> -- >> 2.14.1 >>
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c index 0b0743679f..6d02406acf 100644 --- a/xen/arch/arm/vpl011.c +++ b/xen/arch/arm/vpl011.c @@ -18,6 +18,9 @@ #define XEN_WANT_FLEX_CONSOLE_RING 1 +/* We assume the PL011 default of "1/2 way" for the FIFO trigger level. */ +#define SBSA_UART_FIFO_LEVEL (SBSA_UART_FIFO_SIZE / 2) + #include <xen/errno.h> #include <xen/event.h> #include <xen/guest_access.h> @@ -93,24 +96,37 @@ static uint8_t vpl011_read_data(struct domain *d) */ if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 ) { + unsigned int fifo_level; + data = intf->in[xencons_mask(in_cons, sizeof(intf->in))]; in_cons += 1; smp_mb(); intf->in_cons = in_cons; + + fifo_level = xencons_queued(in_prod, in_cons, sizeof(intf->in)); + + /* If the FIFO is now empty, we clear the receive timeout interrupt. */ + if ( fifo_level == 0 ) + { + vpl011->uartfr |= RXFE; + vpl011->uartris &= ~RTI; + } + + /* If the FIFO is more than half empty, we clear the RX interrupt. */ + if ( fifo_level < sizeof(intf->in) - SBSA_UART_FIFO_LEVEL ) + vpl011->uartris &= ~RXI; + + vpl011_update_interrupt_status(d); } else gprintk(XENLOG_ERR, "vpl011: Unexpected IN ring buffer empty\n"); - if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) == 0 ) - { - vpl011->uartfr |= RXFE; - vpl011->uartris &= ~RXI; - } - + /* + * We have consumed a character or the FIFO was empty, so clear the + * "FIFO full" bit. + */ vpl011->uartfr &= ~RXFF; - vpl011_update_interrupt_status(d); - VPL011_UNLOCK(d, flags); /* @@ -122,6 +138,24 @@ static uint8_t vpl011_read_data(struct domain *d) return data; } +static void vpl011_update_tx_fifo_status(struct vpl011 *vpl011, + unsigned int fifo_level) +{ + struct xencons_interface *intf = vpl011->ring_buf; + unsigned int fifo_threshold = sizeof(intf->out) - SBSA_UART_FIFO_LEVEL; + + BUILD_BUG_ON(sizeof (intf->out) < SBSA_UART_FIFO_SIZE); + + /* + * Set the TXI bit only when there is space for fifo_size/2 bytes which + * is the trigger level for asserting/de-assterting the TX interrupt. + */ + if ( fifo_level <= fifo_threshold ) + vpl011->uartris |= TXI; + else + vpl011->uartris &= ~TXI; +} + static void vpl011_write_data(struct domain *d, uint8_t data) { unsigned long flags; @@ -146,33 +180,37 @@ static void vpl011_write_data(struct domain *d, uint8_t data) if ( xencons_queued(out_prod, out_cons, sizeof(intf->out)) != sizeof (intf->out) ) { + unsigned int fifo_level; + intf->out[xencons_mask(out_prod, sizeof(intf->out))] = data; out_prod += 1; smp_wmb(); intf->out_prod = out_prod; - } - else - gprintk(XENLOG_ERR, "vpl011: Unexpected OUT ring buffer full\n"); - if ( xencons_queued(out_prod, out_cons, sizeof(intf->out)) == - sizeof (intf->out) ) - { - vpl011->uartfr |= TXFF; - vpl011->uartris &= ~TXI; + fifo_level = xencons_queued(out_prod, out_cons, sizeof(intf->out)); - /* - * This bit is set only when FIFO becomes full. This ensures that - * the SBSA UART driver can write the early console data as fast as - * possible, without waiting for the BUSY bit to get cleared before - * writing each byte. - */ - vpl011->uartfr |= BUSY; + if ( fifo_level == sizeof (intf->out) ) + { + vpl011->uartfr |= TXFF; + + /* + * This bit is set only when FIFO becomes full. This ensures that + * the SBSA UART driver can write the early console data as fast as + * possible, without waiting for the BUSY bit to get cleared before + * writing each byte. + */ + vpl011->uartfr |= BUSY; + } + + vpl011_update_tx_fifo_status(vpl011, fifo_level); + + vpl011_update_interrupt_status(d); } + else + gprintk(XENLOG_ERR, "vpl011: Unexpected OUT ring buffer full\n"); vpl011->uartfr &= ~TXFE; - vpl011_update_interrupt_status(d); - VPL011_UNLOCK(d, flags); /* @@ -344,7 +382,7 @@ static void vpl011_data_avail(struct domain *d) struct vpl011 *vpl011 = &d->arch.vpl011; struct xencons_interface *intf = vpl011->ring_buf; XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod; - XENCONS_RING_IDX in_ring_qsize, out_ring_qsize; + XENCONS_RING_IDX in_fifo_level, out_fifo_level; VPL011_LOCK(d, flags); @@ -355,28 +393,41 @@ static void vpl011_data_avail(struct domain *d) smp_rmb(); - in_ring_qsize = xencons_queued(in_prod, + in_fifo_level = xencons_queued(in_prod, in_cons, sizeof(intf->in)); - out_ring_qsize = xencons_queued(out_prod, + out_fifo_level = xencons_queued(out_prod, out_cons, sizeof(intf->out)); - /* Update the uart rx state if the buffer is not empty. */ - if ( in_ring_qsize != 0 ) - { + /**** Update the UART RX state ****/ + + /* Clear the FIFO_EMPTY bit if the FIFO holds at least one character. */ + if ( in_fifo_level > 0 ) vpl011->uartfr &= ~RXFE; - if ( in_ring_qsize == sizeof(intf->in) ) - vpl011->uartfr |= RXFF; + + /* Set the FIFO_FULL bit if the Xen buffer is full. */ + if ( in_fifo_level == sizeof(intf->in) ) + vpl011->uartfr |= RXFF; + + /* Assert the RX interrupt if the FIFO is more than half way filled. */ + if ( in_fifo_level >= sizeof(intf->in) - SBSA_UART_FIFO_LEVEL ) vpl011->uartris |= RXI; - } - /* Update the uart tx state if the buffer is not full. */ - if ( out_ring_qsize != sizeof(intf->out) ) + /* + * If the input queue is not empty, we assert the receive timeout interrupt. + * As we don't emulate any timing here, so we ignore the actual timeout + * of 32 baud cycles. + */ + if ( in_fifo_level > 0 ) + vpl011->uartris |= RTI; + + /**** Update the UART TX state ****/ + + if ( out_fifo_level != sizeof(intf->out) ) { vpl011->uartfr &= ~TXFF; - vpl011->uartris |= TXI; /* * Clear the BUSY bit as soon as space becomes available @@ -385,12 +436,14 @@ static void vpl011_data_avail(struct domain *d) */ vpl011->uartfr &= ~BUSY; - if ( out_ring_qsize == 0 ) - vpl011->uartfr |= TXFE; + vpl011_update_tx_fifo_status(vpl011, out_fifo_level); } vpl011_update_interrupt_status(d); + if ( out_fifo_level == 0 ) + vpl011->uartfr |= TXFE; + VPL011_UNLOCK(d, flags); } diff --git a/xen/include/asm-arm/vpl011.h b/xen/include/asm-arm/vpl011.h index 1b583dac3c..db95ff822f 100644 --- a/xen/include/asm-arm/vpl011.h +++ b/xen/include/asm-arm/vpl011.h @@ -28,6 +28,8 @@ #define VPL011_LOCK(d,flags) spin_lock_irqsave(&(d)->arch.vpl011.lock, flags) #define VPL011_UNLOCK(d,flags) spin_unlock_irqrestore(&(d)->arch.vpl011.lock, flags) +#define SBSA_UART_FIFO_SIZE 32 + struct vpl011 { void *ring_buf; struct page_info *ring_page;