Re: [PATCH v3 05/24] xen/console: introduce consoled_is_enabled()

[Denis Mukhin <dmkhn@xxxxxxxxxxxxxx>]

On Friday, January 3rd, 2025 at 5:58 PM, Denis Mukhin via B4 Relay 
<devnull+dmukhin.ford.com@xxxxxxxxxx> wrote:
> 
> 
> From: Denis Mukhin dmukhin@xxxxxxxx
> 
> 
> There are few places which check pv_shim console under CONFIG_PV_SHIM in xen
> console driver. Instead of #ifdef-ing, use new consoled_is_enabled() to
> customize the logic.
> 
> Header file now can be included w/o CONFIG_X86.
> 
> Signature of consoled_guest_{rx,tx} has changed so the error can be logged.
> 
> Signed-off-by: Denis Mukhin dmukhin@xxxxxxxx

Moved to:
  https://lore.kernel.org/xen-devel/20250222235748.103599-1-dmkhn@xxxxxxxxx/

> 
> ---
> xen/drivers/char/console.c | 13 +++++--------
> xen/drivers/char/consoled.c | 17 +++++++++++++----
> xen/include/xen/consoled.h | 32 +++++++++++++++++++++++++++-----
> 3 files changed, 45 insertions(+), 17 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 
> 4785f0e93a17e3ecba79a7813d2928f946abab8f..2d20a9d7531e069803eaf30ce79354b998c4a52f
>  100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -33,9 +33,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
> @@ -508,11 +508,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 )
> @@ -563,10 +561,9 @@ 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 )
> printk(KERN_ERR "d%pd: failed to process console input: %d\n", d, rc);
> diff --git a/xen/drivers/char/consoled.c b/xen/drivers/char/consoled.c
> index 
> b415b632cecc0a80e161b701d7b70ba4f3cc5fb8..8704ec251eb19e9c1cdc5927f896aeb20cc5af1e
>  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 
> bd7ab6329ee8a7c466484021247241ded8ed03c7..14e5e3284a86201919f0f70a8c2785609f35b15f
>  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 0;
> +}
> +
> +static inline int consoled_guest_tx(char c)
> +{
> + ASSERT_UNREACHABLE();
> + return 0;
> +}
> +
> +#define consoled_is_enabled() ( false )
> +
> +#endif / CONFIG_PV_SHIM /
> +
> +#endif / XEN__CONSOLED_H /
> /
> * Local variables:
> * mode: C
> 
> --
> 2.34.1
>