1. Patchew
  2. Xen
  3. View series

[PATCH] xen/consoled: clean up console handling for pv_shim

dmkhn@proton.me posted 1 patch 8 months, 1 week ago
Failed in applying to current master (apply log)
xen/arch/x86/pv/shim.c      |  3 +--
xen/drivers/char/console.c  | 15 ++++++---------
xen/drivers/char/consoled.c | 17 +++++++++++++----
xen/include/xen/consoled.h  | 32 +++++++++++++++++++++++++++-----
4 files changed, 47 insertions(+), 20 deletions(-)
[PATCH] xen/consoled: clean up console handling for pv_shim
Posted by dmkhn@proton.me 8 months, 1 week ago 
There are few places which check pv_shim console under CONFIG_PV_SHIM or
CONFIG_X86 in xen console driver.

Instead of #ifdef-ing, use new consoled_is_enabled() in switch_serial_input()
and __serial_rx() (where pv_shim condition is now detected correctly).

Signature of consoled_guest_{rx,tx} has changed so the errors can be logged
on the callsites.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/pv/shim.c      |  3 +--
 xen/drivers/char/console.c  | 15 ++++++---------
 xen/drivers/char/consoled.c | 17 +++++++++++++----
 xen/include/xen/consoled.h  | 32 +++++++++++++++++++++++++++-----
 4 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index b9c034ccff..cbc2e3fced 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -605,8 +605,7 @@ long pv_shim_event_channel_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 
         if ( pv_console && send.port == pv_console_evtchn() )
         {
-            consoled_guest_rx();
-            rc = 0;
+            rc = consoled_guest_rx();
         }
         else
             rc = xen_hypercall_event_channel_op(EVTCHNOP_send, &send);
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 992b37962e..c207fd8704 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -32,9 +32,9 @@
 #include <xen/pv_console.h>
 #include <asm/setup.h>
 #include <xen/sections.h>
+#include <xen/consoled.h>
 
 #ifdef CONFIG_X86
-#include <xen/consoled.h>
 #include <asm/guest.h>
 #endif
 #ifdef CONFIG_SBSA_VUART_CONSOLE
@@ -507,11 +507,9 @@ static void switch_serial_input(void)
             break;
         }
 
-#ifdef CONFIG_PV_SHIM
-        if ( next_rx == 1 )
+        if ( consoled_is_enabled() && next_rx == 1 )
             domid = get_initial_domain_id();
         else
-#endif
             domid = next_rx - 1;
         d = rcu_lock_domain_by_id(domid);
         if ( d )
@@ -562,13 +560,12 @@ static void __serial_rx(char c)
         rc = vpl011_rx_char_xen(d, c);
 #endif
 
-#ifdef CONFIG_X86
-    if ( pv_shim && pv_console )
-        consoled_guest_tx(c);
-#endif
+    if ( consoled_is_enabled() )
+        /* Deliver input to the PV shim console. */
+        rc = consoled_guest_tx(c);
 
     if ( rc )
