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(&delta_ts, clocksource_cyc2ns(cyc, rd.mult, rd.shift));
+		timekeeping_inject_sleeptime64(&delta_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