RE: [PATCH v3] Added "Preserve Boot Time Support"

[Mirea, Bogdan-Stefan]

Hello Thomas,

On Friday, May 26, 2017 1:05 PM, Thomas Gleixner wrote: 
> On Wed, 24 May 2017, Mirea, Bogdan-Stefan wrote:
> > I am thinking about using timekeeping_inject_sleeptime64(delta) hook to
> 
> That's not a hook. It's a regular function. Please use proper technical
> terms.
> 
> > add a delta time at boot to the CLOCK_BOOTTIME, but the problem that
> > arise here is that this hook is intended to be used on rtc_resume()
> > code (to add the time spent in suspend to CLOCK_BOOTTIME).
> 
> It's not the intention. It _IS_ used for injecting the suspended time.
> 
> > >From my point of view this will do the trick even if the
> > CONFIG_BOOT_TIME_PRESERVE will then depend on PM_SLEEP &&
> RTC_HCTOSYS
> > (needed by timekeeping_inject_sleeptime64 to work).
> 
> First of all there is no point in having this extra CONFIG switch. This can
> be made unconditionally and depend on a kernel command line argument.
> 
> What has this to do with PM_SLEEP and RTC_HCTOSYS? Just because that
> function is currently only available when these config switches are enabled
> it does not mean that it cannot be made available unconditionally.
> 
> What you are trying to do is to cram that new functionality into the
> existing code without actually spending time on thinking about a proper
> design.
> 
> Let's talk about the objective.
> 
>  1) Insert time from system start (power on, hardware reset) to kernel start
> (timekeeping init) into CLOCK_BOOTTIME, if the system can read out this
> value.
> 
>  2) Coordinate sched_clock and CLOCK_BOOTTIME
> 
> #1 timekeeping_inject_sleeptime64() provides the required functionality
>already. We can rename that function to reflect that it's used for both
>(injecting suspend time and injecting system start time).
> 
>The more interesting question is how to provide a generic mechanism to
>retrieve that value. There are two possibilities:
> 
>A) Value is stored by the early boot code and retrieved from there
> 
>B) Value is made available via a clocksource after initializing and
>   registering it.
> 
>#A can be solved by having a weak function
> 
>   u64 __weak read_system_starttime(void)
>   {
>   return 0;
>   }
> 
>  and let architectures implementing it when required. Though I doubt
>  that we need that right away.
> 
>#B can be made generic in a very simple way
> 
>  Add a flag CLOCK_SOURCE_STARTTIME to the clocksource flags and let the
>  core code use that flag exactly once to inject the system start time
>  into CLOCK_BOOTTIME.
> 
> #2 depends on #1
> 
>When the timekeeping core injects the system start time it can tell the
>sched_clock core to use that offset as well.
> 
>But, that does not solve the problem that sched_clock and CLOCK_BOOTTIME
>will drift apart over time which makes the whole thing moot.
> 
>There was a patch set floating around, which made the clock used for
>printk selectable so it can actually use CLOCK_MONOTONIC or
>CLOCK_BOOTTIME via ktime_get_[mono|boot]_fast_ns(), but that never got
>merged.
> 
>So instead of trying to provide a half correct solution which does not
>allow you to coordinate dmesg timestamps, uptime, tracer timestamps
>etc. reliably due to drift, I rather have that dmesg selectable
>timestamping patch merged and provide coordinated timestamps that way.
>(Cc'ed Prarit, who was working on that)
> 
> Thanks,
> 
>   tglx
> 
> 
Thank you for your valuable and complete answer, I will consider everything you
said and update accordingly. I'll also notify you when this is ready.

Kind Regards,
Bogdan

RE: [PATCH v3] Added "Preserve Boot Time Support"

[Mirea, Bogdan-Stefan]

Hello Thomas,

On Friday, May 26, 2017 1:05 PM, Thomas Gleixner wrote: 
> On Wed, 24 May 2017, Mirea, Bogdan-Stefan wrote:
> > I am thinking about using timekeeping_inject_sleeptime64(delta) hook to
> 
> That's not a hook. It's a regular function. Please use proper technical
> terms.
> 
> > add a delta time at boot to the CLOCK_BOOTTIME, but the problem that
> > arise here is that this hook is intended to be used on rtc_resume()
> > code (to add the time spent in suspend to CLOCK_BOOTTIME).
> 
> It's not the intention. It _IS_ used for injecting the suspended time.
> 
> > >From my point of view this will do the trick even if the
> > CONFIG_BOOT_TIME_PRESERVE will then depend on PM_SLEEP &&
> RTC_HCTOSYS
> > (needed by timekeeping_inject_sleeptime64 to work).
> 
> First of all there is no point in having this extra CONFIG switch. This can
> be made unconditionally and depend on a kernel command line argument.
> 
> What has this to do with PM_SLEEP and RTC_HCTOSYS? Just because that
> function is currently only available when these config switches are enabled
> it does not mean that it cannot be made available unconditionally.
> 
> What you are trying to do is to cram that new functionality into the
> existing code without actually spending time on thinking about a proper
> design.
> 
> Let's talk about the objective.
> 
>  1) Insert time from system start (power on, hardware reset) to kernel start
> (timekeeping init) into CLOCK_BOOTTIME, if the system can read out this
> value.
> 
>  2) Coordinate sched_clock and CLOCK_BOOTTIME
> 
> #1 timekeeping_inject_sleeptime64() provides the required functionality
>already. We can rename that function to reflect that it's used for both
>(injecting suspend time and injecting system start time).
> 
>The more interesting question is how to provide a generic mechanism to
>retrieve that value. There are two possibilities:
> 
>A) Value is stored by the early boot code and retrieved from there
> 
>B) Value is made available via a clocksource after initializing and
>   registering it.
> 
>#A can be solved by having a weak function
> 
>   u64 __weak read_system_starttime(void)
>   {
>   return 0;
>   }
> 
>  and let architectures implementing it when required. Though I doubt
>  that we need that right away.
> 
>#B can be made generic in a very simple way
> 
>  Add a flag CLOCK_SOURCE_STARTTIME to the clocksource flags and let the
>  core code use that flag exactly once to inject the system start time
>  into CLOCK_BOOTTIME.
> 
> #2 depends on #1
> 
>When the timekeeping core injects the system start time it can tell the
>sched_clock core to use that offset as well.
> 
>But, that does not solve the problem that sched_clock and CLOCK_BOOTTIME
>will drift apart over time which makes the whole thing moot.
> 
>There was a patch set floating around, which made the clock used for
>printk selectable so it can actually use CLOCK_MONOTONIC or
>CLOCK_BOOTTIME via ktime_get_[mono|boot]_fast_ns(), but that never got
>merged.
> 
>So instead of trying to provide a half correct solution which does not
>allow you to coordinate dmesg timestamps, uptime, tracer timestamps
>etc. reliably due to drift, I rather have that dmesg selectable
>timestamping patch merged and provide coordinated timestamps that way.
>(Cc'ed Prarit, who was working on that)
> 
> Thanks,
> 
>   tglx
> 
> 
Thank you for your valuable and complete answer, I will consider everything you
said and update accordingly. I'll also notify you when this is ready.

Kind Regards,
Bogdan

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

[Mark Rutland]

On Fri, May 19, 2017 at 01:16:23PM +0300, Bogdan Mirea wrote:
> This option enables Boot Time Preservation between Bootloader and
> Linux Kernel. It is based on the idea that the Bootloader (or any
> other early firmware) will start the HW Timer and Linux Kernel will
> count the time starting with the cycles elapsed since timer start.
> 
> The sched_clock part is preserving boottime for kmsg which should be in
> sync with system uptime. The system uptime part is driver specific and I
> updated the arm_arch_timer with an arch_timer_setsystime() function
> which will call do_settimeofday64() with the values read from arch timer
> counter.
> 
> This way both kmsg and uptime will be in sync, otherwise inconsistencies
> will appear between the two.
> 
> The "preserve_boot_time" parameter should be appended to kernel cmdline
> from bootloader for kernel acknowledgment that the timer is running in
> bootloader.
> 
> Signed-off-by: Bogdan Mirea 
> ---
>  drivers/clocksource/arm_arch_timer.c | 33 +

In future, *please* use get_maintainer.pl, and ensure all relevant
maintainers are on Cc. e.g.

[mark@leverpostej:~/src/linux]% ./scripts/get_maintainer.pl -f 
drivers/clocksource/arm_arch_timer.c
Mark Rutland  (maintainer:ARM ARCHITECTED TIMER DRIVER)
Marc Zyngier  (maintainer:ARM ARCHITECTED TIMER DRIVER)
Daniel Lezcano  (supporter:CLOCKSOURCE, CLOCKEVENT 
DRIVERS)
Thomas Gleixner  (supporter:CLOCKSOURCE, CLOCKEVENT DRIVERS)
linux-arm-ker...@lists.infradead.org (moderated list:ARM ARCHITECTED TIMER 
DRIVER)
linux-kernel@vger.kernel.org (open list:CLOCKSOURCE, CLOCKEVENT DRIVERS)

As I was not Cc'd, I only spotted this by chance.

[...]

> diff --git a/drivers/clocksource/arm_arch_timer.c 
> b/drivers/clocksource/arm_arch_timer.c
> index 5152b38..95699cd 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -475,6 +475,35 @@ struct timecounter *arch_timer_get_timecounter(void)
>   return 
>  }
>  
> +#ifdef CONFIG_BOOT_TIME_PRESERVE
> +/*
> + * Set the real system time(including the time spent in bootloader)
> + * based on the timer counter.
> + */
> +
> +#ifndef BOOT_TIME_PRESERVE_CMDLINE
> + #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
> +#endif
> +void arch_timer_setsystime(void)
> +{
> + static struct timespec64 boot_ts;
> + static cycles_t cycles;
> + unsigned long long nsecs;
> +
> + if (!strstr(boot_command_line, BOOT_TIME_PRESERVE_CMDLINE))
> + return;
> +
> + cycles = arch_timer_read_counter() ? arch_timer_read_counter() : 0;
> +
> + nsecs = clocksource_cyc2ns(cycles, clocksource_counter.mult,
> +clocksource_counter.shift);
> + timespec64_add_ns(_ts, nsecs);
> +
> + if (do_settimeofday64(_ts))
> + pr_warn("arch_timer: unable to set systime\n");
> +}
> +#endif /* CONFIG_BOOT_TIME_PRESERVE */
> +
>  static void __init arch_counter_register(unsigned type)
>  {
>   u64 start_count;
> @@ -504,6 +533,10 @@ static void __init arch_counter_register(unsigned type)
>  
>   /* 56 bits minimum, so we assume worst case rollover */
>   sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate);
> +#ifdef CONFIG_BOOT_TIME_PRESERVE
> + /* Set systime */
> + arch_timer_setsystime();
> +#endif /* CONFIG_BOOT_TIME_PRESERVE */
>  }

