[PATCH v4 0/4] Add support for S3 non-stop TSC support.

[PATCH v4 0/4] Add support for S3 non-stop TSC support.

[Feng Tang]

Hi All,

On some new Intel Atom processors (Penwell and Cloverview), there is
a feature that the TSC won't stop in S3, say the TSC value won't be
reset to 0 after resume. This feature makes TSC a more reliable
clocksource and could benefit the timekeeping code during system
suspend/resume cycles.

The enabling efforts include adding new flags for this feature, 
modifying clocksource.h and timekeeping.c to support and utilizing
it.

The major change to timekeeping is the way to count suspended time,
current way is trying to use the persistent clock first, and then
try the rtc if persistent clock can't do it. This patch will change
the trying order to:
	suspend-nonstop clocksource -> persistent clock -> rtc

Please help to review them, thanks a lot!

Changelog:
	v4:
	    * Don't touch clocksource_cyc2ns() API which is under
	      changes
	    * add an explicit flag for if the suspend time has been
	      calculated by nonstop clocksource or persistent clock 
	v3:
	    * Adopt Jason Gunthorpe's way to convert large cycles to
	      nsec. And put it into clocksource_cyc2ns()
	    * Small change in flag name
	v2:
	    * Dump the code changing a clocksource's mult and shit,
	      as suggested by John to not hurt accuracy
	    * Modify the CPU feature flag name to be consistent with
	      other flags
	    * Solve the problem of judging S3/S4, as the clocksource
	      counter will be reset after coming out S4.

- Feng

-------------

Feng Tang (4):
  x86: Add cpu capability flag X86_FEATURE_NONSTOP_TSC_S3
  clocksource: Add new feature flag CLOCK_SOURCE_SUSPEND_NONSTOP
  x86: tsc: Add support for new S3_NONSTOP feature
  timekeeping: utilize the suspend-nonstop clocksource to count
    suspended time

 arch/x86/include/asm/cpufeature.h |    1 +
 arch/x86/kernel/cpu/intel.c       |   12 ++++++++
 arch/x86/kernel/tsc.c             |    6 +++-
 include/linux/clocksource.h       |    1 +
 kernel/time/timekeeping.c         |   58 ++++++++++++++++++++++++++++++++-----
 5 files changed, 70 insertions(+), 8 deletions(-)

-- 
1.7.9.5

[PATCH v4 1/4] x86: Add cpu capability flag X86_FEATURE_NONSTOP_TSC_S3

[Feng Tang]

On some new Intel Atom processors (Penwell and Cloverview), there is
a feature that the TSC won't stop in S3 state, say the TSC value
won't be reset to 0 after resume. This feature makes TSC a more reliable
clocksource and could benefit the timekeeping code during system
suspend/resume cycle, so add a flag for it.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 arch/x86/include/asm/cpufeature.h |    1 +
 arch/x86/kernel/cpu/intel.c       |   12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 93fe929..a8466f2 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -100,6 +100,7 @@
 #define X86_FEATURE_AMD_DCM     (3*32+27) /* multi-node processor */
 #define X86_FEATURE_APERFMPERF	(3*32+28) /* APERFMPERF */
 #define X86_FEATURE_EAGER_FPU	(3*32+29) /* "eagerfpu" Non lazy FPU restore */
+#define X86_FEATURE_NONSTOP_TSC_S3 (3*32+30) /* TSC doesn't stop in S3 state */
 
 /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
 #define X86_FEATURE_XMM3	(4*32+ 0) /* "pni" SSE-3 */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 1905ce9..fe57544 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -96,6 +96,18 @@ static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
 			sched_clock_stable = 1;
 	}
 
+	/* Penwell and Cloverview have the TSC which doesn't sleep on S3 */
+	if (c->x86 == 6) {
+		switch (c->x86_model) {
+		case 0x27:	/* Penwell */
+		case 0x35:	/* Cloverview */
+			set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC_S3);
+			break;
+		default:
+			;
+		}
+	}
+
 	/*
 	 * There is a known erratum on Pentium III and Core Solo
 	 * and Core Duo CPUs.
-- 
1.7.9.5

Re: [PATCH v4 1/4] x86: Add cpu capability flag X86_FEATURE_NONSTOP_TSC_S3

[John Stultz]

On 03/11/2013 08:56 PM, Feng Tang wrote:
> On some new Intel Atom processors (Penwell and Cloverview), there is
> a feature that the TSC won't stop in S3 state, say the TSC value
> won't be reset to 0 after resume. This feature makes TSC a more reliable
> clocksource and could benefit the timekeeping code during system
> suspend/resume cycle, so add a flag for it.
>
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> ---
>   arch/x86/include/asm/cpufeature.h |    1 +
>   arch/x86/kernel/cpu/intel.c       |   12 ++++++++++++
>   2 files changed, 13 insertions(+)
>
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 93fe929..a8466f2 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -100,6 +100,7 @@
>   #define X86_FEATURE_AMD_DCM     (3*32+27) /* multi-node processor */
>   #define X86_FEATURE_APERFMPERF	(3*32+28) /* APERFMPERF */
>   #define X86_FEATURE_EAGER_FPU	(3*32+29) /* "eagerfpu" Non lazy FPU restore */
> +#define X86_FEATURE_NONSTOP_TSC_S3 (3*32+30) /* TSC doesn't stop in S3 state */
>   
>   /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
>   #define X86_FEATURE_XMM3	(4*32+ 0) /* "pni" SSE-3 */
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 1905ce9..fe57544 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -96,6 +96,18 @@ static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
>   			sched_clock_stable = 1;
>   	}
>   
> +	/* Penwell and Cloverview have the TSC which doesn't sleep on S3 */
> +	if (c->x86 == 6) {
> +		switch (c->x86_model) {
> +		case 0x27:	/* Penwell */
> +		case 0x35:	/* Cloverview */
> +			set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC_S3);
> +			break;
> +		default:
> +			;

Just FYI, checkpatch.sh complains that this should be
     default:
         break;

I've gone ahead and fixed that, but you might be sure to run checkpatch 
in the future before submitting.

thanks
-john

[PATCH v4 2/4] clocksource: Add new feature flag CLOCK_SOURCE_SUSPEND_NONSTOP

[Feng Tang]

Some x86 processors have a TSC clocksource, which continues to run
even when system is suspended. Also most OMAP platforms have a
32 KHz timer which has similar capability. Add a feature flag so that
it could be utilized.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 include/linux/clocksource.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 27cfda4..aa7032c 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -206,6 +206,7 @@ struct clocksource {
 #define CLOCK_SOURCE_WATCHDOG			0x10
 #define CLOCK_SOURCE_VALID_FOR_HRES		0x20
 #define CLOCK_SOURCE_UNSTABLE			0x40
+#define CLOCK_SOURCE_SUSPEND_NONSTOP		0x80
 
 /* simplify initialization of mask field */
 #define CLOCKSOURCE_MASK(bits) (cycle_t)((bits) < 64 ? ((1ULL<<(bits))-1) : -1)
-- 
1.7.9.5

[PATCH v4 3/4] x86: tsc: Add support for new S3_NONSTOP feature

[Feng Tang]

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 arch/x86/kernel/tsc.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 4b9ea10..098b3cf 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -768,7 +768,8 @@ static cycle_t read_tsc(struct clocksource *cs)
 
 static void resume_tsc(struct clocksource *cs)
 {
-	clocksource_tsc.cycle_last = 0;
+	if (!boot_cpu_has(X86_FEATURE_NONSTOP_TSC_S3))
+		clocksource_tsc.cycle_last = 0;
 }
 
 static struct clocksource clocksource_tsc = {
@@ -939,6 +940,9 @@ static int __init init_tsc_clocksource(void)
 		clocksource_tsc.flags &= ~CLOCK_SOURCE_IS_CONTINUOUS;
 	}
 
+	if (boot_cpu_has(X86_FEATURE_NONSTOP_TSC_S3))
+		clocksource_tsc.flags |= CLOCK_SOURCE_SUSPEND_NONSTOP;
+
 	/*
 	 * Trust the results of the earlier calibration on systems
 	 * exporting a reliable TSC.
-- 
1.7.9.5

[PATCH v4 4/4] timekeeping: utilize the suspend-nonstop clocksource to count suspended time

[Feng Tang]

There are some new processors whose TSC clocksource won't stop during
suspend. Currently, after system resumes, kernel will use persistent
clock or RTC to compensate the sleep time, but with these nonstop
clocksources, we could skip the special compensation from external
sources, and just use current clocksource for time recounting.

This can solve some time drift bugs caused by some not-so-accurate or
error-prone RTC devices.

The current way to count suspended time is first try to use the persistent
clock, and then try the RTC if persistent clock can't be used. This
patch will change the trying order to:
	suspend-nonstop clocksource -> persistent clock -> RTC

When counting the sleep time with nonstop clocksource, use an accurate way
suggested by Jason Gunthorpe to cover very large delta cycles.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 kernel/time/timekeeping.c |   58 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 9a0bc98..6d7ea93 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -788,22 +788,66 @@ void timekeeping_inject_sleeptime(struct timespec *delta)
 static void timekeeping_resume(void)
 {
 	struct timekeeper *tk = &timekeeper;
+	struct clocksource *clock = tk->clock;
 	unsigned long flags;
-	struct timespec ts;
+	struct timespec ts_new, ts_delta;
+	cycle_t cycle_now, cycle_delta;
+	bool suspendtime_found = false;
 
-	read_persistent_clock(&ts);
+	read_persistent_clock(&ts_new);
 
 	clockevents_resume();
 	clocksource_resume();
 
 	write_seqlock_irqsave(&tk->lock, flags);
 
-	if (timespec_compare(&ts, &timekeeping_suspend_time) > 0) {
-		ts = timespec_sub(ts, timekeeping_suspend_time);
-		__timekeeping_inject_sleeptime(tk, &ts);
+	/*
+	 * After system resumes, we need to calculate the suspended time and
+	 * compensate it for the OS time. There are 3 sources that could be
+	 * used: Nonstop clocksource during suspend, persistent clock and rtc
+	 * device.
+	 *
+	 * One specific platform may have 1 or 2 or all of them, and the
+	 * preference will be:
+	 *	suspend-nonstop clocksource -> persistent clock -> rtc
+	 * The less preferred source will only be tried if there is no better
+	 * usable source. The rtc part is handled separately in rtc core code.
+	 */
+	cycle_now = clock->read(clock);
+	if ((clock->flags & CLOCK_SOURCE_SUSPEND_NONSTOP) &&
+		cycle_now > clock->cycle_last) {
+		u64 num, max = ULLONG_MAX;
+		u32 mult = clock->mult;
+		u32 shift = clock->shift;
+		s64 nsec = 0;
+
+		cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
+
+		/*
+		 * "cycle_delta * mutl" may cause 64 bits overflow, if the
+		 * suspended time is too long. In that case we need do the
+		 * 64 bits math carefully
+		 */
+		do_div(max, mult);
+		if (cycle_delta > max) {
+			num = div64_u64(cycle_delta, max);
+			nsec = (((u64) max * mult) >> shift) * num;
+			cycle_delta -= num * max;
+		}
+		nsec += ((u64) cycle_delta * mult) >> shift;
+
+		ts_delta = ns_to_timespec(nsec);
+		suspendtime_found = true;
+	} else if (timespec_compare(&ts_new, &timekeeping_suspend_time) > 0) {
+		ts_delta = timespec_sub(ts_new, timekeeping_suspend_time);
+		suspendtime_found = true;
 	}
-	/* re-base the last cycle value */
-	tk->clock->cycle_last = tk->clock->read(tk->clock);
+
+	if (suspendtime_found)
+		__timekeeping_inject_sleeptime(tk, &ts_delta);
+
+	/* Re-base the last cycle value */
+	clock->cycle_last = clock->read(clock);
 	tk->ntp_error = 0;
 	timekeeping_suspended = 0;
 	timekeeping_update(tk, false);
-- 
1.7.9.5

Re: [PATCH v4 4/4] timekeeping: utilize the suspend-nonstop clocksource to count suspended time

[John Stultz]

On 03/11/2013 08:56 PM, Feng Tang wrote:
> +	/*
> +	 * After system resumes, we need to calculate the suspended time and
> +	 * compensate it for the OS time. There are 3 sources that could be
> +	 * used: Nonstop clocksource during suspend, persistent clock and rtc
> +	 * device.
> +	 *
> +	 * One specific platform may have 1 or 2 or all of them, and the
> +	 * preference will be:
> +	 *	suspend-nonstop clocksource -> persistent clock -> rtc
> +	 * The less preferred source will only be tried if there is no better
> +	 * usable source. The rtc part is handled separately in rtc core code.
> +	 */
> +	cycle_now = clock->read(clock);
> +	if ((clock->flags & CLOCK_SOURCE_SUSPEND_NONSTOP) &&
> +		cycle_now > clock->cycle_last) {
> +		u64 num, max = ULLONG_MAX;
> +		u32 mult = clock->mult;
> +		u32 shift = clock->shift;
> +		s64 nsec = 0;
> +
> +		cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
> +
> +		/*
> +		 * "cycle_delta * mutl" may cause 64 bits overflow, if the
> +		 * suspended time is too long. In that case we need do the
> +		 * 64 bits math carefully
> +		 */
> +		do_div(max, mult);
> +		if (cycle_delta > max) {
> +			num = div64_u64(cycle_delta, max);
> +			nsec = (((u64) max * mult) >> shift) * num;
> +			cycle_delta -= num * max;
> +		}
> +		nsec += ((u64) cycle_delta * mult) >> shift;
> +
> +		ts_delta = ns_to_timespec(nsec);
> +		suspendtime_found = true;
> +	} else if (timespec_compare(&ts_new, &timekeeping_suspend_time) > 0) {
> +		ts_delta = timespec_sub(ts_new, timekeeping_suspend_time);
> +		suspendtime_found = true;
>   	}
> -	/* re-base the last cycle value */
> -	tk->clock->cycle_last = tk->clock->read(tk->clock);
> +
> +	if (suspendtime_found)
> +		__timekeeping_inject_sleeptime(tk, &ts_delta);
> +
> +	/* Re-base the last cycle value */
> +	clock->cycle_last = clock->read(clock);
It seems like since we unconditionally read the clock above, this last 
line could be reworked to be:
     clock->cycle_last = cycle_now;

Which would save re-reading the clocksource.

If you don't have any objections I'll fold that small change into your 
patch.

thanks
-john

Re: [PATCH v4 4/4] timekeeping: utilize the suspend-nonstop clocksource to count suspended time

[Feng Tang]

On Tue, Mar 12, 2013 at 02:09:08PM -0700, John Stultz wrote:
> On 03/11/2013 08:56 PM, Feng Tang wrote:
 >  	}
> >-	/* re-base the last cycle value */
> >-	tk->clock->cycle_last = tk->clock->read(tk->clock);
> >+
> >+	if (suspendtime_found)
> >+		__timekeeping_inject_sleeptime(tk, &ts_delta);
> >+
> >+	/* Re-base the last cycle value */
> >+	clock->cycle_last = clock->read(clock);
> It seems like since we unconditionally read the clock above, this
> last line could be reworked to be:
>     clock->cycle_last = cycle_now;
> 
> Which would save re-reading the clocksource.
> 
> If you don't have any objections I'll fold that small change into
> your patch.

No objection at all. Thanks!

- Feng

Re: [PATCH v4 0/4] Add support for S3 non-stop TSC support.

[John Stultz]

On 03/11/2013 08:56 PM, Feng Tang wrote:
> Hi All,
>
> On some new Intel Atom processors (Penwell and Cloverview), there is
> a feature that the TSC won't stop in S3, say the TSC value won't be
> reset to 0 after resume. This feature makes TSC a more reliable
> clocksource and could benefit the timekeeping code during system
> suspend/resume cycles.
>
> The enabling efforts include adding new flags for this feature,
> modifying clocksource.h and timekeeping.c to support and utilizing
> it.
>
> The major change to timekeeping is the way to count suspended time,
> current way is trying to use the persistent clock first, and then
> try the rtc if persistent clock can't do it. This patch will change
> the trying order to:
> 	suspend-nonstop clocksource -> persistent clock -> rtc
>
> Please help to review them, thanks a lot!
>

Thanks!

I've gone ahead and queued these, with my minor modifications, for 3.10 
(pending further testing).

thanks
-john