Re: [Xen-devel] [PATCH v5 23/25] xen/vpl011: buffer out chars when the backend is xen

[Oleksandr Andrushchenko <andr2000@xxxxxxxxx>]

On 10/23/2018 05:03 AM, Stefano Stabellini wrote:
> To avoid mixing the output of different domains on the console, buffer
> the output chars and print line by line. Unless the domain has input
> from the serial, in which case we want to print char by char for a
> smooth user experience.
>
> The size of SBSA_UART_OUT_BUF_SIZE is arbitrary, choose the same size
> as VUART_BUT_SIZE used in vuart.c.
s/VUART_BUT_SIZE/VUART_BUF_SIZE
> Export a function named console_input_domain() to allow others to know
> which domains has input at a given time.
>
> Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> CC: andrew.cooper3@xxxxxxxxxx
> CC: George.Dunlap@xxxxxxxxxxxxx
> CC: ian.jackson@xxxxxxxxxxxxx
> CC: jbeulich@xxxxxxxx
> CC: konrad.wilk@xxxxxxxxxx
> CC: tim@xxxxxxx
> CC: wei.liu2@xxxxxxxxxx
> ---
> XXX: merge this patch with "xen/arm: Allow vpl011 to be used by DomU" on
>       commit
>
> Changes in v5:
> - use rcu_lock in console_input_domain
> - rcu_unlock at the end of vpl011_write_data_xen
> - add a comment on top of console_input_domain as a reminder that the
>    caller needs to rcu_unlock
>
> Changes in v4:
> - make SBSA_UART_OUT_BUF_SIZE the same size of VUART_BUT_SIZE
> - rearrange the code to be clearer input and != input cases
> - code style
> - add \n when printing the out buffer because is full
> - don't print prefix for input domain
> ---
>   xen/arch/arm/vpl011.c        | 36 +++++++++++++++++++++++++++++++++---
>   xen/drivers/char/console.c   |  8 ++++++++
>   xen/include/asm-arm/vpl011.h |  4 ++++
>   xen/include/xen/console.h    |  2 ++
>   4 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index 131507e..f7db18b 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -28,6 +28,7 @@
>   #include <xen/lib.h>
>   #include <xen/mm.h>
>   #include <xen/sched.h>
> +#include <xen/console.h>
>   #include <public/domctl.h>
>   #include <public/io/console.h>
>   #include <asm/pl011-uart.h>
> @@ -85,18 +86,47 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t 
> data)
>   {
>       unsigned long flags;
>       struct vpl011 *vpl011 = &d->arch.vpl011;
> +    struct vpl011_xen_backend *intf = vpl011->backend.xen;
> +    struct domain *input = console_input_domain();
>   
>       VPL011_LOCK(d, flags);
>   
> -    printk("%c", data);
> -    if (data == '\n')
> -        printk("DOM%u: ", d->domain_id);
> +    intf->out[intf->out_prod++] = data;
> +    if ( d == input )
> +    {
> +        if ( intf->out_prod == 1 )
> +        {
> +            printk("%c", data);
> +            intf->out_prod = 0;
> +        }
> +        else
> +        {
> +            if ( data != '\n' )
> +                intf->out[intf->out_prod++] = '\n';
> +            intf->out[intf->out_prod++] = '\0';
> +            printk("%s", intf->out);
> +            intf->out_prod = 0;
> +        }
> +    }
> +    else
> +    {
> +        if ( intf->out_prod == SBSA_UART_OUT_BUF_SIZE - 2 ||
> +             data == '\n' )
> +        {
> +            if ( data != '\n' )
> +                intf->out[intf->out_prod++] = '\n';
> +            intf->out[intf->out_prod++] = '\0';
> +            printk("DOM%u: %s", d->domain_id, intf->out);
> +            intf->out_prod = 0;
> +        }
> +    }
>   
>       vpl011->uartris |= TXI;
>       vpl011->uartfr &= ~TXFE;
>       vpl011_update_interrupt_status(d);
>   
>       VPL011_UNLOCK(d, flags);
> +    rcu_unlock_domain(input);
input can be NULL. Although it won't hurt with the existing
code base things could change any time soon...
>   }
>   
>   /*
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 990c51c..eb1cc1b 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -406,6 +406,14 @@ static void dump_console_ring_key(unsigned char key)
>    */
>   static unsigned int __read_mostly console_rx = 0;
>   
> +/* Make sure to rcu_unlock_domain after use */
> +struct domain *console_input_domain(void)
> +{
> +    if ( console_rx == 0 )
> +            return NULL;
> +    return rcu_lock_domain_by_id(console_rx - 1);
> +}
> +
>   static void switch_serial_input(void)
>   {
>       if ( console_rx++ == max_init_domid + 1 )
> diff --git a/xen/include/asm-arm/vpl011.h b/xen/include/asm-arm/vpl011.h
> index 5eb6d25..ab6fd79 100644
> --- a/xen/include/asm-arm/vpl011.h
> +++ b/xen/include/asm-arm/vpl011.h
> @@ -30,9 +30,13 @@
>   #define VPL011_UNLOCK(d,flags) spin_unlock_irqrestore(&(d)->arch.vpl011.lock, 
> flags)
>   
>   #define SBSA_UART_FIFO_SIZE 32
> +/* Same size as VUART_BUT_SIZE, used in vuart.c */
> +#define SBSA_UART_OUT_BUF_SIZE 128
>   struct vpl011_xen_backend {
>       char in[SBSA_UART_FIFO_SIZE];
> +    char out[SBSA_UART_OUT_BUF_SIZE];
>       XENCONS_RING_IDX in_cons, in_prod;
> +    XENCONS_RING_IDX out_prod;
>   };
>   
>   struct vpl011 {
> diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h
> index 70c9911..c5a85c8 100644
> --- a/xen/include/xen/console.h
> +++ b/xen/include/xen/console.h
> @@ -31,6 +31,8 @@ void console_end_sync(void);
>   void console_start_log_everything(void);
>   void console_end_log_everything(void);
>   
> +struct domain *console_input_domain(void);
> +
>   /*
>    * Steal output from the console. Returns +ve identifier, else -ve error.
>    * Takes the handle of the serial line to steal, and steal callback function.

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