This patch is also uploaded in the following Repo for review:- https://github.com/ashrafj/edk2-staging/commit/8575d730644eda27a4b88888b0eb05ee1f804b35
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.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 feature Relax Ordering > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2313 > > The code changes are made to enable the configuration of new PCI feature > Relax Ordering (OR), which enables the PCI function to initiate requests if > it does > not require strong write ordering for its transactions; as per the PCI Base > Specification 4 Revision 1. > > The code changes are made to configure only those PCI devices which are > requested to override by platform through the new PCI Platform protocol > interface for device-specific policies. > > 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 | 2 ++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c | 78 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h | 26 > ++++++++++++++++++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c | 44 > ++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 150 insertions(+) > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > index 38abd20..9f017b7 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > @@ -82,6 +82,7 @@ typedef enum { > #include "PciHotPlugSupport.h" > #include "PciLib.h" > #include "PciPlatformSupport.h" > +#include "PciFeatureSupport.h" > > #define VGABASE1 0x3B0 > #define VGALIMIT1 0x3BB > @@ -291,6 +292,7 @@ struct _PCI_IO_DEVICE { > // > UINT8 SetupMPS; > UINT8 SetupMRRS; > + PCI_FEATURE_POLICY SetupRO; > }; > > #define PCI_IO_DEVICE_FROM_PCI_IO_THIS(a) \ diff --git > a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > index 614285f..a60cb42 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > @@ -911,6 +911,81 @@ OverrideMaxReadReqSize ( > return Status; > } > > +/** > + Overrides the PCI Device Control register Relax Order register 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 > +OverrideRelaxOrder ( > + IN PCI_IO_DEVICE *PciDevice > + ) > +{ > + PCI_REG_PCIE_DEVICE_CONTROL PcieDev; > + UINT32 Offset; > + EFI_STATUS Status; > + EFI_TPL OldTpl; > + > + PcieDev.Uint16 = 0; > + Offset = PciDevice->PciExpressCapabilityOffset + > + OFFSET_OF (PCI_CAPABILITY_PCIEXP, DeviceControl); > + Status = PciDevice->PciIo.Pci.Read ( > + &PciDevice->PciIo, > + EfiPciIoWidthUint16, > + Offset, > + 1, > + &PcieDev.Uint16 > + ); > + if (EFI_ERROR(Status)){ > + DEBUG (( DEBUG_ERROR, "Unexpected DeviceControl register (0x%x) read > error!", > + Offset > + )); > + return Status; > + } > + if (PciDevice->SetupRO.Override > + && PcieDev.Bits.RelaxedOrdering != PciDevice->SetupRO.Act > + ) { > + PcieDev.Bits.RelaxedOrdering = PciDevice->SetupRO.Act; > + DEBUG (( DEBUG_INFO, "RO=%d,", PciDevice->SetupRO.Act)); > + > + // > + // 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, > + &PcieDev.Uint16 > + ); > + // > + // Restore TPL to its original level > + // > + gBS->RestoreTPL (OldTpl); > + > + if (!EFI_ERROR(Status)) { > + PciDevice->PciExpStruct.DeviceControl.Uint16 = PcieDev.Uint16; > + } else { > + DEBUG (( DEBUG_ERROR, "Unexpected DeviceControl register (0x%x) write > error!", > + Offset > + )); > + } > + } else { > + DEBUG (( DEBUG_INFO, "No write of RO,", PciDevice->SetupRO.Act)); > + } > + > + return Status; > +} > + > /** > helper routine to dump the PCIe Device Port Type **/ @@ -1122,6 +1197,9 > @@ ProgramDevicePciFeatures ( > if (SetupMaxReadReqSize ()) { > Status = OverrideMaxReadReqSize (PciDevice); > } > + if (SetupRelaxOrder ()) { > + Status = OverrideRelaxOrder (PciDevice); } > DEBUG (( DEBUG_INFO, "\n")); > return Status; > } > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > index 96ee6ff..5044dc2 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > @@ -40,6 +40,11 @@ typedef struct > _OTHER_PCI_FEATURES_CONFIGURATION_TABLE > OTHER_PCI_FEATURES_CONFI // typedef struct > _PCI_FEATURE_CONFIGURATION_COMPLETION_LIST > PCI_FEATURE_CONFIGURATION_COMPLETION_LIST; > > +// > +// define the data type for the PCI feature policy support // typedef > +struct _PCI_FEATURE_POLICY PCI_FEATURE_POLICY; > + > // > // Signature value for the PCI Root Port node // @@ -151,6 +156,27 @@ > typedef enum { > > }PCI_FEATURE_CONFIGURATION_PHASE; > > +// > +// declaration for the data type to harbor the PCI feature policies // > +struct _PCI_FEATURE_POLICY { > + // > + // if set, it indicates the feature should be enabled > + // if clear, it indicates the feature should be disabled > + // > + UINT8 Act : 1; > + // > + // this field will be specific to feature, it can be implementation > +specific > + // or it can be reserved and remain unused > + // > + UINT8 Support : 6; > + // > + // if set indicates override the feature policy defined by the > +members above > + // if clear it indicates that this feature policy should be ignored > +completely > + // this means the above two members should not be used > + // > + UINT8 Override : 1; > +}; > > /** > Main routine to indicate platform selection of any of the other PCI > features > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > index f032b5d..f1e7039 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > @@ -470,6 +470,45 @@ TranslateMrrsSetupValueToPci ( > } > } > > +/** > + Routine to set the device-specific policy for the PCI feature Relax > +Ordering > + > + @param RelaxOrder value corresponding to data type > EFI_PCI_CONF_RELAX_ORDER > + @param PciDevice A pointer to PCI_IO_DEVICE > +**/ > +VOID > +SetDevicePolicyRelaxOrder ( > + IN EFI_PCI_CONF_RELAX_ORDER RelaxOrder, > + OUT PCI_IO_DEVICE *PciDevice > + ) > +{ > + // > + // implementation specific rules for the usage of PCI_FEATURE_POLICY > +members > + // exclusively for the PCI Feature Relax Ordering (RO) > + // > + // .Override = 0 to skip this PCI feature RO for the PCI device > + // .Override = 1 to program this RO PCI feature > + // .Act = 1 to enable the RO in the PCI device > + // .Act = 0 to disable the RO in the PCI device > + // > + switch (RelaxOrder) { > + case EFI_PCI_CONF_RO_AUTO: > + PciDevice->SetupRO.Override = 0; > + break; > + case EFI_PCI_CONF_RO_DISABLE: > + PciDevice->SetupRO.Override = 1; > + PciDevice->SetupRO.Act = 0; > + break; > + case EFI_PCI_CONF_RO_ENABLE: > + PciDevice->SetupRO.Override = 1; > + PciDevice->SetupRO.Act = 1; > + break; > + default: > + PciDevice->SetupRO.Override = 0; > + break; > + } > +} > + > /** > Generic routine to setup the PCI features as per its predetermined > defaults. > **/ > @@ -480,6 +519,7 @@ SetupDefaultsDevicePlatformPolicy ( { > PciDevice->SetupMPS = EFI_PCI_CONF_MAX_PAYLOAD_SIZE_AUTO; > PciDevice->SetupMRRS = EFI_PCI_CONF_MAX_READ_REQ_SIZE_AUTO; > + PciDevice->SetupRO.Override = 0; > } > > /** > @@ -517,6 +557,10 @@ GetPciDevicePlatformPolicyEx ( > // > PciIoDevice->SetupMPS = PciPlatformExtendedPolicy.DeviceCtlMPS; > PciIoDevice->SetupMRRS = PciPlatformExtendedPolicy.DeviceCtlMRRS; > + // > + // set device specific policy for Relax Ordering > + // > + SetDevicePolicyRelaxOrder > + (PciPlatformExtendedPolicy.DeviceCtlRelaxOrder, PciIoDevice); > > DEBUG (( > DEBUG_INFO, "[device policy: platform]" > -- > 2.21.0.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#50549): https://edk2.groups.io/g/devel/message/50549 Mute This Topic: https://groups.io/mt/55164383/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
