This patch can also be viewed in the following repo:- https://github.com/ashrafj/edk2-staging/commit/f0d81499e79e4521630a76ae241de6def9aa03b5
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 10/12] > PciBusDxe: New PCI Express feature Extended Tag > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2499 > > This code change enables the PCI Express feature Extended Tag, in compliance > with the PCI Express Base Specification 5, and uses the device policy under > the > following conditions: > (1) As per the PCI Express Base Specification, all the devices under the > root bridge has to be set to a common applicable value > (2) The 5b or 8b Extended Tag capability is defined in the Device Capabi- > lity register, and the 10b requester as well as completer is defined > in the Device Capability 2 register > (3) The Extended Tag device policy would be overruled for any device if > it does not match with its device capabilities register > (4) In case of multiple device policies, due to multiple devices under > the root bridge, the lowest applicable value will be programmed for > all the devices > (5) There is no Extended Tag disable state; hence the default would be > 5b or 8b depending upon device HW-state. The 10b requester is disabled > by default; hence any device policy request of 10b shall lead to 10b > Requester enable state from root bridge to all end devices; if all the > devices support 10b Requester as well as the completer capability. In > this scenario, the default state of 5b/8b Extended Tag state in Device > Control register is ignored as 10b capable devices should be able to > handle lower size Extended Tag packet IDs autonomously. > > This programming of Extended Tag, 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 | 278 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h | 50 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c | 15 > ++++++++++++++- > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h | 4 ++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c | 36 > ++++++++++++++++++++++++++++++++++++ > 6 files changed, 383 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > index 7e43a26..6a6f648 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > @@ -297,6 +297,7 @@ struct _PCI_IO_DEVICE { > PCI_FEATURE_POLICY SetupCTO; > EFI_PCI_EXPRESS_ATOMIC_OP SetupAtomicOp; > BOOLEAN SetupLtr; > + UINT8 SetupExtTag; > }; > > #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 63a243b..eaef3d3 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > @@ -1240,3 +1240,281 @@ ProgramLtr ( > return Status; > } > > +/** > + The main routine to setup the PCI Express feature Extended Tag 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 > +SetupExtTag ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciExpressConfigurationTable > + ) > +{ > + PCI_REG_PCIE_DEVICE_CAPABILITY2 DeviceCap2; > + PCI_REG_PCIE_DEVICE_CAPABILITY DeviceCap; > + EFI_PCI_EXPRESS_EXTENDED_TAG PciExpressExtendedTag; > + > + DeviceCap.Uint32 = > + PciDevice->PciExpressCapabilityStructure.DeviceCapability.Uint32; > + DeviceCap2.Uint32 = > + PciDevice->PciExpressCapabilityStructure.DeviceCapability2.Uint32; > + // > + // The PCI Express feature Extended Tag has to be maintained common > + from a // root bridge device to all its child devices. > + // The Device Capability 2 register is used to determine the 10b > + Extended Tag // capability of a device. The device capability > + register is used to determine // 5b/8b Extended Tag capability of a > + device // if (DeviceCap2.Bits.TenBitTagCompleterSupported & > + DeviceCap2.Bits.TenBitTagRequesterSupported) { > + // > + // device supports the 10b Extended Tag capability > + // > + PciExpressExtendedTag = EFI_PCI_EXPRESS_EXTENDED_TAG_10BIT; > + } else { > + if (DeviceCap.Bits.ExtendedTagField) { > + PciExpressExtendedTag = EFI_PCI_EXPRESS_EXTENDED_TAG_8BIT; > + } else { > + PciExpressExtendedTag = EFI_PCI_EXPRESS_EXTENDED_TAG_5BIT; > + } > + } > + if (PciDevice->SetupExtTag == EFI_PCI_EXPRESS_EXTENDED_TAG_AUTO) { > + PciDevice->SetupExtTag = PciExpressExtendedTag; } // // in case > + of PCI Bridge and its child devices // if > + (PciExpressConfigurationTable) { > + // > + // align the Extended Tag value as per the device supported value > + // > + PciExpressConfigurationTable->ExtendedTag = MIN ( > + PciExpressExtendedTag, > + > PciExpressConfigurationTable->ExtendedTag > + ); > + // > + // check for any invalid platform policy request for the device; if true > than > + // align with the device capability value. Else align as per platform > request > + // > + if (PciDevice->SetupExtTag > PciExpressConfigurationTable->ExtendedTag) { > + // > + // setup the device Extended Tag to common value supported by all the > devices > + // > + PciDevice->SetupExtTag = PciExpressConfigurationTable->ExtendedTag; > + } > + // > + // if the platform policy is to downgrade the device's Extended Tag value > than > + // all the other devices in the PCI tree including the root bridge will > be align > + // with this device override value > + // > + if (PciDevice->SetupExtTag < PciExpressConfigurationTable->ExtendedTag) { > + PciExpressConfigurationTable->ExtendedTag = PciDevice->SetupExtTag; > + } > + } else { > + // > + // in case of RCiEP devices or the bridge device without any child, > overrule > + // the Extended Tag device policy if it does not match with its > capability > + // > + PciDevice->SetupExtTag = MIN ( > + PciDevice->SetupExtTag, > + PciExpressExtendedTag > + ); > + } > + > + DEBUG (( > + DEBUG_INFO, > + "ExtTag: %d [cap:%d],", > + PciDevice->SetupExtTag, > + PciExpressExtendedTag > + )); > + return EFI_SUCCESS; > +} > + > +/** > + Additional routine to setup the PCI Express feature Extended Tag in > +complaince > + with the PCI Express Base specification Revision, a common value for > +all the > + devices in the PCI hierarchy. > + > + @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 > +AlignExtTag ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciExpressConfigurationTable > + ) > +{ > + if (PciExpressConfigurationTable) { > + // > + // align the Extended Tag value to a common value among all the devices > + // > + PciDevice->SetupExtTag = MIN ( > + PciDevice->SetupExtTag, > + PciExpressConfigurationTable->ExtendedTag > + ); > + } > + > + DEBUG (( > + DEBUG_INFO, > + "ExtTag: %d,", > + PciDevice->SetupExtTag > + )); > + return EFI_SUCCESS; > +} > + > +/** > + Program the PCI Device Control 2 register for 10b Extended Tag value, > +or the > + Device Control register for 5b/8b Extended Tag 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 > +ProgramExtTag ( > + IN PCI_IO_DEVICE *PciDevice, > + IN VOID *PciExFeatureConfiguration > + ) > +{ > + PCI_REG_PCIE_DEVICE_CONTROL DevCtl; > + PCI_REG_PCIE_DEVICE_CONTROL2 DevCtl2; > + UINT32 Offset; > + UINT32 Offset2; > + BOOLEAN OverrideDevCtl; > + BOOLEAN OverrideDevCtl2; > + EFI_STATUS Status; > + EFI_TPL OldTpl; > + > + // > + // read the Device Control register for the Extended Tag Field Enable > + // > + DevCtl.Uint16 = 0; > + Offset = PciDevice->PciExpressCapabilityOffset + > + OFFSET_OF (PCI_CAPABILITY_PCIEXP, DeviceControl); Status > + = PciDevice->PciIo.Pci.Read ( > + &PciDevice->PciIo, > + EfiPciIoWidthUint16, > + Offset, > + 1, > + &DevCtl.Uint16 > + ); > + ASSERT (Status == EFI_SUCCESS); > + > + OverrideDevCtl = FALSE; > + // > + // read the Device COntrol 2 register for the 10-Bit Tag Requester > + Enable // > + DevCtl2.Uint16 = 0; > + Offset2 = PciDevice->PciExpressCapabilityOffset + > + OFFSET_OF (PCI_CAPABILITY_PCIEXP, DeviceControl2); > + Status = PciDevice->PciIo.Pci.Read ( > + &PciDevice->PciIo, > + EfiPciIoWidthUint16, > + Offset2, > + 1, > + &DevCtl2.Uint16 > + ); > + ASSERT (Status == EFI_SUCCESS); > + > + OverrideDevCtl2 = FALSE; > + > + if (PciDevice->SetupExtTag == EFI_PCI_EXPRESS_EXTENDED_TAG_5BIT) { > + if (DevCtl.Bits.ExtendedTagField) { > + DevCtl.Bits.ExtendedTagField = 0; > + OverrideDevCtl = TRUE; > + } > + > + if (DevCtl2.Bits.TenBitTagRequesterEnable) { > + DevCtl2.Bits.TenBitTagRequesterEnable = 0; > + OverrideDevCtl2 = TRUE; > + } > + } > + if (PciDevice->SetupExtTag == EFI_PCI_EXPRESS_EXTENDED_TAG_8BIT) { > + if (!DevCtl.Bits.ExtendedTagField) { > + DevCtl.Bits.ExtendedTagField = 1; > + OverrideDevCtl = TRUE; > + } > + if (DevCtl2.Bits.TenBitTagRequesterEnable) { > + DevCtl2.Bits.TenBitTagRequesterEnable = 0; > + OverrideDevCtl2 = TRUE; > + } > + } > + if (PciDevice->SetupExtTag == EFI_PCI_EXPRESS_EXTENDED_TAG_10BIT) { > + if (!DevCtl2.Bits.TenBitTagRequesterEnable) { > + DevCtl2.Bits.TenBitTagRequesterEnable = 1; > + OverrideDevCtl2 = TRUE; > + } > + } > + > + if (OverrideDevCtl) { > + > + DEBUG (( DEBUG_INFO, "ExtTag=%d,", DevCtl.Bits.ExtendedTagField)); > + > + // > + // 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, > + &DevCtl.Uint16 > + ); > + // > + // Restore TPL to its original level > + // > + gBS->RestoreTPL (OldTpl); > + > + if (!EFI_ERROR(Status)) { > + PciDevice->PciExpressCapabilityStructure.DeviceControl.Uint16 = > DevCtl.Uint16; > + } else { > + ReportPciWriteError (PciDevice->BusNumber, PciDevice->DeviceNumber, > PciDevice->FunctionNumber, Offset); > + } > + } else { > + DEBUG (( DEBUG_INFO, "no ExtTag (%d),", > + DevCtl.Bits.ExtendedTagField)); } > + > + if (OverrideDevCtl2) { > + > + DEBUG (( DEBUG_INFO, "10bExtTag=%d,", > + DevCtl2.Bits.TenBitTagRequesterEnable)); > + > + // > + // 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, > + Offset2, > + 1, > + &DevCtl2.Uint16 > + ); > + // > + // Restore TPL to its original level > + // > + gBS->RestoreTPL (OldTpl); > + > + if (!EFI_ERROR(Status)) { > + PciDevice->PciExpressCapabilityStructure.DeviceControl2.Uint16 = > DevCtl2.Uint16; > + } else { > + ReportPciWriteError (PciDevice->BusNumber, PciDevice->DeviceNumber, > PciDevice->FunctionNumber, Offset2); > + } > + } else { > + DEBUG (( DEBUG_INFO, "no 10bExtTag (%d),", > + DevCtl2.Bits.TenBitTagRequesterEnable)); > + } > + > + return Status; > +} > + > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > index 374fe49..1cfca54 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > @@ -243,4 +243,54 @@ ProgramLtr ( > IN VOID *PciExFeatureConfiguration > ); > > +/** > + The main routine to setup the PCI Express feature Extended Tag 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 PciFeaturesConfigurationTable pointer to > + PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > + > + @retval EFI_SUCCESS setup of PCI feature LTR is > successful. > +**/ > +EFI_STATUS > +SetupExtTag ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciFeaturesConfigurationTable > + ); > + > +/** > + Additional routine to setup the PCI Express feature Extended Tag in > +complaince > + with the PCI Express Base specification Revision, a common value for > +all the > + devices in the PCI hierarchy. > + > + @param PciDevice A pointer to the PCI_IO_DEVICE. > + @param PciFeaturesConfigurationTable pointer to > + PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > + > + @retval EFI_SUCCESS setup of PCI feature LTR is > successful. > +**/ > +EFI_STATUS > +AlignExtTag ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciFeaturesConfigurationTable > + ); > + > +/** > + Program the PCI Device Control 2 register for 10b Extended Tag value, > +or the > + Device Control register for 5b/8b Extended Tag 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 > +ProgramExtTag ( > + IN PCI_IO_DEVICE *PciDevice, > + IN VOID *PciExFeatureConfiguration > + ); > + > #endif > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > index bdeb0d2..58d3780 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > @@ -38,7 +38,7 @@ EFI_PCI_EXPRESS_PLATFORM_POLICY > mPciExpressPlatformPolicy = { > // > // support for PCI Express feature - Extended Tag > // > - FALSE, > + TRUE, > // > // support for PCI Express feature - Relax Order > // > @@ -140,6 +140,15 @@ PCI_EXPRESS_FEATURE_INITIALIZATION_POINT > mPciExpressFeatureInitializationList[] > }, > { > PciExpressFeatureProgramPhase, PciExpressLtr, ProgramLtr > + }, > + { > + PciExpressFeatureSetupPhase, PciExpressExtTag, SetupExtTag > + }, > + { > + PciExpressFeatureEntendedSetupPhase, PciExpressExtTag, AlignExtTag > + }, > + { > + PciExpressFeatureProgramPhase, PciExpressExtTag, ProgramExtTag > } > }; > > @@ -675,6 +684,10 @@ CreatePciRootBridgeDeviceNode ( > // tree > // > PciConfigTable->AtomicOpRoutingSupported = TRUE; > + // > + // start by assuming the Extended Tag is 10b Requester capable > + // > + PciConfigTable->ExtendedTag = > EFI_PCI_EXPRESS_EXTENDED_TAG_10BIT; > } > > RootBridgeNode->PciExFeaturesConfigurationTable = PciConfigTable; diff -- > git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > index 5dded7c..c7cc7e5 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > @@ -94,6 +94,10 @@ struct > _PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE { > // the AtomicOp of the EP device > // > BOOLEAN AtomicOpRoutingSupported; > + // > + // to configure a common extended tag size for all the childs of a > + root port // > + UINT8 ExtendedTag; > }; > > // > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > index 83b3aa7..98d9875 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > @@ -368,6 +368,12 @@ SetupDefaultPciExpressDevicePolicy ( > > PciDevice->SetupLtr = FALSE; > > + if (mPciExpressPlatformPolicy.ExtTag) { > + PciDevice->SetupExtTag = EFI_PCI_EXPRESS_EXTENDED_TAG_AUTO; > + } else { > + PciDevice->SetupExtTag = EFI_PCI_EXPRESS_NOT_APPLICABLE; } > + > } > > /** > @@ -502,6 +508,15 @@ GetPciExpressDevicePolicy ( > PciDevice->SetupLtr = FALSE; > } > > + // > + // set the device-specifci policy for the PCI Express feature Extended > Tag > + // > + if (mPciExpressPlatformPolicy.ExtTag) { > + PciDevice->SetupExtTag = PciExpressDevicePolicy.DeviceCtlExtTag; > + } else { > + PciDevice->SetupExtTag = EFI_PCI_EXPRESS_NOT_APPLICABLE; > + } > + > > DEBUG (( > DEBUG_INFO, > @@ -668,6 +683,19 @@ GetPciExpressCto ( > return EFI_PCI_EXPRESS_NOT_APPLICABLE; } > > +EFI_PCI_EXPRESS_EXTENDED_TAG > +GetPciExpressExtTag ( > + IN PCI_IO_DEVICE *PciDevice > + ) > +{ > + if (PciDevice- > >PciExpressCapabilityStructure.DeviceControl2.Bits.TenBitTagRequesterEnable) > { > + return EFI_PCI_EXPRESS_EXTENDED_TAG_10BIT; > + } else if (PciDevice- > >PciExpressCapabilityStructure.DeviceControl.Bits.ExtendedTagField) { > + return EFI_PCI_EXPRESS_EXTENDED_TAG_8BIT; > + } else { > + return EFI_PCI_EXPRESS_EXTENDED_TAG_5BIT; > + } > +} > > /** > Notifies the platform about the current PCI Express state of the device. > @@ -768,6 +796,14 @@ PciExpressPlatformNotifyDeviceState ( > PciExDeviceConfiguration.DeviceCtl2LTR = > EFI_PCI_EXPRESS_NOT_APPLICABLE; > } > > + // > + // get the device-specific state for the PCie Extended Tag in the > + function // if (mPciExpressPlatformPolicy.ExtTag) { > + PciExDeviceConfiguration.DeviceCtlExtTag = GetPciExpressExtTag > + (PciDevice); } else { > + PciExDeviceConfiguration.DeviceCtlExtTag = > + EFI_PCI_EXPRESS_NOT_APPLICABLE; } > > if (mPciExPlatformProtocol != NULL) { > return mPciExPlatformProtocol->NotifyDeviceState ( > -- > 2.21.0.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54086): https://edk2.groups.io/g/devel/message/54086 Mute This Topic: https://groups.io/mt/71063695/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
