[RFC 0/6] xen/arm: Merge early_printk function in console code

[RFC 0/6] xen/arm: Merge early_printk function in console code

[Julien Grall]

Hello all,

This patch series aims to merge early printk in the console code. This will
avoid the developper to avoid to care whether the message is printed before or
after the console is initialized.

Sincerely yours,

Julien Grall (6):
  xen/arm: earlyprintk: move early_flush in early_puts
  xen/arm: earlyprintk: export early_puts
  xen/arm: Rename EARLY_PRINTK compile option to CONFIG_EARLY_PRINTK
  xen/console: Add support for early printk
  xen/console: Add noreturn attribute to panic function
  xen/arm: Replace early_printk call to printk call

 xen/arch/arm/Rules.mk              |  2 +-
 xen/arch/arm/arm32/head.S          | 18 +++++++++---------
 xen/arch/arm/arm64/head.S          | 18 +++++++++---------
 xen/arch/arm/early_printk.c        | 34 +---------------------------------
 xen/arch/arm/setup.c               | 28 +++++++++++++---------------
 xen/common/device_tree.c           | 36 +++++++++++++-----------------------
 xen/drivers/char/console.c         | 17 +++++++++++++++--
 xen/drivers/char/dt-uart.c         |  9 ++++-----
 xen/drivers/char/exynos4210-uart.c | 13 +++++--------
 xen/drivers/char/omap-uart.c       | 13 ++++++-------
 xen/drivers/char/pl011.c           | 13 ++++++-------
 xen/drivers/video/arm_hdlcd.c      | 29 ++++++++++++++---------------
 xen/include/asm-arm/early_printk.h | 25 ++++---------------------
 xen/include/xen/lib.h              |  2 +-
 14 files changed, 101 insertions(+), 156 deletions(-)

-- 
1.8.3.1

[RFC 1/6] xen/arm: earlyprintk: move early_flush in early_puts

[Julien Grall]

early_puts function will be exported to be used in the console code. To
avoid loosing characters (see why in commit cafdceb "xen/arm: avoid lost
characters with early_printk), early_flush needs to be called in this
function.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/early_printk.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c
index 41938bb..7143f9e 100644
--- a/xen/arch/arm/early_printk.c
+++ b/xen/arch/arm/early_printk.c
@@ -29,12 +29,6 @@ static void __init early_puts(const char *s)
         early_putch(*s);
         s++;
     }
-}
-
-static void __init early_vprintk(const char *fmt, va_list args)
-{
-    vsnprintf(buf, sizeof(buf), fmt, args);
-    early_puts(buf);
 
     /*
      * Wait the UART has finished to transfer all characters before
@@ -43,6 +37,12 @@ static void __init early_vprintk(const char *fmt, va_list args)
     early_flush();
 }
 
+static void __init early_vprintk(const char *fmt, va_list args)
+{
+    vsnprintf(buf, sizeof(buf), fmt, args);
+    early_puts(buf);
+}
+
 void __init early_printk(const char *fmt, ...)
 {
     va_list args;
-- 
1.8.3.1

[RFC 2/6] xen/arm: earlyprintk: export early_puts

[Julien Grall]

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/early_printk.c        | 2 +-
 xen/include/asm-arm/early_printk.h | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c
index 7143f9e..affe424 100644
--- a/xen/arch/arm/early_printk.c
+++ b/xen/arch/arm/early_printk.c
@@ -21,7 +21,7 @@ void early_flush(void);
 /* Early printk buffer */
 static char __initdata buf[512];
 
-static void __init early_puts(const char *s)
+void early_puts(const char *s)
 {
     while (*s != '\0') {
         if (*s == '\n')
diff --git a/xen/include/asm-arm/early_printk.h b/xen/include/asm-arm/early_printk.h
index 707bbf7..31024b5 100644
--- a/xen/include/asm-arm/early_printk.h
+++ b/xen/include/asm-arm/early_printk.h
@@ -24,6 +24,7 @@
 
 #ifdef EARLY_PRINTK
 
+void early_puts(const char *s);
 void early_printk(const char *fmt, ...)
     __attribute__((format (printf, 1, 2)));
 void early_panic(const char *fmt, ...) __attribute__((noreturn))
@@ -31,6 +32,9 @@ void early_panic(const char *fmt, ...) __attribute__((noreturn))
 
 #else
 
+static inline void early_puts(const char *)
+{}
+
 static inline  __attribute__((format (printf, 1, 2))) void
 early_printk(const char *fmt, ...)
 {}
-- 
1.8.3.1

[RFC 3/6] xen/arm: Rename EARLY_PRINTK compile option to CONFIG_EARLY_PRINTK

[Julien Grall]

Most of common compile option start with CONFIG_. Rename EARLY_PRINTK option
to CONFIG_EARLY_PRINTK to be compliant.

This option will be used in common code (eg console) later.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/Rules.mk              |  2 +-
 xen/arch/arm/arm32/head.S          | 18 +++++++++---------
 xen/arch/arm/arm64/head.S          | 18 +++++++++---------
 xen/include/asm-arm/early_printk.h |  6 +++---
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index aaa203e..57f2eb1 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -93,7 +93,7 @@ ifneq ($(EARLY_PRINTK_INC),)
 EARLY_PRINTK := y
 endif
 
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK
+CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
 CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DEARLY_PRINTK_INIT_UART
 CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
 CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_BAUD=$(EARLY_PRINTK_BAUD)
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 96230ac..1b1801b 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -34,7 +34,7 @@
 #define PT_UPPER(x) (PT_##x & 0xf00)
 #define PT_LOWER(x) (PT_##x & 0x0ff)
 
-#if (defined (EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
+#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
 #include EARLY_PRINTK_INC
 #endif
 
@@ -59,7 +59,7 @@
  */
 /* Macro to print a string to the UART, if there is one.
  * Clobbers r0-r3. */
-#ifdef EARLY_PRINTK
+#ifdef CONFIG_EARLY_PRINTK
 #define PRINT(_s)       \
         adr   r0, 98f ; \
         bl    puts    ; \
@@ -67,9 +67,9 @@
 98:     .asciz _s     ; \
         .align 2      ; \
 99:
-#else /* EARLY_PRINTK */
+#else /* CONFIG_EARLY_PRINTK */
 #define PRINT(s)
-#endif /* !EARLY_PRINTK */
+#endif /* !CONFIG_EARLY_PRINTK */
 
         .arm
 
@@ -149,7 +149,7 @@ common_start:
         b     2b
 1:
 
-#ifdef EARLY_PRINTK
+#ifdef CONFIG_EARLY_PRINTK
         ldr   r11, =EARLY_UART_BASE_ADDRESS  /* r11 := UART base address */
         teq   r12, #0                /* Boot CPU sets up the UART too */
         bleq  init_uart
@@ -330,7 +330,7 @@ paging:
         /* Now we can install the fixmap and dtb mappings, since we
          * don't need the 1:1 map any more */
         dsb
-#if defined(EARLY_PRINTK) /* Fixmap is only used by early printk */
+#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
         /* Non-boot CPUs don't need to rebuild the fixmap itself, just
 	 * the mapping from boot_second to xen_fixmap */
         teq   r12, #0
@@ -492,7 +492,7 @@ ENTRY(relocate_xen)
 
         mov pc, lr
 
-#ifdef EARLY_PRINTK
+#ifdef CONFIG_EARLY_PRINTK
 /* Bring up the UART.
  * r11: Early UART base address
  * Clobbers r0-r2 */
@@ -537,7 +537,7 @@ putn:
 hex:    .ascii "0123456789abcdef"
         .align 2
 
-#else  /* EARLY_PRINTK */
+#else  /* CONFIG_EARLY_PRINTK */
 
 init_uart:
 .global early_puts
@@ -545,7 +545,7 @@ early_puts:
 puts:
 putn:   mov   pc, lr
 
-#endif /* !EARLY_PRINTK */
+#endif /* !CONFIG_EARLY_PRINTK */
 
 /*
  * Local variables:
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 31afdd0..c97c194 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -30,7 +30,7 @@
 #define PT_DEV    0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */
 #define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */
 
-#if (defined (EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
+#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
 #include EARLY_PRINTK_INC
 #endif
 
@@ -71,7 +71,7 @@
 
 /* Macro to print a string to the UART, if there is one.
  * Clobbers x0-x3. */
-#ifdef EARLY_PRINTK
+#ifdef CONFIG_EARLY_PRINTK
 #define PRINT(_s)       \
         adr   x0, 98f ; \
         bl    puts    ; \
@@ -79,9 +79,9 @@
 98:     .asciz _s     ; \
         .align 2      ; \
 99:
-#else /* EARLY_PRINTK */
+#else /* CONFIG_EARLY_PRINTK */
 #define PRINT(s)
-#endif /* !EARLY_PRINTK */
+#endif /* !CONFIG_EARLY_PRINTK */
 
         /*.aarch64*/
 
@@ -174,7 +174,7 @@ common_start:
         b     2b
 1:
 
-#ifdef EARLY_PRINTK
+#ifdef CONFIG_EARLY_PRINTK
         ldr   x23, =EARLY_UART_BASE_ADDRESS /* x23 := UART base address */
         cbnz  x22, 1f
         bl    init_uart                 /* Boot CPU sets up the UART too */
@@ -343,7 +343,7 @@ paging:
         /* Now we can install the fixmap and dtb mappings, since we
          * don't need the 1:1 map any more */
         dsb   sy
-#if defined(EARLY_PRINTK) /* Fixmap is only used by early printk */
+#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
         /* Non-boot CPUs don't need to rebuild the fixmap itself, just
 	 * the mapping from boot_second to xen_fixmap */
         cbnz  x22, 1f
@@ -489,7 +489,7 @@ ENTRY(relocate_xen)
 
         ret
 
-#ifdef EARLY_PRINTK
+#ifdef CONFIG_EARLY_PRINTK
 /* Bring up the UART.
  * x23: Early UART base address
  * Clobbers x0-x1 */
@@ -536,7 +536,7 @@ putn:
 hex:    .ascii "0123456789abcdef"
         .align 2
 
-#else  /* EARLY_PRINTK */
+#else  /* CONFIG_EARLY_PRINTK */
 
 init_uart:
 .global early_puts
@@ -544,7 +544,7 @@ early_puts:
 puts:
 putn:   ret
 
-#endif /* EARLY_PRINTK */
+#endif /* !CONFIG_EARLY_PRINTK */
 
 /*
  * Local variables:
diff --git a/xen/include/asm-arm/early_printk.h b/xen/include/asm-arm/early_printk.h
index 31024b5..a58e3e7 100644
--- a/xen/include/asm-arm/early_printk.h
+++ b/xen/include/asm-arm/early_printk.h
@@ -12,7 +12,7 @@
 
 #include <xen/config.h>
 
-#ifdef EARLY_PRINTK
+#ifdef CONFIG_EARLY_PRINTK
 
 /* need to add the uart address offset in page to the fixmap address */
 #define EARLY_UART_VIRTUAL_ADDRESS \
@@ -22,7 +22,7 @@
 
 #ifndef __ASSEMBLY__
 
-#ifdef EARLY_PRINTK
+#ifdef CONFIG_EARLY_PRINTK
 
 void early_puts(const char *s);
 void early_printk(const char *fmt, ...)
@@ -43,7 +43,7 @@ static inline void  __attribute__((noreturn))
 __attribute__((format (printf, 1, 2))) early_panic(const char *fmt, ...)
 {while(1);}
 
-#endif
+#endif /* !CONFIG_EARLY_PRINTK */
 
 #endif	/* __ASSEMBLY__ */
 
-- 
1.8.3.1

[RFC 4/6] xen/console: Add support for early printk

[Julien Grall]

On ARM, a function (early_printk) was introduced to output message when the
serial port is not initialized.

This solution is fragile because the developper needs to know when the serial
port is initialized, to use either early_printk or printk. Moreover some
functions (mainly in common code), only use printk. This will result to a loss
of message sometimes.

Directly call early_printk in console code when the serial port is not yet
initialized. For this purpose use serial_steal_fn.

Cc: Keir Fraser <keir@xen.org>
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/drivers/char/console.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 532c426..f83c92e 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -28,6 +28,9 @@
 #include <asm/debugger.h>
 #include <asm/div64.h>
 #include <xen/hypercall.h> /* for do_console_io */
+#ifdef CONFIG_EARLY_PRINTK
+#include <asm/early_printk.h>
+#endif
 
 /* console: comma-separated list of console outputs. */
 static char __initdata opt_console[30] = OPT_CONSOLE_STR;
@@ -245,7 +248,12 @@ long read_console_ring(struct xen_sysctl_readconsole *op)
 static char serial_rx_ring[SERIAL_RX_SIZE];
 static unsigned int serial_rx_cons, serial_rx_prod;
 
-static void (*serial_steal_fn)(const char *);
+#ifndef CONFIG_EARLY_PRINTK
+static inline void early_puts(const char *str)
+{}
+#endif
+
+static void (*serial_steal_fn)(const char *) = early_puts;
 
 int console_steal(int handle, void (*fn)(const char *))
 {
@@ -652,7 +660,10 @@ void __init console_init_preirq(void)
         else if ( !strncmp(p, "none", 4) )
             continue;
         else if ( (sh = serial_parse_handle(p)) >= 0 )
+        {
             sercon_handle = sh;
+            serial_steal_fn = NULL;
+        }
         else
         {
             char *q = strchr(p, ',');
-- 
1.8.3.1

[RFC 5/6] xen/console: Add noreturn attribute to panic function

[Julien Grall]

Panic function will never return. Without this attribute, gcc may output
warnings in call function.

Cc: Keir Fraser <keir@xen.org>
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/drivers/char/console.c | 4 +++-
 xen/include/xen/lib.h      | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index f83c92e..229d48a 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1049,7 +1049,7 @@ __initcall(debugtrace_init);
  * **************************************************************
  */
 
-void panic(const char *fmt, ...)
+void __attribute__((noreturn)) panic(const char *fmt, ...)
 {
     va_list args;
     unsigned long flags;
@@ -1092,6 +1092,8 @@ void panic(const char *fmt, ...)
         watchdog_disable();
         machine_restart(5000);
     }
+
+    while ( 1 );
 }
 
 void __bug(char *file, int line)
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 5b258fd..9c3a242 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -88,7 +88,7 @@ extern void printk(const char *format, ...)
 extern void guest_printk(const struct domain *d, const char *format, ...)
     __attribute__ ((format (printf, 2, 3)));
 extern void panic(const char *format, ...)
-    __attribute__ ((format (printf, 1, 2)));
+    __attribute__ ((format (printf, 1, 2))) __attribute__ ((noreturn));
 extern long vm_assist(struct domain *, unsigned int, unsigned int);
 extern int __printk_ratelimit(int ratelimit_ms, int ratelimit_burst);
 extern int printk_ratelimit(void);
-- 
1.8.3.1

[RFC 6/6] xen/arm: Replace early_printk call to printk call

[Julien Grall]

Now that the console supports earlyprintk, we can get a rid of
early_printk call.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/early_printk.c        | 32 --------------------------------
 xen/arch/arm/setup.c               | 28 +++++++++++++---------------
 xen/common/device_tree.c           | 36 +++++++++++++-----------------------
 xen/drivers/char/dt-uart.c         |  9 ++++-----
 xen/drivers/char/exynos4210-uart.c | 13 +++++--------
 xen/drivers/char/omap-uart.c       | 13 ++++++-------
 xen/drivers/char/pl011.c           | 13 ++++++-------
 xen/drivers/video/arm_hdlcd.c      | 29 ++++++++++++++---------------
 xen/include/asm-arm/early_printk.h | 23 +----------------------
 9 files changed, 62 insertions(+), 134 deletions(-)

diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c
index affe424..9119c8c 100644
--- a/xen/arch/arm/early_printk.c
+++ b/xen/arch/arm/early_printk.c
@@ -18,9 +18,6 @@
 void early_putch(char c);
 void early_flush(void);
 
-/* Early printk buffer */
-static char __initdata buf[512];
-
 void early_puts(const char *s)
 {
     while (*s != '\0') {
@@ -36,32 +33,3 @@ void early_puts(const char *s)
      */
     early_flush();
 }
-
-static void __init early_vprintk(const char *fmt, va_list args)
-{
-    vsnprintf(buf, sizeof(buf), fmt, args);
-    early_puts(buf);
-}
-
-void __init early_printk(const char *fmt, ...)
-{
-    va_list args;
-
-    va_start(args, fmt);
-    early_vprintk(fmt, args);
-    va_end(args);
-}
-
-void __attribute__((noreturn)) __init
-early_panic(const char *fmt, ...)
-{
-    va_list args;
-
-    va_start(args, fmt);
-    early_vprintk(fmt, args);
-    va_end(args);
-
-    early_printk("\n\nEarly Panic: Stopping\n");
-
-    while(1);
-}
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 840b04b..76b4273 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -39,7 +39,6 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/setup.h>
-#include <asm/early_printk.h>
 #include <asm/gic.h>
 #include <asm/cpufeature.h>
 #include <asm/platform.h>
@@ -346,10 +345,10 @@ static paddr_t __init get_xen_paddr(void)
     }
 
     if ( !paddr )
-        early_panic("Not enough memory to relocate Xen");
+        panic("Not enough memory to relocate Xen");
 
-    early_printk("Placing Xen at 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
-                 paddr, paddr + min_size);
+    printk("Placing Xen at 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
+           paddr, paddr + min_size);
 
     early_info.modules.module[MOD_XEN].start = paddr;
     early_info.modules.module[MOD_XEN].size = min_size;
@@ -371,7 +370,7 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
     void *fdt;
 
     if ( !early_info.mem.nr_banks )
-        early_panic("No memory bank");
+        panic("No memory bank");
 
     /*
      * We are going to accumulate two regions here.
@@ -430,8 +429,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
 
     if ( i != early_info.mem.nr_banks )
     {
-        early_printk("WARNING: only using %d out of %d memory banks\n",
-                     i, early_info.mem.nr_banks);
+        printk("WARNING: only using %d out of %d memory banks\n",
+               i, early_info.mem.nr_banks);
         early_info.mem.nr_banks = i;
     }
 
@@ -465,14 +464,13 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
     } while ( xenheap_pages > 128<<(20-PAGE_SHIFT) );
 
     if ( ! e )
-        early_panic("Not not enough space for xenheap");
+        panic("Not not enough space for xenheap");
 
     domheap_pages = heap_pages - xenheap_pages;
 
-    early_printk("Xen heap: %"PRIpaddr"-%"PRIpaddr" (%lu pages)\n",
-                 e - (pfn_to_paddr(xenheap_pages)), e,
-                 xenheap_pages);
-    early_printk("Dom heap: %lu pages\n", domheap_pages);
+    printk("Xen heap: %"PRIpaddr"-%"PRIpaddr" (%lu pages)\n",
+            e - (pfn_to_paddr(xenheap_pages)), e, xenheap_pages);
+    printk("Dom heap: %lu pages\n", domheap_pages);
 
     setup_xenheap_mappings((e >> PAGE_SHIFT) - xenheap_pages, xenheap_pages);
 
@@ -606,8 +604,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
 
     if ( bank != early_info.mem.nr_banks )
     {
-        early_printk("WARNING: only using %d out of %d memory banks\n",
-                     bank, early_info.mem.nr_banks);
+        printk("WARNING: only using %d out of %d memory banks\n",
+               bank, early_info.mem.nr_banks);
         early_info.mem.nr_banks = bank;
     }
 
@@ -672,7 +670,7 @@ void __init start_xen(unsigned long boot_phys_offset,
     fdt_size = device_tree_early_init(device_tree_flattened, fdt_paddr);
 
     cmdline = device_tree_bootargs(device_tree_flattened);
-    early_printk("Command line: %s\n", cmdline);
+    printk("Command line: %s\n", cmdline);
     cmdline_parse(cmdline);
 
     setup_pagetables(boot_phys_offset, get_xen_paddr());
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 84e709d..c35aee1 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -23,7 +23,6 @@
 #include <xen/cpumask.h>
 #include <xen/ctype.h>
 #include <xen/lib.h>
-#include <asm/early_printk.h>
 
 struct dt_early_info __initdata early_info;
 const void *device_tree_flattened;
@@ -54,16 +53,7 @@ struct dt_alias_prop {
 
 static LIST_HEAD(aliases_lookup);
 
-/* Some device tree functions may be called both before and after the
-   console is initialized. */
-#define dt_printk(fmt, ...)                         \
-    do                                              \
-    {                                               \
-        if ( system_state == SYS_STATE_early_boot ) \
-            early_printk(fmt, ## __VA_ARGS__);      \
-        else                                        \
-            printk(fmt, ## __VA_ARGS__);            \
-    } while (0)
+#define dt_printk(fmt, ...) printk(fmt, ## __VA_ARGS__);
 
 // #define DEBUG_DT
 
@@ -316,7 +306,7 @@ static void __init process_memory_node(const void *fdt, int node,
 
     if ( address_cells < 1 || size_cells < 1 )
     {
-        early_printk("fdt: node `%s': invalid #address-cells or #size-cells",
+        dt_printk("fdt: node `%s': invalid #address-cells or #size-cells",
                      name);
         return;
     }
@@ -324,7 +314,7 @@ static void __init process_memory_node(const void *fdt, int node,
     prop = fdt_get_property(fdt, node, "reg", NULL);
     if ( !prop )
     {
-        early_printk("fdt: node `%s': missing `reg' property\n", name);
+        dt_printk("fdt: node `%s': missing `reg' property\n", name);
         return;
     }
 
@@ -355,16 +345,16 @@ static void __init process_multiboot_node(const void *fdt, int node,
     else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0)
         nr = MOD_INITRD;
     else
-        early_panic("%s not a known xen multiboot type\n", name);
+        panic("%s not a known xen multiboot type\n", name);
 
     mod = &early_info.modules.module[nr];
 
     prop = fdt_get_property(fdt, node, "reg", &len);
     if ( !prop )
-        early_panic("node %s missing `reg' property\n", name);
+        panic("node %s missing `reg' property\n", name);
 
     if ( len < dt_cells_to_size(address_cells + size_cells) )
-        early_panic("fdt: node `%s': `reg` property length is too short\n",
+        panic("fdt: node `%s': `reg` property length is too short\n",
                     name);
 
     cell = (const __be32 *)prop->data;
@@ -375,7 +365,7 @@ static void __init process_multiboot_node(const void *fdt, int node,
     if ( prop )
     {
         if ( len > sizeof(mod->cmdline) )
-            early_panic("module %d command line too long\n", nr);
+            panic("module %d command line too long\n", nr);
 
         safe_strcpy(mod->cmdline, prop->data);
     }
@@ -458,12 +448,12 @@ static void __init early_print_info(void)
     int i, nr_rsvd;
 
     for ( i = 0; i < mi->nr_banks; i++ )
-        early_printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n",
+        dt_printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n",
                      mi->bank[i].start,
                      mi->bank[i].start + mi->bank[i].size - 1);
-    early_printk("\n");
+    dt_printk("\n");
     for ( i = 1 ; i < mods->nr_mods + 1; i++ )
-        early_printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %s\n",
+        dt_printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %s\n",
                      i,
                      mods->module[i].start,
                      mods->module[i].start + mods->module[i].size,
@@ -476,10 +466,10 @@ static void __init early_print_info(void)
             continue;
         /* fdt_get_mem_rsv returns length */
         e += s;
-        early_printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n",
+        dt_printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n",
                      i, s, e);
     }
-    early_printk("\n");
+    dt_printk("\n");
 }
 
 /**
@@ -495,7 +485,7 @@ size_t __init device_tree_early_init(const void *fdt, paddr_t paddr)
 
     ret = fdt_check_header(fdt);
     if ( ret < 0 )
-        early_panic("No valid device tree\n");
+        panic("No valid device tree\n");
 
     mod = &early_info.modules.module[MOD_FDT];
     mod->start = paddr;
diff --git a/xen/drivers/char/dt-uart.c b/xen/drivers/char/dt-uart.c
index d7204fb..fa92b5c 100644
--- a/xen/drivers/char/dt-uart.c
+++ b/xen/drivers/char/dt-uart.c
@@ -18,7 +18,6 @@
  */
 
 #include <asm/device.h>
-#include <asm/early_printk.h>
 #include <asm/types.h>
 #include <xen/console.h>
 #include <xen/device_tree.h>
@@ -44,7 +43,7 @@ void __init dt_uart_init(void)
 
     if ( !console_has("dtuart") || !strcmp(opt_dtuart, "") )
     {
-        early_printk("No console\n");
+        printk("No console\n");
         return;
     }
 
@@ -54,7 +53,7 @@ void __init dt_uart_init(void)
     else
         options = "";
 
-    early_printk("Looking for UART console %s\n", devpath);
+    printk("Looking for UART console %s\n", devpath);
     if ( *devpath == '/' )
         dev = dt_find_node_by_path(devpath);
     else
@@ -62,12 +61,12 @@ void __init dt_uart_init(void)
 
     if ( !dev )
     {
-        early_printk("Unable to find device \"%s\"\n", devpath);
+        printk("Unable to find device \"%s\"\n", devpath);
         return;
     }
 
     ret = device_init(dev, DEVICE_SERIAL, options);
 
     if ( ret )
-        early_printk("Unable to initialize serial: %d\n", ret);
+        printk("Unable to initialize serial: %d\n", ret);
 }
diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
index 0a2ac17..17ba010 100644
--- a/xen/drivers/char/exynos4210-uart.c
+++ b/xen/drivers/char/exynos4210-uart.c
@@ -24,7 +24,6 @@
 #include <xen/init.h>
 #include <xen/irq.h>
 #include <xen/mm.h>
-#include <asm/early_printk.h>
 #include <asm/device.h>
 #include <asm/exynos4210-uart.h>
 #include <asm/io.h>
@@ -314,9 +313,7 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
     u64 addr, size;
 
     if ( strcmp(config, "") )
-    {
-        early_printk("WARNING: UART configuration is not supported\n");
-    }
+        printk("WARNING: UART configuration is not supported\n");
 
     uart = &exynos4210_com;
 
@@ -329,21 +326,21 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
     res = dt_device_get_address(dev, 0, &addr, &size);
     if ( res )
     {
-        early_printk("exynos4210: Unable to retrieve the base"
-                     " address of the UART\n");
+        printk("exynos4210: Unable to retrieve the base"
+               " address of the UART\n");
         return res;
     }
 
     uart->regs = ioremap_nocache(addr, size);
     if ( !uart->regs )
     {
-        early_printk("exynos4210: Unable to map the UART memory\n");
+        printk("exynos4210: Unable to map the UART memory\n");
         return -ENOMEM;
     }
     res = dt_device_get_irq(dev, 0, &uart->irq);
     if ( res )
     {
-        early_printk("exynos4210: Unable to retrieve the IRQ\n");
+        printk("exynos4210: Unable to retrieve the IRQ\n");
         return res;
     }
 
diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
index 321e636..ad5aabb 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -15,7 +15,6 @@
 #include <xen/serial.h>
 #include <xen/init.h>
 #include <xen/irq.h>
-#include <asm/early_printk.h>
 #include <xen/device_tree.h>
 #include <asm/device.h>
 #include <xen/errno.h>
@@ -301,14 +300,14 @@ static int __init omap_uart_init(struct dt_device_node *dev,
     u64 addr, size;
 
     if ( strcmp(config, "") )
-        early_printk("WARNING: UART configuration is not supported\n");
+        printk("WARNING: UART configuration is not supported\n");
 
     uart = &omap_com;
 
     res = dt_property_read_u32(dev, "clock-frequency", &clkspec);
     if ( !res )
     {
-        early_printk("omap-uart: Unable to retrieve the clock frequency\n");
+        printk("omap-uart: Unable to retrieve the clock frequency\n");
         return -EINVAL;
     }
 
@@ -321,22 +320,22 @@ static int __init omap_uart_init(struct dt_device_node *dev,
     res = dt_device_get_address(dev, 0, &addr, &size);
     if ( res )
     {
-        early_printk("omap-uart: Unable to retrieve the base"
-                     " address of the UART\n");
+        printk("omap-uart: Unable to retrieve the base"
+               " address of the UART\n");
         return res;
     }
 
     uart->regs = ioremap_attr(addr, size, PAGE_HYPERVISOR_NOCACHE);
     if ( !uart->regs )
     {
-        early_printk("omap-uart: Unable to map the UART memory\n");
+        printk("omap-uart: Unable to map the UART memory\n");
         return -ENOMEM;
     }
 
     res = dt_device_get_irq(dev, 0, &uart->irq);
     if ( res )
     {
-        early_printk("omap-uart: Unable to retrieve the IRQ\n");
+        printk("omap-uart: Unable to retrieve the IRQ\n");
         return res;
     }
 
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 613b9eb..378d37e 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -22,7 +22,6 @@
 #include <xen/serial.h>
 #include <xen/init.h>
 #include <xen/irq.h>
-#include <asm/early_printk.h>
 #include <xen/device_tree.h>
 #include <xen/errno.h>
 #include <asm/device.h>
@@ -107,7 +106,7 @@ static void __init pl011_init_preirq(struct serial_port *port)
         /* Baud rate already set: read it out from the divisor latch. */
         divisor = (pl011_read(uart, IBRD) << 6) | (pl011_read(uart, FBRD));
         if (!divisor)
-            early_panic("pl011: No Baud rate configured\n");
+            panic("pl011: No Baud rate configured\n");
         uart->baud = (uart->clock_hz << 2) / divisor;
     }
     /* This write must follow FBRD and IBRD writes. */
@@ -229,7 +228,7 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
 
     if ( strcmp(config, "") )
     {
-        early_printk("WARNING: UART configuration is not supported\n");
+        printk("WARNING: UART configuration is not supported\n");
     }
 
     uart = &pl011_com;
@@ -243,15 +242,15 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
     res = dt_device_get_address(dev, 0, &addr, &size);
     if ( res )
     {
-        early_printk("pl011: Unable to retrieve the base"
-                     " address of the UART\n");
+        printk("pl011: Unable to retrieve the base"
+               " address of the UART\n");
         return res;
     }
 
     uart->regs = ioremap_attr(addr, size, PAGE_HYPERVISOR_NOCACHE);
     if ( !uart->regs )
     {
-        early_printk("pl011: Unable to map the UART memory\n");
+        printk("pl011: Unable to map the UART memory\n");
 
         return -ENOMEM;
     }
@@ -259,7 +258,7 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
     res = dt_device_get_irq(dev, 0, &uart->irq);
     if ( res )
     {
-        early_printk("pl011: Unable to retrieve the IRQ\n");
+        printk("pl011: Unable to retrieve the IRQ\n");
         return res;
     }
 
diff --git a/xen/drivers/video/arm_hdlcd.c b/xen/drivers/video/arm_hdlcd.c
index 647f22c..2a5f72e 100644
--- a/xen/drivers/video/arm_hdlcd.c
+++ b/xen/drivers/video/arm_hdlcd.c
@@ -25,7 +25,6 @@
 #include <xen/libfdt/libfdt.h>
 #include <xen/init.h>
 #include <xen/mm.h>
-#include <asm/early_printk.h>
 #include "font.h"
 #include "lfb.h"
 #include "modelines.h"
@@ -123,21 +122,21 @@ void __init video_init(void)
 
     if ( !dev )
     {
-        early_printk("HDLCD: Cannot find node compatible with \"arm,hdcld\"\n");
+        printk("HDLCD: Cannot find node compatible with \"arm,hdcld\"\n");
         return;
     }
 
     res = dt_device_get_address(dev, 0, &hdlcd_start, &hdlcd_size);
     if ( !res )
     {
-        early_printk("HDLCD: Unable to retrieve MMIO base address\n");
+        printk("HDLCD: Unable to retrieve MMIO base address\n");
         return;
     }
 
     cells = dt_get_property(dev, "framebuffer", &lenp);
     if ( !cells )
     {
-        early_printk("HDLCD: Unable to retrieve framebuffer property\n");
+        printk("HDLCD: Unable to retrieve framebuffer property\n");
         return;
     }
 
@@ -146,13 +145,13 @@ void __init video_init(void)
 
     if ( !hdlcd_start )
     {
-        early_printk(KERN_ERR "HDLCD: address missing from device tree, disabling driver\n");
+        printk(KERN_ERR "HDLCD: address missing from device tree, disabling driver\n");
         return;
     }
 
     if ( !framebuffer_start )
     {
-        early_printk(KERN_ERR "HDLCD: framebuffer address missing from device tree, disabling driver\n");
+        printk(KERN_ERR "HDLCD: framebuffer address missing from device tree, disabling driver\n");
         return;
     }
 
@@ -166,13 +165,13 @@ void __init video_init(void)
     else if ( strlen(mode_string) < strlen("800x600@60") ||
             strlen(mode_string) > sizeof(_mode_string) - 1 )
     {
-        early_printk(KERN_ERR "HDLCD: invalid modeline=%s\n", mode_string);
+        printk(KERN_ERR "HDLCD: invalid modeline=%s\n", mode_string);
         return;
     } else {
         char *s = strchr(mode_string, '-');
         if ( !s )
         {
-            early_printk(KERN_INFO "HDLCD: bpp not found in modeline %s, assume 32 bpp\n",
+            printk(KERN_INFO "HDLCD: bpp not found in modeline %s, assume 32 bpp\n",
                          mode_string);
             get_color_masks("32", &c);
             memcpy(_mode_string, mode_string, strlen(mode_string) + 1);
@@ -180,13 +179,13 @@ void __init video_init(void)
         } else {
             if ( strlen(s) < 6 )
             {
-                early_printk(KERN_ERR "HDLCD: invalid mode %s\n", mode_string);
+                printk(KERN_ERR "HDLCD: invalid mode %s\n", mode_string);
                 return;
             }
             s++;
             if ( get_color_masks(s, &c) < 0 )
             {
-                early_printk(KERN_WARNING "HDLCD: unsupported bpp %s\n", s);
+                printk(KERN_WARNING "HDLCD: unsupported bpp %s\n", s);
                 return;
             }
             bytes_per_pixel = simple_strtoll(s, NULL, 10) / 8;
@@ -205,23 +204,23 @@ void __init video_init(void)
     }
     if ( !videomode )
     {
-        early_printk(KERN_WARNING "HDLCD: unsupported videomode %s\n",
-                     _mode_string);
+        printk(KERN_WARNING "HDLCD: unsupported videomode %s\n",
+               _mode_string);
         return;
     }
 
     if ( framebuffer_size < bytes_per_pixel * videomode->xres * videomode->yres )
     {
-        early_printk(KERN_ERR "HDLCD: the framebuffer is too small, disabling the HDLCD driver\n");
+        printk(KERN_ERR "HDLCD: the framebuffer is too small, disabling the HDLCD driver\n");
         return;
     }
 
-    early_printk(KERN_INFO "Initializing HDLCD driver\n");
+    printk(KERN_INFO "Initializing HDLCD driver\n");
 
     lfb = ioremap_wc(framebuffer_start, framebuffer_size);
     if ( !lfb )
     {
-        early_printk(KERN_ERR "Couldn't map the framebuffer\n");
+        printk(KERN_ERR "Couldn't map the framebuffer\n");
         return;
     }
     memset(lfb, 0x00, bytes_per_pixel * videomode->xres * videomode->yres);
diff --git a/xen/include/asm-arm/early_printk.h b/xen/include/asm-arm/early_printk.h
index a58e3e7..6569397 100644
--- a/xen/include/asm-arm/early_printk.h
+++ b/xen/include/asm-arm/early_printk.h
@@ -18,33 +18,12 @@
 #define EARLY_UART_VIRTUAL_ADDRESS \
     (FIXMAP_ADDR(FIXMAP_CONSOLE) +(EARLY_UART_BASE_ADDRESS & ~PAGE_MASK))
 
-#endif
-
 #ifndef __ASSEMBLY__
 
-#ifdef CONFIG_EARLY_PRINTK
-
 void early_puts(const char *s);
-void early_printk(const char *fmt, ...)
-    __attribute__((format (printf, 1, 2)));
-void early_panic(const char *fmt, ...) __attribute__((noreturn))
-    __attribute__((format (printf, 1, 2)));
 
-#else
-
-static inline void early_puts(const char *)
-{}
-
-static inline  __attribute__((format (printf, 1, 2))) void
-early_printk(const char *fmt, ...)
-{}
-
-static inline void  __attribute__((noreturn))
-__attribute__((format (printf, 1, 2))) early_panic(const char *fmt, ...)
-{while(1);}
+#endif /* !__ASSEMBLY__ */
 
 #endif /* !CONFIG_EARLY_PRINTK */
 
-#endif	/* __ASSEMBLY__ */
-
 #endif
-- 
1.8.3.1

Re: [RFC 5/6] xen/console: Add noreturn attribute to panic function

[Andrew Cooper]

On 05/01/2014 21:26, Julien Grall wrote:
> Panic function will never return. Without this attribute, gcc may output
> warnings in call function.
>
> Cc: Keir Fraser <keir@xen.org>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

I have a longer series doing rather more noreturn'ing than just this, if
you can wait until the 4.5 dev window opens up again.

~Andrew

> ---
>  xen/drivers/char/console.c | 4 +++-
>  xen/include/xen/lib.h      | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index f83c92e..229d48a 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -1049,7 +1049,7 @@ __initcall(debugtrace_init);
>   * **************************************************************
>   */
>  
> -void panic(const char *fmt, ...)
> +void __attribute__((noreturn)) panic(const char *fmt, ...)
>  {
>      va_list args;
>      unsigned long flags;
> @@ -1092,6 +1092,8 @@ void panic(const char *fmt, ...)
>          watchdog_disable();
>          machine_restart(5000);
>      }
> +
> +    while ( 1 );
>  }
>  
>  void __bug(char *file, int line)
> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> index 5b258fd..9c3a242 100644
> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -88,7 +88,7 @@ extern void printk(const char *format, ...)
>  extern void guest_printk(const struct domain *d, const char *format, ...)
>      __attribute__ ((format (printf, 2, 3)));
>  extern void panic(const char *format, ...)
> -    __attribute__ ((format (printf, 1, 2)));
> +    __attribute__ ((format (printf, 1, 2))) __attribute__ ((noreturn));
>  extern long vm_assist(struct domain *, unsigned int, unsigned int);
>  extern int __printk_ratelimit(int ratelimit_ms, int ratelimit_burst);
>  extern int printk_ratelimit(void);

Re: [RFC 5/6] xen/console: Add noreturn attribute to panic function

[Julien Grall]

On 01/05/2014 10:44 PM, Andrew Cooper wrote:
> On 05/01/2014 21:26, Julien Grall wrote:
>> Panic function will never return. Without this attribute, gcc may output
>> warnings in call function.
>>
>> Cc: Keir Fraser <keir@xen.org>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>
> I have a longer series doing rather more noreturn'ing than just this, if
> you can wait until the 4.5 dev window opens up again.

I'm not sure to get what you said. When early_panic is converted to 
panic function, I got some gcc warning which result in errors, because 
of -Werror.

So without this small patch, this series can't compile.

-- 
Julien Grall

Re: [RFC 5/6] xen/console: Add noreturn attribute to panic function

[Andrew Cooper]

On 06/01/14 11:39, Julien Grall wrote:
>
>
> On 01/05/2014 10:44 PM, Andrew Cooper wrote:
>> On 05/01/2014 21:26, Julien Grall wrote:
>>> Panic function will never return. Without this attribute, gcc may
>>> output
>>> warnings in call function.
>>>
>>> Cc: Keir Fraser <keir@xen.org>
>>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>
>> I have a longer series doing rather more noreturn'ing than just this, if
>> you can wait until the 4.5 dev window opens up again.
>
> I'm not sure to get what you said. When early_panic is converted to
> panic function, I got some gcc warning which result in errors, because
> of -Werror.
>
> So without this small patch, this series can't compile.
>

My series makes panic() (along with other several other function)
properly noreturn, so you don't need the while(1); at the end.

i.e., if your series can wait until after mine is committed, this
problem will disappear.

~Andrew

Re: [RFC 5/6] xen/console: Add noreturn attribute to panic function

[Julien Grall]

On 01/06/2014 11:42 AM, Andrew Cooper wrote:
> On 06/01/14 11:39, Julien Grall wrote:
>
> My series makes panic() (along with other several other function)
> properly noreturn, so you don't need the while(1); at the end.
>
> i.e., if your series can wait until after mine is committed, this
> problem will disappear.

This series is not for Xen 4.4, so I'm fine to wait your series.

-- 
Julien Grall

Re: [RFC 1/6] xen/arm: earlyprintk: move early_flush in early_puts

[Ian Campbell]

On Sun, 2014-01-05 at 21:26 +0000, Julien Grall wrote:
> early_puts function will be exported to be used in the console code. To
> avoid loosing characters (see why in commit cafdceb "xen/arm: avoid lost
> characters with early_printk), early_flush needs to be called in this
> function.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

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

> ---
>  xen/arch/arm/early_printk.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c
> index 41938bb..7143f9e 100644
> --- a/xen/arch/arm/early_printk.c
> +++ b/xen/arch/arm/early_printk.c
> @@ -29,12 +29,6 @@ static void __init early_puts(const char *s)
>          early_putch(*s);
>          s++;
>      }
> -}
> -
> -static void __init early_vprintk(const char *fmt, va_list args)
> -{
> -    vsnprintf(buf, sizeof(buf), fmt, args);
> -    early_puts(buf);
>  
>      /*
>       * Wait the UART has finished to transfer all characters before
> @@ -43,6 +37,12 @@ static void __init early_vprintk(const char *fmt, va_list args)
>      early_flush();
>  }
>  
> +static void __init early_vprintk(const char *fmt, va_list args)
> +{
> +    vsnprintf(buf, sizeof(buf), fmt, args);
> +    early_puts(buf);
> +}
> +
>  void __init early_printk(const char *fmt, ...)
>  {
>      va_list args;

Re: [RFC 2/6] xen/arm: earlyprintk: export early_puts

[Ian Campbell]

On Sun, 2014-01-05 at 21:26 +0000, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

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

> [..]
> +static inline void early_puts(const char *)
> +{}

We'd normally put these at the end of the line with the prototype (I see
this is wrong for early_printk too)

> +
>  static inline  __attribute__((format (printf, 1, 2))) void
>  early_printk(const char *fmt, ...)
>  {}

Re: [RFC 3/6] xen/arm: Rename EARLY_PRINTK compile option to CONFIG_EARLY_PRINTK

[Ian Campbell]

On Sun, 2014-01-05 at 21:26 +0000, Julien Grall wrote:
> Most of common compile option start with CONFIG_. Rename EARLY_PRINTK option
> to CONFIG_EARLY_PRINTK to be compliant.
> 
> This option will be used in common code (eg console) later.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

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

> ---
>  xen/arch/arm/Rules.mk              |  2 +-
>  xen/arch/arm/arm32/head.S          | 18 +++++++++---------
>  xen/arch/arm/arm64/head.S          | 18 +++++++++---------
>  xen/include/asm-arm/early_printk.h |  6 +++---
>  4 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index aaa203e..57f2eb1 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -93,7 +93,7 @@ ifneq ($(EARLY_PRINTK_INC),)
>  EARLY_PRINTK := y
>  endif
>  
> -CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK
> +CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
>  CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DEARLY_PRINTK_INIT_UART
>  CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
>  CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_BAUD=$(EARLY_PRINTK_BAUD)
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 96230ac..1b1801b 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -34,7 +34,7 @@
>  #define PT_UPPER(x) (PT_##x & 0xf00)
>  #define PT_LOWER(x) (PT_##x & 0x0ff)
>  
> -#if (defined (EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
>  #include EARLY_PRINTK_INC
>  #endif
>  
> @@ -59,7 +59,7 @@
>   */
>  /* Macro to print a string to the UART, if there is one.
>   * Clobbers r0-r3. */
> -#ifdef EARLY_PRINTK
> +#ifdef CONFIG_EARLY_PRINTK
>  #define PRINT(_s)       \
>          adr   r0, 98f ; \
>          bl    puts    ; \
> @@ -67,9 +67,9 @@
>  98:     .asciz _s     ; \
>          .align 2      ; \
>  99:
> -#else /* EARLY_PRINTK */
> +#else /* CONFIG_EARLY_PRINTK */
>  #define PRINT(s)
> -#endif /* !EARLY_PRINTK */
> +#endif /* !CONFIG_EARLY_PRINTK */
>  
>          .arm
>  
> @@ -149,7 +149,7 @@ common_start:
>          b     2b
>  1:
>  
> -#ifdef EARLY_PRINTK
> +#ifdef CONFIG_EARLY_PRINTK
>          ldr   r11, =EARLY_UART_BASE_ADDRESS  /* r11 := UART base address */
>          teq   r12, #0                /* Boot CPU sets up the UART too */
>          bleq  init_uart
> @@ -330,7 +330,7 @@ paging:
>          /* Now we can install the fixmap and dtb mappings, since we
>           * don't need the 1:1 map any more */
>          dsb
> -#if defined(EARLY_PRINTK) /* Fixmap is only used by early printk */
> +#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
>          /* Non-boot CPUs don't need to rebuild the fixmap itself, just
>  	 * the mapping from boot_second to xen_fixmap */
>          teq   r12, #0
> @@ -492,7 +492,7 @@ ENTRY(relocate_xen)
>  
>          mov pc, lr
>  
> -#ifdef EARLY_PRINTK
> +#ifdef CONFIG_EARLY_PRINTK
>  /* Bring up the UART.
>   * r11: Early UART base address
>   * Clobbers r0-r2 */
> @@ -537,7 +537,7 @@ putn:
>  hex:    .ascii "0123456789abcdef"
>          .align 2
>  
> -#else  /* EARLY_PRINTK */
> +#else  /* CONFIG_EARLY_PRINTK */
>  
>  init_uart:
>  .global early_puts
> @@ -545,7 +545,7 @@ early_puts:
>  puts:
>  putn:   mov   pc, lr
>  
> -#endif /* !EARLY_PRINTK */
> +#endif /* !CONFIG_EARLY_PRINTK */
>  
>  /*
>   * Local variables:
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 31afdd0..c97c194 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -30,7 +30,7 @@
>  #define PT_DEV    0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */
>  #define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */
>  
> -#if (defined (EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
>  #include EARLY_PRINTK_INC
>  #endif
>  
> @@ -71,7 +71,7 @@
>  
>  /* Macro to print a string to the UART, if there is one.
>   * Clobbers x0-x3. */
> -#ifdef EARLY_PRINTK
> +#ifdef CONFIG_EARLY_PRINTK
>  #define PRINT(_s)       \
>          adr   x0, 98f ; \
>          bl    puts    ; \
> @@ -79,9 +79,9 @@
>  98:     .asciz _s     ; \
>          .align 2      ; \
>  99:
> -#else /* EARLY_PRINTK */
> +#else /* CONFIG_EARLY_PRINTK */
>  #define PRINT(s)
> -#endif /* !EARLY_PRINTK */
> +#endif /* !CONFIG_EARLY_PRINTK */
>  
>          /*.aarch64*/
>  
> @@ -174,7 +174,7 @@ common_start:
>          b     2b
>  1:
>  
> -#ifdef EARLY_PRINTK
> +#ifdef CONFIG_EARLY_PRINTK
>          ldr   x23, =EARLY_UART_BASE_ADDRESS /* x23 := UART base address */
>          cbnz  x22, 1f
>          bl    init_uart                 /* Boot CPU sets up the UART too */
> @@ -343,7 +343,7 @@ paging:
>          /* Now we can install the fixmap and dtb mappings, since we
>           * don't need the 1:1 map any more */
>          dsb   sy
> -#if defined(EARLY_PRINTK) /* Fixmap is only used by early printk */
> +#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
>          /* Non-boot CPUs don't need to rebuild the fixmap itself, just
>  	 * the mapping from boot_second to xen_fixmap */
>          cbnz  x22, 1f
> @@ -489,7 +489,7 @@ ENTRY(relocate_xen)
>  
>          ret
>  
> -#ifdef EARLY_PRINTK
> +#ifdef CONFIG_EARLY_PRINTK
>  /* Bring up the UART.
>   * x23: Early UART base address
>   * Clobbers x0-x1 */
> @@ -536,7 +536,7 @@ putn:
>  hex:    .ascii "0123456789abcdef"
>          .align 2
>  
> -#else  /* EARLY_PRINTK */
> +#else  /* CONFIG_EARLY_PRINTK */
>  
>  init_uart:
>  .global early_puts
> @@ -544,7 +544,7 @@ early_puts:
>  puts:
>  putn:   ret
>  
> -#endif /* EARLY_PRINTK */
> +#endif /* !CONFIG_EARLY_PRINTK */
>  
>  /*
>   * Local variables:
> diff --git a/xen/include/asm-arm/early_printk.h b/xen/include/asm-arm/early_printk.h
> index 31024b5..a58e3e7 100644
> --- a/xen/include/asm-arm/early_printk.h
> +++ b/xen/include/asm-arm/early_printk.h
> @@ -12,7 +12,7 @@
>  
>  #include <xen/config.h>
>  
> -#ifdef EARLY_PRINTK
> +#ifdef CONFIG_EARLY_PRINTK
>  
>  /* need to add the uart address offset in page to the fixmap address */
>  #define EARLY_UART_VIRTUAL_ADDRESS \
> @@ -22,7 +22,7 @@
>  
>  #ifndef __ASSEMBLY__
>  
> -#ifdef EARLY_PRINTK
> +#ifdef CONFIG_EARLY_PRINTK
>  
>  void early_puts(const char *s);
>  void early_printk(const char *fmt, ...)
> @@ -43,7 +43,7 @@ static inline void  __attribute__((noreturn))
>  __attribute__((format (printf, 1, 2))) early_panic(const char *fmt, ...)
>  {while(1);}
>  
> -#endif
> +#endif /* !CONFIG_EARLY_PRINTK */
>  
>  #endif	/* __ASSEMBLY__ */
>  

Re: [RFC 4/6] xen/console: Add support for early printk

[Ian Campbell]

On Sun, 2014-01-05 at 21:26 +0000, Julien Grall wrote:
> On ARM, a function (early_printk) was introduced to output message when the
> serial port is not initialized.
> 
> This solution is fragile because the developper needs to know when the serial
> port is initialized, to use either early_printk or printk. Moreover some
> functions (mainly in common code), only use printk. This will result to a loss
> of message sometimes.
> 
> Directly call early_printk in console code when the serial port is not yet
> initialized. For this purpose use serial_steal_fn.

This relies on nothing stealing the console over the period where the
console is initialised. Perhaps that is already not advisable/possible?

> 
> Cc: Keir Fraser <keir@xen.org>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/drivers/char/console.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 532c426..f83c92e 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -28,6 +28,9 @@
>  #include <asm/debugger.h>
>  #include <asm/div64.h>
>  #include <xen/hypercall.h> /* for do_console_io */
> +#ifdef CONFIG_EARLY_PRINTK
> +#include <asm/early_printk.h>
> +#endif
>  
>  /* console: comma-separated list of console outputs. */
>  static char __initdata opt_console[30] = OPT_CONSOLE_STR;
> @@ -245,7 +248,12 @@ long read_console_ring(struct xen_sysctl_readconsole *op)
>  static char serial_rx_ring[SERIAL_RX_SIZE];
>  static unsigned int serial_rx_cons, serial_rx_prod;
>  
> -static void (*serial_steal_fn)(const char *);
> +#ifndef CONFIG_EARLY_PRINTK
> +static inline void early_puts(const char *str)
> +{}

This duplicates bits of asm-arm/early_printk.h. I think if the feature
is going to be used from common code then the common bits of the asm
header should be moved to xen/early_printk.h. If any per-arch stuff
remains then xen/e_p.h can include asm/e_p.h.

> +#endif
> +
> +static void (*serial_steal_fn)(const char *) = early_puts;
>  
>  int console_steal(int handle, void (*fn)(const char *))
>  {
> @@ -652,7 +660,10 @@ void __init console_init_preirq(void)
>          else if ( !strncmp(p, "none", 4) )
>              continue;
>          else if ( (sh = serial_parse_handle(p)) >= 0 )
> +        {
>              sercon_handle = sh;
> +            serial_steal_fn = NULL;
> +        }
>          else
>          {
>              char *q = strchr(p, ',');

Re: [RFC 6/6] xen/arm: Replace early_printk call to printk call

[Ian Campbell]

On Sun, 2014-01-05 at 21:26 +0000, Julien Grall wrote:
> Now that the console supports earlyprintk, we can get a rid of
> early_printk call.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

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

Now all we need is a way to make it a runtime option :-)

Re: [RFC 6/6] xen/arm: Replace early_printk call to printk call

[Ian Campbell]

On Sun, 2014-01-05 at 21:26 +0000, Julien Grall wrote:
> 
> -/* Some device tree functions may be called both before and after the
> -   console is initialized. */
> -#define dt_printk(fmt, ...)                         \
> -    do                                              \
> -    {                                               \
> -        if ( system_state == SYS_STATE_early_boot ) \
> -            early_printk(fmt, ## __VA_ARGS__);      \
> -        else                                        \
> -            printk(fmt, ## __VA_ARGS__);            \
> -    } while (0)
> +#define dt_printk(fmt, ...) printk(fmt, ## __VA_ARGS__); 

Since dt_printk basically existed solely as a hack to workaround the
distinction between early_printk and printk I think you could just
switch everything to printk directly.

Ian.

Re: [RFC 6/6] xen/arm: Replace early_printk call to printk call

[Julien Grall]

Hi Ian,

On 02/19/2014 11:20 AM, Ian Campbell wrote:
> On Sun, 2014-01-05 at 21:26 +0000, Julien Grall wrote:
>> Now that the console supports earlyprintk, we can get a rid of
>> early_printk call.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks, due to some conflicts I have rebased this patch on top my patch
"xen/serial: Don't leak memory mapping if the serial initialization has
failed".

I will resend this series later.

> Now all we need is a way to make it a runtime option :-)

I let you write a device tree parser in assembly ;).

Cheers,

-- 
Julien Grall

Re: [RFC 6/6] xen/arm: Replace early_printk call to printk call

[Ian Campbell]

On Wed, 2014-02-19 at 17:56 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 02/19/2014 11:20 AM, Ian Campbell wrote:
> > On Sun, 2014-01-05 at 21:26 +0000, Julien Grall wrote:
> >> Now that the console supports earlyprintk, we can get a rid of
> >> early_printk call.
> >>
> >> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Thanks, due to some conflicts I have rebased this patch on top my patch
> "xen/serial: Don't leak memory mapping if the serial initialization has
> failed".
> 
> I will resend this series later.

Thanks.

> > Now all we need is a way to make it a runtime option :-)
> 
> I let you write a device tree parser in assembly ;).

;-)

I was actually thinking more along the lines of a .word at a defined
offset which you could hex edit to a specific value to activate a
particular flavour of early printk handling. That would be sufficient
e.g. for osstest to activate the appropriate stuff for the specific
platform.

TBH I'm not sure a dumb DTB parser which just runs through looking for a
single property on a particular node would be all that hard, but I'm not
all that keen to find out for sure ;-)

Ian.

Re: [RFC 6/6] xen/arm: Replace early_printk call to printk call

[Julien Grall]

On 20/02/14 09:04, Ian Campbell wrote:
> On Wed, 2014-02-19 at 17:56 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 02/19/2014 11:20 AM, Ian Campbell wrote:
>>> On Sun, 2014-01-05 at 21:26 +0000, Julien Grall wrote:
>>>> Now that the console supports earlyprintk, we can get a rid of
>>>> early_printk call.
>>>>
>>>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>>
>>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>>
>> Thanks, due to some conflicts I have rebased this patch on top my patch
>> "xen/serial: Don't leak memory mapping if the serial initialization has
>> failed".
>>
>> I will resend this series later.
>
> Thanks.
>
>>> Now all we need is a way to make it a runtime option :-)
>>
>> I let you write a device tree parser in assembly ;).
>
> ;-)
>
> I was actually thinking more along the lines of a .word at a defined
> offset which you could hex edit to a specific value to activate a
> particular flavour of early printk handling. That would be sufficient
> e.g. for osstest to activate the appropriate stuff for the specific
> platform.

I don't see useful use case to have a such early printk implementation 
in Xen. When the board is fully supported, failed at early stage (e.g 
before console is initialized) is very unlikely. At least if you don't 
play with memory.

I see osstest like a more generic test suite. If it fails at early 
stage, the developper should give a try himself (most of the time early 
printk is not useful to find the really bug), that would mean it need to 
recompile Xen.

Cheers,

-- 
Julien Grall

Re: [RFC 6/6] xen/arm: Replace early_printk call to printk call

[Ian Campbell]

On Thu, 2014-02-20 at 11:01 +0000, Julien Grall wrote:
> 
> On 20/02/14 09:04, Ian Campbell wrote:
> > I was actually thinking more along the lines of a .word at a defined
> > offset which you could hex edit to a specific value to activate a
> > particular flavour of early printk handling. That would be sufficient
> > e.g. for osstest to activate the appropriate stuff for the specific
> > platform.
> 
> I don't see useful use case to have a such early printk implementation 
> in Xen. When the board is fully supported, failed at early stage (e.g 
> before console is initialized) is very unlikely. At least if you don't 
> play with memory.

a) there are boards which aren't fully supported, getting some debug out
of a distro package might be useful

