[PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls

dmkhn@proton.me posted 16 patches 4 months, 1 week ago
[PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls
Posted by dmkhn@proton.me 4 months, 1 week ago
From: Denis Mukhin <dmukhin@ford.com> 

Use generic names prefixed with 'vuart_' in public PL011 emulator data
structures and functions.

No functional change intended.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/arm/dom0less-build.c     |  4 ++--
 xen/arch/arm/domain.c             |  3 ++-
 xen/arch/arm/domctl.c             | 14 +++++++------
 xen/arch/arm/include/asm/vpl011.h | 20 ------------------
 xen/arch/arm/vpl011.c             | 24 +++++++++++-----------
 xen/drivers/char/console.c        |  6 ++----
 xen/include/xen/vuart.h           | 34 ++++++++++++++++++++++++++++++-
 7 files changed, 59 insertions(+), 46 deletions(-)

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 7c1b59750fb5..11b8498d3b22 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -216,7 +216,7 @@ int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
      */
     if ( kinfo->arch.vpl011 )
     {
-        rc = domain_vpl011_init(d, NULL);
+        rc = vuart_init(d, NULL);
         if ( rc < 0 )
             return rc;
     }
@@ -247,7 +247,7 @@ void __init arch_create_domUs(struct dt_device_node *node,
          * d->arch.vpl011.irq. So the logic to find the vIRQ has to
          * be hardcoded.
          * The logic here shall be consistent with the one in
-         * domain_vpl011_init().
+         * vuart_init().
          */
         if ( flags & CDF_directmap )
         {
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index be58a23dd725..68297e619bad 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -11,6 +11,7 @@
 #include <xen/sched.h>
 #include <xen/softirq.h>
 #include <xen/wait.h>
+#include <xen/vuart.h>
 
 #include <asm/arm64/sve.h>
 #include <asm/cpuerrata.h>
@@ -1072,7 +1073,7 @@ int domain_relinquish_resources(struct domain *d)
          * Release the resources allocated for vpl011 which were
          * allocated via a DOMCTL call XEN_DOMCTL_vuart_op.
          */
-        domain_vpl011_deinit(d);
+        vuart_exit(d);
 
 #ifdef CONFIG_IOREQ_SERVER
         ioreq_server_destroy_all(d);
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index ad914c915f81..dde25ceff6d0 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -14,6 +14,7 @@
 #include <xen/mm.h>
 #include <xen/sched.h>
 #include <xen/types.h>
+#include <xen/vuart.h>
 #include <xsm/xsm.h>
 #include <public/domctl.h>
 
@@ -30,10 +31,11 @@ static int handle_vuart_init(struct domain *d,
                              struct xen_domctl_vuart_op *vuart_op)
 {
     int rc;
-    struct vpl011_init_info info;
-
-    info.console_domid = vuart_op->console_domid;
-    info.gfn = _gfn(vuart_op->gfn);
+    struct vuart_params params = {
+        .console_domid = vuart_op->console_domid,
+        .gfn = _gfn(vuart_op->gfn),
+        .evtchn = 0,
+    };
 
     if ( d->creation_finished )
         return -EPERM;
@@ -41,10 +43,10 @@ static int handle_vuart_init(struct domain *d,
     if ( vuart_op->type != XEN_DOMCTL_VUART_TYPE_VPL011 )
         return -EOPNOTSUPP;
 
-    rc = domain_vpl011_init(d, &info);
+    rc = vuart_init(d, &params);
 
     if ( !rc )
-        vuart_op->evtchn = info.evtchn;
+        vuart_op->evtchn = params.evtchn;
 
     return rc;
 }
diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
index be64883b8628..5c308cc8c148 100644
--- a/xen/arch/arm/include/asm/vpl011.h
+++ b/xen/arch/arm/include/asm/vpl011.h
@@ -59,26 +59,6 @@ struct vpl011 {
     evtchn_port_t evtchn;
 };
 
-struct vpl011_init_info {
-    domid_t console_domid;
-    gfn_t gfn;
-    evtchn_port_t evtchn;
-};
-
-#ifdef CONFIG_HAS_VUART_PL011
-int domain_vpl011_init(struct domain *d,
-                       struct vpl011_init_info *info);
-void domain_vpl011_deinit(struct domain *d);
-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)
-{
-    return -ENOSYS;
-}
-
-static inline void domain_vpl011_deinit(struct domain *d) { }
-#endif
 #endif  /* _VPL011_H_ */
 
 /*
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index cafc532cf028..2cf88a70ecdb 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -134,7 +134,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
 
 /*
  * vpl011_read_data_xen reads data when the backend is xen. Characters
- * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
+ * are added to the vpl011 receive buffer by vuart_putchar.
  */
 static uint8_t vpl011_read_data_xen(struct domain *d)
 {
@@ -571,9 +571,9 @@ static void vpl011_data_avail(struct domain *d,
 }
 
 /*
- * vpl011_rx_char_xen adds a char to a domain's vpl011 receive buffer.
+ * vuart_putchar adds a char to a domain's vpl011 receive buffer.
  */
-int vpl011_rx_char_xen(struct domain *d, char c)
+int vuart_putchar(struct domain *d, char c)
 {
     unsigned long flags;
     struct vpl011 *vpl011 = &d->arch.vpl011;
@@ -642,7 +642,7 @@ static void vpl011_notification(struct vcpu *v, unsigned int port)
     VPL011_UNLOCK(d, flags);
 }
 
-int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
+int vuart_init(struct domain *d, struct vuart_params *params)
 {
     int rc;
     struct vpl011 *vpl011 = &d->arch.vpl011;
@@ -694,27 +694,27 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
     }
 
     /*
-     * info is NULL when the backend is in Xen.
-     * info is != NULL when the backend is in a domain.
+     * params is NULL when the backend is in Xen.
+     * params is != NULL when the backend is in a domain.
      */
-    if ( info != NULL )
+    if ( params != NULL )
     {
         vpl011->backend_in_domain = true;
 
         /* Map the guest PFN to Xen address space. */
         rc =  prepare_ring_for_helper(d,
-                                      gfn_x(info->gfn),
+                                      gfn_x(params->gfn),
                                       &vpl011->backend.dom.ring_page,
                                       &vpl011->backend.dom.ring_buf);
         if ( rc < 0 )
             goto out;
 
-        rc = alloc_unbound_xen_event_channel(d, 0, info->console_domid,
+        rc = alloc_unbound_xen_event_channel(d, 0, params->console_domid,
                                              vpl011_notification);
         if ( rc < 0 )
             goto out1;
 
-        vpl011->evtchn = info->evtchn = rc;
+        vpl011->evtchn = params->evtchn = rc;
     }
     else
     {
@@ -746,13 +746,13 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
     return 0;
 
 out1:
-    domain_vpl011_deinit(d);
+    vuart_exit(d);
 
 out:
     return rc;
 }
 
-void domain_vpl011_deinit(struct domain *d)
+void vuart_exit(struct domain *d)
 {
     struct vpl011 *vpl011 = &d->arch.vpl011;
 
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 0f37badc471e..f322d59515ab 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -33,13 +33,11 @@
 #include <asm/setup.h>
 #include <xen/sections.h>
 #include <xen/consoled.h>
+#include <xen/vuart.h>
 
 #ifdef CONFIG_X86
 #include <asm/guest.h>
 #endif
-#ifdef CONFIG_HAS_VUART_PL011
-#include <asm/vpl011.h>
-#endif
 
 /* Internal console flags. */
 enum {
@@ -609,7 +607,7 @@ static void __serial_rx(char c)
 #ifdef CONFIG_HAS_VUART_PL011
     else
         /* Deliver input to the emulated UART. */
-        rc = vpl011_rx_char_xen(d, c);
+        rc = vuart_putchar(d, c);
 #endif
 
     if ( consoled_is_enabled() )
diff --git a/xen/include/xen/vuart.h b/xen/include/xen/vuart.h
index bb883823ea31..cae72ac9c6b9 100644
--- a/xen/include/xen/vuart.h
+++ b/xen/include/xen/vuart.h
@@ -2,14 +2,46 @@
 #ifndef XEN_VUART_H
 #define XEN_VUART_H
 
+#include <public/xen.h>
+#include <public/event_channel.h>
+#include <xen/types.h>
+
+struct vuart_params {
+    domid_t console_domid;
+    gfn_t gfn;
+    evtchn_port_t evtchn;
+};
+
 #ifdef CONFIG_HAS_VUART_PL011
+
 int __init vuart_add_fwnode(struct domain *d, void *node);
+int vuart_init(struct domain *d, struct vuart_params *params);
+void vuart_exit(struct domain *d);
+int vuart_putchar(struct domain *d, char c);
+
 #else
+
 static inline int __init vuart_add_fwnode(struct domain *d, void *node)
 {
     return 0;
 }
-#endif
+
+static inline int vuart_init(struct domain *d, struct vuart_params *params)
+{
+    return 0;
+}
+
+static inline void vuart_exit(struct domain *d)
+{
+}
+
+static inline int vuart_putchar(struct domain *d, char c)
+{
+    ASSERT_UNREACHABLE();
+    return -ENODEV;
+}
+
+#endif /* CONFIG_HAS_VUART_PL011 */
 
 #endif /* XEN_VUART_H */
 
-- 
2.34.1
Re: [PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls
Posted by Orzel, Michal 4 months, 1 week ago

On 24/06/2025 05:55, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Use generic names prefixed with 'vuart_' in public PL011 emulator data
> structures and functions.
> 
> No functional change intended.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>  xen/arch/arm/dom0less-build.c     |  4 ++--
>  xen/arch/arm/domain.c             |  3 ++-
>  xen/arch/arm/domctl.c             | 14 +++++++------
>  xen/arch/arm/include/asm/vpl011.h | 20 ------------------
>  xen/arch/arm/vpl011.c             | 24 +++++++++++-----------
>  xen/drivers/char/console.c        |  6 ++----
>  xen/include/xen/vuart.h           | 34 ++++++++++++++++++++++++++++++-
>  7 files changed, 59 insertions(+), 46 deletions(-)
> 
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index 7c1b59750fb5..11b8498d3b22 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -216,7 +216,7 @@ int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
As can be seen here ...

>       */
>      if ( kinfo->arch.vpl011 )
>      {
> -        rc = domain_vpl011_init(d, NULL);
> +        rc = vuart_init(d, NULL);
we end up with init_vuart() and vuart_init(). That's quite confusing. Maybe
domain_vuart_init() or alike?

>          if ( rc < 0 )
>              return rc;
>      }
> @@ -247,7 +247,7 @@ void __init arch_create_domUs(struct dt_device_node *node,
>           * d->arch.vpl011.irq. So the logic to find the vIRQ has to
>           * be hardcoded.
>           * The logic here shall be consistent with the one in
> -         * domain_vpl011_init().
> +         * vuart_init().
>           */
>          if ( flags & CDF_directmap )
>          {
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index be58a23dd725..68297e619bad 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -11,6 +11,7 @@
>  #include <xen/sched.h>
>  #include <xen/softirq.h>
>  #include <xen/wait.h>
> +#include <xen/vuart.h>
>  
>  #include <asm/arm64/sve.h>
>  #include <asm/cpuerrata.h>
> @@ -1072,7 +1073,7 @@ int domain_relinquish_resources(struct domain *d)
>           * Release the resources allocated for vpl011 which were
>           * allocated via a DOMCTL call XEN_DOMCTL_vuart_op.
>           */
> -        domain_vpl011_deinit(d);
> +        vuart_exit(d);
IMO, deinit is more meaningful here.

>  
>  #ifdef CONFIG_IOREQ_SERVER
>          ioreq_server_destroy_all(d);
> diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> index ad914c915f81..dde25ceff6d0 100644
> --- a/xen/arch/arm/domctl.c
> +++ b/xen/arch/arm/domctl.c
> @@ -14,6 +14,7 @@
>  #include <xen/mm.h>
>  #include <xen/sched.h>
>  #include <xen/types.h>
> +#include <xen/vuart.h>
>  #include <xsm/xsm.h>
>  #include <public/domctl.h>
>  
> @@ -30,10 +31,11 @@ static int handle_vuart_init(struct domain *d,
>                               struct xen_domctl_vuart_op *vuart_op)
>  {
>      int rc;
> -    struct vpl011_init_info info;
> -
> -    info.console_domid = vuart_op->console_domid;
> -    info.gfn = _gfn(vuart_op->gfn);
> +    struct vuart_params params = {
> +        .console_domid = vuart_op->console_domid,
> +        .gfn = _gfn(vuart_op->gfn),
> +        .evtchn = 0,
> +    };
>  
>      if ( d->creation_finished )
>          return -EPERM;
> @@ -41,10 +43,10 @@ static int handle_vuart_init(struct domain *d,
>      if ( vuart_op->type != XEN_DOMCTL_VUART_TYPE_VPL011 )
>          return -EOPNOTSUPP;
>  
> -    rc = domain_vpl011_init(d, &info);
> +    rc = vuart_init(d, &params);
>  
>      if ( !rc )
> -        vuart_op->evtchn = info.evtchn;
> +        vuart_op->evtchn = params.evtchn;
>  
>      return rc;
>  }
> diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
> index be64883b8628..5c308cc8c148 100644
> --- a/xen/arch/arm/include/asm/vpl011.h
> +++ b/xen/arch/arm/include/asm/vpl011.h
> @@ -59,26 +59,6 @@ struct vpl011 {
>      evtchn_port_t evtchn;
>  };
>  
> -struct vpl011_init_info {
> -    domid_t console_domid;
> -    gfn_t gfn;
> -    evtchn_port_t evtchn;
> -};
> -
> -#ifdef CONFIG_HAS_VUART_PL011
> -int domain_vpl011_init(struct domain *d,
> -                       struct vpl011_init_info *info);
> -void domain_vpl011_deinit(struct domain *d);
> -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)
> -{
> -    return -ENOSYS;
> -}
> -
> -static inline void domain_vpl011_deinit(struct domain *d) { }
> -#endif
>  #endif  /* _VPL011_H_ */
>  
>  /*
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index cafc532cf028..2cf88a70ecdb 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -134,7 +134,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
>  
>  /*
>   * vpl011_read_data_xen reads data when the backend is xen. Characters
> - * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
> + * are added to the vpl011 receive buffer by vuart_putchar.
>   */
>  static uint8_t vpl011_read_data_xen(struct domain *d)
>  {
> @@ -571,9 +571,9 @@ static void vpl011_data_avail(struct domain *d,
>  }
>  
>  /*
> - * vpl011_rx_char_xen adds a char to a domain's vpl011 receive buffer.
> + * vuart_putchar adds a char to a domain's vpl011 receive buffer.
>   */
> -int vpl011_rx_char_xen(struct domain *d, char c)
> +int vuart_putchar(struct domain *d, char c)
How can putchar refer to RX? By definition putchar() is used to print data to
STDOUT. Here we receive a character and put it in the RX FIFO.

~Michal
Re: [PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls
Posted by dmkhn@proton.me 3 months ago
On Tue, Jun 24, 2025 at 12:11:10PM +0200, Orzel, Michal wrote:
> 
> 
> On 24/06/2025 05:55, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > Use generic names prefixed with 'vuart_' in public PL011 emulator data
> > structures and functions.
> >
> > No functional change intended.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> >  xen/arch/arm/dom0less-build.c     |  4 ++--
> >  xen/arch/arm/domain.c             |  3 ++-
> >  xen/arch/arm/domctl.c             | 14 +++++++------
> >  xen/arch/arm/include/asm/vpl011.h | 20 ------------------
> >  xen/arch/arm/vpl011.c             | 24 +++++++++++-----------
> >  xen/drivers/char/console.c        |  6 ++----
> >  xen/include/xen/vuart.h           | 34 ++++++++++++++++++++++++++++++-
> >  7 files changed, 59 insertions(+), 46 deletions(-)
> >
> > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> > index 7c1b59750fb5..11b8498d3b22 100644
> > --- a/xen/arch/arm/dom0less-build.c
> > +++ b/xen/arch/arm/dom0less-build.c
> > @@ -216,7 +216,7 @@ int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
> As can be seen here ...
> 
> >       */
> >      if ( kinfo->arch.vpl011 )
> >      {
> > -        rc = domain_vpl011_init(d, NULL);
> > +        rc = vuart_init(d, NULL);
> we end up with init_vuart() and vuart_init(). That's quite confusing. Maybe
> domain_vuart_init() or alike?

Agreed, will update.

> 
> >          if ( rc < 0 )
> >              return rc;
> >      }
> > @@ -247,7 +247,7 @@ void __init arch_create_domUs(struct dt_device_node *node,
> >           * d->arch.vpl011.irq. So the logic to find the vIRQ has to
> >           * be hardcoded.
> >           * The logic here shall be consistent with the one in
> > -         * domain_vpl011_init().
> > +         * vuart_init().
> >           */
> >          if ( flags & CDF_directmap )
> >          {
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index be58a23dd725..68297e619bad 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -11,6 +11,7 @@
> >  #include <xen/sched.h>
> >  #include <xen/softirq.h>
> >  #include <xen/wait.h>
> > +#include <xen/vuart.h>
> >
> >  #include <asm/arm64/sve.h>
> >  #include <asm/cpuerrata.h>
> > @@ -1072,7 +1073,7 @@ int domain_relinquish_resources(struct domain *d)
> >           * Release the resources allocated for vpl011 which were
> >           * allocated via a DOMCTL call XEN_DOMCTL_vuart_op.
> >           */
> > -        domain_vpl011_deinit(d);
> > +        vuart_exit(d);
> IMO, deinit is more meaningful here.

Ack

> 
> >
> >  #ifdef CONFIG_IOREQ_SERVER
> >          ioreq_server_destroy_all(d);
> > diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> > index ad914c915f81..dde25ceff6d0 100644
> > --- a/xen/arch/arm/domctl.c
> > +++ b/xen/arch/arm/domctl.c
> > @@ -14,6 +14,7 @@
> >  #include <xen/mm.h>
> >  #include <xen/sched.h>
> >  #include <xen/types.h>
> > +#include <xen/vuart.h>
> >  #include <xsm/xsm.h>
> >  #include <public/domctl.h>
> >
> > @@ -30,10 +31,11 @@ static int handle_vuart_init(struct domain *d,
> >                               struct xen_domctl_vuart_op *vuart_op)
> >  {
> >      int rc;
> > -    struct vpl011_init_info info;
> > -
> > -    info.console_domid = vuart_op->console_domid;
> > -    info.gfn = _gfn(vuart_op->gfn);
> > +    struct vuart_params params = {
> > +        .console_domid = vuart_op->console_domid,
> > +        .gfn = _gfn(vuart_op->gfn),
> > +        .evtchn = 0,
> > +    };
> >
> >      if ( d->creation_finished )
> >          return -EPERM;
> > @@ -41,10 +43,10 @@ static int handle_vuart_init(struct domain *d,
> >      if ( vuart_op->type != XEN_DOMCTL_VUART_TYPE_VPL011 )
> >          return -EOPNOTSUPP;
> >
> > -    rc = domain_vpl011_init(d, &info);
> > +    rc = vuart_init(d, &params);
> >
> >      if ( !rc )
> > -        vuart_op->evtchn = info.evtchn;
> > +        vuart_op->evtchn = params.evtchn;
> >
> >      return rc;
> >  }
> > diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
> > index be64883b8628..5c308cc8c148 100644
> > --- a/xen/arch/arm/include/asm/vpl011.h
> > +++ b/xen/arch/arm/include/asm/vpl011.h
> > @@ -59,26 +59,6 @@ struct vpl011 {
> >      evtchn_port_t evtchn;
> >  };
> >
> > -struct vpl011_init_info {
> > -    domid_t console_domid;
> > -    gfn_t gfn;
> > -    evtchn_port_t evtchn;
> > -};
> > -
> > -#ifdef CONFIG_HAS_VUART_PL011
> > -int domain_vpl011_init(struct domain *d,
> > -                       struct vpl011_init_info *info);
> > -void domain_vpl011_deinit(struct domain *d);
> > -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)
> > -{
> > -    return -ENOSYS;
> > -}
> > -
> > -static inline void domain_vpl011_deinit(struct domain *d) { }
> > -#endif
> >  #endif  /* _VPL011_H_ */
> >
> >  /*
> > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > index cafc532cf028..2cf88a70ecdb 100644
> > --- a/xen/arch/arm/vpl011.c
> > +++ b/xen/arch/arm/vpl011.c
> > @@ -134,7 +134,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
> >
> >  /*
> >   * vpl011_read_data_xen reads data when the backend is xen. Characters
> > - * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
> > + * are added to the vpl011 receive buffer by vuart_putchar.
> >   */
> >  static uint8_t vpl011_read_data_xen(struct domain *d)
> >  {
> > @@ -571,9 +571,9 @@ static void vpl011_data_avail(struct domain *d,
> >  }
> >
> >  /*
> > - * vpl011_rx_char_xen adds a char to a domain's vpl011 receive buffer.
> > + * vuart_putchar adds a char to a domain's vpl011 receive buffer.
> >   */
> > -int vpl011_rx_char_xen(struct domain *d, char c)
> > +int vuart_putchar(struct domain *d, char c)
> How can putchar refer to RX? By definition putchar() is used to print data to
> STDOUT. Here we receive a character and put it in the RX FIFO.

Yeah, that may be confusing.
I will use vuart_put_rx().

> 
> ~Michal
> 
> 
Re: [PATCH v1 03/16] arm/vpl011: use vuart_ prefix in vpl011 public calls
Posted by dmkhn@proton.me 4 months, 1 week ago
On Tue, Jun 24, 2025 at 12:11:10PM +0200, Orzel, Michal wrote:
> 
> 
> On 24/06/2025 05:55, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > Use generic names prefixed with 'vuart_' in public PL011 emulator data
> > structures and functions.
> >
> > No functional change intended.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> >  xen/arch/arm/dom0less-build.c     |  4 ++--
> >  xen/arch/arm/domain.c             |  3 ++-
> >  xen/arch/arm/domctl.c             | 14 +++++++------
> >  xen/arch/arm/include/asm/vpl011.h | 20 ------------------
> >  xen/arch/arm/vpl011.c             | 24 +++++++++++-----------
> >  xen/drivers/char/console.c        |  6 ++----
> >  xen/include/xen/vuart.h           | 34 ++++++++++++++++++++++++++++++-
> >  7 files changed, 59 insertions(+), 46 deletions(-)
> >
> > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> > index 7c1b59750fb5..11b8498d3b22 100644
> > --- a/xen/arch/arm/dom0less-build.c
> > +++ b/xen/arch/arm/dom0less-build.c
> > @@ -216,7 +216,7 @@ int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
> As can be seen here ...
> 
> >       */
> >      if ( kinfo->arch.vpl011 )
> >      {
> > -        rc = domain_vpl011_init(d, NULL);
> > +        rc = vuart_init(d, NULL);
> we end up with init_vuart() and vuart_init(). That's quite confusing. Maybe
> domain_vuart_init() or alike?

That's right!
But domain_vuart_init() is used by MMIO-based variant :)
I will write an extra patch and put it to the end of the series to update the
name here.

> 
> >          if ( rc < 0 )
> >              return rc;
> >      }
> > @@ -247,7 +247,7 @@ void __init arch_create_domUs(struct dt_device_node *node,
> >           * d->arch.vpl011.irq. So the logic to find the vIRQ has to
> >           * be hardcoded.
> >           * The logic here shall be consistent with the one in
> > -         * domain_vpl011_init().
> > +         * vuart_init().
> >           */
> >          if ( flags & CDF_directmap )
> >          {
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index be58a23dd725..68297e619bad 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -11,6 +11,7 @@
> >  #include <xen/sched.h>
> >  #include <xen/softirq.h>
> >  #include <xen/wait.h>
> > +#include <xen/vuart.h>
> >
> >  #include <asm/arm64/sve.h>
> >  #include <asm/cpuerrata.h>
> > @@ -1072,7 +1073,7 @@ int domain_relinquish_resources(struct domain *d)
> >           * Release the resources allocated for vpl011 which were
> >           * allocated via a DOMCTL call XEN_DOMCTL_vuart_op.
> >           */
> > -        domain_vpl011_deinit(d);
> > +        vuart_exit(d);
> IMO, deinit is more meaningful here.

Yeah, it's just MMIO UART uses init/free, here it is init/deinit.

So I just picked init/exit similar to module_{init,exit} in Linux driver model
and applied to all existing vUARTs.

Can update all vUARTs to deinit()

> 
> >
> >  #ifdef CONFIG_IOREQ_SERVER
> >          ioreq_server_destroy_all(d);
> > diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> > index ad914c915f81..dde25ceff6d0 100644
> > --- a/xen/arch/arm/domctl.c
> > +++ b/xen/arch/arm/domctl.c
> > @@ -14,6 +14,7 @@
> >  #include <xen/mm.h>
> >  #include <xen/sched.h>
> >  #include <xen/types.h>
> > +#include <xen/vuart.h>
> >  #include <xsm/xsm.h>
> >  #include <public/domctl.h>
> >
> > @@ -30,10 +31,11 @@ static int handle_vuart_init(struct domain *d,
> >                               struct xen_domctl_vuart_op *vuart_op)
> >  {
> >      int rc;
> > -    struct vpl011_init_info info;
> > -
> > -    info.console_domid = vuart_op->console_domid;
> > -    info.gfn = _gfn(vuart_op->gfn);
> > +    struct vuart_params params = {
> > +        .console_domid = vuart_op->console_domid,
> > +        .gfn = _gfn(vuart_op->gfn),
> > +        .evtchn = 0,
> > +    };
> >
> >      if ( d->creation_finished )
> >          return -EPERM;
> > @@ -41,10 +43,10 @@ static int handle_vuart_init(struct domain *d,
> >      if ( vuart_op->type != XEN_DOMCTL_VUART_TYPE_VPL011 )
> >          return -EOPNOTSUPP;
> >
> > -    rc = domain_vpl011_init(d, &info);
> > +    rc = vuart_init(d, &params);
> >
> >      if ( !rc )
> > -        vuart_op->evtchn = info.evtchn;
> > +        vuart_op->evtchn = params.evtchn;
> >
> >      return rc;
> >  }
> > diff --git a/xen/arch/arm/include/asm/vpl011.h b/xen/arch/arm/include/asm/vpl011.h
> > index be64883b8628..5c308cc8c148 100644
> > --- a/xen/arch/arm/include/asm/vpl011.h
> > +++ b/xen/arch/arm/include/asm/vpl011.h
> > @@ -59,26 +59,6 @@ struct vpl011 {
> >      evtchn_port_t evtchn;
> >  };
> >
> > -struct vpl011_init_info {
> > -    domid_t console_domid;
> > -    gfn_t gfn;
> > -    evtchn_port_t evtchn;
> > -};
> > -
> > -#ifdef CONFIG_HAS_VUART_PL011
> > -int domain_vpl011_init(struct domain *d,
> > -                       struct vpl011_init_info *info);
> > -void domain_vpl011_deinit(struct domain *d);
> > -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)
> > -{
> > -    return -ENOSYS;
> > -}
> > -
> > -static inline void domain_vpl011_deinit(struct domain *d) { }
> > -#endif
> >  #endif  /* _VPL011_H_ */
> >
> >  /*
> > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > index cafc532cf028..2cf88a70ecdb 100644
> > --- a/xen/arch/arm/vpl011.c
> > +++ b/xen/arch/arm/vpl011.c
> > @@ -134,7 +134,7 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
> >
> >  /*
> >   * vpl011_read_data_xen reads data when the backend is xen. Characters
> > - * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
> > + * are added to the vpl011 receive buffer by vuart_putchar.
> >   */
> >  static uint8_t vpl011_read_data_xen(struct domain *d)
> >  {
> > @@ -571,9 +571,9 @@ static void vpl011_data_avail(struct domain *d,
> >  }
> >
> >  /*
> > - * vpl011_rx_char_xen adds a char to a domain's vpl011 receive buffer.
> > + * vuart_putchar adds a char to a domain's vpl011 receive buffer.
> >   */
> > -int vpl011_rx_char_xen(struct domain *d, char c)
> > +int vuart_putchar(struct domain *d, char c)
> How can putchar refer to RX? By definition putchar() is used to print data to
> STDOUT. Here we receive a character and put it in the RX FIFO.

That's confusing, I agree; I think this is a leftover from the earlier vUART
series.

Will update, thanks!

> 
> ~Michal
> 
>