On 11/30/15 12:48, Ard Biesheuvel wrote: > On 30 November 2015 at 12:09, Laszlo Ersek <[email protected]> wrote: >> On 11/30/15 11:03, Ard Biesheuvel wrote: > [...] >>> >>> Couldn't we simply add EFI_RESOURCE_SYSTEM_MEMORY resource descriptor >>> HOBs the first time we enumerate the nodes? >> >> I didn't suggest it for two reasons. >> >> (1) I recalled that last time we had had a very long discussion about how >> the DXE core selects the initial range for memory allocations (for its own >> purposes, primarily). I had trouble remembering all the details now. So >> there were three options for me: >> >> - recommend the HOBs, and hope for the best, i.e., hope whatever the DXE >> core picks will be good for us >> >> - recommend the HOBs, and actually review what the DXE core does (it was >> modified a little bit last time) >> >> - recommend a single HOB (which implies the DXE core gets initialized >> exactly the same as before, no thinking or code browsing needed), and delay >> the installation of the addtional ranges until later. >> >> I picked the last option for simplicity. >> >> (2) The other reason is that I don't think that the HOB approach would solve >> the question of caching attributes. I don't think the DXE core tries to set >> any attributes on its own when the GCD is primed from the HOBs. >> >> Remember we have >> >> InitializeMemory() >> [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c] >> ArmPlatformInitializeSystemMemory() >> [ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c] >> PcdSet64 (PcdSystemMemorySize, ...) >> MemoryPeim() >> [ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c] >> PcdGet64 (PcdSystemMemorySize) >> BuildResourceDescriptorHob() >> InitMmu() >> [ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c] >> ArmPlatformGetVirtualMemoryMap() >> [ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c] >> PcdGet64 (PcdSystemMemorySize) >> ArmConfigureMmu() >> [ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c] >> >> In other words, all of the following happens in the memory init PEIM (which >> comes from ArmPlatformPkg, but it's plugged chock full of our ArmVirtPkg >> library instances): >> - the in-DRAM DTB is initially parsed for the memory node >> - the size PCD is set >> - the base PCD is verified >> - a memory descriptor HOB is built, dependent on the PCDs >> - a virtual memory map is built, with caching attributes, dependent on the >> PCDs >> - the MMU is configured >> >> Therefore we have a nice fast caching setting for the "base" memory node >> only because you cover it explicitly in ArmPlatformGetVirtualMemoryMap() -- >> dependent on the PCDs --, not because the DXE core covers it when it >> processes the HOBs. So if you want to process several memory nodes *for >> real* in the the memory init PEIM, then not only do you have to create the >> HOBs there, but also extend the virtual memory map for the MMU similarly. >> And a fixed count of PCDs won't be enough to carry the information from the >> DTB to ArmPlatformGetVirtualMemoryMap() -- some loop would have to exist >> that connects the DTB with the virtual memory map. >> >> This is what I meant in my original response by "having more flexibility in >> DXE". >> > > Yes, that does sound like a lot of work for little gain. But I am not > too crazy about adding more 'A PRIORI' modules, to avoid ending up in > dependency hell later.
I agree. Unfortunately, the only other possibility I can see is a separate driver that has a DepEx on the CPU architectural protocol, iterates over the FDT again, and installs the memory ranges (including setting the caching). (I also thought about delaying this logic within VirtFdtDxe: set up a protocol notify callback on the CPU arch protocol, and do the same thing in the callback. Unfortunately, this is exactly what the DXE core does too, and the ordering between protocol notify callbacks is not specified. So if ours ran first, there would be trouble.) Take your pick. If you opt for the separate driver / DepEx approach, I can certainly agree with that as well. It would take a bit more boilerplate (separate directory, separate INF file), but it would be very clean, as far as dispatch order and driver responsibility go. Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
