On 3 December 2015 at 12:52, Laszlo Ersek <ler...@redhat.com> wrote: > Ard, > > On 11/30/15 18:48, Laszlo Ersek wrote: >> On 11/30/15 12:48, Ard Biesheuvel wrote: >>> On 30 November 2015 at 12:09, Laszlo Ersek <ler...@redhat.com> 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. > > What would be your preference here? > > Personally I'm leaning towards a separate driver, not dissimilar to > VirtFdtDxe, that goes over the DTB, hunting for the memory nodes only, > adding them and setting the caching attributes as well. (So it would > have the depex.) > > We can think of this driver as a "memory controller driver", except the > only thing it'd have to do is look at the DTB. >
Yes, that would be my preference, especially since you can choose not to include it in your firmware. -- Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
