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

[Jan Beulich <jbeulich@xxxxxxxx>]

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=".

Jan

> - 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.