Ard, Is the problem GFX console? Would it be possible to have a PCD to assume graphics console, and if non was found on the boot connect those PCI devices and update the NVRAM to cause a console to connect. You might have to do a 2nd connect on the GOP handle after the nvram variable was written to make the ConSpliter see it?
Thanks, Andrew Fish > On Jan 13, 2020, at 9:28 AM, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote: > > On Fri, 10 Jan 2020 at 17:23, Laszlo Ersek <ler...@redhat.com > <mailto:ler...@redhat.com>> wrote: >> >> On 01/10/20 15:37, Ni, Ray wrote: >>> Ard, >>> I understand now that: BeforeConsole() needs to connect Gfx to get the GOP >>> device path >>> for ConOut updating. But the GOP driver is specified in Driver#### and it >>> will be loaded after >>> BeforeConsole() in today's code. This order makes the Gfx connection fails >>> to start the GOP driver >>> resulting ConOut not updated. >>> >>> Moving Driver#### processing before BeforeConsole() helps in your case. But >>> it may impact >>> other platforms: There might be platforms that update Driver#### variables >>> in BeforeConsole() >>> and expect these drivers are processed in the same boot (not the next >>> boot). I don't have the real >>> examples. But today's flow allows this. With your change, Driver#### will >>> not be processed in the first >>> boot. The change impacts these platforms. >>> >>> One backward compatible approach can be: processing Driver#### before >>> BeforeConsole() and processing the new Driver#### (added by >>> BeforeConsole()) after BeforeConsole(). >>> >>> But another problem arises: If BeforeConsole() removes a Driver####, >>> platform's expectation is that deleted Driver#### doesn't run. But that >>> driver already runs. >>> >>> So actually the question is: when BDS core can consume the Driver#### and >>> process them? >>> Right now, it’s the time after BeforeConsole(). Just as right now >>> BeforeConsole() updates ConOut >>> in your case, BeforeConsole() is the place where platform updates all UEFI >>> defined boot related >>> variables. Processing Driver#### before BeforeConsole() requires platform >>> updates Driver#### >>> in other places. It's a new requirement to the platform. >>> >>> With all what I explained in above, I cannot agree with the changes. >>> >>> Another approach is: >>> Platform could connect the GFX in AfterConsole() and update the ConOut. >>> Then platform could manually call EfiBootManagerConnectConsoleVariable >>> (ConOut) to re-connect ConOut. >>> It's a bit ugly I agree. >> >> Let me raise three other ideas (alternatives to each other, and to the >> above), with varying levels of annoyance. :) >> > > Thanks Laszlo > > Ray, given your objection to my approach, could you please consider > the below and give feedback on which approach is suitable to address > the issue I am trying to fix? > >> >> (1) Keep the following logic (i.e. the subject of this patch): >> >> // >> // Execute Driver Options >> // >> LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, >> LoadOptionTypeDriver); >> ProcessLoadOptions (LoadOptions, LoadOptionCount); >> EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); >> >> in *both* places, but gate each one with a bit in a new bitmask PCD. >> >> (Note: it's probably not the best for any platform to permit both branches, >> as driver images would be loaded twice.) >> >> >> (2) EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL has recently been added to edk2. It >> looks like a well-designed (extensible) protocol, for two reasons: >> >> - the protocol structure has a Revision field, >> >> - the only current member function, RefreshAllBootOptions(), is permitted to >> return EFI_UNSUPPORTED -- and the single call site, in the >> EfiBootManagerRefreshAllBootOption() function, handles that return value >> well (it does nothing). >> >> The idea would be to bump the Revision field, and add a new member function. >> Then call this member function (if it exists) in the spot where the current >> patch proposes to move the Driver#### dispatch logic to. >> >> This is almost like a new PlatformBootManagerLib interface, except it does >> not require existent lib instances to be updated. >> >> And, on the EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL implementation side, >> RefreshAllBootOptions() would return EFI_UNSUPPORTED. (Because that is >> irrelevant to Ard's use case.) >> >> Perhaps add yet another member function that can disable the Driver#### >> option processing in the current location. >> >> >> (3) Extend the UEFI specification, section "3.1.3 Load Options". >> >> The LOAD_OPTION_CATEGORY bit-field is almost what we want, except it's >> specified to be ignored for Driver#### options. So, >> >> - either invent the standardese to let us use LOAD_OPTION_CATEGORY for >> Driver#### options too, without breaking existing platforms, *or* >> - introduce a new (not too wide) distinct bitfield called >> LOAD_OPTION_DRIVER_CATEGORY. >> >> Whichever category bitfield proves acceptable, introduce a new "driver >> category bit" in that bitfield, called LOAD_OPTION_DRIVER_CATEGORY_CONSOLE. >> >> Specify that, if any Driver#### option has the >> LOAD_OPTION_DRIVER_CATEGORY_CONSOLE attribute set, then Driver#### options >> will be processed in two passes. In both passes, DriverOrder is to be >> honored, but the first pass will only consider options with the attribute >> set, and the second pass will only consider options with the attribute clear. >> >> Implement this in BdsEntry / UefiBootManagerLib. >> >> ... Maybe don't even introduce LOAD_OPTION_DRIVER_CATEGORY*; just a bitfield >> that is expressly vendor specific? >> >> Thanks >> Laszlo >> > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#53195): https://edk2.groups.io/g/devel/message/53195 Mute This Topic: https://groups.io/mt/67470372/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
