On 04/07/15 16:16, Ard Biesheuvel wrote:
> 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?
Correct.
>>> 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.
If your premise is correct (about removing RAM), then I agree with your
corollary. :)
>> 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
Heh, yeah, that's the hard part. For now I can only speak in
generalities ("memory resource descriptor HOBs should not cover the
flash range" etc). I'll have to get back to this thread after I crawl
out from under the heap of emails that awaited me after my short Easter
vacation.
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