Regardless of what this is trying to achieve, this does not belong in
the arch timer driver, as Thomas has already said.

Thanks,
Mark.

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

[Mark Rutland]

On Fri, May 19, 2017 at 01:16:23PM +0300, Bogdan Mirea wrote:
> This option enables Boot Time Preservation between Bootloader and
> Linux Kernel. It is based on the idea that the Bootloader (or any
> other early firmware) will start the HW Timer and Linux Kernel will
> count the time starting with the cycles elapsed since timer start.
> 
> The sched_clock part is preserving boottime for kmsg which should be in
> sync with system uptime. The system uptime part is driver specific and I
> updated the arm_arch_timer with an arch_timer_setsystime() function
> which will call do_settimeofday64() with the values read from arch timer
> counter.
> 
> This way both kmsg and uptime will be in sync, otherwise inconsistencies
> will appear between the two.
> 
> The "preserve_boot_time" parameter should be appended to kernel cmdline
> from bootloader for kernel acknowledgment that the timer is running in
> bootloader.
> 
> Signed-off-by: Bogdan Mirea 
> ---
>  drivers/clocksource/arm_arch_timer.c | 33 +

In future, *please* use get_maintainer.pl, and ensure all relevant
maintainers are on Cc. e.g.

[mark@leverpostej:~/src/linux]% ./scripts/get_maintainer.pl -f 
drivers/clocksource/arm_arch_timer.c
Mark Rutland  (maintainer:ARM ARCHITECTED TIMER DRIVER)
Marc Zyngier  (maintainer:ARM ARCHITECTED TIMER DRIVER)
Daniel Lezcano  (supporter:CLOCKSOURCE, CLOCKEVENT 
DRIVERS)
Thomas Gleixner  (supporter:CLOCKSOURCE, CLOCKEVENT DRIVERS)
linux-arm-ker...@lists.infradead.org (moderated list:ARM ARCHITECTED TIMER 
DRIVER)
linux-kernel@vger.kernel.org (open list:CLOCKSOURCE, CLOCKEVENT DRIVERS)

