From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mx.groups.io with SMTP id smtpd.web10.2432.1573615842623232465 for ; Tue, 12 Nov 2019 19:30:42 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.31, mailfrom: ashraf.javeed@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Nov 2019 19:30:42 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,299,1569308400"; d="scan'208";a="202626599" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga008.fm.intel.com with ESMTP; 12 Nov 2019 19:30:42 -0800 Received: from fmsmsx606.amr.corp.intel.com (10.18.126.86) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 12 Nov 2019 19:30:41 -0800 Received: from fmsmsx606.amr.corp.intel.com (10.18.126.86) by fmsmsx606.amr.corp.intel.com (10.18.126.86) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 12 Nov 2019 19:30:41 -0800 Received: from bgsmsx110.gar.corp.intel.com (10.223.4.212) by fmsmsx606.amr.corp.intel.com (10.18.126.86) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Tue, 12 Nov 2019 19:30:40 -0800 Received: from bgsmsx101.gar.corp.intel.com ([169.254.1.49]) by BGSMSX110.gar.corp.intel.com ([169.254.11.34]) with mapi id 14.03.0439.000; Wed, 13 Nov 2019 09:00:38 +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 feature Max_Payload_Size Thread-Topic: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 08/12] PciBusDxe: New PCI feature Max_Payload_Size Thread-Index: AQHVkMaEjCtDWRlzIEyNU1vql5bY7KeIg7nQ Date: Wed, 13 Nov 2019 03:30:38 +0000 Message-ID: <95C5C2B113DE604FB208120C742E9824579172AC@BGSMSX101.gar.corp.intel.com> References: <20191101150952.3340-1-ashraf.javeed@intel.com> <15D3127D273722D4.32624@groups.io> In-Reply-To: <15D3127D273722D4.32624@groups.io> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjQ5N2Q2MjAtOTI1My00YWQzLWE1YWQtMGJlZTUzYTQ1OTdkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiekZPM2NVVnRCZjE0aXh1R2dNenNaK094cVRUbit6aENWWWxoNnFLVFRtRVYwdUFVMjNIckt2bURpQzV4Y25XSSJ9 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 the following Repo for review:- https://github.com/ashrafj/edk2-staging/commit/283733b638f1063ed3d37a6d795= 10f30a334c3ac 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 08/12] > PciBusDxe: New PCI feature Max_Payload_Size >=20 > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2194 >=20 > The code changes are made to enable the configuration of new PCI feature > Max_Payload_Size (MPS), which defines the data packet size for the PCI > transactions, as per the PCI Base Specification 4 Revision 1. >=20 > The code changes are made to calibrate highest common value that is appl= - > icable to all the child nodes originating from the primary parent root p= ort of the > root bridge instance. >=20 > This programming of MPS is based on each PCI device's capability, and al= so its > device-specific platform policy obtained using the new PCI Platform Prot= ocol > interface, defined in the below record:- > 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 | 4 ++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c | 157 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h | 5 +++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c | 59 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h | 32 > ++++++++++++++++++++++++++++++++ > 5 files changed, 257 insertions(+) >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > index dc29ef3..065ae54 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > @@ -286,6 +286,10 @@ struct _PCI_IO_DEVICE { > // This field is used to support this case. > // > UINT16 BridgeIoAlignment; > + // > + // Other PCI features setup flags > + // > + UINT8 SetupMPS; > }; >=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 df9e696..8fdaa05 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c > @@ -582,6 +582,146 @@ IsPciRootPortEmpty ( > return FALSE; > } >=20 > +/** > + The main routine which process the PCI feature Max_Payload_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 configurati= on phases: > + PciFeatureGetDevicePolicy & > + PciFeatureSetupPhase @param PciFeaturesConfigurationTable pointer to > + OTHER_PCI_FEATURES_CONFIGURATION_TABLE > + > + @retval EFI_SUCCESS processing of PCI feature Max_P= ayload_Size > + is successful. > +**/ > +EFI_STATUS > +ProcessMaxPayloadSize ( > + 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 MpsValue; > + > + > + PciDeviceCap.Uint32 =3D > + PciDevice->PciExpStruct.DeviceCapability.Uint32; > + > + if (PciConfigPhase =3D=3D PciFeatureGetDevicePolicy) { > + if (SetupMpsAsPerDeviceCapability (PciDevice->SetupMPS)) { > + MpsValue =3D (UINT8)PciDeviceCap.Bits.MaxPayloadSize; > + // > + // no change to PCI Root ports without any endpoint device > + // > + if (IS_PCI_BRIDGE (&PciDevice->Pci) && PciDeviceCap.Bits.MaxPaylo= adSize) > { > + if (IsPciRootPortEmpty (PciDevice)) { > + MpsValue =3D PCIE_MAX_PAYLOAD_SIZE_128B; > + } > + } > + } else { > + MpsValue =3D TranslateMpsSetupValueToPci (PciDevice->SetupMPS); > + } > + // > + // discard device policy override request if greater than PCI devic= e capability > + // > + PciDevice->SetupMPS =3D MIN ((UINT8)PciDeviceCap.Bits.MaxPayloadSiz= e, > + MpsValue); } > + > + // > + // align the MPS of the tree to the HCF with this device // if > + (PciFeaturesConfigurationTable) { > + MpsValue =3D PciFeaturesConfigurationTable->Max_Payload_Size; > + > + MpsValue =3D MIN (PciDevice->SetupMPS, MpsValue); > + PciDevice->SetupMPS =3D MIN (PciDevice->SetupMPS, MpsValue); > + > + if (MpsValue !=3D PciFeaturesConfigurationTable->Max_Payload_Size) = { > + PciFeaturesConfigurationTable->Max_Payload_Size =3D MpsValue; > + } > + } > + > + DEBUG (( DEBUG_INFO, > + "MPS: %d [DevCap:%d],", > + PciDevice->SetupMPS, PciDeviceCap.Bits.MaxPayloadSize > + )); > + return EFI_SUCCESS; > +} > + > +/** > + Overrides the PCI Device Control register MaxPayloadSize 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 > +OverrideMaxPayloadSize ( > + 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 (PcieDev.Bits.MaxPayloadSize !=3D PciDevice->SetupMPS) { > + PcieDev.Bits.MaxPayloadSize =3D PciDevice->SetupMPS; > + DEBUG (( DEBUG_INFO, "MPS=3D%d,", PciDevice->SetupMPS)); > + > + // > + // 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 MPS=3D%d,", PciDevice->SetupMPS))= ; > + } > + > + return Status; > +} >=20 > /** > helper routine to dump the PCIe Device Port Type @@ -669,6 +809,18 @@ > SetupDevicePciFeatures ( > } > } >=20 > + DEBUG ((DEBUG_INFO, "[")); > + // > + // process the PCI device Max_Payload_Size feature // if > + (SetupMaxPayloadSize ()) { > + Status =3D ProcessMaxPayloadSize ( > + PciDevice, > + PciConfigPhase, > + OtherPciFeaturesConfigTable > + ); > + } > + DEBUG ((DEBUG_INFO, "]\n")); > return Status; > } >=20 > @@ -765,6 +917,10 @@ ProgramDevicePciFeatures ( { > EFI_STATUS Status =3D EFI_SUCCESS; >=20 > + if (SetupMaxPayloadSize ()) { > + Status =3D OverrideMaxPayloadSize (PciDevice); } DEBUG (( > + DEBUG_INFO, "\n")); > return Status; > } >=20 > @@ -878,6 +1034,7 @@ AddPrimaryRootPortNode ( > ); > if (PciConfigTable) { > PciConfigTable->ID =3D PortNumber; > + PciConfigTable->Max_Payload_Size =3D > PCIE_MAX_PAYLOAD_SIZE_4096B; > } > RootPortNode->OtherPciFeaturesConfigurationTable =3D PciConfigTable; >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > index f92d008..e5ac2a3 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h > @@ -79,6 +79,11 @@ struct _OTHER_PCI_FEATURES_CONFIGURATION_TABLE > { > // Configuration Table ID > // > UINTN ID; > + // > + // to configure the PCI feature Maximum payload size to maintain the > + data packet // size among all the PCI devices in the PCI hierarchy > + // > + UINT8 Max_Payload_Size; > }; >=20 >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > index 238959e..99badd6 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c > @@ -356,6 +356,63 @@ GetPlatformPciOptionRom ( > return Status; > } >=20 > +/** > + Helper routine to indicate whether the given PCI device specific > +policy value > + dictates to override the Max_Payload_Size to a particular value, or > +set as per > + device capability. > + > + @param MPS Input device-specific policy should be in terms of ty= pe > + EFI_PCI_CONF_MAX_PAYLOAD_SIZE > + > + @retval TRUE Setup Max_Payload_Size as per device capability > + FALSE override as per device-specific platform policy > +**/ > +BOOLEAN > +SetupMpsAsPerDeviceCapability ( > + IN UINT8 MPS > +) > +{ > + if (MPS =3D=3D EFI_PCI_CONF_MAX_PAYLOAD_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 > + Revision 4.0; for the PCI feature Max_Payload_Size. > + > + @param MPS Input device-specific policy should be in terms of ty= pe > + EFI_PCI_CONF_MAX_PAYLOAD_SIZE > + > + @retval Range values for the Max_Payload_Size as defined in t= he PCI > + Base Specification 4.0 **/ > +UINT8 > +TranslateMpsSetupValueToPci ( > + IN UINT8 MPS > +) > +{ > + switch (MPS) { > + case EFI_PCI_CONF_MAX_PAYLOAD_SIZE_128B: > + return PCIE_MAX_PAYLOAD_SIZE_128B; > + case EFI_PCI_CONF_MAX_PAYLOAD_SIZE_256B: > + return PCIE_MAX_PAYLOAD_SIZE_256B; > + case EFI_PCI_CONF_MAX_PAYLOAD_SIZE_512B: > + return PCIE_MAX_PAYLOAD_SIZE_512B; > + case EFI_PCI_CONF_MAX_PAYLOAD_SIZE_1024B: > + return PCIE_MAX_PAYLOAD_SIZE_1024B; > + case EFI_PCI_CONF_MAX_PAYLOAD_SIZE_2048B: > + return PCIE_MAX_PAYLOAD_SIZE_2048B; > + case EFI_PCI_CONF_MAX_PAYLOAD_SIZE_4096B: > + return PCIE_MAX_PAYLOAD_SIZE_4096B; > + default: > + return PCIE_MAX_PAYLOAD_SIZE_128B; > + } > +} > + > /** > Generic routine to setup the PCI features as per its predetermined de= faults. > **/ > @@ -364,6 +421,7 @@ SetupDefaultsDevicePlatformPolicy ( > IN PCI_IO_DEVICE *PciDevice > ) > { > + PciDevice->SetupMPS =3D EFI_PCI_CONF_MAX_PAYLOAD_SIZE_AUTO; > } >=20 > /** > @@ -399,6 +457,7 @@ GetPciDevicePlatformPolicyEx ( > // > // platform chipset policies are returned for this PCI device > // > + PciIoDevice->SetupMPS =3D PciPlatformExtendedPolicy.DeviceCtlMPS; >=20 > DEBUG (( > DEBUG_INFO, "[device policy: platform]" > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h > index a13131c..786c00d 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h > @@ -124,4 +124,36 @@ EFI_STATUS > GetPciDevicePlatformPolicy ( > IN PCI_IO_DEVICE *PciDevice > ); > + > +/** > + Helper routine to indicate whether the given PCI device specific > +policy value > + dictates to override the Max_Payload_Size to a particular value, or > +set as per > + device capability. > + > + @param MPS Input device-specific policy should be in terms of ty= pe > + EFI_PCI_CONF_MAX_PAYLOAD_SIZE > + > + @retval TRUE Setup Max_Payload_Size as per device capability > + FALSE override as per device-specific platform policy > +**/ > +BOOLEAN > +SetupMpsAsPerDeviceCapability ( > + IN UINT8 MPS > +); > + > +/** > + 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 > + Revision 4.0; for the PCI feature Max_Payload_Size. > + > + @param MPS Input device-specific policy should be in terms of ty= pe > + EFI_PCI_CONF_MAX_PAYLOAD_SIZE > + > + @retval Range values for the Max_Payload_Size as defined in t= he PCI > + Base Specification 4.0 **/ > +UINT8 > +TranslateMpsSetupValueToPci ( > + IN UINT8 MPS > +); > #endif > -- > 2.21.0.windows.1 >=20 >=20 >=20