On 07/06/16 22:07, Andrew Fish wrote: > >> On Jul 6, 2016, at 1:02 PM, Laszlo Ersek <[email protected]> wrote: >> >> On 07/06/16 21:01, Kinney, Michael D wrote: >>> Laszlo, >>> >>> Yes. It is possible to use CPU MP PPI on S3 resume. >>> >>> Jeff and I have been working on some proposed changes to the >>> CPU DXE and CPU PEIM modules to share the MP source code in an >>> MP library and eliminate the code duplication between the current >>> DXE driver and PEIM. >> >> Sounds great! >> >>> We will review the issue you are >>> experiencing in the current implementation under OVMF and will >>> make sure OVMF works for normal and S3 resume boot paths. >> >> Thank you! >> >>> In general, we prefer to not use permanent memory allocations >>> for the AP wake vectors. If possible, we prefer to temporarily >>> "borrow" a page a memory below 1MB for the Startup IPI. >> >> The backup buffer needs to be allocated in AcpiNVS memory though, does >> it not? (Wherever the borrowed page is stashed temporarily should be >> reserved from the OS.) So maybe the startup buffer could be placed in >> AcpiNVS immediately. >> >> (Admittedly, these approaches might differ in how many pages need to be >> multiplied by NumProcessors -- I don't know.) >> > > Laszlo, > > On S3 all the PEI memory is in AcpiNVS so a normal allocation should > work? EFI_PEI_SERVICES.AllocatePages(), > EFI_PEI_SERVICES.AllocatePool(), and not to mention the stack.
Yes, that is right. This is precisely the argument that we make in OVMF's PlatformPei, when we install the permanent PEI RAM -- which on the normal boot path was reserved as AcpiNVS -- and don't produce any resource descriptor HOBs. However, for the purposes of CpuMpPei, PEI's pool and page allocation services, and the stack, are not appropriate. CpuMpPei needs a memory allocation strictly below 1MB (so that the APs can be started up in real mode). The PEI page allocation service (unlike the similar UEFI boot service) doesn't seem to offer an "allocate under maximum address" allocation mode. (Even if it did, the permanent PEI RAM that we install in OVMF on the S3 resume path is entirely over 1MB.) So what CpuMpPei does at the moment is: it scans the memory resource descriptor HOBs for system memory, and when it finds a suitable one (one that has enough size under 1MB), it places the buffer there. It stashes the data that is originally there into a backup buffer that *is* in permanent PEI RAM (allocated with the PEI serives), and then uses the low buffer for starting up the APs. The immediate problem is that OVMF's Platform PEIM does not produce any resource descriptor HOBs at S3 resume, so the scanning / filtering comes up empty in CpuMpPei. The above method could work nicely, if: - OVMF could somehow tell CpuMpPei to forego the scan, and use a fixed low address instead, for the "borrow page", and - we could ensure that the permanent PEI RAM that OVMF installs at S3 resume is big enough for CpuMpPei's needs (= the backup buffer, and everything else I don't know about yet). In a nutshell -- and I'm sorry about the wall of text -- the PEI allocation services are not used because they can't guarantee memory under 1MB, which is needed for AP startup on x86. Thanks Laszlo > Thanks, > > Andrew Fish > >> I'll stay tuned. :) >> >> Thanks! >> Laszlo >> >> >>> >>> Best regards, >>> >>> Mike >>> >>>> -----Original Message----- >>>> From: Laszlo Ersek [mailto:[email protected]] >>>> Sent: Wednesday, July 6, 2016 11:52 AM >>>> To: Fan, Jeff <[email protected]>; Kinney, Michael D >>>> <[email protected]>; >>>> Justen, Jordan L <[email protected]> >>>> Cc: Paolo Bonzini <[email protected]>; edk2-devel-01 >>>> <[email protected]> >>>> Subject: Re: [edk2] multiprocessing in PEI? >>>> >>>> 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 >> [email protected] >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

