Re: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 01/12] MdeModulePkg/PciBusDxe:New PCI features separation with PCD

["Javeed, Ashraf" <ashraf.javeed@intel.com>]

This patch is also uploaded in the following Repo:-
https://github.com/ashrafj/edk2-staging/commit/0cc1a9555e1546ad94dd368160ece526d10d96a6

Please review.
Thanks
Ashraf

> -----Original Message-----
> From: devel@edk2.groups.io <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 <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>
> 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=2194
> 
> Definition of bit masks for the new PCD for the following new PCI feature
> 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 defini- tion,
> helper routines defined for features that needs to be supported in.
> future.
> 
> Signed-off-by: Ashraf Javeed <ashraf.javeed@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> ---
>  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 and IO space
> for these devices.
>  #  Please use PCD feature flag PcdPciBusHotplugDeviceSupport to enable hot
> plug supporting.
>  #
> -#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2006 - 2019, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  #  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                  ## CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport                ## CONSUMES
>    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 module..
> +
> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +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 configured
> +          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 Relax
> +Ordering
> +  PCI feature to be configured by this driver
> +
> +  @retval TRUE    platform has selected the Relax Ordering to be configured
> +          FALSE   platform has not selected this feature
> +**/
> +BOOLEAN
> +SetupRelaxOrder (
> +  )
> +{
> +  return (PcdGet32 (PcdOtherPciFeatures) & PCI_FEATURE_SUPPORT_FLAG_RO)
> +? 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_NS)
> +? 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 Atomic
> +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 Common
> +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 module..
> +
> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +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 PCI 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.<BR>
> +  #   Bit 1 - if set, the PCI Bus driver programs the device's
> Max_Read_Req_Size.<BR>
> +  #   Bit 2 - if set, the PCI Bus driver programs the device's Relax Ordering
> state.<BR>
> +  #   Bit 3 - if set, the PCI Bus driver programs the device's No-Snoop state.<BR>
> +  #   Bit 4 - if set, the PCI Bus driver programs the device's Completion Timeout
> range.<BR>
> +  #   Bit 5 - if set, the PCI Bus driver programs the device's Extended Tag
> range.<BR>
> +  #   Bit 6 - if set, the PCI Bus driver programs the device's AtomicOp
> feature.<BR>
> +  #   Bit 7 - if set, the PCI Bus driver programs the device's LTR feature.<BR>
> +  #   Bit 8 to 11 - Reserved for future use by the PCI Bus driver.<BR>
> +  #   Bit 12 - if set, the PCI Bus driver programs the PCIe link ASPM state.<BR>
> +  #   Bit 13 - if set, the PCI Bus driver programs the PCIe link Common Clock
> Configuration.<BR>
> +  #   Bit 14 - if set, the PCI Bus driver programs the PCIe link Extended Synch
> state.<BR>
> +  #   Bit 15 to 19 - Reserved for future use by the PCI Bus driver.<BR>
> +  #   Bit 20 - if set, the PCI Bus driver programs the device's PTM feature.<BR>
> +  #   Bit 21 to 31 - Reserved for future use by the PCI Bus driver.<BR>
> +  # @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 setting
> action.
>    #  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number
> of callback function
> --
> 2.21.0.windows.1
> 
> 
>