Re: [PATCH v2] xen/console: handle multiple domains using console_io hypercalls

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.01.2026 02:30, 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.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> ---
> Changes in v2:
> - fix code style
> - pbuf_idx/idx after ada53067083e
> - don't add extra \0
> - clear input on console switch
> ---
>  xen/drivers/char/console.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 2bdb4d5fb4..6c7a6592c5 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -576,6 +576,8 @@ static void console_switch_input(void)
>              rcu_unlock_domain(d);
>  
>              console_rx = next_rx;
> +            /* don't let the next dom read the previous dom's unread data */

Nit: Comment style.

> +            serial_rx_cons = serial_rx_prod;
>              printk("*** Serial input to DOM%u", domid);

Imo the flushing of input needs to come after the printk(), as it's only
then that the user gets confirmation of the change.

As to flushing (rather than storing) leftover input: That's strictly an
improvement over v1, but imo unhelpful. I may not be willing to ack this
(still need to think about it some more), but at the very least this
somewhat odd behavior needs calling out (and perhaps also justifying) in
the description.

Further, did you think through the interaction with a racing
CONSOLEIO_read? Right now that's the only place where serial_rx_cons is
updated, and hence there was no issue with there being multiple reads
of the variable (perhaps unless a domain issued racing CONSOLEIO_read).
That changes now. I can't convince myself (yet) that's entirely safe,
and hence if it is that also wants discussing in the description.

> @@ -730,6 +732,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 *input;
>  
>      while ( count > 0 )
>      {
> @@ -742,18 +745,28 @@ 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) )
> +        input = console_get_domain();
> +        if ( input && cd == input )
>          {
> +            struct domain_console *cons = cd->console;
> +
> +            if ( cons->idx )
> +            {
> +                console_send(cons->buf, cons->idx, flags);
> +                cons->idx = 0;
> +            }

I probably should have said so on v1 already: What is this about? There's
no comment and nothing in the description that I could associate with this
code.

And then - is this safe without holding cons->lock?

> @@ -815,6 +829,13 @@ long do_console_io(
>          if ( count > INT_MAX )
>              break;
>  
> +        d = console_get_domain();
> +        if ( d != current->domain )
> +        {
> +            console_put_domain(d);
> +            return 0;
> +        }

As of here d == current domain. Why are you holding onto ...

>          rc = 0;
>          while ( (serial_rx_cons != serial_rx_prod) && (rc < count) )
>          {
> @@ -826,12 +847,14 @@ long do_console_io(
>                  len = count - rc;
>              if ( copy_to_guest_offset(buffer, rc, &serial_rx_ring[idx], len) 
> )
>              {
> +                console_put_domain(d);
>                  rc = -EFAULT;
>                  break;
>              }
>              rc += len;
>              serial_rx_cons += len;
>          }
> +        console_put_domain(d);
>          break;

... the domain for this long (requiring multiple console_put_domain()
invocations)? The current domain can't go away under your feet. Hence imo
a single (early) call will do.

Jan