From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mx.groups.io with SMTP id smtpd.web10.10740.1581106921104066872 for ; Fri, 07 Feb 2020 12:22:01 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.24, mailfrom: ashraf.javeed@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Feb 2020 12:22:00 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,414,1574150400"; d="scan'208";a="280069359" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by FMSMGA003.fm.intel.com with ESMTP; 07 Feb 2020 12:22:00 -0800 Received: from fmsmsx121.amr.corp.intel.com (10.18.125.36) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 7 Feb 2020 12:21:59 -0800 Received: from bgsmsx109.gar.corp.intel.com (10.223.4.211) by fmsmsx121.amr.corp.intel.com (10.18.125.36) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 7 Feb 2020 12:21:59 -0800 Received: from bgsmsx101.gar.corp.intel.com ([169.254.1.155]) by BGSMSX109.gar.corp.intel.com ([169.254.10.142]) with mapi id 14.03.0439.000; Sat, 8 Feb 2020 01:51:56 +0530 From: "Javeed, Ashraf" To: "devel@edk2.groups.io" , "Javeed, Ashraf" CC: "Wang, Jian J" , "Wu, Hao A" , "Ni, Ray" Subject: Re: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 05/12] PciBusDxe: New PCI Express feature Relax Ordering Thread-Topic: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 05/12] PciBusDxe: New PCI Express feature Relax Ordering Thread-Index: AQHV3fIU6jAKU4eLuE+bJ5qsIUBPpqgQLG1w Date: Fri, 7 Feb 2020 20:21:55 +0000 Message-ID: <95C5C2B113DE604FB208120C742E9824579AAFC3@BGSMSX101.gar.corp.intel.com> References: <20200207200447.10536-1-ashraf.javeed@intel.com> <15F1378301D514E4.15938@groups.io> In-Reply-To: <15F1378301D514E4.15938@groups.io> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzExN2Y0Y2EtYWMyYS00YmFiLWE2MmEtNWJlNjNhZWFlN2FkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiT3J0bTlRUEh1aGN3cllYQlwvVHFUUlpEVGR0NjZkRXNXd1BuT0laTzlCSkxwdVwvTjdxVk94NURuNFQ0SnBjVmt2In0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.223.10.10] MIME-Version: 1.0 Return-Path: ashraf.javeed@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable This patch can also be viewed in the following repo:- https://github.com/ashrafj/edk2-staging/commit/27d11f3bbba23ff8b55d67da3cc= 50f8ee6029103 Thanks Ashraf > -----Original Message----- > From: 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 ; Wu, Hao A = ; > Ni, Ray > Subject: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 05/12] > PciBusDxe: New PCI Express feature Relax Ordering >=20 > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2313 >=20 > The code changes are made to enable the configuration of PCI Express fea= ture > Relax Ordering (OR), that enables the PCI function to initiate requests = if it does > not require strong write ordering for its transact- ions; as per the PCI= Express > Base Specification 4 Revision 1. >=20 > The code changes are made to configure only those PCI devices which are > requested by platform for override, through the new PCI Express Platform > protocol interface for device-specific policies. >=20 > Signed-off-by: Ashraf Javeed > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Ray Ni > --- > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 4 ++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c | 70 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h | 18 > ++++++++++++++++++ MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > | 5 ++++- MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c | 61 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 157 insertions(+), 1 deletion(-) >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > index 77b44c0..d3d795d 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > @@ -287,8 +287,12 @@ struct _PCI_IO_DEVICE { > // This field is used to support this case. > // > UINT16 BridgeIoAlignment; > + // > + // PCI Express features setup flags > + // > UINT8 SetupMPS; > UINT8 SetupMRRS; > + PCI_FEATURE_POLICY SetupRO; > }; >=20 > #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 2810158..3262b76 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > @@ -381,3 +381,73 @@ ProgramMaxReadReqSize ( > return Status; > } >=20 > +/** > + 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 th= e 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 > +ProgramRelaxOrder ( > + IN PCI_IO_DEVICE *PciDevice, > + IN VOID *PciExFeatureConfiguration > + ) > +{ > + PCI_REG_PCIE_DEVICE_CONTROL PcieDev; > + UINT32 Offset; > + EFI_STATUS Status; > + EFI_TPL OldTpl; > + > + PcieDev.Uint16 =3D 0; > + Offset =3D PciDevice->PciExpressCapabilityOffset + > + OFFSET_OF (PCI_CAPABILITY_PCIEXP, DeviceControl); > + Status =3D PciDevice->PciIo.Pci.Read ( > + &PciDevice->PciIo, > + EfiPciIoWidthUint16, > + Offset, > + 1, > + &PcieDev.Uint16 > + ); > + ASSERT (Status =3D=3D EFI_SUCCESS); > + > + if (PciDevice->SetupRO.Override > + && PcieDev.Bits.RelaxedOrdering !=3D PciDevice->SetupRO.Act > + ) { > + PcieDev.Bits.RelaxedOrdering =3D PciDevice->SetupRO.Act; > + DEBUG (( DEBUG_INFO, "RO=3D%d,", PciDevice->SetupRO.Act)); > + > + // > + // Raise TPL to high level to disable timer interrupt while the wri= te operation > completes > + // > + OldTpl =3D gBS->RaiseTPL (TPL_HIGH_LEVEL); > + > + Status =3D 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->PciExpressCapabilityStructure.DeviceControl.Uint16 =3D > PcieDev.Uint16; > + } else { > + ReportPciWriteError (PciDevice->BusNumber, PciDevice->DeviceNumbe= r, > PciDevice->FunctionNumber, Offset); > + } > + } else { > + DEBUG (( DEBUG_INFO, "No RO,", PciDevice->SetupRO.Act)); } > + > + return Status; > +} > + > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > index b43fba7..0d17801 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > @@ -97,4 +97,22 @@ ProgramMaxReadReqSize ( > IN VOID *PciExFeatureConfiguration > ); >=20 > +/** > + 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 th= e 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 > +ProgramRelaxOrder ( > + 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 1caf1f4..267f570 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > @@ -42,7 +42,7 @@ EFI_PCI_EXPRESS_PLATFORM_POLICY > mPciExpressPlatformPolicy =3D { > // > // support for PCI Express feature - Relax Order > // > - FALSE, > + TRUE, > // > // support for PCI Express feature - No-Snoop > // > @@ -113,6 +113,9 @@ PCI_EXPRESS_FEATURE_INITIALIZATION_POINT > mPciExpressFeatureInitializationList[] > }, > { > PciExpressFeatureProgramPhase, PciExpressMrrs, > ProgramMaxReadReqSize > + }, > + { > + PciExpressFeatureProgramPhase, PciExpressRelaxOrder, > ProgramRelaxOrder > } > }; >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > index f74e566..40eb8a3 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > @@ -149,6 +149,45 @@ SetDevicePolicyPciExpressMrrs ( > } > } >=20 > +/** > + Routine to set the device-specific policy for the PCI feature Relax > +Ordering > + > + @param RelaxOrder value corresponding to data type > EFI_PCI_EXPRESS_RELAX_ORDER > + @param PciDevice A pointer to PCI_IO_DEVICE > +**/ > +VOID > +SetDevicePolicyPciExpressRo ( > + IN EFI_PCI_EXPRESS_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 =3D 0 to skip this PCI feature RO for the PCI device > + // .Override =3D 1 to program this RO PCI feature > + // .Act =3D 1 to enable the RO in the PCI device > + // .Act =3D 0 to disable the RO in the PCI device > + // > + switch (RelaxOrder) { > + case EFI_PCI_EXPRESS_RO_AUTO: > + PciDevice->SetupRO.Override =3D 0; > + break; > + case EFI_PCI_EXPRESS_RO_DISABLE: > + PciDevice->SetupRO.Override =3D 1; > + PciDevice->SetupRO.Act =3D 0; > + break; > + case EFI_PCI_EXPRESS_RO_ENABLE: > + PciDevice->SetupRO.Override =3D 1; > + PciDevice->SetupRO.Act =3D 1; > + break; > + default: > + PciDevice->SetupRO.Override =3D 0; > + break; > + } > +} > + > /** > Generic routine to setup the PCI features as per its predetermined de= faults. > **/ > @@ -170,6 +209,8 @@ SetupDefaultPciExpressDevicePolicy ( > PciDevice->SetupMRRS =3D EFI_PCI_EXPRESS_NOT_APPLICABLE; > } >=20 > + PciDevice->SetupRO.Override =3D 0; > + > } >=20 > /** > @@ -259,6 +300,15 @@ GetPciExpressDevicePolicy ( > } else { > PciDevice->SetupMRRS =3D EFI_PCI_EXPRESS_NOT_APPLICABLE; > } > + // > + // set device specific policy for Relax Ordering > + // > + if (mPciExpressPlatformPolicy.RelaxOrder) { > + SetDevicePolicyPciExpressRo (PciExpressDevicePolicy.DeviceCtlRela= xOrder, > PciDevice); > + } else { > + PciDevice->SetupRO.Override =3D 0; > + } > + >=20 > DEBUG (( > DEBUG_INFO, > @@ -438,6 +488,17 @@ PciExpressPlatformNotifyDeviceState ( > } else { > PciExDeviceConfiguration.DeviceCtlMRRS =3D > EFI_PCI_EXPRESS_NOT_APPLICABLE; > } > + // > + // get the device-specific state for the PCIe Relax Order feature // > + if (mPciExpressPlatformPolicy.RelaxOrder) { > + PciExDeviceConfiguration.DeviceCtlRelaxOrder =3D PciDevice- > >PciExpressCapabilityStructure.DeviceControl.Bits.RelaxedOrdering > + ? EFI_PCI_EXPRESS= _RO_ENABLE > + : > + EFI_PCI_EXPRESS_RO_DISABLE; } else { > + PciExDeviceConfiguration.DeviceCtlRelaxOrder =3D > + EFI_PCI_EXPRESS_NOT_APPLICABLE; } > + >=20 > if (mPciExPlatformProtocol !=3D NULL) { > return mPciExPlatformProtocol->NotifyDeviceState ( > -- > 2.21.0.windows.1 >=20 >=20 >=20