[PATCH v4 0/4] xen/console: cleanup console input switch logic

[PATCH v4 0/4] xen/console: cleanup console input switch logic

[dmkhn]

The patch series originates from the NS16550 UART emulator series [1] (x86)
which requires ability to switch physical console input to a PVH/HVM domain
with an emulated UART.

Currently, on x86, console input can be rotated in round-robin manner only
between dom0, PV shim, and Xen itself. On Arm the input rotation can include
domUs with vpl011.

The main idea of this series is introducing a per-domain permission flag that
is set during domain initialization and used by the console driver to switch
the input across permitted domains.

Patch 1 performs rename of switch_serial_input() to fit the console driver
notation.

Patch 2 introduces a new domain permission flag to mark ownership of the
console input for the console driver.

Patch 3 cleans up the console input switch logic by removing dependency
on max_init_domid. Depends on patches 1, 2 and [2].

Patch 4 performs rename of console_rx to console_domid to match the usage
of the symbol in the code.

[1] https://lore.kernel.org/xen-devel/20250103-vuart-ns8250-v3-v1-0-c5d36b31d66c@ford.com/
[2] https://lore.kernel.org/xen-devel/20250528225030.2652166-1-dmukhin@ford.com/
[3] Link to v3: https://lore.kernel.org/xen-devel/20250519201211.1366244-1-dmukhin@ford.com/
[4] Link to CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1841674752

Denis Mukhin (4):
  xen/console: rename switch_serial_input() to console_switch_input()
  xen/console: introduce console input permission
  xen/console: remove max_init_domid dependency
  xen/console: rename console_rx to console_domid

 xen/arch/arm/include/asm/setup.h        |  2 -
 xen/arch/arm/setup.c                    |  2 -
 xen/arch/arm/vpl011.c                   |  2 +
 xen/arch/ppc/include/asm/setup.h        |  2 -
 xen/arch/riscv/include/asm/setup.h      |  2 -
 xen/arch/x86/include/asm/setup.h        |  2 -
 xen/arch/x86/pv/shim.c                  |  2 +
 xen/common/device-tree/dom0less-build.c |  2 -
 xen/common/domain.c                     | 31 ++++++++
 xen/drivers/char/console.c              | 96 +++++++++++--------------
 xen/include/xen/domain.h                |  1 +
 xen/include/xen/sched.h                 |  8 ++-
 12 files changed, 85 insertions(+), 67 deletions(-)

-- 
2.34.1

[PATCH v4 1/4] xen/console: rename switch_serial_input() to console_switch_input()

[dmkhn]

From: Denis Mukhin <dmkhn@proton.me>

From: Denis Mukhin <dmukhin@ford.com>

Update the function name as per naming notation in the console driver.

No functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes since v3:
- added A-b
---
 xen/drivers/char/console.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index c15987f5bb..30701ae0b0 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -523,7 +523,7 @@ void console_put_domain(struct domain *d)
         rcu_unlock_domain(d);
 }
 
-static void switch_serial_input(void)
+static void console_switch_input(void)
 {
     unsigned int next_rx = console_rx;
 
@@ -617,7 +617,7 @@ static void cf_check serial_rx(char c)
         /* We eat CTRL-<switch_char> in groups of 3 to switch console input. */
         if ( ++switch_code_count == 3 )
         {
-            switch_serial_input();
+            console_switch_input();
             switch_code_count = 0;
         }
         return;
@@ -1171,7 +1171,7 @@ void __init console_endboot(void)
                             "toggle host/guest log level adjustment", 0);
 
     /* Serial input is directed to DOM0 by default. */
-    switch_serial_input();
+    console_switch_input();
 }
 
 int __init console_has(const char *device)
-- 
2.34.1

[PATCH v4 2/4] xen/console: introduce console input permission

[dmkhn]

From: Denis Mukhin <dmkhn@proton.me>

From: Denis Mukhin <dmukhin@ford.com>

Add new flag to domain structure for marking permission to intercept
the physical console input by the domain.

Update console input switch logic accordingly.

No functional change intended.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v3:
- rebased
---
 xen/arch/arm/vpl011.c      |  2 ++
 xen/arch/x86/pv/shim.c     |  2 ++
 xen/common/domain.c        |  2 ++
 xen/drivers/char/console.c | 18 +++++++++++++++++-
 xen/include/xen/sched.h    |  8 +++++++-
 5 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 66047bf33c..147958eee8 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -737,6 +737,8 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
     register_mmio_handler(d, &vpl011_mmio_handler,
                           vpl011->base_addr, GUEST_PL011_SIZE, NULL);
 
+    d->console.input_allowed = true;
+
     return 0;
 
 out1:
diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index c506cc0bec..bc2a7dd5fa 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -238,6 +238,8 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
      * guest from depleting the shim memory pool.
      */
     d->max_pages = domain_tot_pages(d);
+
+    d->console.input_allowed = true;
 }
 
 static void write_start_info(struct domain *d)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 87e5be35e5..9bc66d80c4 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -835,6 +835,8 @@ struct domain *domain_create(domid_t domid,
         flags |= CDF_hardware;
         if ( old_hwdom )
             old_hwdom->cdf &= ~CDF_hardware;
+
+        d->console.input_allowed = true;
     }
 
     /* Holding CDF_* internal flags. */
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 30701ae0b0..8a0bcff78f 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -512,9 +512,21 @@ static unsigned int __read_mostly console_rx = 0;
 
 struct domain *console_get_domain(void)
 {
+    struct domain *d;
+
     if ( console_rx == 0 )
             return NULL;
-    return rcu_lock_domain_by_id(console_rx - 1);
+
+    d = rcu_lock_domain_by_id(console_rx - 1);
+    if ( !d )
+        return NULL;
+
+    if ( d->console.input_allowed )
+        return d;
+
+    rcu_unlock_domain(d);
+
+    return NULL;
 }
 
 void console_put_domain(struct domain *d)
@@ -551,6 +563,10 @@ static void console_switch_input(void)
         if ( d )
         {
             rcu_unlock_domain(d);
+
+            if ( !d->console.input_allowed )
+                break;
+
             console_rx = next_rx;
             printk("*** Serial input to DOM%u", domid);
             break;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 559d201e0c..e91c99a8f3 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -512,7 +512,7 @@ struct domain
     bool             auto_node_affinity;
     /* Is this guest fully privileged (aka dom0)? */
     bool             is_privileged;
-    /* Can this guest access the Xen console? */
+    /* XSM: permission to use HYPERCALL_console_io hypercall */
     bool             is_console;
     /* Is this guest being debugged by dom0? */
     bool             debugger_attached;
@@ -651,6 +651,12 @@ struct domain
     unsigned int num_llc_colors;
     const unsigned int *llc_colors;
 #endif
+
+    /* Console settings. */
+    struct {
+        /* Permission to take ownership of the physical console input. */
+        bool input_allowed;
+    } console;
 } __aligned(PAGE_SIZE);
 
 static inline struct page_list_head *page_to_list(
-- 
2.34.1

[PATCH v4 3/4] xen/console: remove max_init_domid dependency

[dmkhn]

From: Denis Mukhin <dmkhn@proton.me>

From: Denis Mukhin <dmukhin@ford.com>

The physical console input rotation depends on max_init_domid symbol, which is
managed differently across architectures.

Instead of trying to manage max_init_domid in the arch-common code the console
input rotation code can be reworked by removing dependency on max_init_domid
entirely.

To do that, introduce domid_find_with_input_allowed() in arch-independent
location to find the ID of the next possible console owner domain. The IDs
are rotated across non-system domain IDs and DOMID_XEN.

Also, introduce helper console_set_domid() for updating identifier of the
current console input owner (points to Xen or domain).

Use domid_find_with_input_allowed() and console_set_domid() in
console_switch_input().

Remove uses of max_init_domid in the code.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v3:
- switched to RCU lock in domid_find_with_input_allowed()
---
 xen/arch/arm/include/asm/setup.h        |  2 -
 xen/arch/arm/setup.c                    |  2 -
 xen/arch/ppc/include/asm/setup.h        |  2 -
 xen/arch/riscv/include/asm/setup.h      |  2 -
 xen/arch/x86/include/asm/setup.h        |  2 -
 xen/common/device-tree/dom0less-build.c |  2 -
 xen/common/domain.c                     | 29 ++++++++
 xen/drivers/char/console.c              | 90 +++++++++----------------
 xen/include/xen/domain.h                |  1 +
 9 files changed, 61 insertions(+), 71 deletions(-)

diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
index 6cf272c160..f107e8eebb 100644
--- a/xen/arch/arm/include/asm/setup.h
+++ b/xen/arch/arm/include/asm/setup.h
@@ -25,8 +25,6 @@ struct map_range_data
     struct rangeset *irq_ranges;
 };
 
