Re: [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 10/15] MdeModulePkg/PciBusDxe: Enable NoSnoop feature

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Javeed, Ashraf <ashraf.javeed@intel.com>
> Sent: Monday, May 11, 2020 12:14 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 10/15] MdeModulePkg/PciBusDxe: Enable NoSnoop feature
> 
> REF:
>   https://bugzilla.tianocore.org/show_bug.cgi?id=1954
>   https://bugzilla.tianocore.org/show_bug.cgi?id=2194
>   https://bugzilla.tianocore.org/show_bug.cgi?id=2313
>   https://bugzilla.tianocore.org/show_bug.cgi?id=2499
>   https://bugzilla.tianocore.org/show_bug.cgi?id=2500
> 
> Add the Program phase feature init routine for NoSnoop
> PCIe feature.
> 
> Signed-off-by: Ashraf Javeed <ashraf.javeed@intel.com>
> Signed-off-by: Ray Ni <ray.ni@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>
> Cc: Ashraf Javeed <ashraf.javeed@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatureSupport.c |  4 ++++
>  MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatures.c       | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatures.h       |  7 +++++++
>  3 files changed, 62 insertions(+)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatureSupport.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatureSupport.c
> index 9948e17..6bf06b0 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatureSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatureSupport.c
> @@ -58,6 +58,8 @@ PCIE_FEATURE_ENTRY  mPcieFeatures[] = {
>                TRUE, { TRUE,  TRUE }, { NULL,                    MaxReadRequestSizeProgram } },
>    { OFFSET_OF (EFI_PCI_EXPRESS_PLATFORM_POLICY, RelaxedOrdering),
>                TRUE, { TRUE,  TRUE }, { NULL,                    RelaxedOrderingProgram } },
> +  { OFFSET_OF (EFI_PCI_EXPRESS_PLATFORM_POLICY, NoSnoop),
> +              TRUE, { TRUE,  TRUE }, { NULL,                    NoSnoopProgram } },
>  };
> 
>  /**
> @@ -233,6 +235,8 @@ PcieNotifyDeviceState (
>    PcieDeviceState.MaxPayloadSize      = (UINT8)PciIoDevice->PciExpressCapability.DeviceControl.Bits.MaxPayloadSize;
>    PcieDeviceState.MaxReadRequestSize  = (UINT8)PciIoDevice-
> >PciExpressCapability.DeviceControl.Bits.MaxReadRequestSize;
>    PcieDeviceState.RelaxedOrdering     = (UINT8)PciIoDevice->PciExpressCapability.DeviceControl.Bits.RelaxedOrdering;
> +  PcieDeviceState.NoSnoop             = (UINT8)PciIoDevice->PciExpressCapability.DeviceControl.Bits.NoSnoop;
> +
>    return mPciePlatformProtocol->NotifyDeviceState (
>                                    mPciePlatformProtocol,
>                                    PciIoDevice->Handle,
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatures.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatures.c
> index 5216dac..6c22feb 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatures.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatures.c
> @@ -216,3 +216,54 @@ RelaxedOrderingProgram (
>    return EFI_SUCCESS;
>  }
> 
> +/**
> +  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.
> +  @param Level                  The level of the PCI device in the heirarchy.
> +                                Level of root ports is 0.
> +  @param Context                Pointer to feature specific context.
> +
> +  @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
> +NoSnoopProgram (
> +  IN PCI_IO_DEVICE *PciDevice,
> +  IN UINTN         Level,
> +  IN VOID          **Context
> +  )
> +{
> +  ASSERT (*Context == NULL);
> +
> +  if (PciDevice->DeviceState.NoSnoop == EFI_PCI_EXPRESS_DEVICE_POLICY_NOT_APPLICABLE ||
> +      PciDevice->DeviceState.NoSnoop == EFI_PCI_EXPRESS_DEVICE_POLICY_AUTO) {
> +    return EFI_SUCCESS;
> +  }
> +
> +
> +  if (PciDevice->PciExpressCapability.DeviceControl.Bits.NoSnoop != PciDevice->DeviceState.NoSnoop) {
> +    DEBUG ((
> +      DEBUG_INFO, "  %a [%02d|%02d|%02d]: %x -> %x\n",
> +      __FUNCTION__, PciDevice->BusNumber, PciDevice->DeviceNumber, PciDevice->FunctionNumber,
> +      PciDevice->PciExpressCapability.DeviceControl.Bits.NoSnoop,
> +      PciDevice->DeviceState.NoSnoop
> +      ));
> +    PciDevice->PciExpressCapability.DeviceControl.Bits.NoSnoop = PciDevice->DeviceState.NoSnoop;
> +
> +    return PciDevice->PciIo.Pci.Write (
> +                                  &PciDevice->PciIo,
> +                                  EfiPciIoWidthUint16,
> +                                  PciDevice->PciExpressCapabilityOffset
> +                                  + OFFSET_OF (PCI_CAPABILITY_PCIEXP, DeviceControl),
> +                                  1,
> +                                  &PciDevice->PciExpressCapability.DeviceControl.Uint16
> +                                  );
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatures.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatures.h
> index 7d70f06..60b8742 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatures.h
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PcieFeatures.h
> @@ -38,4 +38,11 @@ RelaxedOrderingProgram (
>    IN VOID          **Context
>    );
> 
> +EFI_STATUS
> +NoSnoopProgram (
> +  IN PCI_IO_DEVICE *PciDevice,
> +  IN UINTN         Level,
> +  IN VOID          **Context
> +  );
> +
>  #endif
> --
> 2.21.0.windows.1