[PATCH 0/3] IntelVTd fix

[PATCH 0/3] IntelVTd fix

[Jiewen Yao]

1) Make sure context cache in VTdDxe is flushed in every path.
2) Clarify VTdPolicy PCD usage for PEI and DXE.
3) Add EndOfPei callback in VTdPrmPei for S3 resume.

The patch is tested in Intel Kabylake platform.
Windows 10 boot and S3 resume success with VTd enabled.

Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>

Jiewen Yao (3):
  IntelSiliconPkg/VTdDxe: Clean up DXE flush memory.
  IntelSiliconPkg/dec: Clarify VTdPolicy.
  IntelSiliconPkg/VTdPmrPei: Add EndOfPei callback for S3

 IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c    |  9 ++-
 IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.c  |  4 +-
 IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c   | 59 +++++++++++++++++++-
 IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf |  1 +
 IntelSiliconPkg/IntelSiliconPkg.dec                           |  4 +-
 5 files changed, 69 insertions(+), 8 deletions(-)

-- 
2.7.4.windows.1

[PATCH 1/3] IntelSiliconPkg/VTdDxe: Clean up DXE flush memory.

[Jiewen Yao]

Make sure the context table are flush to memory.

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/IntelVTdDxe/TranslationTable.c   | 9 +++++++--
 IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.c | 4 ++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c
index ccecc95..cecb5d2 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c
@@ -124,7 +124,6 @@ CreateContextEntry (
       RootEntry->Bits.ContextTablePointerHi  = (UINT32) RShiftU64 ((UINT64)(UINTN)Buffer, 32);
       RootEntry->Bits.Present = 1;
       Buffer = (UINT8 *)Buffer + EFI_PAGES_TO_SIZE (ContextPages);
-      FlushPageTableMemory (VtdIndex, (UINTN)RootEntry, sizeof(*RootEntry));
     }
 
     ContextEntryTable = (VTD_CONTEXT_ENTRY *)(UINTN)VTD_64BITS_ADDRESS(RootEntry->Bits.ContextTablePointerLo, RootEntry->Bits.ContextTablePointerHi) ;
@@ -143,9 +142,10 @@ CreateContextEntry (
       ContextEntry->Bits.AddressWidth = 0x2;
       break;
     }
-    FlushPageTableMemory (VtdIndex, (UINTN)ContextEntry, sizeof(*ContextEntry));
   }
 
+  FlushPageTableMemory (VtdIndex, (UINTN)mVtdUnitInformation[VtdIndex].RootEntryTable, EFI_PAGES_TO_SIZE(EntryTablePages));
+
   return EFI_SUCCESS;
 }
 
@@ -196,6 +196,7 @@ CreateSecondLevelPagingEntryTable (
       DEBUG ((DEBUG_ERROR,"Could not Alloc LVL4 PT. \n"));
       return NULL;
     }
+    FlushPageTableMemory (VtdIndex, (UINTN)SecondLevelPagingEntry, EFI_PAGES_TO_SIZE(1));
   }
 
   //
@@ -219,6 +220,7 @@ CreateSecondLevelPagingEntryTable (
         ASSERT(FALSE);
         return NULL;
       }
+      FlushPageTableMemory (VtdIndex, (UINTN)Lvl4PtEntry[Index4].Uint64, SIZE_4KB);
       SetSecondLevelPagingEntryAttribute (&Lvl4PtEntry[Index4], EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE);
     }
 
@@ -239,6 +241,7 @@ CreateSecondLevelPagingEntryTable (
           ASSERT(FALSE);
           return NULL;
         }
+        FlushPageTableMemory (VtdIndex, (UINTN)Lvl3PtEntry[Index3].Uint64, SIZE_4KB);
         SetSecondLevelPagingEntryAttribute (&Lvl3PtEntry[Index3], EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE);
       }
 
@@ -542,6 +545,7 @@ GetSecondLevelPageTableEntry (
       *PageAttribute = PageNone;
       return NULL;
     }