b) even for boards which are fully supported there may still be bugs
which only appear under particular circumstances.

Ian.

Re: [RFC 6/6] xen/arm: Replace early_printk call to printk call

[Julien Grall]

On 20/02/14 11:05, Ian Campbell wrote:
> On Thu, 2014-02-20 at 11:01 +0000, Julien Grall wrote:
>>
>> On 20/02/14 09:04, Ian Campbell wrote:
>>> I was actually thinking more along the lines of a .word at a defined
>>> offset which you could hex edit to a specific value to activate a
>>> particular flavour of early printk handling. That would be sufficient
>>> e.g. for osstest to activate the appropriate stuff for the specific
>>> platform.
>>
>> I don't see useful use case to have a such early printk implementation
>> in Xen. When the board is fully supported, failed at early stage (e.g
>> before console is initialized) is very unlikely. At least if you don't
>> play with memory.
>
> a) there are boards which aren't fully supported, getting some debug out
> of a distro package might be useful

Few months ago we have decided to allow early printk only when Xen is 
compiled with debug enabled. It seems a big mistake to ship distro with 
debug enabled :).

> b) even for boards which are fully supported there may still be bugs
> which only appear under particular circumstances.

I understand this use case. If I understand your previous mail the 
solution would me "Hex editing manually the Xen binary to set the early 
printk", right? If so, you are assuming that the distro (or anything 
else) is proving the zImage. Otherwise the developper has to:
    - unpack from the uImage
    - editing the zImage
    - recreate the uImage

