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

Oleksandr Andrushchenko Oleksandr_Andrushchenko at epam.com
Mon Oct 26 07:54:29 CET 2020 

On 10/26/20 8:50 AM, takahiro.akashi at 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.akashi at 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.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 at 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...

>
>>> 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?
> 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.
>
> -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]

More information about the U-Boot mailing list