[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi,

On 05/10/2018 19:47, 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.

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

That's not going to make the series bisectable as it depends on an intermediate patch for console_input_domain().

The new logic looks better to me, few comments below.


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        | 35 ++++++++++++++++++++++++++++++++---
  xen/drivers/char/console.c   |  7 +++++++
  xen/include/asm-arm/vpl011.h |  4 ++++
  xen/include/xen/console.h    |  2 ++
  4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 131507e..5e57ada 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,12 +86,40 @@ 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;
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 0808bac..9a2b29a 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -406,6 +406,13 @@ static void dump_console_ring_key(unsigned char key)
   */
  static unsigned int __read_mostly console_rx = 0;
+struct domain *console_input_domain(void)
+{
+    if ( console_rx == 0 )
+            return NULL;
+    return get_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 */

s/BUT/BUF/

+#define SBSA_UART_OUT_BUF_SIZE 128

You could directly use VUART_BUF_SIZE here to avoid the comment above.

  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.


--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.