On 26 January 2016 at 14:19, Laszlo Ersek <[email protected]> wrote: > On 01/26/16 10:40, Leif Lindholm wrote: >> On Tue, Jan 26, 2016 at 09:12:39AM +0000, Ryan Harkin wrote: >>>> However there seems to be >>>> some interference from "ArmPlatformPkg/Library/PlatformIntelBdsLib" that >>>> is supposed to connect PcdDefaultConInPaths and PcdDefaultConOutPaths. >>>> In order to experiment with F9, Roy massaged the last node of the >>>> terminal device path (for controlling the parsing of the escape >>>> sequences). But, some of those cases proved untestable. >>>> >>>> Unfortunately, I can't really help with >>>> "ArmPlatformPkg/Library/PlatformIntelBdsLib". ArmVirtPkg used that >>>> library instance as well originally, but I practically rewrote it >>>> separately for ArmVirtPkg, in SVN r16923 and r16924. Since then, the two >>>> instances have lived separate lives. >>> >>> Is this wise? It looks like BDS history repeating itself :-/ >> >> Well, medium-term, I'd (optimally) like to see ArmVirtPkg split and go >> into OvmfPkg one end and ArmPkg the other. >> >> But when Laszlo started out with ArmVirtPkg (before it indeed _was_ >> ArmVirtPkg), he felt the need to make substantial changes to >> PlatformIntelBdsLib, and opted to do it separately. >> We should probably take some time to look into what different needs we >> may actually have on physical rather than virtual platforms, and how >> we can go back to sharing the majority of the code. > > ** The main goals with the divergence visible in console setup were: > > - cut any ties with the ARM BDS > > (because, ArmPlatformPkg/Library/PlatformIntelBdsLib, despite being > an instance of PlatformBdsLib, to be plugged into the Intel BDS > driver, still uses ARM BDS related PCDs) > > - make console detection dynamic > > (think USB keyboards, and VGA cards at non-fixed PCI addresses) > > IIRC, I had also experienced issues with > "ArmPlatformPkg/Library/PlatformIntelBdsLib" whenever a virtual machine > would start up with a brand new (= empty) variable store. For a physical > host this practically never happens, for a virtual machine, it can > frequently happen (you lose the varstore file or decide to delete & > recreate it from scratch). I vaguely recall that in such cases the > serial console wouldn't work at the first startup. (By now I'm fuzzy on > this, sorry.) > > Another example for the obscure console problems can be seen in SVN > r16912 (that patch predates the rewrite). > > The mailing list thread can be found at > > http://thread.gmane.org/gmane.comp.bios.tianocore.devel/12954 > > I do believe my "going separate ways" was justified. At that time > ArmPlatformPkg had no PCI support at all, but ArmVirtualizationPkg did. > Both the VGA and USB controllers (all types of them), emulated by QEMU, > were PCI-based. So dynamic console detection made no sense for FVP, > Foundation, let alone the physical ARM machines, that ArmPlatformPkg > targeted at the time. > > ... The device path of the serial console would not become dynamic; it > remained build-time static. I modified how it was represented though. > First, the ARM-related textual PCDs with the device path(s) in them were > an ARM BDS vestige. (See the first goal above.) Second, they are brittle > to configure for the end user; do we really want to construct > VenMsg(guid)-like strings manually? Typos are the norm, not the > exception. Third, parsing them into binary form in the code is a chore. > Because, do you do if there is indeed a typo and parsing fails? > > I stand by my idea (of patch #3 in the above-linked series) to encode > the device path of the serial console with a fixed binary structure in > the code. > > Later we would customize this for TTYTERM with SVN r17898. That would > still not require the user to edit device paths, just to pass a build > option. > > Finally, the rewrite gave the virt PlatformBdsLib a much-needed > structural cleanup (in my opinion anyway). It is now as minimal as > possible, and every line in there comes with its own justification. > Which is not something I thought of the pre-rewrite code. > > > ** Further features that only make sense for virtual machines (not an > exhaustive list, just what I can think of now immediately): > > - direct kernel boot from QEMU's fw_cfg interface > (see TryRunningQemuKernel() / "QemuKernel.c" / SVN r16578) > > - controlling UEFI boot order from the QEMU command line > (see SVN r16576, and the patches leading up to it) > > ... In fact, these last two features were implemented in one series, > chronologically *first*, and ultimately they are *the* reason for > splitting away from ArmPlatformPkg: > > http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11743/focus=11746 > > > ** Note that Ard and myself have not neglected keeping ArmPlatformPkg's > and ArmVirtPkg's PlatformBdsLibs in sync, whenever it made sense. (For > one, the Xen build of ArmVirtPkg still uses ArmPlatformPkg's > PlatformBdsLib!) For example, compare: > > - SVN r17713 > ArmVirtPkg: signal EndOxDxe event in PlatformBsdInit > > - SVN r18394 > ArmPlatformPkg: signal EndOfDxe event in PlatformBsdInit > > see also the discussion: > http://thread.gmane.org/gmane.comp.bios.edk2.devel/1849/focus=1916 > >
I wasn't criticising the features you've implemented, I hope it didn't seem that way! Mostly, I'm worried that you've made loads of improvements that sounds like things I would love to have, but don't get because we're not using common code. But moving Juno and FVP to Intel BDS leaves me with more questions than answers. I still haven't worked out how I'm going to boot my system with this jumble of stuff. So far, getting EDK2 to boot itself has been a small victory. > ** As for splitting up ArmVirtPkg and distributing it between > ArmPlatformPkg and OvmfPkg -- I think ArmVirtPkg is already as minimal > as possible. It pulls in a large number of libraries and drivers from > both ArmPlatformPkg and OvmfPkg. Speaking of the QEMU build, it only has > three drivers of its own (HighMemDxe, PciHostBridgeDxe, VirtFdtDxe). It > does customize a larger number of library instances, but library > instances are meant just for that. > > I'm not opposing this on principle, mind you, but I've been pretty > pleased with the current structure, so I'll have to see concrete ideas > to set my mind to a different track. :) > > Thanks! > Laszlo > >> >> / >> Leif >> > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
