From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by ml01.01.org (Postfix) with ESMTP id 1FFB01A1DF2 for ; Wed, 17 Aug 2016 08:47:35 -0700 (PDT) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga101.jf.intel.com with ESMTP; 17 Aug 2016 08:47:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,529,1464678000"; d="scan'208";a="1042856890" Received: from orsmsx106.amr.corp.intel.com ([10.22.225.133]) by fmsmga002.fm.intel.com with ESMTP; 17 Aug 2016 08:47:34 -0700 Received: from orsmsx152.amr.corp.intel.com (10.22.226.39) by ORSMSX106.amr.corp.intel.com (10.22.225.133) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 17 Aug 2016 08:47:34 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.118]) by ORSMSX152.amr.corp.intel.com ([169.254.8.227]) with mapi id 14.03.0248.002; Wed, 17 Aug 2016 08:47:33 -0700 From: "Kinney, Michael D" To: Paolo Bonzini , "Zeng, Star" , "edk2-devel@lists.01.org" , "Kinney, Michael D" CC: "Gao, Liming" Thread-Topic: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Wait 363 ACPI timer counts to get TSC Freq Thread-Index: AQHR+C7Zo8HEzrhiFkiFTuRav2afq6BNae4A///iHMA= Date: Wed, 17 Aug 2016 15:47:33 +0000 Message-ID: References: <1471400803-129280-1-git-send-email-star.zeng@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDg2YWFjN2ItN2U3Mi00NzEyLTk3MTYtNzdjZTJhMTNmMGRlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6ImpXbk1nZVdXSDBBWFVEZXpPVWtDYXJRYjUxNkptbnZPcjRPcTlKblJaSWM9In0= x-originating-ip: [10.22.254.139] MIME-Version: 1.0 Subject: Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Wait 363 ACPI timer counts to get TSC Freq 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: Wed, 17 Aug 2016 15:47:35 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Paolo, We did some experiments to wait for the next ACPI Timer tick, and they did= =20 not improve the results. On real HW, the I/O Port read of the ACPI Timer takes ~1 uS, and an ACPI Ti= mer tick is about 0.25 uS, so the action to read the counter takes 4 times long= er than a single count. If the I/O port read was much faster than the tick period, then adding the = loop would make sense. Mike > -----Original Message----- > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo B= onzini > Sent: Wednesday, August 17, 2016 3:31 AM > To: Zeng, Star ; edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Gao, Liming > > Subject: Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Wait 363 ACPI ti= mer counts > to get TSC Freq >=20 >=20 >=20 > On 17/08/2016 04:26, Star Zeng wrote: > > Compute the number of ticks to wait to measure TSC frequency. > > Instead of (ACPI_TIMER_FREQUENCY / 10000) =3D 357 and 357 * 10000 =3D 3= 570000, > > use 363 * 9861 =3D 3579543 Hz which is within 2 Hz of ACPI_TIMER_FREQUE= NCY. > > 363 counts is a calibration time of 101.4 uS. > > > > The idea comes from Michael and Paolo. > > > > Cc: Michael D Kinney > > Cc: Liming Gao > > Cc: Paolo Bonzini > > Cc: Paul A Lohr > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Star Zeng > > --- > > PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 32 +++++++++++++-= -------- > > .../Library/AcpiTimerLib/BaseAcpiTimerLib.c | 14 +++++----- > > .../Library/AcpiTimerLib/DxeAcpiTimerLib.c | 14 +++++----- > > 3 files changed, 33 insertions(+), 27 deletions(-) > > > > diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c > b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c > > index e6fea231123d..020031e3f4a5 100644 > > --- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c > > +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c > > @@ -340,13 +340,13 @@ GetTimeInNanoSecond ( > > Calculate TSC frequency. > > > > The TSC counting frequency is determined by comparing how far it cou= nts > > - during a 100us period as determined by the ACPI timer. The ACPI time= r is > > - used because it counts at a known frequency. > > - The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 1= 0000 > > - clocks of the ACPI timer, or 100us. The TSC is then sampled again. T= he > > - 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. > > + during a 101.4 us 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 363 counts of the ACPI timer= , > > + or 101.4 us. The TSC is then sampled again. The difference multiplie= d by > > + 9861 is the TSC frequency. There will be a small error because of th= e > > + 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. > > > > @@ -366,22 +366,28 @@ InternalCalculateTscFrequency ( > > InterruptState =3D SaveAndDisableInterrupts (); > > > > TimerAddr =3D InternalAcpiGetAcpiTimerIoPort (); > > - Ticks =3D IoRead32 (TimerAddr) + (ACPI_TIMER_FREQUENCY / 10000); = // Set Ticks > to 100us in the future > > + // > > + // Compute the number of ticks to wait to measure TSC frequency. > > + // Use 363 * 9861 =3D 3579543 Hz which is within 2 Hz of ACPI_TIMER_= FREQUENCY. > > + // 363 counts is a calibration time of 101.4 uS. > > + // > > + Ticks =3D IoRead32 (TimerAddr) + 363; > > > > StartTSC =3D AsmReadTsc (); = // Get base > value for the TSC > > // > > - // Wait until the ACPI timer has counted 100us. > > + // Wait until the ACPI timer has counted 101.4 us. > > // Timer wrap-arounds are handled correctly by this function. > > - // When the current ACPI timer value is greater than 'Ticks', the wh= ile loop > will exit. > > + // When the current ACPI timer value is greater than 'Ticks', > > + // the while loop will exit. > > // > > while (((Ticks - IoRead32 (TimerAddr)) & BIT23) =3D=3D 0) { > > CpuPause(); > > } > > - EndTSC =3D AsmReadTsc (); = // TSC value > 100us later > > + EndTSC =3D AsmReadTsc (); = // TSC value > 101.4 us later > > > > TscFrequency =3D MultU64x32 ( > > - (EndTSC - StartTSC), //= Number of > TSC counts in 100us > > - 10000 //= Number of > 100us in a second > > + (EndTSC - StartTSC), //= Number of > TSC counts in 101.4 us > > + 9861 //= Number of > 101.4 us in a second > > ); > > > > SetInterruptState (InterruptState); > > diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c > b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c > > index 8819ebcfccef..29521f8b220b 100644 > > --- a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c > > +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c > > @@ -20,13 +20,13 @@ > > Calculate TSC frequency. > > > > The TSC counting frequency is determined by comparing how far it cou= nts > > - during a 100us period as determined by the ACPI timer. The ACPI time= r is > > - used because it counts at a known frequency. > > - The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 1= 0000 > > - clocks of the ACPI timer, or 100us. The TSC is then sampled again. T= he > > - 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. > > + during a 101.4 us 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 363 counts of the ACPI timer= , > > + or 101.4 us. The TSC is then sampled again. The difference multiplie= d by > > + 9861 is the TSC frequency. There will be a small error because of th= e > > + 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. > > > > diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c > b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c > > index 7f7b0f8f6294..b141c680fb82 100644 > > --- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c > > +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c > > @@ -20,13 +20,13 @@ > > Calculate TSC frequency. > > > > The TSC counting frequency is determined by comparing how far it cou= nts > > - during a 100us period as determined by the ACPI timer. The ACPI time= r is > > - used because it counts at a known frequency. > > - The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 1= 0000 > > - clocks of the ACPI timer, or 100us. The TSC is then sampled again. T= he > > - 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. > > + during a 101.4 us 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 363 counts of the ACPI timer= , > > + or 101.4 us. The TSC is then sampled again. The difference multiplie= d by > > + 9861 is the TSC frequency. There will be a small error because of th= e > > + 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. > > > > >=20 > Reviewed-by: Paolo Bonzini >=20 > On top of this you can add the logic to wait for the beginning of a tick > before calling rdtsc. >=20 > Paolo