diff options
author | Laszlo Ersek <lersek@redhat.com> | 2017-10-24 17:21:25 +0200 |
---|---|---|
committer | Laszlo Ersek <lersek@redhat.com> | 2017-11-17 18:12:18 +0100 |
commit | d41fd8e839a31e6374cf37c8516311db4e4a3dbc (patch) | |
tree | f9adcc11aaa8b37ad88510cd0142544caaaf9a89 | |
parent | 2278b8a82ea945e9e93b0695b78896ba8db414d9 (diff) | |
download | edk2-d41fd8e839a31e6374cf37c8516311db4e4a3dbc.tar.gz edk2-d41fd8e839a31e6374cf37c8516311db4e4a3dbc.tar.bz2 edk2-d41fd8e839a31e6374cf37c8516311db4e4a3dbc.zip |
OvmfPkg: restore temporary SEC/PEI RAM size to 64KB
(1) In the PEI phase, the PCD database is maintained in a GUID HOB. In
OVMF, we load the PCD PEIM before any other PEIMs (using APRIORI PEI),
so that all other PEIMs can use dynamic PCDs. Consequently,
- the PCD GUID HOB is initially allocated from the temporary SEC/PEI
heap,
- whenever we introduce a dynamic PCD to a PEIM built into OVMF such
that the PCD is new to OVMF's whole PEI phase, the PCD GUID HOB (and
its temporary heap footprint) grow.
I've noticed that, if we add just one more dynamic PCD to the PEI
phase, then in the X64 build,
- we get very close to the half of the temporary heap (i.e., 8192
bytes),
- obscure PEI phase hangs or DXE core initialization failures
(ASSERTs) occur. The symptoms vary between the FD_SIZE_2MB and
FD_SIZE_4MB builds of X64 OVMF.
(2) I've found that commit
2bbd7e2fbd4b ("UefiCpuPkg/MtrrLib: Update algorithm to calculate
optimal settings", 2017-09-27)
introduced a large (16KB) stack allocation:
> The patch changes existing MtrrSetMemoryAttributeInMtrrSettings() and
> MtrrSetMemoryAttribute() to use the 4-page stack buffer for calculation.
> ...
> +#define SCRATCH_BUFFER_SIZE (4 * SIZE_4KB)
> ...
> @@ -2207,17 +2462,66 @@ MtrrSetMemoryAttributeInMtrrSettings (
> ...
> + UINT8 Scratch[SCRATCH_BUFFER_SIZE];
(3) OVMF's temp SEC/PEI RAM size has been 32KB ever since commit
7cb6b0e06809 ("OvmfPkg: Move SEC/PEI Temporary RAM from 0x70000 to
0x810000", 2014-01-21)
Of that, the upper 16KB half is stack (growing down), and the lower
16KB half is heap.
Thus, OvmfPkg/PlatformPei's calls to "UefiCpuPkg/Library/MtrrLib", in
QemuInitializeRam(), cause the Scratch array to overflow the entire
stack (heading towards lower addresses), and corrupt the heap below
the stack. It turns out that the total stack demand is about 24KB, so
the overflow is able to corrupt the upper 8KB of the heap. If that
part of the heap is actually used (for example because we grow the PCD
GUID HOB sufficiently), mayhem ensues.
(4) Right after commit 7cb6b0e06809 (see above), there would be no room
left above the 32KB temp SEC/PEI RAM. However, given more recent
commits
45d870815156 ("OvmfPkg/PlatformPei: rebase and resize the permanent
PEI memory for S3", 2016-07-13)
6b04cca4d697 ("OvmfPkg: remove PcdS3AcpiReservedMemoryBase,
PcdS3AcpiReservedMemorySize", 2016-07-12)
we can now restore the temp SEC/PEI RAM size to the original
(pre-7cb6b0e06809) 64KB. This will allow for a 32KB temp SEC/PEI
stack, which accommodates the ~24KB demand mentioned in (3).
(Prior patches in this series will let us monitor the stack usage in
the future.)
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=747
Ref: http://mid.mail-archive.com/a49cc089-12ae-a887-a4d6-4dc509233a74@redhat.com
Ref: http://mid.mail-archive.com/03e369bb-77c4-0134-258f-bdae62cbc8c5@redhat.com
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
-rw-r--r-- | OvmfPkg/OvmfPkgIa32.fdf | 2 | ||||
-rw-r--r-- | OvmfPkg/OvmfPkgIa32X64.fdf | 2 | ||||
-rw-r--r-- | OvmfPkg/OvmfPkgX64.fdf | 2 |
3 files changed, 3 insertions, 3 deletions
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf index 7515224118..06a439f8cb 100644 --- a/OvmfPkg/OvmfPkgIa32.fdf +++ b/OvmfPkg/OvmfPkgIa32.fdf @@ -82,7 +82,7 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid. 0x007000|0x001000
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
-0x010000|0x008000
+0x010000|0x010000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
0x020000|0x0E0000
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf index f1a2044fb7..ced4c5639f 100644 --- a/OvmfPkg/OvmfPkgIa32X64.fdf +++ b/OvmfPkg/OvmfPkgIa32X64.fdf @@ -82,7 +82,7 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid. 0x007000|0x001000
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
-0x010000|0x008000
+0x010000|0x010000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
0x020000|0x0E0000
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf index 32000a3b93..62dd58f6e4 100644 --- a/OvmfPkg/OvmfPkgX64.fdf +++ b/OvmfPkg/OvmfPkgX64.fdf @@ -82,7 +82,7 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid. 0x007000|0x001000
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
-0x010000|0x008000
+0x010000|0x010000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
0x020000|0x0E0000
|