[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v5 21/25] xen: support console_switching between Dom0 and DomUs on ARM
On Fri, 26 Oct 2018, Jan Beulich wrote: > >>> On 23.10.18 at 04:03, <sstabellini@xxxxxxxxxx> wrote: > > @@ -391,31 +394,79 @@ 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> changes input direction, rotating among Xen, Dom0, > > + * and DomUs. > > Proper attribute to be inserted here, as per my (late) v4 comment. OK, I wrote: * CTRL-<switch_char> changes input direction, rotating among Xen, Dom0, * and the DomUs started from Xen at boot. but I would be happy take suggestion for a simple single word attribute to describe this kind of domains. Maybe "dom0less DomUs"? > > + */ > > #define switch_code (opt_conswitch[0]-'a'+1) > > -static int __read_mostly xen_rx = 1; /* FALSE => input passed to domain 0. > > */ > > +/* > > + * console_rx=0 => input to xen > > + * console_rx=1 => input to dom0 > > + * console_rx=N => input to dom(N-1) > > + */ > > +static unsigned int __read_mostly console_rx = 0; > > > > static void switch_serial_input(void) > > { > > - static char *input_str[2] = { "DOM0", "Xen" }; > > - xen_rx = !xen_rx; > > - printk("*** Serial input -> %s", input_str[xen_rx]); > > + if ( console_rx++ == max_init_domid + 1 ) > > + console_rx = 0; > > + > > + if ( console_rx == 0 ) > > + printk("*** Serial input to Xen"); > > I think these two if()-s can and should be combined. Furthermore > I think it would be worthwhile to defer the increment until ... OK > > + else > > + printk("*** Serial input to DOM%d", console_rx - 1); > > ... after this printk(), but (obviously) then inside the else block. Yes, I can do that > > if ( switch_code ) > > - printk(" (type 'CTRL-%c' three times to switch input to %s)", > > - opt_conswitch[0], input_str[!xen_rx]); > > + printk(" (type 'CTRL-%c' three times to switch input)", > > + opt_conswitch[0]); > > printk("\n"); > > } > > > > static void __serial_rx(char c, struct cpu_user_regs *regs) > > { > > - if ( xen_rx ) > > + switch ( console_rx ) > > + { > > + case 0: > > return handle_keypress(c, regs); > > + case 1: > > + { > > Stray figure braces. Blank lines between case blocks please. OK > > + /* > > + * Deliver input to the hardware domain buffer, unless it is > > + * already full. > > + */ > > + if ( (serial_rx_prod - serial_rx_cons) != SERIAL_RX_SIZE ) > > + serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c; > > + > > + /* > > + * Always notify the hardware domain: prevents receive path from > > + * getting stuck. > > + */ > > + send_global_virq(VIRQ_CONSOLE); > > + break; > > + } > > +#if 0 > > I suppose a later patch is meant to remove this again. But > perhaps still a good idea to attach a brief comment, in case > the series doesn't go in all in one go? This is assuming you > did already consider (and discard for whatever reason) the > option of adding this default case when the code can > actually be used. I'll add a comment _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |