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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 19 Jan 2026, Jan Beulich wrote:
> On 14.01.2026 01:39, Stefano Stabellini wrote:
> > @@ -815,6 +831,11 @@ long do_console_io(
> >          if ( count > INT_MAX )
> >              break;
> >  
> > +        d = console_get_domain();
> > +        console_put_domain(d);
> > +        if ( d != current->domain )
> > +            return 0;
> 
> This isn't atomic (as in: in a suitably locked region) with ...
> 
> > @@ -830,7 +851,10 @@ long do_console_io(
> >                  break;
> >              }
> >              rc += len;
> > -            serial_rx_cons += len;
> > +            nrspin_lock_irq(&console_lock);
> > +            if ( serial_rx_cons != serial_rx_prod )
> > +                serial_rx_cons += len;
> > +            nrspin_unlock_irq(&console_lock);
> >          }
> >          break;
> 
> ... this. If the focus domain changes after the check in the earlier hunk,
> I think you need to also return with no input here. Or you need to acquire
> the lock earlier (and then similarly in console_switch_input()), albeit
> that would then mean holding it across a copy-to-guest. Which technically
> is perhaps not a problem, but it leaves an uneasy feeling.

I thought about it when writing this patch and I had the same feeling as
you. However, especially considering the other feedback, I don't see
another viable solution.

I'll extend the locking.


> In no case may you continue the loop if the focus domain changed, or else
> you potentially hand the old one input targeted at the new one.
> 
> For context: What caught my attention is the conditional inside the locked
> region. This, imo, shouldn't be necessary when everything else is properly
> structured.
> 
> Actually, the lock strictly needs holding now across all accesses to
> serial_rx_{cons,prod}. That'll then naturally make the conditional
> unnecessary.