Re: [PATCH v3 2/2] xen/console: unify printout behavior for UART emulators

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 11 Jun 2025, Jan Beulich wrote:
> On 11.06.2025 02:07, dmkhn@xxxxxxxxx wrote:
> > On Tue, Jun 10, 2025 at 10:21:40AM +0200, Jan Beulich wrote:
> >> On 06.06.2025 22:11, dmkhn@xxxxxxxxx wrote:
> >>> From: Denis Mukhin <dmukhin@xxxxxxxx>
> >>>
> >>> If virtual UART from domain X prints on the physical console, the 
> >>> behavior is
> >>> updated to (see [1]):
> >>> - console focus in domain X: do not prefix messages;
> >>> - no console focus in domain X: prefix all messages with "(dX)".
> >>
> >> While, as indicated (much) earlier, I can see why omitting the prefix
> >> may make sense for the domain having input focus, ...
> >>
> >>> --- a/xen/drivers/char/console.c
> >>> +++ b/xen/drivers/char/console.c
> >>> @@ -740,7 +740,17 @@ static long 
> >>> guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
> >>>          if ( is_hardware_domain(cd) )
> >>>          {
> >>>              /* Use direct console output as it could be interactive */
> >>> +            char prefix[16] = "";
> >>> +            struct domain *consd;
> >>> +
> >>> +            consd = console_get_domain();
> >>> +            if ( consd != cd )
> >>> +                snprintf(prefix, sizeof(prefix), "(d%d) ", 
> >>> cd->domain_id);
> >>> +            console_put_domain(consd);
> >>> +
> >>>              nrspin_lock_irq(&console_lock);
> >>> +            if ( prefix[0] != '\0' )
> >>> +                console_send(prefix, strlen(prefix), flags);
> >>>              console_send(kbuf, kcount, flags);
> >>>              nrspin_unlock_irq(&console_lock);
> >>>          }
> >>
> >> ... this, aiui, is a behavioral change for the non-dom0less case, where
> >> Dom0 output will suddenly also gain the prefix. Which I don't think is
> >> wanted: Switching focus between Xen and Dom0 should remain unaffected
> >> in this regard.
> > 
> > This change ensures that dom0 traces aren't mixed with domU traces when domU
> > has input focus, or with Xen traces when the administrator is in the 
> > diagnostic
> > console.
> 
> That's what the description also tries to describe, yet I still regard it as
> a behavioral regression in (at least) the described scenario. The hardware
> domain presently not having (d0) prefixed to its output is deliberate imo,
> not accidental.

If we only consider the classic dom0 and dom0less usage models, then
what you wrote makes perfect sense. In the classic dom0 case, it is best
for dom0 to have no prefix, which is the current behavior.

However, things become more complex with dom0less. As we have discussed
previously, it has already become desirable on both ARM and x86 to align
on the same behavior. During our last discussion, the preference was to
add a '(d0)' prefix to clearly differentiate output from dom0 and other
domains.

Up to now, we could easily detect the two different cases depending on
the boot configuration. The problem arises with Denis' patches, which
add the ability for dynamically created guests via `xl` to access an
emulated NS16550 UART that prints to the console. Because these guests
are created dynamically, it is not clear how we are going to handle
this case.

If we follow the dom0less preference, then we would need a '(d0)' prefix
for dom0. If we follow the classic dom0 model, then dom0 would remain
without a prefix, and the new domUs would have a prefix. This would
cause an inconsistency. However, this is what we have today on ARM with
dom0less.

If Jan feels strongly that we should retain no prefix for the classic
dom0 case, which is understandable, then I believe the best course of
action would be to change our stance on dom0less on both ARM and x86 and
also use no prefix for dom0 in the dom0less case (which is the current
state on ARM).