linux-kernel - [PATCH v4 1/2] arch, x86, tsc deadline clockevent dev: eliminate frequency roundoff error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20160714152255.18295-2-nicstange@gmail.com>
Date:	Thu, 14 Jul 2016 17:22:54 +0200
From:	Nicolai Stange <nicstange@...il.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
	x86@...nel.org, Borislav Petkov <bp@...e.de>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>,
	"Peter Zijlstra (Intel)" <peterz@...radead.org>,
	"Christopher S. Hall" <christopher.s.hall@...el.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	linux-kernel@...r.kernel.org, Nicolai Stange <nicstange@...il.com>
Subject: [PATCH v4 1/2] arch, x86, tsc deadline clockevent dev: eliminate frequency roundoff error

In setup_APIC_timer(), the registered clockevent device's frequency
is calculated by first dividing tsc_khz by TSC_DIVISOR and multiplying
it with 1000 afterwards:

  (tsc_khz / TSC_DIVISOR) * 1000

The multiplication with 1000 is done for converting from kHz to Hz and the
division by TSC_DIVISOR is carried out in order to make sure that the final
result fits into an u32.

However, with the order given in this calculation, the roundoff error
introduced by the division gets magnified by a factor of 1000 by the
following multiplication. Thus, reversing the order of the division and
the multiplication a la

  (tsc_khz * 1000) / TSC_DIVISOR

reduces the roundoff error already.

Furthermore, if TSC_DIVISOR divides 1000, associativity holds

  (tsc_khz * 1000) / TSC_DIVISOR = tsc_khz * (1000 / TSC_DIVISOR)

and thus, the roundoff error even vanishes and the whole operation can be
carried out within 32 bits.

The powers of two that divide 1000 are 2, 4 and 8. A value of 8 for
TSC_DIVISOR still allows for TSC frequencies up to
2^32 / 10^9ns * 8 = 34.4GHz which is way larger than anything to expect
in the next years.

Thus, replace the current TSC_DIVISOR value of 32 by 8. Reverse the
order of the divison and the multiplication in the calculation of the
registered clockevent device's frequency.

Signed-off-by: Nicolai Stange <nicstange@...il.com>
---
 arch/x86/kernel/apic/apic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 89a5bce..39517c0 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -311,7 +311,7 @@ int lapic_get_maxlvt(void)

 /* Clock divisor */
 #define APIC_DIVISOR 16
-#define TSC_DIVISOR  32
+#define TSC_DIVISOR  8

 /*
  * This function sets up the local APIC timer, with a timeout of
@@ -563,7 +563,7 @@ static void setup_APIC_timer(void)
 				    CLOCK_EVT_FEAT_DUMMY);
 		levt->set_next_event = lapic_next_deadline;
 		clockevents_config_and_register(levt,
-						(tsc_khz / TSC_DIVISOR) * 1000,
+						tsc_khz * (1000 / TSC_DIVISOR),
 						0xF, ~0UL);
 	} else
 		clockevents_register_device(levt);
-- 
2.9.0