That seems very complicated compare to recompiling Xen...

-- 
Julien Grall

Re: [RFC 6/6] xen/arm: Replace early_printk call to printk call

[Ian Campbell]

On Thu, 2014-02-20 at 11:14 +0000, Julien Grall wrote:
> 
> On 20/02/14 11:05, Ian Campbell wrote:
> > On Thu, 2014-02-20 at 11:01 +0000, Julien Grall wrote:
> >>
> >> On 20/02/14 09:04, Ian Campbell wrote:
> >>> I was actually thinking more along the lines of a .word at a defined
> >>> offset which you could hex edit to a specific value to activate a
> >>> particular flavour of early printk handling. That would be sufficient
> >>> e.g. for osstest to activate the appropriate stuff for the specific
> >>> platform.
> >>
> >> I don't see useful use case to have a such early printk implementation
> >> in Xen. When the board is fully supported, failed at early stage (e.g
> >> before console is initialized) is very unlikely. At least if you don't
> >> play with memory.
> >
> > a) there are boards which aren't fully supported, getting some debug out
> > of a distro package might be useful
> 
> Few months ago we have decided to allow early printk only when Xen is 
> compiled with debug enabled. It seems a big mistake to ship distro with 
> debug enabled :).

This was because earlyprintk only supports a static single configuration
at compile time. If that restriction was lifted then there would be no
reason to limit earlyprintk to debug builds.

> > b) even for boards which are fully supported there may still be bugs
> > which only appear under particular circumstances.
> 
> I understand this use case. If I understand your previous mail the 
> solution would me "Hex editing manually the Xen binary to set the early 
> printk", right? If so, you are assuming that the distro (or anything 
> else) is proving the zImage. Otherwise the developper has to:
>     - unpack from the uImage
>     - editing the zImage
>     - recreate the uImage

No distro would ship the actual uImage, it's too machine specific.

I would expect this to be used by running:
	xen-enable-early-printk /boot/xen midway

where xen-enable-early-printk is a simple tool we provide.

Then if a uIamge is then required then this would be generated by
whatever distro tooling would have generated it in the non-early-printk
case, by rerunning that tool.

> That seems very complicated compare to recompiling Xen...

Not really and this isn't so much for developers, but for users with
access to problematic systems to provide us with information.

Ian.

Re: [RFC 6/6] xen/arm: Replace early_printk call to printk call

[Julien Grall]

On 20/02/14 11:20, Ian Campbell wrote:
> On Thu, 2014-02-20 at 11:14 +0000, Julien Grall wrote:
>>
>> On 20/02/14 11:05, Ian Campbell wrote:
>>> On Thu, 2014-02-20 at 11:01 +0000, Julien Grall wrote:
>>>>
>>>> On 20/02/14 09:04, Ian Campbell wrote:
>>>>> I was actually thinking more along the lines of a .word at a defined
>>>>> offset which you could hex edit to a specific value to activate a
>>>>> particular flavour of early printk handling. That would be sufficient
>>>>> e.g. for osstest to activate the appropriate stuff for the specific
>>>>> platform.
>>>>
>>>> I don't see useful use case to have a such early printk implementation
>>>> in Xen. When the board is fully supported, failed at early stage (e.g
>>>> before console is initialized) is very unlikely. At least if you don't
>>>> play with memory.
>>>
>>> a) there are boards which aren't fully supported, getting some debug out
>>> of a distro package might be useful
>>
>> Few months ago we have decided to allow early printk only when Xen is
>> compiled with debug enabled. It seems a big mistake to ship distro with
>> debug enabled :).
>
> This was because earlyprintk only supports a static single configuration
> at compile time. If that restriction was lifted then there would be no
> reason to limit earlyprintk to debug builds.
>
>>> b) even for boards which are fully supported there may still be bugs
>>> which only appear under particular circumstances.
>>
>> I understand this use case. If I understand your previous mail the
>> solution would me "Hex editing manually the Xen binary to set the early
>> printk", right? If so, you are assuming that the distro (or anything
>> else) is proving the zImage. Otherwise the developper has to:
>>      - unpack from the uImage
>>      - editing the zImage
>>      - recreate the uImage
>
> No distro would ship the actual uImage, it's too machine specific.
>
> I would expect this to be used by running:
> 	xen-enable-early-printk /boot/xen midway
>
> where xen-enable-early-printk is a simple tool we provide.

