xen/arch/arm/vpl011.c | 2 +- xen/arch/x86/hvm/hvm.c | 16 +++++++++------- xen/arch/x86/pv/shim.c | 2 +- xen/common/domain.c | 19 +++++++++---------- xen/drivers/char/console.c | 28 ++++++++++++++++------------ xen/include/xen/sched.h | 24 +++++++++++++----------- 6 files changed, 49 insertions(+), 42 deletions(-)
From: Denis Mukhin <dmukhin@ford.com>
Introduce domain_console for grouping data structures used for integrating
domain's diagnostic console with Xen's console driver.
Group all pbuf-related data structures under domain_console. Rename the moved
fields to plain .buf, .idx and .lock names, since all uses of the fields are
touched.
Ensure accesses to domain_console pointer are valid in console_switch_input().
Bump the domain console buffer allocation size to 256. No extra symbol for the
value since it is used only once during data structure declaration. All size
checks use ARRAY_SIZE().
Allocate domain_console from the heap so that the parent domain struct size
stays below PAGE_SIZE boundary to account for more console-related fields
added in the future.
Finally, update the domain_console allocation and initialization code.
Not a functional change.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v9:
- kept cd as is in hvm_print_line() as requested
- dropped domain lock in hvm_print_line()
Link to v9: https://lore.kernel.org/xen-devel/20250723001116.186009-1-dmukhin@ford.com/
---
xen/arch/arm/vpl011.c | 2 +-
xen/arch/x86/hvm/hvm.c | 16 +++++++++-------
xen/arch/x86/pv/shim.c | 2 +-
xen/common/domain.c | 19 +++++++++----------
xen/drivers/char/console.c | 28 ++++++++++++++++------------
xen/include/xen/sched.h | 24 +++++++++++++-----------
6 files changed, 49 insertions(+), 42 deletions(-)
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 480fc664fc62..d0d17c76b72c 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -713,7 +713,7 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
}
else
{
- d->console.input_allowed = true;
+ d->console->input_allowed = true;
vpl011->backend_in_domain = false;
vpl011->backend.xen = xzalloc(struct vpl011_xen_backend);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 56c7de39778b..37af507a8d90 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -560,6 +560,7 @@ static int cf_check hvm_print_line(
int dir, unsigned int port, unsigned int bytes, uint32_t *val)
{
struct domain *cd = current->domain;
+ struct domain_console *cons = cd->console;
char c = *val;
ASSERT(bytes == 1 && port == XEN_HVM_DEBUGCONS_IOPORT);
@@ -571,16 +572,17 @@ static int cf_check hvm_print_line(
if ( !is_console_printable(c) )
return X86EMUL_OKAY;
- spin_lock(&cd->pbuf_lock);
+ spin_lock(&cons->lock);
+ ASSERT(cons->idx < ARRAY_SIZE(cons->buf));
if ( c != '\n' )
- cd->pbuf[cd->pbuf_idx++] = c;
- if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c == '\n') )
+ cons->buf[cons->idx++] = c;
+ if ( (cons->idx == (ARRAY_SIZE(cons->buf) - 1)) || (c == '\n') )
{
- cd->pbuf[cd->pbuf_idx] = '\0';
- guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf);
- cd->pbuf_idx = 0;
+ cons->buf[cons->idx] = '\0';
+ guest_printk(cd, XENLOG_G_DEBUG "%s\n", cons->buf);
+ cons->idx = 0;
}
- spin_unlock(&cd->pbuf_lock);
+ spin_unlock(&cons->lock);
return X86EMUL_OKAY;
}
diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index bc2a7dd5fae5..bd29c53a2d34 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -239,7 +239,7 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
*/
d->max_pages = domain_tot_pages(d);
- d->console.input_allowed = true;
+ 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 303c338ef293..caef4cc8d649 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -669,7 +669,7 @@ static void _domain_destroy(struct domain *d)
BUG_ON(!d->is_dying);
BUG_ON(atomic_read(&d->refcnt) != DOMAIN_DESTROYED);
- xfree(d->pbuf);
+ xvfree(d->console);
argo_destroy(d);
@@ -835,8 +835,6 @@ 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. */
@@ -866,8 +864,6 @@ struct domain *domain_create(domid_t domid,
spin_lock_init(&d->shutdown_lock);
d->shutdown_code = SHUTDOWN_CODE_INVALID;
- spin_lock_init(&d->pbuf_lock);
-
rwlock_init(&d->vnuma_rwlock);
#ifdef CONFIG_HAS_PCI
@@ -877,6 +873,14 @@ struct domain *domain_create(domid_t domid,
/* All error paths can depend on the above setup. */
+ err = -ENOMEM;
+ d->console = xvzalloc(typeof(*d->console));
+ if ( !d->console )
+ goto fail;
+
+ spin_lock_init(&d->console->lock);
+ d->console->input_allowed = is_hardware_domain(d);
+
/*
* Allocate d->vcpu[] and set ->max_vcpus up early. Various per-domain
* resources want to be sized based on max_vcpus.
@@ -959,11 +963,6 @@ struct domain *domain_create(domid_t domid,
if ( (err = argo_init(d)) != 0 )
goto fail;
- err = -ENOMEM;
- d->pbuf = xzalloc_array(char, DOMAIN_PBUF_SIZE);
- if ( !d->pbuf )
- goto fail;
-
if ( (err = sched_init_domain(d, config->cpupool_id)) != 0 )
goto fail;
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 963c7b043cd8..75fa033ce74d 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -526,7 +526,7 @@ struct domain *console_get_domain(void)
if ( !d )
return NULL;
- if ( d->console.input_allowed )
+ if ( d->console->input_allowed )
return d;
rcu_unlock_domain(d);
@@ -567,10 +567,13 @@ static void console_switch_input(void)
d = rcu_lock_domain_by_id(domid);
if ( d )
{
- rcu_unlock_domain(d);
-
- if ( !d->console.input_allowed )
+ if ( !d->console->input_allowed )
+ {
+ rcu_unlock_domain(d);
continue;
+ }
+
+ rcu_unlock_domain(d);
console_rx = next_rx;
printk("*** Serial input to DOM%u", domid);
@@ -749,6 +752,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
else
{
char *kin = kbuf, *kout = kbuf, c;
+ struct domain_console *cons = cd->console;
/* Strip non-printable characters */
do
@@ -761,22 +765,22 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
} while ( --kcount > 0 );
*kout = '\0';
- spin_lock(&cd->pbuf_lock);
+ spin_lock(&cons->lock);
kcount = kin - kbuf;
if ( c != '\n' &&
- (cd->pbuf_idx + (kout - kbuf) < (DOMAIN_PBUF_SIZE - 1)) )
+ (cons->idx + (kout - kbuf) < (ARRAY_SIZE(cons->buf) - 1)) )
{
/* buffer the output until a newline */
- memcpy(cd->pbuf + cd->pbuf_idx, kbuf, kout - kbuf);
- cd->pbuf_idx += (kout - kbuf);
+ memcpy(cons->buf + cons->idx, kbuf, kout - kbuf);
+ cons->idx += kout - kbuf;
}
else
{
- cd->pbuf[cd->pbuf_idx] = '\0';
- guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf);
- cd->pbuf_idx = 0;
+ cons->buf[cons->idx] = '\0';
+ guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cons->buf, kbuf);
+ cons->idx = 0;
}
- spin_unlock(&cd->pbuf_lock);
+ spin_unlock(&cons->lock);
}
guest_handle_add_offset(buffer, kcount);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index fe53d4fab7ba..c828d5626dea 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -371,6 +371,17 @@ struct evtchn_port_ops;
#define MAX_NR_IOREQ_SERVERS 8
+/* Domain console settings. */
+struct domain_console {
+ /* Permission to take ownership of the physical console input. */
+ bool input_allowed;
+
+ /* hvm_print_line() and guest_console_write() logging. */
+ unsigned int idx;
+ spinlock_t lock;
+ char buf[256];
+};
+
struct domain
{
domid_t domain_id;
@@ -562,12 +573,6 @@ struct domain
/* Control-plane tools handle for this domain. */
xen_domain_handle_t handle;
- /* hvm_print_line() and guest_console_write() logging. */
-#define DOMAIN_PBUF_SIZE 200
- char *pbuf;
- unsigned int pbuf_idx;
- spinlock_t pbuf_lock;
-
/* OProfile support. */
struct xenoprof *xenoprof;
@@ -652,11 +657,8 @@ struct domain
const unsigned int *llc_colors;
#endif
- /* Console settings. */
- struct {
- /* Permission to take ownership of the physical console input. */
- bool input_allowed;
- } console;
+ /* Pointer to console settings; NULL for system Xen domains. */
+ struct domain_console *console;
} __aligned(PAGE_SIZE);
static inline struct page_list_head *page_to_list(
--
2.34.1On Fri, Jul 25, 2025 at 09:08:02PM +0000, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Introduce domain_console for grouping data structures used for integrating
> domain's diagnostic console with Xen's console driver.
>
> Group all pbuf-related data structures under domain_console. Rename the moved
> fields to plain .buf, .idx and .lock names, since all uses of the fields are
> touched.
>
> Ensure accesses to domain_console pointer are valid in console_switch_input().
>
> Bump the domain console buffer allocation size to 256. No extra symbol for the
> value since it is used only once during data structure declaration. All size
> checks use ARRAY_SIZE().
>
> Allocate domain_console from the heap so that the parent domain struct size
> stays below PAGE_SIZE boundary to account for more console-related fields
> added in the future.
>
> Finally, update the domain_console allocation and initialization code.
>
> Not a functional change.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
Just one request about the allocation of the domain_console in
domain_create().
> ---
> Changes since v9:
> - kept cd as is in hvm_print_line() as requested
> - dropped domain lock in hvm_print_line()
>
> Link to v9: https://lore.kernel.org/xen-devel/20250723001116.186009-1-dmukhin@ford.com/
> ---
> xen/arch/arm/vpl011.c | 2 +-
> xen/arch/x86/hvm/hvm.c | 16 +++++++++-------
> xen/arch/x86/pv/shim.c | 2 +-
> xen/common/domain.c | 19 +++++++++----------
> xen/drivers/char/console.c | 28 ++++++++++++++++------------
> xen/include/xen/sched.h | 24 +++++++++++++-----------
> 6 files changed, 49 insertions(+), 42 deletions(-)
>
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index 480fc664fc62..d0d17c76b72c 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -713,7 +713,7 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
> }
> else
> {
> - d->console.input_allowed = true;
> + d->console->input_allowed = true;
> vpl011->backend_in_domain = false;
>
> vpl011->backend.xen = xzalloc(struct vpl011_xen_backend);
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 56c7de39778b..37af507a8d90 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -560,6 +560,7 @@ static int cf_check hvm_print_line(
> int dir, unsigned int port, unsigned int bytes, uint32_t *val)
> {
> struct domain *cd = current->domain;
> + struct domain_console *cons = cd->console;
> char c = *val;
>
> ASSERT(bytes == 1 && port == XEN_HVM_DEBUGCONS_IOPORT);
> @@ -571,16 +572,17 @@ static int cf_check hvm_print_line(
> if ( !is_console_printable(c) )
> return X86EMUL_OKAY;
>
> - spin_lock(&cd->pbuf_lock);
> + spin_lock(&cons->lock);
> + ASSERT(cons->idx < ARRAY_SIZE(cons->buf));
> if ( c != '\n' )
> - cd->pbuf[cd->pbuf_idx++] = c;
> - if ( (cd->pbuf_idx == (DOMAIN_PBUF_SIZE - 1)) || (c == '\n') )
> + cons->buf[cons->idx++] = c;
> + if ( (cons->idx == (ARRAY_SIZE(cons->buf) - 1)) || (c == '\n') )
> {
> - cd->pbuf[cd->pbuf_idx] = '\0';
> - guest_printk(cd, XENLOG_G_DEBUG "%s\n", cd->pbuf);
> - cd->pbuf_idx = 0;
> + cons->buf[cons->idx] = '\0';
> + guest_printk(cd, XENLOG_G_DEBUG "%s\n", cons->buf);
> + cons->idx = 0;
> }
> - spin_unlock(&cd->pbuf_lock);
> + spin_unlock(&cons->lock);
>
> return X86EMUL_OKAY;
> }
> diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
> index bc2a7dd5fae5..bd29c53a2d34 100644
> --- a/xen/arch/x86/pv/shim.c
> +++ b/xen/arch/x86/pv/shim.c
> @@ -239,7 +239,7 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
> */
> d->max_pages = domain_tot_pages(d);
>
> - d->console.input_allowed = true;
> + 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 303c338ef293..caef4cc8d649 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -669,7 +669,7 @@ static void _domain_destroy(struct domain *d)
> BUG_ON(!d->is_dying);
> BUG_ON(atomic_read(&d->refcnt) != DOMAIN_DESTROYED);
>
> - xfree(d->pbuf);
> + xvfree(d->console);
Nit: even if the struct if being freed just a couple of lines below, I
would use XVFREE() here.
>
> argo_destroy(d);
>
> @@ -835,8 +835,6 @@ 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. */
> @@ -866,8 +864,6 @@ struct domain *domain_create(domid_t domid,
> spin_lock_init(&d->shutdown_lock);
> d->shutdown_code = SHUTDOWN_CODE_INVALID;
>
> - spin_lock_init(&d->pbuf_lock);
> -
> rwlock_init(&d->vnuma_rwlock);
>
> #ifdef CONFIG_HAS_PCI
> @@ -877,6 +873,14 @@ struct domain *domain_create(domid_t domid,
>
> /* All error paths can depend on the above setup. */
>
> + err = -ENOMEM;
> + d->console = xvzalloc(typeof(*d->console));
> + if ( !d->console )
> + goto fail;
> +
> + spin_lock_init(&d->console->lock);
> + d->console->input_allowed = is_hardware_domain(d);
That's too early to do the allocation of the console buffer AFAICT.
It needs to be done after the:
if ( is_system_domain(d) )
return d;
check, as allocating a console buffer for system domains is pointless.
> +
> /*
> * Allocate d->vcpu[] and set ->max_vcpus up early. Various per-domain
> * resources want to be sized based on max_vcpus.
> @@ -959,11 +963,6 @@ struct domain *domain_create(domid_t domid,
> if ( (err = argo_init(d)) != 0 )
> goto fail;
>
> - err = -ENOMEM;
> - d->pbuf = xzalloc_array(char, DOMAIN_PBUF_SIZE);
> - if ( !d->pbuf )
> - goto fail;
> -
> if ( (err = sched_init_domain(d, config->cpupool_id)) != 0 )
> goto fail;
>
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 963c7b043cd8..75fa033ce74d 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -526,7 +526,7 @@ struct domain *console_get_domain(void)
> if ( !d )
> return NULL;
>
> - if ( d->console.input_allowed )
> + if ( d->console->input_allowed )
> return d;
>
> rcu_unlock_domain(d);
> @@ -567,10 +567,13 @@ static void console_switch_input(void)
> d = rcu_lock_domain_by_id(domid);
> if ( d )
> {
> - rcu_unlock_domain(d);
> -
> - if ( !d->console.input_allowed )
> + if ( !d->console->input_allowed )
> + {
> + rcu_unlock_domain(d);
> continue;
> + }
> +
> + rcu_unlock_domain(d);
>
> console_rx = next_rx;
> printk("*** Serial input to DOM%u", domid);
> @@ -749,6 +752,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
> else
> {
> char *kin = kbuf, *kout = kbuf, c;
> + struct domain_console *cons = cd->console;
>
> /* Strip non-printable characters */
> do
> @@ -761,22 +765,22 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
> } while ( --kcount > 0 );
>
> *kout = '\0';
> - spin_lock(&cd->pbuf_lock);
> + spin_lock(&cons->lock);
> kcount = kin - kbuf;
> if ( c != '\n' &&
> - (cd->pbuf_idx + (kout - kbuf) < (DOMAIN_PBUF_SIZE - 1)) )
> + (cons->idx + (kout - kbuf) < (ARRAY_SIZE(cons->buf) - 1)) )
> {
> /* buffer the output until a newline */
> - memcpy(cd->pbuf + cd->pbuf_idx, kbuf, kout - kbuf);
> - cd->pbuf_idx += (kout - kbuf);
> + memcpy(cons->buf + cons->idx, kbuf, kout - kbuf);
> + cons->idx += kout - kbuf;
> }
> else
> {
> - cd->pbuf[cd->pbuf_idx] = '\0';
> - guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf);
> - cd->pbuf_idx = 0;
> + cons->buf[cons->idx] = '\0';
> + guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cons->buf, kbuf);
> + cons->idx = 0;
> }
> - spin_unlock(&cd->pbuf_lock);
> + spin_unlock(&cons->lock);
> }
>
> guest_handle_add_offset(buffer, kcount);
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index fe53d4fab7ba..c828d5626dea 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -371,6 +371,17 @@ struct evtchn_port_ops;
>
> #define MAX_NR_IOREQ_SERVERS 8
>
> +/* Domain console settings. */
> +struct domain_console {
> + /* Permission to take ownership of the physical console input. */
> + bool input_allowed;
> +
> + /* hvm_print_line() and guest_console_write() logging. */
> + unsigned int idx;
> + spinlock_t lock;
> + char buf[256];
> +};
> +
> struct domain
> {
> domid_t domain_id;
> @@ -562,12 +573,6 @@ struct domain
> /* Control-plane tools handle for this domain. */
> xen_domain_handle_t handle;
>
> - /* hvm_print_line() and guest_console_write() logging. */
> -#define DOMAIN_PBUF_SIZE 200
> - char *pbuf;
> - unsigned int pbuf_idx;
> - spinlock_t pbuf_lock;
> -
> /* OProfile support. */
> struct xenoprof *xenoprof;
>
> @@ -652,11 +657,8 @@ struct domain
> const unsigned int *llc_colors;
> #endif
>
> - /* Console settings. */
> - struct {
> - /* Permission to take ownership of the physical console input. */
> - bool input_allowed;
> - } console;
> + /* Pointer to console settings; NULL for system Xen domains. */
Nit: I would drop Xen from the sentence above, "system domains" is a
well known entity in this context.
Thanks, Roger.
© 2016 - 2025 Red Hat, Inc.