[PATCH v1 0/8] xen/console: cleanup console input switch logic

[PATCH v1 0/8] xen/console: cleanup console input switch logic

[dmkhn]

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 patch series introduces the concept of "console focus", which is
defined as the ID of the domain that currently owns the physical console
input.
  
The patch series originates from the NS16550 UART emulator series [1]
for x86, which requires ability to switch physical console input to a
PVH/HVM domain with an emulated UART.
 
The main idea 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 0 removes all the trailing white spaces in the console driver code.
 
Patch 1 introduces a new domain permission flag to mark ownership of the
console input for the console driver.   
 
Patches 2-4 prepare console driver to allow console input rotation
across multiple domains based on the new permission flag.
 
Patches 5-6 perform mechanical renames to fit the usage in the code.
 
Patch 7 cleans up the console input switch logic.
 
Patch 8 simplifies the existing vUART code by using newly introduced APIs.

CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1723361248

[1]: https://lore.kernel.org/xen-devel/20250103-vuart-ns8250-v3-v1-0-c5d36b31d66c@ford.com/

Denis Mukhin (8):
  xen/console: fix trailing whitespaces
  xen/console: introduce console input permission
  xen/domain: introduce domid_top
  xen/domain: introduce domid_alloc()
  xen/console: rename switch_serial_input() to console_switch_focus()
  xen/console: rename console_rx to console_focus
  xen/console: introduce console_set_focus()
  xen/console: introduce console_get_focus()

 xen/arch/arm/dom0less-build.c      |  15 ++-
 xen/arch/arm/domain_build.c        |  19 +++-
 xen/arch/arm/include/asm/setup.h   |   2 -
 xen/arch/arm/setup.c               |   2 -
 xen/arch/arm/vpl011.c              |   7 +-
 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/domain.c                |   5 +
 xen/common/domctl.c                |  71 +++++++++------
 xen/common/kernel.c                |   8 ++
 xen/drivers/char/console.c         | 142 +++++++++++++++++------------
 xen/include/xen/console.h          |   3 +-
 xen/include/xen/domain.h           |   5 +
 xen/include/xen/sched.h            |   8 +-
 16 files changed, 180 insertions(+), 115 deletions(-)

-- 
2.34.1

[PATCH v1 1/8] xen/console: fix trailing whitespaces

[dmkhn]

Remove trailing whitespaces in the console driver.

No functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 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 ba428199d2..c3150fbdb7 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1,8 +1,8 @@
 /******************************************************************************
  * console.c
- * 
+ *
  * Emergency console I/O for Xen and the domain-0 guest OS.
- * 
+ *
  * Copyright (c) 2002-2004, K A Fraser.
  *
  * Added printf_ratelimit
@@ -811,7 +811,7 @@ static int printk_prefix_check(char *p, char **pp)
     return ((atomic_read(&print_everything) != 0) ||
             (loglvl < lower_thresh) ||
             ((loglvl < upper_thresh) && printk_ratelimit()));
-} 
+}
 
 static int cf_check parse_console_timestamps(const char *s)
 {
-- 
2.34.1

[PATCH v1 2/8] xen/console: introduce console input permission

[dmkhn]

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

Update console input switch logic accordingly.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/arm/vpl011.c      |  2 ++
 xen/arch/x86/pv/shim.c     |  2 ++
 xen/common/domain.c        |  2 ++
 xen/drivers/char/console.c | 49 +++++++++++++++++++++++++++++++++-----
 xen/include/xen/sched.h    |  8 ++++++-
 5 files changed, 56 insertions(+), 7 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 585fd726a9..b9f549c617 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -827,6 +827,8 @@ struct domain *domain_create(domid_t domid,
 
         old_hwdom = hardware_domain;
         hardware_domain = d;
+
+        d->console.input_allowed = true;
     }
 
     TRACE_TIME(TRC_DOM0_DOM_ADD, d->domain_id);
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index c3150fbdb7..d7d9800095 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -474,11 +474,26 @@ static unsigned int __read_mostly console_rx = 0;
 
 #define max_console_rx (max_init_domid + 1)
 
+static struct domain *console_get_domain_by_id(domid_t domid)
+{
+    struct domain *d = rcu_lock_domain_by_id(domid);
+
+    if ( !d )
+        return NULL;
+
+    if ( d->console.input_allowed )
+        return d;
+
+    rcu_unlock_domain(d);
+
+    return NULL;
+}
+
 struct domain *console_get_domain(void)
 {
     if ( console_rx == 0 )
             return NULL;
-    return rcu_lock_domain_by_id(console_rx - 1);
+    return console_get_domain_by_id(console_rx - 1);
 }
 
 void console_put_domain(struct domain *d)
@@ -487,6 +502,15 @@ void console_put_domain(struct domain *d)
         rcu_unlock_domain(d);
 }
 
+static bool console_check_focus_by_id(domid_t domid)
+{
+    struct domain *d = console_get_domain_by_id(domid);
+
+    console_put_domain(d);
+
+    return !!d;
+}
+
 static void switch_serial_input(void)
 {
     unsigned int next_rx = console_rx;
@@ -498,7 +522,6 @@ static void switch_serial_input(void)
     for ( ; ; )
     {
         domid_t domid;
-        struct domain *d;
 
         if ( next_rx++ >= max_console_rx )
         {
@@ -511,10 +534,9 @@ static void switch_serial_input(void)
             domid = get_initial_domain_id();
         else
             domid = next_rx - 1;
-        d = rcu_lock_domain_by_id(domid);
-        if ( d )
+
+        if ( console_check_focus_by_id(domid) )
         {
-            rcu_unlock_domain(d);
             console_rx = next_rx;
             printk("*** Serial input to DOM%u", domid);
             break;
@@ -564,10 +586,25 @@ static void __serial_rx(char c)
         /* Deliver input to the PV shim console. */
         rc = consoled_guest_tx(c);
 
