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.)

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

Reply via email to