On 07/07/16 10:56, Fan, Jeff wrote:
> Laszlo,
>
> Any PEI module cannot allocate the memory < 1MB range if using PEI memory
> allocation services only.
Okay.
> If other PEI module wants to allocate the memory by scanning memory resource
> HOB, it has to follow the same rule with the CpuMpPei driver.
Okay.
> If platform build allocation HOB for the whole range < 1MB, how to avoid
> memory conflicts between CpuMpPei and other PEI modules who is scanning
> memory resource HOB?
You are right about this, in theory.
Your suggestion is that any PEIM that needs memory under 1MB look for
memory HOBs, and stay away from memory allocation HOBs. In other words,
to build a map from memory HOBs, subtract the memory allocation HOBs,
grab a chunk from what remains, and then cover that chunk with another
allocation HOB. This will ensure complete understanding between the
platfom PEIM, CpuMpPei, and any other PEIMs.
I agree completely, except for the fact that in practice, CpuMpPei
currently *ignores* any memory allocation HOBs!
CpuMpPei creates a memalloc HOB for the chunk that it ultimately
borrows, yes -- but how can CpuMpPei expect other PEIMs to honor its
memalloc HOB when CpuMpPei ignores memalloc HOBs from other PEIMs?
In other words, I can agree that OVMF's Platform PEIM should not create
a memalloc HOB (only the memory HOB) for 0..640KB, but *only if*
CpuMpPei is patched to honor preexistent memalloc HOBs in GetWakeupBuffer().
Then we can say we have a pattern that all PEIMs are expected to follow:
(1) memory can be allocated for any use with the PEI services
(2) memory can be allocated under 1MB by
(2a) looking for memory HOBs
(2b) *and* honoring existing memalloc HOBs.
(2c) The memory taken this way should be covered by the consumer
with another memalloc HOB.
(2d) Also, if allocation occurs on the S3 resume path, then the
original contents must be restored at End-of-PEI.
This method is safe for all PEIMs to follow, but CpuMpPei itself does
not comply with it (step (2b)).
Thanks
Laszlo
>
> Jeff
>
> -----Original Message-----
> From: Laszlo Ersek [mailto:[email protected]]
> Sent: Thursday, July 07, 2016 4:48 PM
> To: Fan, Jeff; Kinney, Michael D; Justen, Jordan L
> Cc: Paolo Bonzini; edk2-devel-01
> Subject: Re: [edk2] multiprocessing in PEI?
>
> On 07/07/16 10:29, Fan, Jeff wrote:
>> Laszlo,
>>
>> Actually, CpuMpPei has no knowledge to know which memory range < 1MB is
>> available or not. That's why we cannot assume one hardcode address < 1MB for
>> borrowing.
>
> I agree that this makes sense.
>
>>
>> Platform has such knowledge and need one mechanism to tell CpuMpPei.
>
> I agree 100%.
>
>> We choose Memory Resource HOB here.
>
> I believe this is not a perfect choice.
>
> The information that the memory resource HOB conveys can be unwittingly
> mis-used by other PEIMs. The information from the platform is good, but it is
> not targeted / restricted well enough.
>
> As I wrote in my other email, the restriction should be
>
> available for temporary use, but only if the original contents are
> restored at End-of-PEI.
>
> CpuMpPei certainly complies with this restriction, but other PEIMs won't
> realize it just from looking at a memory resource HOB.
>
> So, as a workaround, I have to install a memory allocation HOB as well, in
> OVMF's Platform PEIM. CpuMpPei will ignore the memalloc HOB (and take care to
> restore the borrowed area at End-of-PEI). And any other PEIMs should steer
> clear of the area because of the memalloc HOB.
>
> Thanks
> Laszlo
>
>>
>> Thanks!
>> Jeff
>>
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:[email protected]]
>> Sent: Thursday, July 07, 2016 3:04 PM
>> To: Fan, Jeff; Kinney, Michael D; Justen, Jordan L
>> Cc: Paolo Bonzini; edk2-devel-01
>> Subject: Re: [edk2] multiprocessing in PEI?
>>
>> On 07/07/16 04:52, Fan, Jeff wrote:
>>> Laszlo,
>>>
>>> On PEI phase, whatever it is normal boot or S3 boot, PEI memory allocation
>>> services should not allocate memory < 1 MB address.
>>>
>>> If any other PEIM module wants to allocate memory < 1 MB address, it has to
>>> scan Memory resource HOB to find the available memory range and build
>>> memory allocation HOB like what CpuMpPei did.
>>>
>>> If this method is OK, CpuMpPei or other module are required to check
>>> existing both memory resource HOB and memory allocation HOB.
>>>
>>> If we just borrow one piece of range < 1 MB address around each of
>>> INIT-SIPI-SIPI to AP, we still need to get the available memory address.
>>>
>>> So, platform still needs to build memory resource HOB < 1MB on S3 boot like
>>> normal boot. I think this update on platform should be simpler than new
>>> pair of PCDs introducing.
>>>
>>> What do you think of it?
>>
>> * First, at this point I'm now thinking that "borrowing" and "allocating"
>> are actually separate things:
>>
>> (a) For allocating memory under 1MB, scanning both the memory resource
>> HOBs and the memory allocation HOBs would be necessary, yes.
>>
>> (b) However, in case the memory under 1MB is only borrowed (and the
>> backup buffer is allocated with the PEI serivces), the HOB scanning
>> should not be necessary at all (for any kinds of HOBs). CpuMpPei
>> is intimately tied to Intel real mode anyway (hence the < 1MB
>> requirement in the first place), so why can't CpuMpPei just borrow
>> a few pages at a fixed low address (under 640KB)? That memory is
>> required to exist on platforms where CpuMpPei is supposed to run,
>> isn't it?
>>
>> * Second, the reason I'm reluctant to produce the same set of memory
>> resource and alloc HOBs on the S3 resume path as on the normal boot path is
>> the following:
>>
>> In my opinion, that would be misleading. Other PEIMs that execute (a) --
>> that is, allocation and not borrowing -- might think that any memory that is
>> covered by a resource descriptor HOB, but not covered by any allocation
>> HOBs, is "free memory". While in fact that memory could well be used by the
>> OS.
>>
>> In other words, the reason why we don't produce any of these HOBs is exactly
>> to communicate that there is zero free memory *for allocation*, outside of
>> the PEI memory services. Any PEIM that wants to allocate (as opposed to
>> borrow) *should* fail, unless it uses the PEI memory services. And if a PEIM
>> wants to *borrow* (as opposed to allocate), then it shouldn't need to scan
>> HOBs.
>>
>> * If there is a requirement in the PI spec that the HOBs be produced
>> on the S3 path as well, then I guess we'll have to comply with that.
>> With my current understanding though, I feel that producing the same
>> set of HOBs would give other PEIMs an incorrect view of memory
>> ownership. The result of
>>
>> all system memory HOBs minus all allocation HOBs
>>
>> would appear as
>>
>> memory that is free for further allocation
>>
>> which would be false.
>>
>> * So my proposal would be to allow the platform to advertize a range for
>> CpuMpPei to borrow from, with two new PCDs.
>>
>> If you are strongly opposed to that, then I guess I can write a patch for
>> OvmfPkg that, on the S3 resume path, produces one (1) system memory HOB as
>> an *exception*, specifically to allow CpuMpPei's scanning to succeed (with
>> an appropriate code comment of course). I could also cover the entire area
>> in question with a memory allocation HOB as well, hoping that it would keep
>> away other PEIMs that look for "free memory", like in (a). Something like:
>>
>> //
>> // Allow CpuMpPei to borrow memory from this range for the AP startup
>> // code, but prevent other PEIMs from thinking the range is "free
>> // memory".
>> //
>> AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
>> BuildMemoryAllocationHob (0, BASE_512KB + BASE_128KB,
>> EfiBootServicesData);
>>
>> Actually, let me try that, and see where I land.
>>
>> Thanks,
>> Laszlo
>>
>>
>>> -----Original Message-----
>>> From: Laszlo Ersek [mailto:[email protected]]
>>> Sent: Thursday, July 07, 2016 2:52 AM
>>> To: Fan, Jeff; Kinney, Michael D; Justen, Jordan L
>>> Cc: Paolo Bonzini; edk2-devel-01
>>> 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
