tianocore / edk2

EDK II
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II
Other
4.58k stars 2.47k forks source link

OvmfPkg/Sec: Skip setup MTRR early in TD-Guest #5874

Closed sunceping closed 2 months ago

sunceping commented 3 months ago

With the commit ce4c76e (“OvmfPkg/Sec: Setup MTRR early in the boot process.”), we find an unexpected #VE is triggered in TD-Guest.

The background of importing the above commit is that: Before running lzma uncompress of the main firmware volume, if not correctly set MTRR, that would make the uncompress be extremely slow. Detailed discussion info can refer to below links: https://edk2.groups.io/g/devel/message/114202 https://edk2.groups.io/g/devel/message/114977

Refer to [intel-tdx-module-1.5-base-spec] Section 11.3 and section11.6.1, CR0.CD is enforced to 0 in TD-Guest. And refer to section 18.2.1.4, TDX module MTRR emulation enforces WB in VMM.

Currently the initial MTRR are:

In DXE phase, OVMF/TDVF would check the MTRR Type for MMIO (in CpuSetMemoryAttributes -> MtrrGetMemoryAttribute -> MtrrGetMemoryAttributeworker: https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MtrrLib/MtrrLib.c#L929 ). If MTRR is disabled, it always returns UC. Otherwise, it returns the actual value.

If it checks that the type is not UC then the MTRR is programmed. It is required to disable cache by setting CR0.CD to 1. That will trigger an unexpected #VE in TD-Guest.

Based on above analysis we propose to skip "Setup MTRR early" in TD-Guest because of:

intel-tdx-module-1.5-base-spec: https://cdrdv2.intel.com/v1/dl/getContent/733575

Cc: Erdem Aktas erdemaktas@google.com Cc: Jiewen Yao jiewen.yao@intel.com Cc: Min Xu min.m.xu@intel.com Cc: Gerd Hoffmann kraxel@redhat.com Cc: Elena Reshetova elena.reshetova@intel.com

Description

<_Include a description of the change and why this change was made._> <_For each item, place an "x" in between `[` and `]` if true. Example: `[x]` (you can also check items in GitHub UI)_> <_Create the PR as a Draft PR if it is only created to run CI checks._> <_Delete lines in \<\> tags before creating the PR._> - [ ] Breaking change? - **Breaking change** - Will this cause a break in build or boot behavior? - Examples: Add a new library class or move a module to a different repo. - [ ] Impacts security? - **Security** - Does the change have a direct security impact? - Examples: Crypto algorithm change or buffer overflow fix. - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests or integration tests. ## How This Was Tested <_Describe the test(s) that were run to verify the changes._> ## Integration Instructions <_Describe how these changes should be integrated. Use N/A if nothing is required._>