This patch is also uploaded in the following Repo for review:- https://github.com/ashrafj/edk2-staging/commit/d16d7ea7df972fb711bca22266743c3602cf450b
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 09/12] > PciBusDxe: New PCI feature Max_Read_Req_Size > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2194 > > The code changes are made to enable the configuration of new PCI feature > Max_Read_Req_Size (MRRS), which defines the memory read request size for > the PCI transactions, as per the PCI Base Specification 4 Revision 1. > > The code changes are made to configure a common value that is applicable to > all the child nodes originating from the primary parent root port of the root > bridge instance, based on following 3 criteria:- > (1) if platform defines MRRS device policy for any one PCI device in the > tree than align all the devices in the PCI tree to that same value > (2) if platform does not provide device policy for any of the devices in > the PCI tree than setup the MRRS value equivalent to MPS value for > all PCI devices to meet the criteria for the isochronous traffic > (3) if platform does not provide device policy for any of the devices in > the PCI tree and platform firmware policy has not selected the PCI > bus driver to configure the MPS; than configuration of the MRRS is > performed based on highest common value of the MPS advertized in the > PCI device capability registers of the PCI devices > > This programming of MRRS gets the device-specific platform policy using the > new PCI Platform Protocol interface, defined in the below record:- > 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/PciFeatureSupport.c | 204 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h | 9 +++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c | 59 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h | 32 > ++++++++++++++++++++++++++++++++ > 5 files changed, 305 insertions(+) > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > index 065ae54..38abd20 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > @@ -290,6 +290,7 @@ struct _PCI_IO_DEVICE { > // Other PCI features setup flags > // > UINT8 SetupMPS; > + UINT8 SetupMRRS; > }; > > #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 8fdaa05..614285f 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > @@ -650,6 +650,121 @@ ProcessMaxPayloadSize ( > return EFI_SUCCESS; > } > > +/** > + The main routine which process the PCI feature Max_Read_Req_Size as > +per the > + device-specific platform policy, as well as in complaince with the > +PCI Base > + specification Revision 4, that aligns the value for the entire PCI > +heirarchy > + starting from its physical PCI Root port / Bridge device. > + > + @param PciDevice A pointer to the PCI_IO_DEVICE. > + @param PciConfigPhase for the PCI feature configuration > phases: > + PciFeatureGetDevicePolicy & > + PciFeatureSetupPhase @param PciFeaturesConfigurationTable pointer to > + OTHER_PCI_FEATURES_CONFIGURATION_TABLE > + > + @retval EFI_SUCCESS processing of PCI feature > Max_Read_Req_Size > + is successful. > +**/ > +EFI_STATUS > +ProcessMaxReadReqSize ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_FEATURE_CONFIGURATION_PHASE PciConfigPhase, > + IN OTHER_PCI_FEATURES_CONFIGURATION_TABLE > +*PciFeaturesConfigurationTable > + ) > +{ > + PCI_REG_PCIE_DEVICE_CAPABILITY PciDeviceCap; > + UINT8 MrrsValue; > + > + PciDeviceCap.Uint32 = > + PciDevice->PciExpStruct.DeviceCapability.Uint32; > + > + if (PciConfigPhase == PciFeatureGetDevicePolicy) { > + if (SetupMrrsAsPerDeviceCapability (PciDevice->SetupMRRS)) { > + // > + // The maximum read request size is not the data packet size of the > TLP, > + // but the memory read request size, and set to the function as a > requestor > + // to not exceed this limit. > + // However, for the PCI device capable of isochronous traffic; this > memory > read > + // request size should not extend beyond the Max_Payload_Size. Thus, in > case if > + // device policy return by platform indicates to set as per device > capability > + // than set as per Max_Payload_Size configuration value > + // > + if (SetupMaxPayloadSize ()) { > + MrrsValue = PciDevice->SetupMPS; > + } else { > + // > + // in case this driver is not required to configure the > Max_Payload_Size > + // than consider programming HCF of the device capability's > Max_Payload_Size > + // in this PCI hierarchy; thus making this an implementation > specific feature > + // which the platform should avoid. For better results, the platform > should > + // make both the Max_Payload_Size & Max_Read_Request_Size to be > configured > + // by this driver > + // > + MrrsValue = (UINT8)PciDeviceCap.Bits.MaxPayloadSize; > + } > + } else { > + // > + // override as per platform based device policy > + // > + MrrsValue = TranslateMrrsSetupValueToPci (PciDevice->SetupMRRS); > + // > + // align this device's Max_Read_Request_Size value to the entire PCI > tree > + // > + if (PciFeaturesConfigurationTable) { > + if (!PciFeaturesConfigurationTable->Lock_Max_Read_Request_Size) { > + PciFeaturesConfigurationTable->Lock_Max_Read_Request_Size = TRUE; > + PciFeaturesConfigurationTable->Max_Read_Request_Size = MrrsValue; > + } else { > + // > + // in case of another user enforced value of MRRS within the same > tree, > + // pick the smallest between the locked value and this value; to > set > + // across entire PCI tree nodes > + // > + MrrsValue = MIN ( > + MrrsValue, > + PciFeaturesConfigurationTable->Max_Read_Request_Size > + ); > + PciFeaturesConfigurationTable->Max_Read_Request_Size = MrrsValue; > + } > + } > + } > + // > + // align this device's Max_Read_Request_Size to derived configuration > value > + // > + PciDevice->SetupMRRS = MrrsValue; > + > + } > + > + // > + // align the Max_Read_Request_Size of the PCI tree based on 3 conditions: > + // first, if user defines MRRS for any one PCI device in the tree > + than align // all the devices in the PCI tree. > + // second, if user override is not define for this PCI tree than > + setup the MRRS // based on MPS value of the tree to meet the criteria > + for the isochronous // traffic. > + // third, if no user override, or platform firmware policy has not > + selected // this PCI bus driver to configure the MPS; than configure > + the MRRS to a // highest common value of PCI device capability for > + the MPS found among all // the PCI devices in this tree // if > + (PciFeaturesConfigurationTable) { > + if (PciFeaturesConfigurationTable->Lock_Max_Read_Request_Size) { > + PciDevice->SetupMRRS = PciFeaturesConfigurationTable- > >Max_Read_Request_Size; > + } else { > + if (SetupMaxPayloadSize ()) { > + PciDevice->SetupMRRS = PciDevice->SetupMPS; > + } else { > + PciDevice->SetupMRRS = MIN ( > + PciDevice->SetupMRRS, > + > PciFeaturesConfigurationTable->Max_Read_Request_Size > + ); > + } > + PciFeaturesConfigurationTable->Max_Read_Request_Size = PciDevice- > >SetupMRRS; > + } > + } > + DEBUG (( DEBUG_INFO, "MRRS: %d,", PciDevice->SetupMRRS)); > + > + return EFI_SUCCESS; > +} > + > /** > Overrides the PCI Device Control register MaxPayloadSize register field; if > the hardware value is different than the intended value. > @@ -723,6 +838,79 @@ OverrideMaxPayloadSize ( > return Status; > } > > +/** > + Overrides the PCI Device Control register Max_Read_Req_Size 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 > controller. > + @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 > +OverrideMaxReadReqSize ( > + 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 (PcieDev.Bits.MaxReadRequestSize != PciDevice->SetupMRRS) { > + PcieDev.Bits.MaxReadRequestSize = PciDevice->SetupMRRS; > + DEBUG (( DEBUG_INFO, "Max_Read_Request_Size: %d,", > + PciDevice->SetupMRRS)); > + > + // > + // 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 MRRS=%d,", > + PciDevice->SetupMRRS)); } > + > + return Status; > +} > + > /** > helper routine to dump the PCIe Device Port Type **/ @@ -820,6 +1008,17 > @@ SetupDevicePciFeatures ( > OtherPciFeaturesConfigTable > ); > } > + // > + // implementation specific rule:- the MRRS of any PCI device should > + be processed // only after the MPS is processed for that device // > + if (SetupMaxReadReqSize ()) { > + Status = ProcessMaxReadReqSize ( > + PciDevice, > + PciConfigPhase, > + OtherPciFeaturesConfigTable > + ); > + } > DEBUG ((DEBUG_INFO, "]\n")); > return Status; > } > @@ -920,6 +1119,9 @@ ProgramDevicePciFeatures ( > if (SetupMaxPayloadSize ()) { > Status = OverrideMaxPayloadSize (PciDevice); > } > + if (SetupMaxReadReqSize ()) { > + Status = OverrideMaxReadReqSize (PciDevice); } > DEBUG (( DEBUG_INFO, "\n")); > return Status; > } > @@ -1035,6 +1237,8 @@ AddPrimaryRootPortNode ( > if (PciConfigTable) { > PciConfigTable->ID = PortNumber; > PciConfigTable->Max_Payload_Size = > PCIE_MAX_PAYLOAD_SIZE_4096B; > + PciConfigTable->Max_Read_Request_Size = > PCIE_MAX_READ_REQ_SIZE_4096B; > + PciConfigTable->Lock_Max_Read_Request_Size = FALSE; > } > RootPortNode->OtherPciFeaturesConfigurationTable = PciConfigTable; > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > index e5ac2a3..96ee6ff 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > @@ -84,6 +84,15 @@ struct _OTHER_PCI_FEATURES_CONFIGURATION_TABLE > { > // size among all the PCI devices in the PCI hierarchy > // > UINT8 Max_Payload_Size; > + // > + // to configure the PCI feature maximum read request size to maintain > + the memory // requester size among all the PCI devices in the PCI > + hierarchy // > + UINT8 Max_Read_Request_Size; > + // > + // lock the Max_Read_Request_Size for the entire PCI tree of a root > + port // > + BOOLEAN Lock_Max_Read_Request_Size; > }; > > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > index 99badd6..f032b5d 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > @@ -379,6 +379,29 @@ SetupMpsAsPerDeviceCapability ( > } > } > > +/** > + Helper routine to indicate whether the given PCI device specific > +policy value > + dictates to override the Max_Read_Req_Size to a particular value, or > +set as per > + device capability. > + > + @param MRRS Input device-specific policy should be in terms of type > + EFI_PCI_CONF_MAX_READ_REQ_SIZE > + > + @retval TRUE Setup Max_Read_Req_Size as per device capability > + FALSE override as per device-specific platform policy > +**/ > +BOOLEAN > +SetupMrrsAsPerDeviceCapability ( > + IN UINT8 MRRS > +) > +{ > + if (MRRS == EFI_PCI_CONF_MAX_READ_REQ_SIZE_AUTO) { > + return TRUE; > + } else { > + return FALSE; > + } > +} > + > /** > Routine to translate the given device-specific platform policy from type > EFI_PCI_CONF_MAX_PAYLOAD_SIZE to HW-specific value, as per PCI Base > Specification @@ -413,6 +436,40 @@ TranslateMpsSetupValueToPci ( > } > } > > +/** > + Routine to translate the given device-specific platform policy from > +type > + EFI_PCI_CONF_MAX_READ_REQ_SIZE to HW-specific value, as per PCI Base > +Specification > + Revision 4.0; for the PCI feature Max_Read_Req_Size. > + > + @param MRRS Input device-specific policy should be in terms of type > + EFI_PCI_CONF_MAX_READ_REQ_SIZE > + > + @retval Range values for the Max_Read_Req_Size as defined in the > PCI > + Base Specification 4.0 **/ > +UINT8 > +TranslateMrrsSetupValueToPci ( > + IN UINT8 MRRS > +) > +{ > + switch (MRRS) { > + case EFI_PCI_CONF_MAX_READ_REQ_SIZE_128B: > + return PCIE_MAX_READ_REQ_SIZE_128B; > + case EFI_PCI_CONF_MAX_READ_REQ_SIZE_256B: > + return PCIE_MAX_READ_REQ_SIZE_256B; > + case EFI_PCI_CONF_MAX_READ_REQ_SIZE_512B: > + return PCIE_MAX_READ_REQ_SIZE_512B; > + case EFI_PCI_CONF_MAX_READ_REQ_SIZE_1024B: > + return PCIE_MAX_READ_REQ_SIZE_1024B; > + case EFI_PCI_CONF_MAX_READ_REQ_SIZE_2048B: > + return PCIE_MAX_READ_REQ_SIZE_2048B; > + case EFI_PCI_CONF_MAX_READ_REQ_SIZE_4096B: > + return PCIE_MAX_READ_REQ_SIZE_4096B; > + default: > + return PCIE_MAX_READ_REQ_SIZE_128B; > + } > +} > + > /** > Generic routine to setup the PCI features as per its predetermined > defaults. > **/ > @@ -422,6 +479,7 @@ SetupDefaultsDevicePlatformPolicy ( > ) > { > PciDevice->SetupMPS = EFI_PCI_CONF_MAX_PAYLOAD_SIZE_AUTO; > + PciDevice->SetupMRRS = EFI_PCI_CONF_MAX_READ_REQ_SIZE_AUTO; > } > > /** > @@ -458,6 +516,7 @@ GetPciDevicePlatformPolicyEx ( > // platform chipset policies are returned for this PCI device > // > PciIoDevice->SetupMPS = PciPlatformExtendedPolicy.DeviceCtlMPS; > + PciIoDevice->SetupMRRS = PciPlatformExtendedPolicy.DeviceCtlMRRS; > > DEBUG (( > DEBUG_INFO, "[device policy: platform]" > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h > index 786c00d..8ed3836 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h > @@ -141,6 +141,22 @@ SetupMpsAsPerDeviceCapability ( > IN UINT8 MPS > ); > > +/** > + Helper routine to indicate whether the given PCI device specific > +policy value > + dictates to override the Max_Read_Req_Size to a particular value, or > +set as per > + device capability. > + > + @param MRRS Input device-specific policy should be in terms of type > + EFI_PCI_CONF_MAX_READ_REQ_SIZE > + > + @retval TRUE Setup Max_Read_Req_Size as per device capability > + FALSE override as per device-specific platform policy > +**/ > +BOOLEAN > +SetupMrrsAsPerDeviceCapability ( > + IN UINT8 MRRS > +); > + > /** > Routine to translate the given device-specific platform policy from type > EFI_PCI_CONF_MAX_PAYLOAD_SIZE to HW-specific value, as per PCI Base > Specification @@ -156,4 +172,20 @@ UINT8 TranslateMpsSetupValueToPci ( > IN UINT8 MPS > ); > + > +/** > + Routine to translate the given device-specific platform policy from > +type > + EFI_PCI_CONF_MAX_READ_REQ_SIZE to HW-specific value, as per PCI Base > +Specification > + Revision 4.0; for the PCI feature Max_Read_Req_Size. > + > + @param MRRS Input device-specific policy should be in terms of type > + EFI_PCI_CONF_MAX_READ_REQ_SIZE > + > + @retval Range values for the Max_Read_Req_Size as defined in the > PCI > + Base Specification 4.0 **/ > +UINT8 > +TranslateMrrsSetupValueToPci ( > + IN UINT8 MRRS > +); > #endif > -- > 2.21.0.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#50548): https://edk2.groups.io/g/devel/message/50548 Mute This Topic: https://groups.io/mt/55163466/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