And a similar one to disable, I guess.

>
> Then if a uIamge is then required then this would be generated by
> whatever distro tooling would have generated it in the non-early-printk
> case, by rerunning that tool.

Sounds good. Do you plan to work on it?

It would be nice to have this item on the ARM todo page.

-- 
Julien Grall

Re: [RFC 6/6] xen/arm: Replace early_printk call to printk call

[Ian Campbell]

On Thu, 2014-02-20 at 11:37 +0000, Julien Grall wrote:
> 
> On 20/02/14 11:20, Ian Campbell wrote:
> > On Thu, 2014-02-20 at 11:14 +0000, Julien Grall wrote:
> >>
> >> On 20/02/14 11:05, Ian Campbell wrote:
> >>> On Thu, 2014-02-20 at 11:01 +0000, Julien Grall wrote:
> >>>>
> >>>> On 20/02/14 09:04, Ian Campbell wrote:
> >>>>> I was actually thinking more along the lines of a .word at a defined
> >>>>> offset which you could hex edit to a specific value to activate a
> >>>>> particular flavour of early printk handling. That would be sufficient
> >>>>> e.g. for osstest to activate the appropriate stuff for the specific
> >>>>> platform.
> >>>>
> >>>> I don't see useful use case to have a such early printk implementation
> >>>> in Xen. When the board is fully supported, failed at early stage (e.g
> >>>> before console is initialized) is very unlikely. At least if you don't
> >>>> play with memory.
> >>>
> >>> a) there are boards which aren't fully supported, getting some debug out
> >>> of a distro package might be useful
> >>
> >> Few months ago we have decided to allow early printk only when Xen is
> >> compiled with debug enabled. It seems a big mistake to ship distro with
> >> debug enabled :).
> >
> > This was because earlyprintk only supports a static single configuration
> > at compile time. If that restriction was lifted then there would be no
> > reason to limit earlyprintk to debug builds.
> >
> >>> b) even for boards which are fully supported there may still be bugs
> >>> which only appear under particular circumstances.
> >>
> >> I understand this use case. If I understand your previous mail the
> >> solution would me "Hex editing manually the Xen binary to set the early
> >> printk", right? If so, you are assuming that the distro (or anything
> >> else) is proving the zImage. Otherwise the developper has to:
> >>      - unpack from the uImage
> >>      - editing the zImage
> >>      - recreate the uImage
> >
> > No distro would ship the actual uImage, it's too machine specific.
> >
> > I would expect this to be used by running:
> > 	xen-enable-early-printk /boot/xen midway
> >
> > where xen-enable-early-printk is a simple tool we provide.
> 
> And a similar one to disable, I guess.

