From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web09.10808.1581107268319847500 for ; Fri, 07 Feb 2020 12:27:48 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: ashraf.javeed@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Feb 2020 12:27:48 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,414,1574150400"; d="scan'208";a="255519558" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga004.fm.intel.com with ESMTP; 07 Feb 2020 12:27:47 -0800 Received: from fmsmsx120.amr.corp.intel.com (10.18.124.208) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 7 Feb 2020 12:27:47 -0800 Received: from bgsmsx152.gar.corp.intel.com (10.224.48.50) by fmsmsx120.amr.corp.intel.com (10.18.124.208) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 7 Feb 2020 12:27:46 -0800 Received: from bgsmsx101.gar.corp.intel.com ([169.254.1.155]) by BGSMSX152.gar.corp.intel.com ([169.254.6.38]) with mapi id 14.03.0439.000; Sat, 8 Feb 2020 01:57:44 +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 08/12] PciBusDxe: New PCI Express feature AtomicOp Thread-Topic: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 08/12] PciBusDxe: New PCI Express feature AtomicOp Thread-Index: AQHV3fIdwq3Ock88PkWNthR6x9zNoKgQLY5ggAAAaXA= Date: Fri, 7 Feb 2020 20:27:42 +0000 Message-ID: <95C5C2B113DE604FB208120C742E9824579AB029@BGSMSX101.gar.corp.intel.com> References: <20200207200447.10536-1-ashraf.javeed@intel.com> <15F13784AAF69472.18602@groups.io> <15F1389AA432C2B6.18602@groups.io> In-Reply-To: <15F1389AA432C2B6.18602@groups.io> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDYwNjA1ODEtMTg4NS00OWI2LTg2NTktY2I1MzU3YjhiYzcyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiRk1GNklTTnF0OTdteXFZZlFtU3Bvcjl3cllyQ1Q5bjRvZXR4bFwvNDlod0ZZbVc3XC9oU2llRDhySXp4Y3ZMaWVwIn0= 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 Link correction made below. > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Javeed, > Ashraf > Sent: Saturday, February 8, 2020 1:56 AM > 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 08/12] > PciBusDxe: New PCI Express feature AtomicOp >=20 > This patch can also be viewed in the following repo:- https://github.com/ashrafj/edk2-staging/commit/176fccc85714fbbae616821ba35= 5cd4306cecb87 >=20 > Thanks > Ashraf >=20 > > -----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 08/12] > > PciBusDxe: New PCI Express feature AtomicOp > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2499 > > > > This code change handles two PCI Express features related to AtomicOp, > > in compliance with the PCI Express Base SPecification 5: > > (1) configuring PCI function as an AtomicOp Requester > > (2) Enabling of Port Egress blocking depending on AtomicOp Routing > > Capability > > > > These is programmed based on the following criteria:- For a PCI Bridge= device: > > - enables as AtomicOp Requester solely based on platform provided dev= ice > > policy > > - enabling of Port Egress blocking is based on platform device policy= ; > > only if its device capability register's AtomicOp Routing bit is 1 > > > > For an PCI EndPoint (EP) device: > > - if the platform's device policy wants to enable the AtomicOp Reques= ter > > function for this device, than this device is enabled if all its br= idge > > devices have the AtomicOp Routing capability bit set > > - Enabling of Port Egress disable is not applicable to PCI EP device > > > > For an PCI RCiEP device: > > - the AtomicOp Requester functionality is enabled solely based on the > > device policy provided by platform > > - similar to PCI EP device, the enabling of port Egress blocking is n= ot > > applicable > > > > 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 | 161 > > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > +++++++++++++++++++++++++++++++ > > MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h | 35 > > +++++++++++++++++++++++++++++++++++ > > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c | 23 > > ++++++++++++++++++++++- > > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h | 5 +++++ > > MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c | 23 > > +++++++++++++++++++++++ > > 6 files changed, 247 insertions(+), 1 deletion(-) > > > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > > index 9b03c12..57ef1b2 100644 > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > > @@ -295,6 +295,7 @@ struct _PCI_IO_DEVICE { > > PCI_FEATURE_POLICY SetupRO; > > PCI_FEATURE_POLICY SetupNS; > > PCI_FEATURE_POLICY SetupCTO; > > + EFI_PCI_EXPRESS_ATOMIC_OP SetupAtomicOp; > > }; > > > > #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 f3f4d39..5c76ba4 100644 > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c > > @@ -908,3 +908,164 @@ ProgramCompletionTimeout ( > > return Status; > > } > > > > +/** > > + Routine to setup the AtomicOp Requester in the PCI device, verifies > > +the routing > > + support in the bridge devices, to be complaint as per the PCI Base > > specification. > > + > > + @param PciDevice A pointer to the PCI_IO_DEVIC= E. > > + @param PciExFeatureConfiguration pointer to common configurati= on > > table to > > + initialize the PCI Express > > + feature > > + > > + @retval EFI_SUCCESS bridge device routing capabil= ity is > successful. > > + EFI_INVALID_PARAMETER input parameter is NULL > > +**/ > > +EFI_STATUS > > +SetupAtomicOpRoutingSupport ( > > + IN PCI_IO_DEVICE *PciDevice, > > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > > *PciExFeatureConfiguration > > + ) > > +{ > > + // > > + // to enable the AtomicOp Requester in the PCI EP device; its Root > > +Port (bridge), > > + // and its PCIe switch upstream & downstream ports (if present) > > +needs to support > > + // the AtomicOp Routing capability. > > + // > > + if (IS_PCI_BRIDGE (&PciDevice->Pci)) { > > + if (!PciDevice- > > >PciExpressCapabilityStructure.DeviceCapability2.Bits.AtomicOpRouting) > > >{ > > + // > > + // since the AtomicOp Routing support flag is initialized as TR= UE, negate > > + // in case if any of the PCI Bridge device in the PCI tree does= not support > > + // the AtomicOp Routing capability > > + // > > + if (PciExFeatureConfiguration =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + PciExFeatureConfiguration->AtomicOpRoutingSupported =3D FALSE; > > + } > > + } > > + > > + return EFI_SUCCESS; > > +} > > + > > +/** > > + Overrides the PCI Device Control 2 register AtomicOp Requester > > +enable field; if > > + the hardware value is different than the intended value. > > + > > + @param PciDevice A pointer to the PCI_IO_DEVICE instan= ce. > > + > > + @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 heade= r of the PCI controller. > > + @retval EFI_INVALID_PARAMETER Buffer is NULL or Width is invalid. > > + > > +**/ > > +EFI_STATUS > > +ProgramAtomicOp ( > > + IN PCI_IO_DEVICE *PciDevice, > > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > > +*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->SetupAtomicOp.Override) { > > + // > > + // override AtomicOp requester device control bit of the device b= ased on > the > > + // platform request > > + // > > + if (IS_PCI_BRIDGE (&PciDevice->Pci)) { > > + // > > + // for a bridge device as AtomicOp Requester function; only > > + platform > > override > > + // request is used to set the device control register > > + // > > + if (PcieDev.Bits.AtomicOpRequester !=3D PciDevice- > > >SetupAtomicOp.Enable_AtomicOpRequester) { > > + PcieDev.Bits.AtomicOpRequester =3D PciDevice- > > >SetupAtomicOp.Enable_AtomicOpRequester; > > + } > > + // > > + // if platform also request its AtomicOp Egress blocking to be = enabled; set > > + // only if its device capability's AtomicOpRouting bit is 1. > > + // applicable to only the bridge devices > > + // > > + if (PciDevice->SetupAtomicOp.Enable_AtomicOpEgressBlocking) { > > + if (PciDevice- > > >PciExpressCapabilityStructure.DeviceCapability2.Bits.AtomicOpRouting) > > >{ > > + PcieDev.Bits.AtomicOpEgressBlocking =3D 1; > > + } > > + } > > + } else { > > + // > > + // in the case of non-bridge device > > + // > > + if (PciExFeatureConfiguration) { > > + // > > + // for a device as AtomicOp Requester function; its bridge de= vices should > > + // support the AtomicOp Routing capability to enable the devi= ce's as a > > + // requester function > > + // > > + if (PciExFeatureConfiguration->AtomicOpRoutingSupported) { > > + if (PcieDev.Bits.AtomicOpRequester !=3D PciDevice- > > >SetupAtomicOp.Enable_AtomicOpRequester) { > > + PcieDev.Bits.AtomicOpRequester =3D PciDevice- > > >SetupAtomicOp.Enable_AtomicOpRequester; > > + } > > + } > > + } else { > > + // > > + // for the RCiEP device or the bridge device without any > > + child, setup > > AtomicOp > > + // Requester as per platform's device policy > > + // > > + if (PcieDev.Bits.AtomicOpRequester !=3D PciDevice- > > >SetupAtomicOp.Enable_AtomicOpRequester) { > > + PcieDev.Bits.AtomicOpRequester =3D PciDevice- > > >SetupAtomicOp.Enable_AtomicOpRequester; > > + } > > + } > > + // > > + // the enabling of AtomicOp Egress Blocking is not applicable > > + to a non- > > bridge > > + // device > > + // > > + } > > + DEBUG (( > > + DEBUG_INFO, > > + "AtomicOp=3D%d,", > > + PcieDev.Bits.AtomicOpRequester > > + )); > > + > > + // > > + // Raise TPL to high level to disable timer interrupt while the > > + write 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->DeviceNumber, > > PciDevice->FunctionNumber, Offset); > > + } > > + } else { > > + DEBUG (( DEBUG_INFO, "No AtomicOp,")); } > > + > > + return Status; > > +} > > + > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > > index 2ee7d4d..1e287fc 100644 > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h > > @@ -168,4 +168,39 @@ ProgramCompletionTimeout ( > > IN VOID *PciExFeatureConfiguration > > ); > > > > +/** > > + Routine to setup the AtomicOp Requester in the PCI device, verifies > > +the routing > > + support in the bridge devices, to be complaint as per the PCI Base > > specification. > > + > > + @param PciDevice A pointer to the PCI_IO_DEVIC= E. > > + @param PciExFeatureConfiguration pointer to common configurati= on > > table to > > + initialize the PCI Express > > + feature > > + > > + @retval EFI_SUCCESS bridge device routing capabil= ity is > successful. > > + EFI_INVALID_PARAMETER input parameter is NULL > > +**/ > > +EFI_STATUS > > +SetupAtomicOpRoutingSupport ( > > + IN PCI_IO_DEVICE *PciDevice, > > + IN PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE > > *PciExFeatureConfiguration > > + ); > > + > > +/** > > + Overrides the PCI Device Control 2 register AtomicOp Requester > > +enable field; if > > + the hardware value is different than the intended value. > > + > > + @param PciDevice A pointer to the PCI_IO_DEVICE instan= ce. > > + > > + @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 heade= r of the PCI controller. > > + @retval EFI_INVALID_PARAMETER Buffer is NULL or Width is invalid. > > + > > +**/ > > +EFI_STATUS > > +ProgramAtomicOp ( > > + 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 d4459f3..9d624a0 100644 > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > > @@ -62,7 +62,7 @@ EFI_PCI_EXPRESS_PLATFORM_POLICY > > mPciExpressPlatformPolicy =3D { > > // > > // support for PCI Express feature - Atomic Op > > // > > - FALSE, > > + TRUE, > > // > > // support for PCI Express feature - LTR > > // > > @@ -125,6 +125,12 @@ PCI_EXPRESS_FEATURE_INITIALIZATION_POINT > > mPciExpressFeatureInitializationList[] > > }, > > { > > PciExpressFeatureProgramPhase, PciExpressCto, > > ProgramCompletionTimeout > > + }, > > + { > > + PciExpressFeatureSetupPhase, PciExpressAtomicOp, > > SetupAtomicOpRoutingSupport > > + }, > > + { > > + PciExpressFeatureProgramPhase, PciExpressAtomicOp, > > ProgramAtomicOp > > } > > }; > > > > @@ -297,6 +303,16 @@ IsPciExpressFeatureExtendedSetupRequired ( > > && > > PciExpressPolicy[mPciExpressFeatureInitializationList[idx].PciExpressF > > eatureId] > > =3D=3D TRUE > > ) { > > return TRUE; > > + } else if ( > > + // > > + // the PCI Express feature does not require extended setup ph= ase but it > > + // does require global flag to track the AtomicOpRouting caoa= bility to > > + // be tracked for all its bridge devices > > + // > > + idx =3D=3D PciExpressAtomicOp > > + && PciExpressPolicy[idx] =3D=3D TRUE > > + ) { > > + return TRUE; > > } > > } > > > > @@ -637,6 +653,11 @@ CreatePciRootBridgeDeviceNode ( > > // the devices in the PCI tree > > // > > PciConfigTable->Lock_Max_Read_Request_Size =3D FALSE; > > + // > > + // start by assuming the AtomicOp Routing capability is supported= in the > PCI > > + // tree > > + // > > + PciConfigTable->AtomicOpRoutingSupported =3D TRUE; > > } > > > > RootBridgeNode->PciExFeaturesConfigurationTable =3D PciConfigTable= ; > > diff -- git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > > index a1fc39c..2bd565e 100644 > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > > @@ -80,6 +80,11 @@ struct > > _PCI_EXPRESS_FEATURES_CONFIGURATION_TABLE { > > // lock the Max_Read_Request_Size for the entire PCI tree of a root= port > > // > > BOOLEAN Lock_Max_Read_Request_Siz= e; > > + // > > + // to record the AtomicOp Routing capability of the PCI Heirarchy > > + to enable // the AtomicOp of the EP device // > > + BOOLEAN AtomicOpRoutingSupported; > > }; > > > > // > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > > index 1afea19..2707976 100644 > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > > @@ -334,6 +334,8 @@ SetupDefaultPciExpressDevicePolicy ( > > > > PciDevice->SetupCTO.Override =3D 0; > > > > + PciDevice->SetupAtomicOp.Override =3D 0; > > + > > } > > > > /** > > @@ -450,6 +452,14 @@ GetPciExpressDevicePolicy ( > > PciDevice->SetupCTO.Override =3D 0; > > } > > > > + // > > + // set the device-specific policy for AtomicOp > > + // > > + if (mPciExpressPlatformPolicy.AtomicOp) { > > + PciDevice->SetupAtomicOp =3D > PciExpressDevicePolicy.DeviceCtl2AtomicOp; > > + } else { > > + PciDevice->SetupAtomicOp.Override =3D 0; > > + } > > > > DEBUG (( > > DEBUG_INFO, > > @@ -692,6 +702,19 @@ PciExpressPlatformNotifyDeviceState ( > > PciExDeviceConfiguration.CTOsupport =3D > EFI_PCI_EXPRESS_NOT_APPLICABLE; > > } > > > > + // > > + // get the device-specific state for the PCIe AtomicOp feature // > > + if (mPciExpressPlatformPolicy.AtomicOp) { > > + > PciExDeviceConfiguration.DeviceCtl2AtomicOp.Enable_AtomicOpRequester > > + =3D (UINT8)PciDevice- > > >PciExpressCapabilityStructure.DeviceControl2.Bits.AtomicOpRequester; > > + > > PciExDeviceConfiguration.DeviceCtl2AtomicOp.Enable_AtomicOpEgressBlock > > in > > g > > + =3D > > + (UINT8)PciDevice->PciExpressCapabilityStructure.DeviceControl2.Bits. > > + At > > + omicOpEgressBlocking; > > + } else { > > + PciExDeviceConfiguration.DeviceCtl2AtomicOp.Override =3D 0; > > + > > + PciExDeviceConfiguration.DeviceCtl2AtomicOp.Enable_AtomicOpRequester > > =3D 0; > > + > > + PciExDeviceConfiguration.DeviceCtl2AtomicOp.Enable_AtomicOpEgressBlo > > + ck > > + ing =3D 0; } > > > > if (mPciExPlatformProtocol !=3D NULL) { > > return mPciExPlatformProtocol->NotifyDeviceState ( > > -- > > 2.21.0.windows.1 > > > > > > >=20 >=20 >=20