On 04/07/15 15:14, Ard Biesheuvel wrote: > If we start using the MemoryInitPeiLib under ArmVirtualizationPkg for > QEMU (which was intended by this patch),
Currently there is no executable module in the QEMU build that depends on this library class. > then there is no need IMO to > introduce a feature PCD, since all virt platforms require the cache > invalidation to guarantee correct operation (assuming that, under > virtualization, you never have the guarantee that no system caches are > enabled and you are never migrated to another cpu), and the physical > platforms will keep using the original MemoryInitPeiLib, but as a > library. > > So what I propose instead is to 'fix' the MemoryInitPeiLib,c > dependency in a separate patch, so that the virt targets (including > QEMU) will pull in the correct code. Let me check if I understand: do you propose rebasing ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf to the MemoryInitPeiLib class explicitly? I guess that should work, but then we should also verify that the following two files: ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c have no other differences than the cache invalidation (that is being introduced by this patch of yours). Otherwise, simply switching the class->instance resolution (without the cache invalidation) might cause unintended changes for the QEMU build. Hm, indeed, that seems to be a problem. You introduced "ArmVirtualizationMemoryInitPeiLib" in SVN r16962, and it seems pretty strongly tied to PrePi / Xen. Under QEMU, the guest-phys address range taken up by the pflash chip that carries the firmware binary never becomes reusable by the OS. So, as far as I can see, we need *three* flavors here: - one for PrePi / Xen (including your SVN r16962 *and* the cache invalidation) - one for PrePei / Qemu (*not* including your SVN r16962, but including cache invalidation) - one for physical hardware (stock ArmPlatformPkg version). I guess -- unless I'm wrong of course -- we can do this with three separate library instances, or by sticking with the current two, and customizing the stock one with the FeaturePCD. Thanks Laszlo ------------------------------------------------------------------------------ BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT Develop your own process in accordance with the BPMN 2 standard Learn Process modeling best practices with Bonita BPM through live exercises http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_ source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel