From: Manuel Bouyer <bouyer@antioche.eu.org>

The use of rcu_lock_domain_by_id() right in switch_serial_input() makes
assumptions about domain IDs which don't hold when in shim mode: The
sole (initial) domain there has a non-zero ID. Obtain the real domain ID
in that case (generalized as get_initial_domain_id() returns zero when
not in shim mode).

Note that console_input_domain() isn't altered, for not being used when
in shim mode (or more generally on x86).

Fixes: c2581c58bec9 ("xen/console: skip switching serial input to non existing domains")
Signed-off-by: Manuel Bouyer <bouyer@antioche.eu.org>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -468,7 +468,7 @@ static void cf_check dump_console_ring_k
 #define switch_code (opt_conswitch[0]-'a'+1)
 /*
  * console_rx=0 => input to xen
- * console_rx=1 => input to dom0
+ * console_rx=1 => input to dom0 (or the sole shim domain)
  * console_rx=N => input to dom(N-1)
  */
 static unsigned int __read_mostly console_rx = 0;
@@ -493,6 +493,7 @@ static void switch_serial_input(void)
      */
     for ( ; ; )
     {
+        domid_t domid;
         struct domain *d;
 
         if ( next_rx++ >= max_console_rx )
@@ -502,12 +503,18 @@ static void switch_serial_input(void)
             break;
         }
 
-        d = rcu_lock_domain_by_id(next_rx - 1);
+#ifdef CONFIG_PV_SHIM
+        if ( next_rx == 1 )
+            domid = get_initial_domain_id();
+        else
+#endif
+            domid = next_rx - 1;
+        d = rcu_lock_domain_by_id(domid);
         if ( d )
         {
             rcu_unlock_domain(d);
             console_rx = next_rx;
-            printk("*** Serial input to DOM%u", next_rx - 1);
+            printk("*** Serial input to DOM%u", domid);
             break;
         }
     }

Hi Jan,

On 18/10/2023 15:58, Jan Beulich wrote:
> From: Manuel Bouyer <bouyer@antioche.eu.org>
> 
> The use of rcu_lock_domain_by_id() right in switch_serial_input() makes
> assumptions about domain IDs which don't hold when in shim mode: The
> sole (initial) domain there has a non-zero ID. Obtain the real domain ID
> in that case (generalized as get_initial_domain_id() returns zero when
> not in shim mode).
> 
> Note that console_input_domain() isn't altered, for not being used when
> in shim mode (or more generally on x86).

I think it would be worth to either add a comment in 
console_input_domain() and/or #ifdef the code. In any case...

> 
> Fixes: c2581c58bec9 ("xen/console: skip switching serial input to non existing domains")
> Signed-off-by: Manuel Bouyer <bouyer@antioche.eu.org>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

...

Reviewed-by: Julien Grall <jgrall@amazon.com>


Also, should we consider it for xen 4.18? (I notice there is no for-4.18 
tag).

Cheers,

-- 
Julien Grall

On 18.10.2023 20:00, Julien Grall wrote:
> On 18/10/2023 15:58, Jan Beulich wrote:
>> From: Manuel Bouyer <bouyer@antioche.eu.org>
>>
>> The use of rcu_lock_domain_by_id() right in switch_serial_input() makes
>> assumptions about domain IDs which don't hold when in shim mode: The
>> sole (initial) domain there has a non-zero ID. Obtain the real domain ID
>> in that case (generalized as get_initial_domain_id() returns zero when
>> not in shim mode).
>>
>> Note that console_input_domain() isn't altered, for not being used when
>> in shim mode (or more generally on x86).
> 
> I think it would be worth to either add a comment in 
> console_input_domain() and/or #ifdef the code. In any case...

I have such a patch already, but intend to submit only for post-4.18.

>> Fixes: c2581c58bec9 ("xen/console: skip switching serial input to non existing domains")
>> Signed-off-by: Manuel Bouyer <bouyer@antioche.eu.org>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> ...
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>

Thanks.

> Also, should we consider it for xen 4.18? (I notice there is no for-4.18 
> tag).

Of course we should, as it's a regression. I probably should have added
the tag, despite my dislike for such. Intention was imo nevertheless
clear, by me having Cc-ed Henry.

Jan

Hi Jan,

> On Oct 19, 2023, at 14:35, Jan Beulich <jbeulich@suse.com> wrote:
> 
>> Also, should we consider it for xen 4.18? (I notice there is no for-4.18 
>> tag).
> 
> Of course we should, as it's a regression. I probably should have added
> the tag, despite my dislike for such. Intention was imo nevertheless
> clear, by me having Cc-ed Henry.

I think I’ve provided the release-ack for this patch so please kindly commit
this to fix the staging :) Thank you very much!

Kind regards,
Henry

> 
> Jan

Hi,

> On Oct 19, 2023, at 02:00, Julien Grall <julien@xen.org> wrote:
> 
> Hi Jan,
> 
> On 18/10/2023 15:58, Jan Beulich wrote:
>> From: Manuel Bouyer <bouyer@antioche.eu.org>
>> The use of rcu_lock_domain_by_id() right in switch_serial_input() makes
>> assumptions about domain IDs which don't hold when in shim mode: The
>> sole (initial) domain there has a non-zero ID. Obtain the real domain ID
>> in that case (generalized as get_initial_domain_id() returns zero when
>> not in shim mode).
>> Note that console_input_domain() isn't altered, for not being used when
>> in shim mode (or more generally on x86).
> 
> I think it would be worth to either add a comment in console_input_domain() and/or #ifdef the code. In any case...
> 
>> Fixes: c2581c58bec9 ("xen/console: skip switching serial input to non existing domains")
>> Signed-off-by: Manuel Bouyer <bouyer@antioche.eu.org>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> ...
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>
> 
> 
> Also, should we consider it for xen 4.18? (I notice there is no for-4.18 tag).

Yes, so:

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry


> 
> Cheers,
> 
> -- 
> Julien Grall