-    if ( rc )
+    switch ( rc )
+    {
+    case 0:
+        break;
+
+    case -EBUSY:    /* Loopback mode */
+    case -ENOSPC:   /* FIFO is full */
         guest_printk(d,
                      XENLOG_WARNING "failed to process console input: %d\n",
                      rc);
+        break;
+
+    default:
+        d->console.input_allowed = false;
+        guest_printk(d,
+                     XENLOG_ERR "disabled console input: %d\n",
+                     rc);
+        break;
+    }
 
     console_put_domain(d);
 }
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 559d201e0c..292b1a91f9 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 own physical console input. */
+        bool input_allowed;
+    } console;
 } __aligned(PAGE_SIZE);
 
 static inline struct page_list_head *page_to_list(
-- 
2.34.1

[PATCH v1 3/8] xen/domain: introduce domid_top

[dmkhn]

Rename max_init_domid to domid_top to align with its usage in the code
(Arm), where it represents the upper boundary of the non-system domain
ID range.

Relocate the domid_top declaration to an architecture-independent
location.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/arm/dom0less-build.c      | 4 ++--
 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/domain.c                | 3 +++
 xen/drivers/char/console.c         | 2 +-
 xen/include/xen/domain.h           | 2 ++
 9 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 573b0d25ae..d7d7665c0a 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -981,7 +981,7 @@ void __init create_domUs(void)
         if ( !dt_device_is_compatible(node, "xen,domain") )
             continue;
 
-        if ( (max_init_domid + 1) >= DOMID_FIRST_RESERVED )
+        if ( (domid_top + 1) >= DOMID_FIRST_RESERVED )
             panic("No more domain IDs available\n");
 
         if ( dt_find_property(node, "xen,static-mem", NULL) )
@@ -1112,7 +1112,7 @@ void __init create_domUs(void)
          * very important to use the pre-increment operator to call
          * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
          */
-        d = domain_create(++max_init_domid, &d_cfg, flags);
+        d = domain_create(++domid_top, &d_cfg, flags);
         if ( IS_ERR(d) )
             panic("Error creating domain %s (rc = %ld)\n",
                   dt_node_name(node), PTR_ERR(d));
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 ffcae900d7..ab60f0d189 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -60,8 +60,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 5c2391a868..296348655b 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/domain.c b/xen/common/domain.c
index b9f549c617..dac910d454 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -66,6 +66,9 @@ DEFINE_RCU_READ_LOCK(domlist_read_lock);
 static struct domain *domain_hash[DOMAIN_HASH_SIZE];
 struct domain *domain_list;
 
+/* Highest known non-system domain ID. */
+domid_t domid_top;
+
 /*
  * Insert a domain into the domlist/hash.  This allows the domain to be looked
  * up by domid, and therefore to be the subject of hypercalls/etc.
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index d7d9800095..d04a5335ce 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -472,7 +472,7 @@ static void cf_check dump_console_ring_key(unsigned char key)
  */
 static unsigned int __read_mostly console_rx = 0;
 
-#define max_console_rx (max_init_domid + 1)
+#define max_console_rx (domid_top + 1)
 
 static struct domain *console_get_domain_by_id(domid_t domid)
 {
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 83069de501..b7425827b8 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -35,6 +35,8 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info);
 void arch_get_domain_info(const struct domain *d,
                           struct xen_domctl_getdomaininfo *info);
 
+extern domid_t domid_top;
+
 domid_t get_initial_domain_id(void);
 
 /* CDF_* constant. Internal flags for domain creation. */
-- 
2.34.1

[PATCH v1 4/8] xen/domain: introduce domid_alloc()

[dmkhn]

Move domain ID allocation to a dedicated function domid_alloc() and
use it during domain creation.

Update domid_top within domid_alloc() to reflect the highest known
domain ID.

Initialize domid_top using the result of get_initial_domain_id().

Allocation algorithm:
- If an explicit domain ID is provided, verify its availability and
  use it if free;
- Otherwise, perform an exhaustive search for the first available ID
  within the [0..DOMID_FIRST_RESERVED) range, excluding hardware_domid.

The change is a prerequisite for enabling console input rotation
across domains on x86 (currently, limited to dom0, PV shim and Xen).

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/arm/dom0less-build.c | 15 ++++----
 xen/arch/arm/domain_build.c   | 19 +++++++---
 xen/common/domctl.c           | 71 ++++++++++++++++++++++-------------
 xen/common/kernel.c           |  8 ++++
 xen/include/xen/domain.h      |  3 ++
 5 files changed, 76 insertions(+), 40 deletions(-)

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index d7d7665c0a..4b9e22039e 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -975,14 +975,18 @@ void __init create_domUs(void)
             .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
         };
         unsigned int flags = 0U;
+        domid_t domid;
         uint32_t val;
         int rc;
 
         if ( !dt_device_is_compatible(node, "xen,domain") )
             continue;
 
