From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mx.groups.io with SMTP id smtpd.web10.2452.1573616008291605784 for ; Tue, 12 Nov 2019 19:33:28 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.151, mailfrom: ashraf.javeed@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Nov 2019 19:33:28 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,299,1569308400"; d="scan'208";a="194538882" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga007.jf.intel.com with ESMTP; 12 Nov 2019 19:33:27 -0800 Received: from bgsmsx104.gar.corp.intel.com (10.223.4.190) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 12 Nov 2019 19:33:26 -0800 Received: from bgsmsx101.gar.corp.intel.com ([169.254.1.49]) by BGSMSX104.gar.corp.intel.com ([169.254.5.80]) with mapi id 14.03.0439.000; Wed, 13 Nov 2019 09:03:23 +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 11/12] PciBusDxe: New PCI feature No-Snoop Thread-Topic: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 11/12] PciBusDxe: New PCI feature No-Snoop Thread-Index: AQHVkMaL39Mm6o9y70C3eKz5dF1KTaeIhH9Q Date: Wed, 13 Nov 2019 03:33:23 +0000 Message-ID: <95C5C2B113DE604FB208120C742E9824579172FB@BGSMSX101.gar.corp.intel.com> References: <20191101150952.3340-1-ashraf.javeed@intel.com> <15D3127EB6ED8506.12315@groups.io> In-Reply-To: <15D3127EB6ED8506.12315@groups.io> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTk1ODFkNGMtZWViMS00ZWJmLTk5NjItMzZlNThmMjA5MDY0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiR2lrMW9SR1NKUmFpK1JjYm82WTZCb1NLQ2pcL1NaTjFkc0x5ejRxdzN2eGIwbU5GNDA3SVVwbVRZZGdHWVEwQysifQ== 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 is also uploaded in following Repo for review:- https://github.com/ashrafj/edk2-staging/commit/da0d50ad3b542fc4b63f8debb7d= 690c83ef4218d Thanks Ashraf > -----Original Message----- > From: 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 ; Wu, Hao A = ; > Ni, Ray > Subject: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 11/12] > PciBusDxe: New PCI feature No-Snoop >=20 > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2313 >=20 > The code changes are made; as per the PCI Base Specification 4 Revision = 1; to > enable the configuration of new PCI feature No-Snoop (NS), which enables= the > PCI function to initiate requests if it does not require har- dware enfo= rced > cache-coherency for its transactions. >=20 > 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. >=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/PciFeatureSupport.c | 78 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c | 45 > +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 124 insertions(+) >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > index 9f017b7..be1c341 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > @@ -293,6 +293,7 @@ struct _PCI_IO_DEVICE { > UINT8 SetupMPS; > UINT8 SetupMRRS; > PCI_FEATURE_POLICY SetupRO; > + PCI_FEATURE_POLICY SetupNS; > }; >=20 > #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 a60cb42..a7f0a2f 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > @@ -986,6 +986,81 @@ OverrideRelaxOrder ( > return Status; > } >=20 > +/** > + Overrides the PCI Device Control register No-Snoop 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 > +OverrideNoSnoop ( > + IN PCI_IO_DEVICE *PciDevice > + ) > +{ > + 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 > + ); > + if (EFI_ERROR(Status)){ > + DEBUG (( DEBUG_ERROR, "Unexpected DeviceControl register (0x%x) rea= d > error!", > + Offset > + )); > + return Status; > + } > + if (PciDevice->SetupRO.Override > + && PcieDev.Bits.NoSnoop !=3D PciDevice->SetupNS.Act > + ) { > + PcieDev.Bits.NoSnoop =3D PciDevice->SetupNS.Act; > + DEBUG (( DEBUG_INFO, "NS=3D%d", PciDevice->SetupNS.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->PciExpStruct.DeviceControl.Uint16 =3D PcieDev.Uint16; > + } else { > + DEBUG (( DEBUG_ERROR, "Unexpected DeviceControl register (0x%x) w= rite > error!", > + Offset > + )); > + } > + } else { > + DEBUG (( DEBUG_INFO, "No write of NS,", PciDevice->SetupRO.Act)); > + } > + > + return Status; > +} > + > /** > helper routine to dump the PCIe Device Port Type **/ @@ -1200,6 +127= 5,9 > @@ ProgramDevicePciFeatures ( > if (SetupRelaxOrder ()) { > Status =3D OverrideRelaxOrder (PciDevice); > } > + if (SetupNoSnoop ()) { > + Status =3D OverrideNoSnoop (PciDevice); } > DEBUG (( DEBUG_INFO, "\n")); > return Status; > } > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > index f1e7039..47295cd 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > @@ -509,6 +509,46 @@ SetDevicePolicyRelaxOrder ( > } > } >=20 > +/** > + Routine to set the device-specific policy for the PCI feature > +No-Snoop enable > + or disable > + > + @param NoSnoop value corresponding to data type > EFI_PCI_CONF_NO_SNOOP > + @param PciDevice A pointer to PCI_IO_DEVICE > +**/ > +VOID > +SetDevicePolicyNoSnoop ( > + IN EFI_PCI_CONF_NO_SNOOP NoSnoop, > + OUT PCI_IO_DEVICE *PciDevice > + ) > +{ > + // > + // implementation specific rules for the usage of PCI_FEATURE_POLICY > +members > + // exclusively for the PCI Feature No-Snoop > + // > + // .Override =3D 0 to skip this PCI feature No-Snoop for the PCI devi= ce > + // .Override =3D 1 to program this No-Snoop PCI feature > + // .Act =3D 1 to enable the No-Snoop in the PCI device > + // .Act =3D 0 to disable the No-Snoop in the PCI device > + // > + switch (NoSnoop) { > + case EFI_PCI_CONF_NS_AUTO: > + PciDevice->SetupNS.Override =3D 0; > + break; > + case EFI_PCI_CONF_NS_DISABLE: > + PciDevice->SetupNS.Override =3D 1; > + PciDevice->SetupNS.Act =3D 0; > + break; > + case EFI_PCI_CONF_NS_ENABLE: > + PciDevice->SetupNS.Override =3D 1; > + PciDevice->SetupNS.Act =3D 1; > + break; > + default: > + PciDevice->SetupNS.Override =3D 0; > + break; > + } > +} > + > /** > Generic routine to setup the PCI features as per its predetermined de= faults. > **/ > @@ -520,6 +560,7 @@ SetupDefaultsDevicePlatformPolicy ( > PciDevice->SetupMPS =3D EFI_PCI_CONF_MAX_PAYLOAD_SIZE_AUTO; > PciDevice->SetupMRRS =3D EFI_PCI_CONF_MAX_READ_REQ_SIZE_AUTO; > PciDevice->SetupRO.Override =3D 0; > + PciDevice->SetupNS.Override =3D 0; > } >=20 > /** > @@ -561,6 +602,10 @@ GetPciDevicePlatformPolicyEx ( > // set device specific policy for Relax Ordering > // > SetDevicePolicyRelaxOrder > (PciPlatformExtendedPolicy.DeviceCtlRelaxOrder, PciIoDevice); > + // > + // set the device specific policy for No-Snoop > + // > + SetDevicePolicyNoSnoop > + (PciPlatformExtendedPolicy.DeviceCtlNoSnoop, PciIoDevice); >=20 > DEBUG (( > DEBUG_INFO, "[device policy: platform]" > -- > 2.21.0.windows.1 >=20 >=20 >=20