debug: restructure kernel console handling

Remove the UART handling details from the top level I/O handling that
implements printf(). The architecture's I/O handling must define how
the debug channel is implemented and what special handling is
required. This allows separating UART and SBI debug output handling.

Signed-off-by: Axel Heider <axelheider@gmx.de>

include/drivers/uart.h

@@ -0,0 +1,27 @@
+/*
+ * Copyright 2021, Axel Heider <axelheider@gmx.de>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#pragma once
+
+#ifdef CONFIG_PRINTING
+
+void uart_drv_putchar(unsigned char c);
+
+static inline void uart_console_putchar(
+    unsigned char c)
+{
+    /* UART console requires printing a '\r' (CR) before any '\n' (LF) */
+    if (c == '\n') {
+        uart_drv_putchar('\r');
+    }
+    uart_drv_putchar(c);
+}
+
+#endif /* CONFIG_PRINTING */
+
+#ifdef CONFIG_DEBUG_BUILD
+unsigned char uart_drv_getchar(void);
+#endif

include/machine/io.h

@@ -7,33 +7,122 @@
 #pragma once
 
 #include <config.h>
-#include <util.h>
-#include <arch/types.h>
-
-#define FORMAT(archetype, string_index, first_to_check) \
-        __attribute__((format(archetype, string_index, first_to_check)))
-
-#if (defined CONFIG_DEBUG_BUILD) || (defined CONFIG_PRINTING)
-void putDebugChar(unsigned char c);
-#endif
 
 #ifdef CONFIG_DEBUG_BUILD
 /* io for dumping capdl */
-unsigned char getDebugChar(void);
+unsigned char kernel_getDebugChar(void);
 #endif
 
+
 #ifdef CONFIG_PRINTING
-/* printf will result in output */
-void putchar(char c);
-#define kernel_putchar(c) putchar(c)
-word_t kprintf(const char *format, ...) VISIBLE FORMAT(printf, 1, 2);
-word_t ksnprintf(char *str, word_t size, const char *format, ...);
-word_t puts(const char *s) VISIBLE;
-#define printf(args...) kprintf(args)
-#define snprintf(args...) ksnprintf(args)
-#else /* CONFIG_PRINTING */
-/* printf will NOT result in output */
-#define kernel_putchar(c) ((void)(0))
-#define printf(args...) ((void)(0))
-#define puts(s) ((void)(0))
-#endif /* CONFIG_PRINTING */
+
+#include <arch/types.h>
+#include <stdarg.h>
+
+/* the actual output function */
+void kernel_putDebugChar(unsigned char c);
+
+/* This is the actual implementation of the kernel printing API. It must never
+ * be called directly from anywhere except the function defined in this file.
+ */
+int impl_kvprintf(const char *format, va_list ap);
+int impl_ksnvprintf(char *str, word_t size, const char *format, va_list ap);
+
+/*
+ *------------------------------------------------------------------------------
+ * Kernel printing API
+ *------------------------------------------------------------------------------
+ */
+
+/* Writes a character to the kernel output channel. This is used to implement
+ * the syscall SysDebugPutChar.
+ */
+static inline void kernel_putchar(
+    char c)
+{
+    /* Write to target specific debug output channel. */
+    kernel_putDebugChar(c);
+}
+
+/* Writes a character to the active output channel. This is used by all code
+ * related to printf(). Contrary to the common signature of putchar(), there is
+ * no return value here.
+ */
+static inline void putchar(
+    char c)
+{
+    /* Write to target specific debug output channel. Purposely, we do not call
+     * kernel_putchar() here, as the kernel printf() channel is semantically
+     * different from the syscall SysDebugPutChar channel. The unification
+     * of both channels happens at the lower layer eventually
+     */
+    kernel_putDebugChar(c);
+}
+
+/* Writes the string and a trailing newline. There is no point to enforce a
+ * kernel_puts(), as this is just a wrapper for putchar() anyway.
+ */
+static inline int puts(
+    const char *str)
+{
+    if (str) {
+        while (*str) {
+            putchar(*str++);
+        }
+    }
+    putchar('\n');
+    /* Standards define that a non-negative number is returned on success. */
+    return 0;
+}
+
+/* There should only be a kprintf() that all kernel code must use for printing,
+ * but for convenience we provide a printf() here.
+ */
+static inline __attribute__((format(printf, 1, 2))) int printf(
+    const char *format,
+    ...)
+{
+    va_list args;
+    va_start(args, format);
+    int ret = impl_kvprintf(format, args); /* will call putchar() eventually */
+    va_end(args);
+    return ret;
+}
+
+/* Provide the standard snprintf() for write formatted data into a buffer, which
+ * can then be printed or stored.
+ */
+static inline __attribute__((format(printf, 3, 4))) int snprintf(
+    char *buf,
+    word_t size,
+    const char *format,
+    ...)
+{
+    va_list args;
+    va_start(args, format);
+    int ret = impl_ksnvprintf(buf, size, format, args);
+    va_end(args);
+    return ret;
+}
+
+#else /* not CONFIG_PRINTING */
+
+/* The verification runs on the output of the preprocessing stage of a release
+ * build configuration, CONFIG_PRINTING is not enabled there. We remove all
+ * calls to printf() completely from the code base, because neither printf() nor
+ * the usage of a variable argument list is supported by the verification
+ * toolchain. It would just reject the code if it encounters any unsupported
+ * things.
+ */
+#define printf(...)             ((void)(0))
+
+/* Seems there is no need to define out these functions, they are use by code
+ * that is active with CONFIG_PRINTING only.
+ *
+ *   #define kernel_putchar(...)     ((void)(0))
+ *   #define putchar(...)            ((void)(0))
+ *   #define puts(...)               ((void)(0))
+ *   #define snprintf(...)           ((void)(0))
+ */
+
+#endif /* [not] CONFIG_PRINTING */

