Re: [edk2-devel] [PATCH] MdeModulePkg/PiSmmCore: Drop deprecated image profiling commands

["Marvin Häuser" <mhaeuser@posteo.de>]

Hey Jiewen,

Good point, sure. Will probably add ASSERTs as well, if that is fine 
with you.

Best regards,
Marvin

On 09/08/2021 07:33, Yao, Jiewen wrote:
> Can we define 3 and 4 to be "reserved and do not use", instead of removed ?
> -#define SMRAM_PROFILE_COMMAND_REGISTER_IMAGE             0x3
> -#define SMRAM_PROFILE_COMMAND_UNREGISTER_IMAGE           0x4
>
> For example:
> Command 0x3 and 0x4 are deprecated and reserved. They should not be used in the future.
>
> With that changed, reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
>
>
>> -----Original Message-----
>> From: Ni, Ray <ray.ni@intel.com>
>> Sent: Monday, August 9, 2021 12:24 PM
>> To: mhaeuser@posteo.de; devel@edk2.groups.io
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
>> Dong, Eric <eric.dong@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>;
>> Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
>> Subject: RE: [PATCH] MdeModulePkg/PiSmmCore: Drop deprecated image
>> profiling commands
>>
>> Reviewed-by: Ray Ni <ray.ni@intel.com>
>>
>> +Star and Jiewen for confirmation.
>>
>> -----Original Message-----
>> From: Marvin Häuser <mhaeuser@posteo.de>
>> Sent: Monday, August 9, 2021 3:40 AM
>> To: devel@edk2.groups.io
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
>> Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Vitaly Cheptsov
>> <vit9696@protonmail.com>
>> Subject: [PATCH] MdeModulePkg/PiSmmCore: Drop deprecated image profiling
>> commands
>>
>> The legacy codebase allowed SMM images to be registered for profiling
>> from DXE. Support for this has been dropped entirely, so remove the
>> remaining handlers.
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Hao A Wu <hao.a.wu@intel.com>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
>> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
>> ---
>>   MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c | 80 --------------------
>>   MdeModulePkg/Include/Guid/MemoryProfile.h        |  5 --
>>   2 files changed, 85 deletions(-)
>>
>> diff --git a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
>> b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
>> index 1b302c810cc9..7316df7531fd 100644
>> --- a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
>> +++ b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
>> @@ -2232,64 +2232,6 @@ Done:
>>     mSmramProfileGettingStatus = SmramProfileGettingStatus;
>>
>>   }
>>
>>
>>
>> -/**
>>
>> -  SMRAM profile handler to register SMM image.
>>
>> -
>>
>> -  @param SmramProfileParameterRegisterImage The parameter of SMM
>> profile register image.
>>
>> -
>>
>> -**/
>>
>> -VOID
>>
>> -SmramProfileHandlerRegisterImage (
>>
>> -  IN SMRAM_PROFILE_PARAMETER_REGISTER_IMAGE
>> *SmramProfileParameterRegisterImage
>>
>> -  )
>>
>> -{
>>
>> -  EFI_STATUS                        Status;
>>
>> -  EFI_SMM_DRIVER_ENTRY              DriverEntry;
>>
>> -  VOID                              *EntryPointInImage;
>>
>> -
>>
>> -  ZeroMem (&DriverEntry, sizeof (DriverEntry));
>>
>> -  CopyMem (&DriverEntry.FileName, &SmramProfileParameterRegisterImage-
>>> FileName, sizeof(EFI_GUID));
>> -  DriverEntry.ImageBuffer = SmramProfileParameterRegisterImage-
>>> ImageBuffer;
>> -  DriverEntry.NumberOfPage = (UINTN) SmramProfileParameterRegisterImage-
>>> NumberOfPage;
>> -  Status = InternalPeCoffGetEntryPoint ((VOID *) (UINTN)
>> DriverEntry.ImageBuffer, &EntryPointInImage);
>>
>> -  ASSERT_EFI_ERROR (Status);
>>
>> -  DriverEntry.ImageEntryPoint = (PHYSICAL_ADDRESS) (UINTN)
>> EntryPointInImage;
>>
>> -
>>
>> -  Status = RegisterSmramProfileImage (&DriverEntry, FALSE);
>>
>> -  if (!EFI_ERROR (Status)) {
>>
>> -    SmramProfileParameterRegisterImage->Header.ReturnStatus = 0;
>>
>> -  }
>>
>> -}
>>
>> -
>>
>> -/**
>>
>> -  SMRAM profile handler to unregister SMM image.
>>
>> -
>>
>> -  @param SmramProfileParameterUnregisterImage The parameter of SMM
>> profile unregister image.
>>
>> -
>>
>> -**/
>>
>> -VOID
>>
>> -SmramProfileHandlerUnregisterImage (
>>
>> -  IN SMRAM_PROFILE_PARAMETER_UNREGISTER_IMAGE
>> *SmramProfileParameterUnregisterImage
>>
>> -  )
>>
>> -{
>>
>> -  EFI_STATUS                        Status;
>>
>> -  EFI_SMM_DRIVER_ENTRY              DriverEntry;
>>
>> -  VOID                              *EntryPointInImage;
>>
>> -
>>
>> -  ZeroMem (&DriverEntry, sizeof (DriverEntry));
>>
>> -  CopyMem (&DriverEntry.FileName,
>> &SmramProfileParameterUnregisterImage->FileName, sizeof (EFI_GUID));
>>
>> -  DriverEntry.ImageBuffer = SmramProfileParameterUnregisterImage-
>>> ImageBuffer;
>> -  DriverEntry.NumberOfPage = (UINTN)
>> SmramProfileParameterUnregisterImage->NumberOfPage;
>>
>> -  Status = InternalPeCoffGetEntryPoint ((VOID *) (UINTN)
>> DriverEntry.ImageBuffer, &EntryPointInImage);
>>
>> -  ASSERT_EFI_ERROR (Status);
>>
>> -  DriverEntry.ImageEntryPoint = (PHYSICAL_ADDRESS) (UINTN)
>> EntryPointInImage;
>>
>> -
>>
>> -  Status = UnregisterSmramProfileImage (&DriverEntry, FALSE);
>>
>> -  if (!EFI_ERROR (Status)) {
>>
>> -    SmramProfileParameterUnregisterImage->Header.ReturnStatus = 0;
>>
>> -  }
>>
>> -}
>>
>> -
>>
>>   /**
>>
>>     Dispatch function for a Software SMI handler.
>>
>>
>>
>> @@ -2374,28 +2316,6 @@ SmramProfileHandler (
>>       }
>>
>>       SmramProfileHandlerGetDataByOffset
>> ((SMRAM_PROFILE_PARAMETER_GET_PROFILE_DATA_BY_OFFSET *) (UINTN)
>> CommBuffer);
>>
>>       break;
>>
>> -  case SMRAM_PROFILE_COMMAND_REGISTER_IMAGE:
>>
>> -    DEBUG ((EFI_D_ERROR, "SmramProfileHandlerRegisterImage\n"));
>>
>> -    if (TempCommBufferSize != sizeof
>> (SMRAM_PROFILE_PARAMETER_REGISTER_IMAGE)) {
>>
>> -      DEBUG ((EFI_D_ERROR, "SmramProfileHandler: SMM communication buffer
>> size invalid!\n"));
>>
>> -      return EFI_SUCCESS;
>>
>> -    }
>>
>> -    if (mSmramReadyToLock) {
>>
>> -      return EFI_SUCCESS;
>>
>> -    }
>>
>> -    SmramProfileHandlerRegisterImage
>> ((SMRAM_PROFILE_PARAMETER_REGISTER_IMAGE *) (UINTN) CommBuffer);
>>
>> -    break;
>>
>> -  case SMRAM_PROFILE_COMMAND_UNREGISTER_IMAGE:
>>
>> -    DEBUG ((EFI_D_ERROR, "SmramProfileHandlerUnregisterImage\n"));
>>
>> -    if (TempCommBufferSize != sizeof
>> (SMRAM_PROFILE_PARAMETER_UNREGISTER_IMAGE)) {
>>
>> -      DEBUG ((EFI_D_ERROR, "SmramProfileHandler: SMM communication buffer
>> size invalid!\n"));
>>
>> -      return EFI_SUCCESS;
>>
>> -    }
>>
>> -    if (mSmramReadyToLock) {
>>
>> -      return EFI_SUCCESS;
>>
>> -    }
>>
>> -    SmramProfileHandlerUnregisterImage
>> ((SMRAM_PROFILE_PARAMETER_UNREGISTER_IMAGE *) (UINTN) CommBuffer);
>>
>> -    break;
>>
>>     case SMRAM_PROFILE_COMMAND_GET_RECORDING_STATE:
>>
>>       DEBUG ((EFI_D_ERROR, "SmramProfileHandlerGetRecordingState\n"));
>>
>>       if (TempCommBufferSize != sizeof
>> (SMRAM_PROFILE_PARAMETER_RECORDING_STATE)) {
>>
>> diff --git a/MdeModulePkg/Include/Guid/MemoryProfile.h
>> b/MdeModulePkg/Include/Guid/MemoryProfile.h
>> index eee3b9125240..92cd1e7cf493 100644
>> --- a/MdeModulePkg/Include/Guid/MemoryProfile.h
>> +++ b/MdeModulePkg/Include/Guid/MemoryProfile.h
>> @@ -388,11 +388,6 @@ struct _EDKII_MEMORY_PROFILE_PROTOCOL {
>>   //
>>
>>   #define SMRAM_PROFILE_COMMAND_GET_PROFILE_INFO           0x1
>>
>>   #define SMRAM_PROFILE_COMMAND_GET_PROFILE_DATA           0x2
>>
>> -//
>>
>> -// Below 2 commands are now used by ECP only and only valid before
>> SmmReadyToLock
>>
>> -//
>>
>> -#define SMRAM_PROFILE_COMMAND_REGISTER_IMAGE             0x3
>>
>> -#define SMRAM_PROFILE_COMMAND_UNREGISTER_IMAGE           0x4
>>
>>
>>
>>   #define SMRAM_PROFILE_COMMAND_GET_PROFILE_DATA_BY_OFFSET 0x5
>>
>>   #define SMRAM_PROFILE_COMMAND_GET_RECORDING_STATE        0x6
>>
>> --
>> 2.31.1
>
>
> 
>
>