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. :)


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