Re: [PATCH V2 0/6] Enable SMM page level protection.

[Laszlo Ersek <lersek@redhat.com>]

On 11/04/16 23:46, Yao, Jiewen wrote:
> Ah, yes. Laszlo. You are right.
> 
> I forget to push the last update yesterday. Thank you to remind me.
> Now it is synced.

Thanks! The commit message updates and the v1->v2 differences look
good/reasonable to me (I diffed the code-level end results of the two
versions, plus I compared the commit messages pairwise). I hope to test
v2 sometime next week, and I intend to look into the S3 instability too
(I took note of Paolo's advice with the "info tlb" QEMU monitor command).

Going through the (now documented) SMRAM impact again, I realize the
platform can elect to set PcdCpuSmmStaticPageTable dynamically as well.
I'm sort of guessing that we might want to set the PCD in OVMF's
PlatformPei, based on the guest-phys address width (which we also
calculate in PlatformPei), in combination with availability of 1G
paging. The case we should likely avoid is

>     A) If the system only supports 2M paging,
>     When the whole memory/MMIO is 48bit, we need 1+256+256*256 pages
>       (~ 257M)

Anyway, I don't want to be too clever about this until we see a problem
(out-of-SMRAM) in practice.

Thanks!
Laszlo

> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Saturday, November 5, 2016 6:40 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@ml01.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH V2 0/6] Enable SMM page level protection.
> 
> On 11/04/16 10:30, Jiewen Yao wrote:
>> ==== below is V2 description ====
>> 1) PiSmmCpu: resolve OVMF multiple processors boot hang issue.
>> 2) PiSmmCpu: Add debug info on StartupAp() fails.
>> 3) PiSmmCpu: Add ASSERT for AllocatePages().
>> 4) PiSmmCpu: Add protection detail in commit message.
>> 5) UefiCpuPkg.dsc: Add page table footprint info in commit message.
> 
> Jiewen, can you please push this series to a new branch in your repo?
> 
> I see a branch called "SmmProtection_V2", but it seems to end with an
> incomplete patch (26f482d8b611d0fcb07d3ffbf3f4468fd249767b, subject
> "pismmcpu"), so I figured I'd ask explicitly.
> 
> Thanks
> Laszlo
> 
>> ==== below is V1 description ====
>> This series patch enables SMM page level protection.
>> Features are:
>> 1) PiSmmCore reports SMM PE image code/data information
>> in EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned.
>> 2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable
>> and set XD for data page and RO for code page.
>> 3) PiSmmCpu enables Static Paging for X64 according to
>> PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G
>> is used as long as it is supported.
>> 4) PiSmmCpu sets importance data structure to be read only,
>> such as Gdt, Idt, SmmEntrypoint, and PageTable itself.
>>
>> tested platform:
>> 1) Intel internal platform (X64).
>> 2) EDKII Quark IA32
>> 3) EDKII Vlv2  X64
>> 4) EDKII OVMF IA32 and IA32X64. (with -smp 8)
>>
>> Cc: Jeff Fan <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>
>> Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com>>
>> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
>> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
>>
>> Jiewen Yao (6):
>>   MdeModulePkg/Include: Add PiSmmMemoryAttributesTable.h
>>   MdeModulePkg/dec: Add gEdkiiPiSmmMemoryAttributesTableGuid.
>>   MdeModulePkg/PiSmmCore: Add MemoryAttributes support.
>>   UefiCpuPkg/dec: Add PcdCpuSmmStaticPageTable.
>>   UefiCpuPkg/PiSmmCpuDxeSmm: Add paging protection.
>>   QuarkPlatformPkg/dsc: enable Smm paging protection.
>>
>>  MdeModulePkg/Core/PiSmmCore/Dispatcher.c               |   66 +
>>  MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c    | 1509 ++++++++++++++++++++
>>  MdeModulePkg/Core/PiSmmCore/Page.c                     |  775 +++++++++-
>>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c                |   40 +
>>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h                |   91 ++
>>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf              |    2 +
>>  MdeModulePkg/Core/PiSmmCore/Pool.c                     |   16 +
>>  MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h |   51 +
>>  MdeModulePkg/MdeModulePkg.dec                          |    3 +
>>  QuarkPlatformPkg/Quark.dsc                             |    6 +
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c               |   71 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S              |   67 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm            |   68 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm           |   70 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S          |  226 +--
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm        |   36 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm       |   36 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c          |   37 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c        |    4 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c                  |  127 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c             |  142 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h             |  156 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf           |    5 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c     |  871 +++++++++++
>>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c                 |   39 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h                 |   15 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c                |  274 +++-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S               |   51 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm             |   54 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm            |   61 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.S           |  250 +---
>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.asm         |   35 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.nasm        |   31 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c           |   30 +-
>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c         |    7 +-
>>  UefiCpuPkg/UefiCpuPkg.dec                              |    8 +
>>  36 files changed, 4529 insertions(+), 801 deletions(-)
>>  create mode 100644 MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
>>  create mode 100644 MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h
>>  create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
>>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>