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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan