[PATCH 4/4] serial: serial_xen: add DEBUG_UART support

takahiro.akashi at linaro.org
2020-10-26T06:02:37Z

On Fri, Oct 23, 2020 at 08:53:52AM +0000, Anastasiia Lukianenko wrote:
> Hi,
>
> On Thu, 2020-10-22 at 18:53 +0900, takahiro.akashi at linaro.org wrote:
> > On Thu, Oct 22, 2020 at 09:19:41AM +0000, Anastasiia Lukianenko
> > wrote:
> > > Hi,
> > >
> > > On Thu, 2020-10-15 at 13:25 +0900, AKASHI Takahiro wrote:
> > > > By using a hypervisor call, we can implement DEBUG_UART on xen.
> > > > This will allow us to see messages even earlier than
> > > > serial_init().
> > > >
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > ---
> > > > drivers/serial/Kconfig | 14 +++++++++++---
> > > > drivers/serial/serial_xen.c | 20 ++++++++++++++++++++
> > > > 2 files changed, 31 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> > > > index e344677f91f6..536cf0641773 100644
> > > > --- a/drivers/serial/Kconfig
> > > > +++ b/drivers/serial/Kconfig
> > > > @@ -401,11 +401,19 @@ config DEBUG_UART_MTK
> > > > driver will be available until the real driver model serial
> > > > is
> > > > running.
> > > >
> > > > +config DEBUG_UART_XEN
> > > > + bool "XEN Hypervisor Console"
> > > > + depends on XEN_SERIAL
> > > > + help
> > > > + Select this to enable a debug UART using the serial_xen
> > > > driver. You
> > > > + will not have to provide any parameters to make this work.
> > > > The driver
> > > > + will be available until the real driver-model serial
> > > > is
> > > > running.
> > > > +
> > > > endchoice
> > > >
> > > > config DEBUG_UART_BASE
> > > > hex "Base address of UART"
> > > > - depends on DEBUG_UART
> > > > + depends on DEBUG_UART && !DEBUG_UART_XEN
> > > > default 0 if DEBUG_UART_SANDBOX
> > > > help
> > > > This is the base address of your UART for memory-mapped
> > > > UARTs.
> > > > @@ -415,7 +423,7 @@ config DEBUG_UART_BASE
> > > >
> > > > config DEBUG_UART_CLOCK
> > > > int "UART input clock"
> > > > - depends on DEBUG_UART
> > > > + depends on DEBUG_UART && !DEBUG_UART_XEN
> > > > default 0 if DEBUG_UART_SANDBOX
> > > > help
> > > > The UART input clock determines the speed of the internal
> > > > UART
> > > > @@ -427,7 +435,7 @@ config DEBUG_UART_CLOCK
> > > >
> > > > config DEBUG_UART_SHIFT
> > > > int "UART register shift"
> > > > - depends on DEBUG_UART
> > > > + depends on DEBUG_UART && !DEBUG_UART_XEN
> > > > default 0 if DEBUG_UART
> > > > help
> > > > Some UARTs (notably ns16550) support different register
> > > > layouts
> > > > diff --git a/drivers/serial/serial_xen.c
> > > > b/drivers/serial/serial_xen.c
> > > > index ed191829f059..34c90ece40fc 100644
> > > > --- a/drivers/serial/serial_xen.c
> > > > +++ b/drivers/serial/serial_xen.c
> > > > @@ -5,6 +5,7 @@
> > > > */
> > > > #include <common.h>
> > > > #include <cpu_func.h>
> > > > +#include <debug_uart.h>
> > > > #include <dm.h>
> > > > #include <serial.h>
> > > > #include <watchdog.h>
> > > > @@ -15,11 +16,14 @@
> > > > #include <xen/events.h>
> > > >
> > > > #include <xen/interface/sched.h>
> > > > +#include <xen/interface/xen.h>
> > > > #include <xen/interface/hvm/hvm_op.h>
> > > > #include <xen/interface/hvm/params.h>
> > > > #include <xen/interface/io/console.h>
> > > > #include <xen/interface/io/ring.h>
> > > >
> > > > +#include <asm/xen/hypercall.h>
> > > > +
> > > > DECLARE_GLOBAL_DATA_PTR;
> > > >
> > > > u32 console_evtchn;
> > > > @@ -178,3 +182,19 @@ U_BOOT_DRIVER(serial_xen) = {
> > > > .flags = DM_FLAG_PRE_RELOC,
> > > > };
> > > >
> > > > +#if defined(CONFIG_DEBUG_UART_XEN)
> > > > +static inline void _debug_uart_init(void) {}
> > > > +
> > > > +static inline void _debug_uart_putc(int c)
> > > > +{
> > > > +#if CONFIG_IS_ENABLED(ARM)
> > > > + xen_debug_putc(c);
> > > > +#else
> > > > + /* the type cast should work on LE only */
> > > > + HYPERVISOR_console_io(CONSOLEIO_write, 1, (char *)&ch);
> > >
> > > An error occurs during compilation:
> > > drivers/serial/serial_xen.c: error: ?ch? undeclared (first use in
> > > this
> > > function); did you mean ?c??
> > > HYPERVISOR_console_io(CONSOLEIO_write, 1, (char *)&ch);
> >
> > Ah, yes. You're now using x86, right?
> >
> > So what happens if you have made the fix above?
> > Does it work in your environment?
> > (I have confirmed that HYPERVISOR_console_io version works on
> > arm(64).)
> >
>
> Sorry, I forgot to write in the last letter the question:
> Why can't we simply use HYPERVISOR_console_io(CONSOLEIO_write, 1, (char
> *)&ch); for both ARM and Xen?

Simply because the executed code on Xen side is quite simple.
I assume that simpler it is, more chance of output.

-Takahiro Akashi

> Regards,
> Anastasiia
>
> > Thanks,
> > -Takahiro Akashi
> >
> >
> > > > +#endif
> > > > +}
> > > > +
> > > > +DEBUG_UART_FUNCS
> > > > +
> > > > +#endif
> > >
> > > Regards,
> > > Anastasiia