Thanks a lot Andrew for the followup! I'll make a few comments:

On 08/10/15 23:38, Andrew Fish wrote:
> 
>> On Aug 10, 2015, at 1:51 PM, Benjamin Herrenschmidt
>> <b...@kernel.crashing.org <mailto:b...@kernel.crashing.org>> wrote:
>>
>> On Mon, 2015-08-10 at 20:14 +0200, Laszlo Ersek wrote:
>>> On 08/10/15 18:46, Andrew Fish wrote:
>>
>>>>> Essentially, I am on a platform/architecture which feeds a flat
>>>>> device-tree to EDK and I need to honor the input reserved regions
>>>>> as they cover bits of memory either already used by the HW for various
>>>>> reasons or are occupied with another firmware that I'm not yet ready to
>>>>> kill :-)
>>>>>
>>>>
>>>> You can’t pass in an allocation, so you can only control things from the
>>>> GCD point of view. See CoreInitializeMemoryServices() in
>>>> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
>>>>
>>>> And that is done via resource descriptor HOBs, so you can make a
>>>> reserved range, or not tell the DXE core that that address range is in
>>>> memory.
>>>>
>>>> If you want to pre-allocate a specific range with a specific EFI memory
>>>> type you may need to add a DXE driver that runs 1st
>>>
>>> I'm confused.
>>>
> 
> Sorry I was confusing!
> 
>>> What is BuildMemoryAllocationHob() / EFI_HOB_TYPE_MEMORY_ALLOCATION good
>>> for, if not for passing in a memory allocation at a fixed address from
>>> PEI to DXE?
>>>
> 
> Yes EFI_HOB_TYPE_MEMORY_ALLOCATION should work. 
> 
> I was concerned about overlaps not being covered by I realized that case
> should be handled. 
> For example the DXE IPL does BuildModuleHob() for the DXE Core, and that
> creates a EFI_HOB_TYPE_MEMORY_ALLOCATION HOB of type EfiBootServicesCode. 
> 
> From a PEI perspective AllocatePages, allocates memory that is handed
> off to DXE, and AllocatePool allocates memory that is thrown away in DXE.

Yes, we're aware of that for example in
"ArmVirtPkg/Library/PlatformPeiLib". It "relocates" the initial DTB that
QEMU places initially at the zero address. The module depends on
gEfiPeiMemoryDiscoveredPpiGuid (ie. permanent PEI RAM being available)
and grabs the "new home" for the DTB with AllocatePages(). (Which ends
up in PeiAllocatePages()

>>> - CoreInitializeMemoryServices() indeed doesn't look at
>>>  EFI_HOB_TYPE_MEMORY_ALLOCATION, but CoreInitializeGcdServices() does,
>>>  and it's called by the DXE core right after.
>>>
>>> - The PI spec's description of EFI_HOB_TYPE_MEMORY_ALLOCATION seems to
>>>  imply to me that this HOB type exists exactly for pre-allocating
>>>  memory during PEI.
>>>
>>> - The comments in CoreInitializeGcdServices() -- the function that
>>>  traverses EFI_HOB_TYPE_MEMORY_ALLOCATION HOBs -- say,
>>>
>>>  //
>>>  // Walk the HOB list and allocate all memory space that is consumed by
>>>  // memory allocation HOBs, and Firmware Volume HOBs.  Also update the
>>>  // EFI Memory Map with the memory allocation HOBs.
>>>  //
>>>
>>>  and later
>>>
>>>  //
>>>  // Add and allocate the remaining unallocated system memory to the
>>>  // memory services.
>>>  //
>>>
>>> If EFI_HOB_TYPE_MEMORY_ALLOCATION, without a specific AllocDescriptor
>>> GUID, cannot be used for passing a memory allocation at a fixed address
>>> from PEI to DXE, with a particular EFI_MEMORY_TYPE, then
>>>
>>> - Why does it exist?
>>>
>>> - Why does the PI spec say,
>>>
>>>    The HOB consumer phase reads all memory allocation HOBs and
>>>    allocates memory into the system memory map based on the following
>>>    fields of EFI_HOB_MEMORY_ALLOCATION_HEADER of each memory
>>>    allocation HOB:
>>>
>>>    * MemoryBaseAddress
>>>    * MemoryLength
>>>    * MemoryType
>>>
>>> - Why does CoreInitializeGcdServices() implement the following
>>>  algorithm:
>>>  * for each memalloc HOB, look up the containing GCD range
>>>  * in the GCD memory space map, flip the covered sub-range to
>>>    "allocated"
>>>  * if the GCD memory type that the range is being carved out of is
>>>    System Memory or More Reliable, add the range to the UEFI memmap too
>>>  ?
>>>
>>> ... I grepped the tree for BuildMemoryAllocationHob(). Aside from
>>> ArmVirtPkg and OvmfPkg, at least the following SEC & PEI modules use it
>>> in the above sense, apparently:
>>>
>>> - BeagleBoardPkg/Sec/Cache.c: covers the ARM MMU translation table with
>>>  EfiBootServicesData
>>>
>>> - CorebootModulePkg/CbSupportPei/CbSupportPei.c: covers the Coreboot
>>>  payload FD (ie. itself) with EfiBootServicesData
>>>
>>> - UefiCpuPkg/CpuMpPei/CpuMpPei.c: allocates the "wake up buffer" by
>>>  manually searching the memory resource descriptor HOBs for a suitably
>>>  low & big one, and covering part of it with an EfiBootServicesData
>>>  memalloc HOB.
>>>
>>> Are these all wrong? (Or am I misunderstanding all of them?)
>>
>> I though the same thing but I can't see how anything prevents the few
>> allocations done between CoreInitializeMemoryServices() and
>> CoreInitializeGcdServices() from overwriting any of those reservations.
>>
>> It looks like by default, CoreInitializeMemoryServices() will use the
>> remaining free space in the PHIT which will be fine, but it has the
>> potential of going elsewhere for that initial memory pool, and when that
>> happens, anything in the resource descriptors appears to be "game”.
> 
> But if you had a real PEI core then the PHIT info would ensure the DXE
> Core  "did the right thing”…. 
> 
> The
> CoreInitializeMemoryServices() 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Gcd/Gcd.c,
> is going to start allocating from the resource descriptor HOB that
> contains the PHIT range. If you are actually running PEI than that would
> a range outside of where holes are getting punched. If you are doing the
> PEI less thing then there could be issues with some of the initial
> ranges. You need to make sure the ranges in the PHIT hob don’t overlap
> with your magic ranges. It might also be a good idea to have separate
> resource descriptor HOBs for the ranges. 

Thanks for explaining this. In both (sets of) platforms that I'm closely
following (ArmVirtPkg/ArmVirtQemu.dsc and OvmfPkg/OvmfPkg*.dsc) the
"real" PEI core is used. And, the memory allocation HOBs that we build
(in either platform) overlap with neither the temporary SEC/PEI
core+stack nor the permanent PEI memory.

(Ensuring this predicate was one of the hardest things in both (sets of)
platforms. I recall several iterations of reviews + updates when Ard was
porting ArmPlatformPkg PEIMs to QEMU. And in OvmfPkg we validated the
same role code several times after introducing it originally for S3; for
me the two most memorable "take a step back and go through it from zero"
occasions being (a) documenting it in minute detail in the OVMF
whitepaper, (b) going through all of it again for the SMM series
(because the TSEG range affected it).)

> I think this code should explain the constraints of your fake PHIT hob….
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
>     //
>     // Compute range between PHIT EfiFreeMemoryTop and the end of the
> Resource Descriptor HOB
>     //

This comment in the code is wrong (it's not EfiFreeMemoryTop but
EfiMemoryTop), but the code actually uses the right field. (Figure 11 in
the PI spec vol 3 is very helpful here.)

>     Attributes  = PhitResourceHob->ResourceAttribute;
>     BaseAddress = PageAlignAddress (PhitHob->EfiMemoryTop);
>     Length      = PageAlignLength  (ResourceHob->PhysicalStart +
> ResourceHob->ResourceLength - BaseAddress);
>     if (Length < MINIMUM_INITIAL_MEMORY_SIZE) {
>       //
>       // If that range is not large enough to intialize the DXE Core, then 
>       // Compute range between PHIT EfiFreeMemoryBottom and PHIT
> EfiFreeMemoryTop
>       //
>       BaseAddress = PageAlignAddress (PhitHob->EfiFreeMemoryBottom);
>       Length      = PageAlignLength  (PhitHob->EfiFreeMemoryTop -
> BaseAddress);
>       if (Length < MINIMUM_INITIAL_MEMORY_SIZE) {
>         //
>         // If that range is not large enough to intialize the DXE Core,
> then 
>         // Compute range between the start of the Resource Descriptor
> HOB and the start of the HOB List
>         //
>         BaseAddress = PageAlignAddress (ResourceHob->PhysicalStart);
>         Length      = PageAlignLength  ((UINT64)((UINTN)*HobStart -
> BaseAddress));
>       }
>     }
>     break;
>   }

So practically:
(1) if the memory resource descriptor that the free page area of the
permanent PEI RAM comes from has enough room "above" the permanent PEI
RAM, then the DXE core is tentatively targeted there.
(2) otherwise, if the free page area itself has enough room left, then
the DXE core is tentatively targeted there.
(3) otherwise, if the memory resource descriptor that the free page area
of the permanent PEI RAM comes from has enough room "under" the HOB
list, then the DXE core is tentatively targeted there.

(4) Then, with the next loop, if the highest-located memory resource
descriptor that is (a) tested, (b) big enough for the DXE core, and (c)
different from the descriptor identified above, is actually higher than
the tentative range determined above, then the DXE core goes up there.

In OVMF (4) doesn't come into play because the >= 4GB memory is added as
untested. (But, in any case, we don't pre-allocate with HOBs that high.)
And, from (1) through (3), (2) applies in OVMF; our permanent PEI RAM is
located at the top of the < 4GB memory, so the "above" room checked in
(1) is zero.

... I added some ASSERT()s to this function, and yes, it's (2) for OVMF.

My faith in world order has been restored. Apologies for hijacking Ben's
thread temporarily, but you got me panicked for a minute.

Thanks
Laszlo

> 
> 
> 
> The DXE Core should print out some info to help you debug. 
> 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
>   DEBUG ((DEBUG_INFO | DEBUG_LOAD, "HOBLIST address in DXE = 0x%p\n",
> HobStart));
> 
>   DEBUG_CODE_BEGIN ();
>     EFI_PEI_HOB_POINTERS               Hob;
> 
>     for (Hob.Raw = HobStart; !END_OF_HOB_LIST(Hob); Hob.Raw =
> GET_NEXT_HOB(Hob)) {
>       if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_MEMORY_ALLOCATION) {
>         DEBUG ((DEBUG_INFO | DEBUG_LOAD, "Memory Allocation 0x%08x
> 0x%0lx - 0x%0lx\n", \
>           Hob.MemoryAllocation->AllocDescriptor.MemoryType,            
>          \
>           Hob.MemoryAllocation->AllocDescriptor.MemoryBaseAddress,      
>         \
>           Hob.MemoryAllocation->AllocDescriptor.MemoryBaseAddress +
> Hob.MemoryAllocation->AllocDescriptor.MemoryLength - 1));
>       }
>     }
>     for (Hob.Raw = HobStart; !END_OF_HOB_LIST(Hob); Hob.Raw =
> GET_NEXT_HOB(Hob)) {
>       if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV2) {
>         DEBUG ((DEBUG_INFO | DEBUG_LOAD, "FV2 Hob           0x%0lx -
> 0x%0lx\n", Hob.FirmwareVolume2->BaseAddress,
> Hob.FirmwareVolume2->BaseAddress + Hob.FirmwareVolume2->Length - 1));
>       } else if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV) {
>         DEBUG ((DEBUG_INFO | DEBUG_LOAD, "FV Hob            0x%0lx -
> 0x%0lx\n", Hob.FirmwareVolume->BaseAddress,
> Hob.FirmwareVolume->BaseAddress + Hob.FirmwareVolume->Length - 1));
>       }
>     }
>   DEBUG_CODE_END ();
> 
> 
> 
> Thanks,
> 
> Andrew Fish
> 
> 
>> Cheers,
>> Ben.
>>
>>> Thanks
>>> Laszlo
>>>
>>>> (Depex TRUE, and
>>>> Apriori file to make it run 1st) and have it do:
>>>>
>>>> Memory = ADDRESS_YOU_WANT_RESERVE:
>>>> Status = gBS->AllocatePages (AllocateAddress, MemoryType,
>>>> NumberOfMpages, &Memory);
>>>>
>>>> If the PEI code knows the ranges it can pass up a GUIDed hob that the
>>>> DXE driver uses to know the ranges, or maybe that knowledge can just end
>>>> up in the DXE driver.
>>>>
>>>> Given the way the edk2 works you could all do this in a library that is
>>>> linked into the DXE Core. As long as the library has a constructor it
>>>> will be called by the DXE Core via a call to
>>>> ProcessLibraryConstructorList().
>>>>
>>>> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
>>>>  //
>>>>  // Initialize the Global Coherency Domain Services
>>>>  //
>>>>  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress,
>>>> MemoryLength);
>>>>  ASSERT_EFI_ERROR (Status);
>>>>
>>>>  //
>>>>  // Call constructor for all libraries
>>>>  //
>>>>  ProcessLibraryConstructorList (gDxeCoreImageHandle, gDxeCoreST);
>>>>  PERF_END   (NULL,"PEI", NULL, 0) ;
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Andrew Fish
>>>>
>>
>>
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to