From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x242.google.com (mail-wm0-x242.google.com [IPv6:2a00:1450:400c:c09::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 075A51A1DFE for ; Fri, 12 Aug 2016 01:31:13 -0700 (PDT) Received: by mail-wm0-x242.google.com with SMTP id q128so1563406wma.1 for ; Fri, 12 Aug 2016 01:31:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=fiz++goyAx4H1wzgi1KdfFuSfCM0pV2v5NCiSU/v6Nc=; b=Lk05SfdiFmSMUITEPA6NNmXykbIjPk21EXYzk0ITpjgKkRMw25LhPmY9fKwSEIK66m HGNKe8Nra5ydzJ9UQwSfz1usyb/XPpDcLsGQDwLgfFV+BI+LyuYQNhMlr0/6GLr8mwYh JFVJ3HoFS5iEta04PqW4fi5qqmtpML1U2meusNVYiFL8bschGPCKElVQYqJdRDOEP0Iw q+uZZw5HiNNXIB1DSf+/8f7iY6L/t7t+SN1q/LdTcCSgBzO4xCVK6O0cAgQGIzewYqLs iiEuyShWMfI1fiEiezgrcJci1uYHTIfJGa58g0snlt/1k7ZNh/edabz1UZ+8zQ6QKTZp AlQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:subject:to:references:cc:from:message-id :date:user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=fiz++goyAx4H1wzgi1KdfFuSfCM0pV2v5NCiSU/v6Nc=; b=UChRUeZc+0EM8zj7wUG9j37TZ2ZvKixcszbaQtd0LBukBr47AJsp9Jc47uBnk9lgU9 9FEVcJRGL8+BiiEojI2pANSpHBWaLOW02TJ2Yx7/qgx6Qcs3/oeLu69v6qIsQhZoM6Tw 7I80gcbCjK8uRZQlo/L5F8m3g1M2smB+NBUQUbNVw/6kedPLH4vGrvmhbHJAyyZlKkS/ oZQcGzd+vAZEQhlZ+ZI8feT0B6qULacYsiDc7hRa5+JqLdCqyDYir6b37zVjAYjxW5lt KMS10UL24TXP1CpZ1bkgIWxZd7IbXvaBkkcQ39Vug1XvIf+mT3ebDFNKyWUC+IUR15Vw TbSw== X-Gm-Message-State: AEkoouvfgMJYyyK4Z3ckbI4MlIQzFAp6vrc5SKLa/UhMM80ENB+6S9bmPLAgByGJ6dg6CA== X-Received: by 10.28.87.3 with SMTP id l3mr1732688wmb.71.1470990671215; Fri, 12 Aug 2016 01:31:11 -0700 (PDT) Received: from [10.201.49.221] (nat-pool-mxp-u.redhat.com. [149.6.153.187]) by smtp.googlemail.com with ESMTPSA id 3sm1483880wms.1.2016.08.12.01.31.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 12 Aug 2016 01:31:09 -0700 (PDT) Sender: Paolo Bonzini To: Star Zeng , edk2-devel@lists.01.org References: <1470883079-4472-1-git-send-email-star.zeng@intel.com> Cc: Michael D Kinney , Liming Gao From: Paolo Bonzini Message-ID: <0229321a-849c-b264-7b26-146d6608c754@redhat.com> Date: Fri, 12 Aug 2016 10:31:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1470883079-4472-1-git-send-email-star.zeng@intel.com> Subject: Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Aug 2016 08:31:13 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit 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 > Cc: Liming Gao > Cc: Paul A Lohr > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Star Zeng > --- > 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.
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
> 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.
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
> 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 > > /** > + 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.
> + Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
> 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 > #include > > +/** > + 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; > } >