-        guest_printk(d, XENLOG_G_WARNING
+        guest_printk(d, XENLOG_WARNING
                         "failed to process console input: %d\n", rc);
 
     console_put_domain(d);
diff --git a/xen/drivers/char/consoled.c b/xen/drivers/char/consoled.c
index b415b632ce..8704ec251e 100644
--- a/xen/drivers/char/consoled.c
+++ b/xen/drivers/char/consoled.c
@@ -43,13 +43,13 @@ struct xencons_interface *consoled_get_ring_addr(void)
 static char buf[BUF_SZ + 1];
 
 /* Receives characters from a domain's PV console */
-void consoled_guest_rx(void)
+int consoled_guest_rx(void)
 {
     size_t idx = 0;
     XENCONS_RING_IDX cons, prod;
 
     if ( !cons_ring )
-        return;
+        return -ENODEV;
 
     spin_lock(&rx_lock);
 
@@ -91,15 +91,17 @@ void consoled_guest_rx(void)
 
  out:
     spin_unlock(&rx_lock);
+
+    return 0;
 }
 
 /* Sends a character into a domain's PV console */
-void consoled_guest_tx(char c)
+int consoled_guest_tx(char c)
 {
     XENCONS_RING_IDX cons, prod;
 
     if ( !cons_ring )
-        return;
+        return -ENODEV;
 
     cons = ACCESS_ONCE(cons_ring->in_cons);
     prod = cons_ring->in_prod;
@@ -125,6 +127,13 @@ void consoled_guest_tx(char c)
  notify:
     /* Always notify the guest: prevents receive path from getting stuck. */
     pv_shim_inject_evtchn(pv_console_evtchn());
+
+    return 0;
+}
+
+bool consoled_is_enabled(void)
+{
+    return pv_shim && pv_console;
 }
 
 /*
diff --git a/xen/include/xen/consoled.h b/xen/include/xen/consoled.h
index bd7ab6329e..52a1358bea 100644
--- a/xen/include/xen/consoled.h
+++ b/xen/include/xen/consoled.h
@@ -1,14 +1,36 @@
-#ifndef __XEN_CONSOLED_H__
-#define __XEN_CONSOLED_H__
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef XEN__CONSOLED_H
+#define XEN__CONSOLED_H
 
 #include <public/io/console.h>
 
+#ifdef CONFIG_PV_SHIM
+
 void consoled_set_ring_addr(struct xencons_interface *ring);
 struct xencons_interface *consoled_get_ring_addr(void);
-void consoled_guest_rx(void);
-void consoled_guest_tx(char c);
+int consoled_guest_rx(void);
+int consoled_guest_tx(char c);
+bool consoled_is_enabled(void);
 
-#endif /* __XEN_CONSOLED_H__ */
+#else
+
+static inline int consoled_guest_rx(void)
+{
+    ASSERT_UNREACHABLE();
+    return -ENODEV;
+}
+
+static inline int consoled_guest_tx(char c)
+{
+    ASSERT_UNREACHABLE();
+    return -ENODEV;
+}
+
+#define consoled_is_enabled()   (false)
+
+#endif /* CONFIG_PV_SHIM */
+
+#endif /* XEN__CONSOLED_H */
 /*
  * Local variables:
  * mode: C
-- 
2.34.1
Re: [PATCH] xen/consoled: clean up console handling for pv_shim
Posted by Denis Mukhin 8 months, 1 week ago 
On Friday, February 21st, 2025 at 4:26 PM, dmkhn@proton.me <dmkhn@proton.me> wrote:

> 
> 
> There are few places which check pv_shim console under CONFIG_PV_SHIM or
> CONFIG_X86 in xen console driver.
> 
> Instead of #ifdef-ing, use new consoled_is_enabled() in switch_serial_input()
> and _serial_rx() (where pv_shim condition is now detected correctly).
> 
> Signature of consoled_guest{rx,tx} has changed so the errors can be logged
> on the callsites.
> 
> Signed-off-by: Denis Mukhin dmukhin@ford.com

Sorry, I posted too early, there's missing dependency in this patch.
Posted v2 here:
  https://lore.kernel.org/xen-devel/20250222235748.103599-1-dmkhn@proton.me/

> 
> ---
> xen/arch/x86/pv/shim.c | 3 +--
> xen/drivers/char/console.c | 15 ++++++---------
> xen/drivers/char/consoled.c | 17 +++++++++++++----
> xen/include/xen/consoled.h | 32 +++++++++++++++++++++++++++-----
> 4 files changed, 47 insertions(+), 20 deletions(-)
> 
> diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
> index b9c034ccff..cbc2e3fced 100644
> --- a/xen/arch/x86/pv/shim.c
> +++ b/xen/arch/x86/pv/shim.c
> @@ -605,8 +605,7 @@ long pv_shim_event_channel_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> 
> if ( pv_console && send.port == pv_console_evtchn() )
> {
> - consoled_guest_rx();
> - rc = 0;
> + rc = consoled_guest_rx();
> }
> else
> rc = xen_hypercall_event_channel_op(EVTCHNOP_send, &send);
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 992b37962e..c207fd8704 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -32,9 +32,9 @@
> #include <xen/pv_console.h>
> 
> #include <asm/setup.h>
> 
> #include <xen/sections.h>
> 
> +#include <xen/consoled.h>
> 
> 
> #ifdef CONFIG_X86
> -#include <xen/consoled.h>
> 
> #include <asm/guest.h>
> 
> #endif
> #ifdef CONFIG_SBSA_VUART_CONSOLE
> @@ -507,11 +507,9 @@ static void switch_serial_input(void)
> break;
> }
> 
> -#ifdef CONFIG_PV_SHIM
> - if ( next_rx == 1 )
> + if ( consoled_is_enabled() && next_rx == 1 )
> domid = get_initial_domain_id();
> else
> -#endif
> domid = next_rx - 1;
> d = rcu_lock_domain_by_id(domid);
> if ( d )
> @@ -562,13 +560,12 @@ static void __serial_rx(char c)
> rc = vpl011_rx_char_xen(d, c);
> #endif
> 
> -#ifdef CONFIG_X86
> - if ( pv_shim && pv_console )
> - consoled_guest_tx(c);
> -#endif
> + if ( consoled_is_enabled() )
> + /* Deliver input to the PV shim console. */
> + rc = consoled_guest_tx(c);
> 
> if ( rc )
> - guest_printk(d, XENLOG_G_WARNING
> + guest_printk(d, XENLOG_WARNING
> "failed to process console input: %d\n", rc);
> 
> console_put_domain(d);
> diff --git a/xen/drivers/char/consoled.c b/xen/drivers/char/consoled.c
> index b415b632ce..8704ec251e 100644
> --- a/xen/drivers/char/consoled.c
> +++ b/xen/drivers/char/consoled.c
> @@ -43,13 +43,13 @@ struct xencons_interface consoled_get_ring_addr(void)
> static char buf[BUF_SZ + 1];
> 
> / Receives characters from a domain's PV console /
> -void consoled_guest_rx(void)
> +int consoled_guest_rx(void)
> {
> size_t idx = 0;
> XENCONS_RING_IDX cons, prod;
> 
> if ( !cons_ring )
> - return;
> + return -ENODEV;
> 
> spin_lock(&rx_lock);
> 
> @@ -91,15 +91,17 @@ void consoled_guest_rx(void)
> 
> out:
> spin_unlock(&rx_lock);
> +
> + return 0;
> }
> 
> / Sends a character into a domain's PV console */
> -void consoled_guest_tx(char c)
> +int consoled_guest_tx(char c)
> {
> XENCONS_RING_IDX cons, prod;
> 
> if ( !cons_ring )
> - return;
> + return -ENODEV;
> 
> cons = ACCESS_ONCE(cons_ring->in_cons);
> 
> prod = cons_ring->in_prod;
> 
> @@ -125,6 +127,13 @@ void consoled_guest_tx(char c)
> notify:
> /* Always notify the guest: prevents receive path from getting stuck. /
> pv_shim_inject_evtchn(pv_console_evtchn());
> +
> + return 0;
> +}
> +
> +bool consoled_is_enabled(void)
> +{
> + return pv_shim && pv_console;
> }
> 
> /
> diff --git a/xen/include/xen/consoled.h b/xen/include/xen/consoled.h
> index bd7ab6329e..52a1358bea 100644
> --- a/xen/include/xen/consoled.h
> +++ b/xen/include/xen/consoled.h
> @@ -1,14 +1,36 @@
> -#ifndef XEN_CONSOLED_H
> -#define XEN_CONSOLED_H
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef XEN__CONSOLED_H
> +#define XEN__CONSOLED_H
> 
> #include <public/io/console.h>
> 
> 
> +#ifdef CONFIG_PV_SHIM
> +
> void consoled_set_ring_addr(struct xencons_interface *ring);
> struct xencons_interface consoled_get_ring_addr(void);
> -void consoled_guest_rx(void);
> -void consoled_guest_tx(char c);
> +int consoled_guest_rx(void);
> +int consoled_guest_tx(char c);
> +bool consoled_is_enabled(void);
> 
> -#endif / XEN_CONSOLED_H /
> +#else
> +
> +static inline int consoled_guest_rx(void)
> +{
> + ASSERT_UNREACHABLE();
> + return -ENODEV;
> +}
> +
> +static inline int consoled_guest_tx(char c)
> +{
> + ASSERT_UNREACHABLE();
> + return -ENODEV;
> +}
> +
> +#define consoled_is_enabled() (false)
> +
> +#endif / CONFIG_PV_SHIM /
> +
> +#endif / XEN__CONSOLED_H /
> /
> * Local variables:
> * mode: C
> --
> 2.34.1

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.