public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 2/8] MdeModulePkg/SmmLockBoxPeiLib: Update MM PPI usages.
       [not found] <d625966ce7c0ac98b65f1e946b2cf833f6801706.1532395896.git.Marvin.Haeuser@outlook.com>
@ 2018-07-24  1:40 ` Marvin Häuser
  2018-07-24  1:40 ` [PATCH 3/8] UefiCpuPkg/PiSmmCommunicationPei: " Marvin Häuser
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Marvin Häuser @ 2018-07-24  1:40 UTC (permalink / raw)
  To: edk2-devel@lists.01.org
  Cc: michael.d.kinney@intel.com, liming.gao@intel.com,
	star.zeng@intel.com, eric.dong@intel.com, ruiyu.ni@intel.com,
	lersek@redhat.com, kelly.steele@intel.com,
	jordan.l.justen@intel.com, ard.biesheuvel@linaro.org

Update all references to the SMM PPIs from MdeModulePkg to rather use
MdePkg's MM PPI declarations.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
---
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c   | 52 ++++++++++----------
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf |  4 +-
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
index 8a168663c4af..a0f3ab9e76c6 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
@@ -25,8 +25,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/DebugLib.h>
 #include <Library/PcdLib.h>
 #include <Protocol/SmmCommunication.h>
-#include <Ppi/SmmCommunication.h>
-#include <Ppi/SmmAccess.h>
+#include <Ppi/MmCommunication.h>
+#include <Ppi/MmAccess.h>
 #include <Guid/AcpiS3Context.h>
 #include <Guid/SmmLockBox.h>
 
@@ -249,7 +249,7 @@ InternalRestoreLockBoxFromSmram (
   IN  OUT UINTN                   *Length  OPTIONAL
   )
 {
-  PEI_SMM_ACCESS_PPI             *SmmAccess;
+  EFI_PEI_MM_ACCESS_PPI          *MmAccess;
   UINTN                          Index;
   EFI_STATUS                     Status;
   SMM_LOCK_BOX_CONTEXT           *SmmLockBoxContext;
@@ -261,14 +261,14 @@ InternalRestoreLockBoxFromSmram (
   // Get needed resource
   //
   Status = PeiServicesLocatePpi (
-             &gPeiSmmAccessPpiGuid,
+             &gEfiPeiMmAccessPpiGuid,
              0,
              NULL,
-             (VOID **)&SmmAccess
+             (VOID **)&MmAccess
              );
   if (!EFI_ERROR (Status)) {
     for (Index = 0; !EFI_ERROR (Status); Index++) {
-      Status = SmmAccess->Open ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), SmmAccess, Index);
+      Status = MmAccess->Open ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), MmAccess, Index);
     }
   }
 
@@ -350,7 +350,7 @@ InternalRestoreAllLockBoxInPlaceFromSmram (
   VOID
   )
 {
-  PEI_SMM_ACCESS_PPI             *SmmAccess;
+  EFI_PEI_MM_ACCESS_PPI          *MmAccess;
   UINTN                          Index;
   EFI_STATUS                     Status;
   SMM_LOCK_BOX_CONTEXT           *SmmLockBoxContext;
@@ -362,14 +362,14 @@ InternalRestoreAllLockBoxInPlaceFromSmram (
   // Get needed resource
   //
   Status = PeiServicesLocatePpi (
-             &gPeiSmmAccessPpiGuid,
+             &gEfiPeiMmAccessPpiGuid,
              0,
              NULL,
-             (VOID **)&SmmAccess
+             (VOID **)&MmAccess
              );
   if (!EFI_ERROR (Status)) {
     for (Index = 0; !EFI_ERROR (Status); Index++) {
-      Status = SmmAccess->Open ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), SmmAccess, Index);
+      Status = MmAccess->Open ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), MmAccess, Index);
     }
   }
 
@@ -526,7 +526,7 @@ RestoreLockBox (
   )
 {
   EFI_STATUS                         Status;
-  EFI_PEI_SMM_COMMUNICATION_PPI      *SmmCommunicationPpi;
+  EFI_PEI_MM_COMMUNICATION_PPI       *MmCommunicationPpi;
   EFI_SMM_LOCK_BOX_PARAMETER_RESTORE *LockBoxParameterRestore;
   EFI_SMM_COMMUNICATE_HEADER         *CommHeader;
   UINT8                              CommBuffer[sizeof(EFI_GUID) + sizeof(UINT64) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)];
@@ -557,10 +557,10 @@ RestoreLockBox (
   // Get needed resource
   //
   Status = PeiServicesLocatePpi (
-             &gEfiPeiSmmCommunicationPpiGuid,
+             &gEfiPeiMmCommunicationPpiGuid,
              0,
              NULL,
-             (VOID **)&SmmCommunicationPpi
+             (VOID **)&MmCommunicationPpi
              );
   if (EFI_ERROR (Status)) {
     DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib LocatePpi - (%r)\n", Status));
@@ -607,11 +607,11 @@ RestoreLockBox (
   // Send command
   //
   CommSize = sizeof(CommBuffer);
-  Status = SmmCommunicationPpi->Communicate (
-                                  SmmCommunicationPpi,
-                                  &CommBuffer[0],
-                                  &CommSize
-                                  );
+  Status = MmCommunicationPpi->Communicate (
+                                 MmCommunicationPpi,
+                                 &CommBuffer[0],
+                                 &CommSize
+                                 );
   if (Status == EFI_NOT_STARTED) {
     //
     // Pei SMM communication not ready yet, so we access SMRAM directly
@@ -657,7 +657,7 @@ RestoreAllLockBoxInPlace (
   )
 {
   EFI_STATUS                                      Status;
-  EFI_PEI_SMM_COMMUNICATION_PPI                   *SmmCommunicationPpi;
+  EFI_PEI_MM_COMMUNICATION_PPI                    *MmCommunicationPpi;
   EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE *LockBoxParameterRestoreAllInPlace;
   EFI_SMM_COMMUNICATE_HEADER                      *CommHeader;
   UINT8                                           CommBuffer[sizeof(EFI_GUID) + sizeof(UINT64) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)];
@@ -679,10 +679,10 @@ RestoreAllLockBoxInPlace (
   // Get needed resource
   //
   Status = PeiServicesLocatePpi (
-             &gEfiPeiSmmCommunicationPpiGuid,
+             &gEfiPeiMmCommunicationPpiGuid,
              0,
              NULL,
-             (VOID **)&SmmCommunicationPpi
+             (VOID **)&MmCommunicationPpi
              );
   if (EFI_ERROR (Status)) {
     DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib LocatePpi - (%r)\n", Status));
@@ -716,11 +716,11 @@ RestoreAllLockBoxInPlace (
   // Send command
   //
   CommSize = sizeof(CommBuffer);
-  Status = SmmCommunicationPpi->Communicate (
-                                  SmmCommunicationPpi,
-                                  &CommBuffer[0],
-                                  &CommSize
-                                  );
+  Status = MmCommunicationPpi->Communicate (
+                                 MmCommunicationPpi,
+                                 &CommBuffer[0],
+                                 &CommSize
+                                 );
   if (Status == EFI_NOT_STARTED) {
     //
     // Pei SMM communication not ready yet, so we access SMRAM directly
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
index 093957ec0adf..ab093a4b255c 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
@@ -55,5 +55,5 @@ [Guids]
   gEfiAcpiVariableGuid                  ## SOMETIMES_CONSUMES ## HOB
 
 [Ppis]
-  gEfiPeiSmmCommunicationPpiGuid        ## CONSUMES
-  gPeiSmmAccessPpiGuid                  ## SOMETIMES_CONSUMES
+  gEfiPeiMmCommunicationPpiGuid         ## CONSUMES
+  gEfiPeiMmAccessPpiGuid                ## SOMETIMES_CONSUMES
-- 
2.18.0.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/8] UefiCpuPkg/PiSmmCommunicationPei: Update MM PPI usages.
       [not found] <d625966ce7c0ac98b65f1e946b2cf833f6801706.1532395896.git.Marvin.Haeuser@outlook.com>
  2018-07-24  1:40 ` [PATCH 2/8] MdeModulePkg/SmmLockBoxPeiLib: Update MM PPI usages Marvin Häuser
@ 2018-07-24  1:40 ` Marvin Häuser
  2018-07-24  1:40 ` [PATCH 4/8] UefiCpuPkg/S3Resume2Pei: " Marvin Häuser
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Marvin Häuser @ 2018-07-24  1:40 UTC (permalink / raw)
  To: edk2-devel@lists.01.org
  Cc: michael.d.kinney@intel.com, liming.gao@intel.com,
	star.zeng@intel.com, eric.dong@intel.com, ruiyu.ni@intel.com,
	lersek@redhat.com, kelly.steele@intel.com,
	jordan.l.justen@intel.com, ard.biesheuvel@linaro.org

Update all references to the SMM PPIs from MdeModulePkg to rather use
MdePkg's MM PPI declarations.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
---
 UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.c   | 70 ++++++++++----------
 UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.c   |  4 +-
 UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf | 10 +--
 UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf |  2 +-
 4 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.c b/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.c
index aaeaa0672939..5c90f5193185 100644
--- a/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.c
+++ b/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.c
@@ -22,9 +22,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/HobLib.h>
 #include <Library/DebugLib.h>
 #include <Protocol/SmmCommunication.h>
-#include <Ppi/SmmCommunication.h>
-#include <Ppi/SmmAccess.h>
-#include <Ppi/SmmControl.h>
+#include <Ppi/MmCommunication.h>
+#include <Ppi/MmAccess.h>
+#include <Ppi/MmControl.h>
 #include <Guid/AcpiS3Context.h>
 
 #include "PiSmmCommunicationPrivate.h"
@@ -113,7 +113,7 @@ typedef EFI_CONFIGURATION_TABLE EFI_CONFIGURATION_TABLE64;
 
   This function provides a service to send and receive messages from a registered UEFI service.
 
-  @param[in] This                The EFI_PEI_SMM_COMMUNICATION_PPI instance.
+  @param[in] This                The EFI_PEI_MM_COMMUNICATION_PPI instance.
   @param[in, out] CommBuffer     A pointer to the buffer to convey into SMRAM.
   @param[in, out] CommSize       The size of the data buffer being passed in.On exit, the size of data
                                  being returned. Zero if the handler does not wish to reply with any data.
@@ -125,17 +125,17 @@ typedef EFI_CONFIGURATION_TABLE EFI_CONFIGURATION_TABLE64;
 EFI_STATUS
 EFIAPI
 Communicate (
-  IN CONST EFI_PEI_SMM_COMMUNICATION_PPI   *This,
+  IN CONST EFI_PEI_MM_COMMUNICATION_PPI    *This,
   IN OUT VOID                              *CommBuffer,
   IN OUT UINTN                             *CommSize
   );
 
-EFI_PEI_SMM_COMMUNICATION_PPI      mSmmCommunicationPpi = { Communicate };
+EFI_PEI_MM_COMMUNICATION_PPI       mMmCommunicationPpi = { Communicate };
 
 EFI_PEI_PPI_DESCRIPTOR mPpiList = {
   (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
-  &gEfiPeiSmmCommunicationPpiGuid,
-  &mSmmCommunicationPpi
+  &gEfiPeiMmCommunicationPpiGuid,
+  &mMmCommunicationPpi
 };
 
 /**
@@ -151,7 +151,7 @@ GetCommunicationContext (
   EFI_HOB_GUID_TYPE                *GuidHob;
   EFI_SMM_COMMUNICATION_CONTEXT    *SmmCommunicationContext;
 
-  GuidHob = GetFirstGuidHob (&gEfiPeiSmmCommunicationPpiGuid);
+  GuidHob = GetFirstGuidHob (&gEfiPeiMmCommunicationPpiGuid);
   ASSERT (GuidHob != NULL);
 
   SmmCommunicationContext = (EFI_SMM_COMMUNICATION_CONTEXT *)GET_GUID_HOB_DATA (GuidHob);
@@ -174,7 +174,7 @@ SetCommunicationContext (
 
   BufferSize = sizeof (*SmmCommunicationContext);
   Hob.Raw = BuildGuidHob (
-              &gEfiPeiSmmCommunicationPpiGuid,
+              &gEfiPeiMmCommunicationPpiGuid,
               BufferSize
               );
   ASSERT (Hob.Raw);
@@ -257,7 +257,7 @@ InitCommunicationContext (
   SmmCommunicationContext = (EFI_SMM_COMMUNICATION_CONTEXT *)InternalSmstGetVendorTableByGuid (
                                                                SmmS3ResumeState->Signature,
                                                                (EFI_SMM_SYSTEM_TABLE2 *)(UINTN)SmmS3ResumeState->Smst,
-                                                               &gEfiPeiSmmCommunicationPpiGuid
+                                                               &gEfiPeiMmCommunicationPpiGuid
                                                                );
   ASSERT (SmmCommunicationContext != NULL);
 
@@ -271,7 +271,7 @@ InitCommunicationContext (
 
   This function provides a service to send and receive messages from a registered UEFI service.
 
-  @param[in] This                The EFI_PEI_SMM_COMMUNICATION_PPI instance.
+  @param[in] This                The EFI_PEI_MM_COMMUNICATION_PPI instance.
   @param[in, out] CommBuffer     A pointer to the buffer to convey into SMRAM.
   @param[in, out] CommSize       The size of the data buffer being passed in.On exit, the size of data
                                  being returned. Zero if the handler does not wish to reply with any data.
@@ -283,14 +283,14 @@ InitCommunicationContext (
 EFI_STATUS
 EFIAPI
 Communicate (
-  IN CONST EFI_PEI_SMM_COMMUNICATION_PPI   *This,
+  IN CONST EFI_PEI_MM_COMMUNICATION_PPI    *This,
   IN OUT VOID                              *CommBuffer,
   IN OUT UINTN                             *CommSize
   )
 {
   EFI_STATUS                       Status;
-  PEI_SMM_CONTROL_PPI              *SmmControl;
-  PEI_SMM_ACCESS_PPI               *SmmAccess;
+  EFI_PEI_MM_CONTROL_PPI           *MmControl;
+  EFI_PEI_MM_ACCESS_PPI            *MmAccess;
   UINT8                            SmiCommand;
   UINTN                            Size;
   EFI_SMM_COMMUNICATION_CONTEXT    *SmmCommunicationContext;
@@ -305,20 +305,20 @@ Communicate (
   // Get needed resource
   //
   Status = PeiServicesLocatePpi (
-             &gPeiSmmControlPpiGuid,
+             &gEfiPeiMmControlPpiGuid,
              0,
              NULL,
-             (VOID **)&SmmControl
+             (VOID **)&MmControl
              );
   if (EFI_ERROR (Status)) {
     return EFI_NOT_STARTED;
   }
 
   Status = PeiServicesLocatePpi (
-             &gPeiSmmAccessPpiGuid,
+             &gEfiPeiMmAccessPpiGuid,
              0,
              NULL,
-             (VOID **)&SmmAccess
+             (VOID **)&MmAccess
              );
   if (EFI_ERROR (Status)) {
     return EFI_NOT_STARTED;
@@ -327,8 +327,8 @@ Communicate (
   //
   // Check SMRAM locked, it should be done after SMRAM lock.
   //
-  if (!SmmAccess->LockState) {
-    DEBUG ((EFI_D_INFO, "PiSmmCommunicationPei LockState - %x\n", (UINTN)SmmAccess->LockState));
+  if (!MmAccess->LockState) {
+    DEBUG ((EFI_D_INFO, "PiSmmCommunicationPei LockState - %x\n", (UINTN)MmAccess->LockState));
     return EFI_NOT_STARTED;
   }
 
@@ -346,14 +346,14 @@ Communicate (
   //
   SmiCommand = (UINT8)SmmCommunicationContext->SwSmiNumber;
   Size = sizeof(SmiCommand);
-  Status = SmmControl->Trigger (
-                         (EFI_PEI_SERVICES **)GetPeiServicesTablePointer (),
-                         SmmControl,
-                         (INT8 *)&SmiCommand,
-                         &Size,
-                         FALSE,
-                         0
-                         );
+  Status = MmControl->Trigger (
+                        (EFI_PEI_SERVICES **)GetPeiServicesTablePointer (),
+                        MmControl,
+                        (INT8 *)&SmiCommand,
+                        &Size,
+                        FALSE,
+                        0
+                        );
   ASSERT_EFI_ERROR (Status);
 
   //
@@ -383,7 +383,7 @@ PiSmmCommunicationPeiEntryPoint (
   )
 {
   EFI_STATUS                      Status;
-  PEI_SMM_ACCESS_PPI              *SmmAccess;
+  EFI_PEI_MM_ACCESS_PPI           *MmAccess;
   EFI_BOOT_MODE                   BootMode;
   UINTN                           Index;
 
@@ -393,10 +393,10 @@ PiSmmCommunicationPeiEntryPoint (
   }
 
   Status = PeiServicesLocatePpi (
-             &gPeiSmmAccessPpiGuid,
+             &gEfiPeiMmAccessPpiGuid,
              0,
              NULL,
-             (VOID **)&SmmAccess
+             (VOID **)&MmAccess
              );
   if (EFI_ERROR (Status)) {
     return EFI_NOT_STARTED;
@@ -405,8 +405,8 @@ PiSmmCommunicationPeiEntryPoint (
   //
   // Check SMRAM locked, it should be done before SMRAM lock.
   //
-  if (SmmAccess->LockState) {
-    DEBUG ((EFI_D_INFO, "PiSmmCommunicationPei LockState - %x\n", (UINTN)SmmAccess->LockState));
+  if (MmAccess->LockState) {
+    DEBUG ((EFI_D_INFO, "PiSmmCommunicationPei LockState - %x\n", (UINTN)MmAccess->LockState));
     return EFI_ACCESS_DENIED;
   }
 
@@ -414,7 +414,7 @@ PiSmmCommunicationPeiEntryPoint (
   // Open all SMRAM
   //
   for (Index = 0; !EFI_ERROR (Status); Index++) {
-    Status = SmmAccess->Open ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), SmmAccess, Index);
+    Status = MmAccess->Open ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), MmAccess, Index);
   }
 
   InitCommunicationContext ();
diff --git a/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.c b/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.c
index 2b395f38da64..5f747f04a34d 100644
--- a/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.c
+++ b/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.c
@@ -23,7 +23,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/SmmMemLib.h>
 #include <Protocol/SmmSwDispatch2.h>
 #include <Protocol/SmmCommunication.h>
-#include <Ppi/SmmCommunication.h>
+#include <Ppi/MmCommunication.h>
 
 #include "PiSmmCommunicationPrivate.h"
 
@@ -43,7 +43,7 @@ SetCommunicationContext (
 
   Status = gSmst->SmmInstallConfigurationTable (
                     gSmst,
-                    &gEfiPeiSmmCommunicationPpiGuid,
+                    &gEfiPeiMmCommunicationPpiGuid,
                     &mSmmCommunicationContext,
                     sizeof(mSmmCommunicationContext)
                     );
diff --git a/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf b/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
index 5cb596c5644d..aff80009fcb7 100644
--- a/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
+++ b/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
@@ -54,16 +54,16 @@ [Guids]
 [Ppis]
   ## PRODUCES
   ## UNDEFINED # HOB # SMM Configuration Table
-  gEfiPeiSmmCommunicationPpiGuid
-  gPeiSmmAccessPpiGuid               ## CONSUMES
-  gPeiSmmControlPpiGuid              ## CONSUMES
+  gEfiPeiMmCommunicationPpiGuid
+  gEfiPeiMmAccessPpiGuid             ## CONSUMES
+  gEfiPeiMmControlPpiGuid            ## CONSUMES
 
 # [BootMode]
 #   S3_RESUME                        ## CONSUMES
 
 [Depex]
-  gPeiSmmAccessPpiGuid AND
-  gPeiSmmControlPpiGuid AND
+  gEfiPeiMmAccessPpiGuid AND
+  gEfiPeiMmControlPpiGuid AND
   gEfiPeiMasterBootModePpiGuid
 
 [UserExtensions.TianoCore."ExtraFiles"]
diff --git a/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf b/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
index 67799e9436cd..598cc878a7cb 100644
--- a/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
+++ b/UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
@@ -49,7 +49,7 @@ [LibraryClasses]
   SmmMemLib
 
 [Ppis]
-  gEfiPeiSmmCommunicationPpiGuid     ## UNDEFINED # SMM Configuration Table
+  gEfiPeiMmCommunicationPpiGuid      ## UNDEFINED # SMM Configuration Table
 
 [Protocols]
   gEfiSmmSwDispatch2ProtocolGuid     ## CONSUMES
-- 
2.18.0.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/8] UefiCpuPkg/S3Resume2Pei: Update MM PPI usages.
       [not found] <d625966ce7c0ac98b65f1e946b2cf833f6801706.1532395896.git.Marvin.Haeuser@outlook.com>
  2018-07-24  1:40 ` [PATCH 2/8] MdeModulePkg/SmmLockBoxPeiLib: Update MM PPI usages Marvin Häuser
  2018-07-24  1:40 ` [PATCH 3/8] UefiCpuPkg/PiSmmCommunicationPei: " Marvin Häuser
@ 2018-07-24  1:40 ` Marvin Häuser
  2018-07-24  1:40 ` [PATCH 5/8] QuarkSocPkg/SmmAccessPei: " Marvin Häuser
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Marvin Häuser @ 2018-07-24  1:40 UTC (permalink / raw)
  To: edk2-devel@lists.01.org
  Cc: michael.d.kinney@intel.com, liming.gao@intel.com,
	star.zeng@intel.com, eric.dong@intel.com, ruiyu.ni@intel.com,
	lersek@redhat.com, kelly.steele@intel.com,
	jordan.l.justen@intel.com, ard.biesheuvel@linaro.org

Update all references to the SMM PPIs from MdeModulePkg to rather use
MdePkg's MM PPI declarations.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
---
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c       | 40 ++++++++++----------
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf |  4 +-
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index 28e53ac5d334..8039e8cb200e 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -27,10 +27,10 @@
 #include <Guid/S3SmmInitDone.h>
 #include <Ppi/ReadOnlyVariable2.h>
 #include <Ppi/S3Resume2.h>
-#include <Ppi/SmmAccess.h>
+#include <Ppi/MmAccess.h>
 #include <Ppi/PostBootScriptTable.h>
 #include <Ppi/EndOfPeiPhase.h>
-#include <Ppi/SmmCommunication.h>
+#include <Ppi/MmCommunication.h>
 
 #include <Library/DebugLib.h>
 #include <Library/BaseLib.h>
@@ -340,7 +340,7 @@ SignalToSmmByCommunication (
   )
 {
   EFI_STATUS                         Status;
-  EFI_PEI_SMM_COMMUNICATION_PPI      *SmmCommunicationPpi;
+  EFI_PEI_MM_COMMUNICATION_PPI       *MmCommunicationPpi;
   UINTN                              CommSize;
   SMM_COMMUNICATE_HEADER_32          Header32;
   SMM_COMMUNICATE_HEADER_64          Header64;
@@ -366,23 +366,23 @@ SignalToSmmByCommunication (
   CopyGuid (CommBuffer, HandlerType);
 
   Status = PeiServicesLocatePpi (
-             &gEfiPeiSmmCommunicationPpiGuid,
+             &gEfiPeiMmCommunicationPpiGuid,
              0,
              NULL,
-             (VOID **)&SmmCommunicationPpi
+             (VOID **)&MmCommunicationPpi
              );
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "Locate Smm Communicate Ppi failed (%r)!\n", Status));
     return;
   }
 
-  Status = SmmCommunicationPpi->Communicate (
-                                  SmmCommunicationPpi,
-                                  (VOID *)CommBuffer,
-                                  &CommSize
-                                  );
+  Status = MmCommunicationPpi->Communicate (
+                                 MmCommunicationPpi,
+                                 (VOID *)CommBuffer,
+                                 &CommSize
+                                 );
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "SmmCommunicationPpi->Communicate return failure (%r)!\n", Status));
+    DEBUG ((DEBUG_ERROR, "MmCommunicationPpi->Communicate return failure (%r)!\n", Status));
   }
 
   DEBUG ((DEBUG_INFO, "Signal %g to SMM - Exit (%r)\n", HandlerType, Status));
@@ -749,7 +749,7 @@ S3ResumeExecuteBootScript (
   )
 {
   EFI_STATUS                 Status;
-  PEI_SMM_ACCESS_PPI         *SmmAccess;
+  EFI_PEI_MM_ACCESS_PPI      *MmAccess;
   UINTN                      Index;
   VOID                       *GuidHob;
   IA32_DESCRIPTOR            *IdtDescriptor;
@@ -778,22 +778,22 @@ S3ResumeExecuteBootScript (
     SendSmiIpi (GetApicId ());
 
     Status = PeiServicesLocatePpi (
-                              &gPeiSmmAccessPpiGuid,
+                              &gEfiPeiMmAccessPpiGuid,
                               0,
                               NULL,
-                              (VOID **) &SmmAccess
+                              (VOID **) &MmAccess
                               );
     if (!EFI_ERROR (Status)) {
       DEBUG ((DEBUG_INFO, "Close all SMRAM regions before executing boot script\n"));
 
       for (Index = 0, Status = EFI_SUCCESS; !EFI_ERROR (Status); Index++) {
-        Status = SmmAccess->Close ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), SmmAccess, Index);
+        Status = MmAccess->Close ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), MmAccess, Index);
       }
 
       DEBUG ((DEBUG_INFO, "Lock all SMRAM regions before executing boot script\n"));
 
       for (Index = 0, Status = EFI_SUCCESS; !EFI_ERROR (Status); Index++) {
-        Status = SmmAccess->Lock ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), SmmAccess, Index);
+        Status = MmAccess->Lock ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), MmAccess, Index);
       }
     }
 
@@ -954,7 +954,7 @@ S3RestoreConfig2 (
   )
 {
   EFI_STATUS                                    Status;
-  PEI_SMM_ACCESS_PPI                            *SmmAccess;
+  EFI_PEI_MM_ACCESS_PPI                         *MmAccess;
   UINTN                                         Index;
   ACPI_S3_CONTEXT                               *AcpiS3Context;
   EFI_PHYSICAL_ADDRESS                          TempEfiBootScriptExecutorVariable;
@@ -1048,13 +1048,13 @@ S3RestoreConfig2 (
   GuidHob = GetFirstGuidHob (&gEfiAcpiVariableGuid);
   if (GuidHob != NULL) {
     Status = PeiServicesLocatePpi (
-                              &gPeiSmmAccessPpiGuid,
+                              &gEfiPeiMmAccessPpiGuid,
                               0,
                               NULL,
-                              (VOID **) &SmmAccess
+                              (VOID **) &MmAccess
                               );
     for (Index = 0; !EFI_ERROR (Status); Index++) {
-      Status = SmmAccess->Open ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), SmmAccess, Index);
+      Status = MmAccess->Open ((EFI_PEI_SERVICES **)GetPeiServicesTablePointer (), MmAccess, Index);
     }
 
     SmramDescriptor = (EFI_SMRAM_DESCRIPTOR *) GET_GUID_HOB_DATA (GuidHob);
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
index 407aab67350b..e8ffaab7d789 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
@@ -81,10 +81,10 @@ [Guids]
 [Ppis]
   gEfiPeiReadOnlyVariable2PpiGuid               ## CONSUMES
   gEfiPeiS3Resume2PpiGuid                       ## PRODUCES
-  gPeiSmmAccessPpiGuid                          ## SOMETIMES_CONSUMES
+  gEfiPeiMmAccessPpiGuid                        ## SOMETIMES_CONSUMES
   gPeiPostScriptTablePpiGuid                    ## SOMETIMES_PRODUCES
   gEfiEndOfPeiSignalPpiGuid                     ## SOMETIMES_PRODUCES
-  gEfiPeiSmmCommunicationPpiGuid                ## SOMETIMES_CONSUMES
+  gEfiPeiMmCommunicationPpiGuid                 ## SOMETIMES_CONSUMES
 
 [FeaturePcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode         ## CONSUMES
-- 
2.18.0.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/8] QuarkSocPkg/SmmAccessPei: Update MM PPI usages.
       [not found] <d625966ce7c0ac98b65f1e946b2cf833f6801706.1532395896.git.Marvin.Haeuser@outlook.com>
                   ` (2 preceding siblings ...)
  2018-07-24  1:40 ` [PATCH 4/8] UefiCpuPkg/S3Resume2Pei: " Marvin Häuser
@ 2018-07-24  1:40 ` Marvin Häuser
  2018-07-24  1:40 ` [PATCH 6/8] QuarkSocPkg/SmmControlPei: " Marvin Häuser
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Marvin Häuser @ 2018-07-24  1:40 UTC (permalink / raw)
  To: edk2-devel@lists.01.org
  Cc: michael.d.kinney@intel.com, liming.gao@intel.com,
	star.zeng@intel.com, eric.dong@intel.com, ruiyu.ni@intel.com,
	lersek@redhat.com, kelly.steele@intel.com,
	jordan.l.justen@intel.com, ard.biesheuvel@linaro.org

Update all references to the SMM PPIs from MdeModulePkg to rather use
MdePkg's MM PPI declarations.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
---
 QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmAccessPei/SmmAccessPei.c   | 48 ++++++++++----------
 QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmAccessPei/SmmAccessPei.inf |  2 +-
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmAccessPei/SmmAccessPei.c b/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmAccessPei/SmmAccessPei.c
index 70fdf096117f..6686d87a3fda 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmAccessPei/SmmAccessPei.c
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmAccessPei/SmmAccessPei.c
@@ -14,7 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 **/
 #include <PiPei.h>
-#include <Ppi/SmmAccess.h>
+#include <Ppi/MmAccess.h>
 #include <Guid/SmramMemoryReserve.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/MemoryAllocationLib.h>
@@ -29,7 +29,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
   CR ( \
   a, \
   SMM_ACCESS_PRIVATE_DATA, \
-  SmmAccess, \
+  MmAccess, \
   SMM_ACCESS_PRIVATE_DATA_SIGNATURE \
   )
 
@@ -39,7 +39,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 typedef struct {
   UINTN                            Signature;
   EFI_HANDLE                       Handle;
-  PEI_SMM_ACCESS_PPI               SmmAccess;
+  EFI_PEI_MM_ACCESS_PPI            MmAccess;
   UINTN                            NumberRegions;
   EFI_SMRAM_DESCRIPTOR             SmramDesc[MAX_SMRAM_RANGES];
   UINT8                            TsegSize;
@@ -55,7 +55,7 @@ EFI_STATUS
 EFIAPI
 Open (
   IN EFI_PEI_SERVICES           **PeiServices,
-  IN PEI_SMM_ACCESS_PPI         *This,
+  IN EFI_PEI_MM_ACCESS_PPI      *This,
   IN UINTN                      DescriptorIndex
   )
 /*++
@@ -70,7 +70,7 @@ Routine Description:
 Arguments:
 
   PeiServices      - General purpose services available to every PEIM.
-  This             -  Pointer to the SMM Access Interface.
+  This             -  Pointer to the MM Access Interface.
   DescriptorIndex  -  Region of SMRAM to Open.
 
 Returns:
@@ -102,7 +102,7 @@ Returns:
 
   SmmAccess->SmramDesc[DescriptorIndex].RegionState &= ~(EFI_SMRAM_CLOSED | EFI_ALLOCATED);
   SmmAccess->SmramDesc[DescriptorIndex].RegionState |= EFI_SMRAM_OPEN;
-  SmmAccess->SmmAccess.OpenState = TRUE;
+  SmmAccess->MmAccess.OpenState = TRUE;
 
   return EFI_SUCCESS;
 }
@@ -111,7 +111,7 @@ EFI_STATUS
 EFIAPI
 Close (
   IN EFI_PEI_SERVICES        **PeiServices,
-  IN PEI_SMM_ACCESS_PPI      *This,
+  IN EFI_PEI_MM_ACCESS_PPI   *This,
   IN UINTN                   DescriptorIndex
   )
 /*++
@@ -124,7 +124,7 @@ Routine Description:
 Arguments:
 
   PeiServices      - General purpose services available to every PEIM.
-  This             -  Pointer to the SMM Access Interface.
+  This             -  Pointer to the MM Access Interface.
   DescriptorIndex  -  Region of SMRAM to Close.
 
 Returns:
@@ -174,7 +174,7 @@ Returns:
     }
   }
 
-  SmmAccess->SmmAccess.OpenState = OpenState;
+  SmmAccess->MmAccess.OpenState = OpenState;
 
   return EFI_SUCCESS;
 }
@@ -183,7 +183,7 @@ EFI_STATUS
 EFIAPI
 Lock (
   IN EFI_PEI_SERVICES          **PeiServices,
-  IN PEI_SMM_ACCESS_PPI        *This,
+  IN EFI_PEI_MM_ACCESS_PPI     *This,
   IN UINTN                     DescriptorIndex
   )
 /*++
@@ -198,7 +198,7 @@ Routine Description:
 Arguments:
 
   PeiServices      - General purpose services available to every PEIM.
-  This             -  Pointer to the SMM Access Interface.
+  This             -  Pointer to the MM Access Interface.
   DescriptorIndex  -  Region of SMRAM to Lock.
 
 Returns:
@@ -216,12 +216,12 @@ Returns:
 
   if (DescriptorIndex >= SmmAccess->NumberRegions) {
     return EFI_INVALID_PARAMETER;
-  } else if (SmmAccess->SmmAccess.OpenState) {
+  } else if (SmmAccess->MmAccess.OpenState) {
     return EFI_DEVICE_ERROR;
   }
 
   SmmAccess->SmramDesc[DescriptorIndex].RegionState |= EFI_SMRAM_LOCKED;
-  SmmAccess->SmmAccess.LockState                     = TRUE;
+  SmmAccess->MmAccess.LockState                     = TRUE;
 
   //
   // Lock TSEG
@@ -235,7 +235,7 @@ EFI_STATUS
 EFIAPI
 GetCapabilities (
   IN EFI_PEI_SERVICES                **PeiServices,
-  IN PEI_SMM_ACCESS_PPI              *This,
+  IN EFI_PEI_MM_ACCESS_PPI           *This,
   IN OUT UINTN                       *SmramMapSize,
   IN OUT EFI_SMRAM_DESCRIPTOR        *SmramMap
   )
@@ -293,7 +293,7 @@ SmmAccessPeiEntryPoint (
 
 Routine Description:
 
-    This is the constructor for the SMM Access Ppi
+    This is the constructor for the MM Access Ppi
 
 Arguments:
 
@@ -354,17 +354,17 @@ Returns:
     SmmAccessPrivate->SmramDesc[Index].RegionState   = DescriptorBlock->Descriptor[Index].RegionState;
   }
 
-  SmmAccessPrivate->NumberRegions              = Index;
-  SmmAccessPrivate->SmmAccess.Open             = Open;
-  SmmAccessPrivate->SmmAccess.Close            = Close;
-  SmmAccessPrivate->SmmAccess.Lock             = Lock;
-  SmmAccessPrivate->SmmAccess.GetCapabilities  = GetCapabilities;
-  SmmAccessPrivate->SmmAccess.LockState        = FALSE;
-  SmmAccessPrivate->SmmAccess.OpenState        = FALSE;
+  SmmAccessPrivate->NumberRegions             = Index;
+  SmmAccessPrivate->MmAccess.Open             = Open;
+  SmmAccessPrivate->MmAccess.Close            = Close;
+  SmmAccessPrivate->MmAccess.Lock             = Lock;
+  SmmAccessPrivate->MmAccess.GetCapabilities  = GetCapabilities;
+  SmmAccessPrivate->MmAccess.LockState        = FALSE;
+  SmmAccessPrivate->MmAccess.OpenState        = FALSE;
 
   PpiList->Flags = (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST);
-  PpiList->Guid  = &gPeiSmmAccessPpiGuid;
-  PpiList->Ppi   = &SmmAccessPrivate->SmmAccess;
+  PpiList->Guid  = &gEfiPeiMmAccessPpiGuid;
+  PpiList->Ppi   = &SmmAccessPrivate->MmAccess;
 
   Status      = (**PeiServices).InstallPpi (PeiServices, PpiList);
   ASSERT_EFI_ERROR(Status);
diff --git a/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmAccessPei/SmmAccessPei.inf b/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmAccessPei/SmmAccessPei.inf
index a1e4af7725ac..41566d5ecc2b 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmAccessPei/SmmAccessPei.inf
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmAccessPei/SmmAccessPei.inf
@@ -44,7 +44,7 @@ [Guids]
   gEfiSmmPeiSmramMemoryReserveGuid             # ALWAYS_CONSUMED
 
 [Ppis]
-  gPeiSmmAccessPpiGuid                         # ALWAYS_PRODUCED
+  gEfiPeiMmAccessPpiGuid                       # ALWAYS_PRODUCED
   gEfiPeiMemoryDiscoveredPpiGuid               # ALWAYS_CONSUMED
 
 [Depex]
-- 
2.18.0.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/8] QuarkSocPkg/SmmControlPei: Update MM PPI usages.
       [not found] <d625966ce7c0ac98b65f1e946b2cf833f6801706.1532395896.git.Marvin.Haeuser@outlook.com>
                   ` (3 preceding siblings ...)
  2018-07-24  1:40 ` [PATCH 5/8] QuarkSocPkg/SmmAccessPei: " Marvin Häuser
@ 2018-07-24  1:40 ` Marvin Häuser
  2018-07-24  1:40 ` [PATCH 7/8] OvmfPkg/SmmAccessPei: " Marvin Häuser
  2018-07-24  1:40 ` [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs Marvin Häuser
  6 siblings, 0 replies; 13+ messages in thread
From: Marvin Häuser @ 2018-07-24  1:40 UTC (permalink / raw)
  To: edk2-devel@lists.01.org
  Cc: michael.d.kinney@intel.com, liming.gao@intel.com,
	star.zeng@intel.com, eric.dong@intel.com, ruiyu.ni@intel.com,
	lersek@redhat.com, kelly.steele@intel.com,
	jordan.l.justen@intel.com, ard.biesheuvel@linaro.org

Update all references to the SMM PPIs from MdeModulePkg to rather use
MdePkg's MM PPI declarations.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
---
 QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmControlPei/SmmControlPei.c   | 24 ++++++++++----------
 QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmControlPei/SmmControlPei.inf |  2 +-
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmControlPei/SmmControlPei.c b/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmControlPei/SmmControlPei.c
index 4d19c8f89b0b..98f83d2ce82b 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmControlPei/SmmControlPei.c
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmControlPei/SmmControlPei.c
@@ -16,7 +16,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #include <PiPei.h>
 
-#include <Ppi/SmmControl.h>
+#include <Ppi/MmControl.h>
 
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
@@ -34,7 +34,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
   @param  PeiServices         Describes the list of possible PEI Services.
   @param  This                A pointer to an instance of
-                              EFI_SMM_CONTROL_PPI
+                              EFI_PEI_MM_CONTROL_PPI
   @param  ArgumentBuffer      The argument buffer
   @param  ArgumentBufferSize  The size of the argument buffer
   @param  Periodic            TRUE to indicate a periodical SMI
@@ -48,7 +48,7 @@ EFI_STATUS
 EFIAPI
 PeiActivate (
   IN      EFI_PEI_SERVICES           **PeiServices,
-  IN      PEI_SMM_CONTROL_PPI        *This,
+  IN      EFI_PEI_MM_CONTROL_PPI     *This,
   IN OUT  INT8                       *ArgumentBuffer OPTIONAL,
   IN OUT  UINTN                      *ArgumentBufferSize OPTIONAL,
   IN      BOOLEAN                    Periodic OPTIONAL,
@@ -59,7 +59,7 @@ PeiActivate (
   Clears an SMI.
 
   @param  PeiServices         Describes the list of possible PEI Services.
-  @param  This                Pointer to an instance of EFI_SMM_CONTROL_PPI
+  @param  This                Pointer to an instance of EFI_PEI_MM_CONTROL_PPI
   @param  Periodic            TRUE to indicate a periodical SMI
 
   @return Return value from SmmClear()
@@ -69,18 +69,18 @@ EFI_STATUS
 EFIAPI
 PeiDeactivate (
   IN  EFI_PEI_SERVICES            **PeiServices,
-  IN  PEI_SMM_CONTROL_PPI         *This,
+  IN  EFI_PEI_MM_CONTROL_PPI      *This,
   IN  BOOLEAN                     Periodic OPTIONAL
   );
 
-PEI_SMM_CONTROL_PPI      mSmmControlPpi = {
+EFI_PEI_MM_CONTROL_PPI      mSmmControlPpi = {
   PeiActivate,
   PeiDeactivate
 };
 
 EFI_PEI_PPI_DESCRIPTOR   mPpiList = {
   (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
-  &gPeiSmmControlPpiGuid,
+  &gEfiPeiMmControlPpiGuid,
   &mSmmControlPpi
 };
 
@@ -177,7 +177,7 @@ Returns:
 
   @param  PeiServices         Describes the list of possible PEI Services.
   @param  This                A pointer to an instance of
-                              EFI_SMM_CONTROL_PPI
+                              EFI_PEI_MM_CONTROL_PPI
   @param  ArgumentBuffer      The argument buffer
   @param  ArgumentBufferSize  The size of the argument buffer
   @param  Periodic            TRUE to indicate a periodical SMI
@@ -191,7 +191,7 @@ EFI_STATUS
 EFIAPI
 PeiActivate (
   IN      EFI_PEI_SERVICES           **PeiServices,
-  IN      PEI_SMM_CONTROL_PPI        *This,
+  IN      EFI_PEI_MM_CONTROL_PPI     *This,
   IN OUT  INT8                       *ArgumentBuffer OPTIONAL,
   IN OUT  UINTN                      *ArgumentBufferSize OPTIONAL,
   IN      BOOLEAN                    Periodic OPTIONAL,
@@ -232,7 +232,7 @@ PeiActivate (
   Clears an SMI.
 
   @param  PeiServices         Describes the list of possible PEI Services.
-  @param  This                Pointer to an instance of EFI_SMM_CONTROL_PPI
+  @param  This                Pointer to an instance of EFI_PEI_MM_CONTROL_PPI
   @param  Periodic            TRUE to indicate a periodical SMI
 
   @return Return value from SmmClear()
@@ -242,7 +242,7 @@ EFI_STATUS
 EFIAPI
 PeiDeactivate (
   IN  EFI_PEI_SERVICES            **PeiServices,
-  IN  PEI_SMM_CONTROL_PPI         *This,
+  IN  EFI_PEI_MM_CONTROL_PPI      *This,
   IN  BOOLEAN                     Periodic OPTIONAL
   )
 {
@@ -255,7 +255,7 @@ PeiDeactivate (
 /**
   This is the constructor for the SMM Control Ppi.
 
-  This function installs EFI_SMM_CONTROL_PPI.
+  This function installs EFI_PEI_MM_CONTROL_PPI.
 
   @param   FileHandle       Handle of the file being invoked.
   @param   PeiServices      Describes the list of possible PEI Services.
diff --git a/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmControlPei/SmmControlPei.inf b/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmControlPei/SmmControlPei.inf
index 6b1dd1bcfd1c..0a98e5739416 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmControlPei/SmmControlPei.inf
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmControlPei/SmmControlPei.inf
@@ -46,7 +46,7 @@ [LibraryClasses]
   QNCAccessLib
 
 [Ppis]
-  gPeiSmmControlPpiGuid                        # ALWAYS_PRODUCED
+  gEfiPeiMmControlPpiGuid                      # ALWAYS_PRODUCED
 
 [Pcd]
   gEfiQuarkNcSocIdTokenSpaceGuid.PcdPm1blkIoBaseAddress
-- 
2.18.0.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 7/8] OvmfPkg/SmmAccessPei: Update MM PPI usages.
       [not found] <d625966ce7c0ac98b65f1e946b2cf833f6801706.1532395896.git.Marvin.Haeuser@outlook.com>
                   ` (4 preceding siblings ...)
  2018-07-24  1:40 ` [PATCH 6/8] QuarkSocPkg/SmmControlPei: " Marvin Häuser
@ 2018-07-24  1:40 ` Marvin Häuser
  2018-07-24  9:20   ` Laszlo Ersek
  2018-07-24  1:40 ` [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs Marvin Häuser
  6 siblings, 1 reply; 13+ messages in thread
From: Marvin Häuser @ 2018-07-24  1:40 UTC (permalink / raw)
  To: edk2-devel@lists.01.org
  Cc: michael.d.kinney@intel.com, liming.gao@intel.com,
	star.zeng@intel.com, eric.dong@intel.com, ruiyu.ni@intel.com,
	lersek@redhat.com, kelly.steele@intel.com,
	jordan.l.justen@intel.com, ard.biesheuvel@linaro.org

Update all references to the SMM PPIs from MdeModulePkg to rather use
MdePkg's MM PPI declarations.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
---
 OvmfPkg/SmmAccess/SmmAccessPei.c   | 90 ++++++++++----------
 OvmfPkg/SmmAccess/SmramInternal.c  |  8 +-
 OvmfPkg/SmmAccess/SmmAccessPei.inf |  2 +-
 3 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.c b/OvmfPkg/SmmAccess/SmmAccessPei.c
index 21119f80eefa..340122d6a598 100644
--- a/OvmfPkg/SmmAccess/SmmAccessPei.c
+++ b/OvmfPkg/SmmAccess/SmmAccessPei.c
@@ -3,7 +3,7 @@
   A PEIM with the following responsibilities:
 
   - verify & configure the Q35 TSEG in the entry point,
-  - provide SMRAM access by producing PEI_SMM_ACCESS_PPI,
+  - provide MMRAM access by producing EFI_PEI_MM_ACCESS_PPI,
   - set aside the SMM_S3_RESUME_STATE object at the bottom of TSEG, and expose
     it via the gEfiAcpiVariableGuid GUID HOB.
 
@@ -32,28 +32,28 @@
 #include <Library/PcdLib.h>
 #include <Library/PciLib.h>
 #include <Library/PeiServicesLib.h>
-#include <Ppi/SmmAccess.h>
+#include <Ppi/MmAccess.h>
 
 #include <OvmfPlatforms.h>
 
 #include "SmramInternal.h"
 
 //
-// PEI_SMM_ACCESS_PPI implementation.
+// EFI_PEI_MM_ACCESS_PPI implementation.
 //
 
 /**
-  Opens the SMRAM area to be accessible by a PEIM driver.
+  Opens the MMRAM area to be accessible by a PEIM driver.
 
-  This function "opens" SMRAM so that it is visible while not inside of SMM.
+  This function "opens" MMRAM so that it is visible while not inside of MM.
   The function should return EFI_UNSUPPORTED if the hardware does not support
-  hiding of SMRAM. The function should return EFI_DEVICE_ERROR if the SMRAM
+  hiding of MMRAM. The function should return EFI_DEVICE_ERROR if the MMRAM
   configuration is locked.
 
   @param  PeiServices            General purpose services available to every
                                  PEIM.
-  @param  This                   The pointer to the SMM Access Interface.
-  @param  DescriptorIndex        The region of SMRAM to Open.
+  @param  This                   The pointer to the MM Access Interface.
+  @param  DescriptorIndex        The region of MMRAM to Open.
 
   @retval EFI_SUCCESS            The region was successfully opened.
   @retval EFI_DEVICE_ERROR       The region could not be opened because locked
@@ -64,9 +64,9 @@
 STATIC
 EFI_STATUS
 EFIAPI
-SmmAccessPeiOpen (
+MmAccessPeiOpen (
   IN EFI_PEI_SERVICES                **PeiServices,
-  IN PEI_SMM_ACCESS_PPI              *This,
+  IN EFI_PEI_MM_ACCESS_PPI           *This,
   IN UINTN                           DescriptorIndex
   )
 {
@@ -82,16 +82,16 @@ SmmAccessPeiOpen (
 }
 
 /**
-  Inhibits access to the SMRAM.
+  Inhibits access to the MMRAM.
 
-  This function "closes" SMRAM so that it is not visible while outside of SMM.
+  This function "closes" MMRAM so that it is not visible while outside of MM.
   The function should return EFI_UNSUPPORTED if the hardware does not support
-  hiding of SMRAM.
+  hiding of MMRAM.
 
   @param  PeiServices              General purpose services available to every
                                    PEIM.
-  @param  This                     The pointer to the SMM Access Interface.
-  @param  DescriptorIndex          The region of SMRAM to Close.
+  @param  This                     The pointer to the MM Access Interface.
+  @param  DescriptorIndex          The region of MMRAM to Close.
 
   @retval EFI_SUCCESS              The region was successfully closed.
   @retval EFI_DEVICE_ERROR         The region could not be closed because
@@ -102,9 +102,9 @@ SmmAccessPeiOpen (
 STATIC
 EFI_STATUS
 EFIAPI
-SmmAccessPeiClose (
+MmAccessPeiClose (
   IN EFI_PEI_SERVICES                **PeiServices,
-  IN PEI_SMM_ACCESS_PPI              *This,
+  IN EFI_PEI_MM_ACCESS_PPI           *This,
   IN UINTN                           DescriptorIndex
   )
 {
@@ -120,15 +120,15 @@ SmmAccessPeiClose (
 }
 
 /**
-  Inhibits access to the SMRAM.
+  Inhibits access to the MMRAM.
 
-  This function prohibits access to the SMRAM region.  This function is usually
+  This function prohibits access to the MMRAM region.  This function is usually
   implemented such that it is a write-once operation.
 
   @param  PeiServices              General purpose services available to every
                                    PEIM.
-  @param  This                     The pointer to the SMM Access Interface.
-  @param  DescriptorIndex          The region of SMRAM to Close.
+  @param  This                     The pointer to the MM Access Interface.
+  @param  DescriptorIndex          The region of MMRAM to Close.
 
   @retval EFI_SUCCESS            The region was successfully locked.
   @retval EFI_DEVICE_ERROR       The region could not be locked because at
@@ -139,9 +139,9 @@ SmmAccessPeiClose (
 STATIC
 EFI_STATUS
 EFIAPI
-SmmAccessPeiLock (
+MmAccessPeiLock (
   IN EFI_PEI_SERVICES                **PeiServices,
-  IN PEI_SMM_ACCESS_PPI              *This,
+  IN EFI_PEI_MM_ACCESS_PPI           *This,
   IN UINTN                           DescriptorIndex
   )
 {
@@ -158,11 +158,11 @@ SmmAccessPeiLock (
 
 /**
   Queries the memory controller for the possible regions that will support
-  SMRAM.
+  MMRAM.
 
   @param  PeiServices           General purpose services available to every
                                 PEIM.
-  @param This                   The pointer to the SmmAccessPpi Interface.
+  @param This                   The pointer to the MmAccessPpi Interface.
   @param SmramMapSize           The pointer to the variable containing size of
                                 the buffer to contain the description
                                 information.
@@ -176,11 +176,11 @@ SmmAccessPeiLock (
 STATIC
 EFI_STATUS
 EFIAPI
-SmmAccessPeiGetCapabilities (
+MmAccessPeiGetCapabilities (
   IN EFI_PEI_SERVICES                **PeiServices,
-  IN PEI_SMM_ACCESS_PPI              *This,
+  IN EFI_PEI_MM_ACCESS_PPI           *This,
   IN OUT UINTN                       *SmramMapSize,
-  IN OUT EFI_SMRAM_DESCRIPTOR        *SmramMap
+  IN OUT EFI_MMRAM_DESCRIPTOR        *SmramMap
   )
 {
   return SmramAccessGetCapabilities (This->LockState, This->OpenState,
@@ -190,18 +190,18 @@ SmmAccessPeiGetCapabilities (
 //
 // LockState and OpenState will be filled in by the entry point.
 //
-STATIC PEI_SMM_ACCESS_PPI mAccess = {
-  &SmmAccessPeiOpen,
-  &SmmAccessPeiClose,
-  &SmmAccessPeiLock,
-  &SmmAccessPeiGetCapabilities
+STATIC EFI_PEI_MM_ACCESS_PPI mAccess = {
+  &MmAccessPeiOpen,
+  &MmAccessPeiClose,
+  &MmAccessPeiLock,
+  &MmAccessPeiGetCapabilities
 };
 
 
 STATIC EFI_PEI_PPI_DESCRIPTOR mPpiList[] = {
   {
     EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
-    &gPeiSmmAccessPpiGuid, &mAccess
+    &gEfiPeiMmAccessPpiGuid, &mAccess
   }
 };
 
@@ -255,7 +255,7 @@ SmmAccessPeiEntryPoint (
   VOID                 *GuidHob;
 
   //
-  // This module should only be included if SMRAM support is required.
+  // This module should only be included if MMRAM support is required.
   //
   ASSERT (FeaturePcdGet (PcdSmmSmramRequire));
 
@@ -264,14 +264,14 @@ SmmAccessPeiEntryPoint (
   //
   HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
   if (HostBridgeDevId != INTEL_Q35_MCH_DEVICE_ID) {
-    DEBUG ((EFI_D_ERROR, "%a: no SMRAM with host bridge DID=0x%04x; only "
+    DEBUG ((EFI_D_ERROR, "%a: no MMRAM with host bridge DID=0x%04x; only "
       "DID=0x%04x (Q35) is supported\n", __FUNCTION__, HostBridgeDevId,
       INTEL_Q35_MCH_DEVICE_ID));
     goto WrongConfig;
   }
 
   //
-  // Confirm if QEMU supports SMRAM.
+  // Confirm if QEMU supports MMRAM.
   //
   // With no support for it, the ESMRAMC (Extended System Management RAM
   // Control) register reads as zero. If there is support, the cache-enable
@@ -280,7 +280,7 @@ SmmAccessPeiEntryPoint (
   EsmramcVal = PciRead8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC));
   RegMask8 = MCH_ESMRAMC_SM_CACHE | MCH_ESMRAMC_SM_L1 | MCH_ESMRAMC_SM_L2;
   if ((EsmramcVal & RegMask8) != RegMask8) {
-    DEBUG ((EFI_D_ERROR, "%a: this Q35 implementation lacks SMRAM\n",
+    DEBUG ((EFI_D_ERROR, "%a: this Q35 implementation lacks MMRAM\n",
       __FUNCTION__));
     goto WrongConfig;
   }
@@ -323,27 +323,27 @@ SmmAccessPeiEntryPoint (
     (TopOfLowRamMb - mQ35TsegMbytes) << MCH_TSEGMB_MB_SHIFT);
 
   //
-  // Set TSEG size, and disable TSEG visibility outside of SMM. Note that the
+  // Set TSEG size, and disable TSEG visibility outside of MM. Note that the
   // T_EN bit has inverse meaning; when T_EN is set, then TSEG visibility is
-  // *restricted* to SMM.
+  // *restricted* to MM.
   //
   EsmramcVal &= ~(UINT32)MCH_ESMRAMC_TSEG_MASK;
   EsmramcVal |= mQ35TsegMbytes == 8 ? MCH_ESMRAMC_TSEG_8MB :
                 mQ35TsegMbytes == 2 ? MCH_ESMRAMC_TSEG_2MB :
                 mQ35TsegMbytes == 1 ? MCH_ESMRAMC_TSEG_1MB :
-                MCH_ESMRAMC_TSEG_EXT;
-  EsmramcVal |= MCH_ESMRAMC_T_EN;
+                MCH_EMMRAMC_TSEG_EXT;
+  EsmramcVal |= MCH_EMMRAMC_T_EN;
   PciWrite8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC), EsmramcVal);
 
   //
   // TSEG should be closed (see above), but unlocked, initially. Set G_SMRAME
-  // (Global SMRAM Enable) too, as both D_LCK and T_EN depend on it.
+  // (Global MMRAM Enable) too, as both D_LCK and T_EN depend on it.
   //
   PciAndThenOr8 (DRAMC_REGISTER_Q35 (MCH_SMRAM),
     (UINT8)((~(UINT32)MCH_SMRAM_D_LCK) & 0xff), MCH_SMRAM_G_SMRAME);
 
   //
-  // Create the GUID HOB and point it to the first SMRAM range.
+  // Create the GUID HOB and point it to the first MMRAM range.
   //
   GetStates (&mAccess.LockState, &mAccess.OpenState);
   SmramMapSize = sizeof SmramMap;
@@ -357,7 +357,7 @@ SmmAccessPeiEntryPoint (
     UINTN Idx;
 
     Count = SmramMapSize / sizeof SmramMap[0];
-    DEBUG ((EFI_D_VERBOSE, "%a: SMRAM map follows, %d entries\n", __FUNCTION__,
+    DEBUG ((EFI_D_VERBOSE, "%a: MMRAM map follows, %d entries\n", __FUNCTION__,
       (INT32)Count));
     DEBUG ((EFI_D_VERBOSE, "% 20a % 20a % 20a % 20a\n", "PhysicalStart(0x)",
       "PhysicalSize(0x)", "CpuStart(0x)", "RegionState(0x)"));
diff --git a/OvmfPkg/SmmAccess/SmramInternal.c b/OvmfPkg/SmmAccess/SmramInternal.c
index 18c42d29042d..3b33d2763b4d 100644
--- a/OvmfPkg/SmmAccess/SmramInternal.c
+++ b/OvmfPkg/SmmAccess/SmramInternal.c
@@ -40,10 +40,10 @@ InitQ35TsegMbytes (
 
 /**
   Read the MCH_SMRAM and ESMRAMC registers, and update the LockState and
-  OpenState fields in the PEI_SMM_ACCESS_PPI / EFI_SMM_ACCESS2_PROTOCOL object,
+  OpenState fields in the EFI_PEI_MM_ACCESS_PPI / EFI_SMM_ACCESS2_PROTOCOL object,
   from the D_LCK and T_EN bits.
 
-  PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member functions can rely on
+  EFI_PEI_MM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member functions can rely on
   the LockState and OpenState fields being up-to-date on entry, and they need
   to restore the same invariant on exit, if they touch the bits in question.
 
@@ -68,13 +68,13 @@ GetStates (
 }
 
 //
-// The functions below follow the PEI_SMM_ACCESS_PPI and
+// The functions below follow the EFI_PEI_MM_ACCESS_PPI and
 // EFI_SMM_ACCESS2_PROTOCOL member declarations. The PeiServices and This
 // pointers are removed (TSEG doesn't depend on them), and so is the
 // DescriptorIndex parameter (TSEG doesn't support range-wise locking).
 //
 // The LockState and OpenState members that are common to both
-// PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are taken and updated in
+// EFI_PEI_MM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are taken and updated in
 // isolation from the rest of the (non-shared) members.
 //
 
diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.inf b/OvmfPkg/SmmAccess/SmmAccessPei.inf
index 09f3b63446df..7360b2f5391e 100644
--- a/OvmfPkg/SmmAccess/SmmAccessPei.inf
+++ b/OvmfPkg/SmmAccess/SmmAccessPei.inf
@@ -63,7 +63,7 @@ [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
 
 [Ppis]
-  gPeiSmmAccessPpiGuid           ## PRODUCES
+  gEfiPeiMmAccessPpiGuid         ## PRODUCES
 
 [Depex]
   gEfiPeiMemoryDiscoveredPpiGuid
-- 
2.18.0.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.
       [not found] <d625966ce7c0ac98b65f1e946b2cf833f6801706.1532395896.git.Marvin.Haeuser@outlook.com>
                   ` (5 preceding siblings ...)
  2018-07-24  1:40 ` [PATCH 7/8] OvmfPkg/SmmAccessPei: " Marvin Häuser
@ 2018-07-24  1:40 ` Marvin Häuser
  2018-07-24  6:38   ` Gao, Liming
  6 siblings, 1 reply; 13+ messages in thread
From: Marvin Häuser @ 2018-07-24  1:40 UTC (permalink / raw)
  To: edk2-devel@lists.01.org
  Cc: michael.d.kinney@intel.com, liming.gao@intel.com,
	star.zeng@intel.com, eric.dong@intel.com, ruiyu.ni@intel.com,
	lersek@redhat.com, kelly.steele@intel.com,
	jordan.l.justen@intel.com, ard.biesheuvel@linaro.org

MdeModulePkg's Smm* PPIs have been deprecated entirely by UEFI PI
PPIs. Do not allow their usage when DISABLE_NEW_DEPRECATED_INTERFACES
is defined.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
---
 MdeModulePkg/Include/Ppi/SmmAccess.h        | 4 ++++
 MdeModulePkg/Include/Ppi/SmmCommunication.h | 4 ++++
 MdeModulePkg/Include/Ppi/SmmControl.h       | 4 ++++
 3 files changed, 12 insertions(+)

diff --git a/MdeModulePkg/Include/Ppi/SmmAccess.h b/MdeModulePkg/Include/Ppi/SmmAccess.h
index 795c8815a9c1..0ca6d0e3af22 100644
--- a/MdeModulePkg/Include/Ppi/SmmAccess.h
+++ b/MdeModulePkg/Include/Ppi/SmmAccess.h
@@ -24,6 +24,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 **/
 
+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
+
 #ifndef _SMM_ACCESS_PPI_H_
 #define _SMM_ACCESS_PPI_H_
 
@@ -144,3 +146,5 @@ struct _PEI_SMM_ACCESS_PPI {
 extern EFI_GUID gPeiSmmAccessPpiGuid;
 
 #endif
+
+#endif
diff --git a/MdeModulePkg/Include/Ppi/SmmCommunication.h b/MdeModulePkg/Include/Ppi/SmmCommunication.h
index 8ac86a443a15..7f24d851ae09 100644
--- a/MdeModulePkg/Include/Ppi/SmmCommunication.h
+++ b/MdeModulePkg/Include/Ppi/SmmCommunication.h
@@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 **/
 
+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
+
 
 #ifndef _SMM_COMMUNICATION_PPI_H_
 #define _SMM_COMMUNICATION_PPI_H_
@@ -62,3 +64,5 @@ struct _EFI_PEI_SMM_COMMUNICATION_PPI {
 extern EFI_GUID gEfiPeiSmmCommunicationPpiGuid;
 
 #endif
+
+#endif
diff --git a/MdeModulePkg/Include/Ppi/SmmControl.h b/MdeModulePkg/Include/Ppi/SmmControl.h
index 0c62196fb44c..597a6db07f2c 100644
--- a/MdeModulePkg/Include/Ppi/SmmControl.h
+++ b/MdeModulePkg/Include/Ppi/SmmControl.h
@@ -22,6 +22,8 @@
 
 **/
 
+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
+
 
 #ifndef _SMM_CONTROL_PPI_H_
 #define _SMM_CONTROL_PPI_H_
@@ -94,3 +96,5 @@ struct _PEI_SMM_CONTROL_PPI {
 extern EFI_GUID gPeiSmmControlPpiGuid;
 
 #endif
+
+#endif
-- 
2.18.0.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.
  2018-07-24  1:40 ` [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs Marvin Häuser
@ 2018-07-24  6:38   ` Gao, Liming
  2018-07-24 10:04     ` Zeng, Star
  2018-07-24 10:28     ` Laszlo Ersek
  0 siblings, 2 replies; 13+ messages in thread
From: Gao, Liming @ 2018-07-24  6:38 UTC (permalink / raw)
  To: Marvin.Haeuser@outlook.com, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Zeng, Star, Dong, Eric, Ni, Ruiyu,
	lersek@redhat.com, Steele, Kelly, Justen, Jordan L,
	ard.biesheuvel@linaro.org

To keep compatibility, I suggest to update MdeModulePkg PPI definition to include MdePkg one, then typedef structure name and define macro name for SMM one. MdePkg SMM protocol uses this way to refer to MM protocol. 

Thanks
Liming
>-----Original Message-----
>From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
>Sent: Tuesday, July 24, 2018 9:41 AM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
><liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>; Dong, Eric
><eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; lersek@redhat.com;
>Steele, Kelly <kelly.steele@intel.com>; Justen, Jordan L
><jordan.l.justen@intel.com>; ard.biesheuvel@linaro.org
>Subject: [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.
>
>MdeModulePkg's Smm* PPIs have been deprecated entirely by UEFI PI
>PPIs. Do not allow their usage when
>DISABLE_NEW_DEPRECATED_INTERFACES
>is defined.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
>---
> MdeModulePkg/Include/Ppi/SmmAccess.h        | 4 ++++
> MdeModulePkg/Include/Ppi/SmmCommunication.h | 4 ++++
> MdeModulePkg/Include/Ppi/SmmControl.h       | 4 ++++
> 3 files changed, 12 insertions(+)
>
>diff --git a/MdeModulePkg/Include/Ppi/SmmAccess.h
>b/MdeModulePkg/Include/Ppi/SmmAccess.h
>index 795c8815a9c1..0ca6d0e3af22 100644
>--- a/MdeModulePkg/Include/Ppi/SmmAccess.h
>+++ b/MdeModulePkg/Include/Ppi/SmmAccess.h
>@@ -24,6 +24,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>KIND, EITHER EXPRESS OR IMPLIED.
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
> #ifndef _SMM_ACCESS_PPI_H_
> #define _SMM_ACCESS_PPI_H_
>
>@@ -144,3 +146,5 @@ struct _PEI_SMM_ACCESS_PPI {
> extern EFI_GUID gPeiSmmAccessPpiGuid;
>
> #endif
>+
>+#endif
>diff --git a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>index 8ac86a443a15..7f24d851ae09 100644
>--- a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>+++ b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>@@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>KIND, EITHER EXPRESS OR IMPLIED.
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
>
> #ifndef _SMM_COMMUNICATION_PPI_H_
> #define _SMM_COMMUNICATION_PPI_H_
>@@ -62,3 +64,5 @@ struct _EFI_PEI_SMM_COMMUNICATION_PPI {
> extern EFI_GUID gEfiPeiSmmCommunicationPpiGuid;
>
> #endif
>+
>+#endif
>diff --git a/MdeModulePkg/Include/Ppi/SmmControl.h
>b/MdeModulePkg/Include/Ppi/SmmControl.h
>index 0c62196fb44c..597a6db07f2c 100644
>--- a/MdeModulePkg/Include/Ppi/SmmControl.h
>+++ b/MdeModulePkg/Include/Ppi/SmmControl.h
>@@ -22,6 +22,8 @@
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
>
> #ifndef _SMM_CONTROL_PPI_H_
> #define _SMM_CONTROL_PPI_H_
>@@ -94,3 +96,5 @@ struct _PEI_SMM_CONTROL_PPI {
> extern EFI_GUID gPeiSmmControlPpiGuid;
>
> #endif
>+
>+#endif
>--
>2.18.0.windows.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 7/8] OvmfPkg/SmmAccessPei: Update MM PPI usages.
  2018-07-24  1:40 ` [PATCH 7/8] OvmfPkg/SmmAccessPei: " Marvin Häuser
@ 2018-07-24  9:20   ` Laszlo Ersek
  2018-07-24 12:16     ` Marvin Häuser
  0 siblings, 1 reply; 13+ messages in thread
From: Laszlo Ersek @ 2018-07-24  9:20 UTC (permalink / raw)
  To: Marvin Häuser, edk2-devel@lists.01.org
  Cc: michael.d.kinney@intel.com, liming.gao@intel.com,
	star.zeng@intel.com, eric.dong@intel.com, ruiyu.ni@intel.com,
	kelly.steele@intel.com, jordan.l.justen@intel.com,
	ard.biesheuvel@linaro.org

Hi Marvin,

On 07/24/18 03:40, Marvin Häuser wrote:
> Update all references to the SMM PPIs from MdeModulePkg to rather use
> MdePkg's MM PPI declarations.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
> ---
>  OvmfPkg/SmmAccess/SmmAccessPei.c   | 90 ++++++++++----------
>  OvmfPkg/SmmAccess/SmramInternal.c  |  8 +-
>  OvmfPkg/SmmAccess/SmmAccessPei.inf |  2 +-
>  3 files changed, 50 insertions(+), 50 deletions(-)

two comments:

(1) Please post patch sets using a cover letter message:

git config format.coverletter     true
git config format.numbered        true
git config sendemail.chainreplyto false
git config sendemail.thread       true

Otherwise the individual patches in the series will get intermixed with
other messages / threads on the mailing list (and in people's inboxes),
if the recipient uses a threaded view in their MUA (which most people
should do -- patches and discussions are very hard to follow otherwise).

(2) More specifically, I disagree with this patch. I don't see what's
wrong with calling SMM "SMM" on x86 (or with calling SMI "SMI"). I
understand that the PI spec now calls those things MM and MMI, and I
tend to agree that new code (especially ARM/AARCH64 code) should use
these more generic terms from the most recent PI spec releases. But,
this is not new code, and it is for x86; I don't see why we should
forcefully drag it forward. I don't think the edk2 headers plan to
remove the original / traditional names.

(Just the other day, I learned that EFI_FILE_HANDLE was basically a
typedef for "pointer to EFI_FILE_PROTOCOL" -- it's a non-standard type,
yet it's defined in "MdePkg/Include/Protocol/SimpleFileSystem.h", and
it's used in e.g.
- MdeModulePkg/Library/BootMaintenanceManagerUiLib
- MdeModulePkg/Library/FileExplorerLib
- MdeModulePkg/Universal/Disk/RamDiskDxe
- MdeModulePkg/Universal/EbcDxe
- MdePkg/Library/DxeServicesLib
- MdePkg/Library/UefiFileHandleLib
- NetworkPkg/TlsAuthConfigDxe
- SecurityPkg/VariableAuthenticated/SecureBootConfigDxe

Should we use EFI_FILE_HANDLE in new code we write? No, we shouldn't.
Should we embark on a journey to eradicate EFI_FILE_HANDLE? I would
disagree with that idea too.)

Obviously I'm not arguing against migrating other packages to the new
names, if their respective maintainers like the idea. I don't think it's
useful for OvmfPkg though.

Thanks
Laszlo

> diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.c b/OvmfPkg/SmmAccess/SmmAccessPei.c
> index 21119f80eefa..340122d6a598 100644
> --- a/OvmfPkg/SmmAccess/SmmAccessPei.c
> +++ b/OvmfPkg/SmmAccess/SmmAccessPei.c
> @@ -3,7 +3,7 @@
>    A PEIM with the following responsibilities:
>  
>    - verify & configure the Q35 TSEG in the entry point,
> -  - provide SMRAM access by producing PEI_SMM_ACCESS_PPI,
> +  - provide MMRAM access by producing EFI_PEI_MM_ACCESS_PPI,
>    - set aside the SMM_S3_RESUME_STATE object at the bottom of TSEG, and expose
>      it via the gEfiAcpiVariableGuid GUID HOB.
>  
> @@ -32,28 +32,28 @@
>  #include <Library/PcdLib.h>
>  #include <Library/PciLib.h>
>  #include <Library/PeiServicesLib.h>
> -#include <Ppi/SmmAccess.h>
> +#include <Ppi/MmAccess.h>
>  
>  #include <OvmfPlatforms.h>
>  
>  #include "SmramInternal.h"
>  
>  //
> -// PEI_SMM_ACCESS_PPI implementation.
> +// EFI_PEI_MM_ACCESS_PPI implementation.
>  //
>  
>  /**
> -  Opens the SMRAM area to be accessible by a PEIM driver.
> +  Opens the MMRAM area to be accessible by a PEIM driver.
>  
> -  This function "opens" SMRAM so that it is visible while not inside of SMM.
> +  This function "opens" MMRAM so that it is visible while not inside of MM.
>    The function should return EFI_UNSUPPORTED if the hardware does not support
> -  hiding of SMRAM. The function should return EFI_DEVICE_ERROR if the SMRAM
> +  hiding of MMRAM. The function should return EFI_DEVICE_ERROR if the MMRAM
>    configuration is locked.
>  
>    @param  PeiServices            General purpose services available to every
>                                   PEIM.
> -  @param  This                   The pointer to the SMM Access Interface.
> -  @param  DescriptorIndex        The region of SMRAM to Open.
> +  @param  This                   The pointer to the MM Access Interface.
> +  @param  DescriptorIndex        The region of MMRAM to Open.
>  
>    @retval EFI_SUCCESS            The region was successfully opened.
>    @retval EFI_DEVICE_ERROR       The region could not be opened because locked
> @@ -64,9 +64,9 @@
>  STATIC
>  EFI_STATUS
>  EFIAPI
> -SmmAccessPeiOpen (
> +MmAccessPeiOpen (
>    IN EFI_PEI_SERVICES                **PeiServices,
> -  IN PEI_SMM_ACCESS_PPI              *This,
> +  IN EFI_PEI_MM_ACCESS_PPI           *This,
>    IN UINTN                           DescriptorIndex
>    )
>  {
> @@ -82,16 +82,16 @@ SmmAccessPeiOpen (
>  }
>  
>  /**
> -  Inhibits access to the SMRAM.
> +  Inhibits access to the MMRAM.
>  
> -  This function "closes" SMRAM so that it is not visible while outside of SMM.
> +  This function "closes" MMRAM so that it is not visible while outside of MM.
>    The function should return EFI_UNSUPPORTED if the hardware does not support
> -  hiding of SMRAM.
> +  hiding of MMRAM.
>  
>    @param  PeiServices              General purpose services available to every
>                                     PEIM.
> -  @param  This                     The pointer to the SMM Access Interface.
> -  @param  DescriptorIndex          The region of SMRAM to Close.
> +  @param  This                     The pointer to the MM Access Interface.
> +  @param  DescriptorIndex          The region of MMRAM to Close.
>  
>    @retval EFI_SUCCESS              The region was successfully closed.
>    @retval EFI_DEVICE_ERROR         The region could not be closed because
> @@ -102,9 +102,9 @@ SmmAccessPeiOpen (
>  STATIC
>  EFI_STATUS
>  EFIAPI
> -SmmAccessPeiClose (
> +MmAccessPeiClose (
>    IN EFI_PEI_SERVICES                **PeiServices,
> -  IN PEI_SMM_ACCESS_PPI              *This,
> +  IN EFI_PEI_MM_ACCESS_PPI           *This,
>    IN UINTN                           DescriptorIndex
>    )
>  {
> @@ -120,15 +120,15 @@ SmmAccessPeiClose (
>  }
>  
>  /**
> -  Inhibits access to the SMRAM.
> +  Inhibits access to the MMRAM.
>  
> -  This function prohibits access to the SMRAM region.  This function is usually
> +  This function prohibits access to the MMRAM region.  This function is usually
>    implemented such that it is a write-once operation.
>  
>    @param  PeiServices              General purpose services available to every
>                                     PEIM.
> -  @param  This                     The pointer to the SMM Access Interface.
> -  @param  DescriptorIndex          The region of SMRAM to Close.
> +  @param  This                     The pointer to the MM Access Interface.
> +  @param  DescriptorIndex          The region of MMRAM to Close.
>  
>    @retval EFI_SUCCESS            The region was successfully locked.
>    @retval EFI_DEVICE_ERROR       The region could not be locked because at
> @@ -139,9 +139,9 @@ SmmAccessPeiClose (
>  STATIC
>  EFI_STATUS
>  EFIAPI
> -SmmAccessPeiLock (
> +MmAccessPeiLock (
>    IN EFI_PEI_SERVICES                **PeiServices,
> -  IN PEI_SMM_ACCESS_PPI              *This,
> +  IN EFI_PEI_MM_ACCESS_PPI           *This,
>    IN UINTN                           DescriptorIndex
>    )
>  {
> @@ -158,11 +158,11 @@ SmmAccessPeiLock (
>  
>  /**
>    Queries the memory controller for the possible regions that will support
> -  SMRAM.
> +  MMRAM.
>  
>    @param  PeiServices           General purpose services available to every
>                                  PEIM.
> -  @param This                   The pointer to the SmmAccessPpi Interface.
> +  @param This                   The pointer to the MmAccessPpi Interface.
>    @param SmramMapSize           The pointer to the variable containing size of
>                                  the buffer to contain the description
>                                  information.
> @@ -176,11 +176,11 @@ SmmAccessPeiLock (
>  STATIC
>  EFI_STATUS
>  EFIAPI
> -SmmAccessPeiGetCapabilities (
> +MmAccessPeiGetCapabilities (
>    IN EFI_PEI_SERVICES                **PeiServices,
> -  IN PEI_SMM_ACCESS_PPI              *This,
> +  IN EFI_PEI_MM_ACCESS_PPI           *This,
>    IN OUT UINTN                       *SmramMapSize,
> -  IN OUT EFI_SMRAM_DESCRIPTOR        *SmramMap
> +  IN OUT EFI_MMRAM_DESCRIPTOR        *SmramMap
>    )
>  {
>    return SmramAccessGetCapabilities (This->LockState, This->OpenState,
> @@ -190,18 +190,18 @@ SmmAccessPeiGetCapabilities (
>  //
>  // LockState and OpenState will be filled in by the entry point.
>  //
> -STATIC PEI_SMM_ACCESS_PPI mAccess = {
> -  &SmmAccessPeiOpen,
> -  &SmmAccessPeiClose,
> -  &SmmAccessPeiLock,
> -  &SmmAccessPeiGetCapabilities
> +STATIC EFI_PEI_MM_ACCESS_PPI mAccess = {
> +  &MmAccessPeiOpen,
> +  &MmAccessPeiClose,
> +  &MmAccessPeiLock,
> +  &MmAccessPeiGetCapabilities
>  };
>  
>  
>  STATIC EFI_PEI_PPI_DESCRIPTOR mPpiList[] = {
>    {
>      EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
> -    &gPeiSmmAccessPpiGuid, &mAccess
> +    &gEfiPeiMmAccessPpiGuid, &mAccess
>    }
>  };
>  
> @@ -255,7 +255,7 @@ SmmAccessPeiEntryPoint (
>    VOID                 *GuidHob;
>  
>    //
> -  // This module should only be included if SMRAM support is required.
> +  // This module should only be included if MMRAM support is required.
>    //
>    ASSERT (FeaturePcdGet (PcdSmmSmramRequire));
>  
> @@ -264,14 +264,14 @@ SmmAccessPeiEntryPoint (
>    //
>    HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
>    if (HostBridgeDevId != INTEL_Q35_MCH_DEVICE_ID) {
> -    DEBUG ((EFI_D_ERROR, "%a: no SMRAM with host bridge DID=0x%04x; only "
> +    DEBUG ((EFI_D_ERROR, "%a: no MMRAM with host bridge DID=0x%04x; only "
>        "DID=0x%04x (Q35) is supported\n", __FUNCTION__, HostBridgeDevId,
>        INTEL_Q35_MCH_DEVICE_ID));
>      goto WrongConfig;
>    }
>  
>    //
> -  // Confirm if QEMU supports SMRAM.
> +  // Confirm if QEMU supports MMRAM.
>    //
>    // With no support for it, the ESMRAMC (Extended System Management RAM
>    // Control) register reads as zero. If there is support, the cache-enable
> @@ -280,7 +280,7 @@ SmmAccessPeiEntryPoint (
>    EsmramcVal = PciRead8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC));
>    RegMask8 = MCH_ESMRAMC_SM_CACHE | MCH_ESMRAMC_SM_L1 | MCH_ESMRAMC_SM_L2;
>    if ((EsmramcVal & RegMask8) != RegMask8) {
> -    DEBUG ((EFI_D_ERROR, "%a: this Q35 implementation lacks SMRAM\n",
> +    DEBUG ((EFI_D_ERROR, "%a: this Q35 implementation lacks MMRAM\n",
>        __FUNCTION__));
>      goto WrongConfig;
>    }
> @@ -323,27 +323,27 @@ SmmAccessPeiEntryPoint (
>      (TopOfLowRamMb - mQ35TsegMbytes) << MCH_TSEGMB_MB_SHIFT);
>  
>    //
> -  // Set TSEG size, and disable TSEG visibility outside of SMM. Note that the
> +  // Set TSEG size, and disable TSEG visibility outside of MM. Note that the
>    // T_EN bit has inverse meaning; when T_EN is set, then TSEG visibility is
> -  // *restricted* to SMM.
> +  // *restricted* to MM.
>    //
>    EsmramcVal &= ~(UINT32)MCH_ESMRAMC_TSEG_MASK;
>    EsmramcVal |= mQ35TsegMbytes == 8 ? MCH_ESMRAMC_TSEG_8MB :
>                  mQ35TsegMbytes == 2 ? MCH_ESMRAMC_TSEG_2MB :
>                  mQ35TsegMbytes == 1 ? MCH_ESMRAMC_TSEG_1MB :
> -                MCH_ESMRAMC_TSEG_EXT;
> -  EsmramcVal |= MCH_ESMRAMC_T_EN;
> +                MCH_EMMRAMC_TSEG_EXT;
> +  EsmramcVal |= MCH_EMMRAMC_T_EN;
>    PciWrite8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC), EsmramcVal);
>  
>    //
>    // TSEG should be closed (see above), but unlocked, initially. Set G_SMRAME
> -  // (Global SMRAM Enable) too, as both D_LCK and T_EN depend on it.
> +  // (Global MMRAM Enable) too, as both D_LCK and T_EN depend on it.
>    //
>    PciAndThenOr8 (DRAMC_REGISTER_Q35 (MCH_SMRAM),
>      (UINT8)((~(UINT32)MCH_SMRAM_D_LCK) & 0xff), MCH_SMRAM_G_SMRAME);
>  
>    //
> -  // Create the GUID HOB and point it to the first SMRAM range.
> +  // Create the GUID HOB and point it to the first MMRAM range.
>    //
>    GetStates (&mAccess.LockState, &mAccess.OpenState);
>    SmramMapSize = sizeof SmramMap;
> @@ -357,7 +357,7 @@ SmmAccessPeiEntryPoint (
>      UINTN Idx;
>  
>      Count = SmramMapSize / sizeof SmramMap[0];
> -    DEBUG ((EFI_D_VERBOSE, "%a: SMRAM map follows, %d entries\n", __FUNCTION__,
> +    DEBUG ((EFI_D_VERBOSE, "%a: MMRAM map follows, %d entries\n", __FUNCTION__,
>        (INT32)Count));
>      DEBUG ((EFI_D_VERBOSE, "% 20a % 20a % 20a % 20a\n", "PhysicalStart(0x)",
>        "PhysicalSize(0x)", "CpuStart(0x)", "RegionState(0x)"));
> diff --git a/OvmfPkg/SmmAccess/SmramInternal.c b/OvmfPkg/SmmAccess/SmramInternal.c
> index 18c42d29042d..3b33d2763b4d 100644
> --- a/OvmfPkg/SmmAccess/SmramInternal.c
> +++ b/OvmfPkg/SmmAccess/SmramInternal.c
> @@ -40,10 +40,10 @@ InitQ35TsegMbytes (
>  
>  /**
>    Read the MCH_SMRAM and ESMRAMC registers, and update the LockState and
> -  OpenState fields in the PEI_SMM_ACCESS_PPI / EFI_SMM_ACCESS2_PROTOCOL object,
> +  OpenState fields in the EFI_PEI_MM_ACCESS_PPI / EFI_SMM_ACCESS2_PROTOCOL object,
>    from the D_LCK and T_EN bits.
>  
> -  PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member functions can rely on
> +  EFI_PEI_MM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member functions can rely on
>    the LockState and OpenState fields being up-to-date on entry, and they need
>    to restore the same invariant on exit, if they touch the bits in question.
>  
> @@ -68,13 +68,13 @@ GetStates (
>  }
>  
>  //
> -// The functions below follow the PEI_SMM_ACCESS_PPI and
> +// The functions below follow the EFI_PEI_MM_ACCESS_PPI and
>  // EFI_SMM_ACCESS2_PROTOCOL member declarations. The PeiServices and This
>  // pointers are removed (TSEG doesn't depend on them), and so is the
>  // DescriptorIndex parameter (TSEG doesn't support range-wise locking).
>  //
>  // The LockState and OpenState members that are common to both
> -// PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are taken and updated in
> +// EFI_PEI_MM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are taken and updated in
>  // isolation from the rest of the (non-shared) members.
>  //
>  
> diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.inf b/OvmfPkg/SmmAccess/SmmAccessPei.inf
> index 09f3b63446df..7360b2f5391e 100644
> --- a/OvmfPkg/SmmAccess/SmmAccessPei.inf
> +++ b/OvmfPkg/SmmAccess/SmmAccessPei.inf
> @@ -63,7 +63,7 @@ [Pcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
>  
>  [Ppis]
> -  gPeiSmmAccessPpiGuid           ## PRODUCES
> +  gEfiPeiMmAccessPpiGuid         ## PRODUCES
>  
>  [Depex]
>    gEfiPeiMemoryDiscoveredPpiGuid
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.
  2018-07-24  6:38   ` Gao, Liming
@ 2018-07-24 10:04     ` Zeng, Star
  2018-07-24 10:28     ` Laszlo Ersek
  1 sibling, 0 replies; 13+ messages in thread
From: Zeng, Star @ 2018-07-24 10:04 UTC (permalink / raw)
  To: Gao, Liming, Marvin.Haeuser@outlook.com, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Dong, Eric, Ni, Ruiyu, lersek@redhat.com,
	Steele, Kelly, Justen, Jordan L, ard.biesheuvel@linaro.org,
	Zeng, Star

I do not think we prefer to deprecate the definitions in MdeModulePkg. The definitions are also referred by platforms.
And I do not think we prefer to update the consumer code at this moment. The SMM driver refers the SMM_ prefix definitions, that definitely makes sense.

I agree Liming's suggestion.


Thanks,
Star
-----Original Message-----
From: Gao, Liming 
Sent: Tuesday, July 24, 2018 2:39 PM
To: Marvin.Haeuser@outlook.com; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; lersek@redhat.com; Steele, Kelly <kelly.steele@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; ard.biesheuvel@linaro.org
Subject: RE: [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.

To keep compatibility, I suggest to update MdeModulePkg PPI definition to include MdePkg one, then typedef structure name and define macro name for SMM one. MdePkg SMM protocol uses this way to refer to MM protocol. 

Thanks
Liming
>-----Original Message-----
>From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
>Sent: Tuesday, July 24, 2018 9:41 AM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming 
><liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>; Dong, Eric 
><eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; 
>lersek@redhat.com; Steele, Kelly <kelly.steele@intel.com>; Justen, 
>Jordan L <jordan.l.justen@intel.com>; ard.biesheuvel@linaro.org
>Subject: [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.
>
>MdeModulePkg's Smm* PPIs have been deprecated entirely by UEFI PI PPIs. 
>Do not allow their usage when DISABLE_NEW_DEPRECATED_INTERFACES is 
>defined.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
>---
> MdeModulePkg/Include/Ppi/SmmAccess.h        | 4 ++++
> MdeModulePkg/Include/Ppi/SmmCommunication.h | 4 ++++
> MdeModulePkg/Include/Ppi/SmmControl.h       | 4 ++++
> 3 files changed, 12 insertions(+)
>
>diff --git a/MdeModulePkg/Include/Ppi/SmmAccess.h
>b/MdeModulePkg/Include/Ppi/SmmAccess.h
>index 795c8815a9c1..0ca6d0e3af22 100644
>--- a/MdeModulePkg/Include/Ppi/SmmAccess.h
>+++ b/MdeModulePkg/Include/Ppi/SmmAccess.h
>@@ -24,6 +24,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, 
>EITHER EXPRESS OR IMPLIED.
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
> #ifndef _SMM_ACCESS_PPI_H_
> #define _SMM_ACCESS_PPI_H_
>
>@@ -144,3 +146,5 @@ struct _PEI_SMM_ACCESS_PPI {  extern EFI_GUID 
>gPeiSmmAccessPpiGuid;
>
> #endif
>+
>+#endif
>diff --git a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>index 8ac86a443a15..7f24d851ae09 100644
>--- a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>+++ b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>@@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, 
>EITHER EXPRESS OR IMPLIED.
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
>
> #ifndef _SMM_COMMUNICATION_PPI_H_
> #define _SMM_COMMUNICATION_PPI_H_
>@@ -62,3 +64,5 @@ struct _EFI_PEI_SMM_COMMUNICATION_PPI {  extern 
>EFI_GUID gEfiPeiSmmCommunicationPpiGuid;
>
> #endif
>+
>+#endif
>diff --git a/MdeModulePkg/Include/Ppi/SmmControl.h
>b/MdeModulePkg/Include/Ppi/SmmControl.h
>index 0c62196fb44c..597a6db07f2c 100644
>--- a/MdeModulePkg/Include/Ppi/SmmControl.h
>+++ b/MdeModulePkg/Include/Ppi/SmmControl.h
>@@ -22,6 +22,8 @@
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
>
> #ifndef _SMM_CONTROL_PPI_H_
> #define _SMM_CONTROL_PPI_H_
>@@ -94,3 +96,5 @@ struct _PEI_SMM_CONTROL_PPI {  extern EFI_GUID 
>gPeiSmmControlPpiGuid;
>
> #endif
>+
>+#endif
>--
>2.18.0.windows.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.
  2018-07-24  6:38   ` Gao, Liming
  2018-07-24 10:04     ` Zeng, Star
@ 2018-07-24 10:28     ` Laszlo Ersek
  1 sibling, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-07-24 10:28 UTC (permalink / raw)
  To: Gao, Liming, Marvin.Haeuser@outlook.com, edk2-devel@lists.01.org
  Cc: Ni, Ruiyu, Dong, Eric, Justen, Jordan L, Kinney, Michael D,
	Zeng, Star

On 07/24/18 08:38, Gao, Liming wrote:
> To keep compatibility, I suggest to update MdeModulePkg PPI definition to include MdePkg one, then typedef structure name and define macro name for SMM one. MdePkg SMM protocol uses this way to refer to MM protocol. 

I agree this is a better approach than using
DISABLE_NEW_DEPRECATED_INTERFACES.

That macro is a heavy hammer. I recall using it twice in the past:
- once for the sake of the PCD APIs that end with "S" (i.e., for
deprecating those PCD APIs that didn't return a status),
- and another time for the sake of the string APIs that similary end
with "S" (i.e., for deprecating "unsafe" string functions).

In both of those cases, it had been perfectly possible to write
safe/secure code using the "old" (to-be-deprecated) interfaces, such as:

- a platform might basically never use NVRAM-backed (= HII) dynamic
PCDs, just in-memory dynamic PCDs, hence the set operations were always
expected to succeed -- indeed the conversion to the new interfaces
mostly invovled receiving the status codes and heaping
ASSERT_EFI_ERROR() macro invocations on top,

- a programmer should always track the size of the character arrays
anyway -- sometimes this implies memory functions should be used rather
than string functions altogether, and sometimes this implies that the
carefully called string function can never overflow.

However, the safety / security benefit of deprecating those two sets of
APIs was undeniable on a larger scale; there *were* a good number of
dubious string manipulations that had been caught and fixed through the
deprecation, in practice. (Sometimes the fix would be a quite elaborate
reworking of the code -- I seem to recall some examples in ARM-related
packages that Ard patched and others and I reviewed.)

I see no such safety / security benefit in deprecating the names "SMM"
and "SMI" in existent x86 code.


Another thing I didn't spell out under the OvmfPkg patch: it would not
be just about type names and comments, but variable names too. For
example, consider:

 STATIC
 EFI_STATUS
 EFIAPI
-SmmAccessPeiGetCapabilities (
+MmAccessPeiGetCapabilities (
   IN EFI_PEI_SERVICES                **PeiServices,
-  IN PEI_SMM_ACCESS_PPI              *This,
+  IN EFI_PEI_MM_ACCESS_PPI           *This,
   IN OUT UINTN                       *SmramMapSize,
-  IN OUT EFI_SMRAM_DESCRIPTOR        *SmramMap
+  IN OUT EFI_MMRAM_DESCRIPTOR        *SmramMap
   )

For complete coverage / consistency, we'd have to rename "SmramMapSize"
to "MmramMapSize", and "SmramMap" to "MmramMap". That would introduce a
*lot* more code changes. As I stated above, I don't see the benefit at all.

Thanks
Laszlo

> 
> Thanks
> Liming
>> -----Original Message-----
>> From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
>> Sent: Tuesday, July 24, 2018 9:41 AM
>> To: edk2-devel@lists.01.org
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>> <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>; Dong, Eric
>> <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; lersek@redhat.com;
>> Steele, Kelly <kelly.steele@intel.com>; Justen, Jordan L
>> <jordan.l.justen@intel.com>; ard.biesheuvel@linaro.org
>> Subject: [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.
>>
>> MdeModulePkg's Smm* PPIs have been deprecated entirely by UEFI PI
>> PPIs. Do not allow their usage when
>> DISABLE_NEW_DEPRECATED_INTERFACES
>> is defined.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
>> ---
>> MdeModulePkg/Include/Ppi/SmmAccess.h        | 4 ++++
>> MdeModulePkg/Include/Ppi/SmmCommunication.h | 4 ++++
>> MdeModulePkg/Include/Ppi/SmmControl.h       | 4 ++++
>> 3 files changed, 12 insertions(+)
>>
>> diff --git a/MdeModulePkg/Include/Ppi/SmmAccess.h
>> b/MdeModulePkg/Include/Ppi/SmmAccess.h
>> index 795c8815a9c1..0ca6d0e3af22 100644
>> --- a/MdeModulePkg/Include/Ppi/SmmAccess.h
>> +++ b/MdeModulePkg/Include/Ppi/SmmAccess.h
>> @@ -24,6 +24,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>> KIND, EITHER EXPRESS OR IMPLIED.
>>
>> **/
>>
>> +#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>> +
>> #ifndef _SMM_ACCESS_PPI_H_
>> #define _SMM_ACCESS_PPI_H_
>>
>> @@ -144,3 +146,5 @@ struct _PEI_SMM_ACCESS_PPI {
>> extern EFI_GUID gPeiSmmAccessPpiGuid;
>>
>> #endif
>> +
>> +#endif
>> diff --git a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>> b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>> index 8ac86a443a15..7f24d851ae09 100644
>> --- a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>> +++ b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>> @@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>> KIND, EITHER EXPRESS OR IMPLIED.
>>
>> **/
>>
>> +#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>> +
>>
>> #ifndef _SMM_COMMUNICATION_PPI_H_
>> #define _SMM_COMMUNICATION_PPI_H_
>> @@ -62,3 +64,5 @@ struct _EFI_PEI_SMM_COMMUNICATION_PPI {
>> extern EFI_GUID gEfiPeiSmmCommunicationPpiGuid;
>>
>> #endif
>> +
>> +#endif
>> diff --git a/MdeModulePkg/Include/Ppi/SmmControl.h
>> b/MdeModulePkg/Include/Ppi/SmmControl.h
>> index 0c62196fb44c..597a6db07f2c 100644
>> --- a/MdeModulePkg/Include/Ppi/SmmControl.h
>> +++ b/MdeModulePkg/Include/Ppi/SmmControl.h
>> @@ -22,6 +22,8 @@
>>
>> **/
>>
>> +#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>> +
>>
>> #ifndef _SMM_CONTROL_PPI_H_
>> #define _SMM_CONTROL_PPI_H_
>> @@ -94,3 +96,5 @@ struct _PEI_SMM_CONTROL_PPI {
>> extern EFI_GUID gPeiSmmControlPpiGuid;
>>
>> #endif
>> +
>> +#endif
>> --
>> 2.18.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 7/8] OvmfPkg/SmmAccessPei: Update MM PPI usages.
  2018-07-24  9:20   ` Laszlo Ersek
@ 2018-07-24 12:16     ` Marvin Häuser
  2018-07-24 12:39       ` Laszlo Ersek
  0 siblings, 1 reply; 13+ messages in thread
From: Marvin Häuser @ 2018-07-24 12:16 UTC (permalink / raw)
  To: edk2-devel@lists.01.org, Laszlo Ersek

Hey Laszlo,

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, July 24, 2018 11:20 AM
> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
> devel@lists.01.org
> Cc: michael.d.kinney@intel.com; liming.gao@intel.com;
> star.zeng@intel.com; eric.dong@intel.com; ruiyu.ni@intel.com;
> kelly.steele@intel.com; jordan.l.justen@intel.com;
> ard.biesheuvel@linaro.org
> Subject: Re: [PATCH 7/8] OvmfPkg/SmmAccessPei: Update MM PPI usages.
> 
> Hi Marvin,
> 
> On 07/24/18 03:40, Marvin Häuser wrote:
> > Update all references to the SMM PPIs from MdeModulePkg to rather use
> > MdePkg's MM PPI declarations.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
> > ---
> >  OvmfPkg/SmmAccess/SmmAccessPei.c   | 90 ++++++++++----------
> >  OvmfPkg/SmmAccess/SmramInternal.c  |  8 +-
> > OvmfPkg/SmmAccess/SmmAccessPei.inf |  2 +-
> >  3 files changed, 50 insertions(+), 50 deletions(-)
> 
> two comments:
> 
> (1) Please post patch sets using a cover letter message:
> 
> git config format.coverletter     true
> git config format.numbered        true
> git config sendemail.chainreplyto false
> git config sendemail.thread       true
> 
> Otherwise the individual patches in the series will get intermixed with other
> messages / threads on the mailing list (and in people's inboxes), if the
> recipient uses a threaded view in their MUA (which most people should do --
> patches and discussions are very hard to follow otherwise).

Is V2 fine in this aspect? Outlook has never threaded multiple parts of a patch series for me but only replies to their original message, hence I did not notice something was off.

> 
> (2) More specifically, I disagree with this patch. I don't see what's wrong with
> calling SMM "SMM" on x86 (or with calling SMI "SMI"). I understand that the
> PI spec now calls those things MM and MMI, and I tend to agree that new
> code (especially ARM/AARCH64 code) should use these more generic terms
> from the most recent PI spec releases. But, this is not new code, and it is for
> x86; I don't see why we should forcefully drag it forward. I don't think the
> edk2 headers plan to remove the original / traditional names.
> 
> (Just the other day, I learned that EFI_FILE_HANDLE was basically a typedef
> for "pointer to EFI_FILE_PROTOCOL" -- it's a non-standard type, yet it's
> defined in "MdePkg/Include/Protocol/SimpleFileSystem.h", and it's used in
> e.g.
> - MdeModulePkg/Library/BootMaintenanceManagerUiLib
> - MdeModulePkg/Library/FileExplorerLib
> - MdeModulePkg/Universal/Disk/RamDiskDxe
> - MdeModulePkg/Universal/EbcDxe
> - MdePkg/Library/DxeServicesLib
> - MdePkg/Library/UefiFileHandleLib
> - NetworkPkg/TlsAuthConfigDxe
> - SecurityPkg/VariableAuthenticated/SecureBootConfigDxe
> 
> Should we use EFI_FILE_HANDLE in new code we write? No, we shouldn't.
> Should we embark on a journey to eradicate EFI_FILE_HANDLE? I would
> disagree with that idea too.)
> 
> Obviously I'm not arguing against migrating other packages to the new
> names, if their respective maintainers like the idea. I don't think it's useful for
> OvmfPkg though.
> 
> Thanks
> Laszlo
> 
> > diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.c
> > b/OvmfPkg/SmmAccess/SmmAccessPei.c
> > index 21119f80eefa..340122d6a598 100644
> > --- a/OvmfPkg/SmmAccess/SmmAccessPei.c
> > +++ b/OvmfPkg/SmmAccess/SmmAccessPei.c
> > @@ -3,7 +3,7 @@
> >    A PEIM with the following responsibilities:
> >
> >    - verify & configure the Q35 TSEG in the entry point,
> > -  - provide SMRAM access by producing PEI_SMM_ACCESS_PPI,
> > +  - provide MMRAM access by producing EFI_PEI_MM_ACCESS_PPI,
> >    - set aside the SMM_S3_RESUME_STATE object at the bottom of TSEG,
> and expose
> >      it via the gEfiAcpiVariableGuid GUID HOB.
> >
> > @@ -32,28 +32,28 @@
> >  #include <Library/PcdLib.h>
> >  #include <Library/PciLib.h>
> >  #include <Library/PeiServicesLib.h>
> > -#include <Ppi/SmmAccess.h>
> > +#include <Ppi/MmAccess.h>
> >
> >  #include <OvmfPlatforms.h>
> >
> >  #include "SmramInternal.h"
> >
> >  //
> > -// PEI_SMM_ACCESS_PPI implementation.
> > +// EFI_PEI_MM_ACCESS_PPI implementation.
> >  //
> >
> >  /**
> > -  Opens the SMRAM area to be accessible by a PEIM driver.
> > +  Opens the MMRAM area to be accessible by a PEIM driver.
> >
> > -  This function "opens" SMRAM so that it is visible while not inside of SMM.
> > +  This function "opens" MMRAM so that it is visible while not inside of
> MM.
> >    The function should return EFI_UNSUPPORTED if the hardware does not
> > support
> > -  hiding of SMRAM. The function should return EFI_DEVICE_ERROR if the
> > SMRAM
> > +  hiding of MMRAM. The function should return EFI_DEVICE_ERROR if the
> > + MMRAM
> >    configuration is locked.
> >
> >    @param  PeiServices            General purpose services available to every
> >                                   PEIM.
> > -  @param  This                   The pointer to the SMM Access Interface.
> > -  @param  DescriptorIndex        The region of SMRAM to Open.
> > +  @param  This                   The pointer to the MM Access Interface.
> > +  @param  DescriptorIndex        The region of MMRAM to Open.
> >
> >    @retval EFI_SUCCESS            The region was successfully opened.
> >    @retval EFI_DEVICE_ERROR       The region could not be opened because
> locked
> > @@ -64,9 +64,9 @@
> >  STATIC
> >  EFI_STATUS
> >  EFIAPI
> > -SmmAccessPeiOpen (
> > +MmAccessPeiOpen (
> >    IN EFI_PEI_SERVICES                **PeiServices,
> > -  IN PEI_SMM_ACCESS_PPI              *This,
> > +  IN EFI_PEI_MM_ACCESS_PPI           *This,
> >    IN UINTN                           DescriptorIndex
> >    )
> >  {
> > @@ -82,16 +82,16 @@ SmmAccessPeiOpen (  }
> >
> >  /**
> > -  Inhibits access to the SMRAM.
> > +  Inhibits access to the MMRAM.
> >
> > -  This function "closes" SMRAM so that it is not visible while outside of
> SMM.
> > +  This function "closes" MMRAM so that it is not visible while outside of
> MM.
> >    The function should return EFI_UNSUPPORTED if the hardware does not
> > support
> > -  hiding of SMRAM.
> > +  hiding of MMRAM.
> >
> >    @param  PeiServices              General purpose services available to every
> >                                     PEIM.
> > -  @param  This                     The pointer to the SMM Access Interface.
> > -  @param  DescriptorIndex          The region of SMRAM to Close.
> > +  @param  This                     The pointer to the MM Access Interface.
> > +  @param  DescriptorIndex          The region of MMRAM to Close.
> >
> >    @retval EFI_SUCCESS              The region was successfully closed.
> >    @retval EFI_DEVICE_ERROR         The region could not be closed because
> > @@ -102,9 +102,9 @@ SmmAccessPeiOpen (  STATIC  EFI_STATUS  EFIAPI
> > -SmmAccessPeiClose (
> > +MmAccessPeiClose (
> >    IN EFI_PEI_SERVICES                **PeiServices,
> > -  IN PEI_SMM_ACCESS_PPI              *This,
> > +  IN EFI_PEI_MM_ACCESS_PPI           *This,
> >    IN UINTN                           DescriptorIndex
> >    )
> >  {
> > @@ -120,15 +120,15 @@ SmmAccessPeiClose (  }
> >
> >  /**
> > -  Inhibits access to the SMRAM.
> > +  Inhibits access to the MMRAM.
> >
> > -  This function prohibits access to the SMRAM region.  This function
> > is usually
> > +  This function prohibits access to the MMRAM region.  This function
> > + is usually
> >    implemented such that it is a write-once operation.
> >
> >    @param  PeiServices              General purpose services available to every
> >                                     PEIM.
> > -  @param  This                     The pointer to the SMM Access Interface.
> > -  @param  DescriptorIndex          The region of SMRAM to Close.
> > +  @param  This                     The pointer to the MM Access Interface.
> > +  @param  DescriptorIndex          The region of MMRAM to Close.
> >
> >    @retval EFI_SUCCESS            The region was successfully locked.
> >    @retval EFI_DEVICE_ERROR       The region could not be locked because at
> > @@ -139,9 +139,9 @@ SmmAccessPeiClose (  STATIC  EFI_STATUS  EFIAPI
> > -SmmAccessPeiLock (
> > +MmAccessPeiLock (
> >    IN EFI_PEI_SERVICES                **PeiServices,
> > -  IN PEI_SMM_ACCESS_PPI              *This,
> > +  IN EFI_PEI_MM_ACCESS_PPI           *This,
> >    IN UINTN                           DescriptorIndex
> >    )
> >  {
> > @@ -158,11 +158,11 @@ SmmAccessPeiLock (
> >
> >  /**
> >    Queries the memory controller for the possible regions that will
> > support
> > -  SMRAM.
> > +  MMRAM.
> >
> >    @param  PeiServices           General purpose services available to every
> >                                  PEIM.
> > -  @param This                   The pointer to the SmmAccessPpi Interface.
> > +  @param This                   The pointer to the MmAccessPpi Interface.
> >    @param SmramMapSize           The pointer to the variable containing size
> of
> >                                  the buffer to contain the description
> >                                  information.
> > @@ -176,11 +176,11 @@ SmmAccessPeiLock (  STATIC  EFI_STATUS  EFIAPI
> > -SmmAccessPeiGetCapabilities (
> > +MmAccessPeiGetCapabilities (
> >    IN EFI_PEI_SERVICES                **PeiServices,
> > -  IN PEI_SMM_ACCESS_PPI              *This,
> > +  IN EFI_PEI_MM_ACCESS_PPI           *This,
> >    IN OUT UINTN                       *SmramMapSize,
> > -  IN OUT EFI_SMRAM_DESCRIPTOR        *SmramMap
> > +  IN OUT EFI_MMRAM_DESCRIPTOR        *SmramMap
> >    )
> >  {
> >    return SmramAccessGetCapabilities (This->LockState,
> > This->OpenState, @@ -190,18 +190,18 @@ SmmAccessPeiGetCapabilities (
> > //  // LockState and OpenState will be filled in by the entry point.
> >  //
> > -STATIC PEI_SMM_ACCESS_PPI mAccess = {
> > -  &SmmAccessPeiOpen,
> > -  &SmmAccessPeiClose,
> > -  &SmmAccessPeiLock,
> > -  &SmmAccessPeiGetCapabilities
> > +STATIC EFI_PEI_MM_ACCESS_PPI mAccess = {
> > +  &MmAccessPeiOpen,
> > +  &MmAccessPeiClose,
> > +  &MmAccessPeiLock,
> > +  &MmAccessPeiGetCapabilities
> >  };
> >
> >
> >  STATIC EFI_PEI_PPI_DESCRIPTOR mPpiList[] = {
> >    {
> >      EFI_PEI_PPI_DESCRIPTOR_PPI |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
> > -    &gPeiSmmAccessPpiGuid, &mAccess
> > +    &gEfiPeiMmAccessPpiGuid, &mAccess
> >    }
> >  };
> >
> > @@ -255,7 +255,7 @@ SmmAccessPeiEntryPoint (
> >    VOID                 *GuidHob;
> >
> >    //
> > -  // This module should only be included if SMRAM support is required.
> > +  // This module should only be included if MMRAM support is required.
> >    //
> >    ASSERT (FeaturePcdGet (PcdSmmSmramRequire));
> >
> > @@ -264,14 +264,14 @@ SmmAccessPeiEntryPoint (
> >    //
> >    HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
> >    if (HostBridgeDevId != INTEL_Q35_MCH_DEVICE_ID) {
> > -    DEBUG ((EFI_D_ERROR, "%a: no SMRAM with host bridge DID=0x%04x;
> only "
> > +    DEBUG ((EFI_D_ERROR, "%a: no MMRAM with host bridge DID=0x%04x;
> only "
> >        "DID=0x%04x (Q35) is supported\n", __FUNCTION__, HostBridgeDevId,
> >        INTEL_Q35_MCH_DEVICE_ID));
> >      goto WrongConfig;
> >    }
> >
> >    //
> > -  // Confirm if QEMU supports SMRAM.
> > +  // Confirm if QEMU supports MMRAM.
> >    //
> >    // With no support for it, the ESMRAMC (Extended System Management
> RAM
> >    // Control) register reads as zero. If there is support, the
> > cache-enable @@ -280,7 +280,7 @@ SmmAccessPeiEntryPoint (
> >    EsmramcVal = PciRead8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC));
> >    RegMask8 = MCH_ESMRAMC_SM_CACHE | MCH_ESMRAMC_SM_L1 |
> MCH_ESMRAMC_SM_L2;
> >    if ((EsmramcVal & RegMask8) != RegMask8) {
> > -    DEBUG ((EFI_D_ERROR, "%a: this Q35 implementation lacks SMRAM\n",
> > +    DEBUG ((EFI_D_ERROR, "%a: this Q35 implementation lacks
> MMRAM\n",
> >        __FUNCTION__));
> >      goto WrongConfig;
> >    }
> > @@ -323,27 +323,27 @@ SmmAccessPeiEntryPoint (
> >      (TopOfLowRamMb - mQ35TsegMbytes) << MCH_TSEGMB_MB_SHIFT);
> >
> >    //
> > -  // Set TSEG size, and disable TSEG visibility outside of SMM. Note
> > that the
> > +  // Set TSEG size, and disable TSEG visibility outside of MM. Note
> > + that the
> >    // T_EN bit has inverse meaning; when T_EN is set, then TSEG
> > visibility is
> > -  // *restricted* to SMM.
> > +  // *restricted* to MM.
> >    //
> >    EsmramcVal &= ~(UINT32)MCH_ESMRAMC_TSEG_MASK;
> >    EsmramcVal |= mQ35TsegMbytes == 8 ? MCH_ESMRAMC_TSEG_8MB :
> >                  mQ35TsegMbytes == 2 ? MCH_ESMRAMC_TSEG_2MB :
> >                  mQ35TsegMbytes == 1 ? MCH_ESMRAMC_TSEG_1MB :
> > -                MCH_ESMRAMC_TSEG_EXT;
> > -  EsmramcVal |= MCH_ESMRAMC_T_EN;
> > +                MCH_EMMRAMC_TSEG_EXT;  EsmramcVal |=
> > + MCH_EMMRAMC_T_EN;
> >    PciWrite8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC), EsmramcVal);
> >
> >    //
> >    // TSEG should be closed (see above), but unlocked, initially. Set
> > G_SMRAME
> > -  // (Global SMRAM Enable) too, as both D_LCK and T_EN depend on it.
> > +  // (Global MMRAM Enable) too, as both D_LCK and T_EN depend on it.
> >    //
> >    PciAndThenOr8 (DRAMC_REGISTER_Q35 (MCH_SMRAM),
> >      (UINT8)((~(UINT32)MCH_SMRAM_D_LCK) & 0xff),
> MCH_SMRAM_G_SMRAME);
> >
> >    //
> > -  // Create the GUID HOB and point it to the first SMRAM range.
> > +  // Create the GUID HOB and point it to the first MMRAM range.
> >    //
> >    GetStates (&mAccess.LockState, &mAccess.OpenState);
> >    SmramMapSize = sizeof SmramMap;
> > @@ -357,7 +357,7 @@ SmmAccessPeiEntryPoint (
> >      UINTN Idx;
> >
> >      Count = SmramMapSize / sizeof SmramMap[0];
> > -    DEBUG ((EFI_D_VERBOSE, "%a: SMRAM map follows, %d entries\n",
> __FUNCTION__,
> > +    DEBUG ((EFI_D_VERBOSE, "%a: MMRAM map follows, %d entries\n",
> > + __FUNCTION__,
> >        (INT32)Count));
> >      DEBUG ((EFI_D_VERBOSE, "% 20a % 20a % 20a % 20a\n",
> "PhysicalStart(0x)",
> >        "PhysicalSize(0x)", "CpuStart(0x)", "RegionState(0x)")); diff
> > --git a/OvmfPkg/SmmAccess/SmramInternal.c
> > b/OvmfPkg/SmmAccess/SmramInternal.c
> > index 18c42d29042d..3b33d2763b4d 100644
> > --- a/OvmfPkg/SmmAccess/SmramInternal.c
> > +++ b/OvmfPkg/SmmAccess/SmramInternal.c
> > @@ -40,10 +40,10 @@ InitQ35TsegMbytes (
> >
> >  /**
> >    Read the MCH_SMRAM and ESMRAMC registers, and update the
> LockState
> > and
> > -  OpenState fields in the PEI_SMM_ACCESS_PPI /
> > EFI_SMM_ACCESS2_PROTOCOL object,
> > +  OpenState fields in the EFI_PEI_MM_ACCESS_PPI /
> > + EFI_SMM_ACCESS2_PROTOCOL object,
> >    from the D_LCK and T_EN bits.
> >
> > -  PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member
> functions
> > can rely on
> > +  EFI_PEI_MM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member
> functions
> > + can rely on
> >    the LockState and OpenState fields being up-to-date on entry, and they
> need
> >    to restore the same invariant on exit, if they touch the bits in question.
> >
> > @@ -68,13 +68,13 @@ GetStates (
> >  }
> >
> >  //
> > -// The functions below follow the PEI_SMM_ACCESS_PPI and
> > +// The functions below follow the EFI_PEI_MM_ACCESS_PPI and
> >  // EFI_SMM_ACCESS2_PROTOCOL member declarations. The PeiServices
> and
> > This  // pointers are removed (TSEG doesn't depend on them), and so is
> > the  // DescriptorIndex parameter (TSEG doesn't support range-wise
> locking).
> >  //
> >  // The LockState and OpenState members that are common to both -//
> > PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are taken and
> updated
> > in
> > +// EFI_PEI_MM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are
> taken and
> > +updated in
> >  // isolation from the rest of the (non-shared) members.
> >  //
> >
> > diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.inf
> > b/OvmfPkg/SmmAccess/SmmAccessPei.inf
> > index 09f3b63446df..7360b2f5391e 100644
> > --- a/OvmfPkg/SmmAccess/SmmAccessPei.inf
> > +++ b/OvmfPkg/SmmAccess/SmmAccessPei.inf
> > @@ -63,7 +63,7 @@ [Pcd]
> >    gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
> >
> >  [Ppis]
> > -  gPeiSmmAccessPpiGuid           ## PRODUCES
> > +  gEfiPeiMmAccessPpiGuid         ## PRODUCES
> >
> >  [Depex]
> >    gEfiPeiMemoryDiscoveredPpiGuid
> >


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 7/8] OvmfPkg/SmmAccessPei: Update MM PPI usages.
  2018-07-24 12:16     ` Marvin Häuser
@ 2018-07-24 12:39       ` Laszlo Ersek
  0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-07-24 12:39 UTC (permalink / raw)
  To: Marvin Häuser, edk2-devel@lists.01.org

On 07/24/18 14:16, Marvin Häuser wrote:
> Hey Laszlo,
> 
>> -----Original Message-----

>> (1) Please post patch sets using a cover letter message:
>>
>> git config format.coverletter     true
>> git config format.numbered        true
>> git config sendemail.chainreplyto false
>> git config sendemail.thread       true
>>
>> Otherwise the individual patches in the series will get intermixed with other
>> messages / threads on the mailing list (and in people's inboxes), if the
>> recipient uses a threaded view in their MUA (which most people should do --
>> patches and discussions are very hard to follow otherwise).
> 
> Is V2 fine in this aspect? Outlook has never threaded multiple parts of a patch series for me but only replies to their original message, hence I did not notice something was off.

It doesn't look right to me, looking at the following patches patches:

[edk2] [PATCH v2 0/2] Introduce UEFI PI 1.5 MM PPI.
[edk2] [PATCH v2 1/2] MdePkg: Add PI 1.5 MM PPI declarations.
[edk2] [PATCH v2 2/2] MdeModulePkg: Change SMM PPIs to shim MM PPIs.

* v2 0/2:

Message-ID:
<VI1PR0801MB179040BA004EB739CE6AA07A80550@VI1PR0801MB1790.eurprd08.prod.outlook.com>

* v2 1/2:

Message-ID:
<VI1PR0801MB1790297B718346971DD0A30A80550@VI1PR0801MB1790.eurprd08.prod.outlook.com>

References: <cover.1532434029.git.Marvin.Haeuser@outlook.com>

In-Reply-To: <cover.1532434029.git.Marvin.Haeuser@outlook.com>

* v2 2/2:

Message-ID:
<VI1PR0801MB1790937D22054BF7815580CE80550@VI1PR0801MB1790.eurprd08.prod.outlook.com>

References: <cover.1532434029.git.Marvin.Haeuser@outlook.com>

In-Reply-To: <cover.1532434029.git.Marvin.Haeuser@outlook.com>


Here's what I think happened: the original message-id (formatted by your
local tooling) for the cover letter was

<cover.1532434029.git.Marvin.Haeuser@outlook.com>

Accordingly, your local tools placed this message ID *correctly* into
both "In-Reply-To" and "References" headers of the actual patches #1 and
#2. This is the proper way to ensure threading.

Unfortunately, outlook.com's SMTP server seems to have overwritten your
Message-ID, on the cover letter (and the rest of the emails too),
despite your tools generating and embedding Message-ID in the first
place. Obviously this breaks threading. This is a bug in outlook.com's
SMTP service, most likely.

Can you open a support ticket with them, or else use something else for
posting patches than outlook.com?

Thanks,
Laszlo


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-07-24 12:39 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <d625966ce7c0ac98b65f1e946b2cf833f6801706.1532395896.git.Marvin.Haeuser@outlook.com>
2018-07-24  1:40 ` [PATCH 2/8] MdeModulePkg/SmmLockBoxPeiLib: Update MM PPI usages Marvin Häuser
2018-07-24  1:40 ` [PATCH 3/8] UefiCpuPkg/PiSmmCommunicationPei: " Marvin Häuser
2018-07-24  1:40 ` [PATCH 4/8] UefiCpuPkg/S3Resume2Pei: " Marvin Häuser
2018-07-24  1:40 ` [PATCH 5/8] QuarkSocPkg/SmmAccessPei: " Marvin Häuser
2018-07-24  1:40 ` [PATCH 6/8] QuarkSocPkg/SmmControlPei: " Marvin Häuser
2018-07-24  1:40 ` [PATCH 7/8] OvmfPkg/SmmAccessPei: " Marvin Häuser
2018-07-24  9:20   ` Laszlo Ersek
2018-07-24 12:16     ` Marvin Häuser
2018-07-24 12:39       ` Laszlo Ersek
2018-07-24  1:40 ` [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs Marvin Häuser
2018-07-24  6:38   ` Gao, Liming
2018-07-24 10:04     ` Zeng, Star
2018-07-24 10:28     ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox