This patch can also be viewed in the following repo:- https://github.com/ashrafj/edk2-staging/commit/34a6c33558aed624ca95f719e5df4f3363f7cb05
Thanks Ashraf > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Javeed, > Ashraf > Sent: Saturday, February 8, 2020 1:35 AM > To: devel@edk2.groups.io > Cc: Wang, Jian J <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>; > Ni, Ray <ray...@intel.com> > Subject: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 12/12] > PciBusDxe: New PCI Express feature Common CLock Config > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2500 > > This code change enforces the Link Control register CCC field as per the > following conditions:- > (1) When the Clock Configuration device policy for all the devices are > set to EFI_PCI_EXPRESS_CLK_CFG_AUTO:- > - Based on the Link Status register's Slot Clock Configuration field, > all the devices CCC value shall be aligned > (2) When the Clock Configuration device policy for one or more devices > are either set to EFI_PCI_EXPRESS_CLK_CFG_ASYNCH or EFI_PCI_EXPRESS_ > CLK_CFG_COMMON > - enforces the same clock configuration for all the devices from > root bridge > Note that the recommendation to the platform is to always provide the device > policy as EFI_PCI_EXPRESS_CLK_CFG_AUTO. > In case for any device its Link Control register CCC field is required to be > changed based on its present HW-state, than Link Retraining is preformed on > the > downstream ports as per the PCI Express Base specification. > > This programming of CCC, gets the device-specific platform policy using the > new > PCI Express Platform Protocol interface (ECR version 0.8), defined in the > below > feature request:- > https://bugzilla.tianocore.org/show_bug.cgi?id=1954 > > Signed-off-by: Ashraf Javeed <ashraf.jav...@intel.com> > Cc: Jian J Wang <jian.j.w...@intel.com> > Cc: Hao A Wu <hao.a...@intel.com> > Cc: Ray Ni <ray...@intel.com> > --- > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 + > MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c | 267 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h | 51 > +++++++++++++++++++++++++++++++++++++++++++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c | 20 > ++++++++++++++++++-- > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h | 9 +++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c | 28 > ++++++++++++++++++++++++++++ > 6 files changed, 374 insertions(+), 2 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > index b5caffe..34f482d 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > @@ -299,6 +299,7 @@ struct _PCI_IO_DEVICE { > BOOLEAN SetupLtr; > UINT8 SetupExtTag; > UINT8 SetupAspm; > + EFI_PCI_EXPRESS_COMMON_CLOCK_CFG SetupCcc; > }; > > #define PCI_IO_DEVICE_FROM_PCI_IO_THIS(a) \ diff --git > a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > index 5e350e7..1e2f4a4 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > @@ -1909,3 +1909,270 @@ ProgramAspm ( > return EFI_SUCCESS; > } > > +/** > + The main routine to setup the PCI Express feature Common Clock > +configuration > + as per the device-specific platform policy, as well as in complaince > +with the > + PCI Express Base specification Revision 5. > + > + @param PciDevice A pointer to the PCI_IO_DEVICE. > + @param PciExpressConfigurationTable pointer to > + PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > + > + @retval EFI_SUCCESS setup of PCI feature LTR is > successful. > +**/ > +EFI_STATUS > +SetupCommonClkCfg ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciExpressConfigurationTable > + ) > +{ > + PCI_REG_PCIE_LINK_STATUS LinkSts; > + > + LinkSts.Uint16 = > + PciDevice->PciExpressCapabilityStructure.LinkStatus.Uint16; > + > + // > + // Common Clock Configuration is only applicable to root bridge and > +its child > + // devices. Not applicable to empty bridge devices or RCiEP devices > + // > + if (PciExpressConfigurationTable) { > + if (PciDevice->SetupCcc == EFI_PCI_EXPRESS_CLK_CFG_AUTO) { > + // > + // as per the PCI Express Base Specification, the link status register > + // slot clock configuration of the opposing side of link devices > indicate > + // the clock configuration properly; hence rely on this data to > configure > + // the link's clock configuration > + // > + if (LinkSts.Bits.SlotClockConfiguration) { > + PciExpressConfigurationTable->CommonClockConfiguration = TRUE; > + } else { > + PciExpressConfigurationTable->CommonClockConfiguration = FALSE; > + } > + } else if (PciDevice->SetupCcc == EFI_PCI_EXPRESS_CLK_CFG_ASYNCH) { > + // > + // platform override to any device shall change for other device on the > + // link, the clock configuration has to be maintained common across all > + // the devices > + // > + PciExpressConfigurationTable->CommonClockConfiguration = FALSE; > + } else { > + PciExpressConfigurationTable->CommonClockConfiguration = TRUE; > + } > + } > + return EFI_SUCCESS; > +} > + > +/** > + Program the PCIe Link Control register Coomon Clock Configuration > +field; if > + the hardware value is different than the intended value. > + > + @param PciDevice A pointer to the PCI_IO_DEVICE instance. > + > + @retval EFI_SUCCESS The data was read from or written to the PCI > device. > + @retval EFI_UNSUPPORTED The address range specified by Offset, Width, > and Count is not > + valid for the PCI configuration header of > the PCI controller. > + @retval EFI_INVALID_PARAMETER Buffer is NULL or Width is invalid. > + > +**/ > +EFI_STATUS > +ProgramCcc ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciExFeatureConfiguration > + ) > +{ > + PCI_REG_PCIE_LINK_CONTROL LinkCtl; > + UINT32 Offset; > + EFI_STATUS Status; > + EFI_TPL OldTpl; > + > + // > + // Common Clock Configuration is only applicable to root bridge and > + its child // devices. Not applicable to empty bridge devices or RCiEP > + devices // if (!PciExFeatureConfiguration) { > + return EFI_SUCCESS; > + } > + > + // > + // read the link Control register for the ASPM Control // > + LinkCtl.Uint16 = 0; > + Offset = PciDevice->PciExpressCapabilityOffset + > + OFFSET_OF (PCI_CAPABILITY_PCIEXP, LinkControl); Status = > + PciDevice->PciIo.Pci.Read ( > + &PciDevice->PciIo, > + EfiPciIoWidthUint16, > + Offset, > + 1, > + &LinkCtl.Uint16 > + ); > + ASSERT (Status == EFI_SUCCESS); > + > + // > + // in case Common Clock Configuration is required to be programmed in > + the // downstream ports from the root bridge devices in the heirarchy > + // if (PciExFeatureConfiguration->CommonClockConfiguration == TRUE) { > + if (LinkCtl.Bits.CommonClockConfiguration == 0) { > + LinkCtl.Bits.CommonClockConfiguration = 1; > + // > + // current clock mode does not match hence retrain of the link at > bridge > device > + // is required > + // > + PciExFeatureConfiguration->LinkReTrain = TRUE; > + } > + } else { > + // > + // in case the opposing devices of the PCI link have different reference > clock > + // set the link control register CCC field accordingly > + // > + if (LinkCtl.Bits.CommonClockConfiguration) { > + LinkCtl.Bits.CommonClockConfiguration = 0; > + // > + // current clock mode does not match hence retrain of the link at > bridge > device > + // is required > + // > + PciExFeatureConfiguration->LinkReTrain = TRUE; > + } > + } > + // > + // use the retrain flag as a sigm to also update the CCC of the link > + register // if (PciExFeatureConfiguration->LinkReTrain == TRUE) { > + DEBUG (( > + DEBUG_INFO, > + "CCC: %d,", > + LinkCtl.Bits.CommonClockConfiguration > + )); > + // > + // Raise TPL to high level to disable timer interrupt while the write > operation > completes > + // > + OldTpl = gBS->RaiseTPL (TPL_HIGH_LEVEL); > + > + Status = PciDevice->PciIo.Pci.Write ( > + &PciDevice->PciIo, > + EfiPciIoWidthUint16, > + Offset, > + 1, > + &LinkCtl.Uint16 > + ); > + // > + // Restore TPL to its original level > + // > + gBS->RestoreTPL (OldTpl); > + > + if (!EFI_ERROR (Status)) { > + PciDevice->PciExpressCapabilityStructure.LinkControl.Uint16 = > LinkCtl.Uint16; > + } else { > + ReportPciWriteError (PciDevice->BusNumber, PciDevice->DeviceNumber, > PciDevice->FunctionNumber, Offset); > + return Status; > + } > + } else { > + PciDevice->PciExpressCapabilityStructure.LinkControl.Uint16 = > LinkCtl.Uint16; > + DEBUG (( > + DEBUG_INFO, > + "No CCC (%d),", > + LinkCtl.Bits.CommonClockConfiguration > + )); > + } > + return EFI_SUCCESS; > +} > + > +/** > + Second phase of programming for Common Clock COnfiguration, > +conditoonally done > + only on the downstream ports (bridge devices only). > + > + @param PciDevice A pointer to the PCI_IO_DEVICE instance. > + > + @retval EFI_SUCCESS The data was read from or written to the PCI > device. > + @retval EFI_UNSUPPORTED The address range specified by Offset, Width, > and Count is not > + valid for the PCI configuration header of > the PCI controller. > + @retval EFI_INVALID_PARAMETER Buffer is NULL or Width is invalid. > + > +**/ > +EFI_STATUS > +EnforceCcc ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciExFeatureConfiguration > + ) > +{ > + PCI_REG_PCIE_LINK_CONTROL LinkCtl; > + PCI_REG_PCIE_LINK_STATUS LinkSts; > + PCI_REG_PCIE_CAPABILITY PciExCap; > + UINT32 Offset; > + EFI_STATUS Status; > + EFI_TPL OldTpl; > + > + // > + // Common Clock Configuration is only applicable to root bridge and > + its child // devices. Not applicable to empty bridge devices or RCiEP > + devices // if (!PciExFeatureConfiguration) { > + return EFI_SUCCESS; > + } > + PciExCap.Uint16 = > + PciDevice->PciExpressCapabilityStructure.Capability.Uint16; > + LinkCtl.Uint16 = > + PciDevice->PciExpressCapabilityStructure.LinkControl.Uint16; > + > + // > + // retrain the bridge device (downstream ports including the root > + port) // if (PciExFeatureConfiguration->LinkReTrain == TRUE) { > + if (IS_PCI_BRIDGE (&PciDevice->Pci)) { > + // > + // retrain of the PCI link happens for CCC change only on the > downstream > + // ports > + // > + if ( > + PciExCap.Bits.DevicePortType == PCIE_DEVICE_PORT_TYPE_ROOT_PORT > + || PciExCap.Bits.DevicePortType == > PCIE_DEVICE_PORT_TYPE_DOWNSTREAM_PORT > + ) { > + LinkCtl.Bits.RetrainLink = 1; > + Offset = PciDevice->PciExpressCapabilityOffset + > + OFFSET_OF (PCI_CAPABILITY_PCIEXP, LinkControl); > + // > + // Raise TPL to high level to disable timer interrupt while the write > operation completes > + // > + OldTpl = gBS->RaiseTPL (TPL_HIGH_LEVEL); > + > + Status = PciDevice->PciIo.Pci.Write ( > + &PciDevice->PciIo, > + EfiPciIoWidthUint16, > + Offset, > + 1, > + &LinkCtl.Uint16 > + ); > + // > + // Restore TPL to its original level > + // > + gBS->RestoreTPL (OldTpl); > + > + if (!EFI_ERROR (Status)) { > + // > + // poll the link status register for the link retrain to be > complete > + // > + Offset = PciDevice->PciExpressCapabilityOffset + > + OFFSET_OF (PCI_CAPABILITY_PCIEXP, LinkStatus); > + do { > + Status = PciDevice->PciIo.Pci.Read ( > + &PciDevice->PciIo, > + EfiPciIoWidthUint16, > + Offset, > + 1, > + &LinkSts.Uint16 > + ); > + ASSERT (Status == EFI_SUCCESS); > + } while (LinkSts.Bits.LinkTraining); > + } else { > + ReportPciWriteError (PciDevice->BusNumber, PciDevice->DeviceNumber, > PciDevice->FunctionNumber, Offset); > + return Status; > + } > + } > + // > + // ignore the upstream bridge devices > + // > + } > + // > + // not applicable to endpoint devices > + // > + } > + return EFI_SUCCESS; > +} > + > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > index 351c61e..33df337 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > @@ -345,4 +345,55 @@ ProgramAspm ( > IN VOID *PciExFeatureConfiguration > ); > > +/** > + The main routine to setup the PCI Express feature Common Clock > +configuration > + as per the device-specific platform policy, as well as in complaince > +with the > + PCI Express Base specification Revision 5. > + > + @param PciDevice A pointer to the PCI_IO_DEVICE. > + @param PciExpressConfigurationTable pointer to > + PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > + > + @retval EFI_SUCCESS setup of PCI feature LTR is > successful. > +**/ > +EFI_STATUS > +SetupCommonClkCfg ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciExpressConfigurationTable > + ); > + > +/** > + Program the PCIe Link Control register Coomon Clock Configuration > +field; if > + the hardware value is different than the intended value. > + > + @param PciDevice A pointer to the PCI_IO_DEVICE instance. > + > + @retval EFI_SUCCESS The data was read from or written to the PCI > device. > + @retval EFI_UNSUPPORTED The address range specified by Offset, Width, > and Count is not > + valid for the PCI configuration header of > the PCI controller. > + @retval EFI_INVALID_PARAMETER Buffer is NULL or Width is invalid. > + > +**/ > +EFI_STATUS > +ProgramCcc ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciExFeatureConfiguration > + ); > + > +/** > + Second phase of programming for Common Clock COnfiguration, > +conditoonally done > + only on the downstream ports (bridge devices only). > + > + @param PciDevice A pointer to the PCI_IO_DEVICE instance. > + > + @retval EFI_SUCCESS The data was read from or written to the PCI > device. > + @retval EFI_UNSUPPORTED The address range specified by Offset, Width, > and Count is not > + valid for the PCI configuration header of > the PCI controller. > + @retval EFI_INVALID_PARAMETER Buffer is NULL or Width is invalid. > + > +**/ > +EFI_STATUS > +EnforceCcc ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciExFeatureConfiguration > + ); > #endif > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > index 24781c6..4d3641c 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > @@ -54,7 +54,7 @@ EFI_PCI_EXPRESS_PLATFORM_POLICY > mPciExpressPlatformPolicy = { > // > // support for PCI Express feature - Common Clock Configuration > // > - FALSE, > + TRUE, > // > // support for PCI Express feature - Extended Sync > // > @@ -95,7 +95,15 @@ BOOLEAN mPciExpressGetPlatformPolicyComplete = > FALSE; > // PCI Express feature initialization phase handle routines // > PCI_EXPRESS_FEATURE_INITIALIZATION_POINT > mPciExpressFeatureInitializationList[] = { > - > + { > + PciExpressFeatureSetupPhase, PciExpressCcc, > SetupCommonClkCfg > + }, > + { > + PciExpressFeatureEntendedSetupPhase, PciExpressCcc, ProgramCcc > + }, > + { > + PciExpressFeatureProgramPhase, PciExpressCcc, EnforceCcc > + }, > { > PciExpressFeatureSetupPhase, PciExpressAspm, SetupAspm > }, > @@ -709,6 +717,14 @@ CreatePciRootBridgeDeviceNode ( > // start by assuming less than 1us of L1 Exit Latency > // > PciConfigTable->L1ExitLatency = > PCIE_LINK_CAPABILITY_L1_EXIT_LATENCY_1US; > + // > + // default link retrain is not required > + // > + PciConfigTable->LinkReTrain = FALSE; > + // > + // start by assuming no common clock configuration mode for the device's > link > + // > + PciConfigTable->CommonClockConfiguration = FALSE; > } > > RootBridgeNode->PciExFeaturesConfigurationTable = PciConfigTable; diff -- > git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > index 5e0f43b..481bd90 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > @@ -112,6 +112,15 @@ struct > _PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE { > // bridge device to its downstream bridge and its endpoint device > // > UINT8 L1ExitLatency; > + // > + // flag to indicate the link training is required in the devices of > + downstream // ports // > + BOOLEAN LinkReTrain; > + // > + // link status slot clock configuration // > + BOOLEAN CommonClockConfiguration; > }; > > // > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > index f301557..bf380ab 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > @@ -383,6 +383,14 @@ SetupDefaultPciExpressDevicePolicy ( > PciDevice->SetupAspm = EFI_PCI_EXPRESS_NOT_APPLICABLE; > } > > + // > + // default device policy for the device's link clock configuration > + // if (mPciExpressPlatformPolicy.Ccc) { > + PciDevice->SetupCcc = EFI_PCI_EXPRESS_CLK_CFG_AUTO; } else { > + PciDevice->SetupCcc = EFI_PCI_EXPRESS_NOT_APPLICABLE; } > > } > > @@ -536,6 +544,15 @@ GetPciExpressDevicePolicy ( > PciDevice->SetupAspm = EFI_PCI_EXPRESS_NOT_APPLICABLE; > } > > + // > + // set the device policy for the PCI Express feature Common Clock > Configuration > + // > + if (mPciExpressPlatformPolicy.Ccc) { > + PciDevice->SetupCcc = PciExpressDevicePolicy.LinkCtlCommonClkCfg; > + } else { > + PciDevice->SetupCcc = EFI_PCI_EXPRESS_NOT_APPLICABLE; > + } > + > DEBUG (( > DEBUG_INFO, > "[device policy: platform]" > @@ -850,6 +867,17 @@ PciExpressPlatformNotifyDeviceState ( > PciExDeviceConfiguration.LinkCtlASPMState = > EFI_PCI_EXPRESS_NOT_APPLICABLE; > } > > + // > + // get the device-specific Common CLock Configuration value // if > + (mPciExpressPlatformPolicy.Ccc) { > + PciExDeviceConfiguration.LinkCtlCommonClkCfg = > + PciDevice- > >PciExpressCapabilityStructure.LinkControl.Bits.CommonClockConfiguration ? > + EFI_PCI_EXPRESS_CLK_CFG_COMMON : > + EFI_PCI_EXPRESS_CLK_CFG_ASYNCH; } else { > + PciExDeviceConfiguration.LinkCtlCommonClkCfg = > + EFI_PCI_EXPRESS_NOT_APPLICABLE; } > + > if (mPciExPlatformProtocol != NULL) { > return mPciExPlatformProtocol->NotifyDeviceState ( > mPciExPlatformProtocol, > -- > 2.21.0.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54088): https://edk2.groups.io/g/devel/message/54088 Mute This Topic: https://groups.io/mt/71063741/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
