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

Reply via email to