xen/arch/arm/vpl011.c | 2 +- xen/arch/x86/hvm/hvm.c | 17 +++++++++-------- xen/arch/x86/pv/shim.c | 2 +- xen/common/domain.c | 19 +++++++++---------- xen/drivers/char/console.c | 21 +++++++++++---------- xen/include/xen/sched.h | 24 ++++++++++++++---------- 6 files changed, 45 insertions(+), 40 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.
Bump the domain console buffer allocation size to 256.
Rename domain console buffer size symbol to DOMAIN_CONSOLE_BUF_SIZE.
Finally, update the domain_console allocation and initialization code.
No functional change.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v6:
- reverted allocation of domain_console as in v5
- dropped the commentary for DOMAIN_CONSOLE_BUF_SIZE
Link to v6: https://lore.kernel.org/xen-devel/20250710013421.2321353-1-dmukhin@ford.com/
CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1919473093
---
xen/arch/arm/vpl011.c | 2 +-
xen/arch/x86/hvm/hvm.c | 17 +++++++++--------
xen/arch/x86/pv/shim.c | 2 +-
xen/common/domain.c | 19 +++++++++----------
xen/drivers/char/console.c | 21 +++++++++++----------
xen/include/xen/sched.h | 24 ++++++++++++++----------
6 files changed, 45 insertions(+), 40 deletions(-)
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 2b6f2a09bca6..f4a840da10c5 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..2be98f6ccbd1 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -559,7 +559,8 @@ void hvm_do_resume(struct vcpu *v)
static int cf_check hvm_print_line(
int dir, unsigned int port, unsigned int bytes, uint32_t *val)
{
- struct domain *cd = current->domain;
+ const struct domain *d = current->domain;
+ struct domain_console *cons = d->console;
char c = *val;
ASSERT(bytes == 1 && port == XEN_HVM_DEBUGCONS_IOPORT);
@@ -571,16 +572,16 @@ static int cf_check hvm_print_line(
if ( !is_console_printable(c) )
return X86EMUL_OKAY;
- spin_lock(&cd->pbuf_lock);
+ spin_lock(&cons->lock);
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 == (DOMAIN_CONSOLE_BUF_SIZE - 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(d, 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 f54632bc0811..469f5e8832da 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);
@@ -569,7 +569,7 @@ static void console_switch_input(void)
{
rcu_unlock_domain(d);
- if ( !d->console.input_allowed )
+ if ( !d->console->input_allowed )
continue;
console_rx = next_rx;
@@ -788,6 +788,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
@@ -800,22 +801,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) < (DOMAIN_CONSOLE_BUF_SIZE - 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..8771b7f22b48 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -371,6 +371,19 @@ struct evtchn_port_ops;
#define MAX_NR_IOREQ_SERVERS 8
+#define DOMAIN_CONSOLE_BUF_SIZE (256)
+
+/* 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[DOMAIN_CONSOLE_BUF_SIZE];
+};
+
struct domain
{
domid_t domain_id;
@@ -562,12 +575,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;
@@ -653,10 +660,7 @@ struct domain
#endif
/* Console settings. */
- struct {
- /* Permission to take ownership of the physical console input. */
- bool input_allowed;
- } console;
+ struct domain_console *console;
} __aligned(PAGE_SIZE);
static inline struct page_list_head *page_to_list(
--
2.34.1On Fri, Jul 11, 2025 at 12:40:11AM +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.
>
> Bump the domain console buffer allocation size to 256.
>
> Rename domain console buffer size symbol to DOMAIN_CONSOLE_BUF_SIZE.
>
> Finally, update the domain_console allocation and initialization code.
>
> No functional change.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v6:
> - reverted allocation of domain_console as in v5
> - dropped the commentary for DOMAIN_CONSOLE_BUF_SIZE
>
> Link to v6: https://lore.kernel.org/xen-devel/20250710013421.2321353-1-dmukhin@ford.com/
> CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1919473093
> ---
> xen/arch/arm/vpl011.c | 2 +-
> xen/arch/x86/hvm/hvm.c | 17 +++++++++--------
> xen/arch/x86/pv/shim.c | 2 +-
> xen/common/domain.c | 19 +++++++++----------
> xen/drivers/char/console.c | 21 +++++++++++----------
> xen/include/xen/sched.h | 24 ++++++++++++++----------
> 6 files changed, 45 insertions(+), 40 deletions(-)
>
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index 2b6f2a09bca6..f4a840da10c5 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..2be98f6ccbd1 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -559,7 +559,8 @@ void hvm_do_resume(struct vcpu *v)
> static int cf_check hvm_print_line(
> int dir, unsigned int port, unsigned int bytes, uint32_t *val)
> {
> - struct domain *cd = current->domain;
> + const struct domain *d = current->domain;
> + struct domain_console *cons = d->console;
> char c = *val;
>
> ASSERT(bytes == 1 && port == XEN_HVM_DEBUGCONS_IOPORT);
> @@ -571,16 +572,16 @@ static int cf_check hvm_print_line(
> if ( !is_console_printable(c) )
> return X86EMUL_OKAY;
>
> - spin_lock(&cd->pbuf_lock);
> + spin_lock(&cons->lock);
> 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 == (DOMAIN_CONSOLE_BUF_SIZE - 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(d, 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 f54632bc0811..469f5e8832da 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);
> @@ -569,7 +569,7 @@ static void console_switch_input(void)
> {
> rcu_unlock_domain(d);
>
> - if ( !d->console.input_allowed )
> + if ( !d->console->input_allowed )
> continue;
>
> console_rx = next_rx;
> @@ -788,6 +788,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
> @@ -800,22 +801,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) < (DOMAIN_CONSOLE_BUF_SIZE - 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..8771b7f22b48 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -371,6 +371,19 @@ struct evtchn_port_ops;
>
> #define MAX_NR_IOREQ_SERVERS 8
>
> +#define DOMAIN_CONSOLE_BUF_SIZE (256)
> +
> +/* 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[DOMAIN_CONSOLE_BUF_SIZE];
> +};
> +
> struct domain
> {
> domid_t domain_id;
> @@ -562,12 +575,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;
>
> @@ -653,10 +660,7 @@ struct domain
> #endif
>
> /* Console settings. */
> - struct {
> - /* Permission to take ownership of the physical console input. */
> - bool input_allowed;
> - } console;
> + struct domain_console *console;
I assume I'm missing some context, what's the reason for converting
this field into a pointer, instead of moving the existing fields
inside and allocate the buffer at domain_create()?
Does having the buf field size known by the compiler allows for extra
safety checks?
I think if you make it that way you might want to change usages of
DOMAIN_CONSOLE_BUF_SIZE in the code to instead use ARRAY_SIZE() of the
field.
Thanks, Roger.
On Fri, Jul 11, 2025 at 12:54:11PM +0200, Roger Pau Monné wrote:
> On Fri, Jul 11, 2025 at 12:40:11AM +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.
> >
> > Bump the domain console buffer allocation size to 256.
> >
> > Rename domain console buffer size symbol to DOMAIN_CONSOLE_BUF_SIZE.
> >
> > Finally, update the domain_console allocation and initialization code.
> >
> > No functional change.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> > Changes since v6:
> > - reverted allocation of domain_console as in v5
> > - dropped the commentary for DOMAIN_CONSOLE_BUF_SIZE
> >
> > Link to v6: https://lore.kernel.org/xen-devel/20250710013421.2321353-1-dmukhin@ford.com/
> > CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1919473093
> > ---
> > xen/arch/arm/vpl011.c | 2 +-
> > xen/arch/x86/hvm/hvm.c | 17 +++++++++--------
> > xen/arch/x86/pv/shim.c | 2 +-
> > xen/common/domain.c | 19 +++++++++----------
> > xen/drivers/char/console.c | 21 +++++++++++----------
> > xen/include/xen/sched.h | 24 ++++++++++++++----------
> > 6 files changed, 45 insertions(+), 40 deletions(-)
> >
> > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > index 2b6f2a09bca6..f4a840da10c5 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..2be98f6ccbd1 100644
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -559,7 +559,8 @@ void hvm_do_resume(struct vcpu *v)
> > static int cf_check hvm_print_line(
> > int dir, unsigned int port, unsigned int bytes, uint32_t *val)
> > {
> > - struct domain *cd = current->domain;
> > + const struct domain *d = current->domain;
> > + struct domain_console *cons = d->console;
> > char c = *val;
> >
> > ASSERT(bytes == 1 && port == XEN_HVM_DEBUGCONS_IOPORT);
> > @@ -571,16 +572,16 @@ static int cf_check hvm_print_line(
> > if ( !is_console_printable(c) )
> > return X86EMUL_OKAY;
> >
> > - spin_lock(&cd->pbuf_lock);
> > + spin_lock(&cons->lock);
> > 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 == (DOMAIN_CONSOLE_BUF_SIZE - 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(d, 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 f54632bc0811..469f5e8832da 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);
> > @@ -569,7 +569,7 @@ static void console_switch_input(void)
> > {
> > rcu_unlock_domain(d);
> >
> > - if ( !d->console.input_allowed )
> > + if ( !d->console->input_allowed )
> > continue;
> >
> > console_rx = next_rx;
> > @@ -788,6 +788,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
> > @@ -800,22 +801,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) < (DOMAIN_CONSOLE_BUF_SIZE - 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..8771b7f22b48 100644
> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -371,6 +371,19 @@ struct evtchn_port_ops;
> >
> > #define MAX_NR_IOREQ_SERVERS 8
> >
> > +#define DOMAIN_CONSOLE_BUF_SIZE (256)
> > +
> > +/* 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[DOMAIN_CONSOLE_BUF_SIZE];
> > +};
> > +
> > struct domain
> > {
> > domid_t domain_id;
> > @@ -562,12 +575,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;
> >
> > @@ -653,10 +660,7 @@ struct domain
> > #endif
> >
> > /* Console settings. */
> > - struct {
> > - /* Permission to take ownership of the physical console input. */
> > - bool input_allowed;
> > - } console;
> > + struct domain_console *console;
>
> I assume I'm missing some context, what's the reason for converting
> this field into a pointer, instead of moving the existing fields
> inside and allocate the buffer at domain_create()?
The reason is avoid crossing PAGE_SIZE boundary for domain structure *if* more
console-related fields are added to the "domain console settings" (which will
happen for console focus management and probably for vUARTs).
>
> Does having the buf field size known by the compiler allows for extra
> safety checks?
>
> I think if you make it that way you might want to change usages of
> DOMAIN_CONSOLE_BUF_SIZE in the code to instead use ARRAY_SIZE() of the
> field.
Ack.
>
> Thanks, Roger.
On Fri, Jul 11, 2025 at 07:04:59PM +0000, dmkhn@proton.me wrote:
> On Fri, Jul 11, 2025 at 12:54:11PM +0200, Roger Pau Monné wrote:
> > On Fri, Jul 11, 2025 at 12:40:11AM +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.
> > >
> > > Bump the domain console buffer allocation size to 256.
> > >
> > > Rename domain console buffer size symbol to DOMAIN_CONSOLE_BUF_SIZE.
> > >
> > > Finally, update the domain_console allocation and initialization code.
> > >
> > > No functional change.
> > >
> > > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > > ---
> > > Changes since v6:
> > > - reverted allocation of domain_console as in v5
> > > - dropped the commentary for DOMAIN_CONSOLE_BUF_SIZE
> > >
> > > Link to v6: https://lore.kernel.org/xen-devel/20250710013421.2321353-1-dmukhin@ford.com/
> > > CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1919473093
> > > ---
> > > xen/arch/arm/vpl011.c | 2 +-
> > > xen/arch/x86/hvm/hvm.c | 17 +++++++++--------
> > > xen/arch/x86/pv/shim.c | 2 +-
> > > xen/common/domain.c | 19 +++++++++----------
> > > xen/drivers/char/console.c | 21 +++++++++++----------
> > > xen/include/xen/sched.h | 24 ++++++++++++++----------
> > > 6 files changed, 45 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > > index 2b6f2a09bca6..f4a840da10c5 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..2be98f6ccbd1 100644
> > > --- a/xen/arch/x86/hvm/hvm.c
> > > +++ b/xen/arch/x86/hvm/hvm.c
> > > @@ -559,7 +559,8 @@ void hvm_do_resume(struct vcpu *v)
> > > static int cf_check hvm_print_line(
> > > int dir, unsigned int port, unsigned int bytes, uint32_t *val)
> > > {
> > > - struct domain *cd = current->domain;
> > > + const struct domain *d = current->domain;
> > > + struct domain_console *cons = d->console;
> > > char c = *val;
> > >
> > > ASSERT(bytes == 1 && port == XEN_HVM_DEBUGCONS_IOPORT);
> > > @@ -571,16 +572,16 @@ static int cf_check hvm_print_line(
> > > if ( !is_console_printable(c) )
> > > return X86EMUL_OKAY;
> > >
> > > - spin_lock(&cd->pbuf_lock);
> > > + spin_lock(&cons->lock);
> > > 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 == (DOMAIN_CONSOLE_BUF_SIZE - 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(d, 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 f54632bc0811..469f5e8832da 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);
> > > @@ -569,7 +569,7 @@ static void console_switch_input(void)
> > > {
> > > rcu_unlock_domain(d);
> > >
> > > - if ( !d->console.input_allowed )
> > > + if ( !d->console->input_allowed )
> > > continue;
> > >
> > > console_rx = next_rx;
> > > @@ -788,6 +788,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
> > > @@ -800,22 +801,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) < (DOMAIN_CONSOLE_BUF_SIZE - 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..8771b7f22b48 100644
> > > --- a/xen/include/xen/sched.h
> > > +++ b/xen/include/xen/sched.h
> > > @@ -371,6 +371,19 @@ struct evtchn_port_ops;
> > >
> > > #define MAX_NR_IOREQ_SERVERS 8
> > >
> > > +#define DOMAIN_CONSOLE_BUF_SIZE (256)
> > > +
> > > +/* 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[DOMAIN_CONSOLE_BUF_SIZE];
> > > +};
> > > +
> > > struct domain
> > > {
> > > domid_t domain_id;
> > > @@ -562,12 +575,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;
> > >
> > > @@ -653,10 +660,7 @@ struct domain
> > > #endif
> > >
> > > /* Console settings. */
> > > - struct {
> > > - /* Permission to take ownership of the physical console input. */
> > > - bool input_allowed;
> > > - } console;
> > > + struct domain_console *console;
> >
> > I assume I'm missing some context, what's the reason for converting
> > this field into a pointer, instead of moving the existing fields
> > inside and allocate the buffer at domain_create()?
>
> The reason is avoid crossing PAGE_SIZE boundary for domain structure *if* more
> console-related fields are added to the "domain console settings" (which will
> happen for console focus management and probably for vUARTs).
Oh, I see, maybe worth adding to the commit message, as otherwise
there's not much context as to why moving into a separately allocated
structure is done.
Thanks, Roger.
© 2016 - 2025 Red Hat, Inc.