Re: [PATCH v2] Added "Preserve Boot Time Support"

[Sascha Hauer]

On Wed, May 17, 2017 at 02:42:24PM +0000, Mirea, Bogdan-Stefan wrote:
> Hello Sascha,
> 
> On Wednesday, May 17, 2017 2:30 PM Sascha Hauer wrote:
> > As John already said, there's the read_boot_clock64() interface which
> > should be used here.
> > By using the read_boot_clock64() interface you can make sure that you
> > only provide the functionality when it's actually supposed to work.
> The read_boot_clock64() function is called from timekeeping_init().
> The arm arch timer init callback arch_timer_of_init() function is called
> from time_init() function.
> We will have an uninitialized arm arch timer (this is our only timer
> source) at the read_boot_clock64() function call since both
> timekeeping_init() and time_init() functions are called from
> start_kernel() in the following order:
>     asmlinkage __visible void __init start_kernel(void)
>     {
>         /* ... */
>         timekeeping_init();
>         time_init();
>         /* ... */
>     }
> So in our case, we cannot use a read_boot_clock64() function to read cyc
> value.
> 
> 
> > In your patch you provide a generic option (BOOT_TIME_PRESERVE) which
> > only works as expected in some special cases. You assume that the
> > bootloader has started the same timer with the same frequency as Linux
> > does.
> Yes, I assume that the bootloader and Linux use and configure the same
> timer at the same frequency. This is a must since we want to measure the
> exact boottime. The reason the code is guarded with BOOT_TIME_PRESERVE
> is because we want to avoid situations like the one you described.

IMO Kernel options should be safe to enable everytime. They shouldn't
have side effects on boards that lack the necessary hardware or
bootloader support. I think we should at least have a commandline option
or device tree property in which the bootloader can signal that it has
initialized the timer suitable for calculating the boot time.

> 
> 
> > Also you assume that the timer startup code doesn't reset the
> > timer. When these assumptions are not true then you just get bogus
> > random timing information.
> If the timer initialization code will reset the timer, the Linux system
> will work exactly as before, showing boottime 0 in kmsg and 0 in
> uptime, so no bugs/crashes will occur.

It may also be that the timer initialization code switches to another
frequency, but doesn't reset the timer. All kinds of funny things can
happen when the timer initialization code hasn't been audited to support
this usecase. So really we shouldn't provide the user an option without
giving any hint if this can even work on his hardware

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |