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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

>  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]);
> +    xen_rx++;
> +    if ( xen_rx == max_init_domid + 1 )
> +        xen_rx = 0;
> +
> +    if ( !xen_rx )
> +        printk("*** Serial input xen_rx=%d -> %s", xen_rx, "Xen");
> +    else
> +        printk("*** Serial input xen_rx=%d -> DOM%d", xen_rx, xen_rx - 1);

What are the xen_rx= doing in the format string? They weren't there before.

>      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 )
> +    if ( xen_rx == 0 )
>          return handle_keypress(c, regs);
>  
> -    /* Deliver input to guest 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;
> +    if ( xen_rx == 1 )
> +    {
> +        /* Deliver input to guest buffer, unless it is already full. */
> +        if ( (serial_rx_prod-serial_rx_cons) != SERIAL_RX_SIZE )

Please add blanks around the - .

> +            serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
> +    }
> +#ifdef CONFIG_ARM

CONFIG_HAS_PL011 ?

> +    else
> +    {
> +        struct domain *d = get_domain_by_id(xen_rx - 1);
> +        if ( !d->arch.vpl011.ring_enable && d->arch.vpl011.inring != NULL )

Blank line between these two lines please.

> @@ -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?

Jan



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