-        if ( (domid_top + 1) >= DOMID_FIRST_RESERVED )
-            panic("No more domain IDs available\n");
+        rc = domid_alloc(DOMID_AUTO);
+        if ( rc < 0 )
+            panic("cannot allocate domain ID for domain %s (rc = %d)\n",
+                  dt_node_name(node), rc);
+        domid = rc;
 
         if ( dt_find_property(node, "xen,static-mem", NULL) )
         {
@@ -1107,12 +1111,7 @@ void __init create_domUs(void)
         if ( !llc_coloring_enabled && llc_colors_str )
             panic("'llc-colors' found, but LLC coloring is disabled\n");
 
-        /*
-         * The variable max_init_domid is initialized with zero, so here it's
-         * very important to use the pre-increment operator to call
-         * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
-         */
-        d = domain_create(++domid_top, &d_cfg, flags);
+        d = domain_create(domid, &d_cfg, flags);
         if ( IS_ERR(d) )
             panic("Error creating domain %s (rc = %ld)\n",
                   dt_node_name(node), PTR_ERR(d));
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2b5b433183..2d8c2931d6 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2367,8 +2367,15 @@ void __init create_dom0(void)
         .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
     };
     unsigned int flags = CDF_privileged;
+    domid_t domid;
     int rc;
 
+    rc = domid_alloc(get_initial_domain_id());
+    if ( rc < 0 )
+        panic("Cannot use domain ID %d (rc = %d)\n",
+              get_initial_domain_id(), rc);
+    domid = rc;
+
     /* The vGIC for DOM0 is exactly emulating the hardware GIC */
     dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
     dom0_cfg.arch.nr_spis = VGIC_DEF_NR_SPIS;
@@ -2391,19 +2398,21 @@ void __init create_dom0(void)
     if ( !llc_coloring_enabled )
         flags |= CDF_directmap;
 
-    dom0 = domain_create(0, &dom0_cfg, flags);
+    dom0 = domain_create(domid, &dom0_cfg, flags);
     if ( IS_ERR(dom0) )
-        panic("Error creating domain 0 (rc = %ld)\n", PTR_ERR(dom0));
+        panic("Error creating domain %d (rc = %ld)\n", domid, PTR_ERR(dom0));
 
     if ( llc_coloring_enabled && (rc = dom0_set_llc_colors(dom0)) )
-        panic("Error initializing LLC coloring for domain 0 (rc = %d)\n", rc);
+        panic("Error initializing LLC coloring for domain %d (rc = %d)\n",
+              domid, rc);
 
     if ( alloc_dom0_vcpu0(dom0) == NULL )
-        panic("Error creating domain 0 vcpu0\n");
+        panic("Error creating domain %d vcpu0\n", domid);
 
     rc = construct_dom0(dom0);
     if ( rc )
-        panic("Could not set up DOM0 guest OS (rc = %d)\n", rc);
+        panic("Could not set up guest OS for domain %d (rc = %d)\n",
+              domid, rc);
 }
 
 /*
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index bfe2e1f9f0..9964aa000a 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -63,6 +63,46 @@ static inline int is_free_domid(domid_t dom)
     return 0;
 }
 
+/*
+ * Allocate new domain ID based on the hint.
+ *
+ * If hint is outside of valid [0..DOMID_FIRST_RESERVED] range of IDs,
+ * perform an exhaustive search of the first free domain ID excluding
+ * hardware_domid.
+ */
+int domid_alloc(int hint)
+{
+    domid_t domid;
+
+    if ( hint >= 0 && hint < DOMID_FIRST_RESERVED )
+    {
+        if ( !is_free_domid(hint) )
+            return -EEXIST;
+
+        domid = hint;
+    }
+    else
+    {
+        for ( domid = domid_top + 1; domid != domid_top; domid++ )
+        {
+            if ( domid == DOMID_FIRST_RESERVED )
+                domid = 0;
+            if ( domid == hardware_domid )
+                continue;
+            if ( is_free_domid(domid) )
+                break;
+        }
+
+        if ( domid == domid_top )
+            return -ENOMEM;
+    }
+
+    if ( domid_top < domid )
+        domid_top = domid;
+
+    return domid;
+}
+
 void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
 {
     struct vcpu *v;
@@ -421,34 +461,11 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 
     case XEN_DOMCTL_createdomain:
     {
-        domid_t        dom;
-        static domid_t rover = 0;
+        ret = domid_alloc(op->domain);
+        if ( ret < 0 )
+            break;
 
-        dom = op->domain;
-        if ( (dom > 0) && (dom < DOMID_FIRST_RESERVED) )
-        {
-            ret = -EEXIST;
-            if ( !is_free_domid(dom) )
-                break;
-        }
-        else
-        {
-            for ( dom = rover + 1; dom != rover; dom++ )
-            {
-                if ( dom == DOMID_FIRST_RESERVED )
-                    dom = 1;
-                if ( is_free_domid(dom) )
-                    break;
-            }
-
-            ret = -ENOMEM;
-            if ( dom == rover )
-                break;
-
-            rover = dom;
-        }
-
-        d = domain_create(dom, &op->u.createdomain, false);
+        d = domain_create(ret, &op->u.createdomain, false);
         if ( IS_ERR(d) )
         {
             ret = PTR_ERR(d);
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 8b63ca55f1..50c44b986e 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -568,6 +568,14 @@ static long xenver_varbuf_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     return sz;
 }
 
+static int __init cf_check globals_init(void)
+{
+    domid_top = get_initial_domain_id();
+
+    return 0;
+}
+__initcall(globals_init);
+
 long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     bool deny = xsm_xen_version(XSM_OTHER, cmd);
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index b7425827b8..c91b9704de 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -39,6 +39,9 @@ extern domid_t domid_top;
 
 domid_t get_initial_domain_id(void);
 
+#define DOMID_AUTO               (-1)
+int domid_alloc(int hint);
+
 /* CDF_* constant. Internal flags for domain creation. */
 /* Is this a privileged domain? */
 #define CDF_privileged           (1U << 0)
-- 
2.34.1

[PATCH v1 5/8] xen/console: rename switch_serial_input() to console_switch_focus()

[dmkhn]

Update the name to emphasize the physical console input switch to a
new owner domain following the naming notation in the console driver.

No functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 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 d04a5335ce..5910ff701c 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -511,7 +511,7 @@ static bool console_check_focus_by_id(domid_t domid)
     return !!d;
 }
 
-static void switch_serial_input(void)
+static void console_switch_focus(void)
 {
     unsigned int next_rx = console_rx;
 
@@ -618,7 +618,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_focus();
             switch_code_count = 0;
         }
         return;
@@ -1162,7 +1162,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_focus();
 }
 
 int __init console_has(const char *device)
