On 7 April 2015 at 16:11, Laszlo Ersek <ler...@redhat.com> wrote: > 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. >
This is because of the direct dependency on the .c file rather than the library class, right? >> 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. > Actually, I discussed this with Olivier, and it appears that the whole logic there is not needed in the first place. Perhaps Olivier can comment, but it seems removing the RAM is not necessary anymore for shadowed flash, and it is certainly not necessary for XIP flash such as what we use for the QEMU target. IOW, we could 1) fix the library dependency 2) switch the QEMU target to the current virt version 3) add the cache invalidation to it, so that both the QEMU and Xen targets benefit from it which would involve two additional patches. > 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. > Let us figure out what's going on with MemoryInitPeiLib first, and then I'll propose an appropriate solution -- Ard. ------------------------------------------------------------------------------ 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
