| Message ID | ed275abf-351f-5237-7e19-a3856f6d4272@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | console: make input work again for pv-shim | expand |
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,
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
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
--- 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; } }