Re: [PATCH 3/3] IntelSiliconPkg/VTdPmrPei: Add EndOfPei callback for S3

["Zeng, Star" <star.zeng@intel.com>]

With that updated, Reviewed-by: Star Zeng <star.zeng@intel.com> to this patch series.

Thanks,
Star
-----Original Message-----
From: Yao, Jiewen 
Sent: Friday, September 22, 2017 12:42 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Subject: RE: [PATCH 3/3] IntelSiliconPkg/VTdPmrPei: Add EndOfPei callback for S3

Good catch. Typo. It should be VTdPmr.

Yes, we need add gEfiPeiMasterBootModePpiGuid - that is very good suggestion.

Thank you
Yao Jiewen

> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, September 22, 2017 12:22 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH 3/3] IntelSiliconPkg/VTdPmrPei: Add EndOfPei 
> callback for S3
> 
> Is the code comment correct?
> 
> +  //
> +  // Register EndOfPei Notify for S3 to run FSP NotifyPhase  //
> 
> Should the "VTdPrm" be "VTdPmr"?
> 
> If checking the bootmode in entrypoint, should add 
> gEfiPeiMasterBootModePpiGuid in the depex or not?
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Friday, September 22, 2017 11:45 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH 3/3] IntelSiliconPkg/VTdPmrPei: Add EndOfPei callback 
> for S3
> 
> In S3 resume, before system transfer to waking vector, the VTdPrm need 
> turn off VTd protection based upon VTdPolicy.
> 
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c   | 59
> +++++++++++++++++++-
>  IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf |  1 +
>  2 files changed, 58 insertions(+), 2 deletions(-)
> 
> diff --git 
> a/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
> b/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
> index e768274..3fe6d65 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
> @@ -24,16 +24,18 @@
>  #include <IndustryStandard/Vtd.h>
>  #include <Ppi/IoMmu.h>
>  #include <Ppi/VtdInfo.h>
> +#include <Ppi/EndOfPeiPhase.h>
> 
>  #include "IntelVTdPmrPei.h"
> 
>  #define  TOTAL_DMA_BUFFER_SIZE    SIZE_4MB
> +#define  TOTAL_DMA_BUFFER_SIZE_S3 SIZE_1MB
> 
>  EFI_ACPI_DMAR_HEADER              *mAcpiDmarTable;
>  VTD_INFO                          *mVTdInfo;
>  UINT64                            mEngineMask;
>  UINTN                             mDmaBufferBase;
> -UINTN                             mDmaBufferSize =
> TOTAL_DMA_BUFFER_SIZE;
> +UINTN                             mDmaBufferSize;
>  UINTN                             mDmaBufferCurrentTop;
>  UINTN                             mDmaBufferCurrentBottom;
> 
> @@ -544,6 +546,7 @@ InitDmaProtection (
>    }
>    ASSERT (DmaBufferSize == ALIGN_VALUE(DmaBufferSize, 
> MemoryAlignment));
>    *DmaBufferBase = (UINTN)AllocateAlignedPages 
> (EFI_SIZE_TO_PAGES(DmaBufferSize), MemoryAlignment);
> +  ASSERT (*DmaBufferBase != 0);
>    if (*DmaBufferBase == 0) {
>      DEBUG ((DEBUG_INFO, " InitDmaProtection : OutOfResource\n"));
>      return EFI_OUT_OF_RESOURCES;
> @@ -1105,6 +1108,41 @@ ParseDmarAcpiTableRmrr (  }
> 
>  /**
> +  This function handles S3 resume task at the end of PEI
> +
> +  @param[in] PeiServices    Pointer to PEI Services Table.
> +  @param[in] NotifyDesc     Pointer to the descriptor for the Notification
> event that
> +                            caused this function to execute.
> +  @param[in] Ppi            Pointer to the PPI data associated with this
> function.
> +
> +  @retval EFI_STATUS        Always return EFI_SUCCESS
> +**/
> +EFI_STATUS
> +EFIAPI
> +S3EndOfPeiNotify(
> +  IN EFI_PEI_SERVICES          **PeiServices,
> +  IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDesc,
> +  IN VOID                      *Ppi
> +  )
> +{
> +  UINT64                      EngineMask;
> +
> +  DEBUG((DEBUG_INFO, "VTdPmr S3EndOfPeiNotify\n"));
> +
> +  if ((PcdGet8(PcdVTdPolicyPropertyMask) & BIT1) == 0) {
> +    EngineMask = LShiftU64 (1, mVTdInfo->VTdEngineCount) - 1;
> +    DisableDmaProtection (EngineMask);
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_PEI_NOTIFY_DESCRIPTOR mS3EndOfPeiNotifyDesc = {
> +  (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> +  &gEfiEndOfPeiSignalPpiGuid,
> +  S3EndOfPeiNotify
> +};
> +
> +/**
>    Initializes the Intel VTd PMR PEIM.
> 
>    @param  FileHandle  Handle of the file being invoked.
> @@ -1122,11 +1160,14 @@ IntelVTdPmrInitialize (
>    )
>  {
>    EFI_STATUS                  Status;
> +  EFI_BOOT_MODE               BootMode;
> 
>    if ((PcdGet8(PcdVTdPolicyPropertyMask) & BIT0) == 0) {
>      return EFI_UNSUPPORTED;
>    }
> 
> +  PeiServicesGetBootMode (&BootMode);
> +
>    Status = PeiServicesLocatePpi (
>               &gEdkiiVTdInfoPpiGuid,
>               0,
> @@ -1150,6 +1191,13 @@ IntelVTdPmrInitialize (
>    //
>    ParseDmarAcpiTableRmrr ();
> 
> +  if (BootMode == BOOT_ON_S3_RESUME) {
> +    mDmaBufferSize = TOTAL_DMA_BUFFER_SIZE_S3;  } else {
> +    mDmaBufferSize = TOTAL_DMA_BUFFER_SIZE;  }  DEBUG ((DEBUG_INFO, " 
> + DmaBufferSize : 0x%x\n", mDmaBufferSize));
> +
>    //
>    // Find a pre-memory in resource hob as DMA buffer
>    // Mark PEI memory to be DMA protected.
> @@ -1160,7 +1208,6 @@ IntelVTdPmrInitialize (
>    }
> 
>    DEBUG ((DEBUG_INFO, " DmaBufferBase : 0x%x\n", mDmaBufferBase));
> -  DEBUG ((DEBUG_INFO, " DmaBufferSize : 0x%x\n", mDmaBufferSize));
> 
>    mDmaBufferCurrentTop = mDmaBufferBase + mDmaBufferSize;
>    mDmaBufferCurrentBottom = mDmaBufferBase; @@ -1171,6 +1218,14 @@ 
> IntelVTdPmrInitialize (
>    Status = PeiServicesInstallPpi (&mIoMmuPpiList);
>    ASSERT_EFI_ERROR(Status);
> 
> +  //
> +  // Register EndOfPei Notify for S3 to run FSP NotifyPhase  //  if 
> + (BootMode == BOOT_ON_S3_RESUME) {
> +    Status = PeiServicesNotifyPpi (&mS3EndOfPeiNotifyDesc);
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
>    return Status;
>  }
> 
> diff --git 
> a/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf
> b/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf
> index 86cd7d1..e1aa980 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf
> @@ -46,6 +46,7 @@
>  [Ppis]
>    gEdkiiIoMmuPpiGuid            ## PRODUCES
>    gEdkiiVTdInfoPpiGuid          ## CONSUMES
> +  gEfiEndOfPeiSignalPpiGuid     ## CONSUMES
> 
>  [Pcd]
>    gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask   ##
> CONSUMES
> --
> 2.7.4.windows.1