diff mbox

[Xen-devel,v3,4/5] xen/console: Add support for early printk

Message ID 1394634891-27906-5-git-send-email-julien.grall@linaro.org
State Superseded, archived
Headers show

Commit Message

Julien Grall March 12, 2014, 2:34 p.m. UTC
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>

---
    Changes in v2:
        - Create xen/early_printk.h
---
 xen/arch/arm/early_printk.c        |    1 +
 xen/drivers/char/console.c         |    6 +++++-
 xen/include/asm-arm/early_printk.h |    3 ---
 xen/include/xen/early_printk.h     |   21 +++++++++++++++++++++
 4 files changed, 27 insertions(+), 4 deletions(-)
 create mode 100644 xen/include/xen/early_printk.h

Comments

Jan Beulich March 12, 2014, 3:20 p.m. UTC | #1
>>> On 12.03.14 at 15:34, Julien Grall <julien.grall@linaro.org> wrote:
> --- /dev/null
> +++ b/xen/include/xen/early_printk.h
> @@ -0,0 +1,21 @@
> +/*
> + * printk() for use before the console is initialized
> + */
> +#ifndef __XEN_EARLY_PRINTK_H__
> +#define __XEN_EARLY_PRINTK_H__
> +
> +#ifdef CONFIG_EARLY_PRINTK
> +void early_puts(const char *s);
> +#else
> +static inline void early_puts(const char *s) {};

Considering that the only (source) use of the function is to take its
address, using an inline function for that purpose is bogus. Either
#define it to NULL (thus leaving the serial_steal_fn initializer
unchanged for x86) or have it be a static __init function in
xen/drivers/char/console.c, conditional upon !CONFIG_EARLY_PRINTK.

Jan
Julien Grall March 12, 2014, 3:32 p.m. UTC | #2
Hi Jan,

On 03/12/2014 03:20 PM, Jan Beulich wrote:
>>>> On 12.03.14 at 15:34, Julien Grall <julien.grall@linaro.org> wrote:
>> --- /dev/null
>> +++ b/xen/include/xen/early_printk.h
>> @@ -0,0 +1,21 @@
>> +/*
>> + * printk() for use before the console is initialized
>> + */
>> +#ifndef __XEN_EARLY_PRINTK_H__
>> +#define __XEN_EARLY_PRINTK_H__
>> +
>> +#ifdef CONFIG_EARLY_PRINTK
>> +void early_puts(const char *s);
>> +#else
>> +static inline void early_puts(const char *s) {};
> 
> Considering that the only (source) use of the function is to take its
> address, using an inline function for that purpose is bogus. Either
> #define it to NULL (thus leaving the serial_steal_fn initializer
> unchanged for x86) or have it be a static __init function in
> xen/drivers/char/console.c, conditional upon !CONFIG_EARLY_PRINTK.

Right, I will fix it for the next version of this patch.

Regards,
diff mbox

Patch

diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c
index 6b90998..8aef152 100644
--- a/xen/arch/arm/early_printk.c
+++ b/xen/arch/arm/early_printk.c
@@ -13,6 +13,7 @@ 
 #include <xen/lib.h>
 #include <xen/stdarg.h>
 #include <xen/string.h>
+#include <xen/early_printk.h>
 #include <asm/early_printk.h>
 
 void early_putch(char c);
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 7d4383c..cf7c9bb 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -28,6 +28,7 @@ 
 #include <asm/debugger.h>
 #include <asm/div64.h>
 #include <xen/hypercall.h> /* for do_console_io */
+#include <xen/early_printk.h>
 
 /* console: comma-separated list of console outputs. */
 static char __initdata opt_console[30] = OPT_CONSOLE_STR;
@@ -245,7 +246,7 @@  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 *);
+static void (*serial_steal_fn)(const char *) = early_puts;
 
 int console_steal(int handle, void (*fn)(const char *))
 {
@@ -652,7 +653,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, ',');
diff --git a/xen/include/asm-arm/early_printk.h b/xen/include/asm-arm/early_printk.h
index 5ef2ec4..f5b801e 100644
--- a/xen/include/asm-arm/early_printk.h
+++ b/xen/include/asm-arm/early_printk.h
@@ -24,7 +24,6 @@ 
 
 #ifdef CONFIG_EARLY_PRINTK
 
-void early_puts(const char *s);
 void early_printk(const char *fmt, ...)
     __attribute__((format (printf, 1, 2)));
 void noreturn early_panic(const char *fmt, ...)
@@ -32,8 +31,6 @@  void noreturn early_panic(const char *fmt, ...)
 
 #else
 
-static inline void early_puts(const char *) {}
-
 static inline  __attribute__((format (printf, 1, 2))) void
 early_printk(const char *fmt, ...)
 {}
diff --git a/xen/include/xen/early_printk.h b/xen/include/xen/early_printk.h
new file mode 100644
index 0000000..c213d18
--- /dev/null
+++ b/xen/include/xen/early_printk.h
@@ -0,0 +1,21 @@ 
+/*
+ * printk() for use before the console is initialized
+ */
+#ifndef __XEN_EARLY_PRINTK_H__
+#define __XEN_EARLY_PRINTK_H__
+
+#ifdef CONFIG_EARLY_PRINTK
+void early_puts(const char *s);
+#else
+static inline void early_puts(const char *s) {};
+#endif
+
+#endif
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */