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

[Michal Orzel <michal.orzel@xxxxxxx>]

On 14/03/2023 16:17, Jan Beulich wrote:
> 
> 
> On 14.03.2023 15:27, Michal Orzel wrote:
>> At the moment, we direct serial input to hardware domain by default.
>> This does not make any sense when running in true dom0less mode, since
>> such domain does not exist. As a result, users wishing to write to
>> an emulated UART of a domU are always forced to execute CTRL-AAA first.
>> The same issue is when rotating among serial inputs, where we always
>> have to go through hardware domain case.
>>
>> Modify switch_serial_input() so that, if there is no hardware domain
>> (i.e. true dom0less case) and console_rx is 1 (i.e. input to hwdom),
>> we skip it and move to the next serial input.
>>
>> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> 
> Makes sense, but I think there are more things to consider.
> 
>> --- 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).

> 
> And then I have a question about Dom<N> lifetime in dom0less: Can
> such domains be short-lived? If yes, what you do here for Dom0
> would likely want generalizing, to skip any domain that doesn't
> exist (anymore).
I think there is nothing preventing such domains not to be short-lived and 
checking
for existence is a good idea. So all in all I would do the following 
modifications:

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index e8468c121ad0..d843b8baf162 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -490,7 +490,24 @@ static void switch_serial_input(void)
     }
     else
     {
-        console_rx++;
+        unsigned int next_rx = console_rx + 1;
+
+        /* Skip switching serial input to non existing domains */
+        while ( next_rx < max_init_domid + 1 )
+        {
+            struct domain *d = rcu_lock_domain_by_id(next_rx - 1);
+
+            if ( d )
+            {
+                rcu_unlock_domain(d);
+                break;
+            }
+
+            next_rx++;
+        }
+
+        console_rx = next_rx;
+
         printk("*** Serial input to DOM%d", console_rx - 1);
     }


~Michal