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

[Julien Grall <julien.grall@arm.com>]

Hi Stefano,

On 07/07/2018 00:12, Stefano Stabellini wrote:
> Today Ctrl-AAA is used to switch between Xen and Dom0. Extend the
> mechanism to allow for switching between Xen, Dom0, and any of the
> initial DomU created from Xen alongside Dom0 out of information provided
> via device tree.
> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> CC: andrew.cooper3@citrix.com
> CC: George.Dunlap@eu.citrix.com
> CC: ian.jackson@eu.citrix.com
> CC: jbeulich@suse.com
> CC: konrad.wilk@oracle.com
> CC: tim@xen.org
> CC: wei.liu2@citrix.com
> ---
> Changes in v2:
> - only call vpl011_rx_char if the vpl011 has been initialized
> ---
>   xen/drivers/char/console.c | 50 ++++++++++++++++++++++++++++++++--------------
>   1 file changed, 35 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 0f05369..d590bc0 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -31,10 +31,13 @@
>   #include <xen/early_printk.h>
>   #include <xen/warning.h>
>   #include <xen/pv_console.h>
> +#include <asm/setup.h>
>   
>   #ifdef CONFIG_X86
>   #include <xen/consoled.h>
>   #include <asm/guest.h>
> +#else
> +#include <asm/vpl011.h>
>   #endif
>   
>   /* console: comma-separated list of console outputs. */
> @@ -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. */
>   
>   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);
> +
>       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 )
> +            serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
> +    }
> +#ifdef CONFIG_ARM
> +    else
> +    {
> +        struct domain *d = get_domain_by_id(xen_rx - 1);
> +        if ( !d->arch.vpl011.ring_enable && d->arch.vpl011.inring != NULL )

It sounds like you want to provide a macro here. So would make clearer 
what you are actually checking.

> +            vpl011_rx_char(d, c);

What happen in the case vpl011 is not enabled for that domain? Shouldn't 
you at least warn there are no console available?

> +    }
> +#endif
>       /* Always notify the guest: prevents receive path from getting stuck. */

This comment seems a bit wrong now, altough the comment seem to suggest 
may need to be taken care.

>       send_global_virq(VIRQ_CONSOLE);
>   
> @@ -923,7 +946,7 @@ void __init console_endboot(void)
>        * a useful 'how to switch' message.
>        */
>       if ( opt_conswitch[1] == 'x' )
> -        xen_rx = !xen_rx;
> +        xen_rx = 0;
>   
>       register_keyhandler('w', dump_console_ring_key,
>                           "synchronously dump console ring buffer (dmesg)", 0);
> @@ -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();
>   }
>   
>   int __init console_has(const char *device)
> 

Cheers,

-- 
Julien Grall

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