-extern domid_t max_init_domid;
-
 void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
 
 size_t estimate_efi_size(unsigned int mem_nr_banks);
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 10b46d0684..53e2f8b537 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -61,8 +61,6 @@ struct cpuinfo_arm __read_mostly system_cpuinfo;
 bool __read_mostly acpi_disabled;
 #endif
 
-domid_t __read_mostly max_init_domid;
-
 static __used void init_done(void)
 {
     int rc;
diff --git a/xen/arch/ppc/include/asm/setup.h b/xen/arch/ppc/include/asm/setup.h
index e4f64879b6..956fa6985a 100644
--- a/xen/arch/ppc/include/asm/setup.h
+++ b/xen/arch/ppc/include/asm/setup.h
@@ -1,6 +1,4 @@
 #ifndef __ASM_PPC_SETUP_H__
 #define __ASM_PPC_SETUP_H__
 
-#define max_init_domid (0)
-
 #endif /* __ASM_PPC_SETUP_H__ */
diff --git a/xen/arch/riscv/include/asm/setup.h b/xen/arch/riscv/include/asm/setup.h
index c9d69cdf51..d1fc64b673 100644
--- a/xen/arch/riscv/include/asm/setup.h
+++ b/xen/arch/riscv/include/asm/setup.h
@@ -5,8 +5,6 @@
 
 #include <xen/types.h>
 
-#define max_init_domid (0)
-
 void setup_mm(void);
 
 void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index ac34c69855..b67de8577f 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -69,6 +69,4 @@ extern bool opt_dom0_verbose;
 extern bool opt_dom0_cpuid_faulting;
 extern bool opt_dom0_msr_relaxed;
 
-#define max_init_domid (0)
-
 #endif
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index 9a6015f4ce..703f20faed 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -977,8 +977,6 @@ void __init create_domUs(void)
         domid = domid_alloc(DOMID_INVALID);
         if ( domid == DOMID_INVALID )
             panic("Error allocating ID for domain %s\n", dt_node_name(node));
-        if ( max_init_domid < domid )
-            max_init_domid = domid;
 
         d = domain_create(domid, &d_cfg, flags);
         if ( IS_ERR(d) )
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 9bc66d80c4..704e0907e9 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -2463,6 +2463,35 @@ void domid_free(domid_t domid)
     spin_unlock(&domid_lock);
 }
 
