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

Reply via email to