[PATCH 0/8] Emulate virtual UART for DOM0 and some UART clean up

[PATCH 0/8] Emulate virtual UART for DOM0 and some UART clean up

[Julien Grall]

Hi,

This patch series follows the thread http://patches.linaro.org/18358/ on Linux.

In some configuration, the kernel can use hardcoded code to access to the
UART (for instance early printk). If Xen has stolen the UART for its own use,
DOM0 will abort because Xen has not mapped to DOM0 the UART memory.

With this patch series, Xen will "replace" the real UART by a basic virtual
UART for DOM0.

For the moment, this patch series is not able to cooperate with early printk
in guest kernel. To fix it, there is 2 approach:
    - Expose a pl011 UART for guest
    - Implement xen early in Linux

Any ideas on the best solution?

This patch series also contains clean up for the pl011 and exynos4210 drivers.

Cheers,

Julien Grall (8):
  pl011: Use ioreadl/iowritel
  pl011: Move registers' definition in a separate file
  xen/arm: Use define instead of hardcoded value in debug-pl011
  xen/arm: New callback in uart_driver to retrieve serial information
  xen/arm: Implement a virtual UART
  exynos4210: rename UTRSTAT_TX_EMPTY in UTRSTAT_TXFE
  exynos4210: Implement serial_info callback
  pl011: Implement serial_info callback

 xen/arch/arm/Makefile                   |    2 +-
 xen/arch/arm/arm32/debug-exynos4210.inc |    2 +-
 xen/arch/arm/arm32/debug-pl011.inc      |   18 ++--
 xen/arch/arm/domain.c                   |   12 ++-
 xen/arch/arm/io.c                       |    2 +-
 xen/arch/arm/io.h                       |    2 +-
 xen/arch/arm/vpl011.c                   |  152 -------------------------------
 xen/arch/arm/vpl011.h                   |   35 -------
 xen/arch/arm/vuart.c                    |  150 ++++++++++++++++++++++++++++++
 xen/arch/arm/vuart.h                    |   35 +++++++
 xen/drivers/char/exynos4210-uart.c      |   15 +++
 xen/drivers/char/pl011.c                |  115 +++++++++--------------
 xen/drivers/char/serial.c               |    8 ++
 xen/include/asm-arm/domain.h            |   14 +--
 xen/include/asm-arm/exynos4210-uart.h   |    3 +-
 xen/include/asm-arm/pl011-uart.h        |   81 ++++++++++++++++
 xen/include/xen/serial.h                |   13 +++
 17 files changed, 378 insertions(+), 281 deletions(-)
 delete mode 100644 xen/arch/arm/vpl011.c
 delete mode 100644 xen/arch/arm/vpl011.h
 create mode 100644 xen/arch/arm/vuart.c
 create mode 100644 xen/arch/arm/vuart.h
 create mode 100644 xen/include/asm-arm/pl011-uart.h

-- 
1.7.10.4

[PATCH 2/8] pl011: Move registers' definition in a separate file

[Julien Grall]

---
 xen/drivers/char/pl011.c         |   48 +----------------------
 xen/include/asm-arm/pl011-uart.h |   80 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 47 deletions(-)
 create mode 100644 xen/include/asm-arm/pl011-uart.h

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 9456f20..fd87e68 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -28,6 +28,7 @@
 #include <asm/device.h>
 #include <xen/mm.h>
 #include <xen/vmap.h>
