* [PATCH v10 1/5] xen/console: use ACCESS_ONCE for console_rx
2026-02-04 23:35 [PATCH v10 0/5] xen: console_io for dom0less guests Stefano Stabellini
@ 2026-02-04 23:37 ` Stefano Stabellini
2026-02-05 2:13 ` dmukhin
2026-02-04 23:37 ` [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding Stefano Stabellini
` (3 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Stefano Stabellini @ 2026-02-04 23:37 UTC (permalink / raw)
To: xen-devel
Cc: grygorii_strashko, anthony.perard, michal.orzel, julien,
roger.pau, jason.andryuk, victorm.lira, andrew.cooper3, jbeulich,
sstabellini, Stefano Stabellini
There can be concurrent reads and writes to the console_rx variable so
it is prudent to use ACCESS_ONCE.
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
xen/drivers/char/console.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 2bdb4d5fb4..35f541ca8e 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -518,11 +518,12 @@ static unsigned int __read_mostly console_rx = 0;
struct domain *console_get_domain(void)
{
struct domain *d;
+ unsigned int rx = ACCESS_ONCE(console_rx);
- if ( console_rx == 0 )
- return NULL;
+ if ( rx == 0 )
+ return NULL;
- d = rcu_lock_domain_by_id(console_rx - 1);
+ d = rcu_lock_domain_by_id(rx - 1);
if ( !d )
return NULL;
@@ -542,7 +543,7 @@ void console_put_domain(struct domain *d)
static void console_switch_input(void)
{
- unsigned int next_rx = console_rx;
+ unsigned int next_rx = ACCESS_ONCE(console_rx);
/*
* Rotate among Xen, dom0 and boot-time created domUs while skipping
@@ -555,7 +556,7 @@ static void console_switch_input(void)
if ( next_rx++ >= max_console_rx )
{
- console_rx = 0;
+ ACCESS_ONCE(console_rx) = 0;
printk("*** Serial input to Xen");
break;
}
@@ -575,7 +576,7 @@ static void console_switch_input(void)
rcu_unlock_domain(d);
- console_rx = next_rx;
+ ACCESS_ONCE(console_rx) = next_rx;
printk("*** Serial input to DOM%u", domid);
break;
}
@@ -592,7 +593,7 @@ static void __serial_rx(char c)
struct domain *d;
int rc = 0;
- if ( console_rx == 0 )
+ if ( ACCESS_ONCE(console_rx) == 0 )
return handle_keypress(c, false);
d = console_get_domain();
@@ -1193,7 +1194,7 @@ void __init console_endboot(void)
* a useful 'how to switch' message.
*/
if ( opt_conswitch[1] == 'x' )
- console_rx = max_console_rx;
+ ACCESS_ONCE(console_rx) = max_console_rx;
register_keyhandler('w', conring_dump_keyhandler,
"synchronously dump console ring buffer (dmesg)", 0);
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v10 1/5] xen/console: use ACCESS_ONCE for console_rx
2026-02-04 23:37 ` [PATCH v10 1/5] xen/console: use ACCESS_ONCE for console_rx Stefano Stabellini
@ 2026-02-05 2:13 ` dmukhin
0 siblings, 0 replies; 23+ messages in thread
From: dmukhin @ 2026-02-05 2:13 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, grygorii_strashko, anthony.perard, michal.orzel,
julien, roger.pau, jason.andryuk, victorm.lira, andrew.cooper3,
jbeulich, sstabellini
On Wed, Feb 04, 2026 at 03:37:08PM -0800, Stefano Stabellini wrote:
> There can be concurrent reads and writes to the console_rx variable so
> it is prudent to use ACCESS_ONCE.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding
2026-02-04 23:35 [PATCH v10 0/5] xen: console_io for dom0less guests Stefano Stabellini
2026-02-04 23:37 ` [PATCH v10 1/5] xen/console: use ACCESS_ONCE for console_rx Stefano Stabellini
@ 2026-02-04 23:37 ` Stefano Stabellini
2026-02-06 0:50 ` dmukhin
2026-02-09 9:13 ` Jan Beulich
2026-02-04 23:37 ` [PATCH v10 3/5] xen/console: add locking for serial_rx ring buffer access Stefano Stabellini
` (2 subsequent siblings)
4 siblings, 2 replies; 23+ messages in thread
From: Stefano Stabellini @ 2026-02-04 23:37 UTC (permalink / raw)
To: xen-devel
Cc: grygorii_strashko, anthony.perard, michal.orzel, julien,
roger.pau, jason.andryuk, victorm.lira, andrew.cooper3, jbeulich,
sstabellini, Stefano Stabellini
Today only hwdom can bind VIRQ_CONSOLE. This patch changes the virq from
global to VIRQ_DOMAIN to allow other domains to bind to it.
Note that Linux silently falls back to polling when binding fails, which
masks the issue.
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
Changes in v10:
- update comment in xen/include/public/xen.h
---
xen/common/event_channel.c | 1 +
xen/drivers/char/console.c | 2 +-
xen/include/public/xen.h | 2 +-
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index a3d18bc464..f9becb4ca5 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -138,6 +138,7 @@ static enum virq_type get_virq_type(unsigned int virq)
return VIRQ_VCPU;
case VIRQ_ARGO:
+ case VIRQ_CONSOLE:
return VIRQ_DOMAIN;
case VIRQ_ARCH_0 ... VIRQ_ARCH_7:
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 35f541ca8e..fbc89ca2a4 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -613,7 +613,7 @@ static void __serial_rx(char c)
* Always notify the hardware domain: prevents receive path from
* getting stuck.
*/
- send_global_virq(VIRQ_CONSOLE);
+ send_guest_domain_virq(d, VIRQ_CONSOLE);
}
#ifdef CONFIG_SBSA_VUART_CONSOLE
else
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index b12fd10e63..b8146bd00a 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -176,7 +176,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
/* ` enum virq { */
#define VIRQ_TIMER 0 /* V. Timebase update, and/or requested timeout. */
#define VIRQ_DEBUG 1 /* V. Request guest to dump debug info. */
-#define VIRQ_CONSOLE 2 /* G. Bytes received on emergency console. */
+#define VIRQ_CONSOLE 2 /* D. Bytes received on Xen console. */
#define VIRQ_DOM_EXC 3 /* G. Exceptional event for some domain. */
#define VIRQ_TBUF 4 /* G. Trace buffer has records available. */
#define VIRQ_DEBUGGER 6 /* G. A domain has paused for debugging. */
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding
2026-02-04 23:37 ` [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding Stefano Stabellini
@ 2026-02-06 0:50 ` dmukhin
2026-02-09 9:13 ` Jan Beulich
1 sibling, 0 replies; 23+ messages in thread
From: dmukhin @ 2026-02-06 0:50 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, grygorii_strashko, anthony.perard, michal.orzel,
julien, roger.pau, jason.andryuk, victorm.lira, andrew.cooper3,
jbeulich, sstabellini
On Wed, Feb 04, 2026 at 03:37:09PM -0800, Stefano Stabellini wrote:
> Today only hwdom can bind VIRQ_CONSOLE. This patch changes the virq from
> global to VIRQ_DOMAIN to allow other domains to bind to it.
>
> Note that Linux silently falls back to polling when binding fails, which
> masks the issue.
>
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding
2026-02-04 23:37 ` [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding Stefano Stabellini
2026-02-06 0:50 ` dmukhin
@ 2026-02-09 9:13 ` Jan Beulich
2026-02-09 23:23 ` Stefano Stabellini
1 sibling, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2026-02-09 9:13 UTC (permalink / raw)
To: Stefano Stabellini
Cc: grygorii_strashko, anthony.perard, michal.orzel, julien,
roger.pau, jason.andryuk, victorm.lira, andrew.cooper3,
sstabellini, xen-devel
On 05.02.2026 00:37, Stefano Stabellini wrote:
> Today only hwdom can bind VIRQ_CONSOLE. This patch changes the virq from
> global to VIRQ_DOMAIN to allow other domains to bind to it.
>
> Note that Linux silently falls back to polling when binding fails, which
> masks the issue.
>
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Technically this is an ABI change, and hence I'm uncertain it can go without
that aspect being at least mentioned, perhaps even its implications properly
discussed.
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding
2026-02-09 9:13 ` Jan Beulich
@ 2026-02-09 23:23 ` Stefano Stabellini
2026-02-10 7:47 ` Jan Beulich
0 siblings, 1 reply; 23+ messages in thread
From: Stefano Stabellini @ 2026-02-09 23:23 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, grygorii_strashko, anthony.perard,
michal.orzel, julien, roger.pau, jason.andryuk, victorm.lira,
andrew.cooper3, sstabellini, xen-devel
On Mon, 9 Feb 2026, Jan Beulich wrote:
> On 05.02.2026 00:37, Stefano Stabellini wrote:
> > Today only hwdom can bind VIRQ_CONSOLE. This patch changes the virq from
> > global to VIRQ_DOMAIN to allow other domains to bind to it.
> >
> > Note that Linux silently falls back to polling when binding fails, which
> > masks the issue.
> >
> > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>
> Technically this is an ABI change, and hence I'm uncertain it can go without
> that aspect being at least mentioned, perhaps even its implications properly
> discussed.
I am not sure if it qualifies as an ABI change or not but I am happy to
expand the commit message in any way you might suggest.
The jist of it is already in the commit message, really the key element
is that VIRQ_CONSOLE can be bound by multiple domains.
Aside from spelling out "this is an ABI change" what do you have in
mind?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding
2026-02-09 23:23 ` Stefano Stabellini
@ 2026-02-10 7:47 ` Jan Beulich
2026-02-13 20:09 ` Stefano Stabellini
0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2026-02-10 7:47 UTC (permalink / raw)
To: Stefano Stabellini, Daniel Smith
Cc: Stefano Stabellini, grygorii_strashko, anthony.perard,
michal.orzel, julien, roger.pau, jason.andryuk, victorm.lira,
andrew.cooper3, xen-devel
On 10.02.2026 00:23, Stefano Stabellini wrote:
> On Mon, 9 Feb 2026, Jan Beulich wrote:
>> On 05.02.2026 00:37, Stefano Stabellini wrote:
>>> Today only hwdom can bind VIRQ_CONSOLE. This patch changes the virq from
>>> global to VIRQ_DOMAIN to allow other domains to bind to it.
>>>
>>> Note that Linux silently falls back to polling when binding fails, which
>>> masks the issue.
>>>
>>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>
>> Technically this is an ABI change, and hence I'm uncertain it can go without
>> that aspect being at least mentioned, perhaps even its implications properly
>> discussed.
>
> I am not sure if it qualifies as an ABI change or not but I am happy to
> expand the commit message in any way you might suggest.
>
> The jist of it is already in the commit message, really the key element
> is that VIRQ_CONSOLE can be bound by multiple domains.
>
> Aside from spelling out "this is an ABI change" what do you have in
> mind?
What I mean is discussion of the implications for domains using the vIRQ.
Previously most domains would have attempts to bind this vIRQ rejected.
Technically it is possible that kernels had code paths blindly doing the
binding, relying on it to work only when running as Dom0. And really, you
appear to break XEN_DOMCTL_set_virq_handler when used with VIRQ_CONSOLE,
without which its binding wasn't possible at all before (except for the
hardware domain, which get_global_virq_handler() falls back to when no
other domain is set). Or am I mis-reading things, as I can't spot any use
of VIRQ_CONSOLE under tools/, whereas I would have expected provisions
for (host) console handling to be delegated to a separate control or
console domain? Of course other toolstacks (the XAPI-based one for
example) might actually have such provisions.
And then there's the XSM question: XEN_DOMCTL_set_virq_handler obviously
is subject to XSM checking. The same isn't true for VIRQ_DOMAIN-type
vIRQ-s. Yet this vIRQ isn't supposed to be universally available to
every DomU. Instead the ->console->input_allowed checking is kind of
substituting such a check, which iirc Daniel said (in more general
context) shouldn't ever be done. IOW in patch 5 you're actually effecting
policy, which should be XSM's job aiui.
Bottom line: The patch may need to be more involved, but at the very
least the description would need updating to justify it being as simple
as it is right now.
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding
2026-02-10 7:47 ` Jan Beulich
@ 2026-02-13 20:09 ` Stefano Stabellini
2026-02-16 9:04 ` Jan Beulich
2026-02-18 15:07 ` Daniel P. Smith
0 siblings, 2 replies; 23+ messages in thread
From: Stefano Stabellini @ 2026-02-13 20:09 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Daniel Smith, Stefano Stabellini,
grygorii_strashko, anthony.perard, michal.orzel, julien,
roger.pau, jason.andryuk, victorm.lira, andrew.cooper3, xen-devel
On Tue, 10 Feb 2026, Jan Beulich wrote:
> On 10.02.2026 00:23, Stefano Stabellini wrote:
> > On Mon, 9 Feb 2026, Jan Beulich wrote:
> >> On 05.02.2026 00:37, Stefano Stabellini wrote:
> >>> Today only hwdom can bind VIRQ_CONSOLE. This patch changes the virq from
> >>> global to VIRQ_DOMAIN to allow other domains to bind to it.
> >>>
> >>> Note that Linux silently falls back to polling when binding fails, which
> >>> masks the issue.
> >>>
> >>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> >>
> >> Technically this is an ABI change, and hence I'm uncertain it can go without
> >> that aspect being at least mentioned, perhaps even its implications properly
> >> discussed.
> >
> > I am not sure if it qualifies as an ABI change or not but I am happy to
> > expand the commit message in any way you might suggest.
> >
> > The jist of it is already in the commit message, really the key element
> > is that VIRQ_CONSOLE can be bound by multiple domains.
> >
> > Aside from spelling out "this is an ABI change" what do you have in
> > mind?
>
> What I mean is discussion of the implications for domains using the vIRQ.
> Previously most domains would have attempts to bind this vIRQ rejected.
> Technically it is possible that kernels had code paths blindly doing the
> binding, relying on it to work only when running as Dom0. And really, you
> appear to break XEN_DOMCTL_set_virq_handler when used with VIRQ_CONSOLE,
> without which its binding wasn't possible at all before (except for the
> hardware domain, which get_global_virq_handler() falls back to when no
> other domain is set). Or am I mis-reading things, as I can't spot any use
> of VIRQ_CONSOLE under tools/, whereas I would have expected provisions
> for (host) console handling to be delegated to a separate control or
> console domain? Of course other toolstacks (the XAPI-based one for
> example) might actually have such provisions.
>
> And then there's the XSM question: XEN_DOMCTL_set_virq_handler obviously
> is subject to XSM checking. The same isn't true for VIRQ_DOMAIN-type
> vIRQ-s. Yet this vIRQ isn't supposed to be universally available to
> every DomU. Instead the ->console->input_allowed checking is kind of
> substituting such a check, which iirc Daniel said (in more general
> context) shouldn't ever be done. IOW in patch 5 you're actually effecting
> policy, which should be XSM's job aiui.
>
> Bottom line: The patch may need to be more involved, but at the very
> least the description would need updating to justify it being as simple
> as it is right now.
What do you think of this:
---
xen/console: change VIRQ_CONSOLE from global to per-domain
Previously VIRQ_CONSOLE was a global VIRQ (VIRQ_GLOBAL type), meaning
only the hardware domain (or a domain explicitly set via
XEN_DOMCTL_set_virq_handler) could bind it. Any other domain attempting
to bind would fail with -EBUSY because get_global_virq_handler() would
return hwdom by default.
This patch changes VIRQ_CONSOLE to VIRQ_DOMAIN type, allowing any domain
to bind it independently, similar to VIRQ_ARGO. The console notification
is now sent via send_guest_domain_virq() directly to the focus domain
rather than through send_global_virq().
Implications:
1. Guest kernels that previously called bind on VIRQ_CONSOLE blindly
will now succeed. Linux handles binding failure gracefully by falling
back to polling, so this should not cause regressions.
2. XEN_DOMCTL_set_virq_handler can no longer be used with VIRQ_CONSOLE.
The domctl explicitly rejects non-VIRQ_GLOBAL types. This means
toolstacks that relied on set_virq_handler to delegate console handling
to a separate console domain will need to use a different mechanism.
Note: No known in-tree toolstack uses set_virq_handler with VIRQ_CONSOLE.
3. VIRQ_DOMAIN bindings are not subject to XSM checks beyond the
standard event channel allocation policy. Access control for console
input is enforced via the per-domain console->input_allowed flag,
which is set for:
- The hardware domain (by default in domain_create())
- dom0less domains on ARM (in construct_domU())
- The PV shim domain on x86 (in pv_shim_setup_dom())
- Domains with vpl011 using the Xen backend (in domain_vpl011_init())
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding
2026-02-13 20:09 ` Stefano Stabellini
@ 2026-02-16 9:04 ` Jan Beulich
2026-02-18 15:07 ` Daniel P. Smith
1 sibling, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2026-02-16 9:04 UTC (permalink / raw)
To: Stefano Stabellini, Daniel Smith
Cc: Stefano Stabellini, grygorii_strashko, anthony.perard,
michal.orzel, julien, roger.pau, jason.andryuk, victorm.lira,
andrew.cooper3, xen-devel
On 13.02.2026 21:09, Stefano Stabellini wrote:
> On Tue, 10 Feb 2026, Jan Beulich wrote:
>> On 10.02.2026 00:23, Stefano Stabellini wrote:
>>> On Mon, 9 Feb 2026, Jan Beulich wrote:
>>>> On 05.02.2026 00:37, Stefano Stabellini wrote:
>>>>> Today only hwdom can bind VIRQ_CONSOLE. This patch changes the virq from
>>>>> global to VIRQ_DOMAIN to allow other domains to bind to it.
>>>>>
>>>>> Note that Linux silently falls back to polling when binding fails, which
>>>>> masks the issue.
>>>>>
>>>>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>>>
>>>> Technically this is an ABI change, and hence I'm uncertain it can go without
>>>> that aspect being at least mentioned, perhaps even its implications properly
>>>> discussed.
>>>
>>> I am not sure if it qualifies as an ABI change or not but I am happy to
>>> expand the commit message in any way you might suggest.
>>>
>>> The jist of it is already in the commit message, really the key element
>>> is that VIRQ_CONSOLE can be bound by multiple domains.
>>>
>>> Aside from spelling out "this is an ABI change" what do you have in
>>> mind?
>>
>> What I mean is discussion of the implications for domains using the vIRQ.
>> Previously most domains would have attempts to bind this vIRQ rejected.
>> Technically it is possible that kernels had code paths blindly doing the
>> binding, relying on it to work only when running as Dom0. And really, you
>> appear to break XEN_DOMCTL_set_virq_handler when used with VIRQ_CONSOLE,
>> without which its binding wasn't possible at all before (except for the
>> hardware domain, which get_global_virq_handler() falls back to when no
>> other domain is set). Or am I mis-reading things, as I can't spot any use
>> of VIRQ_CONSOLE under tools/, whereas I would have expected provisions
>> for (host) console handling to be delegated to a separate control or
>> console domain? Of course other toolstacks (the XAPI-based one for
>> example) might actually have such provisions.
>>
>> And then there's the XSM question: XEN_DOMCTL_set_virq_handler obviously
>> is subject to XSM checking. The same isn't true for VIRQ_DOMAIN-type
>> vIRQ-s. Yet this vIRQ isn't supposed to be universally available to
>> every DomU. Instead the ->console->input_allowed checking is kind of
>> substituting such a check, which iirc Daniel said (in more general
>> context) shouldn't ever be done. IOW in patch 5 you're actually effecting
>> policy, which should be XSM's job aiui.
>>
>> Bottom line: The patch may need to be more involved, but at the very
>> least the description would need updating to justify it being as simple
>> as it is right now.
>
> What do you think of this:
Quite a bit better, yet for me at least not something I would feel happy
to take as a basis for an ack.
> ---
>
> xen/console: change VIRQ_CONSOLE from global to per-domain
>
> Previously VIRQ_CONSOLE was a global VIRQ (VIRQ_GLOBAL type), meaning
> only the hardware domain (or a domain explicitly set via
> XEN_DOMCTL_set_virq_handler) could bind it. Any other domain attempting
> to bind would fail with -EBUSY because get_global_virq_handler() would
> return hwdom by default.
>
> This patch changes VIRQ_CONSOLE to VIRQ_DOMAIN type, allowing any domain
> to bind it independently, similar to VIRQ_ARGO. The console notification
> is now sent via send_guest_domain_virq() directly to the focus domain
> rather than through send_global_virq().
>
> Implications:
>
> 1. Guest kernels that previously called bind on VIRQ_CONSOLE blindly
> will now succeed. Linux handles binding failure gracefully by falling
> back to polling, so this should not cause regressions.
>
> 2. XEN_DOMCTL_set_virq_handler can no longer be used with VIRQ_CONSOLE.
> The domctl explicitly rejects non-VIRQ_GLOBAL types. This means
> toolstacks that relied on set_virq_handler to delegate console handling
> to a separate console domain will need to use a different mechanism.
> Note: No known in-tree toolstack uses set_virq_handler with VIRQ_CONSOLE.
XAPI at the very least would want checking here, imo.
> 3. VIRQ_DOMAIN bindings are not subject to XSM checks beyond the
> standard event channel allocation policy. Access control for console
> input is enforced via the per-domain console->input_allowed flag,
> which is set for:
> - The hardware domain (by default in domain_create())
> - dom0less domains on ARM (in construct_domU())
> - The PV shim domain on x86 (in pv_shim_setup_dom())
> - Domains with vpl011 using the Xen backend (in domain_vpl011_init())
Daniel, can you please take a look from (conceptual) XSM/Flask perspective?
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding
2026-02-13 20:09 ` Stefano Stabellini
2026-02-16 9:04 ` Jan Beulich
@ 2026-02-18 15:07 ` Daniel P. Smith
2026-02-18 15:14 ` Jan Beulich
1 sibling, 1 reply; 23+ messages in thread
From: Daniel P. Smith @ 2026-02-18 15:07 UTC (permalink / raw)
To: Stefano Stabellini, Jan Beulich
Cc: Stefano Stabellini, grygorii_strashko, anthony.perard,
michal.orzel, julien, roger.pau, jason.andryuk, victorm.lira,
andrew.cooper3, xen-devel
On 2/13/26 15:09, Stefano Stabellini wrote:
> On Tue, 10 Feb 2026, Jan Beulich wrote:
>> On 10.02.2026 00:23, Stefano Stabellini wrote:
>>> On Mon, 9 Feb 2026, Jan Beulich wrote:
>>>> On 05.02.2026 00:37, Stefano Stabellini wrote:
>>>>> Today only hwdom can bind VIRQ_CONSOLE. This patch changes the virq from
>>>>> global to VIRQ_DOMAIN to allow other domains to bind to it.
>>>>>
>>>>> Note that Linux silently falls back to polling when binding fails, which
>>>>> masks the issue.
>>>>>
>>>>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>>>
>>>> Technically this is an ABI change, and hence I'm uncertain it can go without
>>>> that aspect being at least mentioned, perhaps even its implications properly
>>>> discussed.
>>>
>>> I am not sure if it qualifies as an ABI change or not but I am happy to
>>> expand the commit message in any way you might suggest.
>>>
>>> The jist of it is already in the commit message, really the key element
>>> is that VIRQ_CONSOLE can be bound by multiple domains.
>>>
>>> Aside from spelling out "this is an ABI change" what do you have in
>>> mind?
>>
>> What I mean is discussion of the implications for domains using the vIRQ.
>> Previously most domains would have attempts to bind this vIRQ rejected.
>> Technically it is possible that kernels had code paths blindly doing the
>> binding, relying on it to work only when running as Dom0. And really, you
>> appear to break XEN_DOMCTL_set_virq_handler when used with VIRQ_CONSOLE,
>> without which its binding wasn't possible at all before (except for the
>> hardware domain, which get_global_virq_handler() falls back to when no
>> other domain is set). Or am I mis-reading things, as I can't spot any use
>> of VIRQ_CONSOLE under tools/, whereas I would have expected provisions
>> for (host) console handling to be delegated to a separate control or
>> console domain? Of course other toolstacks (the XAPI-based one for
>> example) might actually have such provisions.
>>
>> And then there's the XSM question: XEN_DOMCTL_set_virq_handler obviously
>> is subject to XSM checking. The same isn't true for VIRQ_DOMAIN-type
>> vIRQ-s. Yet this vIRQ isn't supposed to be universally available to
>> every DomU. Instead the ->console->input_allowed checking is kind of
>> substituting such a check, which iirc Daniel said (in more general
>> context) shouldn't ever be done. IOW in patch 5 you're actually effecting
>> policy, which should be XSM's job aiui.
>>
>> Bottom line: The patch may need to be more involved, but at the very
>> least the description would need updating to justify it being as simple
>> as it is right now.
>
> What do you think of this:
>
> ---
>
> xen/console: change VIRQ_CONSOLE from global to per-domain
>
> Previously VIRQ_CONSOLE was a global VIRQ (VIRQ_GLOBAL type), meaning
> only the hardware domain (or a domain explicitly set via
> XEN_DOMCTL_set_virq_handler) could bind it. Any other domain attempting
> to bind would fail with -EBUSY because get_global_virq_handler() would
> return hwdom by default.
>
> This patch changes VIRQ_CONSOLE to VIRQ_DOMAIN type, allowing any domain
> to bind it independently, similar to VIRQ_ARGO. The console notification
> is now sent via send_guest_domain_virq() directly to the focus domain
> rather than through send_global_virq().
>
> Implications:
>
> 1. Guest kernels that previously called bind on VIRQ_CONSOLE blindly
> will now succeed. Linux handles binding failure gracefully by falling
> back to polling, so this should not cause regressions.
>
> 2. XEN_DOMCTL_set_virq_handler can no longer be used with VIRQ_CONSOLE.
> The domctl explicitly rejects non-VIRQ_GLOBAL types. This means
> toolstacks that relied on set_virq_handler to delegate console handling
> to a separate console domain will need to use a different mechanism.
> Note: No known in-tree toolstack uses set_virq_handler with VIRQ_CONSOLE.
>
> 3. VIRQ_DOMAIN bindings are not subject to XSM checks beyond the
> standard event channel allocation policy. Access control for console
> input is enforced via the per-domain console->input_allowed flag,
> which is set for:
> - The hardware domain (by default in domain_create())
> - dom0less domains on ARM (in construct_domU())
> - The PV shim domain on x86 (in pv_shim_setup_dom())
> - Domains with vpl011 using the Xen backend (in domain_vpl011_init())
Actually this goes back to the concern I have raised many times,
is_hardware_domain() always serves a double purpose. The explicit check
that the domain is where the hardware is, but also the implicit access
control check that it is allowed to do the hardware access. The implicit
access control check is a subversion of XSM and the reality is that the
input_allowed flag is just unmasking this subversion for an explicit
access control check outside the purview of xsm.
v/r,
dps
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding
2026-02-18 15:07 ` Daniel P. Smith
@ 2026-02-18 15:14 ` Jan Beulich
2026-02-18 16:08 ` Daniel P. Smith
0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2026-02-18 15:14 UTC (permalink / raw)
To: Daniel P. Smith
Cc: Stefano Stabellini, grygorii_strashko, anthony.perard,
michal.orzel, julien, roger.pau, jason.andryuk, victorm.lira,
andrew.cooper3, xen-devel, Stefano Stabellini
On 18.02.2026 16:07, Daniel P. Smith wrote:
> On 2/13/26 15:09, Stefano Stabellini wrote:
>> On Tue, 10 Feb 2026, Jan Beulich wrote:
>>> On 10.02.2026 00:23, Stefano Stabellini wrote:
>>>> On Mon, 9 Feb 2026, Jan Beulich wrote:
>>>>> On 05.02.2026 00:37, Stefano Stabellini wrote:
>>>>>> Today only hwdom can bind VIRQ_CONSOLE. This patch changes the virq from
>>>>>> global to VIRQ_DOMAIN to allow other domains to bind to it.
>>>>>>
>>>>>> Note that Linux silently falls back to polling when binding fails, which
>>>>>> masks the issue.
>>>>>>
>>>>>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>>>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>>>>
>>>>> Technically this is an ABI change, and hence I'm uncertain it can go without
>>>>> that aspect being at least mentioned, perhaps even its implications properly
>>>>> discussed.
>>>>
>>>> I am not sure if it qualifies as an ABI change or not but I am happy to
>>>> expand the commit message in any way you might suggest.
>>>>
>>>> The jist of it is already in the commit message, really the key element
>>>> is that VIRQ_CONSOLE can be bound by multiple domains.
>>>>
>>>> Aside from spelling out "this is an ABI change" what do you have in
>>>> mind?
>>>
>>> What I mean is discussion of the implications for domains using the vIRQ.
>>> Previously most domains would have attempts to bind this vIRQ rejected.
>>> Technically it is possible that kernels had code paths blindly doing the
>>> binding, relying on it to work only when running as Dom0. And really, you
>>> appear to break XEN_DOMCTL_set_virq_handler when used with VIRQ_CONSOLE,
>>> without which its binding wasn't possible at all before (except for the
>>> hardware domain, which get_global_virq_handler() falls back to when no
>>> other domain is set). Or am I mis-reading things, as I can't spot any use
>>> of VIRQ_CONSOLE under tools/, whereas I would have expected provisions
>>> for (host) console handling to be delegated to a separate control or
>>> console domain? Of course other toolstacks (the XAPI-based one for
>>> example) might actually have such provisions.
>>>
>>> And then there's the XSM question: XEN_DOMCTL_set_virq_handler obviously
>>> is subject to XSM checking. The same isn't true for VIRQ_DOMAIN-type
>>> vIRQ-s. Yet this vIRQ isn't supposed to be universally available to
>>> every DomU. Instead the ->console->input_allowed checking is kind of
>>> substituting such a check, which iirc Daniel said (in more general
>>> context) shouldn't ever be done. IOW in patch 5 you're actually effecting
>>> policy, which should be XSM's job aiui.
>>>
>>> Bottom line: The patch may need to be more involved, but at the very
>>> least the description would need updating to justify it being as simple
>>> as it is right now.
>>
>> What do you think of this:
>>
>> ---
>>
>> xen/console: change VIRQ_CONSOLE from global to per-domain
>>
>> Previously VIRQ_CONSOLE was a global VIRQ (VIRQ_GLOBAL type), meaning
>> only the hardware domain (or a domain explicitly set via
>> XEN_DOMCTL_set_virq_handler) could bind it. Any other domain attempting
>> to bind would fail with -EBUSY because get_global_virq_handler() would
>> return hwdom by default.
>>
>> This patch changes VIRQ_CONSOLE to VIRQ_DOMAIN type, allowing any domain
>> to bind it independently, similar to VIRQ_ARGO. The console notification
>> is now sent via send_guest_domain_virq() directly to the focus domain
>> rather than through send_global_virq().
>>
>> Implications:
>>
>> 1. Guest kernels that previously called bind on VIRQ_CONSOLE blindly
>> will now succeed. Linux handles binding failure gracefully by falling
>> back to polling, so this should not cause regressions.
>>
>> 2. XEN_DOMCTL_set_virq_handler can no longer be used with VIRQ_CONSOLE.
>> The domctl explicitly rejects non-VIRQ_GLOBAL types. This means
>> toolstacks that relied on set_virq_handler to delegate console handling
>> to a separate console domain will need to use a different mechanism.
>> Note: No known in-tree toolstack uses set_virq_handler with VIRQ_CONSOLE.
>>
>> 3. VIRQ_DOMAIN bindings are not subject to XSM checks beyond the
>> standard event channel allocation policy. Access control for console
>> input is enforced via the per-domain console->input_allowed flag,
>> which is set for:
>> - The hardware domain (by default in domain_create())
>> - dom0less domains on ARM (in construct_domU())
>> - The PV shim domain on x86 (in pv_shim_setup_dom())
>> - Domains with vpl011 using the Xen backend (in domain_vpl011_init())
>
> Actually this goes back to the concern I have raised many times,
> is_hardware_domain() always serves a double purpose. The explicit check
> that the domain is where the hardware is, but also the implicit access
> control check that it is allowed to do the hardware access. The implicit
> access control check is a subversion of XSM and the reality is that the
> input_allowed flag is just unmasking this subversion for an explicit
> access control check outside the purview of xsm.
I don't think I can deduce from this what your view is on the change proposed.
There is, as per what you say, an existing issue with is_hardware_domain().
(Likely at some point you'll propose patches to address this.) What I can't
conclude is whether you deem this new issue "okay(ish)" on the basis that some
vaguely related issue already exists, or whether you object to this new way
of "subversion".
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding
2026-02-18 15:14 ` Jan Beulich
@ 2026-02-18 16:08 ` Daniel P. Smith
0 siblings, 0 replies; 23+ messages in thread
From: Daniel P. Smith @ 2026-02-18 16:08 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, grygorii_strashko, anthony.perard,
michal.orzel, julien, roger.pau, jason.andryuk, victorm.lira,
andrew.cooper3, xen-devel, Stefano Stabellini
On 2/18/26 10:14, Jan Beulich wrote:
> On 18.02.2026 16:07, Daniel P. Smith wrote:
>> On 2/13/26 15:09, Stefano Stabellini wrote:
>>> On Tue, 10 Feb 2026, Jan Beulich wrote:
>>>> On 10.02.2026 00:23, Stefano Stabellini wrote:
>>>>> On Mon, 9 Feb 2026, Jan Beulich wrote:
>>>>>> On 05.02.2026 00:37, Stefano Stabellini wrote:
>>>>>>> Today only hwdom can bind VIRQ_CONSOLE. This patch changes the virq from
>>>>>>> global to VIRQ_DOMAIN to allow other domains to bind to it.
>>>>>>>
>>>>>>> Note that Linux silently falls back to polling when binding fails, which
>>>>>>> masks the issue.
>>>>>>>
>>>>>>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>>>>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>>>>>
>>>>>> Technically this is an ABI change, and hence I'm uncertain it can go without
>>>>>> that aspect being at least mentioned, perhaps even its implications properly
>>>>>> discussed.
>>>>>
>>>>> I am not sure if it qualifies as an ABI change or not but I am happy to
>>>>> expand the commit message in any way you might suggest.
>>>>>
>>>>> The jist of it is already in the commit message, really the key element
>>>>> is that VIRQ_CONSOLE can be bound by multiple domains.
>>>>>
>>>>> Aside from spelling out "this is an ABI change" what do you have in
>>>>> mind?
>>>>
>>>> What I mean is discussion of the implications for domains using the vIRQ.
>>>> Previously most domains would have attempts to bind this vIRQ rejected.
>>>> Technically it is possible that kernels had code paths blindly doing the
>>>> binding, relying on it to work only when running as Dom0. And really, you
>>>> appear to break XEN_DOMCTL_set_virq_handler when used with VIRQ_CONSOLE,
>>>> without which its binding wasn't possible at all before (except for the
>>>> hardware domain, which get_global_virq_handler() falls back to when no
>>>> other domain is set). Or am I mis-reading things, as I can't spot any use
>>>> of VIRQ_CONSOLE under tools/, whereas I would have expected provisions
>>>> for (host) console handling to be delegated to a separate control or
>>>> console domain? Of course other toolstacks (the XAPI-based one for
>>>> example) might actually have such provisions.
>>>>
>>>> And then there's the XSM question: XEN_DOMCTL_set_virq_handler obviously
>>>> is subject to XSM checking. The same isn't true for VIRQ_DOMAIN-type
>>>> vIRQ-s. Yet this vIRQ isn't supposed to be universally available to
>>>> every DomU. Instead the ->console->input_allowed checking is kind of
>>>> substituting such a check, which iirc Daniel said (in more general
>>>> context) shouldn't ever be done. IOW in patch 5 you're actually effecting
>>>> policy, which should be XSM's job aiui.
>>>>
>>>> Bottom line: The patch may need to be more involved, but at the very
>>>> least the description would need updating to justify it being as simple
>>>> as it is right now.
>>>
>>> What do you think of this:
>>>
>>> ---
>>>
>>> xen/console: change VIRQ_CONSOLE from global to per-domain
>>>
>>> Previously VIRQ_CONSOLE was a global VIRQ (VIRQ_GLOBAL type), meaning
>>> only the hardware domain (or a domain explicitly set via
>>> XEN_DOMCTL_set_virq_handler) could bind it. Any other domain attempting
>>> to bind would fail with -EBUSY because get_global_virq_handler() would
>>> return hwdom by default.
>>>
>>> This patch changes VIRQ_CONSOLE to VIRQ_DOMAIN type, allowing any domain
>>> to bind it independently, similar to VIRQ_ARGO. The console notification
>>> is now sent via send_guest_domain_virq() directly to the focus domain
>>> rather than through send_global_virq().
>>>
>>> Implications:
>>>
>>> 1. Guest kernels that previously called bind on VIRQ_CONSOLE blindly
>>> will now succeed. Linux handles binding failure gracefully by falling
>>> back to polling, so this should not cause regressions.
>>>
>>> 2. XEN_DOMCTL_set_virq_handler can no longer be used with VIRQ_CONSOLE.
>>> The domctl explicitly rejects non-VIRQ_GLOBAL types. This means
>>> toolstacks that relied on set_virq_handler to delegate console handling
>>> to a separate console domain will need to use a different mechanism.
>>> Note: No known in-tree toolstack uses set_virq_handler with VIRQ_CONSOLE.
>>>
>>> 3. VIRQ_DOMAIN bindings are not subject to XSM checks beyond the
>>> standard event channel allocation policy. Access control for console
>>> input is enforced via the per-domain console->input_allowed flag,
>>> which is set for:
>>> - The hardware domain (by default in domain_create())
>>> - dom0less domains on ARM (in construct_domU())
>>> - The PV shim domain on x86 (in pv_shim_setup_dom())
>>> - Domains with vpl011 using the Xen backend (in domain_vpl011_init())
>>
>> Actually this goes back to the concern I have raised many times,
>> is_hardware_domain() always serves a double purpose. The explicit check
>> that the domain is where the hardware is, but also the implicit access
>> control check that it is allowed to do the hardware access. The implicit
>> access control check is a subversion of XSM and the reality is that the
>> input_allowed flag is just unmasking this subversion for an explicit
>> access control check outside the purview of xsm.
>
> I don't think I can deduce from this what your view is on the change proposed.
> There is, as per what you say, an existing issue with is_hardware_domain().
> (Likely at some point you'll propose patches to address this.) What I can't
> conclude is whether you deem this new issue "okay(ish)" on the basis that some
> vaguely related issue already exists, or whether you object to this new way
> of "subversion".
Digging deeper, the underlying issue is that when struct domain_console
was introduced and now this series is building upon it. The struct
domain_console_patch added a resource access control check point for all
domains through input_allowed that went around XSM. Now we are here with
this series that is exposing what was done. To your question, am I
okay(ish) with this. Not really, but it's also not the fault of Stefano
or his series that the earlier commit laid this landmine for him.
With all that said, thinking about it in totality, the fine-grained
access control does needs to be there for tightly controlled
environments, but is not necessary for a general security posture. Plus,
I am going to have to think about how to correctly inlay the XSM check,
which is outside the scope of this series. All of that is to say, I am
not okay but don't feel it's right to block this series over it.
v/r,
dps
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v10 3/5] xen/console: add locking for serial_rx ring buffer access
2026-02-04 23:35 [PATCH v10 0/5] xen: console_io for dom0less guests Stefano Stabellini
2026-02-04 23:37 ` [PATCH v10 1/5] xen/console: use ACCESS_ONCE for console_rx Stefano Stabellini
2026-02-04 23:37 ` [PATCH v10 2/5] xen: change VIRQ_CONSOLE to VIRQ_DOMAIN to allow non-hwdom binding Stefano Stabellini
@ 2026-02-04 23:37 ` Stefano Stabellini
2026-02-06 22:24 ` dmukhin
2026-02-10 15:48 ` Jan Beulich
2026-02-04 23:37 ` [PATCH v10 4/5] xen/console: handle multiple domains using console_io hypercalls Stefano Stabellini
2026-02-04 23:37 ` [PATCH v10 5/5] xen: enable dom0less guests to use " Stefano Stabellini
4 siblings, 2 replies; 23+ messages in thread
From: Stefano Stabellini @ 2026-02-04 23:37 UTC (permalink / raw)
To: xen-devel
Cc: grygorii_strashko, anthony.perard, michal.orzel, julien,
roger.pau, jason.andryuk, victorm.lira, andrew.cooper3, jbeulich,
sstabellini, Stefano Stabellini
Guard every mutation of serial_rx_cons/prod with console_lock so that
cross-domain reads can't see stale data:
- In console_switch_input(): protect console_rx assignment with the lock
using irqsave/irqrestore variants since this can be called from
interrupt context
- In __serial_rx(): protect the ring buffer write operation when
delivering input to the hardware domain
- In do_console_io() CONSOLEIO_read: hold the lock around the entire
read loop, using a local buffer copy to avoid holding the lock during
copy_to_guest_offset()
This is preparatory work for allowing multiple domains to use the
console_io hypercalls where proper synchronization is required.
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
Changes in v10:
- patch split into two halves, this is the first half
---
xen/drivers/char/console.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index fbc89ca2a4..ad53073b99 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -553,10 +553,13 @@ static void console_switch_input(void)
{
domid_t domid;
struct domain *d;
+ unsigned long flags;
if ( next_rx++ >= max_console_rx )
{
+ nrspin_lock_irqsave(&console_lock, flags);
ACCESS_ONCE(console_rx) = 0;
+ nrspin_unlock_irqrestore(&console_lock, flags);
printk("*** Serial input to Xen");
break;
}
@@ -576,7 +579,9 @@ static void console_switch_input(void)
rcu_unlock_domain(d);
+ nrspin_lock_irqsave(&console_lock, flags);
ACCESS_ONCE(console_rx) = next_rx;
+ nrspin_unlock_irqrestore(&console_lock, flags);
printk("*** Serial input to DOM%u", domid);
break;
}
@@ -602,12 +607,16 @@ static void __serial_rx(char c)
if ( is_hardware_domain(d) )
{
+ unsigned long flags;
+
/*
* Deliver input to the hardware domain buffer, unless it is
* already full.
*/
+ nrspin_lock_irqsave(&console_lock, flags);
if ( (serial_rx_prod - serial_rx_cons) != SERIAL_RX_SIZE )
serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
+ nrspin_unlock_irqrestore(&console_lock, flags);
/*
* Always notify the hardware domain: prevents receive path from
@@ -796,6 +805,7 @@ long do_console_io(
{
long rc;
unsigned int idx, len;
+ char kbuf[SERIAL_RX_SIZE];
rc = xsm_console_io(XSM_OTHER, current->domain, cmd);
if ( rc )
@@ -817,6 +827,7 @@ long do_console_io(
break;
rc = 0;
+ nrspin_lock_irq(&console_lock);
while ( (serial_rx_cons != serial_rx_prod) && (rc < count) )
{
idx = SERIAL_RX_MASK(serial_rx_cons);
@@ -825,14 +836,19 @@ long do_console_io(
len = SERIAL_RX_SIZE - idx;
if ( (rc + len) > count )
len = count - rc;
- if ( copy_to_guest_offset(buffer, rc, &serial_rx_ring[idx], len) )
- {
- rc = -EFAULT;
- break;
- }
+ memcpy(kbuf, &serial_rx_ring[idx], len);
+
+ nrspin_unlock_irq(&console_lock);
+
+ if ( copy_to_guest_offset(buffer, rc, kbuf, len) )
+ return -EFAULT;
rc += len;
+
+ nrspin_lock_irq(&console_lock);
+
serial_rx_cons += len;
}
+ nrspin_unlock_irq(&console_lock);
break;
default:
rc = -ENOSYS;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v10 3/5] xen/console: add locking for serial_rx ring buffer access
2026-02-04 23:37 ` [PATCH v10 3/5] xen/console: add locking for serial_rx ring buffer access Stefano Stabellini
@ 2026-02-06 22:24 ` dmukhin
2026-02-10 15:48 ` Jan Beulich
1 sibling, 0 replies; 23+ messages in thread
From: dmukhin @ 2026-02-06 22:24 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, grygorii_strashko, anthony.perard, michal.orzel,
julien, roger.pau, jason.andryuk, victorm.lira, andrew.cooper3,
jbeulich, sstabellini
On Wed, Feb 04, 2026 at 03:37:10PM -0800, Stefano Stabellini wrote:
> Guard every mutation of serial_rx_cons/prod with console_lock so that
> cross-domain reads can't see stale data:
>
> - In console_switch_input(): protect console_rx assignment with the lock
> using irqsave/irqrestore variants since this can be called from
> interrupt context
>
> - In __serial_rx(): protect the ring buffer write operation when
> delivering input to the hardware domain
>
> - In do_console_io() CONSOLEIO_read: hold the lock around the entire
> read loop, using a local buffer copy to avoid holding the lock during
> copy_to_guest_offset()
>
> This is preparatory work for allowing multiple domains to use the
> console_io hypercalls where proper synchronization is required.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 3/5] xen/console: add locking for serial_rx ring buffer access
2026-02-04 23:37 ` [PATCH v10 3/5] xen/console: add locking for serial_rx ring buffer access Stefano Stabellini
2026-02-06 22:24 ` dmukhin
@ 2026-02-10 15:48 ` Jan Beulich
1 sibling, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2026-02-10 15:48 UTC (permalink / raw)
To: Stefano Stabellini
Cc: grygorii_strashko, anthony.perard, michal.orzel, julien,
roger.pau, jason.andryuk, victorm.lira, andrew.cooper3,
sstabellini, xen-devel
On 05.02.2026 00:37, Stefano Stabellini wrote:
> Guard every mutation of serial_rx_cons/prod with console_lock so that
> cross-domain reads can't see stale data:
Cross-domain reads become a thing the earliest in the next patch, though?
You say something along these lines ...
> - In console_switch_input(): protect console_rx assignment with the lock
> using irqsave/irqrestore variants since this can be called from
> interrupt context
>
> - In __serial_rx(): protect the ring buffer write operation when
> delivering input to the hardware domain
>
> - In do_console_io() CONSOLEIO_read: hold the lock around the entire
> read loop, using a local buffer copy to avoid holding the lock during
> copy_to_guest_offset()
>
> This is preparatory work for allowing multiple domains to use the
> console_io hypercalls where proper synchronization is required.
... here, but I think that initial part also wants slightly re-phrasing.
At the very least insert "in the future".
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -553,10 +553,13 @@ static void console_switch_input(void)
> {
> domid_t domid;
> struct domain *d;
> + unsigned long flags;
>
> if ( next_rx++ >= max_console_rx )
> {
> + nrspin_lock_irqsave(&console_lock, flags);
> ACCESS_ONCE(console_rx) = 0;
> + nrspin_unlock_irqrestore(&console_lock, flags);
> printk("*** Serial input to Xen");
> break;
> }
> @@ -576,7 +579,9 @@ static void console_switch_input(void)
>
> rcu_unlock_domain(d);
>
> + nrspin_lock_irqsave(&console_lock, flags);
> ACCESS_ONCE(console_rx) = next_rx;
> + nrspin_unlock_irqrestore(&console_lock, flags);
> printk("*** Serial input to DOM%u", domid);
> break;
> }
In __serial_rx() and do_console_io() you guard more than the mere updating.
As said before, with this arrangement of locking next_rx can in principle
be stale by the time you use it for storing into console_rx. This
arrangement may be okay, but would then need commenting upon.
> @@ -796,6 +805,7 @@ long do_console_io(
> {
> long rc;
> unsigned int idx, len;
> + char kbuf[SERIAL_RX_SIZE];
Please can such live in the narrowest possible scope?
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v10 4/5] xen/console: handle multiple domains using console_io hypercalls
2026-02-04 23:35 [PATCH v10 0/5] xen: console_io for dom0less guests Stefano Stabellini
` (2 preceding siblings ...)
2026-02-04 23:37 ` [PATCH v10 3/5] xen/console: add locking for serial_rx ring buffer access Stefano Stabellini
@ 2026-02-04 23:37 ` Stefano Stabellini
2026-02-06 22:29 ` dmukhin
2026-02-10 16:05 ` Jan Beulich
2026-02-04 23:37 ` [PATCH v10 5/5] xen: enable dom0less guests to use " Stefano Stabellini
4 siblings, 2 replies; 23+ messages in thread
From: Stefano Stabellini @ 2026-02-04 23:37 UTC (permalink / raw)
To: xen-devel
Cc: grygorii_strashko, anthony.perard, michal.orzel, julien,
roger.pau, jason.andryuk, victorm.lira, andrew.cooper3, jbeulich,
sstabellini, Stefano Stabellini
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:
- 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 re-check
focus after each chunk in the CONSOLEIO_read loop 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 v10:
- patch split into two halves this is the second half
---
xen/drivers/char/console.c | 52 ++++++++++++++++++++++++++++++++++----
1 file changed, 47 insertions(+), 5 deletions(-)
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index ad53073b99..d3ce925131 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -541,6 +541,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 = ACCESS_ONCE(console_rx);
@@ -559,6 +565,7 @@ static void console_switch_input(void)
{
nrspin_lock_irqsave(&console_lock, flags);
ACCESS_ONCE(console_rx) = 0;
+ serial_rx_cons = serial_rx_prod;
nrspin_unlock_irqrestore(&console_lock, flags);
printk("*** Serial input to Xen");
break;
@@ -581,6 +588,8 @@ static void console_switch_input(void)
nrspin_lock_irqsave(&console_lock, flags);
ACCESS_ONCE(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_irqrestore(&console_lock, flags);
printk("*** Serial input to DOM%u", domid);
break;
@@ -610,7 +619,7 @@ static void __serial_rx(char c)
unsigned long flags;
/*
- * Deliver input to the hardware domain buffer, unless it is
+ * Deliver input to the focus domain buffer, unless it is
* already full.
*/
nrspin_lock_irqsave(&console_lock, flags);
@@ -740,6 +749,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 )
{
@@ -752,17 +762,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 and it is called with cons->lock held. 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
@@ -775,7 +804,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)) )
@@ -828,6 +856,8 @@ long do_console_io(
rc = 0;
nrspin_lock_irq(&console_lock);
+ if ( !is_focus_domain(current->domain) )
+ count = 0;
while ( (serial_rx_cons != serial_rx_prod) && (rc < count) )
{
idx = SERIAL_RX_MASK(serial_rx_cons);
@@ -844,7 +874,19 @@ long do_console_io(
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;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v10 4/5] xen/console: handle multiple domains using console_io hypercalls
2026-02-04 23:37 ` [PATCH v10 4/5] xen/console: handle multiple domains using console_io hypercalls Stefano Stabellini
@ 2026-02-06 22:29 ` dmukhin
2026-02-10 16:05 ` Jan Beulich
1 sibling, 0 replies; 23+ messages in thread
From: dmukhin @ 2026-02-06 22:29 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, grygorii_strashko, anthony.perard, michal.orzel,
julien, roger.pau, jason.andryuk, victorm.lira, andrew.cooper3,
jbeulich, sstabellini
On Wed, Feb 04, 2026 at 03:37:11PM -0800, Stefano Stabellini wrote:
> 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:
>
> - 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 re-check
> focus after each chunk in the CONSOLEIO_read loop 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>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 4/5] xen/console: handle multiple domains using console_io hypercalls
2026-02-04 23:37 ` [PATCH v10 4/5] xen/console: handle multiple domains using console_io hypercalls Stefano Stabellini
2026-02-06 22:29 ` dmukhin
@ 2026-02-10 16:05 ` Jan Beulich
1 sibling, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2026-02-10 16:05 UTC (permalink / raw)
To: Stefano Stabellini
Cc: grygorii_strashko, anthony.perard, michal.orzel, julien,
roger.pau, jason.andryuk, victorm.lira, andrew.cooper3,
sstabellini, xen-devel
On 05.02.2026 00:37, Stefano Stabellini wrote:
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -541,6 +541,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;
> +}
Just as a remark: You using only nrspin_*() further down and in patch 3,
using rspin_is_locked() here is slightly irritating. It looks to be
technically correct, but maybe a comment is warranted nevertheless?
> @@ -559,6 +565,7 @@ static void console_switch_input(void)
> {
> nrspin_lock_irqsave(&console_lock, flags);
> ACCESS_ONCE(console_rx) = 0;
> + serial_rx_cons = serial_rx_prod;
I think you want to have the same comment here as you put ...
> @@ -581,6 +588,8 @@ static void console_switch_input(void)
>
> nrspin_lock_irqsave(&console_lock, flags);
> ACCESS_ONCE(console_rx) = next_rx;
> + /* Don't let the next dom read the previous dom's unread data. */
> + serial_rx_cons = serial_rx_prod;
... here.
> @@ -610,7 +619,7 @@ static void __serial_rx(char c)
> unsigned long flags;
>
> /*
> - * Deliver input to the hardware domain buffer, unless it is
> + * Deliver input to the focus domain buffer, unless it is
> * already full.
> */
> nrspin_lock_irqsave(&console_lock, flags);
The conditional ahead of this comment isn't changed here yet, so changing the
comment is premature. (I'm pretty sure I said so before.)
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v10 5/5] xen: enable dom0less guests to use console_io hypercalls
2026-02-04 23:35 [PATCH v10 0/5] xen: console_io for dom0less guests Stefano Stabellini
` (3 preceding siblings ...)
2026-02-04 23:37 ` [PATCH v10 4/5] xen/console: handle multiple domains using console_io hypercalls Stefano Stabellini
@ 2026-02-04 23:37 ` Stefano Stabellini
2026-02-05 2:09 ` dmukhin
4 siblings, 1 reply; 23+ messages in thread
From: Stefano Stabellini @ 2026-02-04 23:37 UTC (permalink / raw)
To: xen-devel
Cc: grygorii_strashko, anthony.perard, michal.orzel, julien,
roger.pau, jason.andryuk, victorm.lira, andrew.cooper3, jbeulich,
sstabellini, Stefano Stabellini
Enable dom0less guests on ARM to use console_io hypercalls:
- set input_allow = true for dom0less domains
- update the in-code comment in console.c
- prioritize the VUART check to retain the same behavior as today
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
xen/common/device-tree/dom0less-build.c | 2 ++
xen/drivers/char/console.c | 16 ++++++++++------
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index 840d14419d..cb7026fa7e 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -829,6 +829,8 @@ static int __init construct_domU(struct kernel_info *kinfo,
rangeset_destroy(kinfo->xen_reg_assigned);
+ d->console->input_allowed = true;
+
return rc;
}
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index d3ce925131..7f0c3d8376 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -610,11 +610,20 @@ static void __serial_rx(char c)
if ( ACCESS_ONCE(console_rx) == 0 )
return handle_keypress(c, false);
+ /* Includes an is_focus_domain() check. */
d = console_get_domain();
if ( !d )
return;
- if ( is_hardware_domain(d) )
+#ifdef CONFIG_SBSA_VUART_CONSOLE
+ /* Prioritize vpl011 if enabled for this domain */
+ if ( d->arch.vpl011.base_addr )
+ {
+ /* Deliver input to the emulated UART. */
+ rc = vpl011_rx_char_xen(d, c);
+ }
+ else
+#endif
{
unsigned long flags;
@@ -633,11 +642,6 @@ static void __serial_rx(char c)
*/
send_guest_domain_virq(d, VIRQ_CONSOLE);
}
-#ifdef CONFIG_SBSA_VUART_CONSOLE
- else
- /* Deliver input to the emulated UART. */
- rc = vpl011_rx_char_xen(d, c);
-#endif
if ( consoled_is_enabled() )
/* Deliver input to the PV shim console. */
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v10 5/5] xen: enable dom0less guests to use console_io hypercalls
2026-02-04 23:37 ` [PATCH v10 5/5] xen: enable dom0less guests to use " Stefano Stabellini
@ 2026-02-05 2:09 ` dmukhin
2026-02-05 22:09 ` Stefano Stabellini
0 siblings, 1 reply; 23+ messages in thread
From: dmukhin @ 2026-02-05 2:09 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, grygorii_strashko, anthony.perard, michal.orzel,
julien, roger.pau, jason.andryuk, victorm.lira, andrew.cooper3,
jbeulich, sstabellini
On Wed, Feb 04, 2026 at 03:37:12PM -0800, Stefano Stabellini wrote:
> Enable dom0less guests on ARM to use console_io hypercalls:
> - set input_allow = true for dom0less domains
> - update the in-code comment in console.c
> - prioritize the VUART check to retain the same behavior as today
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
The code looks good, just one remark wrt prioritizing VUART check.
> ---
> xen/common/device-tree/dom0less-build.c | 2 ++
> xen/drivers/char/console.c | 16 ++++++++++------
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
> index 840d14419d..cb7026fa7e 100644
> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -829,6 +829,8 @@ static int __init construct_domU(struct kernel_info *kinfo,
>
> rangeset_destroy(kinfo->xen_reg_assigned);
>
> + d->console->input_allowed = true;
> +
> return rc;
> }
>
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index d3ce925131..7f0c3d8376 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -610,11 +610,20 @@ static void __serial_rx(char c)
> if ( ACCESS_ONCE(console_rx) == 0 )
> return handle_keypress(c, false);
>
> + /* Includes an is_focus_domain() check. */
> d = console_get_domain();
> if ( !d )
> return;
>
> - if ( is_hardware_domain(d) )
Hardware domain on x86 may have an emulated UART (not in upstream, through,
I need to send v8 for NS16550 series...). The patch which illustrates the
idea:
https://lore.kernel.org/xen-devel/20250908211149.279143-2-dmukhin@ford.com/
So this code (hopefully soon) will need adjustment again.
I would update the code to something like:
if ( is_hardware_domain(d) && !domain_has_vuart(d) )
{
// handle hardware domain
}
#ifdef CONFIG_SBSA_VUART_CONSOLE
else if ( domain_has_vuart(d) )
/* Deliver input to the emulated UART. */
rc = vpl011_rx_char_xen(d, c);
#endif
But domain_has_vuart() needs to be defined for all architectures
(currently it is hidden in arch/arm/vuart.c).
Or perhaps it is possible to postpone the change?
What do you think?
> +#ifdef CONFIG_SBSA_VUART_CONSOLE
> + /* Prioritize vpl011 if enabled for this domain */
> + if ( d->arch.vpl011.base_addr )
> + {
> + /* Deliver input to the emulated UART. */
> + rc = vpl011_rx_char_xen(d, c);
> + }
> + else
> +#endif
> {
> unsigned long flags;
>
> @@ -633,11 +642,6 @@ static void __serial_rx(char c)
> */
> send_guest_domain_virq(d, VIRQ_CONSOLE);
> }
> -#ifdef CONFIG_SBSA_VUART_CONSOLE
> - else
> - /* Deliver input to the emulated UART. */
> - rc = vpl011_rx_char_xen(d, c);
> -#endif
>
> if ( consoled_is_enabled() )
> /* Deliver input to the PV shim console. */
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v10 5/5] xen: enable dom0less guests to use console_io hypercalls
2026-02-05 2:09 ` dmukhin
@ 2026-02-05 22:09 ` Stefano Stabellini
2026-02-06 0:47 ` dmukhin
0 siblings, 1 reply; 23+ messages in thread
From: Stefano Stabellini @ 2026-02-05 22:09 UTC (permalink / raw)
To: dmukhin
Cc: Stefano Stabellini, xen-devel, grygorii_strashko, anthony.perard,
michal.orzel, julien, roger.pau, jason.andryuk, victorm.lira,
andrew.cooper3, jbeulich, sstabellini
On Wed, 4 Feb 2026, dmukhin@xen.org wrote:
> On Wed, Feb 04, 2026 at 03:37:12PM -0800, Stefano Stabellini wrote:
> > Enable dom0less guests on ARM to use console_io hypercalls:
> > - set input_allow = true for dom0less domains
> > - update the in-code comment in console.c
> > - prioritize the VUART check to retain the same behavior as today
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>
> The code looks good, just one remark wrt prioritizing VUART check.
>
> > ---
> > xen/common/device-tree/dom0less-build.c | 2 ++
> > xen/drivers/char/console.c | 16 ++++++++++------
> > 2 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
> > index 840d14419d..cb7026fa7e 100644
> > --- a/xen/common/device-tree/dom0less-build.c
> > +++ b/xen/common/device-tree/dom0less-build.c
> > @@ -829,6 +829,8 @@ static int __init construct_domU(struct kernel_info *kinfo,
> >
> > rangeset_destroy(kinfo->xen_reg_assigned);
> >
> > + d->console->input_allowed = true;
> > +
> > return rc;
> > }
> >
> > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> > index d3ce925131..7f0c3d8376 100644
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -610,11 +610,20 @@ static void __serial_rx(char c)
> > if ( ACCESS_ONCE(console_rx) == 0 )
> > return handle_keypress(c, false);
> >
> > + /* Includes an is_focus_domain() check. */
> > d = console_get_domain();
> > if ( !d )
> > return;
> >
> > - if ( is_hardware_domain(d) )
>
> Hardware domain on x86 may have an emulated UART (not in upstream, through,
> I need to send v8 for NS16550 series...). The patch which illustrates the
> idea:
> https://lore.kernel.org/xen-devel/20250908211149.279143-2-dmukhin@ford.com/
>
> So this code (hopefully soon) will need adjustment again.
>
> I would update the code to something like:
>
>
>
> if ( is_hardware_domain(d) && !domain_has_vuart(d) )
> {
> // handle hardware domain
> }
> #ifdef CONFIG_SBSA_VUART_CONSOLE
> else if ( domain_has_vuart(d) )
> /* Deliver input to the emulated UART. */
> rc = vpl011_rx_char_xen(d, c);
> #endif
>
>
>
> But domain_has_vuart() needs to be defined for all architectures
> (currently it is hidden in arch/arm/vuart.c).
>
> Or perhaps it is possible to postpone the change?
This change is needed to avoid regressions on ARM.
However, while I wouldn't be surprised if we need a change here for your
upcoming patch series, at the same time at the moment I don't see why
this check wouldn't work as it is for you as well.
On x86, CONFIG_SBSA_VUART_CONSOLE will never be set. It is OK to do this
first:
#ifdef CONFIG_SBSA_VUART_CONSOLE
/* Prioritize vpl011 if enabled for this domain */
if ( d->arch.vpl011.base_addr )
{
/* Deliver input to the emulated UART. */
rc = vpl011_rx_char_xen(d, c);
}
else
#endif
It shouldn't hurt. The is_hardware_domain() check is also not necessary
anymore because any necessary check would be part of this check above:
d = console_get_domain();
if ( !d )
return;
So I am guessing that your series might actually leave this code
unchanged and instead might modify console_get_domain() or
max_console_rx.
> > +#ifdef CONFIG_SBSA_VUART_CONSOLE
> > + /* Prioritize vpl011 if enabled for this domain */
> > + if ( d->arch.vpl011.base_addr )
> > + {
> > + /* Deliver input to the emulated UART. */
> > + rc = vpl011_rx_char_xen(d, c);
> > + }
> > + else
> > +#endif
> > {
> > unsigned long flags;
> >
> > @@ -633,11 +642,6 @@ static void __serial_rx(char c)
> > */
> > send_guest_domain_virq(d, VIRQ_CONSOLE);
> > }
> > -#ifdef CONFIG_SBSA_VUART_CONSOLE
> > - else
> > - /* Deliver input to the emulated UART. */
> > - rc = vpl011_rx_char_xen(d, c);
> > -#endif
> >
> > if ( consoled_is_enabled() )
> > /* Deliver input to the PV shim console. */
> > --
> > 2.25.1
> >
> >
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v10 5/5] xen: enable dom0less guests to use console_io hypercalls
2026-02-05 22:09 ` Stefano Stabellini
@ 2026-02-06 0:47 ` dmukhin
0 siblings, 0 replies; 23+ messages in thread
From: dmukhin @ 2026-02-06 0:47 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Stefano Stabellini, xen-devel, grygorii_strashko, anthony.perard,
michal.orzel, julien, roger.pau, jason.andryuk, victorm.lira,
andrew.cooper3, jbeulich
On Thu, Feb 05, 2026 at 02:09:07PM -0800, Stefano Stabellini wrote:
> On Wed, 4 Feb 2026, dmukhin@xen.org wrote:
> > On Wed, Feb 04, 2026 at 03:37:12PM -0800, Stefano Stabellini wrote:
> > > Enable dom0less guests on ARM to use console_io hypercalls:
> > > - set input_allow = true for dom0less domains
> > > - update the in-code comment in console.c
> > > - prioritize the VUART check to retain the same behavior as today
> > >
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> >
> > The code looks good, just one remark wrt prioritizing VUART check.
> >
> > > ---
> > > xen/common/device-tree/dom0less-build.c | 2 ++
> > > xen/drivers/char/console.c | 16 ++++++++++------
> > > 2 files changed, 12 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
> > > index 840d14419d..cb7026fa7e 100644
> > > --- a/xen/common/device-tree/dom0less-build.c
> > > +++ b/xen/common/device-tree/dom0less-build.c
> > > @@ -829,6 +829,8 @@ static int __init construct_domU(struct kernel_info *kinfo,
> > >
> > > rangeset_destroy(kinfo->xen_reg_assigned);
> > >
> > > + d->console->input_allowed = true;
> > > +
> > > return rc;
> > > }
> > >
> > > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> > > index d3ce925131..7f0c3d8376 100644
> > > --- a/xen/drivers/char/console.c
> > > +++ b/xen/drivers/char/console.c
> > > @@ -610,11 +610,20 @@ static void __serial_rx(char c)
> > > if ( ACCESS_ONCE(console_rx) == 0 )
> > > return handle_keypress(c, false);
> > >
> > > + /* Includes an is_focus_domain() check. */
> > > d = console_get_domain();
> > > if ( !d )
> > > return;
> > >
> > > - if ( is_hardware_domain(d) )
> >
> > Hardware domain on x86 may have an emulated UART (not in upstream, through,
> > I need to send v8 for NS16550 series...). The patch which illustrates the
> > idea:
> > https://lore.kernel.org/xen-devel/20250908211149.279143-2-dmukhin@ford.com/
> >
> > So this code (hopefully soon) will need adjustment again.
> >
> > I would update the code to something like:
> >
> >
> >
> > if ( is_hardware_domain(d) && !domain_has_vuart(d) )
> > {
> > // handle hardware domain
> > }
> > #ifdef CONFIG_SBSA_VUART_CONSOLE
> > else if ( domain_has_vuart(d) )
> > /* Deliver input to the emulated UART. */
> > rc = vpl011_rx_char_xen(d, c);
> > #endif
> >
> >
> >
> > But domain_has_vuart() needs to be defined for all architectures
> > (currently it is hidden in arch/arm/vuart.c).
> >
> > Or perhaps it is possible to postpone the change?
>
> This change is needed to avoid regressions on ARM.
Oh, I see.
>
> However, while I wouldn't be surprised if we need a change here for your
> upcoming patch series, at the same time at the moment I don't see why
> this check wouldn't work as it is for you as well.
>
> On x86, CONFIG_SBSA_VUART_CONSOLE will never be set. It is OK to do this
> first:
>
> #ifdef CONFIG_SBSA_VUART_CONSOLE
> /* Prioritize vpl011 if enabled for this domain */
> if ( d->arch.vpl011.base_addr )
> {
> /* Deliver input to the emulated UART. */
> rc = vpl011_rx_char_xen(d, c);
> }
> else
> #endif
>
> It shouldn't hurt. The is_hardware_domain() check is also not necessary
> anymore because any necessary check would be part of this check above:
>
> d = console_get_domain();
> if ( !d )
> return;
>
> So I am guessing that your series might actually leave this code
> unchanged and instead might modify console_get_domain() or
> max_console_rx.
I think I did experiment w/ processing vUART before hardware domain
locally when debugging NS16550 emulator with dom0 PVH...
Thanks for explanations!
Please consider:
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
>
>
>
> > > +#ifdef CONFIG_SBSA_VUART_CONSOLE
> > > + /* Prioritize vpl011 if enabled for this domain */
> > > + if ( d->arch.vpl011.base_addr )
> > > + {
> > > + /* Deliver input to the emulated UART. */
> > > + rc = vpl011_rx_char_xen(d, c);
> > > + }
> > > + else
> > > +#endif
> > > {
> > > unsigned long flags;
> > >
> > > @@ -633,11 +642,6 @@ static void __serial_rx(char c)
> > > */
> > > send_guest_domain_virq(d, VIRQ_CONSOLE);
> > > }
> > > -#ifdef CONFIG_SBSA_VUART_CONSOLE
> > > - else
> > > - /* Deliver input to the emulated UART. */
> > > - rc = vpl011_rx_char_xen(d, c);
> > > -#endif
> > >
> > > if ( consoled_is_enabled() )
> > > /* Deliver input to the PV shim console. */
> > > --
> > > 2.25.1
> > >
> > >
> >
^ permalink raw reply [flat|nested] 23+ messages in thread