From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.24; helo=mga09.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 72FDE220E8DE4 for ; Tue, 1 May 2018 22:12:05 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 May 2018 22:12:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,353,1520924400"; d="scan'208";a="225043320" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga006.fm.intel.com with ESMTP; 01 May 2018 22:12:05 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 1 May 2018 22:12:05 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.240]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.40]) with mapi id 14.03.0319.002; Wed, 2 May 2018 13:12:03 +0800 From: "Ni, Ruiyu" To: "Zeng, Star" , "edk2-devel@lists.01.org" Thread-Topic: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io overhead when polling Thread-Index: AQHT4cfePwBBnucPPEOhwNAYCPaPlqQb455g Date: Wed, 2 May 2018 05:12:02 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5BC84B22@SHSMSX104.ccr.corp.intel.com> References: <20180426022356.66540-1-ruiyu.ni@intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103BAE658E@shsmsx102.ccr.corp.intel.com> In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103BAE658E@shsmsx102.ccr.corp.intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io overhead when polling X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 May 2018 05:12:06 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable If Multiplicand * Multiplier + Remainder =3D MAX_UINT64, Even Multiplicand =3D MAX_UINT64 / Multiplier, Overflow still happens. So ">=3D" is used here. Thanks/Ray > -----Original Message----- > From: Zeng, Star > Sent: Wednesday, May 2, 2018 11:44 AM > To: Ni, Ruiyu ; edk2-devel@lists.01.org > Cc: Chiu, Chasel ; Zeng, Star > Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io > overhead when polling >=20 > Is it more accurate > if (Multiplicand >=3D DivU64x64Remainder (MAX_UINT64, Multiplier, NULL)) = { > -> > if (Multiplicand > DivU64x64Remainder (MAX_UINT64, Multiplier, NULL)) { >=20 >=20 > Thanks, > Star > -----Original Message----- > From: Ni, Ruiyu > Sent: Thursday, April 26, 2018 10:24 AM > To: edk2-devel@lists.01.org > Cc: Zeng, Star ; Chiu, Chasel > Subject: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io > overhead when polling >=20 > RootBridgeIo.PollMem()/PollIo() originally don't count the IO/MMIO access > overhead when delaying. > The patch changes the implementation to count the access overhead so that > the actually delay equals to user required delay. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ruiyu Ni > Cc: Star Zeng > Cc: Chasel Chiu > --- > .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 3 +- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 3 +- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 151 +++++++++++++++= - > ----- > 3 files changed, 115 insertions(+), 42 deletions(-) >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf > index 42bd8a23cb..2e56959a8f 100644 > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf > @@ -1,7 +1,7 @@ > ## @file > # Generic PCI Host Bridge driver. > # > -# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved. > +# Copyright (c) 2009 - 2018, 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 @@ -43,6 +43,= 7 > @@ [LibraryClasses] > PciSegmentLib > UefiLib > PciHostBridgeLib > + TimerLib >=20 > [Protocols] > gEfiMetronomeArchProtocolGuid ## CONSUMES > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h > index d3dfb57fc6..e2f651aee4 100644 > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h > @@ -2,7 +2,7 @@ >=20 > The PCI Root Bridge header file. >=20 > -Copyright (c) 1999 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 1999 - 2018, 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 > @@ -36,6 +36,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > KIND, EITHER EXPRESS OR IMPLIED. > #include > #include > #include > +#include > #include "PciHostResource.h" >=20 >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > index 5764c2f49f..459e962fe7 100644 > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > @@ -2,7 +2,7 @@ >=20 > PCI Root Bridge Io Protocol code. >=20 > -Copyright (c) 1999 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 1999 - 2018, 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 > @@ -468,6 +468,85 @@ RootBridgeIoGetMemTranslationByAddress ( > return EFI_SUCCESS; > } >=20 > +/** > + Return the result of (Multiplicand * Multiplier / Divisor). > + > + @param Multiplicand A 64-bit unsigned value. > + @param Multiplier A 64-bit unsigned value. > + @param Divisor A 32-bit unsigned value. > + @param Remainder A pointer to a 32-bit unsigned value. This paramet= er > is > + optional and may be NULL. > + > + @return Multiplicand * Multiplier / Divisor. > +**/ > +UINT64 > +MultThenDivU64x64x32 ( > + IN UINT64 Multiplicand, > + IN UINT64 Multiplier, > + IN UINT32 Divisor, > + OUT UINT32 *Remainder OPTIONAL > + ) > +{ > + UINT64 Uint64; > + UINT32 LocalRemainder; > + UINT32 Uint32; > + if (Multiplicand >=3D DivU64x64Remainder (MAX_UINT64, Multiplier, NULL= )) > { > + // > + // Make sure Multiplicand is the bigger one. > + // > + if (Multiplicand < Multiplier) { > + Uint64 =3D Multiplicand; > + Multiplicand =3D Multiplier; > + Multiplier =3D Uint64; > + } > + // > + // Because Multiplicand * Multiplier overflows, > + // Multiplicand * Multiplier / Divisor > + // =3D (2 * Multiplicand' + 1) * Multiplier / Divisor > + // =3D 2 * (Multiplicand' * Multiplier / Divisor) + Multiplier / Div= isor > + // > + Uint64 =3D MultThenDivU64x64x32 (RShiftU64 (Multiplicand, 1), Multip= lier, > Divisor, &LocalRemainder); > + Uint64 =3D LShiftU64 (Uint64, 1); > + Uint32 =3D 0; > + if ((Multiplicand & 0x1) =3D=3D 1) { > + Uint64 +=3D DivU64x32Remainder (Multiplier, Divisor, &Uint32); > + } > + return Uint64 + DivU64x32Remainder (Uint32 + LShiftU64 > +(LocalRemainder, 1), Divisor, Remainder); > + } else { > + return DivU64x32Remainder (MultU64x64 (Multiplicand, Multiplier), > +Divisor, Remainder); > + } > +} > + > +/** > + Return the elapsed tick count from CurrentTick. > + > + @param CurrentTick On input, the previous tick count. > + On output, the current tick count. > + @param StartTick The value the performance counter starts with whe= n it > + rolls over. > + @param EndTick The value that the performance counter ends with > before > + it rolls over. > + > + @return The elapsed tick count from CurrentTick. > +**/ > +UINT64 > +GetElapsedTick ( > + UINT64 *CurrentTick, > + UINT64 StartTick, > + UINT64 EndTick > + ) > +{ > + UINT64 PreviousTick; > + > + PreviousTick =3D *CurrentTick; > + *CurrentTick =3D GetPerformanceCounter(); > + if (StartTick < EndTick) { > + return *CurrentTick - PreviousTick; > + } else { > + return PreviousTick - *CurrentTick; > + } > +} > + > /** > Polls an address in memory mapped I/O space until an exit condition is= met, > or a timeout occurs. > @@ -517,6 +596,11 @@ RootBridgeIoPollMem ( > EFI_STATUS Status; > UINT64 NumberOfTicks; > UINT32 Remainder; > + UINT64 StartTick; > + UINT64 EndTick; > + UINT64 CurrentTick; > + UINT64 ElapsedTick; > + UINT64 Frequency; >=20 > if (Result =3D=3D NULL) { > return EFI_INVALID_PARAMETER; > @@ -542,28 +626,18 @@ RootBridgeIoPollMem ( > return EFI_SUCCESS; >=20 > } else { > - > - // > - // Determine the proper # of metronome ticks to wait for polling the > - // location. The nuber of ticks is Roundup (Delay / > - // mMetronome->TickPeriod)+1 > - // The "+1" to account for the possibility of the first tick being s= hort > - // because we started in the middle of a tick. > // > - // BugBug: overriding mMetronome->TickPeriod with UINT32 until > Metronome > - // protocol definition is updated. > + // NumberOfTicks =3D Frenquency * Delay / > EFI_TIMER_PERIOD_SECONDS(1) > // > - NumberOfTicks =3D DivU64x32Remainder (Delay, (UINT32) mMetronome- > >TickPeriod, > - &Remainder); > - if (Remainder !=3D 0) { > - NumberOfTicks +=3D 1; > + Frequency =3D GetPerformanceCounterProperties (&StartTick, &EndT= ick); > + NumberOfTicks =3D MultThenDivU64x64x32 (Frequency, Delay, > (UINT32)EFI_TIMER_PERIOD_SECONDS(1), &Remainder); > + if (Remainder >=3D (UINTN)EFI_TIMER_PERIOD_SECONDS(1) / 2) { > + NumberOfTicks++; > } > - NumberOfTicks +=3D 1; > - > - while (NumberOfTicks !=3D 0) { > - > - mMetronome->WaitForTick (mMetronome, 1); > - > + for ( ElapsedTick =3D 0, CurrentTick =3D GetPerformanceCounter() > + ; ElapsedTick <=3D NumberOfTicks > + ; ElapsedTick +=3D GetElapsedTick (&CurrentTick, StartTick, EndT= ick) > + ) { > Status =3D This->Mem.Read (This, Width, Address, 1, Result); > if (EFI_ERROR (Status)) { > return Status; > @@ -572,8 +646,6 @@ RootBridgeIoPollMem ( > if ((*Result & Mask) =3D=3D Value) { > return EFI_SUCCESS; > } > - > - NumberOfTicks -=3D 1; > } > } > return EFI_TIMEOUT; > @@ -626,6 +698,11 @@ RootBridgeIoPollIo ( > EFI_STATUS Status; > UINT64 NumberOfTicks; > UINT32 Remainder; > + UINT64 StartTick; > + UINT64 EndTick; > + UINT64 CurrentTick; > + UINT64 ElapsedTick; > + UINT64 Frequency; >=20 > // > // No matter what, always do a single poll. > @@ -651,25 +728,18 @@ RootBridgeIoPollIo ( > return EFI_SUCCESS; >=20 > } else { > - > // > - // Determine the proper # of metronome ticks to wait for polling the > - // location. The number of ticks is Roundup (Delay / > - // mMetronome->TickPeriod)+1 > - // The "+1" to account for the possibility of the first tick being s= hort > - // because we started in the middle of a tick. > + // NumberOfTicks =3D Frenquency * Delay / > EFI_TIMER_PERIOD_SECONDS(1) > // > - NumberOfTicks =3D DivU64x32Remainder (Delay, (UINT32)mMetronome- > >TickPeriod, > - &Remainder); > - if (Remainder !=3D 0) { > - NumberOfTicks +=3D 1; > + Frequency =3D GetPerformanceCounterProperties (&StartTick, &EndT= ick); > + NumberOfTicks =3D MultThenDivU64x64x32 (Frequency, Delay, > (UINT32)EFI_TIMER_PERIOD_SECONDS(1), &Remainder); > + if (Remainder >=3D (UINTN)EFI_TIMER_PERIOD_SECONDS(1) / 2) { > + NumberOfTicks++; > } > - NumberOfTicks +=3D 1; > - > - while (NumberOfTicks !=3D 0) { > - > - mMetronome->WaitForTick (mMetronome, 1); > - > + for ( ElapsedTick =3D 0, CurrentTick =3D GetPerformanceCounter() > + ; ElapsedTick <=3D NumberOfTicks > + ; ElapsedTick +=3D GetElapsedTick (&CurrentTick, StartTick, EndT= ick) > + ) { > Status =3D This->Io.Read (This, Width, Address, 1, Result); > if (EFI_ERROR (Status)) { > return Status; > @@ -678,13 +748,14 @@ RootBridgeIoPollIo ( > if ((*Result & Mask) =3D=3D Value) { > return EFI_SUCCESS; > } > - > - NumberOfTicks -=3D 1; > } > } > return EFI_TIMEOUT; > } >=20 > + > + > + > /** > Enables a PCI driver to access PCI controller registers in the PCI roo= t > bridge memory space. > -- > 2.16.1.windows.1