Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

["Ni, Ray" <ray.ni@intel.com>]

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.

Thanks,
Ray


> -----Original Message-----
> From: Ni, Ray
> Sent: Friday, January 10, 2020 9:20 AM
> To: devel@edk2.groups.io; ard.biesheuvel@linaro.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Gao, Zhichao <zhichao.gao@intel.com>; Ma,
> Maurice <maurice.ma@intel.com>; Dong, Guo <guo.dong@intel.com>; You,
> Benjamin <benjamin.you@intel.com>
> Subject: RE: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow
> consoles on drivers loaded via Driver####
> 
> Ard,
> I will give update on this by end of this week. sorry about the delay.
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> Biesheuvel
> > Sent: Thursday, January 9, 2020 8:54 PM
> > To: Ni, Ray <ray.ni@intel.com>
> > Cc: devel@edk2.groups.io; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Gao, Zhichao
> > <zhichao.gao@intel.com>; Ma, Maurice <maurice.ma@intel.com>; Dong, Guo
> <guo.dong@intel.com>; You, Benjamin
> > <benjamin.you@intel.com>
> > Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow
> consoles on drivers loaded via Driver####
> >
> > On Mon, 23 Dec 2019 at 15:09, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
> > >
> > > On Thu, 12 Dec 2019 at 10:05, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
> > > >
> > > > On Thu, 12 Dec 2019 at 09:59, Ni, Ray <ray.ni@intel.com> wrote:
> > > > >
> > > > > Ard,
> > > > > I still cannot understand it.
> > > > >
> > > > > Since Driver#### are processed (process = LoadImage + StartImage) after
> EndOfDxe, they are not deferred.
> > > > > It means the Driver#### entrypoints are called directly.
> > > > > Inside the entrypoints, driverbinding protocols are installed.
> > > > >
> > > > > After that, EfiBootManagerConnectAllDefaultConsoles () uses driver
> model logic to start the proper GOP driver.
> > > > >
> > > >
> > > > It only does that if the GOP console is already in the
> > > > ConIn/ConOut/ConErr variables, no?
> > > >
> > > > Today, we have code in the PlatformBdsLib to traverse the PCI
> > > > hierarchy to populate those ConXXX variables if we encounter any PCI
> > > > graphics cards, but this is done in
> > > > PlatformBootManagerBeforeConsole(), so if the Driver#### is loaded
> > > > *after* PlatformBootManagerBeforeConsole(), it will not be added to
> > > > ConXXX. So we need to process Driver#### before calling
> > > > PlatformBootManagerBeforeConsole(), so that the driver is dispatches
> > > > right after we call DispatchDeferredImages() but before we do the
> > > > traversal and populate the COnXXX variables.
> > > >
> > > > How else do you propose we could make PCI graphics controllers
> > > > supported by Driver#### options appear in COnXXX before
> > > > EfiBootManagerConnectAllDefaultConsoles() is called?
> > > >
> > >
> > > Ping?
> > >
> > >
> >
> > Ping again?
> >
> > > >
> > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Ard Biesheuvel
> > > > > > Sent: Wednesday, December 11, 2019 6:40 PM
> > > > > > To: Ni, Ray <ray.ni@intel.com>
> > > > > > Cc: Laszlo Ersek <lersek@redhat.com>; edk2-devel-groups-io
> <devel@edk2.groups.io>; Leif Lindholm
> > > > > > <leif.lindholm@linaro.org>; Gao, Zhichao <zhichao.gao@intel.com>;
> Ma, Maurice <maurice.ma@intel.com>; Dong,
> > Guo
> > > > > > <guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>
> > > > > > Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe:
> allow consoles on drivers loaded via Driver####
> > > > > >
> > > > > > On Mon, 9 Dec 2019 at 09:42, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> > > > > > >
> > > > > > > On Mon, 9 Dec 2019 at 03:12, Ni, Ray <ray.ni@intel.com> wrote:
> > > > > > > >
> > > > > > > > > Exactly. This flow is identical to how option ROMs are processed if
> > > > > > > > > they are discovered before EndOfDxe signalling completes (which
> is why
> > > > > > > > > the Juno platform was broken without the call to
> > > > > > > > > EfiBootManagerDispatchDeferredImages() in
> > > > > > > > > PlatformBootManagerBeforeConsole())
> > > > > > > > >
> > > > > > > >
> > > > > > > > Ard,
> > > > > > > > I checked ArmPkg's PlatformBootManagerLib and found it doesn't
> > > > > > > > call *DispatchDeferredImages() after signaling EndOfDxe.
> > > > > > > >
> > > > > > >
> > > > > > > It does. We just added this in
> 0f9395d7c5cc6ae2beaa2d87008fe158d04a8069
> > > > > > >
> > > > > > > > The deferred image dispatch mechanism assumes the platform
> > > > > > > > needs to call the *DispatchDeferredImages() after signaling
> EndOfDxe.
> > > > > > > >
> > > > > > >
> > > > > > > Indeed.
> > > > > > >
> > > > > > > > I don't understand why the deferred image can be loaded with your
> patch.
> > > > > > > > They are still deferred because the loading time is before EndOfDxe.
> > > > > > > >
> > > > > > >
> > > > > > > Yes, but because PlatformBootManagerBeforeConsole () does all of
> this,
> > > > > > > the only way to get Driver#### to work for consoles on GOP drivers,
> we
> > > > > > > need to move it before that call.
> > > > > >
> > > > > > Any further comments on this patch?
> > > > > >
> > > > > >
> > > > >
> >
> >