From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mx.groups.io with SMTP id smtpd.web11.7789.1576654369558600458 for ; Tue, 17 Dec 2019 23:32:49 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.43, mailfrom: ashraf.javeed@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Dec 2019 23:32:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,328,1571727600"; d="scan'208";a="247776674" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga002.fm.intel.com with ESMTP; 17 Dec 2019 23:32:49 -0800 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 17 Dec 2019 23:32:48 -0800 Received: from bgsmsx103.gar.corp.intel.com (10.223.4.130) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 17 Dec 2019 23:32:48 -0800 Received: from bgsmsx101.gar.corp.intel.com ([169.254.1.143]) by BGSMSX103.gar.corp.intel.com ([169.254.4.207]) with mapi id 14.03.0439.000; Wed, 18 Dec 2019 13:02:45 +0530 From: "Javeed, Ashraf" To: "Ni, Ray" , "devel@edk2.groups.io" CC: "Wang, Jian J" , "Wu, Hao A" Subject: Re: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 07/12] PciBusDxe: Record the PCI-Express Capability Structure Thread-Topic: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 07/12] PciBusDxe: Record the PCI-Express Capability Structure Thread-Index: AQHVkMaDc9co8K5pNEClJvkVck+KjKeIg2NwgDWi3QCAAZ48gA== Date: Wed, 18 Dec 2019 07:32:44 +0000 Message-ID: <95C5C2B113DE604FB208120C742E98245797C2C2@BGSMSX101.gar.corp.intel.com> References: <20191101150952.3340-1-ashraf.javeed@intel.com> <15D3127C6DFCD4A7.12315@groups.io> <95C5C2B113DE604FB208120C742E982457917240@BGSMSX101.gar.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C3A05E7@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C3A05E7@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMWM2ZmIzNTgtZDY3ZS00OTVkLWE5YmQtMjA2N2M4NDUxMzA1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoid1wvTGkwQWduMlNBSm81MGg1SFd4cHJwWVRFMXBzOUZpaGt3VDk2WjZZczd4OFBYYjVUcFwvZVpiSWJORTZkbVwvZyJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.223.10.10] MIME-Version: 1.0 Return-Path: ashraf.javeed@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Ni, Ray > Sent: Tuesday, December 17, 2019 5:34 PM > To: Javeed, Ashraf ; devel@edk2.groups.io > Cc: Wang, Jian J ; Wu, Hao A > Subject: RE: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 07/12] > PciBusDxe: Record the PCI-Express Capability Structure >=20 > 2 minor comments. >=20 > And I suggest you don't change EFI_D_xxx in this patch. Or you could cha= nge in a > separate patch. >=20 The below patch check script fails, if I don't change it... /Git/Tianocore/edk2/BaseTools/Scripts/PatchCheck.py > > > + PCI_CAPABILITY_PCIEXP PciExpStruct; >=20 > 1. to align "Pci" which buffers the PCI config space, this can be simply= "PciExp". >=20 Ok > > > + // > > > + // For SR-IOV > > > + // > > > UINT32 AriCapabilityOffset; > > > UINT32 SrIovCapabilityOffset; > > > UINT32 MrIovCapabilityOffset; > > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > > > index c7eafff..2343702 100644 > > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > > > @@ -230,7 +230,7 @@ PciSearchDevice ( > > > PciIoDevice =3D NULL; > > > > > > DEBUG (( > > > - EFI_D_INFO, > > > + DEBUG_INFO, > > > "PciBus: Discovered %s @ [%02x|%02x|%02x]\n", > > > IS_PCI_BRIDGE (Pci) ? L"PPB" : > > > IS_CARDBUS_BRIDGE (Pci) ? L"P2C" : > > > @@ -397,7 +397,7 @@ DumpPpbPaddingResource ( > > > > > > if ((Type !=3D PciBarTypeUnknown) && ((ResourceType =3D=3D > > > PciBarTypeUnknown) > > > || (ResourceType =3D=3D Type))) { > > > DEBUG (( > > > - EFI_D_INFO, > > > + DEBUG_INFO, > > > " Padding: Type =3D %s; Alignment =3D 0x%lx;\tLength =3D = 0x%lx\n", > > > mBarTypeStr[Type], Descriptor->AddrRangeMax, Descriptor->Ad= drLen > > > )); > > > @@ -424,7 +424,7 @@ DumpPciBars ( > > > } > > > > > > DEBUG (( > > > - EFI_D_INFO, > > > + DEBUG_INFO, > > > " BAR[%d]: Type =3D %s; Alignment =3D 0x%lx;\tLength =3D 0x= %lx;\tOffset =3D > > > 0x%02x\n", > > > Index, mBarTypeStr[MIN (PciIoDevice->PciBar[Index].BarType, > > > PciBarTypeMaxType)], > > > PciIoDevice->PciBar[Index].Alignment, > > > PciIoDevice->PciBar[Index].Length, > > > PciIoDevice->PciBar[Index].Offset @@ -437,13 +437,13 @@ DumpPciBars > > > PciIoDevice->( > > > } > > > > > > DEBUG (( > > > - EFI_D_INFO, > > > + DEBUG_INFO, > > > " VFBAR[%d]: Type =3D %s; Alignment =3D 0x%lx;\tLength =3D > > > 0x%lx;\tOffset =3D 0x%02x\n", > > > Index, mBarTypeStr[MIN (PciIoDevice->VfPciBar[Index].BarType, > > > PciBarTypeMaxType)], > > > PciIoDevice->VfPciBar[Index].Alignment, PciIoDevice- > > > >VfPciBar[Index].Length, PciIoDevice->VfPciBar[Index].Offset > > > )); > > > } > > > - DEBUG ((EFI_D_INFO, "\n")); > > > + DEBUG ((DEBUG_INFO, "\n")); > > > } > > > > > > /** > > > @@ -1903,7 +1903,7 @@ PciParseBar ( > > > // Fix the length to support some special 64 bit BAR > > > // > > > if (Value =3D=3D 0) { > > > - DEBUG ((EFI_D_INFO, "[PciBus]BAR probing for upper 32bit of= MEM64 > BAR > > > returns 0, change to 0xFFFFFFFF.\n")); > > > + DEBUG ((DEBUG_INFO, "[PciBus]BAR probing for upper 32bit of > > > + MEM64 BAR returns 0, change to 0xFFFFFFFF.\n")); > > > Value =3D (UINT32) -1; > > > } else { > > > Value |=3D ((UINT32)(-1) << HighBitSet32 (Value)); @@ -2153= ,7 > > > +2153,17 @@ CreatePciIoDevice ( > > > NULL > > > ); > > > if (!EFI_ERROR (Status)) { > > > - PciIoDevice->IsPciExp =3D TRUE; > > > + PciIoDevice->IsPciExp =3D TRUE; > > > + // > > > + // read the PCI device's entire PCI Express Capability structur= e > > > + // > > > + PciIo->Pci.Read ( > > > + PciIo, > > > + EfiPciIoWidthUint8, > > > + PciIoDevice->PciExpressCapabilityOffset, > > > + sizeof (PCI_CAPABILITY_PCIEXP) / sizeof (UINT8), > > > + &PciIoDevice->PciExpStruct > > > + ); >=20 > 2. Please follow EDKII coding style document regarding the indent. >=20 Ok, got missed for this routine. > > > } > > > > > > if (PcdGetBool (PcdAriSupport)) { @@ -2206,7 +2216,7 @@ > > > CreatePciIoDevice ( > > > &Data32 > > > ); > > > DEBUG (( > > > - EFI_D_INFO, > > > + DEBUG_INFO, > > > " ARI: forwarding enabled for PPB[%02x:%02x:%02x]\n", > > > Bridge->BusNumber, > > > Bridge->DeviceNumber, > > > @@ -2215,7 +2225,7 @@ CreatePciIoDevice ( > > > } > > > } > > > > > > - DEBUG ((EFI_D_INFO, " ARI: CapOffset =3D 0x%x\n", PciIoDevice= - > > > >AriCapabilityOffset)); > > > + DEBUG ((DEBUG_INFO, " ARI: CapOffset =3D 0x%x\n", > > > + PciIoDevice->AriCapabilityOffset)); > > > } > > > } > > > > > > @@ -2325,12 +2335,12 @@ CreatePciIoDevice ( > > > PciIoDevice->ReservedBusNum =3D (UINT16)(EFI_PCI_BUS_OF_RID > > > (LastVF) - Bus + 1); > > > > > > DEBUG (( > > > - EFI_D_INFO, > > > + DEBUG_INFO, > > > " SR-IOV: SupportedPageSize =3D 0x%x; SystemPageSize =3D 0x= %x; > > > FirstVFOffset =3D 0x%x;\n", > > > SupportedPageSize, PciIoDevice->SystemPageSize >> 12, First= VFOffset > > > )); > > > DEBUG (( > > > - EFI_D_INFO, > > > + DEBUG_INFO, > > > " InitialVFs =3D 0x%x; ReservedBusNum =3D 0x%x; Cap= Offset =3D 0x%x\n", > > > PciIoDevice->InitialVFs, PciIoDevice->ReservedBusNum, > > > PciIoDevice- > > > >SrIovCapabilityOffset > > > )); > > > @@ -2345,7 +2355,7 @@ CreatePciIoDevice ( > > > NULL > > > ); > > > if (!EFI_ERROR (Status)) { > > > - DEBUG ((EFI_D_INFO, " MR-IOV: CapOffset =3D 0x%x\n", PciIoDev= ice- > > > >MrIovCapabilityOffset)); > > > + DEBUG ((DEBUG_INFO, " MR-IOV: CapOffset =3D 0x%x\n", > > > + PciIoDevice->MrIovCapabilityOffset)); > > > } > > > } > > > > > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > > > index 9e6671d..df9e696 100644 > > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > > > @@ -467,6 +467,14 @@ GetPciFeaturesConfigurationTable ( > > > return EFI_SUCCESS; > > > } > > > > > > + // > > > + // The PCI features configuration table is not built for RCiEP, > > > + return NULL // if > > > + (PciDevice->PciExpStruct.Capability.Bits.DevicePortType =3D=3D \ > > > + PCIE_DEVICE_PORT_TYPE_ROOT_COMPLEX_INTEGRATED_ENDPOINT) > { > > > + *PciFeaturesConfigTable =3D NULL; > > > + return EFI_SUCCESS; > > > + } > > > > > > if (IsDevicePathEnd (PciDevice->DevicePath)){ > > > // > > > @@ -575,6 +583,45 @@ IsPciRootPortEmpty ( } > > > > > > > > > +/** > > > + helper routine to dump the PCIe Device Port Type **/ VOID > > > +DumpDevicePortType ( > > > + IN UINT8 DevicePortType > > > + ) > > > +{ > > > + switch (DevicePortType){ > > > + case PCIE_DEVICE_PORT_TYPE_PCIE_ENDPOINT: > > > + DEBUG (( DEBUG_INFO, "PCIe endpoint found\n")); > > > + break; > > > + case PCIE_DEVICE_PORT_TYPE_LEGACY_PCIE_ENDPOINT: > > > + DEBUG (( DEBUG_INFO, "legacy PCI endpoint found\n")); > > > + break; > > > + case PCIE_DEVICE_PORT_TYPE_ROOT_PORT: > > > + DEBUG (( DEBUG_INFO, "PCIe Root Port found\n")); > > > + break; > > > + case PCIE_DEVICE_PORT_TYPE_UPSTREAM_PORT: > > > + DEBUG (( DEBUG_INFO, "PCI switch upstream port found\n")); > > > + break; > > > + case PCIE_DEVICE_PORT_TYPE_DOWNSTREAM_PORT: > > > + DEBUG (( DEBUG_INFO, "PCI switch downstream port found\n")); > > > + break; > > > + case PCIE_DEVICE_PORT_TYPE_PCIE_TO_PCI_BRIDGE: > > > + DEBUG (( DEBUG_INFO, "PCIe-PCI bridge found\n")); > > > + break; > > > + case PCIE_DEVICE_PORT_TYPE_PCI_TO_PCIE_BRIDGE: > > > + DEBUG (( DEBUG_INFO, "PCI-PCIe bridge found\n")); > > > + break; > > > + case > PCIE_DEVICE_PORT_TYPE_ROOT_COMPLEX_INTEGRATED_ENDPOINT: > > > + DEBUG (( DEBUG_INFO, "RCiEP found\n")); > > > + break; > > > + case PCIE_DEVICE_PORT_TYPE_ROOT_COMPLEX_EVENT_COLLECTOR: > > > + DEBUG (( DEBUG_INFO, "RC Event Collector found\n")); > > > + break; > > > + } > > > +} > > > + > > > /** > > > Process each PCI device as per the pltaform and device-specific = policy. > > > > > > @@ -590,8 +637,12 @@ SetupDevicePciFeatures ( > > > ) > > > { > > > EFI_STATUS Status; > > > + PCI_REG_PCIE_CAPABILITY PcieCap; > > > OTHER_PCI_FEATURES_CONFIGURATION_TABLE > > > *OtherPciFeaturesConfigTable; > > > > > > + PcieCap.Uint16 =3D PciDevice->PciExpStruct.Capability.Uint16; > > > + DumpDevicePortType ((UINT8)PcieCap.Bits.DevicePortType); > > > + > > > OtherPciFeaturesConfigTable =3D NULL; > > > Status =3D GetPciFeaturesConfigurationTable (PciDevice, > > > &OtherPciFeaturesConfigTable); > > > if (EFI_ERROR( Status)) { > > > -- > > > 2.21.0.windows.1 > > > > > > > > >=20