+    FlushPageTableMemory (VtdIndex, (UINTN)L4PageTable[Index4], SIZE_4KB);
     SetSecondLevelPagingEntryAttribute ((VTD_SECOND_LEVEL_PAGING_ENTRY *)&L4PageTable[Index4], EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE);
     FlushPageTableMemory (VtdIndex, (UINTN)&L4PageTable[Index4], sizeof(L4PageTable[Index4]));
   }
@@ -555,6 +559,7 @@ GetSecondLevelPageTableEntry (
       *PageAttribute = PageNone;
       return NULL;
     }
+    FlushPageTableMemory (VtdIndex, (UINTN)L3PageTable[Index3], SIZE_4KB);
     SetSecondLevelPagingEntryAttribute ((VTD_SECOND_LEVEL_PAGING_ENTRY *)&L3PageTable[Index3], EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE);
     FlushPageTableMemory (VtdIndex, (UINTN)&L3PageTable[Index3], sizeof(L3PageTable[Index3]));
   }
diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.c
index 68b25a7..0b3006a 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.c
@@ -73,7 +73,6 @@ CreateExtContextEntry (
       ExtRootEntry->Bits.UpperContextTablePointerLo  = (UINT32) RShiftU64 ((UINT64)(UINTN)Buffer, 12) + 1;
       ExtRootEntry->Bits.UpperContextTablePointerHi  = (UINT32) RShiftU64 (RShiftU64 ((UINT64)(UINTN)Buffer, 12) + 1, 20);
       ExtRootEntry->Bits.UpperPresent = 1;
-      FlushPageTableMemory (VtdIndex, (UINTN)ExtRootEntry, sizeof(*ExtRootEntry));
       Buffer = (UINT8 *)Buffer + EFI_PAGES_TO_SIZE (ContextPages);
     }
 
@@ -93,9 +92,10 @@ CreateExtContextEntry (
       ExtContextEntry->Bits.AddressWidth = 0x2;
       break;
     }
-    FlushPageTableMemory (VtdIndex, (UINTN)ExtContextEntry, sizeof(*ExtContextEntry));
   }
 
+  FlushPageTableMemory (VtdIndex, (UINTN)mVtdUnitInformation[VtdIndex].ExtRootEntryTable, EFI_PAGES_TO_SIZE(EntryTablePages));
+
   return EFI_SUCCESS;
 }
 
-- 
2.7.4.windows.1

[PATCH 2/3] IntelSiliconPkg/dec: Clarify VTdPolicy.

[Jiewen Yao]

Clarify the VTdPolicy is for both PEI and DXE.

Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
 IntelSiliconPkg/IntelSiliconPkg.dec | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/IntelSiliconPkg/IntelSiliconPkg.dec b/IntelSiliconPkg/IntelSiliconPkg.dec
index 941380a..b88630d 100644
--- a/IntelSiliconPkg/IntelSiliconPkg.dec
+++ b/IntelSiliconPkg/IntelSiliconPkg.dec
@@ -48,8 +48,8 @@
   gIntelSiliconPkgTokenSpaceGuid.PcdIntelGraphicsVbtFileGuid|{ 0xa9, 0x2d, 0x75, 0x56, 0x6b, 0xde, 0x95, 0x48, 0x88, 0x19, 0x19, 0x45, 0xb6, 0xb7, 0x6c, 0x22 }|VOID*|0x00000001
 
   ## The mask is used to control VTd behavior.<BR><BR>
-  #  BIT0: Enable IOMMU during boot.
-  #  BIT1: Enable IOMMU on exit boot service.
+  #  BIT0: Enable IOMMU during boot (If DMAR table is installed in DXE. If VTD_INFO_PPI is installed in PEI.)
+  #  BIT1: Enable IOMMU when transfer control to OS (ExitBootService in normal boot. EndOfPEI in S3)
   # @Prompt The policy for VTd driver behavior.
   gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask|1|UINT8|0x00000002
 
-- 
2.7.4.windows.1

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

[Jiewen Yao]

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

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

[Zeng, Star]

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

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

[Yao, Jiewen]

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

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

[Zeng, Star]

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