Re: [PATCH 2/3] xen/arm: vpl011: Handle correctly TXFE when backend in Xen

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

> ---
> 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
>