On 07/06/16 19:08, Laszlo Ersek wrote: > Hi Jeff, > > On 07/05/16 15:50, Fan, Jeff wrote: >> When CPU MP PPI installed, CPU MP PPI Services will be fully usable. > > I included CpuMpPei in OVMF, and it works fine on the normal boot path. > (I haven't done anything special with it thus far, just built it into > the firmware, and it starts up fine.) > > However, on the S3 resume path, it runs into an assertion failure: the > GetWakeupBuffer() function fails. That is actually expected, because at > that point the runtime OS being resumed owns all memory under 1MB. > > On the S3 resume path, OVMF does install a tiny bit of permanent PEI > RAM: 32 KB in size (PcdS3AcpiReservedMemorySize), starting at 8288 KB > (PcdS3AcpiReservedMemoryBase). On the normal boot path, we reserve this > area as AcpiNVS type memory, so that the runtime OS stays out of it. > > Is CpuMpPei meant to be used on the S3 resume path in general?
It looks like CpuMpPei expects memory resource descriptor HOBs to exist on the S3 resume path as well. Then it seems to lay the wakeup buffer over OS-owned memory under 1MB. But, it saves the original memory contents into a backup buffer allocated from permanent PEI RAM. And, at end-of-PEI, the memory contents are restored. OVMF does not produce memory / IO / MMIO resource descriptors on the S3 resume path. The assumption is that no PEIM will directly look for any such HOBs on the S3 path, only use the permament PEI RAM for memory allocation. Does this conflict with the PI spec? I wonder if GetWakeupBuffer()'s scanning of EFI_RESOURCE_SYSTEM_MEMORY descriptor HOBs is a good idea. When it finds a location that is suitable as a wakeup buffer, it "claims" that area by building a memory allocation HOB that covers the area. I think that's fine; however, the scanning itself does not consider any other memory allocation HOBs that may have been carved out of system memory by other PEIMs, earlier. I wonder if platforms would be better served by a (base, size) pair of PCDs. The platform could set these PCDs, and make sure that both the OS and other firmware modules (PEIMs and DXE drivers) stay out of this area. (For example, if S3 is not supported or enabled on the platform, then the platform could allocate this area as Boot Services Data. Otherwise, it would be AcpiNVS.) CpuMpPei could then remove the resource HOB scanning, and it would only need to validate if (a) the size PCD was big enough to contain the wakeup buffer, and (b) the base PCD was low enough to keep the wakeup buffer under 1MB, (c) any necessary alignments. Basically, ask the platform to preallocate the wakeup buffer. (The minimal suitable size can be found statically, by trial and error.) The backup buffer (in permanent PEI RAM) could also be dropped, decreasing footprint. What do you think? Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