-- 
2.34.1

[PATCH v1 6/8] xen/console: rename console_rx to console_focus

[dmkhn]

Update the symbol name in preparation for the semantic change
to the physical console input owner domain identifier.

No functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/drivers/char/console.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 5910ff701c..3d538510f4 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -465,12 +465,9 @@ static void cf_check dump_console_ring_key(unsigned char key)
  * and the DomUs started from Xen at boot.
  */
 #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;
+
+/* Console owner domain identifier. */
+static unsigned int __read_mostly console_focus = 0;
 
 #define max_console_rx (domid_top + 1)
 
@@ -491,9 +488,9 @@ static struct domain *console_get_domain_by_id(domid_t domid)
 
 struct domain *console_get_domain(void)
 {
-    if ( console_rx == 0 )
+    if ( console_focus == 0 )
             return NULL;
-    return console_get_domain_by_id(console_rx - 1);
+    return console_get_domain_by_id(console_focus - 1);
 }
 
 void console_put_domain(struct domain *d)
@@ -513,7 +510,7 @@ static bool console_check_focus_by_id(domid_t domid)
 
 static void console_switch_focus(void)
 {
-    unsigned int next_rx = console_rx;
+    unsigned int next_rx = console_focus;
 
     /*
      * Rotate among Xen, dom0 and boot-time created domUs while skipping
@@ -525,7 +522,7 @@ static void console_switch_focus(void)
 
         if ( next_rx++ >= max_console_rx )
         {
-            console_rx = 0;
+            console_focus = 0;
             printk("*** Serial input to Xen");
             break;
         }
@@ -537,7 +534,7 @@ static void console_switch_focus(void)
 
         if ( console_check_focus_by_id(domid) )
         {
-            console_rx = next_rx;
+            console_focus = next_rx;
             printk("*** Serial input to DOM%u", domid);
             break;
         }
@@ -554,7 +551,7 @@ static void __serial_rx(char c)
     struct domain *d;
     int rc = 0;
 
-    if ( console_rx == 0 )
+    if ( console_focus == 0 )
         return handle_keypress(c, false);
 
     d = console_get_domain();
@@ -1150,7 +1147,7 @@ void __init console_endboot(void)
      * a useful 'how to switch' message.
      */
     if ( opt_conswitch[1] == 'x' )
-        console_rx = max_console_rx;
+        console_focus = max_console_rx;
 
     register_keyhandler('w', dump_console_ring_key,
                         "synchronously dump console ring buffer (dmesg)", 0);
-- 
2.34.1

[PATCH v1 7/8] xen/console: introduce console_set_focus()

[dmkhn]

Switch console_focus address space from integers mapped to domain IDs to
direct domain IDs, simplifying the console input switching code.

Introduce console_set_focus() to set the console owner domain identifier.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/drivers/char/console.c | 81 ++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 47 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 3d538510f4..0e2349a868 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -462,14 +462,12 @@ static void cf_check dump_console_ring_key(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 owner domain identifier. */
-static unsigned int __read_mostly console_focus = 0;
-
-#define max_console_rx (domid_top + 1)
+static domid_t __read_mostly console_focus = DOMID_XEN;
 
 static struct domain *console_get_domain_by_id(domid_t domid)
 {
@@ -488,9 +486,7 @@ static struct domain *console_get_domain_by_id(domid_t domid)
 
 struct domain *console_get_domain(void)
 {
-    if ( console_focus == 0 )
-            return NULL;
-    return console_get_domain_by_id(console_focus - 1);
+    return console_get_domain_by_id(console_focus);
 }
 
 void console_put_domain(struct domain *d)
@@ -508,42 +504,41 @@ static bool console_check_focus_by_id(domid_t domid)
     return !!d;
 }
 
-static void console_switch_focus(void)
+static int console_set_focus(domid_t domid)
 {
-    unsigned int next_rx = console_focus;
+    if ( domid == DOMID_XEN )
+        printk("*** Serial input to Xen");
+    else if ( console_check_focus_by_id(domid) )
+        printk("*** Serial input to DOM%u", domid);
+    else
+        return -ENOENT;
 
-    /*
-     * Rotate among Xen, dom0 and boot-time created domUs while skipping
-     * switching serial input to non existing domains.
-     */
-    for ( ; ; )
-    {
-        domid_t domid;
-
-        if ( next_rx++ >= max_console_rx )
-        {
-            console_focus = 0;
-            printk("*** Serial input to Xen");
-            break;
-        }
-
-        if ( consoled_is_enabled() && next_rx == 1 )
-            domid = get_initial_domain_id();
-        else
-            domid = next_rx - 1;
-
-        if ( console_check_focus_by_id(domid) )
-        {
-            console_focus = next_rx;
-            printk("*** Serial input to DOM%u", domid);
-            break;
-        }
-    }
+    console_focus = domid;
 
     if ( switch_code )
         printk(" (type 'CTRL-%c' three times to switch input)",
                opt_conswitch[0]);
     printk("\n");
+
+    return 0;
+}
+
+/*
+ * Switch console focus.
+ * Rotates input focus among Xen, dom0 and boot-time created domUs while
+ * skipping switching serial input to non existing domains.
+ */
+static void console_switch_focus(void)
+{
+    const domid_t n = domid_top + 1;
+    domid_t i = ( console_focus == DOMID_XEN )
+                ? get_initial_domain_id() : console_focus + 1;
+
+    for ( ; i < n; i++ )
+        if ( !console_set_focus(i) )
+            return;
+
+    console_set_focus(DOMID_XEN);
 }
 
 static void __serial_rx(char c)
@@ -551,7 +546,7 @@ static void __serial_rx(char c)
     struct domain *d;
     int rc = 0;
 
-    if ( console_focus == 0 )
+    if ( console_focus == DOMID_XEN )
         return handle_keypress(c, false);
 
     d = console_get_domain();
@@ -1141,14 +1136,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_focus = max_console_rx;
-
     register_keyhandler('w', dump_console_ring_key,
                         "synchronously dump console ring buffer (dmesg)", 0);
     register_irq_keyhandler('+', &do_inc_thresh,
@@ -1158,8 +1145,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_focus();
+    if ( opt_conswitch[1] != 'x' )
+        console_set_focus( get_initial_domain_id() );
 }
 
 int __init console_has(const char *device)
-- 
2.34.1

[PATCH v1 8/8] xen/console: introduce console_get_focus()

[dmkhn]

Add console_get_focus() as a console public API to the retrieve current
console owner domain ID.

Make console_{get,put}_domain() private and simplify vpl011 code a bit.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/arm/vpl011.c      | 5 +----
 xen/drivers/char/console.c | 9 +++++++--
 xen/include/xen/console.h  | 3 +--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 147958eee8..9ce6151c2a 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -78,12 +78,11 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
     unsigned long flags;
     struct vpl011 *vpl011 = &d->arch.vpl011;
     struct vpl011_xen_backend *intf = vpl011->backend.xen;
-    struct domain *input = console_get_domain();
 
     VPL011_LOCK(d, flags);
 
     intf->out[intf->out_prod++] = data;
-    if ( d == input )
+    if ( d->domain_id == console_get_focus() )
     {
         if ( intf->out_prod == 1 )
         {
@@ -123,8 +122,6 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
     vpl011_update_interrupt_status(d);
 
     VPL011_UNLOCK(d, flags);
-
-    console_put_domain(input);
 }
 
 /*
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 0e2349a868..906a0ae996 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -484,12 +484,12 @@ static struct domain *console_get_domain_by_id(domid_t domid)
     return NULL;
 }
 
-struct domain *console_get_domain(void)
+static struct domain *console_get_domain(void)
 {
     return console_get_domain_by_id(console_focus);
 }
 
-void console_put_domain(struct domain *d)
+static void console_put_domain(struct domain *d)
 {
     if ( d )
         rcu_unlock_domain(d);
@@ -523,6 +523,11 @@ static int console_set_focus(domid_t domid)
     return 0;
 }
 
+domid_t console_get_focus(void)
+{
+    return console_focus;
+}
+
 /*
  * Switch console focus.
  * Rotates input focus among Xen, dom0 and boot-time created domUs while
diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h
index 83cbc9fbda..19da2b755c 100644
--- a/xen/include/xen/console.h
+++ b/xen/include/xen/console.h
@@ -32,8 +32,7 @@ void console_end_sync(void);
 void console_start_log_everything(void);
 void console_end_log_everything(void);
 
-struct domain *console_get_domain(void);
-void console_put_domain(struct domain *d);
+domid_t console_get_focus(void);
 
 /*
  * Steal output from the console. Returns +ve identifier, else -ve error.
-- 
2.34.1

Re: [PATCH v1 1/8] xen/console: fix trailing whitespaces

[Jan Beulich]

On 19.03.2025 00:36, dmkhn@proton.me wrote:
> Remove trailing whitespaces in the console driver.
> 
> No functional change.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v1 0/8] xen/console: cleanup console input switch logic

[Denis Mukhin]

On Tuesday, March 18th, 2025 at 4:36 PM, dmkhn@proton.me <dmkhn@proton.me> wrote:

> 
> 
> 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 patch series introduces the concept of "console focus", which is
> defined as the ID of the domain that currently owns the physical console
> input.
> 
> The patch series originates from the NS16550 UART emulator series [1]
> for x86, which requires ability to switch physical console input to a
> PVH/HVM domain with an emulated UART.
> 
> The main idea 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 0 removes all the trailing white spaces in the console driver code.
> 
> Patch 1 introduces a new domain permission flag to mark ownership of the
> console input for the console driver.
> 
> Patches 2-4 prepare console driver to allow console input rotation
> across multiple domains based on the new permission flag.
> 
> Patches 5-6 perform mechanical renames to fit the usage in the code.
> 
> Patch 7 cleans up the console input switch logic.
> 
> Patch 8 simplifies the existing vUART code by using newly introduced APIs.

Corrected series explanation:

Patch 1 removes all the trailing white spaces in the console driver code.

Patch 2 introduces a new domain permission flag to mark ownership of the
console input for the console driver.
 
Patches 3, 4 prepare console driver to allow console input rotation
across multiple domains based on the new permission flag.

Patches 5, 6 perform mechanical renames to fit the usage in the code.
 
Patch 7 cleans up the console input switch logic.

Patch 8 simplifies the existing vUART code by using newly introduced APIs.


> 
> CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1723361248
> 
> [1]: https://lore.kernel.org/xen-devel/20250103-vuart-ns8250-v3-v1-0-c5d36b31d66c@ford.com/
> 
> Denis Mukhin (8):
> xen/console: fix trailing whitespaces
> xen/console: introduce console input permission
> xen/domain: introduce domid_top
> xen/domain: introduce domid_alloc()
> xen/console: rename switch_serial_input() to console_switch_focus()
> xen/console: rename console_rx to console_focus
> xen/console: introduce console_set_focus()
> xen/console: introduce console_get_focus()
> 
> xen/arch/arm/dom0less-build.c | 15 ++-
> xen/arch/arm/domain_build.c | 19 +++-
> xen/arch/arm/include/asm/setup.h | 2 -
> xen/arch/arm/setup.c | 2 -
> xen/arch/arm/vpl011.c | 7 +-
> 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/domain.c | 5 +
> xen/common/domctl.c | 71 +++++++++------
> xen/common/kernel.c | 8 ++
> xen/drivers/char/console.c | 142 +++++++++++++++++------------
> xen/include/xen/console.h | 3 +-
> xen/include/xen/domain.h | 5 +
> xen/include/xen/sched.h | 8 +-
> 16 files changed, 180 insertions(+), 115 deletions(-)
> 
> --
> 2.34.1

Re: [PATCH v1 7/8] xen/console: introduce console_set_focus()

[Denis Mukhin]

On Tuesday, March 18th, 2025 at 4:37 PM, dmkhn@proton.me <dmkhn@proton.me> wrote:

> 
> 
> Switch console_focus address space from integers mapped to domain IDs to
> direct domain IDs, simplifying the console input switching code.
> 
> Introduce console_set_focus() to set the console owner domain identifier.
> 
> Signed-off-by: Denis Mukhin dmukhin@ford.com
> 
> ---
> xen/drivers/char/console.c | 81 ++++++++++++++++----------------------
> 1 file changed, 34 insertions(+), 47 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 3d538510f4..0e2349a868 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -462,14 +462,12 @@ static void cf_check dump_console_ring_key(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 owner domain identifier. /
> -static unsigned int __read_mostly console_focus = 0;
> -
> -#define max_console_rx (domid_top + 1)
> +static domid_t __read_mostly console_focus = DOMID_XEN;
> 
> static struct domain console_get_domain_by_id(domid_t domid)
> {
> @@ -488,9 +486,7 @@ static struct domain console_get_domain_by_id(domid_t domid)
> 
> struct domain console_get_domain(void)
> {
> - if ( console_focus == 0 )
> - return NULL;
> - return console_get_domain_by_id(console_focus - 1);
> + return console_get_domain_by_id(console_focus);
> }
> 
> void console_put_domain(struct domain d)
> @@ -508,42 +504,41 @@ static bool console_check_focus_by_id(domid_t domid)
> return !!d;
> }
> 
> -static void console_switch_focus(void)
> +static int console_set_focus(domid_t domid)
> {
> - unsigned int next_rx = console_focus;
> + if ( domid == DOMID_XEN )
> + printk(" Serial input to Xen");
> + else if ( console_check_focus_by_id(domid) )
> + printk("* Serial input to DOM%u", domid);
> + else
> + return -ENOENT;
> 
> - /*
> - * Rotate among Xen, dom0 and boot-time created domUs while skipping
> - * switching serial input to non existing domains.
> - */
> - for ( ; ; )
> - {
> - domid_t domid;
> -
> - if ( next_rx++ >= max_console_rx )
> 
> - {
> - console_focus = 0;
> - printk("*** Serial input to Xen");
> - break;
> - }
> -
> - if ( consoled_is_enabled() && next_rx == 1 )
> - domid = get_initial_domain_id();
> - else
> - domid = next_rx - 1;
> -
> - if ( console_check_focus_by_id(domid) )
> - {
> - console_focus = next_rx;
> - printk("*** Serial input to DOM%u", domid);
> - break;
> - }
> - }
> + console_focus = domid;
> 
> if ( switch_code )
> printk(" (type 'CTRL-%c' three times to switch input)",
> opt_conswitch[0]);
> printk("\n");
> +
> + return 0;
> +}
> +
> +/*
> + * Switch console focus.
> + * Rotates input focus among Xen, dom0 and boot-time created domUs while
> + * skipping switching serial input to non existing domains.
> + */
> +static void console_switch_focus(void)
> +{
> + const domid_t n = domid_top + 1;
> + domid_t i = ( console_focus == DOMID_XEN )
> + ? get_initial_domain_id() : console_focus + 1;
> +
> + for ( ; i < n; i++ )
> + if ( !console_set_focus(i) )
> + return;
> +
> + console_set_focus(DOMID_XEN);
> }
> 
> static void __serial_rx(char c)
> @@ -551,7 +546,7 @@ static void __serial_rx(char c)
> struct domain d;
> int rc = 0;
> 
> - if ( console_focus == 0 )
> + if ( console_focus == DOMID_XEN )
> return handle_keypress(c, false);
> 
> d = console_get_domain();
> @@ -1141,14 +1136,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_focus = max_console_rx;
> -
> register_keyhandler('w', dump_console_ring_key,
> "synchronously dump console ring buffer (dmesg)", 0);
> register_irq_keyhandler('+', &do_inc_thresh,
> @@ -1158,8 +1145,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_focus();
> + if ( opt_conswitch[1] != 'x' )
> + console_set_focus( get_initial_domain_id() );

Forgot to drop extra spaces around the function parameter.

> }
> 
> int __init console_has(const char *device)
> --
> 2.34.1

Re: [PATCH v1 2/8] xen/console: introduce console input permission

[Jan Beulich]

On 19.03.2025 00:36, dmkhn@proton.me wrote:
> @@ -564,10 +586,25 @@ static void __serial_rx(char c)
>          /* Deliver input to the PV shim console. */
>          rc = consoled_guest_tx(c);
>  
> -    if ( rc )
> +    switch ( rc )
> +    {
> +    case 0:
> +        break;
> +
> +    case -EBUSY:    /* Loopback mode */
> +    case -ENOSPC:   /* FIFO is full */
>          guest_printk(d,
>                       XENLOG_WARNING "failed to process console input: %d\n",
>                       rc);
> +        break;
> +
> +    default:
> +        d->console.input_allowed = false;

This aspect isn't mentioned / justified in the description, and I also
can't deduce why you would do so. Or to put it differently, why you'd
then not also take away input focus from this domain, for it no longer
being eligible to have focus.

> --- 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 own physical console input. */
> +        bool input_allowed;
> +    } console;

Are further fields going to be added to this sub-struct? If not, is having
a sub-struct here actually worth it?

Jan

Re: [PATCH v1 3/8] xen/domain: introduce domid_top

[Jan Beulich]

On 19.03.2025 00:36, dmkhn@proton.me wrote:
> Rename max_init_domid to domid_top to align with its usage in the code
> (Arm), where it represents the upper boundary of the non-system domain
> ID range.

I think I said before that I don't share this view of yours. The variable
is quite appropriately named; it's the further use you made of it in earlier
versions of work of yours in this area where the name ended up no longer
reflecting the purpose. Yet that's not a reason to change the name.

I'm unconvinced we actually need a variable tracking the largest known
non-system domain ID. The domain list is sorted, so obtaining that ID
should be reasonably easy and cheap without such a variable.

Furthermore such a variable would likely also need reducing when the
domain with the highest ID dies.

> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -472,7 +472,7 @@ static void cf_check dump_console_ring_key(unsigned char key)
>   */
>  static unsigned int __read_mostly console_rx = 0;
>  
> -#define max_console_rx (max_init_domid + 1)
> +#define max_console_rx (domid_top + 1)

This use in particular is bogus. The intention here is to permit console
access only to (some?) domains created during boot. Making a (conceptual)
change here would require separate justification.

Jan

Re: [PATCH v1 4/8] xen/domain: introduce domid_alloc()

[Jan Beulich]

On 19.03.2025 00:36, dmkhn@proton.me wrote:
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2367,8 +2367,15 @@ void __init create_dom0(void)
>          .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
>      };
>      unsigned int flags = CDF_privileged;
> +    domid_t domid;
>      int rc;
>  
> +    rc = domid_alloc(get_initial_domain_id());
> +    if ( rc < 0 )
> +        panic("Cannot use domain ID %d (rc = %d)\n",
> +              get_initial_domain_id(), rc);
> +    domid = rc;
> +
>      /* The vGIC for DOM0 is exactly emulating the hardware GIC */
>      dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
>      dom0_cfg.arch.nr_spis = VGIC_DEF_NR_SPIS;
> @@ -2391,19 +2398,21 @@ void __init create_dom0(void)
>      if ( !llc_coloring_enabled )
>          flags |= CDF_directmap;
>  
> -    dom0 = domain_create(0, &dom0_cfg, flags);
> +    dom0 = domain_create(domid, &dom0_cfg, flags);
>      if ( IS_ERR(dom0) )
> -        panic("Error creating domain 0 (rc = %ld)\n", PTR_ERR(dom0));
> +        panic("Error creating domain %d (rc = %ld)\n", domid, PTR_ERR(dom0));
>  
>      if ( llc_coloring_enabled && (rc = dom0_set_llc_colors(dom0)) )
> -        panic("Error initializing LLC coloring for domain 0 (rc = %d)\n", rc);
> +        panic("Error initializing LLC coloring for domain %d (rc = %d)\n",
> +              domid, rc);
>  
>      if ( alloc_dom0_vcpu0(dom0) == NULL )
> -        panic("Error creating domain 0 vcpu0\n");
> +        panic("Error creating domain %d vcpu0\n", domid);
>  
>      rc = construct_dom0(dom0);
>      if ( rc )
> -        panic("Could not set up DOM0 guest OS (rc = %d)\n", rc);
> +        panic("Could not set up guest OS for domain %d (rc = %d)\n",
> +              domid, rc);
>  }

