1. Patchew
  2. Xen
  3. View series

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

dmkhn@proton.me posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250211075405.191144-1-dmkhn@proton.me
There is a newer version of this series
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(-)
[PATCH] arm/vuart: move vpl011-related code to vpl011 emulator
Posted by dmkhn@proton.me 1 month ago 
From: Denis Mukhin <dmukhin@ford.com>

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.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Link to the original patch:
  https://lore.kernel.org/xen-devel/20250103-vuart-ns8250-v3-v1-2-c5d36b31d66c@ford.com/
---
 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..5e6f0fb062 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 )
+                printk(KERN_WARNING "%pd: failed to process console input: %d\n",
+                       d, rc);
             rcu_unlock_domain(d);
+        }
 
         break;
     }
-- 
2.34.1
Re: [PATCH] arm/vuart: move vpl011-related code to vpl011 emulator
Posted by Stefano Stabellini 4 weeks, 1 day ago 
On Tue, 11 Feb 2025, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> 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.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Link to the original patch:
>   https://lore.kernel.org/xen-devel/20250103-vuart-ns8250-v3-v1-2-c5d36b31d66c@ford.com/
> ---
>  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;

Everything else looks fine. I am a bit unsure about this -ENOSPC return
because...


>      }
>  
>      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..5e6f0fb062 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 )
> +                printk(KERN_WARNING "%pd: failed to process console input: %d\n",
> +                       d, rc);

... it could trigger a warning here. And any prink triggerable by the
guest should be rate limited. We already have a function for that which
is guest_printk. So I think we should probably change this warning to be
guest_printk. Or change return -ENODEV into return 0.


>              rcu_unlock_domain(d);
> +        }
>  
>          break;
>      }
> -- 
> 2.34.1
> 
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.