+/*
+ * Find the ID of the next possible console owner domain.
+ *
+ * @return Domain ID: DOMID_XEN or non-system domain IDs within
+ * the range of [0..DOMID_FIRST_RESERVED-1].
+ */
+domid_t domid_find_with_input_allowed(domid_t hint)
+{
+    const struct domain *d;
+    domid_t domid = DOMID_XEN;
+
+    rcu_read_lock(&domlist_read_lock);
+
+    for ( d = rcu_dereference(domain_list);
+          d && d->domain_id < DOMID_FIRST_RESERVED;
+          d = rcu_dereference(d->next_in_list) )
+    {
+        if ( d->console.input_allowed )
+        {
+            domid = d->domain_id;
+            break;
+        }
+    }
+
+    rcu_read_unlock(&domlist_read_lock);
+
+    return domid;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 8a0bcff78f..37289d5558 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -498,26 +498,17 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
 
 /*
  * CTRL-<switch_char> changes input direction, rotating among Xen, Dom0,
- * and the DomUs started from Xen at boot.
+ * and the DomUs.
  */
 #define switch_code (opt_conswitch[0]-'a'+1)
-/*
- * console_rx=0 => input to xen
- * console_rx=1 => input to dom0 (or the sole shim domain)
- * console_rx=N => input to dom(N-1)
- */
-static unsigned int __read_mostly console_rx = 0;
 
-#define max_console_rx (max_init_domid + 1)
+/* Console owner domain identifier. */
+static domid_t __read_mostly console_rx = DOMID_XEN;
 
 struct domain *console_get_domain(void)
 {
-    struct domain *d;
+    struct domain *d = rcu_lock_domain_by_id(console_rx);
 
-    if ( console_rx == 0 )
-            return NULL;
-
-    d = rcu_lock_domain_by_id(console_rx - 1);
     if ( !d )
         return NULL;
 
@@ -535,43 +526,14 @@ void console_put_domain(struct domain *d)
         rcu_unlock_domain(d);
 }
 
-static void console_switch_input(void)
+static void console_set_domid(domid_t domid)
 {
-    unsigned int next_rx = console_rx;
+    if ( domid == DOMID_XEN )
+        printk("*** Serial input to Xen");
+    else
+        printk("*** Serial input to DOM%u", domid);
 
-    /*
-     * Rotate among Xen, dom0 and boot-time created domUs while skipping
-     * switching serial input to non existing domains.
-     */
-    for ( ; ; )
-    {
-        domid_t domid;
-        struct domain *d;
-
-        if ( next_rx++ >= max_console_rx )
-        {
-            console_rx = 0;
-            printk("*** Serial input to Xen");
-            break;
-        }
-
-        if ( consoled_is_enabled() && next_rx == 1 )
-            domid = get_initial_domain_id();
-        else
-            domid = next_rx - 1;
-        d = rcu_lock_domain_by_id(domid);
-        if ( d )
-        {
-            rcu_unlock_domain(d);
-
-            if ( !d->console.input_allowed )
-                break;
-
-            console_rx = next_rx;
-            printk("*** Serial input to DOM%u", domid);
-            break;
-        }
-    }
+    console_rx = domid;
 
     if ( switch_code )
         printk(" (type 'CTRL-%c' three times to switch input)",
@@ -579,12 +541,30 @@ static void console_switch_input(void)
     printk("\n");
 }
 
+/*
+ * Switch console focus.
+ * Rotates input focus among Xen and domains with console input permission.
+ */
+static void console_switch_input(void)
+{
+    domid_t hint;
+
+    if ( console_rx == DOMID_XEN )
+        hint = get_initial_domain_id();
+    else
+        hint = console_rx + 1;
+
+    hint = domid_find_with_input_allowed(hint);
+
+    console_set_domid(hint);
+}
+
 static void __serial_rx(char c)
 {
     struct domain *d;
     int rc = 0;
 
-    if ( console_rx == 0 )
+    if ( console_rx == DOMID_XEN )
         return handle_keypress(c, false);
 
     d = console_get_domain();
@@ -1169,14 +1149,6 @@ void __init console_endboot(void)
 
     video_endboot();
 
-    /*
-     * If user specifies so, we fool the switch routine to redirect input
-     * straight back to Xen. I use this convoluted method so we still print
-     * a useful 'how to switch' message.
-     */
-    if ( opt_conswitch[1] == 'x' )
-        console_rx = max_console_rx;
-
     register_keyhandler('w', conring_dump_keyhandler,
                         "synchronously dump console ring buffer (dmesg)", 0);
     register_irq_keyhandler('+', &do_inc_thresh,
@@ -1186,8 +1158,8 @@ void __init console_endboot(void)
     register_irq_keyhandler('G', &do_toggle_guest,
                             "toggle host/guest log level adjustment", 0);
 
-    /* Serial input is directed to DOM0 by default. */
-    console_switch_input();
+    if ( opt_conswitch[1] != 'x' )
+        (void)console_set_domid(get_initial_domain_id());
 }
 
 int __init console_has(const char *device)
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 8aab05ae93..a88eb34f3f 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -36,6 +36,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info);
 void arch_get_domain_info(const struct domain *d,
                           struct xen_domctl_getdomaininfo *info);
 
+domid_t domid_find_with_input_allowed(domid_t hint);
 domid_t get_initial_domain_id(void);
 
 domid_t domid_alloc(domid_t domid);
-- 
2.34.1

[PATCH v4 4/4] xen/console: rename console_rx to console_domid

[dmkhn]

From: Denis Mukhin <dmkhn@proton.me>

From: Denis Mukhin <dmukhin@ford.com>

Update the symbol name to match the code better.

No functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v3:
- rebased
---
 xen/drivers/char/console.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 37289d5558..5797f29d31 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -503,11 +503,11 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
 #define switch_code (opt_conswitch[0]-'a'+1)
 
 /* Console owner domain identifier. */
-static domid_t __read_mostly console_rx = DOMID_XEN;
+static domid_t __read_mostly console_domid = DOMID_XEN;
 
 struct domain *console_get_domain(void)
 {
-    struct domain *d = rcu_lock_domain_by_id(console_rx);
+    struct domain *d = rcu_lock_domain_by_id(console_domid);
 
     if ( !d )
         return NULL;
@@ -533,7 +533,7 @@ static void console_set_domid(domid_t domid)
     else
         printk("*** Serial input to DOM%u", domid);
 
-    console_rx = domid;
+    console_domid = domid;
 
     if ( switch_code )
         printk(" (type 'CTRL-%c' three times to switch input)",
@@ -549,10 +549,10 @@ static void console_switch_input(void)
 {
     domid_t hint;
 
-    if ( console_rx == DOMID_XEN )
+    if ( console_domid == DOMID_XEN )
         hint = get_initial_domain_id();
     else
-        hint = console_rx + 1;
+        hint = console_domid + 1;
 
     hint = domid_find_with_input_allowed(hint);
 
@@ -564,7 +564,7 @@ static void __serial_rx(char c)
     struct domain *d;
     int rc = 0;
 
-    if ( console_rx == DOMID_XEN )
+    if ( console_domid == DOMID_XEN )
         return handle_keypress(c, false);
 
     d = console_get_domain();
-- 
2.34.1

Re: [PATCH v4 2/4] xen/console: introduce console input permission

[Stefano Stabellini]

On Thu, 29 May 2025, dmkhn@proton.me wrote:
> Add new flag to domain structure for marking permission to intercept
> the physical console input by the domain.
> 
> Update console input switch logic accordingly.
> 
> No functional change intended.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v3:
> - rebased
> ---
>  xen/arch/arm/vpl011.c      |  2 ++
>  xen/arch/x86/pv/shim.c     |  2 ++
>  xen/common/domain.c        |  2 ++
>  xen/drivers/char/console.c | 18 +++++++++++++++++-
>  xen/include/xen/sched.h    |  8 +++++++-
>  5 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index 66047bf33c..147958eee8 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -737,6 +737,8 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
>      register_mmio_handler(d, &vpl011_mmio_handler,
>                            vpl011->base_addr, GUEST_PL011_SIZE, NULL);
>  
> +    d->console.input_allowed = true;

This should be set only when backend_in_domain = false.


>      return 0;
>  
>  out1:
> diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
> index c506cc0bec..bc2a7dd5fa 100644
> --- a/xen/arch/x86/pv/shim.c
> +++ b/xen/arch/x86/pv/shim.c
> @@ -238,6 +238,8 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
>       * guest from depleting the shim memory pool.
>       */
>      d->max_pages = domain_tot_pages(d);
> +
> +    d->console.input_allowed = true;
>  }
>  
>  static void write_start_info(struct domain *d)
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 87e5be35e5..9bc66d80c4 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -835,6 +835,8 @@ struct domain *domain_create(domid_t domid,
>          flags |= CDF_hardware;
>          if ( old_hwdom )
>              old_hwdom->cdf &= ~CDF_hardware;
> +
> +        d->console.input_allowed = true;
>      }
>  
>      /* Holding CDF_* internal flags. */
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 30701ae0b0..8a0bcff78f 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -512,9 +512,21 @@ static unsigned int __read_mostly console_rx = 0;
>  
>  struct domain *console_get_domain(void)
>  {
> +    struct domain *d;
> +
>      if ( console_rx == 0 )
>              return NULL;
> -    return rcu_lock_domain_by_id(console_rx - 1);
> +
> +    d = rcu_lock_domain_by_id(console_rx - 1);
> +    if ( !d )
> +        return NULL;
> +
> +    if ( d->console.input_allowed )
> +        return d;
> +
> +    rcu_unlock_domain(d);
> +
> +    return NULL;

The original idea was to skip over domains that cannot have any input so
I don't think we should get in this situation. We could even have an
assert.


>  }
>  
>  void console_put_domain(struct domain *d)
> @@ -551,6 +563,10 @@ static void console_switch_input(void)
>          if ( d )
>          {
>              rcu_unlock_domain(d);
> +
> +            if ( !d->console.input_allowed )
> +                break;

shouldn't this be continue instead of break?


>              console_rx = next_rx;
>              printk("*** Serial input to DOM%u", domid);
>              break;
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 559d201e0c..e91c99a8f3 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -512,7 +512,7 @@ struct domain
>      bool             auto_node_affinity;
>      /* Is this guest fully privileged (aka dom0)? */
>      bool             is_privileged;
> -    /* Can this guest access the Xen console? */
> +    /* XSM: permission to use HYPERCALL_console_io hypercall */
>      bool             is_console;

While I am in favor of this direction and we certainly need a better way
to distinguish domains that can use HYPERCALL_console_io hypercall from
others, could we simplify this and just assume that "is_console" implies
input_allowed and also set is_console = true in all the same places you
are setting input_allowed = true in this patch?

For clarity, I am suggesting:
- do not add input_allowed
- set is_console = true in domain_vpl011_init, pv_shim_setup_dom, etc.

The only side effect is that we would allow domains with vpl011 to also
use console hypercalls but I don't think there is any harm in that?

I don't feel strongly about this, I am just trying to find ways to make
things simple. I apologize if it was already discussed during review of
one of the previous versions.

I am also OK with this approach.


>      /* Is this guest being debugged by dom0? */
>      bool             debugger_attached;
> @@ -651,6 +651,12 @@ struct domain
>      unsigned int num_llc_colors;
>      const unsigned int *llc_colors;
>  #endif
> +
> +    /* Console settings. */
> +    struct {
> +        /* Permission to take ownership of the physical console input. */
> +        bool input_allowed;
> +    } console;
>  } __aligned(PAGE_SIZE);
>  
>  static inline struct page_list_head *page_to_list(
> -- 
> 2.34.1
> 
> 

Re: [PATCH v4 3/4] xen/console: remove max_init_domid dependency

[Stefano Stabellini]

On Thu, 29 May 2025, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmkhn@proton.me>
> 
> From: Denis Mukhin <dmukhin@ford.com>
> 
> The physical console input rotation depends on max_init_domid symbol, which is
> managed differently across architectures.
> 
> Instead of trying to manage max_init_domid in the arch-common code the console
> input rotation code can be reworked by removing dependency on max_init_domid
> entirely.
> 
> To do that, introduce domid_find_with_input_allowed() in arch-independent
> location to find the ID of the next possible console owner domain. The IDs
> are rotated across non-system domain IDs and DOMID_XEN.
> 
> Also, introduce helper console_set_domid() for updating identifier of the
> current console input owner (points to Xen or domain).
> 
> Use domid_find_with_input_allowed() and console_set_domid() in
> console_switch_input().
> 
> Remove uses of max_init_domid in the code.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v3:
> - switched to RCU lock in domid_find_with_input_allowed()
> ---
>  xen/arch/arm/include/asm/setup.h        |  2 -
>  xen/arch/arm/setup.c                    |  2 -
>  xen/arch/ppc/include/asm/setup.h        |  2 -
>  xen/arch/riscv/include/asm/setup.h      |  2 -
>  xen/arch/x86/include/asm/setup.h        |  2 -
>  xen/common/device-tree/dom0less-build.c |  2 -
>  xen/common/domain.c                     | 29 ++++++++
>  xen/drivers/char/console.c              | 90 +++++++++----------------
>  xen/include/xen/domain.h                |  1 +
>  9 files changed, 61 insertions(+), 71 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
> index 6cf272c160..f107e8eebb 100644
> --- a/xen/arch/arm/include/asm/setup.h
> +++ b/xen/arch/arm/include/asm/setup.h
> @@ -25,8 +25,6 @@ struct map_range_data
>      struct rangeset *irq_ranges;
>  };
>  
> -extern domid_t max_init_domid;
> -
>  void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
>  
>  size_t estimate_efi_size(unsigned int mem_nr_banks);
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 10b46d0684..53e2f8b537 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -61,8 +61,6 @@ struct cpuinfo_arm __read_mostly system_cpuinfo;
>  bool __read_mostly acpi_disabled;
>  #endif
>  
> -domid_t __read_mostly max_init_domid;
> -
>  static __used void init_done(void)
>  {
>      int rc;
> diff --git a/xen/arch/ppc/include/asm/setup.h b/xen/arch/ppc/include/asm/setup.h
> index e4f64879b6..956fa6985a 100644
> --- a/xen/arch/ppc/include/asm/setup.h
> +++ b/xen/arch/ppc/include/asm/setup.h
> @@ -1,6 +1,4 @@
>  #ifndef __ASM_PPC_SETUP_H__
>  #define __ASM_PPC_SETUP_H__
>  
> -#define max_init_domid (0)
> -
>  #endif /* __ASM_PPC_SETUP_H__ */
> diff --git a/xen/arch/riscv/include/asm/setup.h b/xen/arch/riscv/include/asm/setup.h
> index c9d69cdf51..d1fc64b673 100644
> --- a/xen/arch/riscv/include/asm/setup.h
> +++ b/xen/arch/riscv/include/asm/setup.h
> @@ -5,8 +5,6 @@
>  
>  #include <xen/types.h>
>  
> -#define max_init_domid (0)
> -
>  void setup_mm(void);
>  
>  void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
> diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
> index ac34c69855..b67de8577f 100644
> --- a/xen/arch/x86/include/asm/setup.h
> +++ b/xen/arch/x86/include/asm/setup.h
> @@ -69,6 +69,4 @@ extern bool opt_dom0_verbose;
>  extern bool opt_dom0_cpuid_faulting;
>  extern bool opt_dom0_msr_relaxed;
>  
> -#define max_init_domid (0)
> -
>  #endif
> diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
> index 9a6015f4ce..703f20faed 100644
> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -977,8 +977,6 @@ void __init create_domUs(void)
>          domid = domid_alloc(DOMID_INVALID);
>          if ( domid == DOMID_INVALID )
>              panic("Error allocating ID for domain %s\n", dt_node_name(node));
> -        if ( max_init_domid < domid )
> -            max_init_domid = domid;
>  
>          d = domain_create(domid, &d_cfg, flags);
>          if ( IS_ERR(d) )
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 9bc66d80c4..704e0907e9 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -2463,6 +2463,35 @@ void domid_free(domid_t domid)
>      spin_unlock(&domid_lock);
>  }
>  
> +/*
> + * Find the ID of the next possible console owner domain.
> + *
> + * @return Domain ID: DOMID_XEN or non-system domain IDs within
> + * the range of [0..DOMID_FIRST_RESERVED-1].
> + */
> +domid_t domid_find_with_input_allowed(domid_t hint)
> +{
> +    const struct domain *d;
> +    domid_t domid = DOMID_XEN;
> +
> +    rcu_read_lock(&domlist_read_lock);
> +
> +    for ( d = rcu_dereference(domain_list);
> +          d && d->domain_id < DOMID_FIRST_RESERVED;
> +          d = rcu_dereference(d->next_in_list) )
> +    {
> +        if ( d->console.input_allowed )
> +        {
> +            domid = d->domain_id;
> +            break;
> +        }
> +    }
> +
> +    rcu_read_unlock(&domlist_read_lock);

Doesn't this always return the first domid with input_allowed given that
hint is not used? It looks like it wouldn't work right...


> +    return domid;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 8a0bcff78f..37289d5558 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -498,26 +498,17 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
>  
>  /*
>   * CTRL-<switch_char> changes input direction, rotating among Xen, Dom0,
> - * and the DomUs started from Xen at boot.
> + * and the DomUs.
>   */
>  #define switch_code (opt_conswitch[0]-'a'+1)
> -/*
> - * console_rx=0 => input to xen
> - * console_rx=1 => input to dom0 (or the sole shim domain)
> - * console_rx=N => input to dom(N-1)
> - */
> -static unsigned int __read_mostly console_rx = 0;
>  
> -#define max_console_rx (max_init_domid + 1)
> +/* Console owner domain identifier. */
> +static domid_t __read_mostly console_rx = DOMID_XEN;
>  
>  struct domain *console_get_domain(void)
>  {
> -    struct domain *d;
> +    struct domain *d = rcu_lock_domain_by_id(console_rx);
>  
> -    if ( console_rx == 0 )
> -            return NULL;
> -
> -    d = rcu_lock_domain_by_id(console_rx - 1);
>      if ( !d )
>          return NULL;
>  
> @@ -535,43 +526,14 @@ void console_put_domain(struct domain *d)
>          rcu_unlock_domain(d);
>  }
>  
> -static void console_switch_input(void)
> +static void console_set_domid(domid_t domid)
>  {
> -    unsigned int next_rx = console_rx;
> +    if ( domid == DOMID_XEN )
> +        printk("*** Serial input to Xen");
> +    else
> +        printk("*** Serial input to DOM%u", domid);
>  
> -    /*
> -     * Rotate among Xen, dom0 and boot-time created domUs while skipping
> -     * switching serial input to non existing domains.
> -     */
> -    for ( ; ; )
> -    {
> -        domid_t domid;
> -        struct domain *d;
> -
> -        if ( next_rx++ >= max_console_rx )
> -        {
> -            console_rx = 0;
> -            printk("*** Serial input to Xen");
> -            break;
> -        }
> -
> -        if ( consoled_is_enabled() && next_rx == 1 )
> -            domid = get_initial_domain_id();
> -        else
> -            domid = next_rx - 1;
> -        d = rcu_lock_domain_by_id(domid);
> -        if ( d )
> -        {
> -            rcu_unlock_domain(d);
> -
> -            if ( !d->console.input_allowed )
> -                break;
> -
> -            console_rx = next_rx;
> -            printk("*** Serial input to DOM%u", domid);
> -            break;
> -        }
> -    }
> +    console_rx = domid;
>  
>      if ( switch_code )
>          printk(" (type 'CTRL-%c' three times to switch input)",
> @@ -579,12 +541,30 @@ static void console_switch_input(void)
>      printk("\n");
>  }
>  
> +/*
> + * Switch console focus.
> + * Rotates input focus among Xen and domains with console input permission.
> + */
> +static void console_switch_input(void)
> +{
> +    domid_t hint;
> +
> +    if ( console_rx == DOMID_XEN )
> +        hint = get_initial_domain_id();
> +    else
> +        hint = console_rx + 1;
> +
> +    hint = domid_find_with_input_allowed(hint);
> +
> +    console_set_domid(hint);
> +}
> +
>  static void __serial_rx(char c)
>  {
>      struct domain *d;
>      int rc = 0;
>  
> -    if ( console_rx == 0 )
> +    if ( console_rx == DOMID_XEN )
>          return handle_keypress(c, false);
>  
>      d = console_get_domain();
> @@ -1169,14 +1149,6 @@ void __init console_endboot(void)
>  
>      video_endboot();
>  
> -    /*
> -     * If user specifies so, we fool the switch routine to redirect input
> -     * straight back to Xen. I use this convoluted method so we still print
> -     * a useful 'how to switch' message.
> -     */
> -    if ( opt_conswitch[1] == 'x' )
> -        console_rx = max_console_rx;
> -
>      register_keyhandler('w', conring_dump_keyhandler,
>                          "synchronously dump console ring buffer (dmesg)", 0);
>      register_irq_keyhandler('+', &do_inc_thresh,
> @@ -1186,8 +1158,8 @@ void __init console_endboot(void)
>      register_irq_keyhandler('G', &do_toggle_guest,
>                              "toggle host/guest log level adjustment", 0);
>  
> -    /* Serial input is directed to DOM0 by default. */
> -    console_switch_input();
> +    if ( opt_conswitch[1] != 'x' )
> +        (void)console_set_domid(get_initial_domain_id());

We should use domid_find_with_input_allowed instead of assuming
get_initial_domain_id() has input_allowed? 



>  }
>  
>  int __init console_has(const char *device)
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index 8aab05ae93..a88eb34f3f 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -36,6 +36,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info);
>  void arch_get_domain_info(const struct domain *d,
>                            struct xen_domctl_getdomaininfo *info);
>  
> +domid_t domid_find_with_input_allowed(domid_t hint);
>  domid_t get_initial_domain_id(void);
>  
>  domid_t domid_alloc(domid_t domid);
> -- 
> 2.34.1
> 
> 

Re: [PATCH v4 3/4] xen/console: remove max_init_domid dependency

[dmkhn]

On Thu, May 29, 2025 at 05:58:20PM -0700, Stefano Stabellini wrote:
> On Thu, 29 May 2025, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmkhn@proton.me>
> >
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > The physical console input rotation depends on max_init_domid symbol, which is
> > managed differently across architectures.
> >
> > Instead of trying to manage max_init_domid in the arch-common code the console
> > input rotation code can be reworked by removing dependency on max_init_domid
> > entirely.
> >
> > To do that, introduce domid_find_with_input_allowed() in arch-independent
> > location to find the ID of the next possible console owner domain. The IDs
> > are rotated across non-system domain IDs and DOMID_XEN.
> >
> > Also, introduce helper console_set_domid() for updating identifier of the
> > current console input owner (points to Xen or domain).
> >
> > Use domid_find_with_input_allowed() and console_set_domid() in
> > console_switch_input().
> >
> > Remove uses of max_init_domid in the code.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> > Changes since v3:
> > - switched to RCU lock in domid_find_with_input_allowed()
> > ---
> >  xen/arch/arm/include/asm/setup.h        |  2 -
> >  xen/arch/arm/setup.c                    |  2 -
> >  xen/arch/ppc/include/asm/setup.h        |  2 -
> >  xen/arch/riscv/include/asm/setup.h      |  2 -
> >  xen/arch/x86/include/asm/setup.h        |  2 -
> >  xen/common/device-tree/dom0less-build.c |  2 -
> >  xen/common/domain.c                     | 29 ++++++++
> >  xen/drivers/char/console.c              | 90 +++++++++----------------
> >  xen/include/xen/domain.h                |  1 +
> >  9 files changed, 61 insertions(+), 71 deletions(-)
> >
> > diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
> > index 6cf272c160..f107e8eebb 100644
> > --- a/xen/arch/arm/include/asm/setup.h
> > +++ b/xen/arch/arm/include/asm/setup.h
> > @@ -25,8 +25,6 @@ struct map_range_data
> >      struct rangeset *irq_ranges;
> >  };
> >
> > -extern domid_t max_init_domid;
> > -
> >  void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
> >
> >  size_t estimate_efi_size(unsigned int mem_nr_banks);
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 10b46d0684..53e2f8b537 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -61,8 +61,6 @@ struct cpuinfo_arm __read_mostly system_cpuinfo;
> >  bool __read_mostly acpi_disabled;
> >  #endif
> >
> > -domid_t __read_mostly max_init_domid;
> > -
> >  static __used void init_done(void)
> >  {
> >      int rc;
> > diff --git a/xen/arch/ppc/include/asm/setup.h b/xen/arch/ppc/include/asm/setup.h
> > index e4f64879b6..956fa6985a 100644
> > --- a/xen/arch/ppc/include/asm/setup.h
> > +++ b/xen/arch/ppc/include/asm/setup.h
> > @@ -1,6 +1,4 @@
> >  #ifndef __ASM_PPC_SETUP_H__
> >  #define __ASM_PPC_SETUP_H__
> >
> > -#define max_init_domid (0)
> > -
> >  #endif /* __ASM_PPC_SETUP_H__ */
> > diff --git a/xen/arch/riscv/include/asm/setup.h b/xen/arch/riscv/include/asm/setup.h
> > index c9d69cdf51..d1fc64b673 100644
> > --- a/xen/arch/riscv/include/asm/setup.h
> > +++ b/xen/arch/riscv/include/asm/setup.h
> > @@ -5,8 +5,6 @@
> >
> >  #include <xen/types.h>
> >
> > -#define max_init_domid (0)
> > -
> >  void setup_mm(void);
> >
> >  void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
> > diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
> > index ac34c69855..b67de8577f 100644
> > --- a/xen/arch/x86/include/asm/setup.h
> > +++ b/xen/arch/x86/include/asm/setup.h
> > @@ -69,6 +69,4 @@ extern bool opt_dom0_verbose;
> >  extern bool opt_dom0_cpuid_faulting;
> >  extern bool opt_dom0_msr_relaxed;
> >
> > -#define max_init_domid (0)
> > -
> >  #endif
> > diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
> > index 9a6015f4ce..703f20faed 100644
> > --- a/xen/common/device-tree/dom0less-build.c
> > +++ b/xen/common/device-tree/dom0less-build.c
> > @@ -977,8 +977,6 @@ void __init create_domUs(void)
> >          domid = domid_alloc(DOMID_INVALID);
> >          if ( domid == DOMID_INVALID )
> >              panic("Error allocating ID for domain %s\n", dt_node_name(node));
> > -        if ( max_init_domid < domid )
> > -            max_init_domid = domid;
> >
> >          d = domain_create(domid, &d_cfg, flags);
> >          if ( IS_ERR(d) )
> > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > index 9bc66d80c4..704e0907e9 100644
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -2463,6 +2463,35 @@ void domid_free(domid_t domid)
> >      spin_unlock(&domid_lock);
> >  }
> >
> > +/*
> > + * Find the ID of the next possible console owner domain.
> > + *
> > + * @return Domain ID: DOMID_XEN or non-system domain IDs within
> > + * the range of [0..DOMID_FIRST_RESERVED-1].
> > + */
> > +domid_t domid_find_with_input_allowed(domid_t hint)
> > +{
> > +    const struct domain *d;
> > +    domid_t domid = DOMID_XEN;
> > +
> > +    rcu_read_lock(&domlist_read_lock);
> > +
> > +    for ( d = rcu_dereference(domain_list);
> > +          d && d->domain_id < DOMID_FIRST_RESERVED;
> > +          d = rcu_dereference(d->next_in_list) )
> > +    {
> > +        if ( d->console.input_allowed )
> > +        {
> > +            domid = d->domain_id;
> > +            break;
> > +        }
> > +    }
> > +
> > +    rcu_read_unlock(&domlist_read_lock);
> 
> Doesn't this always return the first domid with input_allowed given that
> hint is not used? It looks like it wouldn't work right...

Yes, that will not work.
Looks like I posted the series from the wrong local branch.
Will update.

Thanks!

> 
> 
> > +    return domid;
> > +}
> > +
> >  /*
> >   * Local variables:
> >   * mode: C
> > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> > index 8a0bcff78f..37289d5558 100644
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -498,26 +498,17 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
> >
> >  /*
> >   * CTRL-<switch_char> changes input direction, rotating among Xen, Dom0,
> > - * and the DomUs started from Xen at boot.
> > + * and the DomUs.
> >   */
> >  #define switch_code (opt_conswitch[0]-'a'+1)
> > -/*
> > - * console_rx=0 => input to xen
> > - * console_rx=1 => input to dom0 (or the sole shim domain)
> > - * console_rx=N => input to dom(N-1)
> > - */
> > -static unsigned int __read_mostly console_rx = 0;
> >
> > -#define max_console_rx (max_init_domid + 1)
> > +/* Console owner domain identifier. */
> > +static domid_t __read_mostly console_rx = DOMID_XEN;
> >
> >  struct domain *console_get_domain(void)
> >  {
> > -    struct domain *d;
> > +    struct domain *d = rcu_lock_domain_by_id(console_rx);
> >
> > -    if ( console_rx == 0 )
> > -            return NULL;
> > -
> > -    d = rcu_lock_domain_by_id(console_rx - 1);
> >      if ( !d )
> >          return NULL;
> >
> > @@ -535,43 +526,14 @@ void console_put_domain(struct domain *d)
> >          rcu_unlock_domain(d);
> >  }
> >
> > -static void console_switch_input(void)
> > +static void console_set_domid(domid_t domid)
> >  {
> > -    unsigned int next_rx = console_rx;
> > +    if ( domid == DOMID_XEN )
> > +        printk("*** Serial input to Xen");
> > +    else
> > +        printk("*** Serial input to DOM%u", domid);
> >
> > -    /*
> > -     * Rotate among Xen, dom0 and boot-time created domUs while skipping
> > -     * switching serial input to non existing domains.
> > -     */
> > -    for ( ; ; )
> > -    {
> > -        domid_t domid;
> > -        struct domain *d;
> > -
> > -        if ( next_rx++ >= max_console_rx )
> > -        {
> > -            console_rx = 0;
> > -            printk("*** Serial input to Xen");
> > -            break;
> > -        }
> > -
> > -        if ( consoled_is_enabled() && next_rx == 1 )
> > -            domid = get_initial_domain_id();
> > -        else
> > -            domid = next_rx - 1;
> > -        d = rcu_lock_domain_by_id(domid);
> > -        if ( d )
> > -        {
> > -            rcu_unlock_domain(d);
> > -
> > -            if ( !d->console.input_allowed )
> > -                break;
> > -
> > -            console_rx = next_rx;
> > -            printk("*** Serial input to DOM%u", domid);
> > -            break;
> > -        }
> > -    }
> > +    console_rx = domid;
> >
> >      if ( switch_code )
> >          printk(" (type 'CTRL-%c' three times to switch input)",
> > @@ -579,12 +541,30 @@ static void console_switch_input(void)
> >      printk("\n");
> >  }
> >
> > +/*
> > + * Switch console focus.
> > + * Rotates input focus among Xen and domains with console input permission.
> > + */
> > +static void console_switch_input(void)
> > +{
> > +    domid_t hint;
> > +
> > +    if ( console_rx == DOMID_XEN )
> > +        hint = get_initial_domain_id();
> > +    else
> > +        hint = console_rx + 1;
> > +
> > +    hint = domid_find_with_input_allowed(hint);
> > +
> > +    console_set_domid(hint);
> > +}
> > +
> >  static void __serial_rx(char c)
> >  {
> >      struct domain *d;
> >      int rc = 0;
> >
> > -    if ( console_rx == 0 )
> > +    if ( console_rx == DOMID_XEN )
> >          return handle_keypress(c, false);
> >
> >      d = console_get_domain();
> > @@ -1169,14 +1149,6 @@ void __init console_endboot(void)
> >
> >      video_endboot();
> >
> > -    /*
> > -     * If user specifies so, we fool the switch routine to redirect input
> > -     * straight back to Xen. I use this convoluted method so we still print
> > -     * a useful 'how to switch' message.
> > -     */
> > -    if ( opt_conswitch[1] == 'x' )
> > -        console_rx = max_console_rx;
> > -
> >      register_keyhandler('w', conring_dump_keyhandler,
> >                          "synchronously dump console ring buffer (dmesg)", 0);
> >      register_irq_keyhandler('+', &do_inc_thresh,
> > @@ -1186,8 +1158,8 @@ void __init console_endboot(void)
> >      register_irq_keyhandler('G', &do_toggle_guest,
> >                              "toggle host/guest log level adjustment", 0);
> >
> > -    /* Serial input is directed to DOM0 by default. */
> > -    console_switch_input();
> > +    if ( opt_conswitch[1] != 'x' )
> > +        (void)console_set_domid(get_initial_domain_id());
> 
> We should use domid_find_with_input_allowed instead of assuming
> get_initial_domain_id() has input_allowed?
> 
> 
> 
> >  }
> >
> >  int __init console_has(const char *device)
> > diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> > index 8aab05ae93..a88eb34f3f 100644
> > --- a/xen/include/xen/domain.h
> > +++ b/xen/include/xen/domain.h
> > @@ -36,6 +36,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info);
> >  void arch_get_domain_info(const struct domain *d,
> >                            struct xen_domctl_getdomaininfo *info);
> >
> > +domid_t domid_find_with_input_allowed(domid_t hint);
> >  domid_t get_initial_domain_id(void);
> >
> >  domid_t domid_alloc(domid_t domid);
> > --
> > 2.34.1
> >
> >
> 

Re: [PATCH v4 2/4] xen/console: introduce console input permission

[dmkhn]

On Thu, May 29, 2025 at 05:58:00PM -0700, Stefano Stabellini wrote:
> On Thu, 29 May 2025, dmkhn@proton.me wrote:
> > Add new flag to domain structure for marking permission to intercept
> > the physical console input by the domain.
> >
> > Update console input switch logic accordingly.
> >
> > No functional change intended.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> > Changes since v3:
> > - rebased
> > ---
> >  xen/arch/arm/vpl011.c      |  2 ++
> >  xen/arch/x86/pv/shim.c     |  2 ++
> >  xen/common/domain.c        |  2 ++
> >  xen/drivers/char/console.c | 18 +++++++++++++++++-
> >  xen/include/xen/sched.h    |  8 +++++++-
> >  5 files changed, 30 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > index 66047bf33c..147958eee8 100644
> > --- a/xen/arch/arm/vpl011.c
> > +++ b/xen/arch/arm/vpl011.c
> > @@ -737,6 +737,8 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
> >      register_mmio_handler(d, &vpl011_mmio_handler,
> >                            vpl011->base_addr, GUEST_PL011_SIZE, NULL);
> >
> > +    d->console.input_allowed = true;
> 
> This should be set only when backend_in_domain = false.
> 
> 
> >      return 0;
> >
> >  out1:
> > diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
> > index c506cc0bec..bc2a7dd5fa 100644
> > --- a/xen/arch/x86/pv/shim.c
> > +++ b/xen/arch/x86/pv/shim.c
> > @@ -238,6 +238,8 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
> >       * guest from depleting the shim memory pool.
> >       */
> >      d->max_pages = domain_tot_pages(d);
> > +
> > +    d->console.input_allowed = true;
> >  }
> >
> >  static void write_start_info(struct domain *d)
> > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > index 87e5be35e5..9bc66d80c4 100644
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -835,6 +835,8 @@ struct domain *domain_create(domid_t domid,
> >          flags |= CDF_hardware;
> >          if ( old_hwdom )
> >              old_hwdom->cdf &= ~CDF_hardware;
> > +
> > +        d->console.input_allowed = true;
> >      }
> >
> >      /* Holding CDF_* internal flags. */
> > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> > index 30701ae0b0..8a0bcff78f 100644
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -512,9 +512,21 @@ static unsigned int __read_mostly console_rx = 0;
> >
> >  struct domain *console_get_domain(void)
> >  {
> > +    struct domain *d;
> > +
> >      if ( console_rx == 0 )
> >              return NULL;
> > -    return rcu_lock_domain_by_id(console_rx - 1);
> > +
> > +    d = rcu_lock_domain_by_id(console_rx - 1);
> > +    if ( !d )
> > +        return NULL;
> > +
> > +    if ( d->console.input_allowed )
> > +        return d;
> > +
> > +    rcu_unlock_domain(d);
> > +
> > +    return NULL;
> 
> The original idea was to skip over domains that cannot have any input so
> I don't think we should get in this situation. We could even have an
> assert.
> 
> 
> >  }
> >
> >  void console_put_domain(struct domain *d)
> > @@ -551,6 +563,10 @@ static void console_switch_input(void)
> >          if ( d )
> >          {
> >              rcu_unlock_domain(d);
> > +
> > +            if ( !d->console.input_allowed )
> > +                break;
> 
> shouldn't this be continue instead of break?
> 
> 
> >              console_rx = next_rx;
> >              printk("*** Serial input to DOM%u", domid);
> >              break;
> > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> > index 559d201e0c..e91c99a8f3 100644
> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -512,7 +512,7 @@ struct domain
> >      bool             auto_node_affinity;
> >      /* Is this guest fully privileged (aka dom0)? */
> >      bool             is_privileged;
> > -    /* Can this guest access the Xen console? */
> > +    /* XSM: permission to use HYPERCALL_console_io hypercall */
> >      bool             is_console;
> 
> While I am in favor of this direction and we certainly need a better way
> to distinguish domains that can use HYPERCALL_console_io hypercall from
> others, could we simplify this and just assume that "is_console" implies
> input_allowed and also set is_console = true in all the same places you
> are setting input_allowed = true in this patch?
> 
> For clarity, I am suggesting:
> - do not add input_allowed
> - set is_console = true in domain_vpl011_init, pv_shim_setup_dom, etc.
> 
> The only side effect is that we would allow domains with vpl011 to also
> use console hypercalls but I don't think there is any harm in that?
> 
> I don't feel strongly about this, I am just trying to find ways to make
> things simple. I apologize if it was already discussed during review of
> one of the previous versions.

There was feedback on using is_console:

  https://lore.kernel.org/xen-devel/e899f63b-6182-4b53-9fb4-9a821e75648b@suse.com/

AFAIU, since XSM is the existing user of is_console, there should be a new
separate flag to avoid collision with the existing one.

> 
> I am also OK with this approach.
> 
> 
> >      /* Is this guest being debugged by dom0? */
> >      bool             debugger_attached;
> > @@ -651,6 +651,12 @@ struct domain
> >      unsigned int num_llc_colors;
> >      const unsigned int *llc_colors;
> >  #endif
> > +
> > +    /* Console settings. */
> > +    struct {
> > +        /* Permission to take ownership of the physical console input. */
> > +        bool input_allowed;
> > +    } console;
> >  } __aligned(PAGE_SIZE);
> >
> >  static inline struct page_list_head *page_to_list(
> > --
> > 2.34.1
> >
> >
> 

Re: [PATCH v4 2/4] xen/console: introduce console input permission

[Stefano Stabellini]

On Fri, 30 May 2025, dmkhn@proton.me wrote:
> On Thu, May 29, 2025 at 05:58:00PM -0700, Stefano Stabellini wrote:
> > On Thu, 29 May 2025, dmkhn@proton.me wrote:
> > > Add new flag to domain structure for marking permission to intercept
> > > the physical console input by the domain.
> > >
> > > Update console input switch logic accordingly.
> > >
> > > No functional change intended.
> > >
> > > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > > ---
> > > Changes since v3:
> > > - rebased
> > > ---
> > >  xen/arch/arm/vpl011.c      |  2 ++
> > >  xen/arch/x86/pv/shim.c     |  2 ++
> > >  xen/common/domain.c        |  2 ++
> > >  xen/drivers/char/console.c | 18 +++++++++++++++++-
> > >  xen/include/xen/sched.h    |  8 +++++++-
> > >  5 files changed, 30 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > > index 66047bf33c..147958eee8 100644
> > > --- a/xen/arch/arm/vpl011.c
> > > +++ b/xen/arch/arm/vpl011.c
> > > @@ -737,6 +737,8 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
> > >      register_mmio_handler(d, &vpl011_mmio_handler,
> > >                            vpl011->base_addr, GUEST_PL011_SIZE, NULL);
> > >
> > > +    d->console.input_allowed = true;
> > 
> > This should be set only when backend_in_domain = false.
> > 
> > 
> > >      return 0;
> > >
> > >  out1:
> > > diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
> > > index c506cc0bec..bc2a7dd5fa 100644
> > > --- a/xen/arch/x86/pv/shim.c
> > > +++ b/xen/arch/x86/pv/shim.c
> > > @@ -238,6 +238,8 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
> > >       * guest from depleting the shim memory pool.
> > >       */
> > >      d->max_pages = domain_tot_pages(d);
> > > +
> > > +    d->console.input_allowed = true;
> > >  }
> > >
> > >  static void write_start_info(struct domain *d)
> > > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > > index 87e5be35e5..9bc66d80c4 100644
> > > --- a/xen/common/domain.c
> > > +++ b/xen/common/domain.c
> > > @@ -835,6 +835,8 @@ struct domain *domain_create(domid_t domid,
> > >          flags |= CDF_hardware;
> > >          if ( old_hwdom )
> > >              old_hwdom->cdf &= ~CDF_hardware;
> > > +
> > > +        d->console.input_allowed = true;
> > >      }
> > >
> > >      /* Holding CDF_* internal flags. */
> > > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> > > index 30701ae0b0..8a0bcff78f 100644
> > > --- a/xen/drivers/char/console.c
> > > +++ b/xen/drivers/char/console.c
> > > @@ -512,9 +512,21 @@ static unsigned int __read_mostly console_rx = 0;
> > >
> > >  struct domain *console_get_domain(void)
> > >  {
> > > +    struct domain *d;
> > > +
> > >      if ( console_rx == 0 )
> > >              return NULL;
> > > -    return rcu_lock_domain_by_id(console_rx - 1);
> > > +
> > > +    d = rcu_lock_domain_by_id(console_rx - 1);
> > > +    if ( !d )
> > > +        return NULL;
> > > +
> > > +    if ( d->console.input_allowed )
> > > +        return d;
> > > +
> > > +    rcu_unlock_domain(d);
> > > +
> > > +    return NULL;
> > 
> > The original idea was to skip over domains that cannot have any input so
> > I don't think we should get in this situation. We could even have an
> > assert.
> > 
> > 
> > >  }
> > >
> > >  void console_put_domain(struct domain *d)
> > > @@ -551,6 +563,10 @@ static void console_switch_input(void)
> > >          if ( d )
> > >          {
> > >              rcu_unlock_domain(d);
> > > +
> > > +            if ( !d->console.input_allowed )
> > > +                break;
> > 
> > shouldn't this be continue instead of break?
> > 
> > 
> > >              console_rx = next_rx;
> > >              printk("*** Serial input to DOM%u", domid);
> > >              break;
> > > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> > > index 559d201e0c..e91c99a8f3 100644
> > > --- a/xen/include/xen/sched.h
> > > +++ b/xen/include/xen/sched.h
> > > @@ -512,7 +512,7 @@ struct domain
> > >      bool             auto_node_affinity;
> > >      /* Is this guest fully privileged (aka dom0)? */
> > >      bool             is_privileged;
> > > -    /* Can this guest access the Xen console? */
> > > +    /* XSM: permission to use HYPERCALL_console_io hypercall */
> > >      bool             is_console;
> > 
> > While I am in favor of this direction and we certainly need a better way
> > to distinguish domains that can use HYPERCALL_console_io hypercall from
> > others, could we simplify this and just assume that "is_console" implies
> > input_allowed and also set is_console = true in all the same places you
> > are setting input_allowed = true in this patch?
> > 
> > For clarity, I am suggesting:
> > - do not add input_allowed
> > - set is_console = true in domain_vpl011_init, pv_shim_setup_dom, etc.
> > 
> > The only side effect is that we would allow domains with vpl011 to also
> > use console hypercalls but I don't think there is any harm in that?
> > 
> > I don't feel strongly about this, I am just trying to find ways to make
> > things simple. I apologize if it was already discussed during review of
> > one of the previous versions.
> 
> There was feedback on using is_console:
> 
>   https://lore.kernel.org/xen-devel/e899f63b-6182-4b53-9fb4-9a821e75648b@suse.com/
> 
> AFAIU, since XSM is the existing user of is_console, there should be a new
> separate flag to avoid collision with the existing one.

OK, I suspected as much. In that case, it is fine to continue with the
new flag.

Re: [PATCH v4 3/4] xen/console: remove max_init_domid dependency

[dmkhn]

On Thu, May 29, 2025 at 05:58:20PM -0700, Stefano Stabellini wrote:
> On Thu, 29 May 2025, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmkhn@proton.me>
> >
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > The physical console input rotation depends on max_init_domid symbol, which is
> > managed differently across architectures.
> >
> > Instead of trying to manage max_init_domid in the arch-common code the console
> > input rotation code can be reworked by removing dependency on max_init_domid
> > entirely.
> >
> > To do that, introduce domid_find_with_input_allowed() in arch-independent
> > location to find the ID of the next possible console owner domain. The IDs
> > are rotated across non-system domain IDs and DOMID_XEN.
> >
> > Also, introduce helper console_set_domid() for updating identifier of the
> > current console input owner (points to Xen or domain).
> >
> > Use domid_find_with_input_allowed() and console_set_domid() in
> > console_switch_input().
> >
> > Remove uses of max_init_domid in the code.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> > Changes since v3:
> > - switched to RCU lock in domid_find_with_input_allowed()
> > ---
> >  xen/arch/arm/include/asm/setup.h        |  2 -
> >  xen/arch/arm/setup.c                    |  2 -
> >  xen/arch/ppc/include/asm/setup.h        |  2 -
> >  xen/arch/riscv/include/asm/setup.h      |  2 -
> >  xen/arch/x86/include/asm/setup.h        |  2 -
> >  xen/common/device-tree/dom0less-build.c |  2 -
> >  xen/common/domain.c                     | 29 ++++++++
> >  xen/drivers/char/console.c              | 90 +++++++++----------------
> >  xen/include/xen/domain.h                |  1 +
> >  9 files changed, 61 insertions(+), 71 deletions(-)
> >
> > diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
> > index 6cf272c160..f107e8eebb 100644
> > --- a/xen/arch/arm/include/asm/setup.h
> > +++ b/xen/arch/arm/include/asm/setup.h
> > @@ -25,8 +25,6 @@ struct map_range_data
> >      struct rangeset *irq_ranges;
> >  };
> >
> > -extern domid_t max_init_domid;
> > -
> >  void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
> >
> >  size_t estimate_efi_size(unsigned int mem_nr_banks);
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 10b46d0684..53e2f8b537 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -61,8 +61,6 @@ struct cpuinfo_arm __read_mostly system_cpuinfo;
> >  bool __read_mostly acpi_disabled;
> >  #endif
> >
> > -domid_t __read_mostly max_init_domid;
> > -
> >  static __used void init_done(void)
> >  {
> >      int rc;
> > diff --git a/xen/arch/ppc/include/asm/setup.h b/xen/arch/ppc/include/asm/setup.h
> > index e4f64879b6..956fa6985a 100644
> > --- a/xen/arch/ppc/include/asm/setup.h
> > +++ b/xen/arch/ppc/include/asm/setup.h
> > @@ -1,6 +1,4 @@
> >  #ifndef __ASM_PPC_SETUP_H__
> >  #define __ASM_PPC_SETUP_H__
> >
> > -#define max_init_domid (0)
> > -
> >  #endif /* __ASM_PPC_SETUP_H__ */
> > diff --git a/xen/arch/riscv/include/asm/setup.h b/xen/arch/riscv/include/asm/setup.h
> > index c9d69cdf51..d1fc64b673 100644
> > --- a/xen/arch/riscv/include/asm/setup.h
> > +++ b/xen/arch/riscv/include/asm/setup.h
> > @@ -5,8 +5,6 @@
> >
> >  #include <xen/types.h>
> >
> > -#define max_init_domid (0)
> > -
> >  void setup_mm(void);
> >
> >  void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
> > diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
> > index ac34c69855..b67de8577f 100644
> > --- a/xen/arch/x86/include/asm/setup.h
> > +++ b/xen/arch/x86/include/asm/setup.h
> > @@ -69,6 +69,4 @@ extern bool opt_dom0_verbose;
> >  extern bool opt_dom0_cpuid_faulting;
> >  extern bool opt_dom0_msr_relaxed;
> >
> > -#define max_init_domid (0)
> > -
> >  #endif
> > diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
> > index 9a6015f4ce..703f20faed 100644
> > --- a/xen/common/device-tree/dom0less-build.c
> > +++ b/xen/common/device-tree/dom0less-build.c
> > @@ -977,8 +977,6 @@ void __init create_domUs(void)
> >          domid = domid_alloc(DOMID_INVALID);
> >          if ( domid == DOMID_INVALID )
> >              panic("Error allocating ID for domain %s\n", dt_node_name(node));
> > -        if ( max_init_domid < domid )
> > -            max_init_domid = domid;
> >
> >          d = domain_create(domid, &d_cfg, flags);
> >          if ( IS_ERR(d) )
> > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > index 9bc66d80c4..704e0907e9 100644
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -2463,6 +2463,35 @@ void domid_free(domid_t domid)
> >      spin_unlock(&domid_lock);
> >  }
> >
> > +/*
> > + * Find the ID of the next possible console owner domain.
> > + *
> > + * @return Domain ID: DOMID_XEN or non-system domain IDs within
> > + * the range of [0..DOMID_FIRST_RESERVED-1].
> > + */
> > +domid_t domid_find_with_input_allowed(domid_t hint)
> > +{
> > +    const struct domain *d;
> > +    domid_t domid = DOMID_XEN;
> > +
> > +    rcu_read_lock(&domlist_read_lock);
> > +
> > +    for ( d = rcu_dereference(domain_list);
> > +          d && d->domain_id < DOMID_FIRST_RESERVED;
> > +          d = rcu_dereference(d->next_in_list) )
> > +    {
> > +        if ( d->console.input_allowed )
> > +        {
> > +            domid = d->domain_id;
> > +            break;
> > +        }
> > +    }
> > +
> > +    rcu_read_unlock(&domlist_read_lock);
> 
> Doesn't this always return the first domid with input_allowed given that
> hint is not used? It looks like it wouldn't work right...
> 
> 
> > +    return domid;
> > +}
> > +
> >  /*
> >   * Local variables:
> >   * mode: C
> > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> > index 8a0bcff78f..37289d5558 100644
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -498,26 +498,17 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
> >
> >  /*
> >   * CTRL-<switch_char> changes input direction, rotating among Xen, Dom0,
> > - * and the DomUs started from Xen at boot.
> > + * and the DomUs.
> >   */
> >  #define switch_code (opt_conswitch[0]-'a'+1)
> > -/*
> > - * console_rx=0 => input to xen
> > - * console_rx=1 => input to dom0 (or the sole shim domain)
> > - * console_rx=N => input to dom(N-1)
> > - */
> > -static unsigned int __read_mostly console_rx = 0;
> >
> > -#define max_console_rx (max_init_domid + 1)
> > +/* Console owner domain identifier. */
> > +static domid_t __read_mostly console_rx = DOMID_XEN;
> >
> >  struct domain *console_get_domain(void)
> >  {
> > -    struct domain *d;
> > +    struct domain *d = rcu_lock_domain_by_id(console_rx);
> >
> > -    if ( console_rx == 0 )
> > -            return NULL;
> > -
> > -    d = rcu_lock_domain_by_id(console_rx - 1);
> >      if ( !d )
> >          return NULL;
> >
> > @@ -535,43 +526,14 @@ void console_put_domain(struct domain *d)
> >          rcu_unlock_domain(d);
> >  }
> >
> > -static void console_switch_input(void)
> > +static void console_set_domid(domid_t domid)
> >  {
> > -    unsigned int next_rx = console_rx;
> > +    if ( domid == DOMID_XEN )
> > +        printk("*** Serial input to Xen");
> > +    else
> > +        printk("*** Serial input to DOM%u", domid);
> >
> > -    /*
> > -     * Rotate among Xen, dom0 and boot-time created domUs while skipping
> > -     * switching serial input to non existing domains.
> > -     */
> > -    for ( ; ; )
> > -    {
> > -        domid_t domid;
> > -        struct domain *d;
> > -
> > -        if ( next_rx++ >= max_console_rx )
> > -        {
> > -            console_rx = 0;
> > -            printk("*** Serial input to Xen");
> > -            break;
> > -        }
> > -
> > -        if ( consoled_is_enabled() && next_rx == 1 )
> > -            domid = get_initial_domain_id();
> > -        else
> > -            domid = next_rx - 1;
> > -        d = rcu_lock_domain_by_id(domid);
> > -        if ( d )
> > -        {
> > -            rcu_unlock_domain(d);
> > -
> > -            if ( !d->console.input_allowed )
> > -                break;
> > -
> > -            console_rx = next_rx;
> > -            printk("*** Serial input to DOM%u", domid);
> > -            break;
> > -        }
> > -    }
> > +    console_rx = domid;
> >
> >      if ( switch_code )
> >          printk(" (type 'CTRL-%c' three times to switch input)",
> > @@ -579,12 +541,30 @@ static void console_switch_input(void)
> >      printk("\n");
> >  }
> >
> > +/*
> > + * Switch console focus.
> > + * Rotates input focus among Xen and domains with console input permission.
> > + */
> > +static void console_switch_input(void)
> > +{
> > +    domid_t hint;
> > +
> > +    if ( console_rx == DOMID_XEN )
> > +        hint = get_initial_domain_id();
> > +    else
> > +        hint = console_rx + 1;
> > +
> > +    hint = domid_find_with_input_allowed(hint);
> > +
> > +    console_set_domid(hint);
> > +}
> > +
> >  static void __serial_rx(char c)
> >  {
> >      struct domain *d;
> >      int rc = 0;
> >
> > -    if ( console_rx == 0 )
> > +    if ( console_rx == DOMID_XEN )
> >          return handle_keypress(c, false);
> >
> >      d = console_get_domain();
> > @@ -1169,14 +1149,6 @@ void __init console_endboot(void)
> >
> >      video_endboot();
> >
> > -    /*
> > -     * If user specifies so, we fool the switch routine to redirect input
> > -     * straight back to Xen. I use this convoluted method so we still print
> > -     * a useful 'how to switch' message.
> > -     */
> > -    if ( opt_conswitch[1] == 'x' )
> > -        console_rx = max_console_rx;
> > -
> >      register_keyhandler('w', conring_dump_keyhandler,
> >                          "synchronously dump console ring buffer (dmesg)", 0);
> >      register_irq_keyhandler('+', &do_inc_thresh,
> > @@ -1186,8 +1158,8 @@ void __init console_endboot(void)
> >      register_irq_keyhandler('G', &do_toggle_guest,
> >                              "toggle host/guest log level adjustment", 0);
> >
> > -    /* Serial input is directed to DOM0 by default. */
> > -    console_switch_input();
> > +    if ( opt_conswitch[1] != 'x' )
> > +        (void)console_set_domid(get_initial_domain_id());
> 
> We should use domid_find_with_input_allowed instead of assuming
> get_initial_domain_id() has input_allowed?

AFAIU, get_initial_domain_id() is not necessarily created by the time this code
is reached. In this case, relying on domid_find_with_input_allowed() will keep
console focus in Xen, which will be a behavior change.

I kept this hunk as is in v5.

> 
> 
> 
> >  }
> >
> >  int __init console_has(const char *device)
> > diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> > index 8aab05ae93..a88eb34f3f 100644
> > --- a/xen/include/xen/domain.h
> > +++ b/xen/include/xen/domain.h
> > @@ -36,6 +36,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info);
> >  void arch_get_domain_info(const struct domain *d,
> >                            struct xen_domctl_getdomaininfo *info);
> >
> > +domid_t domid_find_with_input_allowed(domid_t hint);
> >  domid_t get_initial_domain_id(void);
> >
> >  domid_t domid_alloc(domid_t domid);
> > --
> > 2.34.1
> >
> >