Re: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 08/12] PciBusDxe: New PCI feature Max_Payload_Size

["Javeed, Ashraf" <ashraf.javeed@intel.com>]

This patch is also uploaded in the following Repo for review:-
https://github.com/ashrafj/edk2-staging/commit/283733b638f1063ed3d37a6d79510f30a334c3ac

Thanks
Ashraf

> -----Original Message-----
> From: devel@edk2.groups.io <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 <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 08/12]
> PciBusDxe: New PCI feature Max_Payload_Size
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2194
> 
> 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.
> 
> 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 port of the
> root bridge instance.
> 
> This programming of MPS is based on each PCI device's capability, and also its
> device-specific platform policy obtained using the new PCI Platform Protocol
> interface, defined in the below record:-
> https://bugzilla.tianocore.org/show_bug.cgi?id=1954
> 
> Signed-off-by: Ashraf Javeed <ashraf.javeed@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> ---
>  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(+)
> 
> 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;
>  };
> 
>  #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;
>  }
> 
> +/**
> +  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 configuration phases:
> +                                        PciFeatureGetDevicePolicy &
> + PciFeatureSetupPhase  @param PciFeaturesConfigurationTable  pointer to
> + OTHER_PCI_FEATURES_CONFIGURATION_TABLE
> +
> +  @retval EFI_SUCCESS                   processing of PCI feature Max_Payload_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 =
> + PciDevice->PciExpStruct.DeviceCapability.Uint32;
> +
> +  if (PciConfigPhase == PciFeatureGetDevicePolicy) {
> +    if (SetupMpsAsPerDeviceCapability (PciDevice->SetupMPS)) {
> +      MpsValue = (UINT8)PciDeviceCap.Bits.MaxPayloadSize;
> +      //
> +      // no change to PCI Root ports without any endpoint device
> +      //
> +      if (IS_PCI_BRIDGE (&PciDevice->Pci) && PciDeviceCap.Bits.MaxPayloadSize)
> {
> +        if (IsPciRootPortEmpty (PciDevice)) {
> +          MpsValue = PCIE_MAX_PAYLOAD_SIZE_128B;
> +        }
> +      }
> +    } else {
> +      MpsValue = TranslateMpsSetupValueToPci (PciDevice->SetupMPS);
> +    }
> +    //
> +    // discard device policy override request if greater than PCI device capability
> +    //
> +    PciDevice->SetupMPS = MIN ((UINT8)PciDeviceCap.Bits.MaxPayloadSize,
> + MpsValue);  }
> +
> +  //
> +  // align the MPS of the tree to the HCF with this device  //  if
> + (PciFeaturesConfigurationTable) {
> +    MpsValue = PciFeaturesConfigurationTable->Max_Payload_Size;
> +
> +    MpsValue = MIN (PciDevice->SetupMPS, MpsValue);
> +    PciDevice->SetupMPS = MIN (PciDevice->SetupMPS, MpsValue);
> +
> +    if (MpsValue != PciFeaturesConfigurationTable->Max_Payload_Size) {
> +      PciFeaturesConfigurationTable->Max_Payload_Size = 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 the 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 = 0;
> +  Offset = PciDevice->PciExpressCapabilityOffset +
> +               OFFSET_OF (PCI_CAPABILITY_PCIEXP, DeviceControl);
> + Status = PciDevice->PciIo.Pci.Read (
> +                                  &PciDevice->PciIo,
> +                                  EfiPciIoWidthUint16,
> +                                  Offset,
> +                                  1,
> +                                  &PcieDev.Uint16
> +                                );
> +  if (EFI_ERROR(Status)){
> +    DEBUG (( DEBUG_ERROR, "Unexpected DeviceControl register (0x%x) read
> error!",
> +        Offset
> +    ));
> +    return Status;
> +  }
> +  if (PcieDev.Bits.MaxPayloadSize != PciDevice->SetupMPS) {
> +    PcieDev.Bits.MaxPayloadSize = PciDevice->SetupMPS;
> +    DEBUG (( DEBUG_INFO, "MPS=%d,", PciDevice->SetupMPS));
> +
> +    //
> +    // Raise TPL to high level to disable timer interrupt while the write operation
> completes
> +    //
> +    OldTpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
> +
> +    Status = 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 = PcieDev.Uint16;
> +    } else {
> +      DEBUG (( DEBUG_ERROR, "Unexpected DeviceControl register (0x%x) write
> error!",
> +          Offset
> +      ));
> +    }
> +  } else {
> +    DEBUG (( DEBUG_INFO, "No write of MPS=%d,", PciDevice->SetupMPS));
> + }
> +
> +  return Status;
> +}
> 
>  /**
>    helper routine to dump the PCIe Device Port Type @@ -669,6 +809,18 @@
> SetupDevicePciFeatures (
>      }
>    }
> 
> +  DEBUG ((DEBUG_INFO, "["));
> +  //
> +  // process the PCI device Max_Payload_Size feature  //  if
> + (SetupMaxPayloadSize ()) {
> +    Status = ProcessMaxPayloadSize (
> +              PciDevice,
> +              PciConfigPhase,
> +              OtherPciFeaturesConfigTable
> +              );
> +  }
> +  DEBUG ((DEBUG_INFO, "]\n"));
>    return Status;
>  }
> 
> @@ -765,6 +917,10 @@ ProgramDevicePciFeatures (  {
>    EFI_STATUS           Status = EFI_SUCCESS;
> 
> +  if (SetupMaxPayloadSize ()) {
> +    Status = OverrideMaxPayloadSize (PciDevice);  }  DEBUG ((
> + DEBUG_INFO, "\n"));
>    return Status;
>  }
> 
> @@ -878,6 +1034,7 @@ AddPrimaryRootPortNode (
>                      );
>    if (PciConfigTable) {
>      PciConfigTable->ID                          = PortNumber;
> +    PciConfigTable->Max_Payload_Size            =
> PCIE_MAX_PAYLOAD_SIZE_4096B;
>    }
>    RootPortNode->OtherPciFeaturesConfigurationTable  = PciConfigTable;
> 
> 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;
>  };
> 
> 
> 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;
>  }
> 
> +/**
> +  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 type
> +                  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 == 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 type
> +                  EFI_PCI_CONF_MAX_PAYLOAD_SIZE
> +
> +  @retval         Range values for the Max_Payload_Size as defined in the 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 defaults.
>  **/
> @@ -364,6 +421,7 @@ SetupDefaultsDevicePlatformPolicy (
>    IN  PCI_IO_DEVICE               *PciDevice
>    )
>  {
> +  PciDevice->SetupMPS = EFI_PCI_CONF_MAX_PAYLOAD_SIZE_AUTO;
>  }
> 
>  /**
> @@ -399,6 +457,7 @@ GetPciDevicePlatformPolicyEx (
>        //
>        // platform chipset policies are returned for this PCI device
>        //
> +      PciIoDevice->SetupMPS = PciPlatformExtendedPolicy.DeviceCtlMPS;
> 
>        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 type
> +                  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 type
> +                  EFI_PCI_CONF_MAX_PAYLOAD_SIZE
> +
> +  @retval         Range values for the Max_Payload_Size as defined in the PCI
> +                  Base Specification 4.0 **/
> +UINT8
> +TranslateMpsSetupValueToPci (
> +  IN  UINT8                   MPS
> +);
>  #endif
> --
> 2.21.0.windows.1
> 
> 
>