As I was not Cc'd, I only spotted this by chance.

[...]

> diff --git a/drivers/clocksource/arm_arch_timer.c 
> b/drivers/clocksource/arm_arch_timer.c
> index 5152b38..95699cd 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -475,6 +475,35 @@ struct timecounter *arch_timer_get_timecounter(void)
>   return 
>  }
>  
> +#ifdef CONFIG_BOOT_TIME_PRESERVE
> +/*
> + * Set the real system time(including the time spent in bootloader)
> + * based on the timer counter.
> + */
> +
> +#ifndef BOOT_TIME_PRESERVE_CMDLINE
> + #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
> +#endif
> +void arch_timer_setsystime(void)
> +{
> + static struct timespec64 boot_ts;
> + static cycles_t cycles;
> + unsigned long long nsecs;
> +
> + if (!strstr(boot_command_line, BOOT_TIME_PRESERVE_CMDLINE))
> + return;
> +
> + cycles = arch_timer_read_counter() ? arch_timer_read_counter() : 0;
> +
> + nsecs = clocksource_cyc2ns(cycles, clocksource_counter.mult,
> +clocksource_counter.shift);
> + timespec64_add_ns(_ts, nsecs);
> +
> + if (do_settimeofday64(_ts))
> + pr_warn("arch_timer: unable to set systime\n");
> +}
> +#endif /* CONFIG_BOOT_TIME_PRESERVE */
> +
>  static void __init arch_counter_register(unsigned type)
>  {
>   u64 start_count;
> @@ -504,6 +533,10 @@ static void __init arch_counter_register(unsigned type)
>  
>   /* 56 bits minimum, so we assume worst case rollover */
>   sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate);
> +#ifdef CONFIG_BOOT_TIME_PRESERVE
> + /* Set systime */
> + arch_timer_setsystime();
> +#endif /* CONFIG_BOOT_TIME_PRESERVE */
>  }

Regardless of what this is trying to achieve, this does not belong in
the arch timer driver, as Thomas has already said.

Thanks,
Mark.

RE: [PATCH v3] Added "Preserve Boot Time Support"

[Thomas Gleixner]

On Wed, 24 May 2017, Mirea, Bogdan-Stefan wrote:
> I am thinking about using timekeeping_inject_sleeptime64(delta) hook to

That's not a hook. It's a regular function. Please use proper technical
terms.

> add a delta time at boot to the CLOCK_BOOTTIME, but the problem that
> arise here is that this hook is intended to be used on rtc_resume()
> code (to add the time spent in suspend to CLOCK_BOOTTIME).

It's not the intention. It _IS_ used for injecting the suspended time.

> >From my point of view this will do the trick even if the
> CONFIG_BOOT_TIME_PRESERVE will then depend on PM_SLEEP && RTC_HCTOSYS
> (needed by timekeeping_inject_sleeptime64 to work).

First of all there is no point in having this extra CONFIG switch. This can
be made unconditionally and depend on a kernel command line argument.

What has this to do with PM_SLEEP and RTC_HCTOSYS? Just because that
function is currently only available when these config switches are enabled
it does not mean that it cannot be made available unconditionally.

What you are trying to do is to cram that new functionality into the
existing code without actually spending time on thinking about a proper
design.

Let's talk about the objective.

 1) Insert time from system start (power on, hardware reset) to kernel start
(timekeeping init) into CLOCK_BOOTTIME, if the system can read out this
value.

 2) Coordinate sched_clock and CLOCK_BOOTTIME

#1 timekeeping_inject_sleeptime64() provides the required functionality
   already. We can rename that function to reflect that it's used for both
   (injecting suspend time and injecting system start time).

   The more interesting question is how to provide a generic mechanism to
   retrieve that value. There are two possibilities:

   A) Value is stored by the early boot code and retrieved from there

   B) Value is made available via a clocksource after initializing and
  registering it.

   #A can be solved by having a weak function

u64 __weak read_system_starttime(void)
{
return 0;
}

 and let architectures implementing it when required. Though I doubt
 that we need that right away.

   #B can be made generic in a very simple way

 Add a flag CLOCK_SOURCE_STARTTIME to the clocksource flags and let the
 core code use that flag exactly once to inject the system start time
 into CLOCK_BOOTTIME.

#2 depends on #1

   When the timekeeping core injects the system start time it can tell the
   sched_clock core to use that offset as well.

   But, that does not solve the problem that sched_clock and CLOCK_BOOTTIME
   will drift apart over time which makes the whole thing moot.

   There was a patch set floating around, which made the clock used for
   printk selectable so it can actually use CLOCK_MONOTONIC or
   CLOCK_BOOTTIME via ktime_get_[mono|boot]_fast_ns(), but that never got
   merged.

   So instead of trying to provide a half correct solution which does not
   allow you to coordinate dmesg timestamps, uptime, tracer timestamps
   etc. reliably due to drift, I rather have that dmesg selectable
   timestamping patch merged and provide coordinated timestamps that way.
   (Cc'ed Prarit, who was working on that)

Thanks,

tglx

   

RE: [PATCH v3] Added "Preserve Boot Time Support"

[Thomas Gleixner]

On Wed, 24 May 2017, Mirea, Bogdan-Stefan wrote:
> I am thinking about using timekeeping_inject_sleeptime64(delta) hook to

That's not a hook. It's a regular function. Please use proper technical
terms.

> add a delta time at boot to the CLOCK_BOOTTIME, but the problem that
> arise here is that this hook is intended to be used on rtc_resume()
> code (to add the time spent in suspend to CLOCK_BOOTTIME).

It's not the intention. It _IS_ used for injecting the suspended time.

> >From my point of view this will do the trick even if the
> CONFIG_BOOT_TIME_PRESERVE will then depend on PM_SLEEP && RTC_HCTOSYS
> (needed by timekeeping_inject_sleeptime64 to work).

First of all there is no point in having this extra CONFIG switch. This can
be made unconditionally and depend on a kernel command line argument.

What has this to do with PM_SLEEP and RTC_HCTOSYS? Just because that
function is currently only available when these config switches are enabled
it does not mean that it cannot be made available unconditionally.

What you are trying to do is to cram that new functionality into the
existing code without actually spending time on thinking about a proper
design.

Let's talk about the objective.

 1) Insert time from system start (power on, hardware reset) to kernel start
(timekeeping init) into CLOCK_BOOTTIME, if the system can read out this
value.

 2) Coordinate sched_clock and CLOCK_BOOTTIME

#1 timekeeping_inject_sleeptime64() provides the required functionality
   already. We can rename that function to reflect that it's used for both
   (injecting suspend time and injecting system start time).

   The more interesting question is how to provide a generic mechanism to
   retrieve that value. There are two possibilities:

   A) Value is stored by the early boot code and retrieved from there

   B) Value is made available via a clocksource after initializing and
  registering it.

   #A can be solved by having a weak function

u64 __weak read_system_starttime(void)
{
return 0;
}

 and let architectures implementing it when required. Though I doubt
 that we need that right away.

   #B can be made generic in a very simple way

 Add a flag CLOCK_SOURCE_STARTTIME to the clocksource flags and let the
 core code use that flag exactly once to inject the system start time
 into CLOCK_BOOTTIME.

#2 depends on #1

   When the timekeeping core injects the system start time it can tell the
   sched_clock core to use that offset as well.

   But, that does not solve the problem that sched_clock and CLOCK_BOOTTIME
   will drift apart over time which makes the whole thing moot.

   There was a patch set floating around, which made the clock used for
   printk selectable so it can actually use CLOCK_MONOTONIC or
   CLOCK_BOOTTIME via ktime_get_[mono|boot]_fast_ns(), but that never got
   merged.

   So instead of trying to provide a half correct solution which does not
   allow you to coordinate dmesg timestamps, uptime, tracer timestamps
   etc. reliably due to drift, I rather have that dmesg selectable
   timestamping patch merged and provide coordinated timestamps that way.
   (Cc'ed Prarit, who was working on that)

Thanks,

tglx

   

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

[Pavel Machek]

Hi!

> +#ifndef BOOT_TIME_PRESERVE_CMDLINE
> + #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
> +#endif

Needs documentation in Doc*/, should not have separate define, that
just makes it complex.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

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

[Pavel Machek]

Hi!

> +#ifndef BOOT_TIME_PRESERVE_CMDLINE
> + #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
> +#endif

Needs documentation in Doc*/, should not have separate define, that
just makes it complex.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

RE: [PATCH v3] Added "Preserve Boot Time Support"

[Mirea, Bogdan-Stefan]

On Tuesday, May 23, 2017 7:39 PM, Thomas Gleixner wrote:
> On Tue, 23 May 2017, Mirea, Bogdan-Stefan wrote:
> > On Monday, May 22, 2017 12:36 AM, Thomas Gleixner wrote:
> > > On Fri, 19 May 2017, Bogdan Mirea wrote:
> > > This adds a arch_timer specific command line option. Why is this
> > > arch_timer
> > > specific? So if any other platform wants to gain this feature then we
> > > end
> > > up copying that mess to every single timer implementation? Certainly
> > > NOT!
> > > Exactly nothing. settimeofday() modifies CLOCK_REALTIME and if the
> > > platform
> > > has an early accessible RTC, you hereby wreckaged wall_time. If the
> > > RTC
> > > readout comes later then CLOCK_REALTIME is overwritten. So what is
> > > this
> > > supposed to do?
> > >
> > > It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is
> > > based
> > > on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since system
> > > boot. The
> > > difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is that
> > > CLOCK_MONOTONIC does not advance during suspend, but
> CLOCK_BOOTTIME
> > > takes
> > > the suspended time into account.
> > Thanks for feedback.
> > The idea of this patch was of a POC and this is why the code was
> > isolated in timer driver, which I agree is not a good idea for other
> > platforms to copy this since we can simply do all the things in
> > sched_clock_register() function guarded with
> CONFIG_BOOT_TIME_PRESERVE.
> > The patch was created for an internal project where no RTC was available
> > and no user-space apps were making any settimeofday(), and the use of
> > do_settimeofday() seemed safe. But yes, considering that the
> > CLOCK_REALTIME can be easily changed it should not be used here.
> 
> It does not matter at all whether you have a RTC or settimeofday() is used
> or not.
> 
> Again:
> 
> > > It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is
> > > based on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since
> system
> > > boot. The difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME
> is that
> > > CLOCK_MONOTONIC does not advance during suspend, but
> CLOCK_BOOTTIME
> > > takes the suspended time into account.
> 
> So using settimeofday() for any of what you want to do is bogus and
> useless.

Got it. Thanks!

I am thinking about using timekeeping_inject_sleeptime64(delta) hook to
add a delta time at boot to the CLOCK_BOOTTIME, but the problem that
arise here is that this hook is intended to be used on rtc_resume()
code (to add the time spent in suspend to CLOCK_BOOTTIME).
>From my point of view this will do the trick even if the
CONFIG_BOOT_TIME_PRESERVE will then depend on PM_SLEEP && RTC_HCTOSYS
(needed by timekeeping_inject_sleeptime64 to work).
Do you think this is a good approach?

Regards,
Bogdan

RE: [PATCH v3] Added "Preserve Boot Time Support"

[Mirea, Bogdan-Stefan]

On Tuesday, May 23, 2017 7:39 PM, Thomas Gleixner wrote:
> On Tue, 23 May 2017, Mirea, Bogdan-Stefan wrote:
> > On Monday, May 22, 2017 12:36 AM, Thomas Gleixner wrote:
> > > On Fri, 19 May 2017, Bogdan Mirea wrote:
> > > This adds a arch_timer specific command line option. Why is this
> > > arch_timer
> > > specific? So if any other platform wants to gain this feature then we
> > > end
> > > up copying that mess to every single timer implementation? Certainly
> > > NOT!
> > > Exactly nothing. settimeofday() modifies CLOCK_REALTIME and if the
> > > platform
> > > has an early accessible RTC, you hereby wreckaged wall_time. If the
> > > RTC
> > > readout comes later then CLOCK_REALTIME is overwritten. So what is
> > > this
> > > supposed to do?
> > >
> > > It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is
> > > based
> > > on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since system
> > > boot. The
> > > difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is that
> > > CLOCK_MONOTONIC does not advance during suspend, but
> CLOCK_BOOTTIME
> > > takes
> > > the suspended time into account.
> > Thanks for feedback.
> > The idea of this patch was of a POC and this is why the code was
> > isolated in timer driver, which I agree is not a good idea for other
> > platforms to copy this since we can simply do all the things in
> > sched_clock_register() function guarded with
> CONFIG_BOOT_TIME_PRESERVE.
> > The patch was created for an internal project where no RTC was available
> > and no user-space apps were making any settimeofday(), and the use of
> > do_settimeofday() seemed safe. But yes, considering that the
> > CLOCK_REALTIME can be easily changed it should not be used here.
> 
> It does not matter at all whether you have a RTC or settimeofday() is used
> or not.
> 
> Again:
> 
> > > It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is
> > > based on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since
> system
> > > boot. The difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME
> is that
> > > CLOCK_MONOTONIC does not advance during suspend, but
> CLOCK_BOOTTIME
> > > takes the suspended time into account.
> 
> So using settimeofday() for any of what you want to do is bogus and
> useless.

Got it. Thanks!

I am thinking about using timekeeping_inject_sleeptime64(delta) hook to
add a delta time at boot to the CLOCK_BOOTTIME, but the problem that
arise here is that this hook is intended to be used on rtc_resume()
code (to add the time spent in suspend to CLOCK_BOOTTIME).
>From my point of view this will do the trick even if the
CONFIG_BOOT_TIME_PRESERVE will then depend on PM_SLEEP && RTC_HCTOSYS
(needed by timekeeping_inject_sleeptime64 to work).
Do you think this is a good approach?

Regards,
Bogdan

RE: [PATCH v3] Added "Preserve Boot Time Support"

[Thomas Gleixner]

On Tue, 23 May 2017, Mirea, Bogdan-Stefan wrote:
> On Monday, May 22, 2017 12:36 AM, Thomas Gleixner wrote:
> > On Fri, 19 May 2017, Bogdan Mirea wrote:
> > This adds a arch_timer specific command line option. Why is this
> > arch_timer
> > specific? So if any other platform wants to gain this feature then we
> > end
> > up copying that mess to every single timer implementation? Certainly
> > NOT!
> > Exactly nothing. settimeofday() modifies CLOCK_REALTIME and if the
> > platform
> > has an early accessible RTC, you hereby wreckaged wall_time. If the
> > RTC
> > readout comes later then CLOCK_REALTIME is overwritten. So what is
> > this
> > supposed to do?
> >
> > It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is
> > based
> > on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since system
> > boot. The
> > difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is that
> > CLOCK_MONOTONIC does not advance during suspend, but CLOCK_BOOTTIME
> > takes
> > the suspended time into account.
> Thanks for feedback.
> The idea of this patch was of a POC and this is why the code was
> isolated in timer driver, which I agree is not a good idea for other
> platforms to copy this since we can simply do all the things in
> sched_clock_register() function guarded with CONFIG_BOOT_TIME_PRESERVE.
> The patch was created for an internal project where no RTC was available
> and no user-space apps were making any settimeofday(), and the use of
> do_settimeofday() seemed safe. But yes, considering that the
> CLOCK_REALTIME can be easily changed it should not be used here.

It does not matter at all whether you have a RTC or settimeofday() is used
or not.

Again:

> > It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is
> > based on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since system
> > boot. The difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is that
> > CLOCK_MONOTONIC does not advance during suspend, but CLOCK_BOOTTIME
> > takes the suspended time into account.

So using settimeofday() for any of what you want to do is bogus and
useless.

Thanks,

tglx

RE: [PATCH v3] Added "Preserve Boot Time Support"

[Thomas Gleixner]

On Tue, 23 May 2017, Mirea, Bogdan-Stefan wrote:
> On Monday, May 22, 2017 12:36 AM, Thomas Gleixner wrote:
> > On Fri, 19 May 2017, Bogdan Mirea wrote:
> > This adds a arch_timer specific command line option. Why is this
> > arch_timer
> > specific? So if any other platform wants to gain this feature then we
> > end
> > up copying that mess to every single timer implementation? Certainly
> > NOT!
> > Exactly nothing. settimeofday() modifies CLOCK_REALTIME and if the
> > platform
> > has an early accessible RTC, you hereby wreckaged wall_time. If the
> > RTC
> > readout comes later then CLOCK_REALTIME is overwritten. So what is
> > this
> > supposed to do?
> >
> > It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is
> > based
> > on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since system
> > boot. The
> > difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is that
> > CLOCK_MONOTONIC does not advance during suspend, but CLOCK_BOOTTIME
> > takes
> > the suspended time into account.
> Thanks for feedback.
> The idea of this patch was of a POC and this is why the code was
> isolated in timer driver, which I agree is not a good idea for other
> platforms to copy this since we can simply do all the things in
> sched_clock_register() function guarded with CONFIG_BOOT_TIME_PRESERVE.
> The patch was created for an internal project where no RTC was available
> and no user-space apps were making any settimeofday(), and the use of
> do_settimeofday() seemed safe. But yes, considering that the
> CLOCK_REALTIME can be easily changed it should not be used here.

It does not matter at all whether you have a RTC or settimeofday() is used
or not.

Again:

> > It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is
> > based on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since system
> > boot. The difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is that
> > CLOCK_MONOTONIC does not advance during suspend, but CLOCK_BOOTTIME
> > takes the suspended time into account.

So using settimeofday() for any of what you want to do is bogus and
useless.

Thanks,

tglx

RE: [PATCH v3] Added "Preserve Boot Time Support"

[Mirea, Bogdan-Stefan]

On Monday, May 22, 2017 12:36 AM, Thomas Gleixner wrote:
> On Fri, 19 May 2017, Bogdan Mirea wrote:
> This adds a arch_timer specific command line option. Why is this
> arch_timer
> specific? So if any other platform wants to gain this feature then we
> end
> up copying that mess to every single timer implementation? Certainly
> NOT!
> Exactly nothing. settimeofday() modifies CLOCK_REALTIME and if the
> platform
> has an early accessible RTC, you hereby wreckaged wall_time. If the
> RTC
> readout comes later then CLOCK_REALTIME is overwritten. So what is
> this
> supposed to do?
>
> It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is
> based
> on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since system
> boot. The
> difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is that
> CLOCK_MONOTONIC does not advance during suspend, but CLOCK_BOOTTIME
> takes
> the suspended time into account.
Thanks for feedback.
The idea of this patch was of a POC and this is why the code was
isolated in timer driver, which I agree is not a good idea for other
platforms to copy this since we can simply do all the things in
sched_clock_register() function guarded with CONFIG_BOOT_TIME_PRESERVE.
The patch was created for an internal project where no RTC was available
and no user-space apps were making any settimeofday(), and the use of
do_settimeofday() seemed safe. But yes, considering that the
CLOCK_REALTIME can be easily changed it should not be used here.



A way of setting the CLOCK_BOOTTIME is through the
timekeeping_inject_sleeptime64(delta) hook, but the problem that arise
here is that this hook is intended to be used on rtc_resume() code.
Adding delta to CLOCK_BOOTTIME this way, the CONFIG_BOOT_TIME_PRESERVE
will depend on PM_SLEEP && RTC_HCTOSYS.


The changes will be the following:
diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index a26036d..6c8ad44 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -193,6 +193,26 @@ sched_clock_register(u64 (*read)(void), int bits, unsigned 
long rate)
/* Update epoch for new counter and update 'epoch_ns' from old counter*/
new_epoch = read();
cyc = cd.actual_read_sched_clock();
+
+#ifdef CONFIG_BOOT_TIME_PRESERVE
+
+#ifndef BOOT_TIME_PRESERVE_CMDLINE
+   #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
+#endif
+   if (strstr(boot_command_line, BOOT_TIME_PRESERVE_CMDLINE)) {
+   static struct timespec64 delta_ts;
+   cyc = new_epoch;
+   rd.sched_clock_mask = new_mask;
+   rd.mult = new_mult;
+   rd.shift = new_shift;
+
+   timespec64_add_ns(_ts, clocksource_cyc2ns(cyc, rd.mult, 
rd.shift));
+   timekeeping_inject_sleeptime64(_ts);
+   
+   }
+#endif /* CONFIG_BOOT_TIME_PRESERVE */
+
ns = rd.epoch_ns + cyc_to_ns((cyc - rd.epoch_cyc) & 
rd.sched_clock_mask, rd.mult, rd.shift);
cd.actual_read_sched_clock = read;

diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index 4008d9f..2e392aa 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -193,5 +193,18 @@ config HIGH_RES_TIMERS
  hardware is not capable then this option only increases
  the size of the kernel image.
 
+config BOOT_TIME_PRESERVE
+   bool "Preserve Boot Time Support"
+   default n
+   depends on PM_SLEEP && RTC_HCTOSYS
+   help
+ This option enables Boot Time Preservation between Bootloader and
+ Linux Kernel. It is based on the idea that the Bootloader (or any
+ other early firmware) will start the HW Timer and Linux Kernel will
+ count the time starting with the cycles elapsed since timer start.
+
+ The "preserve_boot_time" parameter should be appended to kernel 
cmdline
+ from bootloader for kernel acknowledgment that the timer is running in
+ bootloader.
 endmenu
 endif

Waiting for feedback.

Thanks,
Bogdan

RE: [PATCH v3] Added "Preserve Boot Time Support"

[Mirea, Bogdan-Stefan]

On Monday, May 22, 2017 12:36 AM, Thomas Gleixner wrote:
> On Fri, 19 May 2017, Bogdan Mirea wrote:
> This adds a arch_timer specific command line option. Why is this
> arch_timer
> specific? So if any other platform wants to gain this feature then we
> end
> up copying that mess to every single timer implementation? Certainly
> NOT!
> Exactly nothing. settimeofday() modifies CLOCK_REALTIME and if the
> platform
> has an early accessible RTC, you hereby wreckaged wall_time. If the
> RTC
> readout comes later then CLOCK_REALTIME is overwritten. So what is
> this
> supposed to do?
>
> It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is
> based
> on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since system
> boot. The
> difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is that
> CLOCK_MONOTONIC does not advance during suspend, but CLOCK_BOOTTIME
> takes
> the suspended time into account.
Thanks for feedback.
The idea of this patch was of a POC and this is why the code was
isolated in timer driver, which I agree is not a good idea for other
platforms to copy this since we can simply do all the things in
sched_clock_register() function guarded with CONFIG_BOOT_TIME_PRESERVE.
The patch was created for an internal project where no RTC was available
and no user-space apps were making any settimeofday(), and the use of
do_settimeofday() seemed safe. But yes, considering that the
CLOCK_REALTIME can be easily changed it should not be used here.



A way of setting the CLOCK_BOOTTIME is through the
timekeeping_inject_sleeptime64(delta) hook, but the problem that arise
here is that this hook is intended to be used on rtc_resume() code.
Adding delta to CLOCK_BOOTTIME this way, the CONFIG_BOOT_TIME_PRESERVE
will depend on PM_SLEEP && RTC_HCTOSYS.


The changes will be the following:
diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index a26036d..6c8ad44 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -193,6 +193,26 @@ sched_clock_register(u64 (*read)(void), int bits, unsigned 
long rate)
/* Update epoch for new counter and update 'epoch_ns' from old counter*/
new_epoch = read();
cyc = cd.actual_read_sched_clock();
+
+#ifdef CONFIG_BOOT_TIME_PRESERVE
+
+#ifndef BOOT_TIME_PRESERVE_CMDLINE
+   #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
+#endif
+   if (strstr(boot_command_line, BOOT_TIME_PRESERVE_CMDLINE)) {
+   static struct timespec64 delta_ts;
+   cyc = new_epoch;
+   rd.sched_clock_mask = new_mask;
+   rd.mult = new_mult;
+   rd.shift = new_shift;
+
+   timespec64_add_ns(_ts, clocksource_cyc2ns(cyc, rd.mult, 
rd.shift));
+   timekeeping_inject_sleeptime64(_ts);
+   
+   }
+#endif /* CONFIG_BOOT_TIME_PRESERVE */
+
ns = rd.epoch_ns + cyc_to_ns((cyc - rd.epoch_cyc) & 
rd.sched_clock_mask, rd.mult, rd.shift);
cd.actual_read_sched_clock = read;

diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index 4008d9f..2e392aa 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -193,5 +193,18 @@ config HIGH_RES_TIMERS
  hardware is not capable then this option only increases
  the size of the kernel image.
 
+config BOOT_TIME_PRESERVE
+   bool "Preserve Boot Time Support"
+   default n
+   depends on PM_SLEEP && RTC_HCTOSYS
+   help
+ This option enables Boot Time Preservation between Bootloader and
+ Linux Kernel. It is based on the idea that the Bootloader (or any
+ other early firmware) will start the HW Timer and Linux Kernel will
+ count the time starting with the cycles elapsed since timer start.
+
+ The "preserve_boot_time" parameter should be appended to kernel 
cmdline
+ from bootloader for kernel acknowledgment that the timer is running in
+ bootloader.
 endmenu
 endif

Waiting for feedback.

Thanks,
Bogdan

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

[Thomas Gleixner]

On Fri, 19 May 2017, Bogdan Mirea wrote:
> +#ifdef CONFIG_BOOT_TIME_PRESERVE
> +/*
> + * Set the real system time(including the time spent in bootloader)
> + * based on the timer counter.
> + */
> +
> +#ifndef BOOT_TIME_PRESERVE_CMDLINE
> + #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
> +#endif
> +void arch_timer_setsystime(void)
> +{
> + static struct timespec64 boot_ts;
> + static cycles_t cycles;
> + unsigned long long nsecs;
> +
> + if (!strstr(boot_command_line, BOOT_TIME_PRESERVE_CMDLINE))
> + return;

This adds a arch_timer specific command line option. Why is this arch_timer
specific? So if any other platform wants to gain this feature then we end
up copying that mess to every single timer implementation? Certainly NOT!

> +
> + cycles = arch_timer_read_counter() ? arch_timer_read_counter() : 0;
> +
> + nsecs = clocksource_cyc2ns(cycles, clocksource_counter.mult,
> +clocksource_counter.shift);
> + timespec64_add_ns(_ts, nsecs);
> +
> + if (do_settimeofday64(_ts))
> + pr_warn("arch_timer: unable to set systime\n");

What the heck is this? What has boot_ts to do with do_settimeofday()?

Exactly nothing. settimeofday() modifies CLOCK_REALTIME and if the platform
has an early accessible RTC, you hereby wreckaged wall_time. If the RTC
readout comes later then CLOCK_REALTIME is overwritten. So what is this
supposed to do?

It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is based
on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since system boot. The
difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is that
CLOCK_MONOTONIC does not advance during suspend, but CLOCK_BOOTTIME takes
the suspended time into account.

I have not the faintest idea how you can claim that this patch actually
does what it is supposed to do. It's simply crap and CANNOT work at all.

Thanks,

tglx

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

[Thomas Gleixner]

On Fri, 19 May 2017, Bogdan Mirea wrote:
> +#ifdef CONFIG_BOOT_TIME_PRESERVE
> +/*
> + * Set the real system time(including the time spent in bootloader)
> + * based on the timer counter.
> + */
> +
> +#ifndef BOOT_TIME_PRESERVE_CMDLINE
> + #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
> +#endif
> +void arch_timer_setsystime(void)
> +{
> + static struct timespec64 boot_ts;
> + static cycles_t cycles;
> + unsigned long long nsecs;
> +
> + if (!strstr(boot_command_line, BOOT_TIME_PRESERVE_CMDLINE))
> + return;

This adds a arch_timer specific command line option. Why is this arch_timer
specific? So if any other platform wants to gain this feature then we end
up copying that mess to every single timer implementation? Certainly NOT!

> +
> + cycles = arch_timer_read_counter() ? arch_timer_read_counter() : 0;
> +
> + nsecs = clocksource_cyc2ns(cycles, clocksource_counter.mult,
> +clocksource_counter.shift);
> + timespec64_add_ns(_ts, nsecs);
> +
> + if (do_settimeofday64(_ts))
> + pr_warn("arch_timer: unable to set systime\n");

What the heck is this? What has boot_ts to do with do_settimeofday()?

Exactly nothing. settimeofday() modifies CLOCK_REALTIME and if the platform
has an early accessible RTC, you hereby wreckaged wall_time. If the RTC
readout comes later then CLOCK_REALTIME is overwritten. So what is this
supposed to do?

It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is based
on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since system boot. The
difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is that
CLOCK_MONOTONIC does not advance during suspend, but CLOCK_BOOTTIME takes
the suspended time into account.

I have not the faintest idea how you can claim that this patch actually
does what it is supposed to do. It's simply crap and CANNOT work at all.

Thanks,

tglx

[PATCH v3] Added "Preserve Boot Time Support"

[Bogdan Mirea]

This option enables Boot Time Preservation between Bootloader and
Linux Kernel. It is based on the idea that the Bootloader (or any
other early firmware) will start the HW Timer and Linux Kernel will
count the time starting with the cycles elapsed since timer start.

The sched_clock part is preserving boottime for kmsg which should be in
sync with system uptime. The system uptime part is driver specific and I
updated the arm_arch_timer with an arch_timer_setsystime() function
which will call do_settimeofday64() with the values read from arch timer
counter.

This way both kmsg and uptime will be in sync, otherwise inconsistencies
will appear between the two.

The "preserve_boot_time" parameter should be appended to kernel cmdline
from bootloader for kernel acknowledgment that the timer is running in
bootloader.

Signed-off-by: Bogdan Mirea 
---
 drivers/clocksource/arm_arch_timer.c | 33 +
 kernel/time/Kconfig  | 12 
 kernel/time/sched_clock.c| 14 ++
 3 files changed, 59 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c 
b/drivers/clocksource/arm_arch_timer.c
index 5152b38..95699cd 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -475,6 +475,35 @@ struct timecounter *arch_timer_get_timecounter(void)
return 
 }
 
+#ifdef CONFIG_BOOT_TIME_PRESERVE
+/*
+ * Set the real system time(including the time spent in bootloader)
+ * based on the timer counter.
+ */
+
+#ifndef BOOT_TIME_PRESERVE_CMDLINE
+   #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
+#endif
+void arch_timer_setsystime(void)
+{
+   static struct timespec64 boot_ts;
+   static cycles_t cycles;
+   unsigned long long nsecs;
+
+   if (!strstr(boot_command_line, BOOT_TIME_PRESERVE_CMDLINE))
+   return;
+
+   cycles = arch_timer_read_counter() ? arch_timer_read_counter() : 0;
+
+   nsecs = clocksource_cyc2ns(cycles, clocksource_counter.mult,
+  clocksource_counter.shift);
+   timespec64_add_ns(_ts, nsecs);
+
+   if (do_settimeofday64(_ts))
+   pr_warn("arch_timer: unable to set systime\n");
+}
+#endif /* CONFIG_BOOT_TIME_PRESERVE */
+
 static void __init arch_counter_register(unsigned type)
 {
u64 start_count;
@@ -504,6 +533,10 @@ static void __init arch_counter_register(unsigned type)
 
/* 56 bits minimum, so we assume worst case rollover */
sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate);
+#ifdef CONFIG_BOOT_TIME_PRESERVE
+   /* Set systime */
+   arch_timer_setsystime();
+#endif /* CONFIG_BOOT_TIME_PRESERVE */
 }
 
 static void arch_timer_stop(struct clock_event_device *clk)
diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index 4008d9f..1edd518 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -193,5 +193,17 @@ config HIGH_RES_TIMERS
  hardware is not capable then this option only increases
  the size of the kernel image.
 
+config BOOT_TIME_PRESERVE
+   bool "Preserve Boot Time Support"
+   default n
+   help
+ This option enables Boot Time Preservation between Bootloader and
+ Linux Kernel. It is based on the idea that the Bootloader (or any
+ other early firmware) will start the HW Timer and Linux Kernel will
+ count the time starting with the cycles elapsed since timer start.
+
+ The "preserve_boot_time" parameter should be appended to kernel 
cmdline
+ from bootloader for kernel acknowledgment that the timer is running in
+ bootloader.
 endmenu
 endif
diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index a26036d..efd66bf 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -193,6 +193,20 @@ sched_clock_register(u64 (*read)(void), int bits, unsigned 
long rate)
/* Update epoch for new counter and update 'epoch_ns' from old counter*/
new_epoch = read();
cyc = cd.actual_read_sched_clock();
+
+#ifdef CONFIG_BOOT_TIME_PRESERVE
+
+#ifndef BOOT_TIME_PRESERVE_CMDLINE
+   #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
+#endif
+   if (strstr(boot_command_line, BOOT_TIME_PRESERVE_CMDLINE)) {
+   cyc = new_epoch;
+   rd.sched_clock_mask = new_mask;
+   rd.mult = new_mult;
+   rd.shift= new_shift;
+   }
+#endif /* CONFIG_BOOT_TIME_PRESERVE */
+
ns = rd.epoch_ns + cyc_to_ns((cyc - rd.epoch_cyc) & 
rd.sched_clock_mask, rd.mult, rd.shift);
cd.actual_read_sched_clock = read;
 
-- 
1.9.1

[PATCH v3] Added "Preserve Boot Time Support"

[Bogdan Mirea]

This option enables Boot Time Preservation between Bootloader and
Linux Kernel. It is based on the idea that the Bootloader (or any
other early firmware) will start the HW Timer and Linux Kernel will
count the time starting with the cycles elapsed since timer start.

The sched_clock part is preserving boottime for kmsg which should be in
sync with system uptime. The system uptime part is driver specific and I
updated the arm_arch_timer with an arch_timer_setsystime() function
which will call do_settimeofday64() with the values read from arch timer
counter.

This way both kmsg and uptime will be in sync, otherwise inconsistencies
will appear between the two.

The "preserve_boot_time" parameter should be appended to kernel cmdline
from bootloader for kernel acknowledgment that the timer is running in
bootloader.

Signed-off-by: Bogdan Mirea 
---
 drivers/clocksource/arm_arch_timer.c | 33 +
 kernel/time/Kconfig  | 12 
 kernel/time/sched_clock.c| 14 ++
 3 files changed, 59 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c 
b/drivers/clocksource/arm_arch_timer.c
index 5152b38..95699cd 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -475,6 +475,35 @@ struct timecounter *arch_timer_get_timecounter(void)
return 
 }
 
+#ifdef CONFIG_BOOT_TIME_PRESERVE
+/*
+ * Set the real system time(including the time spent in bootloader)
+ * based on the timer counter.
+ */
+
+#ifndef BOOT_TIME_PRESERVE_CMDLINE
+   #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
+#endif
+void arch_timer_setsystime(void)
+{
+   static struct timespec64 boot_ts;
+   static cycles_t cycles;
+   unsigned long long nsecs;
+
+   if (!strstr(boot_command_line, BOOT_TIME_PRESERVE_CMDLINE))
+   return;
+
+   cycles = arch_timer_read_counter() ? arch_timer_read_counter() : 0;
+
+   nsecs = clocksource_cyc2ns(cycles, clocksource_counter.mult,
+  clocksource_counter.shift);
+   timespec64_add_ns(_ts, nsecs);
+
+   if (do_settimeofday64(_ts))
+   pr_warn("arch_timer: unable to set systime\n");
+}
+#endif /* CONFIG_BOOT_TIME_PRESERVE */
+
 static void __init arch_counter_register(unsigned type)
 {
u64 start_count;
@@ -504,6 +533,10 @@ static void __init arch_counter_register(unsigned type)
 
/* 56 bits minimum, so we assume worst case rollover */
sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate);
+#ifdef CONFIG_BOOT_TIME_PRESERVE
+   /* Set systime */
+   arch_timer_setsystime();
+#endif /* CONFIG_BOOT_TIME_PRESERVE */
 }
 
 static void arch_timer_stop(struct clock_event_device *clk)
diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index 4008d9f..1edd518 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -193,5 +193,17 @@ config HIGH_RES_TIMERS
  hardware is not capable then this option only increases
  the size of the kernel image.
 
+config BOOT_TIME_PRESERVE
+   bool "Preserve Boot Time Support"
+   default n
+   help
+ This option enables Boot Time Preservation between Bootloader and
+ Linux Kernel. It is based on the idea that the Bootloader (or any
+ other early firmware) will start the HW Timer and Linux Kernel will
+ count the time starting with the cycles elapsed since timer start.
+
+ The "preserve_boot_time" parameter should be appended to kernel 
cmdline
+ from bootloader for kernel acknowledgment that the timer is running in
+ bootloader.
 endmenu
 endif
diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index a26036d..efd66bf 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -193,6 +193,20 @@ sched_clock_register(u64 (*read)(void), int bits, unsigned 
long rate)
/* Update epoch for new counter and update 'epoch_ns' from old counter*/
new_epoch = read();
cyc = cd.actual_read_sched_clock();
+
+#ifdef CONFIG_BOOT_TIME_PRESERVE
+
+#ifndef BOOT_TIME_PRESERVE_CMDLINE
+   #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time"
+#endif
+   if (strstr(boot_command_line, BOOT_TIME_PRESERVE_CMDLINE)) {
+   cyc = new_epoch;
+   rd.sched_clock_mask = new_mask;
+   rd.mult = new_mult;
+   rd.shift= new_shift;
+   }
+#endif /* CONFIG_BOOT_TIME_PRESERVE */
+
ns = rd.epoch_ns + cyc_to_ns((cyc - rd.epoch_cyc) & 
rd.sched_clock_mask, rd.mult, rd.shift);
cd.actual_read_sched_clock = read;
 
-- 
1.9.1