Yes, by choosing "none" I suppose.

> > Then if a uIamge is then required then this would be generated by
> > whatever distro tooling would have generated it in the non-early-printk
> > case, by rerunning that tool.
> 
> Sounds good. Do you plan to work on it?

Not in the immediate future.

> It would be nice to have this item on the ARM todo page.

Done.

Re: [RFC 4/6] xen/console: Add support for early printk

[Julien Grall]

Hi Ian,

On 02/19/2014 11:19 AM, Ian Campbell wrote:
> On Sun, 2014-01-05 at 21:26 +0000, Julien Grall wrote:
>> On ARM, a function (early_printk) was introduced to output message when the
>> serial port is not initialized.
>>
>> This solution is fragile because the developper needs to know when the serial
>> port is initialized, to use either early_printk or printk. Moreover some
>> functions (mainly in common code), only use printk. This will result to a loss
>> of message sometimes.
>>
>> Directly call early_printk in console code when the serial port is not yet
>> initialized. For this purpose use serial_steal_fn.
> 
> This relies on nothing stealing the console over the period where the
> console is initialised. Perhaps that is already not advisable/possible?

serial_steal_fn is set in console_steal. This function checks if the
serial handle is valid.

This handle is only valid after console_init_preirq (which set
serial_steal_fn to NULL). So I think we are safe.