+#include <asm/pl011-uart.h>
 
 static struct pl011 {
     unsigned int baud, clock_hz, data_bits, parity, stop_bits;
@@ -41,53 +42,6 @@ static struct pl011 {
     /* bool_t probing, intr_works; */
 } pl011_com = {0};
 
-/* PL011 register addresses */
-#define DR     (0x00)
-#define RSR    (0x04)
-#define FR     (0x18)
-#define ILPR   (0x20)
-#define IBRD   (0x24)
-#define FBRD   (0x28)
-#define LCR_H  (0x2c)
-#define CR     (0x30)
-#define IFLS   (0x34)
-#define IMSC   (0x38)
-#define RIS    (0x3c)
-#define MIS    (0x40)
-#define ICR    (0x44)
-#define DMACR  (0x48)
-
-/* CR bits */
-#define RXE    (1<<9) /* Receive enable */
-#define TXE    (1<<8) /* Transmit enable */
-#define UARTEN (1<<0) /* UART enable */
-
-/* FR bits */
-#define TXFE   (1<<7) /* TX FIFO empty */
-#define RXFE   (1<<4) /* RX FIFO empty */
-
-/* LCR_H bits */
-#define SPS    (1<<7) /* Stick parity select */
-#define FEN    (1<<4) /* FIFO enable */
-#define STP2   (1<<3) /* Two stop bits select */
-#define EPS    (1<<2) /* Even parity select */
-#define PEN    (1<<1) /* Parity enable */
-#define BRK    (1<<0) /* Send break */
-
-/* Interrupt bits (IMSC, MIS, ICR) */
-#define OEI   (1<<10) /* Overrun Error interrupt mask */
-#define BEI   (1<<9)  /* Break Error interrupt mask */
-#define PEI   (1<<8)  /* Parity Error interrupt mask */
-#define FEI   (1<<7)  /* Framing Error interrupt mask */
-#define RTI   (1<<6)  /* Receive Timeout interrupt mask */
-#define TXI   (1<<5)  /* Transmit interrupt mask */
-#define RXI   (1<<4)  /* Receive interrupt mask */
-#define DSRMI (1<<3)  /* nUARTDSR Modem interrupt mask */
-#define DCDMI (1<<2)  /* nUARTDCD Modem interrupt mask */
-#define CTSMI (1<<1)  /* nUARTCTS Modem interrupt mask */
-#define RIMI  (1<<0)  /* nUARTRI Modem interrupt mask */
-#define ALLI  OEI|BEI|PEI|FEI|RTI|TXI|RXI|DSRMI|DCDMI|CTSMI|RIMI
-
 /* These parity settings can be ORed directly into the LCR. */
 #define PARITY_NONE  (0)
 #define PARITY_ODD   (PEN)
diff --git a/xen/include/asm-arm/pl011-uart.h b/xen/include/asm-arm/pl011-uart.h
new file mode 100644
index 0000000..8c4edd4
--- /dev/null
+++ b/xen/include/asm-arm/pl011-uart.h
@@ -0,0 +1,80 @@
+/*
+ * xen/include/asm-arm/pl011-uart.h
+ *
+ * Common constant definition between early printk and the UART driver
+ * for the pl011 UART
+ *
+ * Tim Deegan <tim@xen.org>
+ * Copyright (c) 2011 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __ASM_ARM_PL011_H
+#define __ASM_ARM_PL011_H
+
+/* PL011 register addresses */
+#define DR     (0x00)
+#define RSR    (0x04)
+#define FR     (0x18)
+#define ILPR   (0x20)
+#define IBRD   (0x24)
+#define FBRD   (0x28)
+#define LCR_H  (0x2c)
+#define CR     (0x30)
+#define IFLS   (0x34)
+#define IMSC   (0x38)
+#define RIS    (0x3c)
+#define MIS    (0x40)
+#define ICR    (0x44)
+#define DMACR  (0x48)
+
+/* CR bits */
+#define RXE    (1<<9) /* Receive enable */
+#define TXE    (1<<8) /* Transmit enable */
+#define UARTEN (1<<0) /* UART enable */
+
+/* FR bits */
+#define TXFE   (1<<7) /* TX FIFO empty */
+#define RXFE   (1<<4) /* RX FIFO empty */
+
+/* LCR_H bits */
+#define SPS    (1<<7) /* Stick parity select */
+#define FEN    (1<<4) /* FIFO enable */
+#define STP2   (1<<3) /* Two stop bits select */
+#define EPS    (1<<2) /* Even parity select */
+#define PEN    (1<<1) /* Parity enable */
+#define BRK    (1<<0) /* Send break */
+
+/* Interrupt bits (IMSC, MIS, ICR) */
+#define OEI   (1<<10) /* Overrun Error interrupt mask */
+#define BEI   (1<<9)  /* Break Error interrupt mask */
+#define PEI   (1<<8)  /* Parity Error interrupt mask */
+#define FEI   (1<<7)  /* Framing Error interrupt mask */
+#define RTI   (1<<6)  /* Receive Timeout interrupt mask */
+#define TXI   (1<<5)  /* Transmit interrupt mask */
+#define RXI   (1<<4)  /* Receive interrupt mask */
+#define DSRMI (1<<3)  /* nUARTDSR Modem interrupt mask */
+#define DCDMI (1<<2)  /* nUARTDCD Modem interrupt mask */
+#define CTSMI (1<<1)  /* nUARTCTS Modem interrupt mask */
+#define RIMI  (1<<0)  /* nUARTRI Modem interrupt mask */
+#define ALLI  OEI|BEI|PEI|FEI|RTI|TXI|RXI|DSRMI|DCDMI|CTSMI|RIMI
+
+#endif /* __ASM_ARM_PL011_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.7.10.4

[PATCH 3/8] xen/arm: Use define instead of hardcoded value in debug-pl011

[Julien Grall]

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/arm32/debug-pl011.inc |   18 ++++++++++--------
 xen/include/asm-arm/pl011-uart.h   |    1 +
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/arm32/debug-pl011.inc b/xen/arch/arm/arm32/debug-pl011.inc
index 8b085b8..6a64dbf 100644
--- a/xen/arch/arm/arm32/debug-pl011.inc
+++ b/xen/arch/arm/arm32/debug-pl011.inc
@@ -16,19 +16,21 @@
  * GNU General Public License for more details.
  */
 
+#include <asm/pl011-uart.h>
+
 /* PL011 UART initialization
  * rb: register which contains the UART base address
  * rc: scratch register 1
  * rd: scratch register 2 (unused here) */
 .macro early_uart_init rb, rc, rd
         mov   \rc, #(7372800 / EARLY_PRINTK_BAUD % 16)
-        str   \rc, [\rb, #0x28]     /* -> UARTFBRD (Baud divisor fraction) */
+        str   \rc, [\rb, #FBRD]     /* -> UARTFBRD (Baud divisor fraction) */
         mov   \rc, #(7372800 / EARLY_PRINTK_BAUD / 16)
-        str   \rc, [\rb, #0x24]     /* -> UARTIBRD (Baud divisor integer) */
+        str   \rc, [\rb, #IBRD]     /* -> UARTIBRD (Baud divisor integer) */
         mov   \rc, #0x60            /* 8n1 */
-        str   \rc, [\rb, #0x2C]     /* -> UARTLCR_H (Line control) */
-        ldr   \rc, =0x00000301      /* RXE | TXE | UARTEN */
-        str   \rc, [\rb, #0x30]     /* -> UARTCR (Control Register) */
+        str   \rc, [\rb, #LCR_H]     /* -> UARTLCR_H (Line control) */
+        ldr   \rc, =(RXE | TXE | UARTEN)      /* RXE | TXE | UARTEN */
+        str   \rc, [\rb, #CR]     /* -> UARTCR (Control Register) */
 .endm
 
 /* PL011 UART wait UART to be ready to transmit
@@ -36,8 +38,8 @@
  * rc: scratch register */
 .macro early_uart_ready rb, rc
 1:
-        ldr   \rc, [\rb, #0x18]     /* <- UARTFR (Flag register) */
-        tst   \rc, #0x8             /* Check BUSY bit */
+        ldr   \rc, [\rb, #FR]       /* <- UARTFR (Flag register) */
+        tst   \rc, #BUSY             /* Check BUSY bit */
         bne   1b                    /* Wait for the UART to be ready */
 .endm
 
@@ -45,7 +47,7 @@
  * rb: register which contains the UART base address
  * rt: register which contains the character to transmit */
 .macro early_uart_transmit rb, rt
-        str   \rt, [\rb]            /* -> UARTDR (Data Register) */
+        str   \rt, [\rb, #DR]            /* -> UARTDR (Data Register) */
 .endm
 
 /*
diff --git a/xen/include/asm-arm/pl011-uart.h b/xen/include/asm-arm/pl011-uart.h
index 8c4edd4..3332c51 100644
--- a/xen/include/asm-arm/pl011-uart.h
+++ b/xen/include/asm-arm/pl011-uart.h
@@ -45,6 +45,7 @@
 /* FR bits */
 #define TXFE   (1<<7) /* TX FIFO empty */
 #define RXFE   (1<<4) /* RX FIFO empty */
+#define BUSY   (1<<3) /* Transmit is not complete */
 
 /* LCR_H bits */
 #define SPS    (1<<7) /* Stick parity select */
-- 
1.7.10.4

[PATCH 4/8] xen/arm: New callback in uart_driver to retrieve serial information

[Julien Grall]

There is no way to retrieve basic informations (base address, size, ....) for
an UART. This callback will be used later to partially emulate the real UART
for DOM0 on ARM.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
CC: Keir Fraser <keir@xen.org>
---
 xen/drivers/char/serial.c |    8 ++++++++
 xen/include/xen/serial.h  |   13 +++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
index e1c3f47..7640f8e 100644
--- a/xen/drivers/char/serial.c
+++ b/xen/drivers/char/serial.c
@@ -497,6 +497,14 @@ const struct dt_irq __init *serial_dt_irq(int idx)
     return NULL;
 }
 
+const struct serial_info *serial_info(int idx)
+{
+    if ( (idx >= 0) && (idx < ARRAY_SIZE(com)) &&
+         com[idx].driver && com[idx].driver->info )
+        return com[idx].driver->info(&com[idx]);
+
+    return NULL;
+}
 
 void serial_suspend(void)
 {
diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
index 9caf776..c312032 100644
--- a/xen/include/xen/serial.h
+++ b/xen/include/xen/serial.h
@@ -32,6 +32,14 @@ enum serial_port_state {
     serial_initialized
 };
 
+struct serial_info {
+    unsigned long base_addr;    /* Base address of the UART */
+    unsigned long size;         /* Size of the memory region */
+    unsigned long data_off;     /* Data register offset */
+    unsigned long status_off;   /* Status register offset */
+    unsigned long status;       /* Ready status value */
+};
+
 struct serial_port {
     /* Uart-driver parameters. */
     struct uart_driver *driver;
@@ -74,6 +82,8 @@ struct uart_driver {
     int  (*irq)(struct serial_port *);
     /* Get IRQ device node for this port's serial line: returns NULL if none. */
     const struct dt_irq *(*dt_irq_get)(struct serial_port *);
+    /* Get serial information */
+    const struct serial_info *(*info)(struct serial_port *);
 };
 
 /* 'Serial handles' are composed from the following fields. */
@@ -127,6 +137,9 @@ int serial_irq(int idx);
 /* Return irq device node for specified serial port (identified by index). */
 const struct dt_irq *serial_dt_irq(int idx);
 
+/* Retrieve basic UART information (base address, size, ...) */
+const struct serial_info* serial_info(int idx);
+
 /* Serial suspend/resume. */
 void serial_suspend(void);
 void serial_resume(void);
-- 
1.7.10.4

[PATCH 5/8] xen/arm: Implement a virtual UART

[Julien Grall]

This code is based on the previous vuart0 implementation. Unlike the latter,
it's intend to replace UART stolen by XEN to DOM0 via dtuart=... on its
command line.

It's useful when the kernel is compiled with early printk enabled or for a
single platform. Most of the time, the hardcoded code to handle the UART
will need 2 registers: status and data, the others registers can be
implemented as RAZ/WI.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/Makefile        |    2 +-
 xen/arch/arm/domain.c        |   12 ++--
 xen/arch/arm/io.c            |    2 +-
 xen/arch/arm/io.h            |    2 +-
 xen/arch/arm/vpl011.c        |  152 ------------------------------------------
 xen/arch/arm/vpl011.h        |   35 ----------
 xen/arch/arm/vuart.c         |  150 +++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/vuart.h         |   35 ++++++++++
 xen/include/asm-arm/domain.h |   14 ++--
 9 files changed, 204 insertions(+), 200 deletions(-)
 delete mode 100644 xen/arch/arm/vpl011.c
 delete mode 100644 xen/arch/arm/vpl011.h
 create mode 100644 xen/arch/arm/vuart.c
 create mode 100644 xen/arch/arm/vuart.h

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 5ae5831..6e1208f 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -27,7 +27,7 @@ obj-y += shutdown.o
 obj-y += traps.o
 obj-y += vgic.o
 obj-y += vtimer.o
-obj-y += vpl011.o
+obj-y += vuart.o
 obj-y += hvm.o
 obj-y += device.o
 
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index cfe0fbd..84b2f82 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -32,7 +32,7 @@
 
 #include <asm/gic.h>
 #include "vtimer.h"
-#include "vpl011.h"
+#include "vuart.h"
 
 DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
 
@@ -518,8 +518,12 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
     if ( (rc = vcpu_domain_init(d)) != 0 )
         goto fail;
 
-    /* Domain 0 gets a real UART not an emulated one */
-    if ( d->domain_id && (rc = domain_uart0_init(d)) != 0 )
+    /*
+     * Virtual UART is only used by linux early printk and decompress code.
+     * Only use it for dom0 because the linux kernel may not support
+     * multi-platform.
+     */
+    if ( (d->domain_id == 0) && (rc = domain_vuart_init(d)) )
         goto fail;
 
     return 0;
@@ -535,7 +539,7 @@ void arch_domain_destroy(struct domain *d)
 {
     p2m_teardown(d);
     domain_vgic_free(d);
-    domain_uart0_free(d);
+    domain_vuart_free(d);
     free_xenheap_page(d->shared_info);
 }
 
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index ad28c26..a6db00b 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -25,7 +25,7 @@
 static const struct mmio_handler *const mmio_handlers[] =
 {
     &vgic_distr_mmio_handler,
-    &uart0_mmio_handler,
+    &vuart_mmio_handler,
 };
 #define MMIO_HANDLER_NR ARRAY_SIZE(mmio_handlers)
 
diff --git a/xen/arch/arm/io.h b/xen/arch/arm/io.h
index 661dce1..8d252c0 100644
--- a/xen/arch/arm/io.h
+++ b/xen/arch/arm/io.h
@@ -41,7 +41,7 @@ struct mmio_handler {
 };
 
 extern const struct mmio_handler vgic_distr_mmio_handler;
-extern const struct mmio_handler uart0_mmio_handler;
+extern const struct mmio_handler vuart_mmio_handler;
 
 extern int handle_mmio(mmio_info_t *info);
 
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
deleted file mode 100644
index 13ba623..0000000
--- a/xen/arch/arm/vpl011.c
+++ /dev/null
@@ -1,152 +0,0 @@
-/*
- * xen/arch/arm/vpl011.c
- *
- * ARM PL011 UART Emulator (DEBUG)
- *
- * Ian Campbell <ian.campbell@citrix.com>
- * Copyright (c) 2012 Citrix Systems.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-/*
- * This is not intended to be a full emulation of a PL011
- * device. Rather it is intended to provide a sufficient veneer of one
- * that early code (such as Linux's boot time decompressor) which
- * hardcodes output directly to such a device are able to make progress.
- *
- * This device is not intended to be enumerable or exposed to the OS
- * (e.g. via Device Tree).
- */
-
-#include <xen/config.h>
-#include <xen/lib.h>
-#include <xen/sched.h>
-#include <xen/errno.h>
-#include <xen/ctype.h>
-
-#include "io.h"
-
-#define UART0_START 0x1c090000
-#define UART0_END   (UART0_START+65536)
-
-#define UARTDR 0x000
-#define UARTFR 0x018
-
-int domain_uart0_init(struct domain *d)
-{
-    ASSERT( d->domain_id );
-
-    spin_lock_init(&d->arch.uart0.lock);
-    d->arch.uart0.idx = 0;
-
-    d->arch.uart0.buf = xzalloc_array(char, VPL011_BUF_SIZE);
-    if ( !d->arch.uart0.buf )
-        return -ENOMEM;
-
-    return 0;
-
-}
-
-void domain_uart0_free(struct domain *d)
-{
-    xfree(d->arch.uart0.buf);
-}
-
-static void uart0_print_char(char c)
-{
-    struct vpl011 *uart = &current->domain->arch.uart0;
-
-    /* Accept only printable characters, newline, and horizontal tab. */
-    if ( !isprint(c) && (c != '\n') && (c != '\t') )
-        return ;
-
-    spin_lock(&uart->lock);
-    uart->buf[uart->idx++] = c;
-    if ( (uart->idx == (VPL011_BUF_SIZE - 2)) || (c == '\n') )
-    {
-        if ( c != '\n' )
-            uart->buf[uart->idx++] = '\n';
-        uart->buf[uart->idx] = '\0';
-        printk(XENLOG_G_DEBUG "DOM%u: %s",
-               current->domain->domain_id, uart->buf);
-        uart->idx = 0;
-    }
-    spin_unlock(&uart->lock);
-}
-
-static int uart0_mmio_check(struct vcpu *v, paddr_t addr)
-{
-    struct domain *d = v->domain;
-
-    return d->domain_id != 0 && addr >= UART0_START && addr < UART0_END;
-}
-
-static int uart0_mmio_read(struct vcpu *v, mmio_info_t *info)
-{
-    struct hsr_dabt dabt = info->dabt;
-    struct cpu_user_regs *regs = guest_cpu_user_regs();
-    register_t *r = select_user_reg(regs, dabt.reg);
-    int offset = (int)(info->gpa - UART0_START);
-
-    switch ( offset )
-    {
-    case UARTDR:
-        *r = 0;
-        return 1;
-    case UARTFR:
-        *r = 0x87; /* All holding registers empty, ready to send etc */
-        return 1;
-    default:
-        printk("VPL011: unhandled read r%d offset %#08x\n",
-               dabt.reg, offset);
-        domain_crash_synchronous();
-    }
-}
-
-static int uart0_mmio_write(struct vcpu *v, mmio_info_t *info)
-{
-    struct hsr_dabt dabt = info->dabt;
-    struct cpu_user_regs *regs = guest_cpu_user_regs();
-    register_t *r = select_user_reg(regs, dabt.reg);
-    int offset = (int)(info->gpa - UART0_START);
-
-    switch ( offset )
-    {
-    case UARTDR:
-        /* ignore any status bits */
-        uart0_print_char((int)((*r) & 0xFF));
-        return 1;
-    case UARTFR:
-        /* Silently ignore */
-        return 1;
-    default:
-        printk("VPL011: unhandled write r%d=%"PRIregister" offset %#08x\n",
-               dabt.reg, *r, offset);
-        domain_crash_synchronous();
-    }
-}
-
-const struct mmio_handler uart0_mmio_handler = {
-    .check_handler = uart0_mmio_check,
-    .read_handler  = uart0_mmio_read,
-    .write_handler = uart0_mmio_write,
-};
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
-
diff --git a/xen/arch/arm/vpl011.h b/xen/arch/arm/vpl011.h
deleted file mode 100644
index f0d0a82..0000000
--- a/xen/arch/arm/vpl011.h
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- * xen/arch/arm/vpl011.h
- *
- * ARM PL011 Emulation Support
- *
- * Ian Campbell <ian.campbell@citrix.com>
- * Copyright (c) 2012 Citrix Systems.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#ifndef __ARCH_ARM_VPL011_H__
-#define __ARCH_ARM_VPL011_H__
-
-extern int domain_uart0_init(struct domain *d);
-extern void domain_uart0_free(struct domain *d);
-
-#endif
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/arm/vuart.c b/xen/arch/arm/vuart.c
new file mode 100644
index 0000000..5c3a84c
--- /dev/null
+++ b/xen/arch/arm/vuart.c
@@ -0,0 +1,150 @@
+/*
+ * xen/arch/arm/vuart.c
+ *
+ * Virtual UART Emulator.
+ *
+ * The emulator uses the information from dtuart. It will expose a basic
+ * UART which will allow the guest to log with an hardcode UART (early printk +
+ * decompressor).
+ *
+ * Julien Grall <julien.grall@linaro.org>
+ * Ian Campbell <ian.campbell@citrix.com>
+ * Copyright (c) 2012 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+/*
+ * This is not intended to be a full emulation of a PL011
+ * device. Rather it is intended to provide a sufficient veneer of one
+ * that early code (such as Linux's boot time decompressor) which
+ * hardcodes output directly to such a device are able to make progress.
+ *
+ * This device is not intended to be enumerable or exposed to the OS
+ * (e.g. via Device Tree).
+ */
+
+#include <xen/config.h>
+#include <xen/lib.h>
+#include <xen/sched.h>
+#include <xen/errno.h>
+#include <xen/ctype.h>
+#include <xen/serial.h>
+
+#include "vuart.h"
+#include "io.h"
+
+#define domain_has_vuart(d) ((d)->arch.vuart.info != NULL)
+
+int domain_vuart_init(struct domain *d)
+{
+    ASSERT( !d->domain_id );
+
+    d->arch.vuart.info = serial_info(SERHND_DTUART);
+    if ( !d->arch.vuart.info )
+        return 0;
+
+    spin_lock_init(&d->arch.vuart.lock);
+    d->arch.vuart.idx = 0;
+
+    d->arch.vuart.buf = xzalloc_array(char, VUART_BUF_SIZE);
+    if ( !d->arch.vuart.buf )
+        return -ENOMEM;
+
+    return 0;
+}
+
+void domain_vuart_free(struct domain *d)
+{
+    if ( !domain_has_vuart(d) )
+        return;
+
+    xfree(d->arch.vuart.buf);
+}
+
+static void vuart_print_char(char c)
+{
+    struct vuart *uart = &current->domain->arch.vuart;
+
+    /* Accept only printable characters, newline, and horizontal tab. */
+    if ( !isprint(c) && (c != '\n') && (c != '\t') )
+        return ;
+
+    spin_lock(&uart->lock);
+    uart->buf[uart->idx++] = c;
+    if ( (uart->idx == (VUART_BUF_SIZE - 2)) || (c == '\n') )
+    {
+        if ( c != '\n' )
+            uart->buf[uart->idx++] = '\n';
+        uart->buf[uart->idx] = '\0';
+        printk(XENLOG_G_DEBUG "DOM%u: %s",
+               current->domain->domain_id, uart->buf);
+        uart->idx = 0;
+    }
+    spin_unlock(&uart->lock);
+}
+
+static int vuart_mmio_check(struct vcpu *v, paddr_t addr)
+{
+    const struct serial_info *info = v->domain->arch.vuart.info;
+
+    return (domain_has_vuart(v->domain) && addr >= info->base_addr &&
+            addr <= (info->base_addr + info->size));
+}
+
+static int vuart_mmio_read(struct vcpu *v, mmio_info_t *info)
+{
+    struct domain *d = v->domain;
+    struct hsr_dabt dabt = info->dabt;
+    struct cpu_user_regs *regs = guest_cpu_user_regs();
+    register_t *r = select_user_reg(regs, dabt.reg);
+    paddr_t offset = info->gpa - d->arch.vuart.info->base_addr;
+
+    /* By default zeroed the register */
+    *r = 0;
+
+    if ( offset == d->arch.vuart.info->status_off )
+        /* All holding registers empty, ready to send etc */
+        *r = d->arch.vuart.info->status;
+
+    return 1;
+}
+
+static int vuart_mmio_write(struct vcpu *v, mmio_info_t *info)
+{
+    struct domain *d = v->domain;
+    struct hsr_dabt dabt = info->dabt;
+    struct cpu_user_regs *regs = guest_cpu_user_regs();
+    register_t *r = select_user_reg(regs, dabt.reg);
+    paddr_t offset = (int)(info->gpa - d->arch.vuart.info->base_addr);
+
+    if ( offset == d->arch.vuart.info->data_off )
+        /* ignore any status bits */
+        vuart_print_char((int)((*r) & 0xFF));
+
+    return 1;
+}
+
+const struct mmio_handler vuart_mmio_handler = {
+    .check_handler = vuart_mmio_check,
+    .read_handler  = vuart_mmio_read,
+    .write_handler = vuart_mmio_write,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
+
diff --git a/xen/arch/arm/vuart.h b/xen/arch/arm/vuart.h
new file mode 100644
index 0000000..9445e50
--- /dev/null
+++ b/xen/arch/arm/vuart.h
@@ -0,0 +1,35 @@
+/* 
+ * xen/arch/arm/vuart.h
+ *
+ * Virtual UART Emulation Support
+ *
+ * Ian Campbell <ian.campbell@citrix.com>
+ * Copyright (c) 2012 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __ARCH_ARM_VUART_H__
+#define __ARCH_ARM_VUART_H__
+
+int domain_vuart_init(struct domain *d);
+void domain_vuart_free(struct domain *d);
+
+#endif /* __ARCH_ARM_VUART_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 19bbe8a..216ce0b 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -9,6 +9,7 @@
 #include <asm/vfp.h>
 #include <public/hvm/params.h>
 #include <asm/atomic.h>
+#include <xen/serial.h>
 
 /* Represents state corresponding to a block of 32 interrupts */
 struct vgic_irq_rank {
@@ -104,12 +105,13 @@ struct arch_domain
         paddr_t cbase; /* CPU base address */
     } vgic;
 
-    struct vpl011 {
-#define VPL011_BUF_SIZE 128
-        char                  *buf;
-        int                    idx;
-        spinlock_t             lock;
-    } uart0;
+    struct vuart {
+#define VUART_BUF_SIZE 128
+        char                        *buf;
+        int                         idx;
+        const struct serial_info    *info;
+        spinlock_t                  lock;
+    } vuart;
 
 }  __cacheline_aligned;
 
-- 
1.7.10.4

[PATCH 6/8] exynos4210: rename UTRSTAT_TX_EMPTY in UTRSTAT_TXFE

[Julien Grall]

---
 xen/arch/arm/arm32/debug-exynos4210.inc |    2 +-
 xen/include/asm-arm/exynos4210-uart.h   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/arm32/debug-exynos4210.inc b/xen/arch/arm/arm32/debug-exynos4210.inc
index d746c35..5a5ff68 100644
--- a/xen/arch/arm/arm32/debug-exynos4210.inc
+++ b/xen/arch/arm/arm32/debug-exynos4210.inc
@@ -56,7 +56,7 @@
 .macro early_uart_ready rb rc
 1:
         ldr   \rc, [\rb, #UTRSTAT]   /* <- UTRSTAT (Flag register) */
-        tst   \rc, #UTRSTAT_TX_EMPTY /* Check BUSY bit */
+        tst   \rc, #UTRSTAT_TXFE /* Check BUSY bit */
         beq   1b                     /* Wait for the UART to be ready */
 .endm
 
diff --git a/xen/include/asm-arm/exynos4210-uart.h b/xen/include/asm-arm/exynos4210-uart.h
index 330e1c0..bd9a4be 100644
--- a/xen/include/asm-arm/exynos4210-uart.h
+++ b/xen/include/asm-arm/exynos4210-uart.h
@@ -87,7 +87,7 @@
 #define UFSTAT_RX_COUNT_MASK    (0xff << UFSTAT_RX_COUNT_SHIFT)
 
 /* UTRSTAT */
-#define UTRSTAT_TX_EMPTY        (1 << 1)
+#define UTRSTAT_TXFE            (1 << 1)
 
 /* URHX */
 #define URXH_DATA_MASK  (0xff)
-- 
1.7.10.4

[PATCH 7/8] exynos4210: Implement serial_info callback

[Julien Grall]

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/drivers/char/exynos4210-uart.c    |   15 +++++++++++++++
 xen/include/asm-arm/exynos4210-uart.h |    1 +
 2 files changed, 16 insertions(+)

diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
index f7971da..9ed70c2 100644
--- a/xen/drivers/char/exynos4210-uart.c
+++ b/xen/drivers/char/exynos4210-uart.c
@@ -33,6 +33,7 @@ static struct exynos4210_uart {
     struct dt_irq irq;
     void *regs;
     struct irqaction irqaction;
+    struct serial_info info;
 } exynos4210_com = {0};
 
 /* These parity settings can be ORed directly into the ULCON. */
@@ -281,6 +282,13 @@ static const struct dt_irq __init *exynos4210_uart_dt_irq(struct serial_port *po
     return &uart->irq;
 }
 
+static const struct serial_info *exynos4210_uart_info(struct serial_port *port)
+{
+    struct exynos4210_uart *uart = port->uart;
+
+    return &uart->info;
+}
+
 static struct uart_driver __read_mostly exynos4210_uart_driver = {
     .init_preirq  = exynos4210_uart_init_preirq,
     .init_postirq = exynos4210_uart_init_postirq,
@@ -292,6 +300,7 @@ static struct uart_driver __read_mostly exynos4210_uart_driver = {
     .getc         = exynos4210_uart_getc,
     .irq          = exynos4210_uart_irq,
     .dt_irq_get   = exynos4210_uart_dt_irq,
+    .info         = exynos4210_uart_info,
 };
 
 /* TODO: Parse UART config from the command line */
@@ -337,6 +346,12 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
         return res;
     }
 
+    uart->info.base_addr = addr;
+    uart->info.size = size;
+    uart->info.data_off = UTXH;
+    uart->info.status_off = UTRSTAT;
+    uart->info.status = UTRSTAT_TXE | UTRSTAT_TXFE;
+
     /* Register with generic serial driver. */
     serial_register_uart(SERHND_DTUART, &exynos4210_uart_driver, uart);
 
diff --git a/xen/include/asm-arm/exynos4210-uart.h b/xen/include/asm-arm/exynos4210-uart.h
index bd9a4be..e2ab4a4 100644
--- a/xen/include/asm-arm/exynos4210-uart.h
+++ b/xen/include/asm-arm/exynos4210-uart.h
@@ -88,6 +88,7 @@
 
 /* UTRSTAT */
 #define UTRSTAT_TXFE            (1 << 1)
+#define UTRSTAT_TXE             (1 << 2)
 
 /* URHX */
 #define URXH_DATA_MASK  (0xff)
-- 
1.7.10.4

[PATCH 8/8] pl011: Implement serial_info callback

[Julien Grall]

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/drivers/char/pl011.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index fd87e68..352e5b2 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -36,6 +36,7 @@ static struct pl011 {
     void __iomem *regs;
     /* UART with IRQ line: interrupt-driven I/O. */
     struct irqaction irqaction;
+    struct serial_info info;
     /* /\* UART with no IRQ line: periodically-polled I/O. *\/ */
     /* struct timer timer; */
     /* unsigned int timeout_ms; */
@@ -190,6 +191,13 @@ static const struct dt_irq __init *pl011_dt_irq(struct serial_port *port)
     return &uart->irq;
 }
 
+static const struct serial_info *pl011_info(struct serial_port *port)
+{
+    struct pl011 *uart = port->uart;
+
+    return &uart->info;
+}
+
 static struct uart_driver __read_mostly pl011_driver = {
     .init_preirq  = pl011_init_preirq,
     .init_postirq = pl011_init_postirq,
@@ -201,6 +209,7 @@ static struct uart_driver __read_mostly pl011_driver = {
     .getc         = pl011_getc,
     .irq          = pl011_irq,
     .dt_irq_get   = pl011_dt_irq,
+    .info         = pl011_info,
 };
 
 /* TODO: Parse UART config from the command line */
@@ -248,6 +257,12 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
         return res;
     }
 
+    uart->info.base_addr = addr;
+    uart->info.size = size;
+    uart->info.data_off = DR;
+    uart->info.status_off = FR;
+    uart->info.status = 0;
+
     /* Register with generic serial driver. */
     serial_register_uart(SERHND_DTUART, &pl011_driver, uart);
 
-- 
1.7.10.4

Re: [PATCH 1/8] pl011: Use ioreadl/iowritel

[Ian Campbell]

On Thu, 2013-07-25 at 17:59 +0100, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Did you observe any particular issue which caused this change?

Re: [PATCH 2/8] pl011: Move registers' definition in a separate file

[Ian Campbell]

Should we prefix them with PL011 or something?
 On Thu, 2013-07-25 at 17:59 +0100, Julien Grall wrote:
> ---
>  xen/drivers/char/pl011.c         |   48 +----------------------
>  xen/include/asm-arm/pl011-uart.h |   80 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 81 insertions(+), 47 deletions(-)
>  create mode 100644 xen/include/asm-arm/pl011-uart.h
> 
> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
> index 9456f20..fd87e68 100644
> --- a/xen/drivers/char/pl011.c
> +++ b/xen/drivers/char/pl011.c
> @@ -28,6 +28,7 @@
>  #include <asm/device.h>
>  #include <xen/mm.h>
>  #include <xen/vmap.h>
> +#include <asm/pl011-uart.h>
>  
>  static struct pl011 {
>      unsigned int baud, clock_hz, data_bits, parity, stop_bits;
> @@ -41,53 +42,6 @@ static struct pl011 {
>      /* bool_t probing, intr_works; */
>  } pl011_com = {0};
>  
> -/* PL011 register addresses */
> -#define DR     (0x00)
> -#define RSR    (0x04)
> -#define FR     (0x18)
> -#define ILPR   (0x20)
> -#define IBRD   (0x24)
> -#define FBRD   (0x28)
> -#define LCR_H  (0x2c)
> -#define CR     (0x30)
> -#define IFLS   (0x34)
> -#define IMSC   (0x38)
> -#define RIS    (0x3c)
> -#define MIS    (0x40)
> -#define ICR    (0x44)
> -#define DMACR  (0x48)
> -
> -/* CR bits */
> -#define RXE    (1<<9) /* Receive enable */
> -#define TXE    (1<<8) /* Transmit enable */
> -#define UARTEN (1<<0) /* UART enable */
> -
> -/* FR bits */
> -#define TXFE   (1<<7) /* TX FIFO empty */
> -#define RXFE   (1<<4) /* RX FIFO empty */
> -
> -/* LCR_H bits */
> -#define SPS    (1<<7) /* Stick parity select */
> -#define FEN    (1<<4) /* FIFO enable */
> -#define STP2   (1<<3) /* Two stop bits select */
> -#define EPS    (1<<2) /* Even parity select */
> -#define PEN    (1<<1) /* Parity enable */
> -#define BRK    (1<<0) /* Send break */
> -
> -/* Interrupt bits (IMSC, MIS, ICR) */
> -#define OEI   (1<<10) /* Overrun Error interrupt mask */
> -#define BEI   (1<<9)  /* Break Error interrupt mask */
> -#define PEI   (1<<8)  /* Parity Error interrupt mask */
> -#define FEI   (1<<7)  /* Framing Error interrupt mask */
> -#define RTI   (1<<6)  /* Receive Timeout interrupt mask */
> -#define TXI   (1<<5)  /* Transmit interrupt mask */
> -#define RXI   (1<<4)  /* Receive interrupt mask */
> -#define DSRMI (1<<3)  /* nUARTDSR Modem interrupt mask */
> -#define DCDMI (1<<2)  /* nUARTDCD Modem interrupt mask */
> -#define CTSMI (1<<1)  /* nUARTCTS Modem interrupt mask */
> -#define RIMI  (1<<0)  /* nUARTRI Modem interrupt mask */
> -#define ALLI  OEI|BEI|PEI|FEI|RTI|TXI|RXI|DSRMI|DCDMI|CTSMI|RIMI
> -
>  /* These parity settings can be ORed directly into the LCR. */
>  #define PARITY_NONE  (0)
>  #define PARITY_ODD   (PEN)
> diff --git a/xen/include/asm-arm/pl011-uart.h b/xen/include/asm-arm/pl011-uart.h
> new file mode 100644
> index 0000000..8c4edd4
> --- /dev/null
> +++ b/xen/include/asm-arm/pl011-uart.h
> @@ -0,0 +1,80 @@
> +/*
> + * xen/include/asm-arm/pl011-uart.h
> + *
> + * Common constant definition between early printk and the UART driver
> + * for the pl011 UART
> + *
> + * Tim Deegan <tim@xen.org>
> + * Copyright (c) 2011 Citrix Systems.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __ASM_ARM_PL011_H
> +#define __ASM_ARM_PL011_H
> +
> +/* PL011 register addresses */
> +#define DR     (0x00)
> +#define RSR    (0x04)
> +#define FR     (0x18)
> +#define ILPR   (0x20)
> +#define IBRD   (0x24)
> +#define FBRD   (0x28)
> +#define LCR_H  (0x2c)
> +#define CR     (0x30)
> +#define IFLS   (0x34)
> +#define IMSC   (0x38)
> +#define RIS    (0x3c)
> +#define MIS    (0x40)
> +#define ICR    (0x44)
> +#define DMACR  (0x48)
> +
> +/* CR bits */
> +#define RXE    (1<<9) /* Receive enable */
> +#define TXE    (1<<8) /* Transmit enable */
> +#define UARTEN (1<<0) /* UART enable */
> +
> +/* FR bits */
> +#define TXFE   (1<<7) /* TX FIFO empty */
> +#define RXFE   (1<<4) /* RX FIFO empty */
> +
> +/* LCR_H bits */
> +#define SPS    (1<<7) /* Stick parity select */
> +#define FEN    (1<<4) /* FIFO enable */
> +#define STP2   (1<<3) /* Two stop bits select */
> +#define EPS    (1<<2) /* Even parity select */
> +#define PEN    (1<<1) /* Parity enable */
> +#define BRK    (1<<0) /* Send break */
> +
> +/* Interrupt bits (IMSC, MIS, ICR) */
> +#define OEI   (1<<10) /* Overrun Error interrupt mask */
> +#define BEI   (1<<9)  /* Break Error interrupt mask */
> +#define PEI   (1<<8)  /* Parity Error interrupt mask */
> +#define FEI   (1<<7)  /* Framing Error interrupt mask */
> +#define RTI   (1<<6)  /* Receive Timeout interrupt mask */
> +#define TXI   (1<<5)  /* Transmit interrupt mask */
> +#define RXI   (1<<4)  /* Receive interrupt mask */
> +#define DSRMI (1<<3)  /* nUARTDSR Modem interrupt mask */
> +#define DCDMI (1<<2)  /* nUARTDCD Modem interrupt mask */
> +#define CTSMI (1<<1)  /* nUARTCTS Modem interrupt mask */
> +#define RIMI  (1<<0)  /* nUARTRI Modem interrupt mask */
> +#define ALLI  OEI|BEI|PEI|FEI|RTI|TXI|RXI|DSRMI|DCDMI|CTSMI|RIMI
> +
> +#endif /* __ASM_ARM_PL011_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */

Re: [PATCH 5/8] xen/arm: Implement a virtual UART

[Ian Campbell]

On Thu, 2013-07-25 at 17:59 +0100, Julien Grall wrote:
> This code is based on the previous vuart0 implementation. Unlike the latter,
> it's intend to replace UART stolen by XEN to DOM0 via dtuart=... on its
> command line.
> 
> It's useful when the kernel is compiled with early printk enabled or for a
> single platform. Most of the time, the hardcoded code to handle the UART
> will need 2 registers: status and data, the others registers can be
> implemented as RAZ/WI.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/Makefile        |    2 +-
>  xen/arch/arm/domain.c        |   12 ++--
>  xen/arch/arm/io.c            |    2 +-
>  xen/arch/arm/io.h            |    2 +-
>  xen/arch/arm/vpl011.c        |  152 ------------------------------------------
>  xen/arch/arm/vpl011.h        |   35 ----------
>  xen/arch/arm/vuart.c         |  150 +++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/vuart.h         |   35 ++++++++++

Please can you use the -M option to format-patch/send-email so that the
rename is handled in a way which lets us review the actual diff between
the old and new files.

>  xen/include/asm-arm/domain.h |   14 ++--
>  9 files changed, 204 insertions(+), 200 deletions(-)
>  delete mode 100644 xen/arch/arm/vpl011.c
>  delete mode 100644 xen/arch/arm/vpl011.h
>  create mode 100644 xen/arch/arm/vuart.c
>  create mode 100644 xen/arch/arm/vuart.h
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 5ae5831..6e1208f 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -27,7 +27,7 @@ obj-y += shutdown.o
>  obj-y += traps.o
>  obj-y += vgic.o
>  obj-y += vtimer.o
> -obj-y += vpl011.o
> +obj-y += vuart.o
>  obj-y += hvm.o
>  obj-y += device.o
>  
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index cfe0fbd..84b2f82 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -32,7 +32,7 @@
>  
>  #include <asm/gic.h>
>  #include "vtimer.h"
> -#include "vpl011.h"
> +#include "vuart.h"
>  
>  DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
>  
> @@ -518,8 +518,12 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
>      if ( (rc = vcpu_domain_init(d)) != 0 )
>          goto fail;
>  
> -    /* Domain 0 gets a real UART not an emulated one */
> -    if ( d->domain_id && (rc = domain_uart0_init(d)) != 0 )
> +    /*
> +     * Virtual UART is only used by linux early printk and decompress code.
> +     * Only use it for dom0 because the linux kernel may not support
> +     * multi-platform.
> +     */
> +    if ( (d->domain_id == 0) && (rc = domain_vuart_init(d)) )
>          goto fail;
>  
>      return 0;
> @@ -535,7 +539,7 @@ void arch_domain_destroy(struct domain *d)
>  {
>      p2m_teardown(d);
>      domain_vgic_free(d);
> -    domain_uart0_free(d);
> +    domain_vuart_free(d);
>      free_xenheap_page(d->shared_info);
>  }
>  
> diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
> index ad28c26..a6db00b 100644
> --- a/xen/arch/arm/io.c
> +++ b/xen/arch/arm/io.c
> @@ -25,7 +25,7 @@
>  static const struct mmio_handler *const mmio_handlers[] =
>  {
>      &vgic_distr_mmio_handler,
> -    &uart0_mmio_handler,
> +    &vuart_mmio_handler,
>  };
>  #define MMIO_HANDLER_NR ARRAY_SIZE(mmio_handlers)
>  
> diff --git a/xen/arch/arm/io.h b/xen/arch/arm/io.h
> index 661dce1..8d252c0 100644
> --- a/xen/arch/arm/io.h
> +++ b/xen/arch/arm/io.h
> @@ -41,7 +41,7 @@ struct mmio_handler {
>  };
>  
>  extern const struct mmio_handler vgic_distr_mmio_handler;
> -extern const struct mmio_handler uart0_mmio_handler;
> +extern const struct mmio_handler vuart_mmio_handler;
>  
>  extern int handle_mmio(mmio_info_t *info);
>  
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> deleted file mode 100644
> index 13ba623..0000000
> --- a/xen/arch/arm/vpl011.c
> +++ /dev/null
> @@ -1,152 +0,0 @@
> -/*
> - * xen/arch/arm/vpl011.c
> - *
> - * ARM PL011 UART Emulator (DEBUG)
> - *
> - * Ian Campbell <ian.campbell@citrix.com>
> - * Copyright (c) 2012 Citrix Systems.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - */
> -
> -/*
> - * This is not intended to be a full emulation of a PL011
> - * device. Rather it is intended to provide a sufficient veneer of one
> - * that early code (such as Linux's boot time decompressor) which
> - * hardcodes output directly to such a device are able to make progress.
> - *
> - * This device is not intended to be enumerable or exposed to the OS
> - * (e.g. via Device Tree).
> - */
> -
> -#include <xen/config.h>
> -#include <xen/lib.h>
> -#include <xen/sched.h>
> -#include <xen/errno.h>
> -#include <xen/ctype.h>
> -
> -#include "io.h"
> -
> -#define UART0_START 0x1c090000
> -#define UART0_END   (UART0_START+65536)
> -
> -#define UARTDR 0x000
> -#define UARTFR 0x018
> -
> -int domain_uart0_init(struct domain *d)
> -{
> -    ASSERT( d->domain_id );
> -
> -    spin_lock_init(&d->arch.uart0.lock);
> -    d->arch.uart0.idx = 0;
> -
> -    d->arch.uart0.buf = xzalloc_array(char, VPL011_BUF_SIZE);
> -    if ( !d->arch.uart0.buf )
> -        return -ENOMEM;
> -
> -    return 0;
> -
> -}
> -
> -void domain_uart0_free(struct domain *d)
> -{
> -    xfree(d->arch.uart0.buf);
> -}
> -
> -static void uart0_print_char(char c)
> -{
> -    struct vpl011 *uart = &current->domain->arch.uart0;
> -
> -    /* Accept only printable characters, newline, and horizontal tab. */
> -    if ( !isprint(c) && (c != '\n') && (c != '\t') )
> -        return ;
> -
> -    spin_lock(&uart->lock);
> -    uart->buf[uart->idx++] = c;
> -    if ( (uart->idx == (VPL011_BUF_SIZE - 2)) || (c == '\n') )
> -    {
> -        if ( c != '\n' )
> -            uart->buf[uart->idx++] = '\n';
> -        uart->buf[uart->idx] = '\0';
> -        printk(XENLOG_G_DEBUG "DOM%u: %s",
> -               current->domain->domain_id, uart->buf);
> -        uart->idx = 0;
> -    }
> -    spin_unlock(&uart->lock);
> -}
> -
> -static int uart0_mmio_check(struct vcpu *v, paddr_t addr)
> -{
> -    struct domain *d = v->domain;
> -
> -    return d->domain_id != 0 && addr >= UART0_START && addr < UART0_END;
> -}
> -
> -static int uart0_mmio_read(struct vcpu *v, mmio_info_t *info)
> -{
> -    struct hsr_dabt dabt = info->dabt;
> -    struct cpu_user_regs *regs = guest_cpu_user_regs();
> -    register_t *r = select_user_reg(regs, dabt.reg);
> -    int offset = (int)(info->gpa - UART0_START);
> -
> -    switch ( offset )
> -    {
> -    case UARTDR:
> -        *r = 0;
> -        return 1;
> -    case UARTFR:
> -        *r = 0x87; /* All holding registers empty, ready to send etc */
> -        return 1;
> -    default:
> -        printk("VPL011: unhandled read r%d offset %#08x\n",
> -               dabt.reg, offset);
> -        domain_crash_synchronous();
> -    }
> -}
> -
> -static int uart0_mmio_write(struct vcpu *v, mmio_info_t *info)
> -{
> -    struct hsr_dabt dabt = info->dabt;
> -    struct cpu_user_regs *regs = guest_cpu_user_regs();
> -    register_t *r = select_user_reg(regs, dabt.reg);
> -    int offset = (int)(info->gpa - UART0_START);
> -
> -    switch ( offset )
> -    {
> -    case UARTDR:
> -        /* ignore any status bits */
> -        uart0_print_char((int)((*r) & 0xFF));
> -        return 1;
> -    case UARTFR:
> -        /* Silently ignore */
> -        return 1;
> -    default:
> -        printk("VPL011: unhandled write r%d=%"PRIregister" offset %#08x\n",
> -               dabt.reg, *r, offset);
> -        domain_crash_synchronous();
> -    }
> -}
> -
> -const struct mmio_handler uart0_mmio_handler = {
> -    .check_handler = uart0_mmio_check,
> -    .read_handler  = uart0_mmio_read,
> -    .write_handler = uart0_mmio_write,
> -};
> -
> -/*
> - * Local variables:
> - * mode: C
> - * c-file-style: "BSD"
> - * c-basic-offset: 4
> - * indent-tabs-mode: nil
> - * End:
> - */
> -
> diff --git a/xen/arch/arm/vpl011.h b/xen/arch/arm/vpl011.h
> deleted file mode 100644
> index f0d0a82..0000000
> --- a/xen/arch/arm/vpl011.h
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -/*
> - * xen/arch/arm/vpl011.h
> - *
> - * ARM PL011 Emulation Support
> - *
> - * Ian Campbell <ian.campbell@citrix.com>
> - * Copyright (c) 2012 Citrix Systems.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - */
> -
> -#ifndef __ARCH_ARM_VPL011_H__
> -#define __ARCH_ARM_VPL011_H__
> -
> -extern int domain_uart0_init(struct domain *d);
> -extern void domain_uart0_free(struct domain *d);
> -
> -#endif
> -
> -/*
> - * Local variables:
> - * mode: C
> - * c-file-style: "BSD"
> - * c-basic-offset: 4
> - * indent-tabs-mode: nil
> - * End:
> - */
> diff --git a/xen/arch/arm/vuart.c b/xen/arch/arm/vuart.c
> new file mode 100644
> index 0000000..5c3a84c
> --- /dev/null
> +++ b/xen/arch/arm/vuart.c
> @@ -0,0 +1,150 @@
> +/*
> + * xen/arch/arm/vuart.c
> + *
> + * Virtual UART Emulator.
> + *
> + * The emulator uses the information from dtuart. It will expose a basic
> + * UART which will allow the guest to log with an hardcode UART (early printk +
> + * decompressor).
> + *
> + * Julien Grall <julien.grall@linaro.org>
> + * Ian Campbell <ian.campbell@citrix.com>
> + * Copyright (c) 2012 Citrix Systems.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +/*
> + * This is not intended to be a full emulation of a PL011
> + * device. Rather it is intended to provide a sufficient veneer of one
> + * that early code (such as Linux's boot time decompressor) which
> + * hardcodes output directly to such a device are able to make progress.
> + *
> + * This device is not intended to be enumerable or exposed to the OS
> + * (e.g. via Device Tree).
> + */
> +
> +#include <xen/config.h>
> +#include <xen/lib.h>
> +#include <xen/sched.h>
> +#include <xen/errno.h>
> +#include <xen/ctype.h>
> +#include <xen/serial.h>
> +
> +#include "vuart.h"
> +#include "io.h"
> +
> +#define domain_has_vuart(d) ((d)->arch.vuart.info != NULL)
> +
> +int domain_vuart_init(struct domain *d)
> +{
> +    ASSERT( !d->domain_id );
> +
> +    d->arch.vuart.info = serial_info(SERHND_DTUART);
> +    if ( !d->arch.vuart.info )
> +        return 0;
> +
> +    spin_lock_init(&d->arch.vuart.lock);
> +    d->arch.vuart.idx = 0;
> +
> +    d->arch.vuart.buf = xzalloc_array(char, VUART_BUF_SIZE);
> +    if ( !d->arch.vuart.buf )
> +        return -ENOMEM;
> +
> +    return 0;
> +}
> +
> +void domain_vuart_free(struct domain *d)
> +{
> +    if ( !domain_has_vuart(d) )
> +        return;
> +
> +    xfree(d->arch.vuart.buf);
> +}
> +
> +static void vuart_print_char(char c)
> +{
> +    struct vuart *uart = &current->domain->arch.vuart;
> +
> +    /* Accept only printable characters, newline, and horizontal tab. */
> +    if ( !isprint(c) && (c != '\n') && (c != '\t') )
> +        return ;
> +
> +    spin_lock(&uart->lock);
> +    uart->buf[uart->idx++] = c;
> +    if ( (uart->idx == (VUART_BUF_SIZE - 2)) || (c == '\n') )
> +    {
> +        if ( c != '\n' )
> +            uart->buf[uart->idx++] = '\n';
> +        uart->buf[uart->idx] = '\0';
> +        printk(XENLOG_G_DEBUG "DOM%u: %s",
> +               current->domain->domain_id, uart->buf);
> +        uart->idx = 0;
> +    }
> +    spin_unlock(&uart->lock);
> +}
> +
> +static int vuart_mmio_check(struct vcpu *v, paddr_t addr)
> +{
> +    const struct serial_info *info = v->domain->arch.vuart.info;
> +
> +    return (domain_has_vuart(v->domain) && addr >= info->base_addr &&
> +            addr <= (info->base_addr + info->size));
> +}
> +
> +static int vuart_mmio_read(struct vcpu *v, mmio_info_t *info)
> +{
> +    struct domain *d = v->domain;
> +    struct hsr_dabt dabt = info->dabt;
> +    struct cpu_user_regs *regs = guest_cpu_user_regs();
> +    register_t *r = select_user_reg(regs, dabt.reg);
> +    paddr_t offset = info->gpa - d->arch.vuart.info->base_addr;
> +
> +    /* By default zeroed the register */
> +    *r = 0;
> +
> +    if ( offset == d->arch.vuart.info->status_off )
> +        /* All holding registers empty, ready to send etc */
> +        *r = d->arch.vuart.info->status;
> +
> +    return 1;
> +}
> +
> +static int vuart_mmio_write(struct vcpu *v, mmio_info_t *info)
> +{
> +    struct domain *d = v->domain;
> +    struct hsr_dabt dabt = info->dabt;
> +    struct cpu_user_regs *regs = guest_cpu_user_regs();
> +    register_t *r = select_user_reg(regs, dabt.reg);
> +    paddr_t offset = (int)(info->gpa - d->arch.vuart.info->base_addr);
> +
> +    if ( offset == d->arch.vuart.info->data_off )
> +        /* ignore any status bits */
> +        vuart_print_char((int)((*r) & 0xFF));
> +
> +    return 1;
> +}
> +
> +const struct mmio_handler vuart_mmio_handler = {
> +    .check_handler = vuart_mmio_check,
> +    .read_handler  = vuart_mmio_read,
> +    .write_handler = vuart_mmio_write,
> +};
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> +
> diff --git a/xen/arch/arm/vuart.h b/xen/arch/arm/vuart.h
> new file mode 100644
> index 0000000..9445e50
> --- /dev/null
> +++ b/xen/arch/arm/vuart.h
> @@ -0,0 +1,35 @@
> +/* 
> + * xen/arch/arm/vuart.h
> + *
> + * Virtual UART Emulation Support
> + *
> + * Ian Campbell <ian.campbell@citrix.com>
> + * Copyright (c) 2012 Citrix Systems.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __ARCH_ARM_VUART_H__
> +#define __ARCH_ARM_VUART_H__
> +
> +int domain_vuart_init(struct domain *d);
> +void domain_vuart_free(struct domain *d);
> +
> +#endif /* __ARCH_ARM_VUART_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 19bbe8a..216ce0b 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -9,6 +9,7 @@
>  #include <asm/vfp.h>
>  #include <public/hvm/params.h>
>  #include <asm/atomic.h>
> +#include <xen/serial.h>
>  
>  /* Represents state corresponding to a block of 32 interrupts */
>  struct vgic_irq_rank {
> @@ -104,12 +105,13 @@ struct arch_domain
>          paddr_t cbase; /* CPU base address */
>      } vgic;
>  
> -    struct vpl011 {
> -#define VPL011_BUF_SIZE 128
> -        char                  *buf;
> -        int                    idx;
> -        spinlock_t             lock;
> -    } uart0;
> +    struct vuart {
> +#define VUART_BUF_SIZE 128
> +        char                        *buf;
> +        int                         idx;
> +        const struct serial_info    *info;
> +        spinlock_t                  lock;
> +    } vuart;
>  
>  }  __cacheline_aligned;
>  

Re: [PATCH 6/8] exynos4210: rename UTRSTAT_TX_EMPTY in UTRSTAT_TXFE

[Ian Campbell]

Why the rename?

Also no S-o-b..

On Thu, 2013-07-25 at 17:59 +0100, Julien Grall wrote:
> ---
>  xen/arch/arm/arm32/debug-exynos4210.inc |    2 +-
>  xen/include/asm-arm/exynos4210-uart.h   |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/debug-exynos4210.inc b/xen/arch/arm/arm32/debug-exynos4210.inc
> index d746c35..5a5ff68 100644
> --- a/xen/arch/arm/arm32/debug-exynos4210.inc
> +++ b/xen/arch/arm/arm32/debug-exynos4210.inc
> @@ -56,7 +56,7 @@
>  .macro early_uart_ready rb rc
>  1:
>          ldr   \rc, [\rb, #UTRSTAT]   /* <- UTRSTAT (Flag register) */
> -        tst   \rc, #UTRSTAT_TX_EMPTY /* Check BUSY bit */
> +        tst   \rc, #UTRSTAT_TXFE /* Check BUSY bit */

Pls keep the comment aligned.

>          beq   1b                     /* Wait for the UART to be ready */
>  .endm
>  
> diff --git a/xen/include/asm-arm/exynos4210-uart.h b/xen/include/asm-arm/exynos4210-uart.h
> index 330e1c0..bd9a4be 100644
> --- a/xen/include/asm-arm/exynos4210-uart.h
> +++ b/xen/include/asm-arm/exynos4210-uart.h
> @@ -87,7 +87,7 @@
>  #define UFSTAT_RX_COUNT_MASK    (0xff << UFSTAT_RX_COUNT_SHIFT)
>  
>  /* UTRSTAT */
> -#define UTRSTAT_TX_EMPTY        (1 << 1)
> +#define UTRSTAT_TXFE            (1 << 1)
>  
>  /* URHX */
>  #define URXH_DATA_MASK  (0xff)

Re: [PATCH 4/8] xen/arm: New callback in uart_driver to retrieve serial information

[Ian Campbell]

On Thu, 2013-07-25 at 17:59 +0100, Julien Grall wrote:
> There is no way to retrieve basic informations (base address, size, ....) for
> an UART. This callback will be used later to partially emulate the real UART
> for DOM0 on ARM.

The serial_info name makes me think it is going to contain the baud
rate, parity bits etc.

Since this is (I suppose) going to be used by your vuart code perhaps
vuart_info?

> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> CC: Keir Fraser <keir@xen.org>
> ---
>  xen/drivers/char/serial.c |    8 ++++++++
>  xen/include/xen/serial.h  |   13 +++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
> index e1c3f47..7640f8e 100644
> --- a/xen/drivers/char/serial.c
> +++ b/xen/drivers/char/serial.c
> @@ -497,6 +497,14 @@ const struct dt_irq __init *serial_dt_irq(int idx)
>      return NULL;
>  }
>  
> +const struct serial_info *serial_info(int idx)
> +{
> +    if ( (idx >= 0) && (idx < ARRAY_SIZE(com)) &&
> +         com[idx].driver && com[idx].driver->info )
> +        return com[idx].driver->info(&com[idx]);
> +
> +    return NULL;
> +}
>  
>  void serial_suspend(void)
>  {
> diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
> index 9caf776..c312032 100644
> --- a/xen/include/xen/serial.h
> +++ b/xen/include/xen/serial.h
> @@ -32,6 +32,14 @@ enum serial_port_state {
>      serial_initialized
>  };
>  
> +struct serial_info {
> +    unsigned long base_addr;    /* Base address of the UART */
> +    unsigned long size;         /* Size of the memory region */
> +    unsigned long data_off;     /* Data register offset */
> +    unsigned long status_off;   /* Status register offset */
> +    unsigned long status;       /* Ready status value */
> +};
> +
>  struct serial_port {
>      /* Uart-driver parameters. */
>      struct uart_driver *driver;
> @@ -74,6 +82,8 @@ struct uart_driver {
>      int  (*irq)(struct serial_port *);
>      /* Get IRQ device node for this port's serial line: returns NULL if none. */
>      const struct dt_irq *(*dt_irq_get)(struct serial_port *);
> +    /* Get serial information */
> +    const struct serial_info *(*info)(struct serial_port *);
>  };
>  
>  /* 'Serial handles' are composed from the following fields. */
> @@ -127,6 +137,9 @@ int serial_irq(int idx);
>  /* Return irq device node for specified serial port (identified by index). */
>  const struct dt_irq *serial_dt_irq(int idx);
>  
> +/* Retrieve basic UART information (base address, size, ...) */
> +const struct serial_info* serial_info(int idx);
> +
>  /* Serial suspend/resume. */
>  void serial_suspend(void);
>  void serial_resume(void);

Re: [PATCH 1/8] pl011: Use ioreadl/iowritel

[Julien Grall]

On 07/29/2013 05:23 PM, Ian Campbell wrote:
> On Thu, 2013-07-25 at 17:59 +0100, Julien Grall wrote:
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Did you observe any particular issue which caused this change?

No. It's to use the same API as the other drivers.

-- 
Julien

Re: [PATCH 2/8] pl011: Move registers' definition in a separate file

[Julien Grall]

On 07/29/2013 05:24 PM, Ian Campbell wrote:
> Should we prefix them with PL011 or something?

This header is intended to be used only in the C drivers and early printk.

But I can prefix by UART... which is the real name for the registers. I
didn't choose this solution to avoid lots of changes in the PL011 driver.

-- 
Julien

Re: [PATCH 4/8] xen/arm: New callback in uart_driver to retrieve serial information

[Julien Grall]

On 07/29/2013 05:29 PM, Ian Campbell wrote:
> On Thu, 2013-07-25 at 17:59 +0100, Julien Grall wrote:
>> There is no way to retrieve basic informations (base address, size, ....) for
>> an UART. This callback will be used later to partially emulate the real UART
>> for DOM0 on ARM.
> 
> The serial_info name makes me think it is going to contain the baud
> rate, parity bits etc.
> 
> Since this is (I suppose) going to be used by your vuart code perhaps
> vuart_info?

This name is better, I will rename the function for the next patch series.

-- 
Julien

Re: [PATCH 5/8] xen/arm: Implement a virtual UART

[Julien Grall]

On 07/29/2013 05:26 PM, Ian Campbell wrote:
> On Thu, 2013-07-25 at 17:59 +0100, Julien Grall wrote:
>> This code is based on the previous vuart0 implementation. Unlike the latter,
>> it's intend to replace UART stolen by XEN to DOM0 via dtuart=... on its
>> command line.
>>
>> It's useful when the kernel is compiled with early printk enabled or for a
>> single platform. Most of the time, the hardcoded code to handle the UART
>> will need 2 registers: status and data, the others registers can be
>> implemented as RAZ/WI.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>>  xen/arch/arm/Makefile        |    2 +-
>>  xen/arch/arm/domain.c        |   12 ++--
>>  xen/arch/arm/io.c            |    2 +-
>>  xen/arch/arm/io.h            |    2 +-
>>  xen/arch/arm/vpl011.c        |  152 ------------------------------------------
>>  xen/arch/arm/vpl011.h        |   35 ----------
>>  xen/arch/arm/vuart.c         |  150 +++++++++++++++++++++++++++++++++++++++++
>>  xen/arch/arm/vuart.h         |   35 ++++++++++
> 
> Please can you use the -M option to format-patch/send-email so that the
> rename is handled in a way which lets us review the actual diff between
> the old and new files.

below the patch generated with -M option.

commit d5be5261c93abac0a9c50e4bdb69883e469fd49f
Author: Julien Grall <julien.grall@linaro.org>
Date:   Thu Jul 25 17:11:59 2013 +0100

    xen/arm: Implement a virtual UART
    
    This code is based on the previous vuart0 implementation. Unlike the latter,
    it's intend to replace UART stolen by XEN to DOM0 via dtuart=... on its
    command line.
    
    It's useful when the kernel is compiled with early printk enabled or for a
    single platform. Most of the time, the hardcoded code to handle the UART
    will need 2 registers: status and data, the others registers can be
    implemented as RAZ/WI.
    
    Signed-off-by: Julien Grall <julien.grall@linaro.org>

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 5ae5831..6e1208f 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -27,7 +27,7 @@ obj-y += shutdown.o
 obj-y += traps.o
 obj-y += vgic.o
 obj-y += vtimer.o
-obj-y += vpl011.o
+obj-y += vuart.o
 obj-y += hvm.o
 obj-y += device.o
 
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 4e9cece..cb0424d 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -32,7 +32,7 @@
 
 #include <asm/gic.h>
 #include "vtimer.h"
-#include "vpl011.h"
+#include "vuart.h"
 
 DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
 
@@ -525,8 +525,12 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
     if ( (rc = vcpu_domain_init(d)) != 0 )
         goto fail;
 
-    /* Domain 0 gets a real UART not an emulated one */
-    if ( d->domain_id && (rc = domain_uart0_init(d)) != 0 )
+    /*
+     * Virtual UART is only used by linux early printk and decompress code.
+     * Only use it for dom0 because the linux kernel may not support
+     * multi-platform.
+     */
+    if ( (d->domain_id == 0) && (rc = domain_vuart_init(d)) )
         goto fail;
 
     return 0;
@@ -542,7 +546,7 @@ void arch_domain_destroy(struct domain *d)
 {
     p2m_teardown(d);
     domain_vgic_free(d);
-    domain_uart0_free(d);
+    domain_vuart_free(d);
     free_xenheap_page(d->shared_info);
 }
 
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index ad28c26..a6db00b 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -25,7 +25,7 @@
 static const struct mmio_handler *const mmio_handlers[] =
 {
     &vgic_distr_mmio_handler,
-    &uart0_mmio_handler,
+    &vuart_mmio_handler,
 };
 #define MMIO_HANDLER_NR ARRAY_SIZE(mmio_handlers)
 
diff --git a/xen/arch/arm/io.h b/xen/arch/arm/io.h
index 661dce1..8d252c0 100644
--- a/xen/arch/arm/io.h
+++ b/xen/arch/arm/io.h
@@ -41,7 +41,7 @@ struct mmio_handler {
 };
 
 extern const struct mmio_handler vgic_distr_mmio_handler;
-extern const struct mmio_handler uart0_mmio_handler;
+extern const struct mmio_handler vuart_mmio_handler;
 
 extern int handle_mmio(mmio_info_t *info);
 
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vuart.c
similarity index 51%
rename from xen/arch/arm/vpl011.c
rename to xen/arch/arm/vuart.c
index 13ba623..5c3a84c 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vuart.c
@@ -1,8 +1,13 @@
 /*
- * xen/arch/arm/vpl011.c
+ * xen/arch/arm/vuart.c
  *
- * ARM PL011 UART Emulator (DEBUG)
+ * Virtual UART Emulator.
  *
+ * The emulator uses the information from dtuart. It will expose a basic
+ * UART which will allow the guest to log with an hardcode UART (early printk +
+ * decompressor).
+ *
+ * Julien Grall <julien.grall@linaro.org>
  * Ian Campbell <ian.campbell@citrix.com>
  * Copyright (c) 2012 Citrix Systems.
  *
@@ -32,38 +37,42 @@
 #include <xen/sched.h>
 #include <xen/errno.h>
 #include <xen/ctype.h>
+#include <xen/serial.h>
 
+#include "vuart.h"
 #include "io.h"
 
-#define UART0_START 0x1c090000
-#define UART0_END   (UART0_START+65536)
-
-#define UARTDR 0x000
-#define UARTFR 0x018
+#define domain_has_vuart(d) ((d)->arch.vuart.info != NULL)
 
-int domain_uart0_init(struct domain *d)
+int domain_vuart_init(struct domain *d)
 {
-    ASSERT( d->domain_id );
+    ASSERT( !d->domain_id );
 
-    spin_lock_init(&d->arch.uart0.lock);
-    d->arch.uart0.idx = 0;
+    d->arch.vuart.info = serial_info(SERHND_DTUART);
+    if ( !d->arch.vuart.info )
+        return 0;
 
-    d->arch.uart0.buf = xzalloc_array(char, VPL011_BUF_SIZE);
-    if ( !d->arch.uart0.buf )
+    spin_lock_init(&d->arch.vuart.lock);
+    d->arch.vuart.idx = 0;
+
+    d->arch.vuart.buf = xzalloc_array(char, VUART_BUF_SIZE);
+    if ( !d->arch.vuart.buf )
         return -ENOMEM;
 
     return 0;
-
 }
 
-void domain_uart0_free(struct domain *d)
+void domain_vuart_free(struct domain *d)
 {
-    xfree(d->arch.uart0.buf);
+    if ( !domain_has_vuart(d) )
+        return;
+
+    xfree(d->arch.vuart.buf);
 }
 
-static void uart0_print_char(char c)
+static void vuart_print_char(char c)
 {
-    struct vpl011 *uart = &current->domain->arch.uart0;
+    struct vuart *uart = &current->domain->arch.vuart;
 
     /* Accept only printable characters, newline, and horizontal tab. */
     if ( !isprint(c) && (c != '\n') && (c != '\t') )
@@ -71,7 +80,7 @@ static void uart0_print_char(char c)
 
     spin_lock(&uart->lock);
     uart->buf[uart->idx++] = c;
-    if ( (uart->idx == (VPL011_BUF_SIZE - 2)) || (c == '\n') )
+    if ( (uart->idx == (VUART_BUF_SIZE - 2)) || (c == '\n') )
     {
         if ( c != '\n' )
             uart->buf[uart->idx++] = '\n';
@@ -83,62 +92,51 @@ static void uart0_print_char(char c)
     spin_unlock(&uart->lock);
 }
 
-static int uart0_mmio_check(struct vcpu *v, paddr_t addr)
+static int vuart_mmio_check(struct vcpu *v, paddr_t addr)
 {
-    struct domain *d = v->domain;
+    const struct serial_info *info = v->domain->arch.vuart.info;
 
-    return d->domain_id != 0 && addr >= UART0_START && addr < UART0_END;
+    return (domain_has_vuart(v->domain) && addr >= info->base_addr &&
+            addr <= (info->base_addr + info->size));
 }
 
-static int uart0_mmio_read(struct vcpu *v, mmio_info_t *info)
+static int vuart_mmio_read(struct vcpu *v, mmio_info_t *info)
 {
+    struct domain *d = v->domain;
     struct hsr_dabt dabt = info->dabt;
     struct cpu_user_regs *regs = guest_cpu_user_regs();
     register_t *r = select_user_reg(regs, dabt.reg);
-    int offset = (int)(info->gpa - UART0_START);
+    paddr_t offset = info->gpa - d->arch.vuart.info->base_addr;
 
-    switch ( offset )
-    {
-    case UARTDR:
-        *r = 0;
-        return 1;
-    case UARTFR:
-        *r = 0x87; /* All holding registers empty, ready to send etc */
-        return 1;
-    default:
-        printk("VPL011: unhandled read r%d offset %#08x\n",
-               dabt.reg, offset);
-        domain_crash_synchronous();
-    }
+    /* By default zeroed the register */
+    *r = 0;
+
+    if ( offset == d->arch.vuart.info->status_off )
+        /* All holding registers empty, ready to send etc */
+        *r = d->arch.vuart.info->status;
+
+    return 1;
 }
 
-static int uart0_mmio_write(struct vcpu *v, mmio_info_t *info)
+static int vuart_mmio_write(struct vcpu *v, mmio_info_t *info)
 {
+    struct domain *d = v->domain;
     struct hsr_dabt dabt = info->dabt;
     struct cpu_user_regs *regs = guest_cpu_user_regs();
     register_t *r = select_user_reg(regs, dabt.reg);
-    int offset = (int)(info->gpa - UART0_START);
+    paddr_t offset = (int)(info->gpa - d->arch.vuart.info->base_addr);
 
-    switch ( offset )
-    {
-    case UARTDR:
+    if ( offset == d->arch.vuart.info->data_off )
         /* ignore any status bits */
-        uart0_print_char((int)((*r) & 0xFF));
-        return 1;
-    case UARTFR:
-        /* Silently ignore */
-        return 1;
-    default:
-        printk("VPL011: unhandled write r%d=%"PRIregister" offset %#08x\n",
-               dabt.reg, *r, offset);
-        domain_crash_synchronous();
-    }
+        vuart_print_char((int)((*r) & 0xFF));
+
+    return 1;
 }
 
-const struct mmio_handler uart0_mmio_handler = {
-    .check_handler = uart0_mmio_check,
-    .read_handler  = uart0_mmio_read,
-    .write_handler = uart0_mmio_write,
+const struct mmio_handler vuart_mmio_handler = {
+    .check_handler = vuart_mmio_check,
+    .read_handler  = vuart_mmio_read,
+    .write_handler = vuart_mmio_write,
 };
 
 /*
diff --git a/xen/arch/arm/vpl011.h b/xen/arch/arm/vuart.h
similarity index 75%
rename from xen/arch/arm/vpl011.h
rename to xen/arch/arm/vuart.h
index f0d0a82..9445e50 100644
--- a/xen/arch/arm/vpl011.h
+++ b/xen/arch/arm/vuart.h
@@ -1,7 +1,7 @@
-/*
- * xen/arch/arm/vpl011.h
+/* 
+ * xen/arch/arm/vuart.h
  *
- * ARM PL011 Emulation Support
+ * Virtual UART Emulation Support
  *
  * Ian Campbell <ian.campbell@citrix.com>
  * Copyright (c) 2012 Citrix Systems.
@@ -17,13 +17,13 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __ARCH_ARM_VPL011_H__
-#define __ARCH_ARM_VPL011_H__
+#ifndef __ARCH_ARM_VUART_H__
+#define __ARCH_ARM_VUART_H__
 
-extern int domain_uart0_init(struct domain *d);
-extern void domain_uart0_free(struct domain *d);
+int domain_vuart_init(struct domain *d);
+void domain_vuart_free(struct domain *d);
 
-#endif
+#endif /* __ARCH_ARM_VUART_H__ */
 
 /*
  * Local variables:
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 89f88f6..394e574 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -8,6 +8,7 @@
 #include <asm/p2m.h>
 #include <asm/vfp.h>
 #include <public/hvm/params.h>
+#include <xen/serial.h>
 
 /* Represents state corresponding to a block of 32 interrupts */
 struct vgic_irq_rank {
@@ -103,12 +104,13 @@ struct arch_domain
         paddr_t cbase; /* CPU base address */
     } vgic;
 
-    struct vpl011 {
-#define VPL011_BUF_SIZE 128
-        char                  *buf;
-        int                    idx;
-        spinlock_t             lock;
-    } uart0;
+    struct vuart {
+#define VUART_BUF_SIZE 128
+        char                        *buf;
+        int                         idx;
+        const struct serial_info    *info;
+        spinlock_t                  lock;
+    } vuart;
 
 }  __cacheline_aligned;
 
-- 
Julien

Re: [PATCH 6/8] exynos4210: rename UTRSTAT_TX_EMPTY in UTRSTAT_TXFE

[Julien Grall]

On 07/29/2013 05:27 PM, Ian Campbell wrote:
> Why the rename?

To avoid confusion with UTRSTAT_TXE (introduce on the next patch). The
real name of this bit is UTRSTAT_TXFE.

> Also no S-o-b..

Oh right. Will be add on the next patch series.

> On Thu, 2013-07-25 at 17:59 +0100, Julien Grall wrote:
>> ---
>>  xen/arch/arm/arm32/debug-exynos4210.inc |    2 +-
>>  xen/include/asm-arm/exynos4210-uart.h   |    2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm32/debug-exynos4210.inc b/xen/arch/arm/arm32/debug-exynos4210.inc
>> index d746c35..5a5ff68 100644
>> --- a/xen/arch/arm/arm32/debug-exynos4210.inc
>> +++ b/xen/arch/arm/arm32/debug-exynos4210.inc
>> @@ -56,7 +56,7 @@
>>  .macro early_uart_ready rb rc
>>  1:
>>          ldr   \rc, [\rb, #UTRSTAT]   /* <- UTRSTAT (Flag register) */
>> -        tst   \rc, #UTRSTAT_TX_EMPTY /* Check BUSY bit */
>> +        tst   \rc, #UTRSTAT_TXFE /* Check BUSY bit */
> 
> Pls keep the comment aligned.
> 
>>          beq   1b                     /* Wait for the UART to be ready */
>>  .endm
>>  
>> diff --git a/xen/include/asm-arm/exynos4210-uart.h b/xen/include/asm-arm/exynos4210-uart.h
>> index 330e1c0..bd9a4be 100644
>> --- a/xen/include/asm-arm/exynos4210-uart.h
>> +++ b/xen/include/asm-arm/exynos4210-uart.h
>> @@ -87,7 +87,7 @@
>>  #define UFSTAT_RX_COUNT_MASK    (0xff << UFSTAT_RX_COUNT_SHIFT)
>>  
>>  /* UTRSTAT */
>> -#define UTRSTAT_TX_EMPTY        (1 << 1)
>> +#define UTRSTAT_TXFE            (1 << 1)
>>  
>>  /* URHX */
>>  #define URXH_DATA_MASK  (0xff)
> 
> 

Re: [PATCH 2/8] pl011: Move registers' definition in a separate file

[Ian Campbell]

On Mon, 2013-07-29 at 17:35 +0100, Julien Grall wrote:
> On 07/29/2013 05:24 PM, Ian Campbell wrote:
> > Should we prefix them with PL011 or something?
> 
> This header is intended to be used only in the C drivers and early printk.
> 
> But I can prefix by UART... which is the real name for the registers.

"real name" in what sense? The offsets are specific to the PL011, aren't
they? Or are you saying they are common with the 16550 offsets?

It's bad enough that the 16550 header uses UART prefix without adding a
second potentially conflicting user of that namespace

> I didn't choose this solution to avoid lots of changes in the PL011 driver.

That's a reasonable point. Lets leave them alone until it becomes a real
problem.

Ian.

Re: [PATCH 5/8] xen/arm: Implement a virtual UART

[Ian Campbell]

On Mon, 2013-07-29 at 17:40 +0100, Julien Grall wrote:
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vuart.c
> similarity index 51%
> rename from xen/arch/arm/vpl011.c
> rename to xen/arch/arm/vuart.c
> index 13ba623..5c3a84c 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vuart.c
> @@ -1,8 +1,13 @@
>  /*
> - * xen/arch/arm/vpl011.c
> + * xen/arch/arm/vuart.c
>   *
> - * ARM PL011 UART Emulator (DEBUG)
> + * Virtual UART Emulator.
>   *
> + * The emulator uses the information from dtuart. It will expose a basic
     ^This ?

> + * UART which will allow the guest to log with an hardcode UART (early printk +
                                                  a  hardcoded
> + * decompressor).

Please can you combine this with the comment that follows "This is not
intended to be a full emulation" etc so as people are not accidentally
mislead into thinking this is a proper UART emulator. (Also you have a
missed a "PL011" in that other comment)

It might also be worth expanding on what "hardcoded" means, i.e. a
single status register (with a fixed value) and a single byte transmit
register.

The rest looks good, thanks.

Ian.

Re: [PATCH 2/8] pl011: Move registers' definition in a separate file

[Julien Grall]

On 30 July 2013 11:00, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2013-07-29 at 17:35 +0100, Julien Grall wrote:
>> On 07/29/2013 05:24 PM, Ian Campbell wrote:
>> > Should we prefix them with PL011 or something?
>>
>> This header is intended to be used only in the C drivers and early printk.
>>
>> But I can prefix by UART... which is the real name for the registers.
>
> "real name" in what sense? The offsets are specific to the PL011, aren't
> they? Or are you saying they are common with the 16550 offsets?

Theses offsets are specific to the PL011. For "real name" I mean the
name used in the documentation. For instance DR is named UARTDR...

> It's bad enough that the 16550 header uses UART prefix without adding a
> second potentially conflicting user of that namespace
>
>> I didn't choose this solution to avoid lots of changes in the PL011 driver.
>
> That's a reasonable point. Lets leave them alone until it becomes a real
> problem.
>
> Ian.
>
>



-- 
Julien Grall