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

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

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

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 08/12]
> PciBusDxe: New PCI Express feature AtomicOp
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2499
> 
> 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 device
>    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 Requester
>    function for this device, than this device is enabled if all its bridge
>    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 not
>    applicable
> 
> 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             |   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_DEVICE.
> +  @param PciExFeatureConfiguration      pointer to common configuration
> table to
> +                                        initialize the PCI Express
> + feature
> +
> +  @retval EFI_SUCCESS                   bridge device routing capability 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 TRUE, negate
> +      // in case if any of the PCI Bridge device in the PCI tree does not support
> +      // the AtomicOp Routing capability
> +      //
> +      if (PciExFeatureConfiguration == NULL) {
> +        return EFI_INVALID_PARAMETER;
> +      }
> +      PciExFeatureConfiguration->AtomicOpRoutingSupported = 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 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
> +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 = 0;
> +  Offset = PciDevice->PciExpressCapabilityOffset +
> +               OFFSET_OF (PCI_CAPABILITY_PCIEXP, DeviceControl2);
> + Status = PciDevice->PciIo.Pci.Read (
> +                                  &PciDevice->PciIo,
> +                                  EfiPciIoWidthUint16,
> +                                  Offset,
> +                                  1,
> +                                  &PcieDev.Uint16
> +                                  );
> +  ASSERT (Status == EFI_SUCCESS);
> +
> +  if (PciDevice->SetupAtomicOp.Override) {
> +    //
> +    // override AtomicOp requester device control bit of the device based 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 != PciDevice-
> >SetupAtomicOp.Enable_AtomicOpRequester) {
> +        PcieDev.Bits.AtomicOpRequester = 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 = 1;
> +        }
> +      }
> +    } else {
> +      //
> +      // in the case of non-bridge device
> +      //
> +      if (PciExFeatureConfiguration) {
> +        //
> +        // for a device as AtomicOp Requester function; its bridge devices should
> +        // support the AtomicOp Routing capability to enable the device's as a
> +        // requester function
> +        //
> +        if (PciExFeatureConfiguration->AtomicOpRoutingSupported) {
> +          if (PcieDev.Bits.AtomicOpRequester != PciDevice-
> >SetupAtomicOp.Enable_AtomicOpRequester) {
> +            PcieDev.Bits.AtomicOpRequester = 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 != PciDevice-
> >SetupAtomicOp.Enable_AtomicOpRequester) {
> +          PcieDev.Bits.AtomicOpRequester = PciDevice-
> >SetupAtomicOp.Enable_AtomicOpRequester;
> +        }
> +      }
> +      //
> +      // the enabling of AtomicOp Egress Blocking is not applicable to a non-
> bridge
> +      // device
> +      //
> +    }
> +    DEBUG ((
> +      DEBUG_INFO,
> +      "AtomicOp=%d,",
> +      PcieDev.Bits.AtomicOpRequester
> +      ));
> +
> +    //
> +    // 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.DeviceControl2.Uint16 =
> 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_DEVICE.
> +  @param PciExFeatureConfiguration      pointer to common configuration
> table to
> +                                        initialize the PCI Express
> + feature
> +
> +  @retval EFI_SUCCESS                   bridge device routing capability 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 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
> +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 = {
>      //
>      // 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].PciExpressFeatureId]
> == TRUE
>      ) {
>        return TRUE;
> +    } else if (
> +        //
> +        // the PCI Express feature does not require extended setup phase but it
> +        // does require global flag to track the AtomicOpRouting caoability to
> +        // be tracked for all its bridge devices
> +        //
> +        idx == PciExpressAtomicOp
> +        && PciExpressPolicy[idx] == TRUE
> +        ) {
> +      return TRUE;
>      }
>    }
> 
> @@ -637,6 +653,11 @@ CreatePciRootBridgeDeviceNode (
>      // the devices in the PCI tree
>      //
>      PciConfigTable->Lock_Max_Read_Request_Size  = FALSE;
> +    //
> +    // start by assuming the AtomicOp Routing capability is supported in the PCI
> +    // tree
> +    //
> +    PciConfigTable->AtomicOpRoutingSupported    = TRUE;
>    }
> 
>    RootBridgeNode->PciExFeaturesConfigurationTable  = 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_Size;
> +  //
> +  // 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 = 0;
> 
> +  PciDevice->SetupAtomicOp.Override = 0;
> +
>  }
> 
>  /**
> @@ -450,6 +452,14 @@ GetPciExpressDevicePolicy (
>        PciDevice->SetupCTO.Override = 0;
>      }
> 
> +    //
> +    // set the device-specific policy for AtomicOp
> +    //
> +    if (mPciExpressPlatformPolicy.AtomicOp) {
> +      PciDevice->SetupAtomicOp = PciExpressDevicePolicy.DeviceCtl2AtomicOp;
> +    } else {
> +      PciDevice->SetupAtomicOp.Override = 0;
> +    }
> 
>      DEBUG ((
>        DEBUG_INFO,
> @@ -692,6 +702,19 @@ PciExpressPlatformNotifyDeviceState (
>      PciExDeviceConfiguration.CTOsupport = EFI_PCI_EXPRESS_NOT_APPLICABLE;
>    }
> 
> +  //
> +  // get the device-specific state for the PCIe AtomicOp feature  //
> + if (mPciExpressPlatformPolicy.AtomicOp) {
> +    PciExDeviceConfiguration.DeviceCtl2AtomicOp.Enable_AtomicOpRequester
> +    = (UINT8)PciDevice-
> >PciExpressCapabilityStructure.DeviceControl2.Bits.AtomicOpRequester;
> +
> PciExDeviceConfiguration.DeviceCtl2AtomicOp.Enable_AtomicOpEgressBlockin
> g
> +    =
> + (UINT8)PciDevice->PciExpressCapabilityStructure.DeviceControl2.Bits.At
> + omicOpEgressBlocking;
> +  } else {
> +    PciExDeviceConfiguration.DeviceCtl2AtomicOp.Override = 0;
> +    PciExDeviceConfiguration.DeviceCtl2AtomicOp.Enable_AtomicOpRequester
> = 0;
> +
> + PciExDeviceConfiguration.DeviceCtl2AtomicOp.Enable_AtomicOpEgressBlock
> + ing = 0;  }
> 
>    if (mPciExPlatformProtocol != NULL) {
>      return mPciExPlatformProtocol->NotifyDeviceState (
> --
> 2.21.0.windows.1
> 
> 
>