Re: [PATCH v2] arm/vuart: move vpl011-related code to vpl011 emulator

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 12 Feb 2025, dmkhn@xxxxxxxxx wrote:
> Xen console driver has vpl011-related logic which shall belong vpl011 emulator
> code (Arm port). Move vpl011-related code from arch-independent console driver
> to Arm's vpl011.c.
> 
> Use rate-limiting guest_printk() for error logging in console driver in case
> vpl011 cannot handle the console input.
> 
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> Changes in v2:
> - switched to rate-limited guest_printk()
> - Link to v1: 
> https://lore.kernel.org/xen-devel/20250211075405.191144-1-dmkhn@xxxxxxxxx/
> - Link to CI: 
> https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1668396058
> ---
>  xen/arch/arm/include/asm/vpl011.h |  2 +-
>  xen/arch/arm/vpl011.c             | 15 +++++++++++----
>  xen/drivers/char/console.c        | 21 +++++++--------------
>  3 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/vpl011.h 
> b/xen/arch/arm/include/asm/vpl011.h
> index c09abcd7a9..cc83868281 100644
> --- a/xen/arch/arm/include/asm/vpl011.h
> +++ b/xen/arch/arm/include/asm/vpl011.h
> @@ -69,7 +69,7 @@ struct vpl011_init_info {
>  int domain_vpl011_init(struct domain *d,
>                         struct vpl011_init_info *info);
>  void domain_vpl011_deinit(struct domain *d);
> -void vpl011_rx_char_xen(struct domain *d, char c);
> +int vpl011_rx_char_xen(struct domain *d, char c);
>  #else
>  static inline int domain_vpl011_init(struct domain *d,
>                                       struct vpl011_init_info *info)
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index 1fc3114cce..c72f3778bf 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -567,16 +567,21 @@ static void vpl011_data_avail(struct domain *d,
>  
>  /*
>   * vpl011_rx_char_xen adds a char to a domain's vpl011 receive buffer.
> - * It is only used when the vpl011 backend is in Xen.
>   */
> -void vpl011_rx_char_xen(struct domain *d, char c)
> +int vpl011_rx_char_xen(struct domain *d, char c)
>  {
>      unsigned long flags;
>      struct vpl011 *vpl011 = &d->arch.vpl011;
>      struct vpl011_xen_backend *intf = vpl011->backend.xen;
>      XENCONS_RING_IDX in_cons, in_prod, in_fifo_level;
>  
> -    ASSERT(!vpl011->backend_in_domain);
> +    /* Forward input iff the vpl011 backend is in Xen. */
> +    if ( vpl011->backend_in_domain )
> +        return -ENODEV;
> +
> +    if ( intf == NULL )
> +        return -ENODEV;
> +
>      VPL011_LOCK(d, flags);
>  
>      in_cons = intf->in_cons;
> @@ -584,7 +589,7 @@ void vpl011_rx_char_xen(struct domain *d, char c)
>      if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) == 
> sizeof(intf->in) )
>      {
>          VPL011_UNLOCK(d, flags);
> -        return;
> +        return -ENOSPC;
>      }
>  
>      intf->in[xencons_mask(in_prod, sizeof(intf->in))] = c;
> @@ -596,6 +601,8 @@ void vpl011_rx_char_xen(struct domain *d, char c)
>  
>      vpl011_data_avail(d, in_fifo_level, sizeof(intf->in), 0, 
> SBSA_UART_FIFO_SIZE);
>      VPL011_UNLOCK(d, flags);
> +
> +    return 0;
>  }
>  
>  static void vpl011_notification(struct vcpu *v, unsigned int port)
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index b4cec77247..ee5f720de4 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -553,21 +553,14 @@ static void __serial_rx(char c)
>      {
>          struct domain *d = rcu_lock_domain_by_id(console_rx - 1);
>  
> -        /*
> -         * If we have a properly initialized vpl011 console for the
> -         * domain, without a full PV ring to Dom0 (in that case input
> -         * comes from the PV ring), then send the character to it.
> -         */
> -        if ( d != NULL &&
> -             !d->arch.vpl011.backend_in_domain &&
> -             d->arch.vpl011.backend.xen != NULL )
> -            vpl011_rx_char_xen(d, c);
> -        else
> -            printk("Cannot send chars to Dom%d: no UART available\n",
> -                   console_rx - 1);
> -
> -        if ( d != NULL )
> +        if ( d )
> +        {
> +            int rc = vpl011_rx_char_xen(d, c);
> +            if ( rc )
> +                guest_printk(d, XENLOG_G_WARNING
> +                                "failed to process console input: %d\n", rc);
>              rcu_unlock_domain(d);
> +        }
>  
>          break;
>      }
> -- 
> 2.34.1
> 
>