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

[takahiro.aka...@linaro.org]

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.aka...@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).)

Thanks,
-Takahiro Akashi


> > +#endif
> > +}
> > +
> > +DEBUG_UART_FUNCS
> > +
> > +#endif
> 
> Regards,
> Anastasiia