Re: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 05/12] PciBusDxe: New PCI Express feature Relax Ordering

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

This patch can also be viewed in the following repo:-
https://github.com/ashrafj/edk2-staging/commit/27d11f3bbba23ff8b55d67da3cc50f8ee6029103

Thanks
Ashraf

> -----Original Message-----
> From: devel@edk2.groups.io <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 <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 05/12]
> PciBusDxe: New PCI Express feature Relax Ordering
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2313
> 
> The code changes are made to enable the configuration of PCI Express feature
> Relax Ordering (OR), that enables the PCI function to initiate requests if it does
> not require strong write ordering for its transact- ions; as per the PCI Express
> Base Specification 4 Revision 1.
> 
> The code changes are made to configure only those PCI devices which are
> requested by platform for override, through the new PCI Express Platform
> protocol interface for device-specific policies.
> 
> 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/PciExpressFeatures.c | 70
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h | 18
> ++++++++++++++++++  MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c
> |  5 ++++-  MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c | 61
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 157 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> index 77b44c0..d3d795d 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> @@ -287,8 +287,12 @@ struct _PCI_IO_DEVICE {
>    // This field is used to support this case.
>    //
>    UINT16                                    BridgeIoAlignment;
> +  //
> +  // PCI Express features setup flags
> +  //
>    UINT8                                     SetupMPS;
>    UINT8                                     SetupMRRS;
> +  PCI_FEATURE_POLICY                        SetupRO;
>  };
> 
>  #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 2810158..3262b76 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.c
> @@ -381,3 +381,73 @@ ProgramMaxReadReqSize (
>    return Status;
>  }
> 
> +/**
> +  Overrides the PCI Device Control register Relax Order 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
> +ProgramRelaxOrder (
> +  IN PCI_IO_DEVICE          *PciDevice,
> +  IN VOID                   *PciExFeatureConfiguration
> +  )
> +{
> +  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
> +                                  );
> +  ASSERT (Status == EFI_SUCCESS);
> +
> +  if (PciDevice->SetupRO.Override
> +      &&  PcieDev.Bits.RelaxedOrdering != PciDevice->SetupRO.Act
> +      ) {
> +    PcieDev.Bits.RelaxedOrdering = PciDevice->SetupRO.Act;
> +    DEBUG (( DEBUG_INFO, "RO=%d,", PciDevice->SetupRO.Act));
> +
> +    //
> +    // 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->PciExpressCapabilityStructure.DeviceControl.Uint16 =
> PcieDev.Uint16;
> +    } else {
> +      ReportPciWriteError (PciDevice->BusNumber, PciDevice->DeviceNumber,
> PciDevice->FunctionNumber, Offset);
> +    }
> +  } else {
> +    DEBUG (( DEBUG_INFO, "No RO,", PciDevice->SetupRO.Act));  }
> +
> +  return Status;
> +}
> +
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h
> index b43fba7..0d17801 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciExpressFeatures.h
> @@ -97,4 +97,22 @@ ProgramMaxReadReqSize (
>    IN VOID                   *PciExFeatureConfiguration
>    );
> 
> +/**
> +  Overrides the PCI Device Control register Relax Order 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
> +ProgramRelaxOrder (
> +  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 1caf1f4..267f570 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c
> @@ -42,7 +42,7 @@ EFI_PCI_EXPRESS_PLATFORM_POLICY
> mPciExpressPlatformPolicy = {
>      //
>      // support for PCI Express feature - Relax Order
>      //
> -    FALSE,
> +    TRUE,
>      //
>      // support for PCI Express feature - No-Snoop
>      //
> @@ -113,6 +113,9 @@ PCI_EXPRESS_FEATURE_INITIALIZATION_POINT
> mPciExpressFeatureInitializationList[]
>    },
>    {
>      PciExpressFeatureProgramPhase,        PciExpressMrrs,
> ProgramMaxReadReqSize
> +  },
> +  {
> +    PciExpressFeatureProgramPhase,        PciExpressRelaxOrder,
> ProgramRelaxOrder
>    }
>  };
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c
> index f74e566..40eb8a3 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c
> @@ -149,6 +149,45 @@ SetDevicePolicyPciExpressMrrs (
>    }
>  }
> 
> +/**
> +  Routine to set the device-specific policy for the PCI feature Relax
> +Ordering
> +
> +  @param  RelaxOrder    value corresponding to data type
> EFI_PCI_EXPRESS_RELAX_ORDER
> +  @param  PciDevice     A pointer to PCI_IO_DEVICE
> +**/
> +VOID
> +SetDevicePolicyPciExpressRo (
> +  IN  EFI_PCI_EXPRESS_RELAX_ORDER RelaxOrder,
> +  OUT PCI_IO_DEVICE               *PciDevice
> +  )
> +{
> +  //
> +  // implementation specific rules for the usage of PCI_FEATURE_POLICY
> +members
> +  // exclusively for the PCI Feature Relax Ordering (RO)
> +  //
> +  // .Override = 0 to skip this PCI feature RO for the PCI device
> +  // .Override = 1 to program this RO PCI feature
> +  //      .Act = 1 to enable the RO in the PCI device
> +  //      .Act = 0 to disable the RO in the PCI device
> +  //
> +  switch (RelaxOrder) {
> +    case  EFI_PCI_EXPRESS_RO_AUTO:
> +      PciDevice->SetupRO.Override = 0;
> +      break;
> +    case  EFI_PCI_EXPRESS_RO_DISABLE:
> +      PciDevice->SetupRO.Override = 1;
> +      PciDevice->SetupRO.Act = 0;
> +      break;
> +    case  EFI_PCI_EXPRESS_RO_ENABLE:
> +      PciDevice->SetupRO.Override = 1;
> +      PciDevice->SetupRO.Act = 1;
> +      break;
> +    default:
> +      PciDevice->SetupRO.Override = 0;
> +      break;
> +  }
> +}
> +
>  /**
>    Generic routine to setup the PCI features as per its predetermined defaults.
>  **/
> @@ -170,6 +209,8 @@ SetupDefaultPciExpressDevicePolicy (
>      PciDevice->SetupMRRS = EFI_PCI_EXPRESS_NOT_APPLICABLE;
>    }
> 
> +  PciDevice->SetupRO.Override = 0;
> +
>  }
> 
>  /**
> @@ -259,6 +300,15 @@ GetPciExpressDevicePolicy (
>      } else {
>        PciDevice->SetupMRRS = EFI_PCI_EXPRESS_NOT_APPLICABLE;
>      }
> +    //
> +    // set device specific policy for Relax Ordering
> +    //
> +    if (mPciExpressPlatformPolicy.RelaxOrder) {
> +      SetDevicePolicyPciExpressRo (PciExpressDevicePolicy.DeviceCtlRelaxOrder,
> PciDevice);
> +    } else {
> +      PciDevice->SetupRO.Override = 0;
> +    }
> +
> 
>      DEBUG ((
>        DEBUG_INFO,
> @@ -438,6 +488,17 @@ PciExpressPlatformNotifyDeviceState (
>    } else {
>      PciExDeviceConfiguration.DeviceCtlMRRS =
> EFI_PCI_EXPRESS_NOT_APPLICABLE;
>    }
> +  //
> +  // get the device-specific state for the PCIe Relax Order feature  //
> + if (mPciExpressPlatformPolicy.RelaxOrder) {
> +    PciExDeviceConfiguration.DeviceCtlRelaxOrder = PciDevice-
> >PciExpressCapabilityStructure.DeviceControl.Bits.RelaxedOrdering
> +                                                      ? EFI_PCI_EXPRESS_RO_ENABLE
> +                                                      :
> + EFI_PCI_EXPRESS_RO_DISABLE;  } else {
> +    PciExDeviceConfiguration.DeviceCtlRelaxOrder =
> + EFI_PCI_EXPRESS_NOT_APPLICABLE;  }
> +
> 
>    if (mPciExPlatformProtocol != NULL) {
>      return mPciExPlatformProtocol->NotifyDeviceState (
> --
> 2.21.0.windows.1
> 
> 
>