Re: [edk2-devel] [PATCH v2 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: Optimize PatchSmmSaveStateMap and FlushTlbForAll

["Laszlo Ersek" <lersek@redhat.com>]

On 1/5/24 13:52, Ni, Ray wrote:
> Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks, please feel free to merge this!
Laszlo

> 
> 
> Thanks,
> Ray
>> -----Original Message-----
>> From: Jin, Zhi <zhi.jin@intel.com>
>> Sent: Friday, January 5, 2024 10:54 AM
>> To: devel@edk2.groups.io
>> Cc: Jin, Zhi <zhi.jin@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek
>> <lersek@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd
>> Hoffmann <kraxel@redhat.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
>> Subject: [PATCH v2 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: Optimize
>> PatchSmmSaveStateMap and FlushTlbForAll
>>
>> PatchSmmSaveStateMap patches the SMM entry (code) and SmmSaveState
>> region (data) for each core, which can be improved to flush TLB once
>> after all the memory entries have been patched.
>> FlushTlbForAll flushes TLB for each core in serial, which can be
>> improved to flush TLB in parrallel.
>>
>> v2:
>>    Add the missing FlushTlbForAll() back in PatchSmmSaveStateMap().
>>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>> Signed-off-by: Zhi Jin <zhi.jin@intel.com>
>> ---
>>  .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c   | 97
>> +++++++++++++------
>>  1 file changed, 65 insertions(+), 32 deletions(-)
>>
>> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
>> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
>> index 15f998e501..12f3c0b8e8 100644
>> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
>> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
>> @@ -547,17 +547,14 @@ FlushTlbForAll (
>>    VOID
>>    )
>>  {
>> -  UINTN  Index;
>> -
>>    FlushTlbOnCurrentProcessor (NULL);
>> -
>> -  for (Index = 0; Index < gSmst->NumberOfCpus; Index++) {
>> -    if (Index != gSmst->CurrentlyExecutingCpu) {
>> -      // Force to start up AP in blocking mode,
>> -      SmmBlockingStartupThisAp (FlushTlbOnCurrentProcessor, Index, NULL);
>> -      // Do not check return status, because AP might not be present in some
>> corner cases.
>> -    }
>> -  }
>> +  InternalSmmStartupAllAPs (
>> +    (EFI_AP_PROCEDURE2)FlushTlbOnCurrentProcessor,
>> +    0,
>> +    NULL,
>> +    NULL,
>> +    NULL
>> +    );
>>  }
>>
>>  /**
>> @@ -799,72 +796,108 @@ PatchSmmSaveStateMap (
>>    UINTN  TileCodeSize;
>>    UINTN  TileDataSize;
>>    UINTN  TileSize;
>> +  UINTN  PageTableBase;
>>
>> -  TileCodeSize = GetSmiHandlerSize ();
>> -  TileCodeSize = ALIGN_VALUE (TileCodeSize, SIZE_4KB);
>> -  TileDataSize = (SMRAM_SAVE_STATE_MAP_OFFSET - SMM_PSD_OFFSET) +
>> sizeof (SMRAM_SAVE_STATE_MAP);
>> -  TileDataSize = ALIGN_VALUE (TileDataSize, SIZE_4KB);
>> -  TileSize     = TileDataSize + TileCodeSize - 1;
>> -  TileSize     = 2 * GetPowerOfTwo32 ((UINT32)TileSize);
>> +  TileCodeSize  = GetSmiHandlerSize ();
>> +  TileCodeSize  = ALIGN_VALUE (TileCodeSize, SIZE_4KB);
>> +  TileDataSize  = (SMRAM_SAVE_STATE_MAP_OFFSET - SMM_PSD_OFFSET) +
>> sizeof (SMRAM_SAVE_STATE_MAP);
>> +  TileDataSize  = ALIGN_VALUE (TileDataSize, SIZE_4KB);
>> +  TileSize      = TileDataSize + TileCodeSize - 1;
>> +  TileSize      = 2 * GetPowerOfTwo32 ((UINT32)TileSize);
>> +  PageTableBase = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;
>>
>>    DEBUG ((DEBUG_INFO, "PatchSmmSaveStateMap:\n"));
>>    for (Index = 0; Index < mMaxNumberOfCpus - 1; Index++) {
>>      //
>>      // Code
>>      //
>> -    SmmSetMemoryAttributes (
>> +    ConvertMemoryPageAttributes (
>> +      PageTableBase,
>> +      mPagingMode,
>>        mCpuHotPlugData.SmBase[Index] + SMM_HANDLER_OFFSET,
>>        TileCodeSize,
>> -      EFI_MEMORY_RO
>> +      EFI_MEMORY_RO,
>> +      TRUE,
>> +      NULL
>>        );
>> -    SmmClearMemoryAttributes (
>> +    ConvertMemoryPageAttributes (
>> +      PageTableBase,
>> +      mPagingMode,
>>        mCpuHotPlugData.SmBase[Index] + SMM_HANDLER_OFFSET,
>>        TileCodeSize,
>> -      EFI_MEMORY_XP
>> +      EFI_MEMORY_XP,
>> +      FALSE,
>> +      NULL
>>        );
>>
>>      //
>>      // Data
>>      //
>> -    SmmClearMemoryAttributes (
>> +    ConvertMemoryPageAttributes (
>> +      PageTableBase,
>> +      mPagingMode,
>>        mCpuHotPlugData.SmBase[Index] + SMM_HANDLER_OFFSET +
>> TileCodeSize,
>>        TileSize - TileCodeSize,
>> -      EFI_MEMORY_RO
>> +      EFI_MEMORY_RO,
>> +      FALSE,
>> +      NULL
>>        );
>> -    SmmSetMemoryAttributes (
>> +    ConvertMemoryPageAttributes (
>> +      PageTableBase,
>> +      mPagingMode,
>>        mCpuHotPlugData.SmBase[Index] + SMM_HANDLER_OFFSET +
>> TileCodeSize,
>>        TileSize - TileCodeSize,
>> -      EFI_MEMORY_XP
>> +      EFI_MEMORY_XP,
>> +      TRUE,
>> +      NULL
>>        );
>>    }
>>
>>    //
>>    // Code
>>    //
>> -  SmmSetMemoryAttributes (
>> +  ConvertMemoryPageAttributes (
>> +    PageTableBase,
>> +    mPagingMode,
>>      mCpuHotPlugData.SmBase[mMaxNumberOfCpus - 1] +
>> SMM_HANDLER_OFFSET,
>>      TileCodeSize,
>> -    EFI_MEMORY_RO
>> +    EFI_MEMORY_RO,
>> +    TRUE,
>> +    NULL
>>      );
>> -  SmmClearMemoryAttributes (
>> +  ConvertMemoryPageAttributes (
>> +    PageTableBase,
>> +    mPagingMode,
>>      mCpuHotPlugData.SmBase[mMaxNumberOfCpus - 1] +
>> SMM_HANDLER_OFFSET,
>>      TileCodeSize,
>> -    EFI_MEMORY_XP
>> +    EFI_MEMORY_XP,
>> +    FALSE,
>> +    NULL
>>      );
>>
>>    //
>>    // Data
>>    //
>> -  SmmClearMemoryAttributes (
>> +  ConvertMemoryPageAttributes (
>> +    PageTableBase,
>> +    mPagingMode,
>>      mCpuHotPlugData.SmBase[mMaxNumberOfCpus - 1] +
>> SMM_HANDLER_OFFSET + TileCodeSize,
>>      SIZE_32KB - TileCodeSize,
>> -    EFI_MEMORY_RO
>> +    EFI_MEMORY_RO,
>> +    FALSE,
>> +    NULL
>>      );
>> -  SmmSetMemoryAttributes (
>> +  ConvertMemoryPageAttributes (
>> +    PageTableBase,
>> +    mPagingMode,
>>      mCpuHotPlugData.SmBase[mMaxNumberOfCpus - 1] +
>> SMM_HANDLER_OFFSET + TileCodeSize,
>>      SIZE_32KB - TileCodeSize,
>> -    EFI_MEMORY_XP
>> +    EFI_MEMORY_XP,
>> +    TRUE,
>> +    NULL
>>      );
>> +
>> +  FlushTlbForAll ();
>>  }
>>
>>  /**
>> --
>> 2.39.2
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113286): https://edk2.groups.io/g/devel/message/113286
Mute This Topic: https://groups.io/mt/103536553/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-