Re: [PATCH v4 2/3] xen/console: introduce console_send()

2025-05-16 Thread Stefano Stabellini
On Fri, 16 May 2025, dm...@proton.me wrote:
> From: Denis Mukhin 
> 
> guest_console_write() duplicates the code from __putstr(), eliminate code
> duplication.
> 
> Introduce console_send() for sending a message on console devices.
> 
> Also, introduce internal console flags to control which console devices
> should be used.
> 
> No functional change intended.
> 
> Signed-off-by: Denis Mukhin 

Reviewed-by: Stefano Stabellini 


> ---
> Changes since v3:
> - renamed console_puts() to console_send()
> ---
>  xen/drivers/char/console.c | 131 +++--
>  1 file changed, 82 insertions(+), 49 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index b4757844e6..3420e9630a 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -41,6 +41,28 @@
>  #include 
>  #endif
>  
> +/* Internal console flags. */
> +enum {
> +CONSOLE_SERIAL  = BIT(0, U),/* Use serial device. */
> +CONSOLE_PV  = BIT(1, U),/* Use PV console. */
> +CONSOLE_VIDEO   = BIT(2, U),/* Use video device. */
> +CONSOLE_DEBUG   = BIT(3, U),/* Use debug device. */
> +CONSOLE_RING= BIT(4, U),/* Use console ring. */
> +CONSOLE_RING_VIRQ   = BIT(5, U),/* Use console ring VIRQ. */
> +
> +/* Default console flags. */
> +CONSOLE_DEFAULT = CONSOLE_SERIAL |
> +  CONSOLE_PV |
> +  CONSOLE_VIDEO |
> +  CONSOLE_RING_VIRQ |
> +  CONSOLE_DEBUG,
> +
> +/* Use all known console devices. */
> +CONSOLE_ALL = CONSOLE_DEFAULT | CONSOLE_RING,
> +};
> +
> +static void console_send(const char *str, size_t len, unsigned int flags);
> +
>  /* console: comma-separated list of console outputs. */
>  static char __initdata opt_console[30] = OPT_CONSOLE_STR;
>  string_param("console", opt_console);
> @@ -428,9 +450,6 @@ void console_serial_puts(const char *s, size_t nr)
>  serial_steal_fn(s, nr);
>  else
>  serial_puts(sercon_handle, s, nr);
> -
> -/* Copy all serial output into PV console */
> -pv_console_puts(s, nr);
>  }
>  
>  static void cf_check dump_console_ring_key(unsigned char key)
> @@ -464,8 +483,7 @@ static void cf_check dump_console_ring_key(unsigned char 
> key)
>  c += len;
>  }
>  
> -console_serial_puts(buf, sofar);
> -video_puts(buf, sofar);
> +console_send(buf, sofar, CONSOLE_SERIAL | CONSOLE_VIDEO | CONSOLE_PV);
>  
>  free_xenheap_pages(buf, order);
>  }
> @@ -614,11 +632,69 @@ static inline void xen_console_write_debug_port(const 
> char *buf, size_t len)
>  }
>  #endif
>  
> +static inline void console_debug_puts(const char *str, size_t len)
> +{
> +#ifdef CONFIG_X86
> +if ( opt_console_xen )
> +{
> +if ( xen_guest )
> +xen_hypercall_console_write(str, len);
> +else
> +xen_console_write_debug_port(str, len);
> +}
> +#endif
> +}
> +
> +/*
> + * Send a message on console device(s).
> + *
> + * That will handle all possible scenarios working w/ console
> + * - physical console (serial console, VGA console (x86 only));
> + * - PV console;
> + * - debug console (x86 only): debug I/O port or __HYPERVISOR_console_io
> + *   hypercall;
> + * - console ring.
> + */
> +static void console_send(const char *str, size_t len, unsigned int flags)
> +{
> +if ( flags & CONSOLE_SERIAL )
> +console_serial_puts(str, len);
> +
> +if ( flags & CONSOLE_PV )
> +pv_console_puts(str, len);
> +
> +if ( flags & CONSOLE_VIDEO )
> +video_puts(str, len);
> +
> +if ( flags & CONSOLE_DEBUG )
> +console_debug_puts(str, len);
> +
> +if ( flags & CONSOLE_RING )
> +conring_puts(str, len);
> +
> +if ( flags & CONSOLE_RING_VIRQ )
> +tasklet_schedule(&conring_tasklet);
> +}
> +
> +static inline void __putstr(const char *str)
> +{
> +unsigned int flags = CONSOLE_ALL;
> +
> +ASSERT(rspin_is_locked(&console_lock));
> +
> +if ( conring_no_notify )
> +flags &= ~CONSOLE_RING_VIRQ;
> +
> +console_send(str, strlen(str), flags);
> +}
> +
>  static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
>  unsigned int count)
>  {
>  char kbuf[128];
>  unsigned int kcount = 0;
> +unsigned int flags = opt_console_to_ring
> + ? CONSOLE_ALL : CONSOLE_DEFAULT;
>  struct domain *cd = current->domain;
>  
>  while ( count > 0 )
> @@ -636,26 +712,7 @@ static long 
> guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
>  {
>  /* Use direct console output as it could be interactive */
>  nrspin_lock_irq(&console_lock);
> -
> -console_serial_puts(kbuf, kcount);
> -video_puts(kbuf, kcount);
> -
> -#ifdef CONFIG_X86
> -if ( opt_cons

[PATCH v4 2/3] xen/console: introduce console_send()

2025-05-15 Thread dmkhn
From: Denis Mukhin 

guest_console_write() duplicates the code from __putstr(), eliminate code
duplication.

Introduce console_send() for sending a message on console devices.

Also, introduce internal console flags to control which console devices
should be used.

No functional change intended.

Signed-off-by: Denis Mukhin 
---
Changes since v3:
- renamed console_puts() to console_send()
---
 xen/drivers/char/console.c | 131 +++--
 1 file changed, 82 insertions(+), 49 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index b4757844e6..3420e9630a 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -41,6 +41,28 @@
 #include 
 #endif
 
+/* Internal console flags. */
+enum {
+CONSOLE_SERIAL  = BIT(0, U),/* Use serial device. */
+CONSOLE_PV  = BIT(1, U),/* Use PV console. */
+CONSOLE_VIDEO   = BIT(2, U),/* Use video device. */
+CONSOLE_DEBUG   = BIT(3, U),/* Use debug device. */
+CONSOLE_RING= BIT(4, U),/* Use console ring. */
+CONSOLE_RING_VIRQ   = BIT(5, U),/* Use console ring VIRQ. */
+
+/* Default console flags. */
+CONSOLE_DEFAULT = CONSOLE_SERIAL |
+  CONSOLE_PV |
+  CONSOLE_VIDEO |
+  CONSOLE_RING_VIRQ |
+  CONSOLE_DEBUG,
+
+/* Use all known console devices. */
+CONSOLE_ALL = CONSOLE_DEFAULT | CONSOLE_RING,
+};
+
+static void console_send(const char *str, size_t len, unsigned int flags);
+
 /* console: comma-separated list of console outputs. */
 static char __initdata opt_console[30] = OPT_CONSOLE_STR;
 string_param("console", opt_console);
@@ -428,9 +450,6 @@ void console_serial_puts(const char *s, size_t nr)
 serial_steal_fn(s, nr);
 else
 serial_puts(sercon_handle, s, nr);
-
-/* Copy all serial output into PV console */
-pv_console_puts(s, nr);
 }
 
 static void cf_check dump_console_ring_key(unsigned char key)
@@ -464,8 +483,7 @@ static void cf_check dump_console_ring_key(unsigned char 
key)
 c += len;
 }
 
-console_serial_puts(buf, sofar);
-video_puts(buf, sofar);
+console_send(buf, sofar, CONSOLE_SERIAL | CONSOLE_VIDEO | CONSOLE_PV);
 
 free_xenheap_pages(buf, order);
 }
