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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.06.2025 02:45, Stefano Stabellini wrote:
> On Wed, 18 Jun 2025, Jan Beulich wrote:
>> On 18.06.2025 02:39, Stefano Stabellini wrote:
>>> 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
>>
>> Except in the non-dom0less case, at least up and until there's at least
>> one other domain. I.e. I'd like to keep Dom0 boot output as it is today,
>> regardless of the presence of e.g. "conswitch=".
> 
> In the non-dom0less case, since dom0 has focus, it would naturally be
> without (d<N>) prefix. Unless the user passes "conswitch=". Honestly, I
> wouldn't special-case conswitch= that way and would prefer keep things
> simple and consistent without corner cases. I don't think conswitch= is
> so widely used that it is worth the complexity to special-case it.

Widely used or not - _I_ use it all the time in debug configs where serial
is available.

Jan