Assuming we really needed this new function, why would there be an Arm side
change to Dom0 creation, but no equivalent change on the x86 side?

Jan

Re: [PATCH v1 5/8] xen/console: rename switch_serial_input() to console_switch_focus()

[Jan Beulich]

On 19.03.2025 00:36, dmkhn@proton.me wrote:
> Update the name to emphasize the physical console input switch to a
> new owner domain following the naming notation in the console driver.
> 
> No functional change.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

While I don't strictly mind the change, I'd like to point out that ...

> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -511,7 +511,7 @@ static bool console_check_focus_by_id(domid_t domid)
>      return !!d;
>  }
>  
> -static void switch_serial_input(void)
> +static void console_switch_focus(void)

... unlike the description says, "input" isn't reflected at all in the new
name.

Jan

Re: [PATCH v1 6/8] xen/console: rename console_rx to console_focus

[Jan Beulich]

On 19.03.2025 00:36, dmkhn@proton.me wrote:
> Update the symbol name in preparation for the semantic change
> to the physical console input owner domain identifier.

As you say, no semantic change here just yet, but then ...

> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -465,12 +465,9 @@ static void cf_check dump_console_ring_key(unsigned char key)
>   * and the DomUs started from Xen at boot.
>   */
>  #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)
> - */

... this (correct) comment is replaced by ...

> -static unsigned int __read_mostly console_rx = 0;
> +
> +/* Console owner domain identifier. */

... this (right now) incorrect one.

Jan

Re: [PATCH v1 7/8] xen/console: introduce console_set_focus()

[Jan Beulich]

On 19.03.2025 20:53, Denis Mukhin wrote:
> On Tuesday, March 18th, 2025 at 4:37 PM, dmkhn@proton.me <dmkhn@proton.me> wrote:
>> +/*
>> + * Switch console focus.
>> + * Rotates input focus among Xen, dom0 and boot-time created domUs while
>> + * skipping switching serial input to non existing domains.
>> + */
>> +static void console_switch_focus(void)
>> +{
>> + const domid_t n = domid_top + 1;
>> + domid_t i = ( console_focus == DOMID_XEN )

Same issue here as ...

>> @@ -1158,8 +1145,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_focus();
>> + if ( opt_conswitch[1] != 'x' )
>> + console_set_focus( get_initial_domain_id() );
> 
> Forgot to drop extra spaces around the function parameter.

... you noticed here.

Jan

Re: [PATCH v1 7/8] xen/console: introduce console_set_focus()

[Jan Beulich]

On 19.03.2025 00:37, dmkhn@proton.me wrote:
> +/*
> + * Switch console focus.
> + * Rotates input focus among Xen, dom0 and boot-time created domUs while
> + * skipping switching serial input to non existing domains.
> + */
> +static void console_switch_focus(void)
> +{
> +    const domid_t n = domid_top + 1;

The use of this renamed identifier is now in sharp conflict with the comment,
talking about boot-time created domains only.

> +    domid_t i = ( console_focus == DOMID_XEN )
> +                ? get_initial_domain_id() : console_focus + 1;
> +
> +    for ( ; i < n; i++ )
> +        if ( !console_set_focus(i) )

This being the sole call caring about the function's return value, why isn't
the function returning a mere boolean?

Jan

Re: [PATCH v1 2/8] xen/console: introduce console input permission

[Denis Mukhin]

On Wednesday, March 26th, 2025 at 6:44 AM, Jan Beulich <jbeulich@suse.com> wrote:

>
>
> On 19.03.2025 00:36, dmkhn@proton.me wrote:
>
> > @@ -564,10 +586,25 @@ static void __serial_rx(char c)
> > /* Deliver input to the PV shim console. */
> > rc = consoled_guest_tx(c);
> >
> > - if ( rc )
> > + switch ( rc )
> > + {
> > + case 0:
> > + break;
> > +
> > + case -EBUSY: /* Loopback mode /
> > + case -ENOSPC: / FIFO is full */
> > guest_printk(d,
> > XENLOG_WARNING "failed to process console input: %d\n",
> > rc);
> > + break;
> > +
> > + default:
> > + d->console.input_allowed = false;
>
>
> This aspect isn't mentioned / justified in the description, and I also
> can't deduce why you would do so. Or to put it differently, why you'd
> then not also take away input focus from this domain, for it no longer
> being eligible to have focus.

My idea was to explicitly distinguish "recoverable" errors, such as
"FIFO is full", from "emulator logical" errors when input character cannot
be delivered to the domain because of an error in emulator, so those are
easily seen in the debug logs.

I re-inspected return values, the only values currently supported are:
  (1) 0: success
  (2) -ENODEV: domain is not configured to have vUART
  (3) -ENOSPC: FIFO is full

-EBUSY is supposed to mean "vUART is in loopback mode" and it is leaked
from the future NS16550 emulator code.

I will drop that change in v2.

>
> > --- 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 own physical console input. */
> > + bool input_allowed;
> > + } console;
>
>
> Are further fields going to be added to this sub-struct? If not, is having
> a sub-struct here actually worth it?

I was thinking about grouping all console-related fields in struct domain
here in the future: e.g. move pbuf-related fields under "struct console".

>
> Jan