src/arch/arm/config.cmake

@@ -231,6 +231,7 @@ add_sources(
         machine/errata.c
         machine/debug.c
         machine/hardware.c
+        machine/io.c
         object/interrupt.c
         object/tcb.c
         object/iospace.c

src/arch/arm/machine/io.c

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2021, Axel Heider <axelheider@gmx.de>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <config.h>
+#include <machine/io.h>
+#include <drivers/uart.h>
+
+#ifdef CONFIG_PRINTING
+void kernel_putDebugChar(unsigned char c)
+{
+    uart_console_putchar(c);
+}
+#endif /* CONFIG_PRINTING */
+
+#ifdef CONFIG_DEBUG_BUILD
+unsigned char kernel_getDebugChar(void)
+{
+    return uart_drv_getchar();
+}
+#endif /* CONFIG_DEBUG_BUILD */

src/arch/riscv/machine/io.c

@@ -5,12 +5,27 @@
  * SPDX-License-Identifier: GPL-2.0-only
  */
 
+#include <config.h>
 #include <machine/io.h>
 #include <arch/sbi.h>
 
-#if defined(CONFIG_PRINTING) || defined(CONFIG_DEBUG_BUILD)
-void putDebugChar(unsigned char c)
+#ifdef CONFIG_PRINTING
+void kernel_putDebugChar(unsigned char c)
 {
+    /* Don't use any UART driver, but write to the SBI console. It depends on
+     * the SBI implementation if printing a '\r' (CR) before any '\n' (LF) is
+     * required explicitly or if SBI takes care of this. Currently BBL requires
+     * it, while OpenSBI takes care of this internally. Since we dropped BBL
+     * support in favor of OpenSBI, we do not print a '\r' (CR) here.
+     */
     sbi_console_putchar(c);
 }
-#endif
+#endif /* CONFIG_PRINTING */
+
+#ifdef CONFIG_DEBUG_BUILD
+unsigned char kernel_getDebugChar(void)
+{
+    /* Don't use UART, but read from the SBI console. */
+    return sbi_console_getchar();
+}
+#endif /* CONFIG_DEBUG_BUILD */

src/drivers/serial/bcm2835-aux-uart.c

@@ -36,18 +36,18 @@
 
 #define UART_REG(x) ((volatile uint32_t *)(UART_PPTR + (x)))
 
-#if defined(CONFIG_DEBUG_BUILD) || defined(CONFIG_PRINTING)
-void putDebugChar(unsigned char c)
+#ifdef CONFIG_PRINTING
+void uart_drv_putchar(unsigned char c)
 {
     while (!(*UART_REG(MU_LSR) & MU_LSR_TXIDLE));
     *UART_REG(MU_IO) = (c & 0xff);
 }
-#endif
+#endif /* CONFIG_PRINTING */
 
 #ifdef CONFIG_DEBUG_BUILD
-unsigned char getDebugChar(void)
+unsigned char uart_drv_getchar(void)
 {
     while (!(*UART_REG(MU_LSR) & MU_LSR_DATAREADY));
     return *UART_REG(MU_IO);
 }
-#endif //CONFIG_DEBUG_BUILD
+#endif /* CONFIG_DEBUG_BUILD */

src/drivers/serial/exynos4210-uart.c

@@ -32,16 +32,16 @@
 
 #define UART_REG(X) ((volatile uint32_t *)(UART_PPTR + (X)))
 
-#if defined(CONFIG_DEBUG_BUILD) || defined(CONFIG_PRINTING)
-void putDebugChar(unsigned char c)
+#ifdef CONFIG_PRINTING
+void uart_drv_putchar(unsigned char c)
 {
     while ((*UART_REG(UTRSTAT) & TXBUF_EMPTY) == 0);
     *UART_REG(UTXH) = (c & 0xff);
 }
-#endif
+#endif /* CONFIG_PRINTING */
 
 #ifdef CONFIG_DEBUG_BUILD
-unsigned char getDebugChar(void)
+unsigned char uart_drv_getchar(void)
 {
     if ((*UART_REG(UTRSTAT) & RXBUF_READY)) {
         return (unsigned char) * UART_REG(URXH);

src/drivers/serial/imx.c

@@ -34,16 +34,16 @@
 
 #define UART_REG(x) ((volatile uint32_t *)(UART_PPTR + (x)))
 
-#if defined(CONFIG_DEBUG_BUILD) || defined(CONFIG_PRINTING)
-void putDebugChar(unsigned char c)
+#ifdef CONFIG_PRINTING
+void uart_drv_putchar(unsigned char c)
 {
     while (!(*UART_REG(USR2) & UART_SR2_TXFIFO_EMPTY));
     *UART_REG(UTXD) = c;
 }
-#endif
+#endif /* CONFIG_PRINTING */
 
 #ifdef CONFIG_DEBUG_BUILD
-unsigned char getDebugChar(void)
+unsigned char uart_drv_getchar(void)
 {
     while (!(*UART_REG(USR2) & UART_SR2_RXFIFO_RDR));
     return *UART_REG(URXD);

src/drivers/serial/meson-gx-uart.c

@@ -19,18 +19,18 @@
 
 #define UART_REG(x) ((volatile uint32_t *)(UART_PPTR + (x)))
 
-#if defined(CONFIG_DEBUG_BUILD) || defined(CONFIG_PRINTING)
-void putDebugChar(unsigned char c)
+#ifdef CONFIG_PRINTING
+void uart_drv_putchar(unsigned char c)
 {
     while ((*UART_REG(UART_STATUS) & UART_TX_FULL));
 
     /* Add character to the buffer. */
     *UART_REG(UART_WFIFO) = c;
 }
-#endif
+#endif /* CONFIG_PRINTING */
 
 #ifdef CONFIG_DEBUG_BUILD
-unsigned char getDebugChar(void)
+unsigned char uart_drv_getchar(void)
 {
     while ((*UART_REG(UART_STATUS) & UART_RX_EMPTY));
     return *UART_REG(UART_RFIFO);

src/drivers/serial/msm-uartdm.c

@@ -20,8 +20,8 @@
 
 #define UART_REG(X) ((volatile uint32_t *)(UART_PPTR + (X)))
 
-#if defined(CONFIG_DEBUG_BUILD) || defined(CONFIG_PRINTING)
-void putDebugChar(unsigned char c)
+#ifdef CONFIG_PRINTING
+void uart_drv_putchar(unsigned char c)
 {
     while ((*UART_REG(USR) & USR_TXEMP) == 0);
     /* Tell the peripheral how many characters to send */
@@ -29,10 +29,10 @@ void putDebugChar(unsigned char c)
     /* Write the character into the FIFO */
     *UART_REG(UTF) = c & 0xff;
 }
-#endif
+#endif /* CONFIG_PRINTING */
 
 #ifdef CONFIG_DEBUG_BUILD
-unsigned char getDebugChar(void)
+unsigned char uart_drv_getchar(void)
 {
     while ((*UART_REG(USR) & USR_RXRDY) == 0);
 

src/drivers/serial/pl011.c

@@ -18,20 +18,20 @@
 
 #define UART_REG(x) ((volatile uint32_t *)(UART_PPTR + (x)))
 
-#if defined(CONFIG_DEBUG_BUILD) || defined(CONFIG_PRINTING)
-void putDebugChar(unsigned char c)
+#ifdef CONFIG_PRINTING
+void uart_drv_putchar(unsigned char c)
 {
     while ((*UART_REG(UARTFR) & PL011_UARTFR_TXFF) != 0);
 
     *UART_REG(UARTDR) = c;
 }
-#endif
+#endif /* CONFIG_PRINTING */
 
 #ifdef CONFIG_DEBUG_BUILD
-unsigned char getDebugChar(void)
+unsigned char uart_drv_getchar(void)
 {
     while ((*UART_REG(UARTFR) & PL011_UARTFR_RXFE) != 0);
 
     return *UART_REG(UARTDR);
 }
-#endif //CONFIG_DEBUG_BUILD
+#endif  /* CONFIG_DEBUG_BUILD */

src/drivers/serial/tegra_omap3_dwapb.c

@@ -18,20 +18,20 @@
 
 #define UART_REG(x) ((volatile uint32_t *)(UART_PPTR + (x)))
 
-#if defined(CONFIG_DEBUG_BUILD) || defined(CONFIG_PRINTING)
-void putDebugChar(unsigned char c)
+#ifdef CONFIG_PRINTING
+void uart_drv_putchar(unsigned char c)
 {
     while ((*UART_REG(ULSR) & ULSR_THRE) == 0);
 
     *UART_REG(UTHR) = c;
 }
-#endif
+#endif /* CONFIG_PRINTING */
 
 #ifdef CONFIG_DEBUG_BUILD
-unsigned char getDebugChar(void)
+unsigned char uart_drv_getchar(void)
 {
     while ((*UART_REG(ULSR) & ULSR_RDR) == 0);
 
     return *UART_REG(UTHR);
 }
-#endif
+#endif /* CONFIG_DEBUG_BUILD */

src/drivers/serial/xuartps.c

@@ -32,16 +32,16 @@
 
 #define UART_REG(x) ((volatile uint32_t *)(UART_PPTR + (x)))
 
-#if defined(CONFIG_DEBUG_BUILD) || defined(CONFIG_PRINTING)
-void putDebugChar(unsigned char c)
+#ifdef CONFIG_PRINTING
+void uart_drv_putchar(unsigned char c)
 {
     while (!(*UART_REG(UART_CHANNEL_STS) & UART_CHANNEL_STS_TXEMPTY));
     *UART_REG(UART_TX_RX_FIFO) = c;
 }
-#endif
+#endif /* CONFIG_PRINTING */
 
 #ifdef CONFIG_DEBUG_BUILD
-unsigned char getDebugChar(void)
+unsigned char uart_drv_getchar(void)
 {
     while (!(*UART_REG(UART_CHANNEL_STS) & BIT(UART_CHANNEL_STS_TXEMPTY)));
     return *UART_REG(UART_TX_RX_FIFO);

src/machine/io.c

@@ -595,24 +595,7 @@ static int vprintf(out_wrap_t *out, const char *fmt, va_list ap)
  *------------------------------------------------------------------------------
  */
 
-void putchar(char c)
-{
-    if (c == '\n') {
-        putDebugChar('\r');
-    }
-    putDebugChar(c);
-}
-
-word_t puts(const char *s)
-{
-    for (; *s; s++) {
-        putchar(*s);
-    }
-    putchar('\n');
-    return 0;
-}
-
-word_t kprintf(const char *format, ...)
+int impl_kvprintf(const char *format, va_list ap)
 {
     out_wrap_t out_wrap =  {
         .write  = do_output_to_putchar,
@@ -621,14 +604,10 @@ word_t kprintf(const char *format, ...)
         .used   = 0
     };
 
-    va_list args;
-    va_start(args, format);
-    int ret = vprintf(&out_wrap, format, args);
-    va_end(args);
-    return ret;
+    return vprintf(&out_wrap, format, ap);
 }
 
-word_t ksnprintf(char *str, word_t size, const char *format, ...)
+int impl_ksnvprintf(char *str, word_t size, const char *format, va_list ap)
 {
     if (!str) {
         size = 0;
@@ -641,10 +620,7 @@ word_t ksnprintf(char *str, word_t size, const char *format, ...)
         .used   = 0
     };
 
-    va_list args;
-    va_start(args, format);
-    int ret = vprintf(&out_wrap, format, args);
-    va_end(args);
+    int ret = vprintf(&out_wrap, format, ap);
 
     /* We return the number of characters written into the buffer, excluding the
      * terminating null char. However, we do never write more than 'size' bytes,

src/plat/pc99/machine/io.c

@@ -9,6 +9,7 @@
 #include <arch/model/statedata.h>
 #include <machine/io.h>
 #include <plat/machine/io.h>
+#include <drivers/uart.h>
 
 #if defined(CONFIG_DEBUG_BUILD) || defined(CONFIG_PRINTING)
 void serial_init(uint16_t port)
@@ -26,17 +27,30 @@ void serial_init(uint16_t port)
     in8(port + 5); /* clear line status port */
     in8(port + 6); /* clear modem status port */
 }
+#endif /* defined(CONFIG_DEBUG_BUILD) || defined(CONFIG_PRINTING) */
 
-void putDebugChar(unsigned char a)
+#ifdef CONFIG_PRINTING
+
+/* there is no UART driver at drivers/serial/... for the pc99 platform, but we
+ * implement parts of the API here, so we can reuse the generic code.
+ */
+void uart_drv_putchar(
+    unsigned char c)
 {
     while (x86KSdebugPort && (in8(x86KSdebugPort + 5) & 0x20) == 0);
-    out8(x86KSdebugPort, a);
+    out8(x86KSdebugPort, c);
 }
 
-#endif /* CONFIG_PRINTING || CONFIG_DEBUG_BUILD */
+void kernel_putDebugChar(unsigned char c)
+{
+    /* this will take care of CR/LF handling and call uart_drv_putchar() */
+    uart_console_putchar(c);
+}
+
+#endif /* CONFIG_PRINTING */
 
 #ifdef CONFIG_DEBUG_BUILD
-unsigned char getDebugChar(void)
+unsigned char kernel_getDebugChar(void)
 {
     while ((in8(x86KSdebugPort + 5) & 1) == 0);
     return in8(x86KSdebugPort);