| Message ID | 20230405111750.12491-3-michal.orzel@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: vpl011 fixes continuation | expand |
On Wed, 5 Apr 2023, Michal Orzel wrote: > When backend is in Xen, the handling of data written to DR register is a > bit special because we want to tell guest that we are always ready for new > data to be written (i.e. no real FIFO, TXFF/BUSY never set and TXI always > set). This conflicts with the current handling of TXFE bit, which we > always clear and never set on a write path (we happen to set it when we > receive char from serial input due to use of vpl011_data_avail() but this > might never be called). This can lead to issues if a guest driver makes > use of TXFE bit to check for TX transmission completion (such guest could > then wait endlessly). Fix it by keeping TXFE always set to match the > current emulation logic. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > We don't have to look far for an example of a PL011/SBSA driver relying on TXFE. > If a guest had a driver like we have in Xen, we would end up with no messages > being printed. > --- > xen/arch/arm/vpl011.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c > index 0186d8a31834..ff06deeb645c 100644 > --- a/xen/arch/arm/vpl011.c > +++ b/xen/arch/arm/vpl011.c > @@ -112,8 +112,14 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data) > } > } > > + /* > + * When backend is in Xen, we tell guest we are always ready for new data > + * to be written. This is fulfilled by having: > + * - TXI/TXFE -> always set, > + * - TXFF/BUSY -> never set. > + */ > vpl011->uartris |= TXI; > - vpl011->uartfr &= ~TXFE; > + vpl011->uartfr |= TXFE; > vpl011_update_interrupt_status(d); > > VPL011_UNLOCK(d, flags); > -- > 2.25.1 >
Hi Michal, > -----Original Message----- > Subject: [PATCH 2/3] xen/arm: vpl011: Handle correctly TXFE when backend in > Xen > > When backend is in Xen, the handling of data written to DR register is a > bit special because we want to tell guest that we are always ready for new > data to be written (i.e. no real FIFO, TXFF/BUSY never set and TXI always > set). This conflicts with the current handling of TXFE bit, which we > always clear and never set on a write path (we happen to set it when we > receive char from serial input due to use of vpl011_data_avail() but this > might never be called). This can lead to issues if a guest driver makes > use of TXFE bit to check for TX transmission completion (such guest could > then wait endlessly). Fix it by keeping TXFE always set to match the > current emulation logic. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Henry Wang <Henry.Wang@arm.com> Tested-by: Henry Wang <Henry.Wang@arm.com> Kind regards, Henry
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c index 0186d8a31834..ff06deeb645c 100644 --- a/xen/arch/arm/vpl011.c +++ b/xen/arch/arm/vpl011.c @@ -112,8 +112,14 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data) } } + /* + * When backend is in Xen, we tell guest we are always ready for new data + * to be written. This is fulfilled by having: + * - TXI/TXFE -> always set, + * - TXFF/BUSY -> never set. + */ vpl011->uartris |= TXI; - vpl011->uartfr &= ~TXFE; + vpl011->uartfr |= TXFE; vpl011_update_interrupt_status(d); VPL011_UNLOCK(d, flags);
When backend is in Xen, the handling of data written to DR register is a bit special because we want to tell guest that we are always ready for new data to be written (i.e. no real FIFO, TXFF/BUSY never set and TXI always set). This conflicts with the current handling of TXFE bit, which we always clear and never set on a write path (we happen to set it when we receive char from serial input due to use of vpl011_data_avail() but this might never be called). This can lead to issues if a guest driver makes use of TXFE bit to check for TX transmission completion (such guest could then wait endlessly). Fix it by keeping TXFE always set to match the current emulation logic. Signed-off-by: Michal Orzel <michal.orzel@amd.com> --- We don't have to look far for an example of a PL011/SBSA driver relying on TXFE. If a guest had a driver like we have in Xen, we would end up with no messages being printed. --- xen/arch/arm/vpl011.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)