On 10/02/19 16:43, Lendacky, Thomas wrote: > On 10/2/19 5:23 AM, Laszlo Ersek wrote: >> On 09/19/19 21:52, Lendacky, Thomas wrote:
>>> @@ -38,6 +44,34 @@ AmdSevEsInitialize ( >>> >>> PcdStatus = PcdSetBoolS (PcdSevEsActive, 1); >>> ASSERT_RETURN_ERROR (PcdStatus); >>> + >>> + // >>> + // Allocate GHCB pages. >>> + // >>> + GhcbPageCount = mMaxCpuCount; >>> + GhcbBase = AllocatePages (GhcbPageCount); >> >> Yes, AllocatePages() looks safe, per >> <4029f533-9f2a-ba71-2ba6-348187dc7684@redhat.com">http://mid.mail-archive.com/4029f533-9f2a-ba71-2ba6-348187dc7684@redhat.com>. >> >>> + ASSERT (GhcbBase); >> >> (3) I don't really like using *only* an ASSERT for stopping, when we run >> out of resources in a place like this (i.e. where there's no way to >> recover, or to report the issue nicely). I'd suggest throwing in an >> explicit check and a CpuDeadLoop(), in addition to the ASSERT. Anyway, >> not really important, up to you. > > Ok, let me think about that. If ASSERT is enabled, you'll get the ASSERT > message (since the SEC GHCB is in place and OVMF is running in 64-bit > mode). If ASSERT is not enabled, then the ZeroMem will segfault on a NULL > pointer, which will give a bit more info than the CpuDeadLoop() which > would look more like a hang. I don't insist on "strengthening" the ASSERT(). For technical correctness though: accessing page#0 has -- by default -- no ill effects in the firmware. Page#0 is normal RAM, and mapped as such (by default). MdeModulePkg offers null pointer detection; see "PcdNullPointerDetectionPropertyMask". It's a debug feature (not a production feature), to my understanding anyway. It's disabled by default (per DEC file), and OVMF does not enable it. See also commit 90f3922b018e ("OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing", 2017-10-11). Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48378): https://edk2.groups.io/g/devel/message/48378 Mute This Topic: https://groups.io/mt/34203543/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
