From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (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 AEB1A2095C3D8 for ; Mon, 17 Apr 2017 06:33:35 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Apr 2017 06:33:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,215,1488873600"; d="scan'208";a="75123226" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga002.jf.intel.com with ESMTP; 17 Apr 2017 06:33:34 -0700 Received: from fmsmsx117.amr.corp.intel.com (10.18.116.17) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 17 Apr 2017 06:33:34 -0700 Received: from fmsmsx103.amr.corp.intel.com ([169.254.2.246]) by fmsmsx117.amr.corp.intel.com ([169.254.3.194]) with mapi id 14.03.0319.002; Mon, 17 Apr 2017 06:33:33 -0700 From: "Carsey, Jaben" To: "Ni, Ruiyu" , "edk2-devel@lists.01.org" Thread-Topic: [PATCH] ShellPkg/Pci: Always dump the extended config space for PCIE Thread-Index: AQHStyiKnuKnr79uPECAD1ges6WY2aHJkBLA Date: Mon, 17 Apr 2017 13:33:33 +0000 Message-ID: References: <20170417031257.108080-1-ruiyu.ni@intel.com> In-Reply-To: <20170417031257.108080-1-ruiyu.ni@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.1.200.106] MIME-Version: 1.0 Subject: Re: [PATCH] ShellPkg/Pci: Always dump the extended config space for PCIE X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Apr 2017 13:33:36 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Jaben Carsey > -----Original Message----- > From: Ni, Ruiyu > Sent: Sunday, April 16, 2017 8:13 PM > To: edk2-devel@lists.01.org > Cc: Carsey, Jaben ; Jim Dailey > Subject: [PATCH] ShellPkg/Pci: Always dump the extended config space for = PCIE > Importance: High >=20 > It is to align to the original behavior before "-ec" option was added. >=20 > The patch also refines the code to make it more readable. >=20 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Cc: Jaben Carsey > Cc: Jim Dailey > --- > ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c | 318 +++++++++-------= ---- > -- > 1 file changed, 125 insertions(+), 193 deletions(-) >=20 > diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > index 37f15d6..99335f0 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > @@ -1905,16 +1905,12 @@ PciGetNextBusRange ( > @param[in] ConfigSpace Data in PCI configuration space. > @param[in] Address Address used to access configuration space = of this > PCI device. > @param[in] IoDev Handle used to access configuration space o= f PCI > device. > - @param[in] EnhancedDump The print format for the dump data. > - > - @retval EFI_SUCCESS The command completed successfully. > **/ > -EFI_STATUS > -PciExplainData ( > +VOID > +PciExplainPci ( > IN PCI_CONFIG_SPACE *ConfigSpace, > IN UINT64 Address, > - IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL *IoDev, > - IN CONST UINT16 EnhancedDump > + IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL *IoDev > ); >=20 > /** > @@ -2030,40 +2026,31 @@ PciExplainBridgeControl ( > ); >=20 > /** > - Print each capability structure. > + Locate capability register block per capability ID. >=20 > - @param[in] IoDev The pointer to the deivce. > - @param[in] Address The address to start at. > - @param[in] CapPtr The offset from the address. > - @param[in] EnhancedDump The print format for the dump data. > + @param[in] ConfigSpace Data in PCI configuration space. > + @param[in] CapabilityId The capability ID. >=20 > - @retval EFI_SUCCESS The operation was successful. > + @return The offset of the register block per capability ID. > **/ > -EFI_STATUS > -PciExplainCapabilityStruct ( > - IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL *IoDev, > - IN UINT64 Address, > - IN UINT8 CapPtr, > - IN CONST UINT16 EnhancedDump > +UINT8 > +LocatePciCapability ( > + IN PCI_CONFIG_SPACE *ConfigSpace, > + IN UINT8 CapabilityId > ); >=20 > /** > Display Pcie device structure. >=20 > - @param[in] IoDev The pointer to the root pci protocol. > - @param[in] Address The Address to start at. > - @param[in] CapabilityPtr The offset from the address to start. > - @param[in] EnhancedDump The print format for the dump data. > - > - @retval EFI_SUCCESS The command completed successfully. > - @retval @retval EFI_SUCCESS Pci express extend space IO is not suppo= ted. > + @param[in] PciExpressCap PCI Express capability buffer. > + @param[in] ExtendedConfigSpace PCI Express extended configuration spac= e. > + @param[in] ExtendedCapability PCI Express extended capability ID to e= xplain. > **/ > -EFI_STATUS > +VOID > PciExplainPciExpress ( > - IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL *IoDev, > - IN UINT64 Address, > - IN UINT8 CapabilityPtr, > - IN CONST UINT16 EnhancedDump > + IN PCI_CAPABILITY_PCIEXP *PciExpressCap, > + IN UINT8 *ExtendedConfigSpace, > + IN CONST UINT16 ExtendedCapability > ); >=20 > /** > @@ -2473,7 +2460,10 @@ ShellCommandRunPci ( > SHELL_STATUS ShellStatus; > CONST CHAR16 *Temp; > UINT64 RetVal; > - UINT16 EnhancedDump; > + UINT16 ExtendedCapability; > + UINT8 PcieCapabilityPtr; > + UINT8 *ExtendedConfigSpace; > + UINTN ExtendedConfigSize; >=20 > ShellStatus =3D SHELL_SUCCESS; > Status =3D EFI_SUCCESS; > @@ -2726,7 +2716,7 @@ ShellCommandRunPci ( > Bus =3D 0; > Device =3D 0; > Func =3D 0; > - EnhancedDump =3D 0xFFFF; > + ExtendedCapability =3D 0xFFFF; > if (ShellCommandLineGetFlag(Package, L"-i")) { > ExplainData =3D TRUE; > } > @@ -2814,7 +2804,7 @@ ShellCommandRunPci ( > // Input converted to hexadecimal number. > // > if (!EFI_ERROR (ShellConvertStringToUint64 (Temp, &RetVal, TRUE, T= RUE))) { > - EnhancedDump =3D (UINT16) RetVal; > + ExtendedCapability =3D (UINT16) RetVal; > } else { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV_H= EX), > gShellDebug1HiiHandle, L"pci", Temp); > ShellStatus =3D SHELL_INVALID_PARAMETER; @@ -2894,11 +2884,51 @@ > ShellCommandRunPci ( > ConfigSpace.Data > ); >=20 > + ExtendedConfigSpace =3D NULL; > + PcieCapabilityPtr =3D LocatePciCapability (&ConfigSpace, > EFI_PCI_CAPABILITY_ID_PCIEXP); > + if (PcieCapabilityPtr !=3D 0) { > + ExtendedConfigSize =3D 0x1000 - EFI_PCIE_CAPABILITY_BASE_OFFSET; > + ExtendedConfigSpace =3D AllocatePool (ExtendedConfigSize); > + if (ExtendedConfigSpace !=3D NULL) { > + Status =3D IoDev->Pci.Read ( > + IoDev, > + EfiPciWidthUint32, > + EFI_PCI_ADDRESS (Bus, Device, Func, > EFI_PCIE_CAPABILITY_BASE_OFFSET), > + ExtendedConfigSize / sizeof (UINT32), > + ExtendedConfigSpace > + ); > + if (EFI_ERROR (Status)) { > + SHELL_FREE_NON_NULL (ExtendedConfigSpace); > + } > + } > + } > + > + if ((ExtendedConfigSpace !=3D NULL) && !ShellGetExecutionBreakFlag (= )) { > + // > + // Print the PciEx extend space in raw bytes ( 0xFF-0xFFF) > + // > + ShellPrintEx (-1, -1, L"\r\n%HStart dumping PCIex extended > + configuration space (0x100 - 0xFFF).%N\r\n\r\n"); > + > + DumpHex ( > + 2, > + EFI_PCIE_CAPABILITY_BASE_OFFSET, > + ExtendedConfigSize, > + ExtendedConfigSpace > + ); > + } > + > // > // If "-i" appears in command line, interpret data in configuration = space > // > if (ExplainData) { > - Status =3D PciExplainData (&ConfigSpace, Address, IoDev, EnhancedD= ump); > + PciExplainPci (&ConfigSpace, Address, IoDev); > + if ((PcieCapabilityPtr !=3D 0) && !ShellGetExecutionBreakFlag ()) = { > + PciExplainPciExpress ( > + (PCI_CAPABILITY_PCIEXP *) ((UINT8 *) &ConfigSpace + PcieCapabi= lityPtr), > + ExtendedConfigSpace, > + ExtendedCapability > + ); > + } > } > } > Done: > @@ -3092,22 +3122,16 @@ PciGetNextBusRange ( > @param[in] ConfigSpace Data in PCI configuration space. > @param[in] Address Address used to access configuration space = of this > PCI device. > @param[in] IoDev Handle used to access configuration space o= f PCI > device. > - @param[in] EnhancedDump The print format for the dump data. > - > - @retval EFI_SUCCESS The command completed successfully. > **/ > -EFI_STATUS > -PciExplainData ( > +VOID > +PciExplainPci ( > IN PCI_CONFIG_SPACE *ConfigSpace, > IN UINT64 Address, > - IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL *IoDev, > - IN CONST UINT16 EnhancedDump > + IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL *IoDev > ) > { > PCI_DEVICE_INDEPENDENT_REGION *Common; > PCI_HEADER_TYPE HeaderType; > - EFI_STATUS Status; > - UINT8 CapPtr; >=20 > Common =3D &(ConfigSpace->Common); >=20 > @@ -3213,56 +3237,6 @@ PciExplainData ( > ShellPrintHiiEx(-1, -1, NULL,STRING_TOKEN (STR_PCI2_CLASS), > gShellDebug1HiiHandle); > PciPrintClassCode ((UINT8 *) Common->ClassCode, TRUE); > ShellPrintEx (-1, -1, L"\r\n"); > - > - if (ShellGetExecutionBreakFlag()) { > - return EFI_SUCCESS; > - } > - > - // > - // Interpret remaining part of PCI configuration header depending on > - // HeaderType > - // > - CapPtr =3D 0; > - Status =3D EFI_SUCCESS; > - switch (HeaderType) { > - case PciDevice: > - Status =3D PciExplainDeviceData ( > - &(ConfigSpace->NonCommon.Device), > - Address, > - IoDev > - ); > - CapPtr =3D ConfigSpace->NonCommon.Device.CapabilityPtr; > - break; > - > - case PciP2pBridge: > - Status =3D PciExplainBridgeData ( > - &(ConfigSpace->NonCommon.Bridge), > - Address, > - IoDev > - ); > - CapPtr =3D ConfigSpace->NonCommon.Bridge.CapabilityPtr; > - break; > - > - case PciCardBusBridge: > - Status =3D PciExplainCardBusData ( > - &(ConfigSpace->NonCommon.CardBus), > - Address, > - IoDev > - ); > - CapPtr =3D ConfigSpace->NonCommon.CardBus.Cap_Ptr; > - break; > - case PciUndefined: > - default: > - break; > - } > - // > - // If Status bit4 is 1, dump or explain capability structure > - // > - if ((Common->Status) & EFI_PCI_STATUS_CAPABILITY) { > - PciExplainCapabilityStruct (IoDev, Address, CapPtr, EnhancedDump); > - } > - > - return Status; > } >=20 > /** > @@ -4221,53 +4195,62 @@ PciExplainBridgeControl ( } >=20 > /** > - Print each capability structure. > + Locate capability register block per capability ID. >=20 > - @param[in] IoDev The pointer to the deivce. > - @param[in] Address The address to start at. > - @param[in] CapPtr The offset from the address. > - @param[in] EnhancedDump The print format for the dump data. > + @param[in] ConfigSpace Data in PCI configuration space. > + @param[in] CapabilityId The capability ID. >=20 > - @retval EFI_SUCCESS The operation was successful. > + @return The offset of the register block per capability ID, > + or 0 if the register block cannot be found. > **/ > -EFI_STATUS > -PciExplainCapabilityStruct ( > - IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL *IoDev, > - IN UINT64 Address, > - IN UINT8 CapPtr, > - IN CONST UINT16 EnhancedDump > +UINT8 > +LocatePciCapability ( > + IN PCI_CONFIG_SPACE *ConfigSpace, > + IN UINT8 CapabilityId > ) > { > - UINT8 CapabilityPtr; > - UINT16 CapabilityEntry; > - UINT8 CapabilityID; > - UINT64 RegAddress; > - > - CapabilityPtr =3D CapPtr; > + UINT8 CapabilityPtr; > + EFI_PCI_CAPABILITY_HDR *CapabilityEntry; >=20 > // > - // Go through the Capability list > + // To check the cpability of this device supports > // > - while ((CapabilityPtr >=3D 0x40) && ((CapabilityPtr & 0x03) =3D=3D 0x0= 0)) { > - RegAddress =3D Address + CapabilityPtr; > - IoDev->Pci.Read (IoDev, EfiPciWidthUint16, RegAddress, 1, > &CapabilityEntry); > + if ((ConfigSpace->Common.Status & EFI_PCI_STATUS_CAPABILITY) =3D=3D 0)= { > + return 0; > + } >=20 > - CapabilityID =3D (UINT8) CapabilityEntry; > + switch ((PCI_HEADER_TYPE)(ConfigSpace->Common.HeaderType & 0x7f)) { > + case PciDevice: > + CapabilityPtr =3D ConfigSpace->NonCommon.Device.CapabilityPtr; > + break; > + case PciP2pBridge: > + CapabilityPtr =3D ConfigSpace->NonCommon.Bridge.CapabilityPtr; > + break; > + case PciCardBusBridge: > + CapabilityPtr =3D ConfigSpace->NonCommon.CardBus.Cap_Ptr; > + break; > + default: > + return 0; > + } >=20 > - // > - // Explain PciExpress data > - // > - if (EFI_PCI_CAPABILITY_ID_PCIEXP =3D=3D CapabilityID) { > - PciExplainPciExpress (IoDev, Address, CapabilityPtr, EnhancedDump)= ; > - return EFI_SUCCESS; > + while ((CapabilityPtr >=3D 0x40) && ((CapabilityPtr & 0x03) =3D=3D 0x0= 0)) { > + CapabilityEntry =3D (EFI_PCI_CAPABILITY_HDR *) ((UINT8 *) ConfigSpac= e + > CapabilityPtr); > + if (CapabilityEntry->CapabilityID =3D=3D CapabilityId) { > + return CapabilityPtr; > } > + > // > - // Explain other capabilities here > + // Certain PCI device may incorrectly have capability pointing to it= self, > + // break to avoid dead loop. > // > - CapabilityPtr =3D (UINT8) (CapabilityEntry >> 8); > + if (CapabilityPtr =3D=3D CapabilityEntry->NextItemPtr) { > + break; > + } > + > + CapabilityPtr =3D CapabilityEntry->NextItemPtr; > } >=20 > - return EFI_SUCCESS; > + return 0; > } >=20 > /** > @@ -5706,53 +5689,32 @@ PrintPciExtendedCapabilityDetails( > /** > Display Pcie device structure. >=20 > - @param[in] IoDev The pointer to the root pci protocol. > - @param[in] Address The Address to start at. > - @param[in] CapabilityPtr The offset from the address to start. > - @param[in] EnhancedDump The print format for the dump data. > - > + @param[in] PciExpressCap PCI Express capability buffer. > + @param[in] ExtendedConfigSpace PCI Express extended configuration spac= e. > + @param[in] ExtendedCapability PCI Express extended capability ID to e= xplain. > **/ > -EFI_STATUS > +VOID > PciExplainPciExpress ( > - IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL *IoDev, > - IN UINT64 Address, > - IN UINT8 CapabilityPtr, > - IN CONST UINT16 EnhancedDump > + IN PCI_CAPABILITY_PCIEXP *PciExpressCap, > + IN UINT8 *ExtendedConfigSpace, > + IN CONST UINT16 ExtendedCapability > ) > { > - PCI_CAPABILITY_PCIEXP PciExpressCap; > - EFI_STATUS Status; > - UINT64 CapRegAddress; > - UINT8 Bus; > - UINT8 Dev; > - UINT8 Func; > - UINT8 *ExRegBuffer; > - UINTN ExtendRegSize; > - UINT64 Pciex_Address; > UINT8 DevicePortType; > UINTN Index; > UINT8 *RegAddr; > UINTN RegValue; > PCI_EXP_EXT_HDR *ExtHdr; >=20 > - CapRegAddress =3D Address + CapabilityPtr; > - IoDev->Pci.Read ( > - IoDev, > - EfiPciWidthUint32, > - CapRegAddress, > - sizeof (PciExpressCap) / sizeof (UINT32), > - &PciExpressCap > - ); > - > - DevicePortType =3D (UINT8)PciExpressCap.Capability.Bits.DevicePortType= ; > + DevicePortType =3D > + (UINT8)PciExpressCap->Capability.Bits.DevicePortType; >=20 > ShellPrintEx (-1, -1, L"\r\nPci Express device capability structure:\r= \n"); >=20 > for (Index =3D 0; PcieExplainList[Index].Type < PcieExplainTypeMax; In= dex++) { > if (ShellGetExecutionBreakFlag()) { > - goto Done; > + return; > } > - RegAddr =3D ((UINT8 *) &PciExpressCap) + PcieExplainList[Index].Offs= et; > + RegAddr =3D (UINT8 *) PciExpressCap + PcieExplainList[Index].Offset; > switch (PcieExplainList[Index].Width) { > case FieldWidthUINT8: > RegValue =3D *(UINT8 *) RegAddr; > @@ -5797,7 +5759,7 @@ PciExplainPciExpress ( > // > if ((DevicePortType !=3D PCIE_DEVICE_PORT_TYPE_ROOT_PORT && > DevicePortType !=3D PCIE_DEVICE_PORT_TYPE_DOWNSTREAM_PORT) = || > - !PciExpressCap.Capability.Bits.SlotImplemented) { > + !PciExpressCap->Capability.Bits.SlotImplemented) { > continue; > } > break; > @@ -5813,58 +5775,28 @@ PciExplainPciExpress ( > default: > break; > } > - PcieExplainList[Index].Func (&PciExpressCap); > + PcieExplainList[Index].Func (PciExpressCap); > } >=20 > - Bus =3D (UINT8) (RShiftU64 (Address, 24)); > - Dev =3D (UINT8) (RShiftU64 (Address, 16)); > - Func =3D (UINT8) (RShiftU64 (Address, 8)); > - > - Pciex_Address =3D EFI_PCI_ADDRESS (Bus, Dev, Func, > EFI_PCIE_CAPABILITY_BASE_OFFSET); > - > - ExtendRegSize =3D 0x1000 - EFI_PCIE_CAPABILITY_BASE_OFFSET; > - > - ExRegBuffer =3D (UINT8 *) AllocateZeroPool (ExtendRegSize); > - > - // > - // PciRootBridgeIo protocol should support pci express extend space IO > - // (Begins at offset EFI_PCIE_CAPABILITY_BASE_OFFSET) > - // > - Status =3D IoDev->Pci.Read ( > - IoDev, > - EfiPciWidthUint32, > - Pciex_Address, > - (ExtendRegSize) / sizeof (UINT32), > - (VOID *) (ExRegBuffer) > - ); > - if (EFI_ERROR (Status) || ExRegBuffer =3D=3D NULL) { > - SHELL_FREE_NON_NULL(ExRegBuffer); > - return EFI_UNSUPPORTED; > - } > - > - ExtHdr =3D (PCI_EXP_EXT_HDR*)ExRegBuffer; > + ExtHdr =3D (PCI_EXP_EXT_HDR*)ExtendedConfigSpace; > while (ExtHdr->CapabilityId !=3D 0 && ExtHdr->CapabilityVersion !=3D 0= ) { > // > // Process this item > // > - if (EnhancedDump =3D=3D 0xFFFF || EnhancedDump =3D=3D ExtHdr->Capabi= lityId) { > + if (ExtendedCapability =3D=3D 0xFFFF || ExtendedCapability =3D=3D > + ExtHdr->CapabilityId) { > // > // Print this item > // > - PrintPciExtendedCapabilityDetails((PCI_EXP_EXT_HDR*)ExRegBuffer, > ExtHdr, &PciExpressCap); > + > + PrintPciExtendedCapabilityDetails((PCI_EXP_EXT_HDR*)ExtendedConfigSpac > + e, ExtHdr, PciExpressCap); > } >=20 > // > // Advance to the next item if it exists > // > if (ExtHdr->NextCapabilityOffset !=3D 0) { > - ExtHdr =3D (PCI_EXP_EXT_HDR*)((UINT8*)ExRegBuffer + ExtHdr- > >NextCapabilityOffset - EFI_PCIE_CAPABILITY_BASE_OFFSET); > + ExtHdr =3D (PCI_EXP_EXT_HDR*)(ExtendedConfigSpace + > + ExtHdr->NextCapabilityOffset - EFI_PCIE_CAPABILITY_BASE_OFFSET); > } else { > break; > } > } > - SHELL_FREE_NON_NULL(ExRegBuffer); > - > -Done: > - return EFI_SUCCESS; > } > -- > 2.9.0.windows.1