On Wednesday, May 17, 2017 8:52 PM Sascha Hauer wrote: > 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
I agree, I will add a check for a cmdline parameter passed from bootloader. I will create a patch v3. Kind Regards, Bogdan