* [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency
@ 2016-08-11 2:37 Star Zeng
2016-08-11 2:42 ` Gao, Liming
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Star Zeng @ 2016-08-11 2:37 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Michael D Kinney, Liming Gao, Paul A Lohr
Minimize the code overhead between the two TSC reads by adding
new internal API to calculate TSC Frequency instead of reusing
MicroSecondDelay ().
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Paul A Lohr <paul.a.lohr@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 56 +++++++++++++++++++++-
.../Library/AcpiTimerLib/BaseAcpiTimerLib.c | 33 ++++++++-----
.../Library/AcpiTimerLib/DxeAcpiTimerLib.c | 31 ++++++++----
3 files changed, 99 insertions(+), 21 deletions(-)
diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
index 806a4f7ce24c..e6fea231123d 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
@@ -1,7 +1,7 @@
/** @file
ACPI Timer implements one instance of Timer Library.
- Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -335,3 +335,57 @@ GetTimeInNanoSecond (
return NanoSeconds;
}
+
+/**
+ Calculate TSC frequency.
+
+ The TSC counting frequency is determined by comparing how far it counts
+ during a 100us period as determined by the ACPI timer. The ACPI timer is
+ used because it counts at a known frequency.
+ The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
+ clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
+ difference multiplied by 10000 is the TSC frequency. There will be a small
+ error because of the overhead of reading the ACPI timer. An attempt is
+ made to determine and compensate for this error.
+
+ @return The number of TSC counts per second.
+
+**/
+UINT64
+InternalCalculateTscFrequency (
+ VOID
+ )
+{
+ UINT64 StartTSC;
+ UINT64 EndTSC;
+ UINT16 TimerAddr;
+ UINT32 Ticks;
+ UINT64 TscFrequency;
+ BOOLEAN InterruptState;
+
+ InterruptState = SaveAndDisableInterrupts ();
+
+ TimerAddr = InternalAcpiGetAcpiTimerIoPort ();
+ Ticks = IoRead32 (TimerAddr) + (ACPI_TIMER_FREQUENCY / 10000); // Set Ticks to 100us in the future
+
+ StartTSC = AsmReadTsc (); // Get base value for the TSC
+ //
+ // Wait until the ACPI timer has counted 100us.
+ // Timer wrap-arounds are handled correctly by this function.
+ // When the current ACPI timer value is greater than 'Ticks', the while loop will exit.
+ //
+ while (((Ticks - IoRead32 (TimerAddr)) & BIT23) == 0) {
+ CpuPause();
+ }
+ EndTSC = AsmReadTsc (); // TSC value 100us later
+
+ TscFrequency = MultU64x32 (
+ (EndTSC - StartTSC), // Number of TSC counts in 100us
+ 10000 // Number of 100us in a second
+ );
+
+ SetInterruptState (InterruptState);
+
+ return TscFrequency;
+}
+
diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
index 21fdb79908b8..8819ebcfccef 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
@@ -1,7 +1,7 @@
/** @file
ACPI Timer implements one instance of Timer Library.
- Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -17,6 +17,26 @@
#include <Library/BaseLib.h>
/**
+ Calculate TSC frequency.
+
+ The TSC counting frequency is determined by comparing how far it counts
+ during a 100us period as determined by the ACPI timer. The ACPI timer is
+ used because it counts at a known frequency.
+ The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
+ clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
+ difference multiplied by 10000 is the TSC frequency. There will be a small
+ error because of the overhead of reading the ACPI timer. An attempt is
+ made to determine and compensate for this error.
+
+ @return The number of TSC counts per second.
+
+**/
+UINT64
+InternalCalculateTscFrequency (
+ VOID
+ );
+
+/**
Internal function to retrieves the 64-bit frequency in Hz.
Internal function to retrieves the 64-bit frequency in Hz.
@@ -29,14 +49,5 @@ InternalGetPerformanceCounterFrequency (
VOID
)
{
- BOOLEAN InterruptState;
- UINT64 Count;
- UINT64 Frequency;
-
- InterruptState = SaveAndDisableInterrupts ();
- Count = GetPerformanceCounter ();
- MicroSecondDelay (100);
- Frequency = MultU64x32 (GetPerformanceCounter () - Count, 10000);
- SetInterruptState (InterruptState);
- return Frequency;
+ return InternalCalculateTscFrequency ();
}
diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
index 6f5c07a4f0b4..7f7b0f8f6294 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
@@ -1,7 +1,7 @@
/** @file
ACPI Timer implements one instance of Timer Library.
- Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -16,6 +16,26 @@
#include <Library/TimerLib.h>
#include <Library/BaseLib.h>
+/**
+ Calculate TSC frequency.
+
+ The TSC counting frequency is determined by comparing how far it counts
+ during a 100us period as determined by the ACPI timer. The ACPI timer is
+ used because it counts at a known frequency.
+ The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
+ clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
+ difference multiplied by 10000 is the TSC frequency. There will be a small
+ error because of the overhead of reading the ACPI timer. An attempt is
+ made to determine and compensate for this error.
+
+ @return The number of TSC counts per second.
+
+**/
+UINT64
+InternalCalculateTscFrequency (
+ VOID
+ );
+
//
// Cached performance counter frequency
//
@@ -34,15 +54,8 @@ InternalGetPerformanceCounterFrequency (
VOID
)
{
- BOOLEAN InterruptState;
- UINT64 Count;
-
if (mPerformanceCounterFrequency == 0) {
- InterruptState = SaveAndDisableInterrupts ();
- Count = GetPerformanceCounter ();
- MicroSecondDelay (100);
- mPerformanceCounterFrequency = MultU64x32 (GetPerformanceCounter () - Count, 10000);
- SetInterruptState (InterruptState);
+ mPerformanceCounterFrequency = InternalCalculateTscFrequency ();
}
return mPerformanceCounterFrequency;
}
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency
2016-08-11 2:37 [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency Star Zeng
@ 2016-08-11 2:42 ` Gao, Liming
2016-08-11 5:01 ` Kinney, Michael D
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Gao, Liming @ 2016-08-11 2:42 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Kinney, Michael D, Lohr, Paul A
Reviewed-by: Liming Gao <liming.gao@intel.com>
> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, August 11, 2016 10:38 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Lohr,
> Paul A <paul.a.lohr@intel.com>
> Subject: [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC
> Frequency
>
> Minimize the code overhead between the two TSC reads by adding
> new internal API to calculate TSC Frequency instead of reusing
> MicroSecondDelay ().
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Paul A Lohr <paul.a.lohr@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 56
> +++++++++++++++++++++-
> .../Library/AcpiTimerLib/BaseAcpiTimerLib.c | 33 ++++++++-----
> .../Library/AcpiTimerLib/DxeAcpiTimerLib.c | 31 ++++++++----
> 3 files changed, 99 insertions(+), 21 deletions(-)
>
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> index 806a4f7ce24c..e6fea231123d 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Timer implements one instance of Timer Library.
>
> - Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at
> @@ -335,3 +335,57 @@ GetTimeInNanoSecond (
>
> return NanoSeconds;
> }
> +
> +/**
> + Calculate TSC frequency.
> +
> + The TSC counting frequency is determined by comparing how far it counts
> + during a 100us period as determined by the ACPI timer. The ACPI timer is
> + used because it counts at a known frequency.
> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY /
> 10000
> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> + difference multiplied by 10000 is the TSC frequency. There will be a small
> + error because of the overhead of reading the ACPI timer. An attempt is
> + made to determine and compensate for this error.
> +
> + @return The number of TSC counts per second.
> +
> +**/
> +UINT64
> +InternalCalculateTscFrequency (
> + VOID
> + )
> +{
> + UINT64 StartTSC;
> + UINT64 EndTSC;
> + UINT16 TimerAddr;
> + UINT32 Ticks;
> + UINT64 TscFrequency;
> + BOOLEAN InterruptState;
> +
> + InterruptState = SaveAndDisableInterrupts ();
> +
> + TimerAddr = InternalAcpiGetAcpiTimerIoPort ();
> + Ticks = IoRead32 (TimerAddr) + (ACPI_TIMER_FREQUENCY / 10000); //
> Set Ticks to 100us in the future
> +
> + StartTSC = AsmReadTsc (); // Get base value for the
> TSC
> + //
> + // Wait until the ACPI timer has counted 100us.
> + // Timer wrap-arounds are handled correctly by this function.
> + // When the current ACPI timer value is greater than 'Ticks', the while loop
> will exit.
> + //
> + while (((Ticks - IoRead32 (TimerAddr)) & BIT23) == 0) {
> + CpuPause();
> + }
> + EndTSC = AsmReadTsc (); // TSC value 100us later
> +
> + TscFrequency = MultU64x32 (
> + (EndTSC - StartTSC), // Number of TSC counts in
> 100us
> + 10000 // Number of 100us in a second
> + );
> +
> + SetInterruptState (InterruptState);
> +
> + return TscFrequency;
> +}
> +
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> index 21fdb79908b8..8819ebcfccef 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Timer implements one instance of Timer Library.
>
> - Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at
> @@ -17,6 +17,26 @@
> #include <Library/BaseLib.h>
>
> /**
> + Calculate TSC frequency.
> +
> + The TSC counting frequency is determined by comparing how far it counts
> + during a 100us period as determined by the ACPI timer. The ACPI timer is
> + used because it counts at a known frequency.
> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY /
> 10000
> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> + difference multiplied by 10000 is the TSC frequency. There will be a small
> + error because of the overhead of reading the ACPI timer. An attempt is
> + made to determine and compensate for this error.
> +
> + @return The number of TSC counts per second.
> +
> +**/
> +UINT64
> +InternalCalculateTscFrequency (
> + VOID
> + );
> +
> +/**
> Internal function to retrieves the 64-bit frequency in Hz.
>
> Internal function to retrieves the 64-bit frequency in Hz.
> @@ -29,14 +49,5 @@ InternalGetPerformanceCounterFrequency (
> VOID
> )
> {
> - BOOLEAN InterruptState;
> - UINT64 Count;
> - UINT64 Frequency;
> -
> - InterruptState = SaveAndDisableInterrupts ();
> - Count = GetPerformanceCounter ();
> - MicroSecondDelay (100);
> - Frequency = MultU64x32 (GetPerformanceCounter () - Count, 10000);
> - SetInterruptState (InterruptState);
> - return Frequency;
> + return InternalCalculateTscFrequency ();
> }
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> index 6f5c07a4f0b4..7f7b0f8f6294 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Timer implements one instance of Timer Library.
>
> - Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at
> @@ -16,6 +16,26 @@
> #include <Library/TimerLib.h>
> #include <Library/BaseLib.h>
>
> +/**
> + Calculate TSC frequency.
> +
> + The TSC counting frequency is determined by comparing how far it counts
> + during a 100us period as determined by the ACPI timer. The ACPI timer is
> + used because it counts at a known frequency.
> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY /
> 10000
> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> + difference multiplied by 10000 is the TSC frequency. There will be a small
> + error because of the overhead of reading the ACPI timer. An attempt is
> + made to determine and compensate for this error.
> +
> + @return The number of TSC counts per second.
> +
> +**/
> +UINT64
> +InternalCalculateTscFrequency (
> + VOID
> + );
> +
> //
> // Cached performance counter frequency
> //
> @@ -34,15 +54,8 @@ InternalGetPerformanceCounterFrequency (
> VOID
> )
> {
> - BOOLEAN InterruptState;
> - UINT64 Count;
> -
> if (mPerformanceCounterFrequency == 0) {
> - InterruptState = SaveAndDisableInterrupts ();
> - Count = GetPerformanceCounter ();
> - MicroSecondDelay (100);
> - mPerformanceCounterFrequency = MultU64x32
> (GetPerformanceCounter () - Count, 10000);
> - SetInterruptState (InterruptState);
> + mPerformanceCounterFrequency = InternalCalculateTscFrequency ();
> }
> return mPerformanceCounterFrequency;
> }
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency
2016-08-11 2:37 [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency Star Zeng
2016-08-11 2:42 ` Gao, Liming
@ 2016-08-11 5:01 ` Kinney, Michael D
2016-08-11 13:41 ` Brian J. Johnson
2016-08-12 8:31 ` Paolo Bonzini
3 siblings, 0 replies; 8+ messages in thread
From: Kinney, Michael D @ 2016-08-11 5:01 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org, Kinney, Michael D
Cc: Gao, Liming, Lohr, Paul A
Reviewed-by: : Michael D Kinney <michael.d.kinney@intel.com>
> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, August 10, 2016 7:38 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Gao, Liming <liming.gao@intel.com>; Lohr, Paul A <paul.a.lohr@intel.com>
> Subject: [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency
>
> Minimize the code overhead between the two TSC reads by adding
> new internal API to calculate TSC Frequency instead of reusing
> MicroSecondDelay ().
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Paul A Lohr <paul.a.lohr@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 56 +++++++++++++++++++++-
> .../Library/AcpiTimerLib/BaseAcpiTimerLib.c | 33 ++++++++-----
> .../Library/AcpiTimerLib/DxeAcpiTimerLib.c | 31 ++++++++----
> 3 files changed, 99 insertions(+), 21 deletions(-)
>
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> index 806a4f7ce24c..e6fea231123d 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Timer implements one instance of Timer Library.
>
> - Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> @@ -335,3 +335,57 @@ GetTimeInNanoSecond (
>
> return NanoSeconds;
> }
> +
> +/**
> + Calculate TSC frequency.
> +
> + The TSC counting frequency is determined by comparing how far it counts
> + during a 100us period as determined by the ACPI timer. The ACPI timer is
> + used because it counts at a known frequency.
> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> + difference multiplied by 10000 is the TSC frequency. There will be a small
> + error because of the overhead of reading the ACPI timer. An attempt is
> + made to determine and compensate for this error.
> +
> + @return The number of TSC counts per second.
> +
> +**/
> +UINT64
> +InternalCalculateTscFrequency (
> + VOID
> + )
> +{
> + UINT64 StartTSC;
> + UINT64 EndTSC;
> + UINT16 TimerAddr;
> + UINT32 Ticks;
> + UINT64 TscFrequency;
> + BOOLEAN InterruptState;
> +
> + InterruptState = SaveAndDisableInterrupts ();
> +
> + TimerAddr = InternalAcpiGetAcpiTimerIoPort ();
> + Ticks = IoRead32 (TimerAddr) + (ACPI_TIMER_FREQUENCY / 10000); // Set Ticks to
> 100us in the future
> +
> + StartTSC = AsmReadTsc (); // Get base
> value for the TSC
> + //
> + // Wait until the ACPI timer has counted 100us.
> + // Timer wrap-arounds are handled correctly by this function.
> + // When the current ACPI timer value is greater than 'Ticks', the while loop will
> exit.
> + //
> + while (((Ticks - IoRead32 (TimerAddr)) & BIT23) == 0) {
> + CpuPause();
> + }
> + EndTSC = AsmReadTsc (); // TSC value
> 100us later
> +
> + TscFrequency = MultU64x32 (
> + (EndTSC - StartTSC), // Number of TSC
> counts in 100us
> + 10000 // Number of
> 100us in a second
> + );
> +
> + SetInterruptState (InterruptState);
> +
> + return TscFrequency;
> +}
> +
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> index 21fdb79908b8..8819ebcfccef 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Timer implements one instance of Timer Library.
>
> - Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> @@ -17,6 +17,26 @@
> #include <Library/BaseLib.h>
>
> /**
> + Calculate TSC frequency.
> +
> + The TSC counting frequency is determined by comparing how far it counts
> + during a 100us period as determined by the ACPI timer. The ACPI timer is
> + used because it counts at a known frequency.
> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> + difference multiplied by 10000 is the TSC frequency. There will be a small
> + error because of the overhead of reading the ACPI timer. An attempt is
> + made to determine and compensate for this error.
> +
> + @return The number of TSC counts per second.
> +
> +**/
> +UINT64
> +InternalCalculateTscFrequency (
> + VOID
> + );
> +
> +/**
> Internal function to retrieves the 64-bit frequency in Hz.
>
> Internal function to retrieves the 64-bit frequency in Hz.
> @@ -29,14 +49,5 @@ InternalGetPerformanceCounterFrequency (
> VOID
> )
> {
> - BOOLEAN InterruptState;
> - UINT64 Count;
> - UINT64 Frequency;
> -
> - InterruptState = SaveAndDisableInterrupts ();
> - Count = GetPerformanceCounter ();
> - MicroSecondDelay (100);
> - Frequency = MultU64x32 (GetPerformanceCounter () - Count, 10000);
> - SetInterruptState (InterruptState);
> - return Frequency;
> + return InternalCalculateTscFrequency ();
> }
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> index 6f5c07a4f0b4..7f7b0f8f6294 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Timer implements one instance of Timer Library.
>
> - Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> @@ -16,6 +16,26 @@
> #include <Library/TimerLib.h>
> #include <Library/BaseLib.h>
>
> +/**
> + Calculate TSC frequency.
> +
> + The TSC counting frequency is determined by comparing how far it counts
> + during a 100us period as determined by the ACPI timer. The ACPI timer is
> + used because it counts at a known frequency.
> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> + difference multiplied by 10000 is the TSC frequency. There will be a small
> + error because of the overhead of reading the ACPI timer. An attempt is
> + made to determine and compensate for this error.
> +
> + @return The number of TSC counts per second.
> +
> +**/
> +UINT64
> +InternalCalculateTscFrequency (
> + VOID
> + );
> +
> //
> // Cached performance counter frequency
> //
> @@ -34,15 +54,8 @@ InternalGetPerformanceCounterFrequency (
> VOID
> )
> {
> - BOOLEAN InterruptState;
> - UINT64 Count;
> -
> if (mPerformanceCounterFrequency == 0) {
> - InterruptState = SaveAndDisableInterrupts ();
> - Count = GetPerformanceCounter ();
> - MicroSecondDelay (100);
> - mPerformanceCounterFrequency = MultU64x32 (GetPerformanceCounter () - Count,
> 10000);
> - SetInterruptState (InterruptState);
> + mPerformanceCounterFrequency = InternalCalculateTscFrequency ();
> }
> return mPerformanceCounterFrequency;
> }
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency
2016-08-11 2:37 [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency Star Zeng
2016-08-11 2:42 ` Gao, Liming
2016-08-11 5:01 ` Kinney, Michael D
@ 2016-08-11 13:41 ` Brian J. Johnson
2016-08-12 1:07 ` Zeng, Star
2016-08-12 8:31 ` Paolo Bonzini
3 siblings, 1 reply; 8+ messages in thread
From: Brian J. Johnson @ 2016-08-11 13:41 UTC (permalink / raw)
To: Star Zeng, edk2-devel; +Cc: Michael D Kinney, Liming Gao
On 08/10/2016 09:37 PM, Star Zeng wrote:
> Minimize the code overhead between the two TSC reads by adding
> new internal API to calculate TSC Frequency instead of reusing
> MicroSecondDelay ().
Why not just use the MSR_PLATFORM_INFO (0xce) register bits 15..8, on
CPUs where it's available, to read the TSC frequency directly?
--
Brian J. Johnson
--------------------------------------------------------------------
My statements are my own, are not authorized by SGI, and do not
necessarily represent SGI’s positions.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency
2016-08-11 13:41 ` Brian J. Johnson
@ 2016-08-12 1:07 ` Zeng, Star
0 siblings, 0 replies; 8+ messages in thread
From: Zeng, Star @ 2016-08-12 1:07 UTC (permalink / raw)
To: Brian J. Johnson, edk2-devel; +Cc: Michael D Kinney, Liming Gao
On 2016/8/11 21:41, Brian J. Johnson wrote:
> On 08/10/2016 09:37 PM, Star Zeng wrote:
>> Minimize the code overhead between the two TSC reads by adding
>> new internal API to calculate TSC Frequency instead of reusing
>> MicroSecondDelay ().
>
> Why not just use the MSR_PLATFORM_INFO (0xce) register bits 15..8, on
> CPUs where it's available, to read the TSC frequency directly?
Per my understanding, not every generation CPU has this
MSR_PLATFORM_INFO (0xce) register defined, and also MSR_PLATFORM_INFO
(0xce) register bits 15..8 is just the ratio, then the radio needs to
multiply external clock frequency which may be different for various
generation CPU.
I believe the library's position stands as a generic library and not for
specific CPU.
Thanks,
Star
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency
2016-08-11 2:37 [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency Star Zeng
` (2 preceding siblings ...)
2016-08-11 13:41 ` Brian J. Johnson
@ 2016-08-12 8:31 ` Paolo Bonzini
2016-08-12 8:43 ` Zeng, Star
2016-08-12 18:27 ` Kinney, Michael D
3 siblings, 2 replies; 8+ messages in thread
From: Paolo Bonzini @ 2016-08-12 8:31 UTC (permalink / raw)
To: Star Zeng, edk2-devel; +Cc: Michael D Kinney, Liming Gao
On 11/08/2016 04:37, Star Zeng wrote:
> Minimize the code overhead between the two TSC reads by adding
> new internal API to calculate TSC Frequency instead of reusing
> MicroSecondDelay ().
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Paul A Lohr <paul.a.lohr@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 56 +++++++++++++++++++++-
> .../Library/AcpiTimerLib/BaseAcpiTimerLib.c | 33 ++++++++-----
> .../Library/AcpiTimerLib/DxeAcpiTimerLib.c | 31 ++++++++----
> 3 files changed, 99 insertions(+), 21 deletions(-)
>
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> index 806a4f7ce24c..e6fea231123d 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Timer implements one instance of Timer Library.
>
> - Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> @@ -335,3 +335,57 @@ GetTimeInNanoSecond (
>
> return NanoSeconds;
> }
> +
> +/**
> + Calculate TSC frequency.
> +
> + The TSC counting frequency is determined by comparing how far it counts
> + during a 100us period as determined by the ACPI timer. The ACPI timer is
> + used because it counts at a known frequency.
> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> + difference multiplied by 10000 is the TSC frequency. There will be a small
> + error because of the overhead of reading the ACPI timer. An attempt is
> + made to determine and compensate for this error.
> +
> + @return The number of TSC counts per second.
> +
> +**/
> +UINT64
> +InternalCalculateTscFrequency (
> + VOID
> + )
> +{
> + UINT64 StartTSC;
> + UINT64 EndTSC;
> + UINT16 TimerAddr;
> + UINT32 Ticks;
> + UINT64 TscFrequency;
> + BOOLEAN InterruptState;
> +
> + InterruptState = SaveAndDisableInterrupts ();
> +
> + TimerAddr = InternalAcpiGetAcpiTimerIoPort ();
> + Ticks = IoRead32 (TimerAddr) + (ACPI_TIMER_FREQUENCY / 10000); // Set Ticks to 100us in the future
ACPI_TIMER_FREQUENCY is 3579545, thus you're waiting 357 ticks but the
actual result of the division is much closer to 358. The error is only
0.26%, but it's so simple to reduce it that I think it's worth it. Just
change (ACPI_TIMER_FREQUENCY / 10000) to (ACPI_TIMER_FREQUENCY + 5000) /
10000.
Another possibility is to count 343 ticks and multiply by 10436; 343 *
10436 is almost exactly ACPI_TIMER_FREQUENCY.
Paolo
> + StartTSC = AsmReadTsc (); // Get base value for the TSC
> + //
> + // Wait until the ACPI timer has counted 100us.
> + // Timer wrap-arounds are handled correctly by this function.
> + // When the current ACPI timer value is greater than 'Ticks', the while loop will exit.
> + //
> + while (((Ticks - IoRead32 (TimerAddr)) & BIT23) == 0) {
> + CpuPause();
> + }
> + EndTSC = AsmReadTsc (); // TSC value 100us later
> +
> + TscFrequency = MultU64x32 (
> + (EndTSC - StartTSC), // Number of TSC counts in 100us
> + 10000 // Number of 100us in a second
> + );
> +
> + SetInterruptState (InterruptState);
> +
> + return TscFrequency;
> +}
> +
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> index 21fdb79908b8..8819ebcfccef 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Timer implements one instance of Timer Library.
>
> - Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> @@ -17,6 +17,26 @@
> #include <Library/BaseLib.h>
>
> /**
> + Calculate TSC frequency.
> +
> + The TSC counting frequency is determined by comparing how far it counts
> + during a 100us period as determined by the ACPI timer. The ACPI timer is
> + used because it counts at a known frequency.
> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> + difference multiplied by 10000 is the TSC frequency. There will be a small
> + error because of the overhead of reading the ACPI timer. An attempt is
> + made to determine and compensate for this error.
> +
> + @return The number of TSC counts per second.
> +
> +**/
> +UINT64
> +InternalCalculateTscFrequency (
> + VOID
> + );
> +
> +/**
> Internal function to retrieves the 64-bit frequency in Hz.
>
> Internal function to retrieves the 64-bit frequency in Hz.
> @@ -29,14 +49,5 @@ InternalGetPerformanceCounterFrequency (
> VOID
> )
> {
> - BOOLEAN InterruptState;
> - UINT64 Count;
> - UINT64 Frequency;
> -
> - InterruptState = SaveAndDisableInterrupts ();
> - Count = GetPerformanceCounter ();
> - MicroSecondDelay (100);
> - Frequency = MultU64x32 (GetPerformanceCounter () - Count, 10000);
> - SetInterruptState (InterruptState);
> - return Frequency;
> + return InternalCalculateTscFrequency ();
> }
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> index 6f5c07a4f0b4..7f7b0f8f6294 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Timer implements one instance of Timer Library.
>
> - Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> @@ -16,6 +16,26 @@
> #include <Library/TimerLib.h>
> #include <Library/BaseLib.h>
>
> +/**
> + Calculate TSC frequency.
> +
> + The TSC counting frequency is determined by comparing how far it counts
> + during a 100us period as determined by the ACPI timer. The ACPI timer is
> + used because it counts at a known frequency.
> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> + difference multiplied by 10000 is the TSC frequency. There will be a small
> + error because of the overhead of reading the ACPI timer. An attempt is
> + made to determine and compensate for this error.
> +
> + @return The number of TSC counts per second.
> +
> +**/
> +UINT64
> +InternalCalculateTscFrequency (
> + VOID
> + );
> +
> //
> // Cached performance counter frequency
> //
> @@ -34,15 +54,8 @@ InternalGetPerformanceCounterFrequency (
> VOID
> )
> {
> - BOOLEAN InterruptState;
> - UINT64 Count;
> -
> if (mPerformanceCounterFrequency == 0) {
> - InterruptState = SaveAndDisableInterrupts ();
> - Count = GetPerformanceCounter ();
> - MicroSecondDelay (100);
> - mPerformanceCounterFrequency = MultU64x32 (GetPerformanceCounter () - Count, 10000);
> - SetInterruptState (InterruptState);
> + mPerformanceCounterFrequency = InternalCalculateTscFrequency ();
> }
> return mPerformanceCounterFrequency;
> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency
2016-08-12 8:31 ` Paolo Bonzini
@ 2016-08-12 8:43 ` Zeng, Star
2016-08-12 18:27 ` Kinney, Michael D
1 sibling, 0 replies; 8+ messages in thread
From: Zeng, Star @ 2016-08-12 8:43 UTC (permalink / raw)
To: Paolo Bonzini, edk2-devel; +Cc: Michael D Kinney, Liming Gao
On 2016/8/12 16:31, Paolo Bonzini wrote:
>
>
> On 11/08/2016 04:37, Star Zeng wrote:
>> Minimize the code overhead between the two TSC reads by adding
>> new internal API to calculate TSC Frequency instead of reusing
>> MicroSecondDelay ().
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Paul A Lohr <paul.a.lohr@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Star Zeng <star.zeng@intel.com>
>> ---
>> PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 56 +++++++++++++++++++++-
>> .../Library/AcpiTimerLib/BaseAcpiTimerLib.c | 33 ++++++++-----
>> .../Library/AcpiTimerLib/DxeAcpiTimerLib.c | 31 ++++++++----
>> 3 files changed, 99 insertions(+), 21 deletions(-)
>>
>> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
>> index 806a4f7ce24c..e6fea231123d 100644
>> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
>> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
>> @@ -1,7 +1,7 @@
>> /** @file
>> ACPI Timer implements one instance of Timer Library.
>>
>> - Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.<BR>
>> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
>> This program and the accompanying materials
>> are licensed and made available under the terms and conditions of the BSD License
>> which accompanies this distribution. The full text of the license may be found at
>> @@ -335,3 +335,57 @@ GetTimeInNanoSecond (
>>
>> return NanoSeconds;
>> }
>> +
>> +/**
>> + Calculate TSC frequency.
>> +
>> + The TSC counting frequency is determined by comparing how far it counts
>> + during a 100us period as determined by the ACPI timer. The ACPI timer is
>> + used because it counts at a known frequency.
>> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
>> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
>> + difference multiplied by 10000 is the TSC frequency. There will be a small
>> + error because of the overhead of reading the ACPI timer. An attempt is
>> + made to determine and compensate for this error.
>> +
>> + @return The number of TSC counts per second.
>> +
>> +**/
>> +UINT64
>> +InternalCalculateTscFrequency (
>> + VOID
>> + )
>> +{
>> + UINT64 StartTSC;
>> + UINT64 EndTSC;
>> + UINT16 TimerAddr;
>> + UINT32 Ticks;
>> + UINT64 TscFrequency;
>> + BOOLEAN InterruptState;
>> +
>> + InterruptState = SaveAndDisableInterrupts ();
>> +
>> + TimerAddr = InternalAcpiGetAcpiTimerIoPort ();
>> + Ticks = IoRead32 (TimerAddr) + (ACPI_TIMER_FREQUENCY / 10000); // Set Ticks to 100us in the future
>
> ACPI_TIMER_FREQUENCY is 3579545, thus you're waiting 357 ticks but the
> actual result of the division is much closer to 358. The error is only
> 0.26%, but it's so simple to reduce it that I think it's worth it. Just
> change (ACPI_TIMER_FREQUENCY / 10000) to (ACPI_TIMER_FREQUENCY + 5000) /
> 10000.
Paolo,
It is nice to have it I think, and I prefer to use (ACPI_TIMER_FREQUENCY
+ 5000) / 10000.
Since I have pushed this patch, do you mind to contribute the proposal
with a new patch?
Thanks,
Star
>
> Another possibility is to count 343 ticks and multiply by 10436; 343 *
> 10436 is almost exactly ACPI_TIMER_FREQUENCY.
>
> Paolo
>
>> + StartTSC = AsmReadTsc (); // Get base value for the TSC
>> + //
>> + // Wait until the ACPI timer has counted 100us.
>> + // Timer wrap-arounds are handled correctly by this function.
>> + // When the current ACPI timer value is greater than 'Ticks', the while loop will exit.
>> + //
>> + while (((Ticks - IoRead32 (TimerAddr)) & BIT23) == 0) {
>> + CpuPause();
>> + }
>> + EndTSC = AsmReadTsc (); // TSC value 100us later
>> +
>> + TscFrequency = MultU64x32 (
>> + (EndTSC - StartTSC), // Number of TSC counts in 100us
>> + 10000 // Number of 100us in a second
>> + );
>> +
>> + SetInterruptState (InterruptState);
>> +
>> + return TscFrequency;
>> +}
>> +
>> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
>> index 21fdb79908b8..8819ebcfccef 100644
>> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
>> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
>> @@ -1,7 +1,7 @@
>> /** @file
>> ACPI Timer implements one instance of Timer Library.
>>
>> - Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
>> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
>> This program and the accompanying materials
>> are licensed and made available under the terms and conditions of the BSD License
>> which accompanies this distribution. The full text of the license may be found at
>> @@ -17,6 +17,26 @@
>> #include <Library/BaseLib.h>
>>
>> /**
>> + Calculate TSC frequency.
>> +
>> + The TSC counting frequency is determined by comparing how far it counts
>> + during a 100us period as determined by the ACPI timer. The ACPI timer is
>> + used because it counts at a known frequency.
>> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
>> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
>> + difference multiplied by 10000 is the TSC frequency. There will be a small
>> + error because of the overhead of reading the ACPI timer. An attempt is
>> + made to determine and compensate for this error.
>> +
>> + @return The number of TSC counts per second.
>> +
>> +**/
>> +UINT64
>> +InternalCalculateTscFrequency (
>> + VOID
>> + );
>> +
>> +/**
>> Internal function to retrieves the 64-bit frequency in Hz.
>>
>> Internal function to retrieves the 64-bit frequency in Hz.
>> @@ -29,14 +49,5 @@ InternalGetPerformanceCounterFrequency (
>> VOID
>> )
>> {
>> - BOOLEAN InterruptState;
>> - UINT64 Count;
>> - UINT64 Frequency;
>> -
>> - InterruptState = SaveAndDisableInterrupts ();
>> - Count = GetPerformanceCounter ();
>> - MicroSecondDelay (100);
>> - Frequency = MultU64x32 (GetPerformanceCounter () - Count, 10000);
>> - SetInterruptState (InterruptState);
>> - return Frequency;
>> + return InternalCalculateTscFrequency ();
>> }
>> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
>> index 6f5c07a4f0b4..7f7b0f8f6294 100644
>> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
>> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
>> @@ -1,7 +1,7 @@
>> /** @file
>> ACPI Timer implements one instance of Timer Library.
>>
>> - Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
>> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
>> This program and the accompanying materials
>> are licensed and made available under the terms and conditions of the BSD License
>> which accompanies this distribution. The full text of the license may be found at
>> @@ -16,6 +16,26 @@
>> #include <Library/TimerLib.h>
>> #include <Library/BaseLib.h>
>>
>> +/**
>> + Calculate TSC frequency.
>> +
>> + The TSC counting frequency is determined by comparing how far it counts
>> + during a 100us period as determined by the ACPI timer. The ACPI timer is
>> + used because it counts at a known frequency.
>> + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
>> + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
>> + difference multiplied by 10000 is the TSC frequency. There will be a small
>> + error because of the overhead of reading the ACPI timer. An attempt is
>> + made to determine and compensate for this error.
>> +
>> + @return The number of TSC counts per second.
>> +
>> +**/
>> +UINT64
>> +InternalCalculateTscFrequency (
>> + VOID
>> + );
>> +
>> //
>> // Cached performance counter frequency
>> //
>> @@ -34,15 +54,8 @@ InternalGetPerformanceCounterFrequency (
>> VOID
>> )
>> {
>> - BOOLEAN InterruptState;
>> - UINT64 Count;
>> -
>> if (mPerformanceCounterFrequency == 0) {
>> - InterruptState = SaveAndDisableInterrupts ();
>> - Count = GetPerformanceCounter ();
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency
2016-08-12 8:31 ` Paolo Bonzini
2016-08-12 8:43 ` Zeng, Star
@ 2016-08-12 18:27 ` Kinney, Michael D
1 sibling, 0 replies; 8+ messages in thread
From: Kinney, Michael D @ 2016-08-12 18:27 UTC (permalink / raw)
To: Paolo Bonzini, Zeng, Star, edk2-devel@lists.01.org,
Kinney, Michael D
Cc: Gao, Liming
Paolo,
That is a good idea to find factors of ACPI_TIMER_FREQUENCY 3579545 Hz
and find a value that is close to 100 uS for calibration. The pair you
found 343 * 10436 = 3579548 Hz. If we look at frequencies lower and
higher than ACPI_TIMER_FREQUENCY, the value 3579543 has many more factors
and is only 2 Hz from the target.
Number Factors
======= ========
3579542 1, 2, 67, 134, 26713, 53426, 1789771, 3579542
3579543 1, 3, 9, 11, 19, 33, 57, 99, 121, 171, 173, 209, 363,
519, 627, 1089, 1557, 1881, 1903, 2299, 3287, 5709,
6897, 9861, 17127, 20691, 20933, 29583, 36157, 62799,
108471, 188397, 325413, 397727, 1193181, 3579543
3579544 1, 2, 4, 8, 447443, 894886, 1789772, 3579544
3579545 1, 5, 715909, 3579545
3579546 1, 2, 3, 6, 41, 82, 123, 246, 14551, 29102, 43653,
87306, 596591, 1193182, 1789773, 3579546
3579547 1, 3579547
3579548 1, 2, 4, 7, 14, 28, 49, 98, 196, 343, 686, 1372, 2609,
5218, 10436, 18263, 36526, 73052, 127841, 255682, 511364,
894887, 1789774, 3579548
So we could choose from the following pairs for calibration:
209 * 17127 209 ticks is 58.3 uS calibration time
363 * 9861 363 ticks is 101.4 uS calibration time
519 * 6897 519 ticks is 145.0 uS calibration time
627 * 5709 627 ticks is 175.2 uS calibration time
I would recommend the pair 363 * 9861 which is closest to the
Current 100 uS calibration time.
Another accuracy improvement on the current algorithm is to
Wait for the ACPI timer value to change before capturing the
Initial TSC value, so only whole ticks are counted. The current
algorithm could start the beginning, middle, or end of an ACPI
counter tick.
Here is an updated measurement algorithm.
//
// Wait for ACPI timer to start next count
//
Ticks = IoRead32 (TimerAddr);
while (Ticks == IoRead32 (TimerAddr)) {
CpuPause();
}
//
// Immediately capture start TSC value
//
StartTSC = AsmReadTsc ();
//
// Compute the number of ticks to wait to measure TSC frequency.
// Use 363 * 9861 = 3579543 Hz which is within 2 Hz of
// ACPI_TIMER_FREQUENCY. 363 counts is a calibration time of
// 101.4 uS.
// Subtract 1 because the calibration loop waits one extra count.
//
Ticks = IoRead32 (TimerAddr) + 363 - 1;
//
// Wait until the ACPI timer has counted the number of calibration ticks.
// Timer wrap-arounds are handled correctly by this function.
// When the current ACPI timer value is greater than 'Ticks',
// the while loop will exit.
//
while (((Ticks - IoRead32 (TimerAddr)) & BIT23) == 0) {
CpuPause();
}
//
// Immediately capture end TSC value
//
EndTSC = AsmReadTsc (); // TSC value 363 ticks later
TscFrequency = MultU64x32 (
(EndTSC - StartTSC), // Total number of TSC counts
9861
);
We would have to run some experiments to see if this further improves the
accuracy and consistency of the measurements.
Mike
> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Friday, August 12, 2016 1:31 AM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC
> Frequency
>
>
>
> On 11/08/2016 04:37, Star Zeng wrote:
> > Minimize the code overhead between the two TSC reads by adding
> > new internal API to calculate TSC Frequency instead of reusing
> > MicroSecondDelay ().
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Paul A Lohr <paul.a.lohr@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > ---
> > PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 56 +++++++++++++++++++++-
> > .../Library/AcpiTimerLib/BaseAcpiTimerLib.c | 33 ++++++++-----
> > .../Library/AcpiTimerLib/DxeAcpiTimerLib.c | 31 ++++++++----
> > 3 files changed, 99 insertions(+), 21 deletions(-)
> >
> > diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> > index 806a4f7ce24c..e6fea231123d 100644
> > --- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> > +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> > @@ -1,7 +1,7 @@
> > /** @file
> > ACPI Timer implements one instance of Timer Library.
> >
> > - Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of the BSD
> License
> > which accompanies this distribution. The full text of the license may be found
> at
> > @@ -335,3 +335,57 @@ GetTimeInNanoSecond (
> >
> > return NanoSeconds;
> > }
> > +
> > +/**
> > + Calculate TSC frequency.
> > +
> > + The TSC counting frequency is determined by comparing how far it counts
> > + during a 100us period as determined by the ACPI timer. The ACPI timer is
> > + used because it counts at a known frequency.
> > + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
> > + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> > + difference multiplied by 10000 is the TSC frequency. There will be a small
> > + error because of the overhead of reading the ACPI timer. An attempt is
> > + made to determine and compensate for this error.
> > +
> > + @return The number of TSC counts per second.
> > +
> > +**/
> > +UINT64
> > +InternalCalculateTscFrequency (
> > + VOID
> > + )
> > +{
> > + UINT64 StartTSC;
> > + UINT64 EndTSC;
> > + UINT16 TimerAddr;
> > + UINT32 Ticks;
> > + UINT64 TscFrequency;
> > + BOOLEAN InterruptState;
> > +
> > + InterruptState = SaveAndDisableInterrupts ();
> > +
> > + TimerAddr = InternalAcpiGetAcpiTimerIoPort ();
> > + Ticks = IoRead32 (TimerAddr) + (ACPI_TIMER_FREQUENCY / 10000); // Set Ticks
> to 100us in the future
>
> ACPI_TIMER_FREQUENCY is 3579545, thus you're waiting 357 ticks but the
> actual result of the division is much closer to 358. The error is only
> 0.26%, but it's so simple to reduce it that I think it's worth it. Just
> change (ACPI_TIMER_FREQUENCY / 10000) to (ACPI_TIMER_FREQUENCY + 5000) /
> 10000.
>
> Another possibility is to count 343 ticks and multiply by 10436; 343 *
> 10436 is almost exactly ACPI_TIMER_FREQUENCY.
>
> Paolo
>
> > + StartTSC = AsmReadTsc (); // Get base
> value for the TSC
> > + //
> > + // Wait until the ACPI timer has counted 100us.
> > + // Timer wrap-arounds are handled correctly by this function.
> > + // When the current ACPI timer value is greater than 'Ticks', the while loop
> will exit.
> > + //
> > + while (((Ticks - IoRead32 (TimerAddr)) & BIT23) == 0) {
> > + CpuPause();
> > + }
> > + EndTSC = AsmReadTsc (); // TSC value
> 100us later
> > +
> > + TscFrequency = MultU64x32 (
> > + (EndTSC - StartTSC), // Number of
> TSC counts in 100us
> > + 10000 // Number of
> 100us in a second
> > + );
> > +
> > + SetInterruptState (InterruptState);
> > +
> > + return TscFrequency;
> > +}
> > +
> > diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> > index 21fdb79908b8..8819ebcfccef 100644
> > --- a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> > +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> > @@ -1,7 +1,7 @@
> > /** @file
> > ACPI Timer implements one instance of Timer Library.
> >
> > - Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of the BSD
> License
> > which accompanies this distribution. The full text of the license may be found
> at
> > @@ -17,6 +17,26 @@
> > #include <Library/BaseLib.h>
> >
> > /**
> > + Calculate TSC frequency.
> > +
> > + The TSC counting frequency is determined by comparing how far it counts
> > + during a 100us period as determined by the ACPI timer. The ACPI timer is
> > + used because it counts at a known frequency.
> > + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
> > + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> > + difference multiplied by 10000 is the TSC frequency. There will be a small
> > + error because of the overhead of reading the ACPI timer. An attempt is
> > + made to determine and compensate for this error.
> > +
> > + @return The number of TSC counts per second.
> > +
> > +**/
> > +UINT64
> > +InternalCalculateTscFrequency (
> > + VOID
> > + );
> > +
> > +/**
> > Internal function to retrieves the 64-bit frequency in Hz.
> >
> > Internal function to retrieves the 64-bit frequency in Hz.
> > @@ -29,14 +49,5 @@ InternalGetPerformanceCounterFrequency (
> > VOID
> > )
> > {
> > - BOOLEAN InterruptState;
> > - UINT64 Count;
> > - UINT64 Frequency;
> > -
> > - InterruptState = SaveAndDisableInterrupts ();
> > - Count = GetPerformanceCounter ();
> > - MicroSecondDelay (100);
> > - Frequency = MultU64x32 (GetPerformanceCounter () - Count, 10000);
> > - SetInterruptState (InterruptState);
> > - return Frequency;
> > + return InternalCalculateTscFrequency ();
> > }
> > diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> > index 6f5c07a4f0b4..7f7b0f8f6294 100644
> > --- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> > +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> > @@ -1,7 +1,7 @@
> > /** @file
> > ACPI Timer implements one instance of Timer Library.
> >
> > - Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of the BSD
> License
> > which accompanies this distribution. The full text of the license may be found
> at
> > @@ -16,6 +16,26 @@
> > #include <Library/TimerLib.h>
> > #include <Library/BaseLib.h>
> >
> > +/**
> > + Calculate TSC frequency.
> > +
> > + The TSC counting frequency is determined by comparing how far it counts
> > + during a 100us period as determined by the ACPI timer. The ACPI timer is
> > + used because it counts at a known frequency.
> > + The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 10000
> > + clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> > + difference multiplied by 10000 is the TSC frequency. There will be a small
> > + error because of the overhead of reading the ACPI timer. An attempt is
> > + made to determine and compensate for this error.
> > +
> > + @return The number of TSC counts per second.
> > +
> > +**/
> > +UINT64
> > +InternalCalculateTscFrequency (
> > + VOID
> > + );
> > +
> > //
> > // Cached performance counter frequency
> > //
> > @@ -34,15 +54,8 @@ InternalGetPerformanceCounterFrequency (
> > VOID
> > )
> > {
> > - BOOLEAN InterruptState;
> > - UINT64 Count;
> > -
> > if (mPerformanceCounterFrequency == 0) {
> > - InterruptState = SaveAndDisableInterrupts ();
> > - Count = GetPerformanceCounter ();
> > - MicroSecondDelay (100);
> > - mPerformanceCounterFrequency = MultU64x32 (GetPerformanceCounter () - Count,
> 10000);
> > - SetInterruptState (InterruptState);
> > + mPerformanceCounterFrequency = InternalCalculateTscFrequency ();
> > }
> > return mPerformanceCounterFrequency;
> > }
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-08-12 18:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-11 2:37 [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency Star Zeng
2016-08-11 2:42 ` Gao, Liming
2016-08-11 5:01 ` Kinney, Michael D
2016-08-11 13:41 ` Brian J. Johnson
2016-08-12 1:07 ` Zeng, Star
2016-08-12 8:31 ` Paolo Bonzini
2016-08-12 8:43 ` Zeng, Star
2016-08-12 18:27 ` Kinney, Michael D
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox