On Mon, Oct 26, 2020 at 06:54:29AM +0000, Oleksandr Andrushchenko wrote:
>
> On 10/26/20 8:50 AM, takahiro.aka...@linaro.org wrote:
> > On Mon, Oct 26, 2020 at 06:18:08AM +0000, Oleksandr Andrushchenko wrote:
> >> Hi,
> >>
> >> On 10/26/20 7:58 AM, takahiro.aka...@linaro.org wrote:
> >>> On Fri, Oct 23, 2020 at 08:50:56AM +0000, Anastasiia Lukianenko wrote:
> >>>> Hello,
> >>>>
> >>>> On Thu, 2020-10-22 at 18:53 +0900, takahiro.aka...@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.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?
> >>>> No, I just tried different options and accidentally discovered this
> >>>> error.
> >>> No?
> >>> My code is protected with "if CONFIG_IS_ENABLED(ARM)", and so
> >>> you have no chance of building "else" clause unless you use x86.
> >> The question here is that if x86 is selected it won't compile. Another
> >>
> >> question if we tested that with x86: no, we didn't. The reason we tried x86
> >>
> >> part was that HYPERVISOR_console_io is a generic definition for all the
> >> platforms,
> >>
> >> so it was natural to try that as a way to debug things.
> > Anastasiia said, "No, I just tried different options."
> > Instead of different options, you tried modified code.
>
> That was to debug why the original code didn't work, I see nothing wrong
>
> in that she tried helping you to figure out why...
I really appreciate your assistance here.
But without knowing the detailed environment on your side, it may sometimes
be difficult to find out the root cause.
> >
> >>> Anyway,
> >>>
> >>>>> 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).)
> >>>> Unfortunately, nothing happened (there are no logs mentioned earlier).
> >>> 1. Have you ever executed HYPERVISOR_console_io on your platform before?
> >> Yes, we did that. You may have noticed that in [1] which I referred earlier
> >> and the problems we faced with that.
> > Okay. Since I started to play with Xen just a few weeks ago,
> > I actually don't know the past discussions at all.
> >
> > So the issue you have mentioned has been fixed, hasn't it?
Please confirm this.
> > Even if so, you must have submitted your patch in June or later
> > this year.
> >
> > Anastasiia said that she had used xen 4.13(+?) to test my code.
> > So obviously, there will be no chance that you patch be merged
> > in her test environment.
> >
> >>> 2. If possible, please try to my code on qemu, as my patch intended, so
> >>> that
> >>> we can determine if your issue is generic or specific on your
> >>> environment?
> >> The code runs on two different platforms with the same result (non-QEMU
> >> though).
> > Please check on qemu with the latest Xen so that, as I said, we can
> > determine if the issue is platform or environment (including a difference
> > of version used) specific or not.
> > I believe that it is quite easy for you to run U-Boot on qemu.
Please try this first. I believe that it is the first step to take.
Since I don't have any real hardwrare at this moment,
it will be difficult for me to dig into the issue
unless it can be reproduced on qemu.
Thanks,
-Takahiro Akashi
> >
> > -Takahiro Akashi
> >
> >> Thank you,
> >>
> >> Oleksandr
> >>
> >>> Thanks,
> >>> -Takahiro Akashi
> >>>
> >>>
> >>>> Regards,
> >>>> Anastasiia
> >>>>> Thanks,
> >>>>> -Takahiro Akashi
> >>>>>
> >>>>>
> >>>>>>> +#endif
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +DEBUG_UART_FUNCS
> >>>>>>> +
> >>>>>>> +#endif
> >>>>>> Regards,
> >>>>>> Anastasiia
> >> [1]
> >> https://urldefense.com/v3/__https://lists.xenproject.org/archives/html/xen-devel/2020-06/msg00737.html__;!!GF_29dbcQIUBPA!lbY6WN0YDxFiqUvVTZI8ZkwzoiY08y_oHciOZ7lrUxxpdo-aX37PHDwcSwc3Mb_7uRivJJpP0A$
> >> [lists[.]xenproject[.]org]