>>
>> Cc: Keir Fraser <keir@xen.org>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>>  xen/drivers/char/console.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
>> index 532c426..f83c92e 100644
>> --- a/xen/drivers/char/console.c
>> +++ b/xen/drivers/char/console.c
>> @@ -28,6 +28,9 @@
>>  #include <asm/debugger.h>
>>  #include <asm/div64.h>
>>  #include <xen/hypercall.h> /* for do_console_io */
>> +#ifdef CONFIG_EARLY_PRINTK
>> +#include <asm/early_printk.h>
>> +#endif
>>  
>>  /* console: comma-separated list of console outputs. */
>>  static char __initdata opt_console[30] = OPT_CONSOLE_STR;
>> @@ -245,7 +248,12 @@ long read_console_ring(struct xen_sysctl_readconsole *op)
>>  static char serial_rx_ring[SERIAL_RX_SIZE];
>>  static unsigned int serial_rx_cons, serial_rx_prod;
>>  
>> -static void (*serial_steal_fn)(const char *);
>> +#ifndef CONFIG_EARLY_PRINTK
>> +static inline void early_puts(const char *str)
>> +{}
> 
> This duplicates bits of asm-arm/early_printk.h. I think if the feature
> is going to be used from common code then the common bits of the asm
> header should be moved to xen/early_printk.h. If any per-arch stuff
> remains then xen/e_p.h can include asm/e_p.h.

I will do.

Cheers,

-- 
Julien Grall

Re: [RFC 6/6] xen/arm: Replace early_printk call to printk call

[Julien Grall]

Hi Ian,

On 19/02/14 20:33, Ian Campbell wrote:
> On Sun, 2014-01-05 at 21:26 +0000, Julien Grall wrote:
>>
>> -/* Some device tree functions may be called both before and after the
>> -   console is initialized. */
>> -#define dt_printk(fmt, ...)                         \
>> -    do                                              \
>> -    {                                               \
>> -        if ( system_state == SYS_STATE_early_boot ) \
>> -            early_printk(fmt, ## __VA_ARGS__);      \
>> -        else                                        \
>> -            printk(fmt, ## __VA_ARGS__);            \
>> -    } while (0)
>> +#define dt_printk(fmt, ...) printk(fmt, ## __VA_ARGS__);
>
> Since dt_printk basically existed solely as a hack to workaround the
> distinction between early_printk and printk I think you could just
> switch everything to printk directly.

I forgot to replace dt_printk by printk in v2. I will send another version.

Sincerely yours,


-- 
Julien Grall