Re: [PATCH] xen/console: Handle true dom0less case when switching serial input

[Michal Orzel <michal.orzel@xxxxxxx>]

On 16/03/2023 08:24, Jan Beulich wrote:
> 
> 
> On 16.03.2023 02:49, Stefano Stabellini wrote:
>> On Wed, 15 Mar 2023, Michal Orzel wrote:
>>> On 15/03/2023 14:11, Jan Beulich wrote:
>>>> On 15.03.2023 13:34, Michal Orzel wrote:
>>>>> On 14/03/2023 16:17, Jan Beulich wrote:
>>>>>> On 14.03.2023 15:27, Michal Orzel wrote:
>>>>>>> --- a/xen/drivers/char/console.c
>>>>>>> +++ b/xen/drivers/char/console.c
>>>>>>> @@ -491,6 +491,14 @@ static void switch_serial_input(void)
>>>>>>>      else
>>>>>>>      {
>>>>>>>          console_rx++;
>>>>>>> +
>>>>>>> +        /*
>>>>>>> +         * Skip switching serial input to hardware domain if it does 
>>>>>>> not exist
>>>>>>> +         * (i.e. true dom0less mode).
>>>>>>> +         */
>>>>>>> +        if ( !hardware_domain && (console_rx == 1) )
>>>>>>> +            console_rx++;
>>>>>>
>>>>>> The consumers of this variable aren't really serialized with this
>>>>>> updating. That's probably okay-ish prior to your change, but now
>>>>>> there can be two updates in rapid succession. I think it would be
>>>>>> better if the variable was written only once here.
>>>>> ok, makes sense.
>>>>>
>>>>>>
>>>>>>>          printk("*** Serial input to DOM%d", console_rx - 1);
>>>>>>
>>>>>> When invoked from console_endboot() this will now switch to Dom1,
>>>>>> i.e. that domain becomes kind of "preferred", which I think is
>>>>>> wrong. Instead I think in such a case we should direct input to
>>>>>> Xen by default.
>>>>> Switching serial input to the first usable domain is the major motivation 
>>>>> behind this patch.
>>>>> The number of times I got pinged by users with *apparent* Xen issue on 
>>>>> true dom0less
>>>>> just because input was directed to dom0 which was not there (not everyone 
>>>>> seems to read the
>>>>> boot logs) forced me to create this patch and manifests that this is not 
>>>>> the behavior user wants.
>>>>> Switching to Xen console would not help at all. Also, we already have a 
>>>>> way to set switch code to 'x'
>>>>> to default serial input to Xen.
>>>>> So I think what I did (switching to the first existing domain) should be 
>>>>> the default behavior (just like it was done for dom0).
>>>>
>>>> Well, I'm not going to stand in the way, but if one of several supposedly
>>>> equal domains is to be "preferred" in some way, then I for one would
>>>> expect justification for doing so. If that's the route to go, then the
>>>> patch snippet you provided looks good to me.
>>> Well, the explanation is that we are directing input to the first existing 
>>> domain
>>> which also is the first domain created (this is what users expect at least 
>>> by default).
>>> This for now creates simplest/cleanest solution that matches the current 
>>> behavior
>>> with dom0 and solves the users inconveniences I mentioned earlier.
>>> There is no other real preference apart from being first domain created and 
>>> to help keep the order simple.
>>
>> My two cents. Given the feedback we are getting from users, I think it
>> is important that the input goes to a real valid domain (not Xen, not
>> Dom0 that doesn't exist). "Which dom0less domain?" is a valid question,
>> and I don't know what would be the best answer. But any of those domains
>> would be better than what we have today.
> 
> Could boot time configuration perhaps indicate which domain to "prefer"?
> Otherwise I'm pretty inclined to suggest to make it actually random ...
Random is not great because in a system with e.g. 20 domUs and directing to 
e.g. 10th domU
user would have to go through a lot of domains executing CTRL-AAA several 
times. Also, for me
it would be difficult to reason about as such approach is definitely not 
something users expect.

May I ask so that we proceed with a patch I proposed to start from the first 
usable domain
to match the current behavior and to help users?. In the meantime I will start 
working on
a support for indicating which domain to prefer.
All in all, the new patch would touch console_endboot() and not 
switch_serial_input().

~Michal