Allow multiple dom0less domains to use the console_io hypercalls to
print to the console. Handle them in a similar way to vpl011: only the
domain which has focus can read from the console. All domains can write
to the console but the ones without focus have a prefix. In this case
the prefix is applied by using guest_printk instead of printk or
console_puts which is what the original code was already doing.
When switching focus using Ctrl-AAA, discard any unread data in the
input buffer. Input is read quickly and the user would be aware of it
being slow or stuck as they use Ctrl-AAA to switch focus domain.
In that situation, it is to be expected that the unread input is lost.
The domain writes are buffered when the domain is not in focus. Push out
the buffer after the domain enters focus on the first guest write.
Locking updates:
- Guard every mutation of serial_rx_cons/prod with console_lock and
discard unread input under the lock when switching focus (including when
returning to Xen) so that cross-domain reads can't see stale data
- Require is_focus_domain() callers to hold console_lock, and take that
lock around the entire CONSOLEIO_read loop, re-checking focus after each
chunk so a focus change simply stops further copies without duplicating
or leaking input
- Hold cons->lock while flushing buffered writes in the focus path
so the direct-write fast path does not race buffered guests or HVM
debug output
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
Changes in v7:
- move vpl011 hunk to new patch
- updated commit message
- add ASSERT
- const pointer
- more in-code comments
- move spin_unlock earlier
- code style
---
xen/drivers/char/console.c | 75 +++++++++++++++++++++++++++++++++-----
1 file changed, 65 insertions(+), 10 deletions(-)
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 2bdb4d5fb4..09354db2e0 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -540,6 +540,12 @@ void console_put_domain(struct domain *d)
rcu_unlock_domain(d);
}
+static bool is_focus_domain(const struct domain *d)
+{
+ ASSERT(rspin_is_locked(&console_lock));
+ return d != NULL && d->domain_id == console_rx - 1;
+}
+
static void console_switch_input(void)
{
unsigned int next_rx = console_rx;
@@ -555,8 +561,11 @@ static void console_switch_input(void)
if ( next_rx++ >= max_console_rx )
{
- console_rx = 0;
printk("*** Serial input to Xen");
+ nrspin_lock_irq(&console_lock);
+ console_rx = 0;
+ serial_rx_cons = serial_rx_prod;
+ nrspin_unlock_irq(&console_lock);
break;
}
@@ -575,8 +584,12 @@ static void console_switch_input(void)
rcu_unlock_domain(d);
- console_rx = next_rx;
printk("*** Serial input to DOM%u", domid);
+ nrspin_lock_irq(&console_lock);
+ console_rx = next_rx;
+ /* Don't let the next dom read the previous dom's unread data. */
+ serial_rx_cons = serial_rx_prod;
+ nrspin_unlock_irq(&console_lock);
break;
}
}
@@ -605,8 +618,10 @@ static void __serial_rx(char c)
* Deliver input to the hardware domain buffer, unless it is
* already full.
*/
+ nrspin_lock_irq(&console_lock);
if ( (serial_rx_prod - serial_rx_cons) != SERIAL_RX_SIZE )
serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
+ nrspin_unlock_irq(&console_lock);
/*
* Always notify the hardware domain: prevents receive path from
@@ -730,6 +745,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
unsigned int flags = opt_console_to_ring
? CONSOLE_ALL : CONSOLE_DEFAULT;
struct domain *cd = current->domain;
+ struct domain_console *cons = cd->console;
while ( count > 0 )
{
@@ -742,17 +758,36 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
if ( copy_from_guest(kbuf, buffer, kcount) )
return -EFAULT;
- if ( is_hardware_domain(cd) )
+ /*
+ * Take both cons->lock and console_lock:
+ * - cons->lock protects cons->buf and cons->idx
+ * - console_lock protects console_send and is_focus_domain
+ * checks
+ *
+ * The order must be respected. guest_printk takes the
+ * console_lock so it is important that cons->lock is taken
+ * first.
+ */
+ spin_lock(&cons->lock);
+ nrspin_lock_irq(&console_lock);
+ if ( is_focus_domain(cd) )
{
+ if ( cons->idx )
+ {
+ console_send(cons->buf, cons->idx, flags);
+ cons->idx = 0;
+ }
+ spin_unlock(&cons->lock);
+
/* Use direct console output as it could be interactive */
- nrspin_lock_irq(&console_lock);
console_send(kbuf, kcount, flags);
nrspin_unlock_irq(&console_lock);
}
else
{
char *kin = kbuf, *kout = kbuf, c;
- struct domain_console *cons = cd->console;
+
+ nrspin_unlock_irq(&console_lock);
/* Strip non-printable characters */
do
@@ -765,7 +800,6 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
} while ( --kcount > 0 );
*kout = '\0';
- spin_lock(&cons->lock);
kcount = kin - kbuf;
if ( c != '\n' &&
(cons->idx + (kout - kbuf) < (ARRAY_SIZE(cons->buf) - 1)) )
@@ -815,6 +849,13 @@ long do_console_io(
if ( count > INT_MAX )
break;
+ nrspin_lock_irq(&console_lock);
+ if ( !is_focus_domain(current->domain) )
+ {
+ nrspin_unlock_irq(&console_lock);
+ return 0;
+ }
+
rc = 0;
while ( (serial_rx_cons != serial_rx_prod) && (rc < count) )
{
@@ -824,14 +865,28 @@ long do_console_io(
len = SERIAL_RX_SIZE - idx;
if ( (rc + len) > count )
len = count - rc;
+ nrspin_unlock_irq(&console_lock);
if ( copy_to_guest_offset(buffer, rc, &serial_rx_ring[idx], len) )
- {
- rc = -EFAULT;
- break;
- }
+ return -EFAULT;
rc += len;
+
+ /*
+ * First get console_lock again, then check is_focus_domain.
+ * It is possible that we switched focus domain during
+ * copy_to_guest_offset. In that case, serial_rx_cons and
+ * serial_rx_prod have been reset so it would be wrong to
+ * update serial_rx_cons here. Get out of the loop instead.
+ *
+ * rc is updated regardless to provide the correct return
+ * value to the VM as the data has been copied.
+ */
+ nrspin_lock_irq(&console_lock);
+ if ( !is_focus_domain(current->domain) )
+ break;
+
serial_rx_cons += len;
}
+ nrspin_unlock_irq(&console_lock);
break;
default:
rc = -ENOSYS;
--
2.25.1On 23.01.2026 02:06, Stefano Stabellini wrote:
> @@ -555,8 +561,11 @@ static void console_switch_input(void)
>
> if ( next_rx++ >= max_console_rx )
> {
> - console_rx = 0;
> printk("*** Serial input to Xen");
> + nrspin_lock_irq(&console_lock);
> + console_rx = 0;
> + serial_rx_cons = serial_rx_prod;
> + nrspin_unlock_irq(&console_lock);
> break;
> }
>
> @@ -575,8 +584,12 @@ static void console_switch_input(void)
>
> rcu_unlock_domain(d);
>
> - console_rx = next_rx;
> printk("*** Serial input to DOM%u", domid);
> + nrspin_lock_irq(&console_lock);
> + console_rx = next_rx;
> + /* Don't let the next dom read the previous dom's unread data. */
> + serial_rx_cons = serial_rx_prod;
> + nrspin_unlock_irq(&console_lock);
> break;
> }
As in both cases you're also moving console_rx updates into the locked
region, what about readers of the variable, first and foremost
console_get_domain()? Else why the movement?
Also I think that the printk()s would better be last, indicating the
completion of the switching.
> @@ -605,8 +618,10 @@ static void __serial_rx(char c)
> * Deliver input to the hardware domain buffer, unless it is
> * already full.
> */
This comment goes stale with the buffer being used for whichever is the
focus domain in do_console_io().
> + nrspin_lock_irq(&console_lock);
> if ( (serial_rx_prod - serial_rx_cons) != SERIAL_RX_SIZE )
> serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
> + nrspin_unlock_irq(&console_lock);
I don't think you can unconditionally enable interrupts here, as this may
be running in the context of an IRQ handler.
> @@ -742,17 +758,36 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
> if ( copy_from_guest(kbuf, buffer, kcount) )
> return -EFAULT;
>
> - if ( is_hardware_domain(cd) )
> + /*
> + * Take both cons->lock and console_lock:
> + * - cons->lock protects cons->buf and cons->idx
> + * - console_lock protects console_send and is_focus_domain
> + * checks
> + *
> + * The order must be respected. guest_printk takes the
> + * console_lock so it is important that cons->lock is taken
> + * first.
> + */
> + spin_lock(&cons->lock);
> + nrspin_lock_irq(&console_lock);
> + if ( is_focus_domain(cd) )
Why would any of the domains possibly being permitted to be "focus" suddenly
gain direct access here? Full access in do_console_io() is still prevented by
the XSM check there, afaict. Cc-ing Daniel, as it's not quite clear to me
whether to introduce another XSM check here, whether to check ->is_console
directly, or yet something else.
> {
> + if ( cons->idx )
> + {
> + console_send(cons->buf, cons->idx, flags);
> + cons->idx = 0;
> + }
> + spin_unlock(&cons->lock);
> +
> /* Use direct console output as it could be interactive */
> - nrspin_lock_irq(&console_lock);
> console_send(kbuf, kcount, flags);
> nrspin_unlock_irq(&console_lock);
> }
> else
> {
> char *kin = kbuf, *kout = kbuf, c;
> - struct domain_console *cons = cd->console;
> +
> + nrspin_unlock_irq(&console_lock);
>
> /* Strip non-printable characters */
> do
> @@ -765,7 +800,6 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
> } while ( --kcount > 0 );
>
> *kout = '\0';
> - spin_lock(&cons->lock);
> kcount = kin - kbuf;
> if ( c != '\n' &&
> (cons->idx + (kout - kbuf) < (ARRAY_SIZE(cons->buf) - 1)) )
> @@ -815,6 +849,13 @@ long do_console_io(
> if ( count > INT_MAX )
> break;
>
> + nrspin_lock_irq(&console_lock);
> + if ( !is_focus_domain(current->domain) )
> + {
> + nrspin_unlock_irq(&console_lock);
> + return 0;
> + }
To avoid the extra unlock-and-return, how about simply setting "count" to 0,
leveraging ...
> rc = 0;
> while ( (serial_rx_cons != serial_rx_prod) && (rc < count) )
... the rhs check here?
> @@ -824,14 +865,28 @@ long do_console_io(
> len = SERIAL_RX_SIZE - idx;
> if ( (rc + len) > count )
> len = count - rc;
> + nrspin_unlock_irq(&console_lock);
This can be moved up a few lines, as none of the local variable manipulations
needs guarding.
> if ( copy_to_guest_offset(buffer, rc, &serial_rx_ring[idx], len) )
> - {
> - rc = -EFAULT;
> - break;
> - }
> + return -EFAULT;
> rc += len;
> +
> + /*
> + * First get console_lock again, then check is_focus_domain.
> + * It is possible that we switched focus domain during
> + * copy_to_guest_offset. In that case, serial_rx_cons and
Please can you append () to the function name, to identify it as a function,
as opposed to ...
> + * serial_rx_prod have been reset so it would be wrong to
> + * update serial_rx_cons here. Get out of the loop instead.
... the two variables referenced here?
Jan
On Wed, 28 Jan 2026, Jan Beulich wrote:
> On 23.01.2026 02:06, Stefano Stabellini wrote:
> > @@ -555,8 +561,11 @@ static void console_switch_input(void)
> >
> > if ( next_rx++ >= max_console_rx )
> > {
> > - console_rx = 0;
> > printk("*** Serial input to Xen");
> > + nrspin_lock_irq(&console_lock);
> > + console_rx = 0;
> > + serial_rx_cons = serial_rx_prod;
> > + nrspin_unlock_irq(&console_lock);
> > break;
> > }
> >
> > @@ -575,8 +584,12 @@ static void console_switch_input(void)
> >
> > rcu_unlock_domain(d);
> >
> > - console_rx = next_rx;
> > printk("*** Serial input to DOM%u", domid);
> > + nrspin_lock_irq(&console_lock);
> > + console_rx = next_rx;
> > + /* Don't let the next dom read the previous dom's unread data. */
> > + serial_rx_cons = serial_rx_prod;
> > + nrspin_unlock_irq(&console_lock);
> > break;
> > }
>
> As in both cases you're also moving console_rx updates into the locked
> region, what about readers of the variable, first and foremost
> console_get_domain()? Else why the movement?
You're right. I've added locking in console_get_domain() to read
console_rx under console_lock into a local variable, ensuring
consistency with the now-locked writes.
> Also I think that the printk()s would better be last, indicating the
> completion of the switching.
OK
> > @@ -605,8 +618,10 @@ static void __serial_rx(char c)
> > * Deliver input to the hardware domain buffer, unless it is
> > * already full.
> > */
>
> This comment goes stale with the buffer being used for whichever is the
> focus domain in do_console_io().
>
> > + nrspin_lock_irq(&console_lock);
> > if ( (serial_rx_prod - serial_rx_cons) != SERIAL_RX_SIZE )
> > serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
> > + nrspin_unlock_irq(&console_lock);
>
> I don't think you can unconditionally enable interrupts here, as this may
> be running in the context of an IRQ handler.
Good catch! I've changed __serial_rx() to use nrspin_lock_irqsave()/
nrspin_unlock_irqrestore() instead of
nrspin_lock_irq()/nrspin_unlock_irq().
> > @@ -742,17 +758,36 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
> > if ( copy_from_guest(kbuf, buffer, kcount) )
> > return -EFAULT;
> >
> > - if ( is_hardware_domain(cd) )
> > + /*
> > + * Take both cons->lock and console_lock:
> > + * - cons->lock protects cons->buf and cons->idx
> > + * - console_lock protects console_send and is_focus_domain
> > + * checks
> > + *
> > + * The order must be respected. guest_printk takes the
> > + * console_lock so it is important that cons->lock is taken
> > + * first.
> > + */
> > + spin_lock(&cons->lock);
> > + nrspin_lock_irq(&console_lock);
> > + if ( is_focus_domain(cd) )
>
> Why would any of the domains possibly being permitted to be "focus" suddenly
> gain direct access here? Full access in do_console_io() is still prevented by
> the XSM check there, afaict. Cc-ing Daniel, as it's not quite clear to me
> whether to introduce another XSM check here, whether to check ->is_console
> directly, or yet something else.
The XSM check still happens first in do_console_io() via
xsm_console_io(XSM_OTHER, current->domain, cmd), which validates that
the domain has permission to use console_io hypercalls. The focus check
is an additional restriction that only allows reading when the domain
has focus: it doesn't grant new permissions. Dom0less domains with
input_allowed = true are already permitted by XSM policy to use
console_io; the focus mechanism just ensures only one domain can read
input at a time (similar to vpl011 behavior). Additionally, this is also
allowed for dom0less domains which are quite special and manually
configured at boot by the user. There are no arbitrary unknown dom0less
domains that can be started later.
> > {
> > + if ( cons->idx )
> > + {
> > + console_send(cons->buf, cons->idx, flags);
> > + cons->idx = 0;
> > + }
> > + spin_unlock(&cons->lock);
> > +
> > /* Use direct console output as it could be interactive */
> > - nrspin_lock_irq(&console_lock);
> > console_send(kbuf, kcount, flags);
> > nrspin_unlock_irq(&console_lock);
> > }
> > else
> > {
> > char *kin = kbuf, *kout = kbuf, c;
> > - struct domain_console *cons = cd->console;
> > +
> > + nrspin_unlock_irq(&console_lock);
> >
> > /* Strip non-printable characters */
> > do
> > @@ -765,7 +800,6 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
> > } while ( --kcount > 0 );
> >
> > *kout = '\0';
> > - spin_lock(&cons->lock);
> > kcount = kin - kbuf;
> > if ( c != '\n' &&
> > (cons->idx + (kout - kbuf) < (ARRAY_SIZE(cons->buf) - 1)) )
> > @@ -815,6 +849,13 @@ long do_console_io(
> > if ( count > INT_MAX )
> > break;
> >
> > + nrspin_lock_irq(&console_lock);
> > + if ( !is_focus_domain(current->domain) )
> > + {
> > + nrspin_unlock_irq(&console_lock);
> > + return 0;
> > + }
>
> To avoid the extra unlock-and-return, how about simply setting "count" to 0,
> leveraging ...
Good idea.
> > rc = 0;
> > while ( (serial_rx_cons != serial_rx_prod) && (rc < count) )
>
> ... the rhs check here?
>
> > @@ -824,14 +865,28 @@ long do_console_io(
> > len = SERIAL_RX_SIZE - idx;
> > if ( (rc + len) > count )
> > len = count - rc;
> > + nrspin_unlock_irq(&console_lock);
>
> This can be moved up a few lines, as none of the local variable manipulations
> needs guarding.
OK
> > if ( copy_to_guest_offset(buffer, rc, &serial_rx_ring[idx], len) )
> > - {
> > - rc = -EFAULT;
> > - break;
> > - }
> > + return -EFAULT;
> > rc += len;
> > +
> > + /*
> > + * First get console_lock again, then check is_focus_domain.
> > + * It is possible that we switched focus domain during
> > + * copy_to_guest_offset. In that case, serial_rx_cons and
>
> Please can you append () to the function name, to identify it as a function,
> as opposed to ...
>
> > + * serial_rx_prod have been reset so it would be wrong to
> > + * update serial_rx_cons here. Get out of the loop instead.
>
> ... the two variables referenced here?
Done
On 29.01.2026 03:42, Stefano Stabellini wrote: > On Wed, 28 Jan 2026, Jan Beulich wrote: >> On 23.01.2026 02:06, Stefano Stabellini wrote: >>> @@ -742,17 +758,36 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, >>> if ( copy_from_guest(kbuf, buffer, kcount) ) >>> return -EFAULT; >>> >>> - if ( is_hardware_domain(cd) ) >>> + /* >>> + * Take both cons->lock and console_lock: >>> + * - cons->lock protects cons->buf and cons->idx >>> + * - console_lock protects console_send and is_focus_domain >>> + * checks >>> + * >>> + * The order must be respected. guest_printk takes the >>> + * console_lock so it is important that cons->lock is taken >>> + * first. >>> + */ >>> + spin_lock(&cons->lock); >>> + nrspin_lock_irq(&console_lock); >>> + if ( is_focus_domain(cd) ) >> >> Why would any of the domains possibly being permitted to be "focus" suddenly >> gain direct access here? Full access in do_console_io() is still prevented by >> the XSM check there, afaict. Cc-ing Daniel, as it's not quite clear to me >> whether to introduce another XSM check here, whether to check ->is_console >> directly, or yet something else. > > The XSM check still happens first in do_console_io() via > xsm_console_io(XSM_OTHER, current->domain, cmd), which validates that > the domain has permission to use console_io hypercalls. The focus check > is an additional restriction that only allows reading when the domain > has focus: it doesn't grant new permissions. Dom0less domains with > input_allowed = true are already permitted by XSM policy to use > console_io; Are they? I don't see any XSM or Flask code checking that flag. What the dummy xsm_console_io() checks is ->is_console. However, what indeed I didn't pay attention to when writing the original comment is that guest_console_write() has only a single caller, do_console_io(). So there's no concern in this regard here as long as no new caller appears. Jan
On 1/29/26 3:24 AM, Jan Beulich wrote: > On 29.01.2026 03:42, Stefano Stabellini wrote: >> On Wed, 28 Jan 2026, Jan Beulich wrote: >>> On 23.01.2026 02:06, Stefano Stabellini wrote: >>>> @@ -742,17 +758,36 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, >>>> if ( copy_from_guest(kbuf, buffer, kcount) ) >>>> return -EFAULT; >>>> >>>> - if ( is_hardware_domain(cd) ) >>>> + /* >>>> + * Take both cons->lock and console_lock: >>>> + * - cons->lock protects cons->buf and cons->idx >>>> + * - console_lock protects console_send and is_focus_domain >>>> + * checks >>>> + * >>>> + * The order must be respected. guest_printk takes the >>>> + * console_lock so it is important that cons->lock is taken >>>> + * first. >>>> + */ >>>> + spin_lock(&cons->lock); >>>> + nrspin_lock_irq(&console_lock); >>>> + if ( is_focus_domain(cd) ) >>> >>> Why would any of the domains possibly being permitted to be "focus" suddenly >>> gain direct access here? Full access in do_console_io() is still prevented by >>> the XSM check there, afaict. Cc-ing Daniel, as it's not quite clear to me >>> whether to introduce another XSM check here, whether to check ->is_console >>> directly, or yet something else. >> >> The XSM check still happens first in do_console_io() via >> xsm_console_io(XSM_OTHER, current->domain, cmd), which validates that >> the domain has permission to use console_io hypercalls. The focus check >> is an additional restriction that only allows reading when the domain >> has focus: it doesn't grant new permissions. Dom0less domains with >> input_allowed = true are already permitted by XSM policy to use >> console_io; > > Are they? I don't see any XSM or Flask code checking that flag. What the > dummy xsm_console_io() checks is ->is_console. Unless I am misunderstanding what you are asking here, I don't see why XSM would be concerned with this check. The `is_focus_domain()` conditional is not an access decision but a decision whether write to the console or buffer the write. > However, what indeed I didn't pay attention to when writing the original > comment is that guest_console_write() has only a single caller, > do_console_io(). So there's no concern in this regard here as long as no > new caller appears. Correct, the `xsm_console_io()` hook is the access check if the guest is allowed to read/write to the console. Any paths to this function should be guarded by a call to this hook. v/r, dps
© 2016 - 2026 Red Hat, Inc.