Re: [PATCH 3/3] xen/arm: vpl011: Do not try to handle TX FIFO status when backend in Xen

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 5 Apr 2023, Michal Orzel wrote:
> >From vpl011_rx_char_xen(), we call vpl011_data_avail() that handles both
> RX and TX state. Because we are passing 0 as out_fifo_level and
> SBSA_UART_FIFO_SIZE as out_size, we end up calling a function
> vpl011_update_tx_fifo_status() which performs TXI bit handling
> depending on the FIFO trigger level. This does not make sense when backend
> is in Xen, as we maintain a single TX state where data can always be
> written and as such there is no TX FIFO handling. Furthermore, this
> function assumes that the backend is in domain by making use of struct
> xencons_interface unconditionally. Fix it by calling this function only
> when backend is in domain. Also add an assert for sanity.
> 
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>  xen/arch/arm/vpl011.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index ff06deeb645c..7856b4b5f5a3 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -261,6 +261,9 @@ static void vpl011_update_tx_fifo_status(struct vpl011 
> *vpl011,
>      struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
>      unsigned int fifo_threshold = sizeof(intf->out) - SBSA_UART_FIFO_LEVEL;
>  
> +    /* No TX FIFO handling when backend is in Xen */
> +    ASSERT(vpl011->backend_in_domain);
> +
>      BUILD_BUG_ON(sizeof(intf->out) < SBSA_UART_FIFO_SIZE);
>  
>      /*
> @@ -547,7 +550,13 @@ static void vpl011_data_avail(struct domain *d,
>           */
>          vpl011->uartfr &= ~BUSY;
>  
> -        vpl011_update_tx_fifo_status(vpl011, out_fifo_level);
> +        /*
> +         * When backend is in Xen, we are always ready for new data to be
> +         * written (i.e. no TX FIFO handling), therefore we do not want
> +         * to change the TX FIFO status in such case.
> +         */
> +        if ( vpl011->backend_in_domain )
> +            vpl011_update_tx_fifo_status(vpl011, out_fifo_level);
>      }
>  
>      vpl011_update_interrupt_status(d);
> -- 
> 2.25.1
>