From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web12.5955.1576500375249951261 for ; Mon, 16 Dec 2019 04:46:15 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: ray.ni@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Dec 2019 04:46:14 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,321,1571727600"; d="scan'208";a="217405675" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga003.jf.intel.com with ESMTP; 16 Dec 2019 04:46:14 -0800 Received: from shsmsx106.ccr.corp.intel.com (10.239.4.159) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 16 Dec 2019 04:46:13 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.90]) by SHSMSX106.ccr.corp.intel.com ([169.254.10.236]) with mapi id 14.03.0439.000; Mon, 16 Dec 2019 20:46:11 +0800 From: "Ni, Ray" 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 01/12] MdeModulePkg/PciBusDxe:New PCI features separation with PCD Thread-Topic: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 01/12] MdeModulePkg/PciBusDxe:New PCI features separation with PCD Thread-Index: AQHVmdGLxIATphx8vU6IHkk/P1kxdKe86R4w Date: Mon, 16 Dec 2019 12:46:11 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C39EE9A@SHSMSX104.ccr.corp.intel.com> References: <20191101150952.3340-1-ashraf.javeed@intel.com> <15D3127A726D26A6.7420@groups.io> <95C5C2B113DE604FB208120C742E982457917168@BGSMSX101.gar.corp.intel.com> In-Reply-To: <95C5C2B113DE604FB208120C742E982457917168@BGSMSX101.gar.corp.intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiY2EwNmVlOGUtZmRkMy00YzM5LWI4MGItOGQ0MjI0MGY0YWQ0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiN2plOVpsdlQ1d1VEeVhDVGluU1hqdVZZWXhPdzRhRGVyYlI5dlpCdnBGMVh4aGd3VXFtSG9PWkV6YTNwZ0ZcL1AifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable With the new protocol discussed in https://bugzilla.tianocore.org/show_bug.= cgi?id=3D1954, this patch review is skipped. > -----Original Message----- > From: Javeed, Ashraf > Sent: Wednesday, November 13, 2019 11:22 AM > To: devel@edk2.groups.io; Javeed, Ashraf > Cc: Wang, Jian J ; Wu, Hao A = ; Ni, Ray > Subject: RE: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 01/12] = MdeModulePkg/PciBusDxe:New PCI features > separation with PCD >=20 > This patch is also uploaded in the following Repo:- > https://github.com/ashrafj/edk2-staging/commit/0cc1a9555e1546ad94dd36816= 0ece526d10d96a6 >=20 > Please review. > Thanks > Ashraf >=20 > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Javeed, > > Ashraf > > Sent: Friday, November 1, 2019 8:40 PM > > To: devel@edk2.groups.io > > Cc: Wang, Jian J ; Wu, Hao A ; > > Ni, Ray > > Subject: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 01/12] > > MdeModulePkg/PciBusDxe:New PCI features separation with PCD > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2194 > > > > Definition of bit masks for the new PCD for the following new PCI feat= ure > > set:- > > 1. Maximum Payload Size (MPS) > > 2. Maximum Read Request Size (MRRS) > > 3. Completion Timeout (CTO) > > 4. Relax Order (RO) Enable > > 5. No Snoop (NS) Enable > > 6. Extended Tag > > 7. ASPM support > > 8. Common Clock Configuration > > 9. Extended SYNC > > 10. Atomic Op > > 11. LTR Enable > > 12. PTM support > > > > Code changes made to the PCI Bus driver to adopt to these new PCD defi= ni- tion, > > helper routines defined for features that needs to be supported in. > > future. > > > > Signed-off-by: Ashraf Javeed > > Cc: Jian J Wang > > Cc: Hao A Wu > > Cc: Ray Ni > > --- > > MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 5 ++++- > > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c | 177 > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > +++++++++++++++++++++++++++++++++++++++++++++++ > > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h | 26 > > ++++++++++++++++++++++++++ > > MdeModulePkg/MdeModulePkg.dec | 22 > > ++++++++++++++++++++++ > > 4 files changed, 229 insertions(+), 1 deletion(-) > > > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > > index 05c2202..6dab970 100644 > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > > @@ -2,7 +2,7 @@ > > # The PCI bus driver will probe all PCI devices and allocate MMIO an= d IO space > > for these devices. > > # Please use PCD feature flag PcdPciBusHotplugDeviceSupport to enabl= e hot > > plug supporting. > > # > > -# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.=
> > +# Copyright (c) 2006 - 2019, Intel Corporation. All rights > > +reserved.
> > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -57,6 +57,8 @@ > > PciCommand.h > > PciIo.h > > PciBus.h > > + PciFeatureSupport.c > > + PciFeatureSupport.h > > > > [Packages] > > MdePkg/MdePkg.dec > > @@ -104,6 +106,7 @@ > > gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport ## CO= NSUMES > > gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport ## CO= NSUMES > > gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration ## > > SOMETIMES_CONSUMES > > + gEfiMdeModulePkgTokenSpaceGuid.PcdOtherPciFeatures ## > > CONSUMES > > > > [UserExtensions.TianoCore."ExtraFiles"] > > PciBusDxeExtra.uni > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > > new file mode 100644 > > index 0000000..8be227a > > --- /dev/null > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > > @@ -0,0 +1,177 @@ > > +/** @file > > + PCI standard feature support functions implementation for PCI Bus m= odule.. > > + > > +Copyright (c) 2019, Intel Corporation. All rights reserved.
> > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#include "PciBus.h" > > +#include "PciFeatureSupport.h" > > + > > +/** > > + Main routine to indicate whether the platform has selected the > > +Max_Payload_Size > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the Max_Payload_Size to be co= nfigured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupMaxPayloadSize ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & > > +PCI_FEATURE_SUPPORT_FLAG_MPS) ? TRUE : FALSE; } > > + > > +/** > > + Main routine to indicate whether the platform has selected the > > +Max_Read_Req_Size > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the Max_Read_Req_Size to be > > configured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupMaxReadReqSize ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & > > +PCI_FEATURE_SUPPORT_FLAG_MRRS) ? TRUE : FALSE; } > > + > > +/** > > + Main routine to indicate whether the platform has selected the Rela= x > > +Ordering > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the Relax Ordering to be conf= igured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupRelaxOrder ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & PCI_FEATURE_SUPPORT_FLAG_R= O) > > +? TRUE : FALSE; } > > + > > +/** > > + Main routine to indicate whether the platform has selected the > > +No-Snoop > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the No-Snoop to be configured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupNoSnoop ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & PCI_FEATURE_SUPPORT_FLAG_N= S) > > +? TRUE : FALSE; } > > + > > +/** > > + Main routine to indicate whether the platform has selected the > > +Completion Timeout > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the Completion Timeout to be > > configured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupCompletionTimeout ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & > > +PCI_FEATURE_SUPPORT_FLAG_CTO) ? TRUE : FALSE; } > > + > > +/** > > + Main routine to indicate whether the platform has selected the > > +Extended Tag > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the Completion Timeout to be > > configured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupExtendedTag ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & > > +PCI_FEATURE_SUPPORT_FLAG_ETAG) ? TRUE : FALSE; } > > + > > +/** > > + Main routine to indicate whether the platform has selected the Atom= ic > > +Op > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the Completion Timeout to be > > configured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupAtomicOp ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & > > +PCI_FEATURE_SUPPORT_FLAG_AOP) ? TRUE : FALSE; } > > +/** > > + Main routine to indicate whether the platform has selected the LTR > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the Completion Timeout to be > > configured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupLtr ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & > > +PCI_FEATURE_SUPPORT_FLAG_LTR) ? TRUE : FALSE; } > > + > > +/** > > + Main routine to indicate whether the platform has selected the ASPM > > +state > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the Completion Timeout to be > > configured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupAspm ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & > > +PCI_FEATURE_SUPPORT_FLAG_ASPM) ? TRUE : FALSE; } > > + > > +/** > > + Main routine to indicate whether the platform has selected the Comm= on > > +Clock Configuration > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the Completion Timeout to be > > configured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupCommonClkCfg ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & > > +PCI_FEATURE_SUPPORT_FLAG_CCC) ? TRUE : FALSE; } > > + > > +/** > > + Main routine to indicate whether the platform has selected the > > +Extended Synch > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the Completion Timeout to be > > configured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupExtendedSynch ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & > > +PCI_FEATURE_SUPPORT_FLAG_ESYN) ? TRUE : FALSE; } > > + > > +/** > > + Main routine to indicate whether the platform has selected the PIM > > +Control > > + PCI feature to be configured by this driver > > + > > + @retval TRUE platform has selected the Completion Timeout to be > > configured > > + FALSE platform has not selected this feature > > +**/ > > +BOOLEAN > > +SetupPtm ( > > + ) > > +{ > > + return (PcdGet32 (PcdOtherPciFeatures) & > > +PCI_FEATURE_SUPPORT_FLAG_PTM) ? TRUE : FALSE; } > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > > new file mode 100644 > > index 0000000..d06a5e8 > > --- /dev/null > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > > @@ -0,0 +1,26 @@ > > +/** @file > > + PCI standard feature support functions implementation for PCI Bus m= odule.. > > + > > +Copyright (c) 2019, Intel Corporation. All rights reserved.
> > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#ifndef _EFI_PCI_FEATURES_SUPPORT_H_ > > +#define _EFI_PCI_FEATURES_SUPPORT_H_ > > +// > > +// Macro definitions for the PCI Features support PCD // #define > > +PCI_FEATURE_SUPPORT_FLAG_MPS BIT0 #define > > +PCI_FEATURE_SUPPORT_FLAG_MRRS BIT1 > > +#define PCI_FEATURE_SUPPORT_FLAG_RO BIT2 > > +#define PCI_FEATURE_SUPPORT_FLAG_NS BIT3 > > +#define PCI_FEATURE_SUPPORT_FLAG_CTO BIT4 #define > > +PCI_FEATURE_SUPPORT_FLAG_ETAG BIT5 #define > > PCI_FEATURE_SUPPORT_FLAG_AOP > > +BIT6 #define PCI_FEATURE_SUPPORT_FLAG_LTR BIT7 #define > > +PCI_FEATURE_SUPPORT_FLAG_ASPM BIT12 #define > > +PCI_FEATURE_SUPPORT_FLAG_CCC BIT13 #define > > +PCI_FEATURE_SUPPORT_FLAG_ESYN BIT14 #define > > +PCI_FEATURE_SUPPORT_FLAG_PTM BIT20 #endif > > diff --git a/MdeModulePkg/MdeModulePkg.dec > > b/MdeModulePkg/MdeModulePkg.dec index 12e0bbf..ed82e85 100644 > > --- a/MdeModulePkg/MdeModulePkg.dec > > +++ b/MdeModulePkg/MdeModulePkg.dec > > @@ -1036,6 +1036,28 @@ > > # @Prompt Enable UEFI Stack Guard. > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x30 > > 001055 > > > > + ## This PCD is to indicate the PCI Bus driver to setup other new PC= I features. > > + # Each PCI feature is represented by its mask bit position and it > > + configures # if that bit is set. > > + # > > + # Bit 0 - if set, the PCI Bus driver programs the device's > > Max_Payload_Size.
> > + # Bit 1 - if set, the PCI Bus driver programs the device's > > Max_Read_Req_Size.
> > + # Bit 2 - if set, the PCI Bus driver programs the device's Relax = Ordering > > state.
> > + # Bit 3 - if set, the PCI Bus driver programs the device's No-Sno= op state.
> > + # Bit 4 - if set, the PCI Bus driver programs the device's Comple= tion Timeout > > range.
> > + # Bit 5 - if set, the PCI Bus driver programs the device's Extend= ed Tag > > range.
> > + # Bit 6 - if set, the PCI Bus driver programs the device's Atomic= Op > > feature.
> > + # Bit 7 - if set, the PCI Bus driver programs the device's LTR fe= ature.
> > + # Bit 8 to 11 - Reserved for future use by the PCI Bus driver. > > + # Bit 12 - if set, the PCI Bus driver programs the PCIe link ASPM= state.
> > + # Bit 13 - if set, the PCI Bus driver programs the PCIe link Comm= on Clock > > Configuration.
> > + # Bit 14 - if set, the PCI Bus driver programs the PCIe link Exte= nded Synch > > state.
> > + # Bit 15 to 19 - Reserved for future use by the PCI Bus driver. > > + # Bit 20 - if set, the PCI Bus driver programs the device's PTM f= eature.
> > + # Bit 21 to 31 - Reserved for future use by the PCI Bus driver. > > + # @Prompt The UEFI PCI Bus driver enables the new set of other PCI = Features. > > + > > + > > gEfiMdeModulePkgTokenSpaceGuid.PcdOtherPciFeatures|0x001070FF|UINT32| > > 0 > > + x30001056 > > + > > [PcdsFixedAtBuild, PcdsPatchableInModule] > > ## Dynamic type PCD can be registered callback function for Pcd set= ting > > action. > > # PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum numb= er > > of callback function > > -- > > 2.21.0.windows.1 > > > > > >=20