Re: [Xen-devel] [PATCH v2 20/21] xen: support console_switching between Dom0 and DomUs on ARM

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 17 Jul 2018, Jan Beulich wrote:
> >>> On 16.07.18 at 23:55, <sstabellini@xxxxxxxxxx> wrote:
> > On Mon, 16 Jul 2018, Jan Beulich wrote:
> >> >>> On 07.07.18 at 01:12, <sstabellini@xxxxxxxxxx> wrote:
> >> > @@ -389,29 +392,49 @@ static void dump_console_ring_key(unsigned char 
> >> > key)
> >> >      free_xenheap_pages(buf, order);
> >> >  }
> >> >  
> >> > -/* CTRL-<switch_char> switches input direction between Xen and DOM0. */
> >> > +/*
> >> > + * CTRL-<switch_char> switches input direction between Xen, Dom0 and
> >> > + * DomUs.
> >> > + */
> >> >  #define switch_code (opt_conswitch[0]-'a'+1)
> >> > -static int __read_mostly xen_rx = 1; /* FALSE => input passed to domain 
> >> > 0. */
> >> > +static int __read_mostly xen_rx = 1; /* 1 => input passed to domain 0. 
> >> > */
> >> 
> >> I guess this variable wants renaming now.
> > 
> > Yeah. What about `console_rx'? 
> 
> That's reasonable. Another option would be to use rx_domid, and store
> actual domain IDs there (DOMID_XEN for Xen). That'll make the switching
> logic slightly larger, but perhaps provide a better understandable end
> result.

What about keeping the logic simple and adding a better comment to
explain how console_rx works:

/*
 * console_rx=0 => input to xen
 * console_rx=1 => input to dom0
 * console_rx=N => input dom(N-1)
 */


> >> > @@ -933,9 +956,6 @@ void __init console_endboot(void)
> >> >                              "decrease log level threshold", 0);
> >> >      register_irq_keyhandler('G', &do_toggle_guest,
> >> >                              "toggle host/guest log level adjustment", 
> >> > 0);
> >> > -
> >> > -    /* Serial input is directed to DOM0 by default. */
> >> > -    switch_serial_input();
> >> 
> >> This removes an imo helpful boot time message. Is that intentional,
> >> and if so why?
> >  
> > Yes, it was intentional. switch_serial_input increases xen_rx, I thought
> > it didn't make too much sense to do that at boot, and would be clearer
> > to just initialize xen_rx to the wanted value from the get go (the value
> > would be 1 for dom0). Also, in previous implementations of this patch it
> > was actually required, but not anymore.
> > 
> > In fact, if you prefer, I could also keep this switch_serial_input()
> > call as-is and change the initial value of xen_rx to 0. That would also
> > work, as the increase of xen_rx here would end up selecting still dom0
> > for input.
> 
> Yes, I'd prefer if you left it untouched. The idea after all is that prior
> to reaching here input goes to Xen, and only from here on it goes to
> Dom0.
 
OK

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel