Re: [PATCH v2] xen/console: Skip switching serial input to non existing domains

[Michal Orzel <michal.orzel@xxxxxxx>]

On 16/03/2023 12:11, Jan Beulich wrote:
> Caution: This message originated from an External Source. Use proper caution 
> when opening attachments, clicking links, or responding.
> 
> 
> On 16.03.2023 11:26, Michal Orzel wrote:
>> --- 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);
>>      }
> 
> While at the first glance (when you sent it in reply to v1) it looked okay,
> I'm afraid it really isn't: Please consider what happens when the last of
> the DomU-s doesn't exist anymore. (You don't really check whether it still
> exists, because the range check comes ahead of the existence one.) In that
> case you want to move from second-to-last to Xen. I expect the entire
> if/else construct wants to be inside the loop.
I did this deliberately because I do not think the situation you describe is 
possible
(i.e. no domains at all - Xen still usable). With hardware domain in place, we 
can e.g. destroy the domain
which would invoke domain_kill() -> domain_destroy() that would free domain 
struct.
Without hwdom, the domain cannot kill/destroy itself. It can do the shutdown 
but it will not
destroy it (at least this is what I tested). So I do not think there can be a 
scenario where
there is not a single domain while Xen running and be usable.

~Michal