Re: [PATCH v2 19/35] xen/console: introduce console_set_owner()

[Denis Mukhin <dmkhn@xxxxxxxxx>]

On Tuesday, December 10th, 2024 at 7:02 AM, Jan Beulich <jbeulich@xxxxxxxx> 
wrote:

>
>
> On 06.12.2024 05:41, Denis Mukhin via B4 Relay wrote:
>
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -463,82 +463,100 @@ static void cf_check dump_console_ring_key(unsigned 
> > char key)
> >
> > /*
> > * CTRL-<switch_char> changes input direction, rotating among Xen, Dom0,
> > - * and the DomUs started from Xen at boot.
> > + * and the DomUs.
> > /
> > #define switch_code (opt_conswitch[0]-'a'+1)
> > +
> > /
> > - * console_owner=0 => input to xen
> > - * console_owner=1 => input to dom0 (or the sole shim domain)
> > - * console_owner=N => input to dom(N-1)
> > + * Current console owner domain ID: either Xen or domain w/ d->is_console 
> > ==
> > + * true.
>
>
> The switching of number space may better have been a separate patch.
> Albeit maybe I'm just not seeing why it wants combining with the
> introduction of console_set_owner().

This is the part I tried in different variations and finally
ended up w/ plumbing new console owner IDs "address space"
here: console_set_owner() takes domid_t and I decided against intermediate
patch, since it is not a big (in term of lines of code) change.

>
> Actually, is this switching actually complete? What about late hwdom,
> which has a non-zero domain ID?

I did reworks for max_init_domid in v3, I believe it should address late
hwdom scenario.

>
> > + * Initialized in console_endboot().
> > */
> > -static unsigned int __read_mostly console_owner = 0;
> > +static domid_t __read_mostly console_owner;
> >
> > -#define max_console_rx (max_init_domid + 1)
> > +static struct domain *rcu_lock_domain_console_by_id(domid_t domid)
>
>
> I think "domain" and "console" want switching in the name, as it's a
> domain you're locking, not a console.

Renamed to "console_get_domain_by_id".

>
> > +int console_set_owner(domid_t domid)
>
>
> static? Iirc Misra doesn't like non-static functions which aren't called
> from any other CU.

Yes, but there's a follow on patch which will undo static - hwdom_crashconsole=
patch - to drop the user to xen console once dom0 has crashed.
So since there's a need in globally visible symbol, I decided to get rid of 
static
right away.

>
> Jan