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.web09.10828.1581107340878888361 for ; Fri, 07 Feb 2020 12:29: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 orsmga004.jf.intel.com ([10.7.209.38]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Feb 2020 12:29:00 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,414,1574150400"; d="scan'208";a="379503724" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga004.jf.intel.com with ESMTP; 07 Feb 2020 12:29:00 -0800 Received: from fmsmsx605.amr.corp.intel.com (10.18.126.85) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 7 Feb 2020 12:29:00 -0800 Received: from fmsmsx605.amr.corp.intel.com (10.18.126.85) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Fri, 7 Feb 2020 12:28:59 -0800 Received: from bgsmsx109.gar.corp.intel.com (10.223.4.211) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Fri, 7 Feb 2020 12:28: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:58: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 09/12] PciBusDxe: New PCI Express feature LTR Thread-Topic: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 09/12] PciBusDxe: New PCI Express feature LTR Thread-Index: AQHV3fIejFwdewxIYky8mKVKvJb3gagQLlag Date: Fri, 7 Feb 2020 20:28:56 +0000 Message-ID: <95C5C2B113DE604FB208120C742E9824579AB040@BGSMSX101.gar.corp.intel.com> References: <20200207200447.10536-1-ashraf.javeed@intel.com> <15F13785436D3273.18602@groups.io> In-Reply-To: <15F13785436D3273.18602@groups.io> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZmFhMDI0OWYtYjI3ZS00ZmRkLWJlYTktMzc1YmQ1ZWNlMDY5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiTitBM2FGeHo0MzhqS3lmS3Q0VW5FSTg2cTN1WURydlwvQnk0R3FNdFIwbjk5MzdYaHpMdWE3b25FaWpqYk1pc2UifQ== 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/fd03852add274895880971e38f0= 7e30d5fd79863 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 09/12] > PciBusDxe: New PCI Express feature LTR >=20 > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2499 >=20 > This code change enables the PCI Express feature LTR, in compliance with= the > PCI Express Base Specification 5, as well as in accordance its device po= licy, as > follows: > (1) all the devices capability register needs to indicate that LTR mecha= - > nism is supported. ANy device not capable shall lead to all devices > state in LTR disable > (2) if device policy of any device requests LTR mechanism enabled, than > based on above condition (1) is TRUE, all of the devices from root > bridge are enabled. > (3) Even in case of RCiEP device without any root bridge device; if devi= ce > policy requests LTR enabled and device capability does not support L= TR > shall enforce the default LTR disabled state for that device >=20 > This programming of LTR, gets the device-specific platform policy using = the new > PCI Express Platform Protocol interface (ECR version 0.8), defined in th= e below > feature request:- > https://bugzilla.tianocore.org/show_bug.cgi?id=3D1954 >=20 > Signed-off-by: Ashraf Javeed > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Ray Ni > --- > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 + > MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c | 171 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h | 40 > ++++++++++++++++++++++++++++++++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c | 19 > ++++++++++++++++++- > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h | 9 +++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c | 53 > +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 292 insertions(+), 1 deletion(-) >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > index 57ef1b2..7e43a26 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > @@ -296,6 +296,7 @@ struct _PCI_IO_DEVICE { > PCI_FEATURE_POLICY SetupNS; > PCI_FEATURE_POLICY SetupCTO; > EFI_PCI_EXPRESS_ATOMIC_OP SetupAtomicOp; > + BOOLEAN SetupLtr; > }; >=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 5c76ba4..63a243b 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > @@ -1069,3 +1069,174 @@ ProgramAtomicOp ( > return Status; > } >=20 > +/** > + The main routine which process the PCI feature LTR enable/disable 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 suc= cessful. > +**/ > +EFI_STATUS > +SetupLtr ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciExpressConfigurationTable > + ) > +{ > + PCI_REG_PCIE_DEVICE_CAPABILITY2 DeviceCap2; > + // > + // as per the PCI-Express Base Specification, in order to enable LTR > +mechanism > + // in the upstream ports, all the upstream ports and its downstream > +ports has > + // to support the LTR mechanism reported in its Device Capability 2 > +register > + // > + DeviceCap2.Uint32 =3D > +PciDevice->PciExpressCapabilityStructure.DeviceCapability2.Uint32; > + > + if (PciExpressConfigurationTable) { > + // > + // in this phase establish 2 requirements: > + // (1) all the PCI devices in the hierarchy supports the LTR mechan= ism > + // (2) check and record any device-specific platform policy that wa= nts to > + // enable the LTR mechanism > + // > + if > + (!PciDevice->PciExpressCapabilityStructure.DeviceCapability2.Bits.LtrM > + echanism) { > + > + // > + // it starts with the assumption that all the PCI devices support= LTR > mechanism > + // and negates the flag if any PCI device Device Capability 2 reg= ister > advertizes > + // as not supported > + // > + PciExpressConfigurationTable->LtrSupported =3D FALSE; > + } > + > + if (PciDevice->SetupLtr =3D=3D TRUE) { > + // > + // it starts with the assumption that device-specific platform po= licy would > + // be set to LTR disable, and negates the flag if any PCI device = platform > + // policy wants to override to enable the LTR mechanism > + // > + PciExpressConfigurationTable->LtrEnable =3D TRUE; > + } > + } else { > + // > + // in case of RCiEP device or the bridge device with out any child = device, > + // overrule the device policy if the device in not capable > + // > + if (!PciDevice- > >PciExpressCapabilityStructure.DeviceCapability2.Bits.LtrMechanism > + && PciDevice->SetupLtr =3D=3D TRUE) { > + PciDevice->SetupLtr =3D FALSE; > + } > + // > + // for any bridge device which is Hot-Plug capable, it is expected = that > platform > + // will not enforce the enabling of LTR mechanism only for the brid= ge device > + // > + } > + > + DEBUG (( DEBUG_INFO, "LTR En: %d (LTR Cap: %d),", > + PciDevice->SetupLtr ? 1 : 0, > + PciDevice- > >PciExpressCapabilityStructure.DeviceCapability2.Bits.LtrMechanism > + )); > + return EFI_SUCCESS; > +} > + > +EFI_STATUS > +ReSetupLtr ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciExpressConfigurationTable > + ) > +{ > + // > + // not applicable to RCiEP device... > + // for the bridge device without any child device, the policy is > +already overruled > + // based on capability in the above routine > + // > + if (PciExpressConfigurationTable) { > + // > + // in this phase align the device policy to enable LTR policy of an= y PCI device > + // in the tree if all the devices are capable to support the LTR me= chanism > + // > + if (PciExpressConfigurationTable->LtrSupported =3D=3D TRUE > + && PciExpressConfigurationTable->LtrEnable =3D=3D TRUE > + ) { > + PciDevice->SetupLtr =3D TRUE; > + } else { > + PciDevice->SetupLtr =3D FALSE; > + } > + } > + > + DEBUG (( DEBUG_INFO, "LTR En: %d (LTR Cap: %d),", > + PciDevice->SetupLtr ? 1 : 0, > + PciDevice- > >PciExpressCapabilityStructure.DeviceCapability2.Bits.LtrMechanism > + )); > + return EFI_SUCCESS; > +} > + > +/** > + Program the PCI Device Control 2 register LTR mechanism 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 > +ProgramLtr ( > + IN PCI_IO_DEVICE *PciDevice, > + IN VOID *PciExFeatureConfiguration > + ) > +{ > + PCI_REG_PCIE_DEVICE_CONTROL2 PcieDev; > + UINT32 Offset; > + EFI_STATUS Status; > + EFI_TPL OldTpl; > + > + PcieDev.Uint16 =3D 0; > + Offset =3D PciDevice->PciExpressCapabilityOffset + > + OFFSET_OF (PCI_CAPABILITY_PCIEXP, DeviceControl2); > + Status =3D PciDevice->PciIo.Pci.Read ( > + &PciDevice->PciIo, > + EfiPciIoWidthUint16, > + Offset, > + 1, > + &PcieDev.Uint16 > + ); > + ASSERT (Status =3D=3D EFI_SUCCESS); > + > + if (PciDevice->SetupLtr !=3D (BOOLEAN) PcieDev.Bits.LtrMechanism) { > + PcieDev.Bits.LtrMechanism =3D PciDevice->SetupLtr ? 1 : 0; > + DEBUG (( DEBUG_INFO, "LTR=3D%d,", PcieDev.Bits.LtrMechanism)); > + > + // > + // 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.DeviceControl2.Uint16 = =3D > PcieDev.Uint16; > + } else { > + ReportPciWriteError (PciDevice->BusNumber, PciDevice->DeviceNumbe= r, > PciDevice->FunctionNumber, Offset); > + } > + } else { > + DEBUG (( DEBUG_INFO, "no LTR,")); > + } > + > + return Status; > +} > + > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > index 1e287fc..374fe49 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > @@ -203,4 +203,44 @@ ProgramAtomicOp ( > IN VOID *PciExFeatureConfiguration > ); >=20 > +/** > + The main routine which process the PCI feature LTR enable/disable 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 suc= cessful. > +**/ > +EFI_STATUS > +SetupLtr ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciFeaturesConfigurationTable > + ); > + > +EFI_STATUS > +ReSetupLtr ( > + IN PCI_IO_DEVICE *PciDevice, > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > +*PciFeaturesConfigurationTable > + ); > + > +/** > + Program the PCI Device Control 2 register LTR mechanism 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 > +ProgramLtr ( > + 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 9d624a0..bdeb0d2 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > @@ -66,7 +66,7 @@ EFI_PCI_EXPRESS_PLATFORM_POLICY > mPciExpressPlatformPolicy =3D { > // > // support for PCI Express feature - LTR > // > - FALSE, > + TRUE, > // > // support for PCI Express feature - PTM > // > @@ -131,6 +131,15 @@ PCI_EXPRESS_FEATURE_INITIALIZATION_POINT > mPciExpressFeatureInitializationList[] > }, > { > PciExpressFeatureProgramPhase, PciExpressAtomicOp, > ProgramAtomicOp > + }, > + { > + PciExpressFeatureSetupPhase, PciExpressLtr, SetupLt= r > + }, > + { > + PciExpressFeatureEntendedSetupPhase, PciExpressLtr, ReSetup= Ltr > + }, > + { > + PciExpressFeatureProgramPhase, PciExpressLtr, Program= Ltr > } > }; >=20 > @@ -654,6 +663,14 @@ CreatePciRootBridgeDeviceNode ( > // > PciConfigTable->Lock_Max_Read_Request_Size =3D FALSE; > // > + // start by assuming the LTR mechanism is supported in a PCI tree > + // > + PciConfigTable->LtrSupported =3D TRUE; > + // > + // the default LTR mechanism is disabled as per the PCI Base specif= ication > + // > + PciConfigTable->LtrEnable =3D FALSE; > + // > // start by assuming the AtomicOp Routing capability is supported i= n the PCI > // tree > // > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > index 2bd565e..5dded7c 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > @@ -81,6 +81,15 @@ struct > _PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE { > // > BOOLEAN Lock_Max_Read_Request_Size; > // > + // to record the adversity in LTR mechanism support capability among > + the PCI // device of an heirarchy // > + BOOLEAN LtrSupported; > + // > + // to enable the LTR mechansim for the entire PCI tree from a root > + port // > + BOOLEAN LtrEnable; > + // > // to record the AtomicOp Routing capability of the PCI Heirarchy to = enable > // the AtomicOp of the EP device > // > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > index 2707976..83b3aa7 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > @@ -307,6 +307,36 @@ SetDevicePolicyPciExpressCto ( > } > } >=20 > +/** > + Routine to set the device-specific policy for the PCI feature LTR > +enable/disable > + > + @param AtomicOp value corresponding to data type > EFI_PCI_EXPRESS_ATOMIC_OP > + @param PciDevice A pointer to PCI_IO_DEVICE > + > +**/ > +VOID > +SetDevicePolicyPciExpressLtr ( > + IN EFI_PCI_EXPRESS_LTR Ltr, > + OUT PCI_IO_DEVICE *PciDevice > + ) > +{ > + switch (Ltr){ > + case EFI_PCI_EXPRESS_LTR_AUTO: > + case EFI_PCI_EXPRESS_LTR_DISABLE: > + // > + // leave the LTR mechanism disable or restore to its default stat= e > + // > + PciDevice->SetupLtr =3D FALSE; > + break; > + case EFI_PCI_EXPRESS_LTR_ENABLE: > + // > + // LTR mechanism enable > + // > + PciDevice->SetupLtr =3D TRUE; > + break; > + } > +} > + > /** > Generic routine to setup the PCI features as per its predetermined de= faults. > **/ > @@ -336,6 +366,8 @@ SetupDefaultPciExpressDevicePolicy ( >=20 > PciDevice->SetupAtomicOp.Override =3D 0; >=20 > + PciDevice->SetupLtr =3D FALSE; > + > } >=20 > /** > @@ -461,6 +493,16 @@ GetPciExpressDevicePolicy ( > PciDevice->SetupAtomicOp.Override =3D 0; > } >=20 > + // > + // set the device-specific policy for LTR mechanism in the function > + // > + if (mPciExpressPlatformPolicy.Ltr) { > + SetDevicePolicyPciExpressLtr (PciExpressDevicePolicy.DeviceCtl2LT= R, > PciDevice); > + } else { > + PciDevice->SetupLtr =3D FALSE; > + } > + > + > DEBUG (( > DEBUG_INFO, > "[device policy: platform]" > @@ -715,6 +757,17 @@ PciExpressPlatformNotifyDeviceState ( > PciExDeviceConfiguration.DeviceCtl2AtomicOp.Enable_AtomicOpRequeste= r =3D > 0; >=20 > PciExDeviceConfiguration.DeviceCtl2AtomicOp.Enable_AtomicOpEgressBlockin > g =3D 0; > } > + // > + // get the device-specific state for LTR mechanism in the function > + // if (mPciExpressPlatformPolicy.Ltr) { > + PciExDeviceConfiguration.DeviceCtl2LTR =3D PciDevice- > >PciExpressCapabilityStructure.DeviceControl2.Bits.LtrMechanism > + ? EFI_PCI_EXPRESS_LTR_E= NABLE > + : > + EFI_PCI_EXPRESS_LTR_DISABLE; } else { > + PciExDeviceConfiguration.DeviceCtl2LTR =3D > + EFI_PCI_EXPRESS_NOT_APPLICABLE; } > + >=20 > if (mPciExPlatformProtocol !=3D NULL) { > return mPciExPlatformProtocol->NotifyDeviceState ( > -- > 2.21.0.windows.1 >=20 >=20 >=20