@@ -614,11 +632,69 @@ static inline void xen_console_write_debug_port(const 
char *buf, size_t len)
 }
 #endif
 
+static inline void console_debug_puts(const char *str, size_t len)
+{
+#ifdef CONFIG_X86
+if ( opt_console_xen )
+{
+if ( xen_guest )
+xen_hypercall_console_write(str, len);
+else
+xen_console_write_debug_port(str, len);
+}
+#endif
+}
+
+/*
+ * Send a message on console device(s).
+ *
+ * That will handle all possible scenarios working w/ console
+ * - physical console (serial console, VGA console (x86 only));
+ * - PV console;
+ * - debug console (x86 only): debug I/O port or __HYPERVISOR_console_io
+ *   hypercall;
+ * - console ring.
+ */
+static void console_send(const char *str, size_t len, unsigned int flags)
+{
+if ( flags & CONSOLE_SERIAL )
+console_serial_puts(str, len);
+
+if ( flags & CONSOLE_PV )
+pv_console_puts(str, len);
+
+if ( flags & CONSOLE_VIDEO )
+video_puts(str, len);
+
+if ( flags & CONSOLE_DEBUG )
+console_debug_puts(str, len);
+
+if ( flags & CONSOLE_RING )
+conring_puts(str, len);
+
+if ( flags & CONSOLE_RING_VIRQ )
+tasklet_schedule(&conring_tasklet);
+}
+
+static inline void __putstr(const char *str)
+{
+unsigned int flags = CONSOLE_ALL;
+
+ASSERT(rspin_is_locked(&console_lock));
+
+if ( conring_no_notify )
+flags &= ~CONSOLE_RING_VIRQ;
+
+console_send(str, strlen(str), flags);
+}
+
 static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
 unsigned int count)
 {
 char kbuf[128];
 unsigned int kcount = 0;
+unsigned int flags = opt_console_to_ring
+ ? CONSOLE_ALL : CONSOLE_DEFAULT;
 struct domain *cd = current->domain;
 
 while ( count > 0 )
@@ -636,26 +712,7 @@ static long 
guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
 {
 /* Use direct console output as it could be interactive */
 nrspin_lock_irq(&console_lock);
-
-console_serial_puts(kbuf, kcount);
-video_puts(kbuf, kcount);
-
-#ifdef CONFIG_X86
-if ( opt_console_xen )
-{
-if ( xen_guest )
-xen_hypercall_console_write(kbuf, kcount);
-else
-xen_console_write_debug_port(kbuf, kcount);
-}
-#endif
-
-if ( opt_console_to_ring )
-{
-conring_puts(kbuf, kcount);
-tasklet_schedule(&conring_tasklet);
-