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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 12 Jun 2025, Jan Beulich wrote:
> On 11.06.2025 21:07, Stefano Stabellini wrote:
> > 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.
> 
> Why would this be not clear? We already prefix their output with "(d<N>)"
> when going the traditional way. The same would then apply to output
> coming through the emulated UART.
>
> > 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).
> 
> Leaving aside that "dom0 in the dom0less" ought to really be not-a-thing,
> I disagree. Present behavior of not prefixing the domain's output which
> has input focus continues to make sense. That requires Dom0 to have a
> prefix whenever it doesn't have input focus.

If I understood correctly I like your proposal. Let me rephrase it to
make sure we are aligned. You are suggesting that:

- domains without input focus will print with a (d<N>) prefix
- the domain with input focus will print without a (d<N>) prefix
- this applies to both DomUs and Dom0
- this applies to both predefined domains and also dynamic domains

I am OK with that. I believe this is not the current behavior on ARM but
I can appreciate the simple consistency of it.