> On Jan 14, 2020, at 8:46 AM, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote:
> 
> On Mon, 13 Jan 2020 at 18:57, Andrew Fish via Groups.Io
> <afish=apple....@groups.io> wrote:
>> 
>> 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?
>> 
> 
> I'm not sure I follow. Do you mean update the console variable if it
> doesn't contain the GOP produced by the Driver#### option and reboot?
> 

Ard,

I was thinking this specific case was no active GOP driver in the system due to 
the emulation of the ROM. I was saying there could be a platform policy to 
require GOP, and there could be a  platform hook point to check for no GOP and 
take action. This is not really a Driver#### path, but a fallback path for no 
GOP. So that is kind of cheating :). 

Also I was thinking the ConSpliter would activate the new GOP if you do a 2nd 
gBS->ConnectController on the Graphics PCI or GOP handle after updating the 
console nvram variable. 

Sorry I'm not really familiar with the modern TianoCore BDS, as we have a 
custom BDS on Macs, so I'm talking more hypothetically about how BDS could work.

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> 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 (#53242): https://edk2.groups.io/g/devel/message/53242
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]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to