* [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page @ 2020-04-08 17:28 Ard Biesheuvel 2020-04-09 9:29 ` [edk2-devel] " Laszlo Ersek 0 siblings, 1 reply; 11+ messages in thread From: Ard Biesheuvel @ 2020-04-08 17:28 UTC (permalink / raw) To: devel; +Cc: Ard Biesheuvel, Zhichao Gao, Ray Ni, Jian J Wang, Hao A Wu, Dandan Bi The device manager UI library creates a UiApp submenu that contains a list of network devices in the system. The logic that creates this menu assumes that all handles have been connected to their drivers, but this is not guaranteed in the general case. So work around this by doing an explicit ConnectAll() before populating the pages. Cc: Zhichao Gao <zhichao.gao@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Dandan Bi <dandan.bi@intel.com> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com> --- MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c | 7 +++++++ MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h | 1 + MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf | 1 + 3 files changed, 9 insertions(+) diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c index 0540e6fa8a44..3bc13d340775 100644 --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c @@ -892,6 +892,13 @@ DeviceManagerUiLibConstructor ( ); ASSERT (gDeviceManagerPrivate.HiiHandle != NULL); + // + // The device manager form contains a page listing all the network + // controllers in the system. This list can only be populated if all + // handles have been connected, so do it here. + // + EfiBootManagerConnectAll (); + // // Update boot manager page // diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h index 22fe12d2a5e8..c53c2a1a0e1a 100644 --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include <Library/BaseLib.h> #include <Library/HiiLib.h> #include <Library/DevicePathLib.h> +#include <Library/UefiBootManagerLib.h> #include <Library/UefiHiiServicesLib.h> // diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf index cb01b3b85180..d7f833d8b23a 100644 --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf @@ -40,6 +40,7 @@ [LibraryClasses] DebugLib PrintLib HiiLib + UefiBootManagerLib UefiHiiServicesLib [Guids] -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page 2020-04-08 17:28 [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page Ard Biesheuvel @ 2020-04-09 9:29 ` Laszlo Ersek 2020-04-09 9:37 ` Ard Biesheuvel 0 siblings, 1 reply; 11+ messages in thread From: Laszlo Ersek @ 2020-04-09 9:29 UTC (permalink / raw) To: devel, ard.biesheuvel Cc: Zhichao Gao, Ray Ni, Jian J Wang, Hao A Wu, Dandan Bi On 04/08/20 19:28, Ard Biesheuvel wrote: > The device manager UI library creates a UiApp submenu that contains a > list of network devices in the system. The logic that creates this menu > assumes that all handles have been connected to their drivers, but this > is not guaranteed in the general case. > > So work around this by doing an explicit ConnectAll() before populating > the pages. > > Cc: Zhichao Gao <zhichao.gao@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Hao A Wu <hao.a.wu@intel.com> > Cc: Dandan Bi <dandan.bi@intel.com> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com> > --- > MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c | 7 +++++++ > MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h | 1 + > MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf | 1 + > 3 files changed, 9 insertions(+) Can you describe one example that's (a) relevant to you and (b) affected by this issue? EfiBootManagerConnectAll() feels quite heavy-weight; I'd *vaguely* prefer adding it elsewhere, instead of a "UI lib"'s constructor. I don't have a particular suggestion however. I guess part of the issue must be that the EfiBootManagerConnectAll() calls in BootManagerMenuApp / UiApp are not reached, somehow. Maybe those are the calls that DeviceManagerUiLib takes for granted. Thanks, Laszlo > diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > index 0540e6fa8a44..3bc13d340775 100644 > --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > @@ -892,6 +892,13 @@ DeviceManagerUiLibConstructor ( > ); > ASSERT (gDeviceManagerPrivate.HiiHandle != NULL); > > + // > + // The device manager form contains a page listing all the network > + // controllers in the system. This list can only be populated if all > + // handles have been connected, so do it here. > + // > + EfiBootManagerConnectAll (); > + > // > // Update boot manager page > // > diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h > index 22fe12d2a5e8..c53c2a1a0e1a 100644 > --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h > +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h > @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include <Library/BaseLib.h> > #include <Library/HiiLib.h> > #include <Library/DevicePathLib.h> > +#include <Library/UefiBootManagerLib.h> > #include <Library/UefiHiiServicesLib.h> > > // > diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > index cb01b3b85180..d7f833d8b23a 100644 > --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > @@ -40,6 +40,7 @@ [LibraryClasses] > DebugLib > PrintLib > HiiLib > + UefiBootManagerLib > UefiHiiServicesLib > > [Guids] > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page 2020-04-09 9:29 ` [edk2-devel] " Laszlo Ersek @ 2020-04-09 9:37 ` Ard Biesheuvel 2020-04-09 13:30 ` Laszlo Ersek 2020-04-12 7:55 ` Dandan Bi 0 siblings, 2 replies; 11+ messages in thread From: Ard Biesheuvel @ 2020-04-09 9:37 UTC (permalink / raw) To: devel, lersek; +Cc: Zhichao Gao, Ray Ni, Jian J Wang, Hao A Wu, Dandan Bi On 4/9/20 11:29 AM, Laszlo Ersek via groups.io wrote: > On 04/08/20 19:28, Ard Biesheuvel wrote: >> The device manager UI library creates a UiApp submenu that contains a >> list of network devices in the system. The logic that creates this menu >> assumes that all handles have been connected to their drivers, but this >> is not guaranteed in the general case. >> >> So work around this by doing an explicit ConnectAll() before populating >> the pages. >> >> Cc: Zhichao Gao <zhichao.gao@intel.com> >> Cc: Ray Ni <ray.ni@intel.com> >> Cc: Jian J Wang <jian.j.wang@intel.com> >> Cc: Hao A Wu <hao.a.wu@intel.com> >> Cc: Dandan Bi <dandan.bi@intel.com> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com> >> --- >> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c | 7 +++++++ >> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h | 1 + >> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf | 1 + >> 3 files changed, 9 insertions(+) > > Can you describe one example that's (a) relevant to you and (b) affected > by this issue? > Yes. The SynQuacer platform has a NIC driver that waits up to 5 seconds for the link to come up after it resets the PHY, which never happens if you boot without a network cable connected. But in the general case, connecting all network drivers on a typical BootOrder based boot should be be necessary. Currently, ArmPkg's PlatformBootManagerLib does a EfiBootManagerConnectAll () in its AfterConsole() callback, and so this delay is always induced. I intend to 'fix' this platform (and this library in general) to only do a EfiBootManagerConnectAllDefaultConsoles () at this point, which is sufficient to interrupt the boot if necessary, and otherwise, boot as fast as possible, without connecting any devices that are not needed for this. This works surprisingly well, with the exception of the 'Network Device List' in the device manager UiApp menu, which no longer lists the network interface, even though UiApp does a connect-all as it starts up. As you guessed, the constructor ordering versus the connect-all call results in the 'network device list' menu to be populated before any of the non-connected handles are connected. This is obviously against the intent of doing a connect-all and enumerating all devices, and so this is a bug in UiApp. > EfiBootManagerConnectAll() feels quite heavy-weight; I'd *vaguely* > prefer adding it elsewhere, instead of a "UI lib"'s constructor. I don't > have a particular suggestion however. > > I guess part of the issue must be that the EfiBootManagerConnectAll() > calls in BootManagerMenuApp / UiApp are not reached, somehow. Maybe > those are the calls that DeviceManagerUiLib takes for granted. > I agree that this approach is not the most elegant, but the way UiApp is constructed makes it very hard to find a better way without some major refactoring. >> diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c >> index 0540e6fa8a44..3bc13d340775 100644 >> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c >> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c >> @@ -892,6 +892,13 @@ DeviceManagerUiLibConstructor ( >> ); >> ASSERT (gDeviceManagerPrivate.HiiHandle != NULL); >> >> + // >> + // The device manager form contains a page listing all the network >> + // controllers in the system. This list can only be populated if all >> + // handles have been connected, so do it here. >> + // >> + EfiBootManagerConnectAll (); >> + >> // >> // Update boot manager page >> // >> diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h >> index 22fe12d2a5e8..c53c2a1a0e1a 100644 >> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h >> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h >> @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >> #include <Library/BaseLib.h> >> #include <Library/HiiLib.h> >> #include <Library/DevicePathLib.h> >> +#include <Library/UefiBootManagerLib.h> >> #include <Library/UefiHiiServicesLib.h> >> >> // >> diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf >> index cb01b3b85180..d7f833d8b23a 100644 >> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf >> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf >> @@ -40,6 +40,7 @@ [LibraryClasses] >> DebugLib >> PrintLib >> HiiLib >> + UefiBootManagerLib >> UefiHiiServicesLib >> >> [Guids] >> > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page 2020-04-09 9:37 ` Ard Biesheuvel @ 2020-04-09 13:30 ` Laszlo Ersek 2020-04-12 7:55 ` Dandan Bi 1 sibling, 0 replies; 11+ messages in thread From: Laszlo Ersek @ 2020-04-09 13:30 UTC (permalink / raw) To: Ard Biesheuvel, devel Cc: Zhichao Gao, Ray Ni, Jian J Wang, Hao A Wu, Dandan Bi On 04/09/20 11:37, Ard Biesheuvel wrote: > On 4/9/20 11:29 AM, Laszlo Ersek via groups.io wrote: >> On 04/08/20 19:28, Ard Biesheuvel wrote: >>> The device manager UI library creates a UiApp submenu that contains a >>> list of network devices in the system. The logic that creates this menu >>> assumes that all handles have been connected to their drivers, but this >>> is not guaranteed in the general case. >>> >>> So work around this by doing an explicit ConnectAll() before populating >>> the pages. >>> >>> Cc: Zhichao Gao <zhichao.gao@intel.com> >>> Cc: Ray Ni <ray.ni@intel.com> >>> Cc: Jian J Wang <jian.j.wang@intel.com> >>> Cc: Hao A Wu <hao.a.wu@intel.com> >>> Cc: Dandan Bi <dandan.bi@intel.com> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com> >>> --- >>> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c | 7 >>> +++++++ >>> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h | 1 + >>> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf | 1 + >>> 3 files changed, 9 insertions(+) >> >> Can you describe one example that's (a) relevant to you and (b) affected >> by this issue? >> > > Yes. The SynQuacer platform has a NIC driver that waits up to 5 seconds > for the link to come up after it resets the PHY, which never happens if > you boot without a network cable connected. But in the general case, > connecting all network drivers on a typical BootOrder based boot should > be be necessary. > > Currently, ArmPkg's PlatformBootManagerLib does a > EfiBootManagerConnectAll () in its AfterConsole() callback, and so this > delay is always induced. I intend to 'fix' this platform (and this > library in general) to only do a EfiBootManagerConnectAllDefaultConsoles > () at this point, which is sufficient to interrupt the boot if > necessary, and otherwise, boot as fast as possible, without connecting > any devices that are not needed for this. OK. > This works surprisingly well, with the exception of the 'Network Device > List' in the device manager UiApp menu, which no longer lists the > network interface, even though UiApp does a connect-all as it starts up. Yes, this was the connect-all I seemed to recall. > As you guessed, the constructor ordering versus the connect-all call > results in the 'network device list' menu to be populated before any of > the non-connected handles are connected. This is obviously against the > intent of doing a connect-all and enumerating all devices, and so this > is a bug in UiApp. Sigh, thanks. I understand now. I assumed that DeviceManagerUiLib's constructor (linked into UiApp via NULL class resolution) would only register a callback into a list. Then UiApp would run that list of callbacks at the right time. This is a frequently used pattern in edk2 (with various "pluggable" modules). > >> EfiBootManagerConnectAll() feels quite heavy-weight; I'd *vaguely* >> prefer adding it elsewhere, instead of a "UI lib"'s constructor. I don't >> have a particular suggestion however. >> >> I guess part of the issue must be that the EfiBootManagerConnectAll() >> calls in BootManagerMenuApp / UiApp are not reached, somehow. Maybe >> those are the calls that DeviceManagerUiLib takes for granted. >> > > I agree that this approach is not the most elegant, but the way UiApp is > constructed makes it very hard to find a better way without some major > refactoring. I agree. New APIs would likely be needed. Thanks for the explanation. If the MdeModulePkg folks are happy with this patch, I have nothing against it! In that case, I think it would be helpful to include a short version of the above explanation in the commit message (no need to repost, of course). Thanks! Laszlo > > >>> diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c >>> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c >>> index 0540e6fa8a44..3bc13d340775 100644 >>> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c >>> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c >>> @@ -892,6 +892,13 @@ DeviceManagerUiLibConstructor ( >>> ); >>> ASSERT (gDeviceManagerPrivate.HiiHandle != NULL); >>> + // >>> + // The device manager form contains a page listing all the network >>> + // controllers in the system. This list can only be populated if all >>> + // handles have been connected, so do it here. >>> + // >>> + EfiBootManagerConnectAll (); >>> + >>> // >>> // Update boot manager page >>> // >>> diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h >>> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h >>> index 22fe12d2a5e8..c53c2a1a0e1a 100644 >>> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h >>> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h >>> @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >>> #include <Library/BaseLib.h> >>> #include <Library/HiiLib.h> >>> #include <Library/DevicePathLib.h> >>> +#include <Library/UefiBootManagerLib.h> >>> #include <Library/UefiHiiServicesLib.h> >>> // >>> diff --git >>> a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf >>> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf >>> index cb01b3b85180..d7f833d8b23a 100644 >>> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf >>> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf >>> @@ -40,6 +40,7 @@ [LibraryClasses] >>> DebugLib >>> PrintLib >>> HiiLib >>> + UefiBootManagerLib >>> UefiHiiServicesLib >>> [Guids] >>> >> >> >> >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page 2020-04-09 9:37 ` Ard Biesheuvel 2020-04-09 13:30 ` Laszlo Ersek @ 2020-04-12 7:55 ` Dandan Bi 2020-04-12 8:11 ` Hot Tian 2020-04-12 8:21 ` Ard Biesheuvel 1 sibling, 2 replies; 11+ messages in thread From: Dandan Bi @ 2020-04-12 7:55 UTC (permalink / raw) To: devel@edk2.groups.io, ard.biesheuvel@arm.com, lersek@redhat.com Cc: Gao, Zhichao, Ni, Ray, Wang, Jian J, Wu, Hao A Hi Ard, It seems that the root cause is the 'Network Device List' in the device manager menu is crated before EfiBootManagerConnectAll () is called in UiEntry function. If we choose to add the EfiBootManagerConnectAll() in DeviceManagerUiLib with this patch, could we don't call the EfiBootManagerConnectAll() in UiEntry to avoid it's called twice when enter UiApp? Thanks, Dandan > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Ard Biesheuvel > Sent: Thursday, April 9, 2020 5:37 PM > To: devel@edk2.groups.io; lersek@redhat.com > Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; > Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, > Dandan <dandan.bi@intel.com> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: > connect all before creating menu page > > On 4/9/20 11:29 AM, Laszlo Ersek via groups.io wrote: > > On 04/08/20 19:28, Ard Biesheuvel wrote: > >> The device manager UI library creates a UiApp submenu that contains a > >> list of network devices in the system. The logic that creates this > >> menu assumes that all handles have been connected to their drivers, > >> but this is not guaranteed in the general case. > >> > >> So work around this by doing an explicit ConnectAll() before > >> populating the pages. > >> > >> Cc: Zhichao Gao <zhichao.gao@intel.com> > >> Cc: Ray Ni <ray.ni@intel.com> > >> Cc: Jian J Wang <jian.j.wang@intel.com> > >> Cc: Hao A Wu <hao.a.wu@intel.com> > >> Cc: Dandan Bi <dandan.bi@intel.com> > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com> > >> --- > >> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c | 7 > +++++++ > >> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h | 1 + > >> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf | > 1 + > >> 3 files changed, 9 insertions(+) > > > > Can you describe one example that's (a) relevant to you and (b) > > affected by this issue? > > > > Yes. The SynQuacer platform has a NIC driver that waits up to 5 seconds for > the link to come up after it resets the PHY, which never happens if you boot > without a network cable connected. But in the general case, connecting all > network drivers on a typical BootOrder based boot should be be necessary. > > Currently, ArmPkg's PlatformBootManagerLib does a > EfiBootManagerConnectAll () in its AfterConsole() callback, and so this delay > is always induced. I intend to 'fix' this platform (and this library in general) to > only do a EfiBootManagerConnectAllDefaultConsoles > () at this point, which is sufficient to interrupt the boot if necessary, and > otherwise, boot as fast as possible, without connecting any devices that are > not needed for this. > > This works surprisingly well, with the exception of the 'Network Device List' in > the device manager UiApp menu, which no longer lists the network interface, > even though UiApp does a connect-all as it starts up. > As you guessed, the constructor ordering versus the connect-all call results in > the 'network device list' menu to be populated before any of the non- > connected handles are connected. This is obviously against the intent of > doing a connect-all and enumerating all devices, and so this is a bug in UiApp. > > > EfiBootManagerConnectAll() feels quite heavy-weight; I'd *vaguely* > > prefer adding it elsewhere, instead of a "UI lib"'s constructor. I > > don't have a particular suggestion however. > > > > I guess part of the issue must be that the EfiBootManagerConnectAll() > > calls in BootManagerMenuApp / UiApp are not reached, somehow. Maybe > > those are the calls that DeviceManagerUiLib takes for granted. > > > > I agree that this approach is not the most elegant, but the way UiApp is > constructed makes it very hard to find a better way without some major > refactoring. > > > >> diff --git > a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > >> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > >> index 0540e6fa8a44..3bc13d340775 100644 > >> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > >> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > >> @@ -892,6 +892,13 @@ DeviceManagerUiLibConstructor ( > >> ); > >> ASSERT (gDeviceManagerPrivate.HiiHandle != NULL); > >> > >> + // > >> + // The device manager form contains a page listing all the network > >> + // controllers in the system. This list can only be populated if > >> + all // handles have been connected, so do it here. > >> + // > >> + EfiBootManagerConnectAll (); > >> + > >> // > >> // Update boot manager page > >> // > >> diff --git > a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h > >> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h > >> index 22fe12d2a5e8..c53c2a1a0e1a 100644 > >> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h > >> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h > >> @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > >> #include <Library/BaseLib.h> > >> #include <Library/HiiLib.h> > >> #include <Library/DevicePathLib.h> > >> +#include <Library/UefiBootManagerLib.h> > >> #include <Library/UefiHiiServicesLib.h> > >> > >> // > >> diff --git > >> a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > >> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > >> index cb01b3b85180..d7f833d8b23a 100644 > >> --- > a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > >> +++ > b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > >> @@ -40,6 +40,7 @@ [LibraryClasses] > >> DebugLib > >> PrintLib > >> HiiLib > >> + UefiBootManagerLib > >> UefiHiiServicesLib > >> > >> [Guids] > >> > > > > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page 2020-04-12 7:55 ` Dandan Bi @ 2020-04-12 8:11 ` Hot Tian 2020-04-14 10:02 ` Laszlo Ersek 2020-04-12 8:21 ` Ard Biesheuvel 1 sibling, 1 reply; 11+ messages in thread From: Hot Tian @ 2020-04-12 8:11 UTC (permalink / raw) To: devel@edk2.groups.io, Bi, Dandan, ard.biesheuvel@arm.com, lersek@redhat.com Cc: Gao, Zhichao, Ni, Ray, Wang, Jian J, Wu, Hao A EfiBootManagerConnectAll is kind of BDS policy. Should it be controlled by Ui App or Ui Lib? Thanks, Hot -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dandan Bi Sent: Sunday, April 12, 2020 15:56 To: devel@edk2.groups.io; ard.biesheuvel@arm.com; lersek@redhat.com Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page Hi Ard, It seems that the root cause is the 'Network Device List' in the device manager menu is crated before EfiBootManagerConnectAll () is called in UiEntry function. If we choose to add the EfiBootManagerConnectAll() in DeviceManagerUiLib with this patch, could we don't call the EfiBootManagerConnectAll() in UiEntry to avoid it's called twice when enter UiApp? Thanks, Dandan > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Ard Biesheuvel > Sent: Thursday, April 9, 2020 5:37 PM > To: devel@edk2.groups.io; lersek@redhat.com > Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; > Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; > Bi, Dandan <dandan.bi@intel.com> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: > connect all before creating menu page > > On 4/9/20 11:29 AM, Laszlo Ersek via groups.io wrote: > > On 04/08/20 19:28, Ard Biesheuvel wrote: > >> The device manager UI library creates a UiApp submenu that contains > >> a list of network devices in the system. The logic that creates > >> this menu assumes that all handles have been connected to their > >> drivers, but this is not guaranteed in the general case. > >> > >> So work around this by doing an explicit ConnectAll() before > >> populating the pages. > >> > >> Cc: Zhichao Gao <zhichao.gao@intel.com> > >> Cc: Ray Ni <ray.ni@intel.com> > >> Cc: Jian J Wang <jian.j.wang@intel.com> > >> Cc: Hao A Wu <hao.a.wu@intel.com> > >> Cc: Dandan Bi <dandan.bi@intel.com> > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com> > >> --- > >> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c | 7 > +++++++ > >> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h | 1 + > >> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf | > 1 + > >> 3 files changed, 9 insertions(+) > > > > Can you describe one example that's (a) relevant to you and (b) > > affected by this issue? > > > > Yes. The SynQuacer platform has a NIC driver that waits up to 5 > seconds for the link to come up after it resets the PHY, which never > happens if you boot without a network cable connected. But in the > general case, connecting all network drivers on a typical BootOrder based boot should be be necessary. > > Currently, ArmPkg's PlatformBootManagerLib does a > EfiBootManagerConnectAll () in its AfterConsole() callback, and so > this delay is always induced. I intend to 'fix' this platform (and > this library in general) to only do a > EfiBootManagerConnectAllDefaultConsoles > () at this point, which is sufficient to interrupt the boot if > necessary, and otherwise, boot as fast as possible, without connecting > any devices that are not needed for this. > > This works surprisingly well, with the exception of the 'Network > Device List' in the device manager UiApp menu, which no longer lists > the network interface, even though UiApp does a connect-all as it starts up. > As you guessed, the constructor ordering versus the connect-all call > results in the 'network device list' menu to be populated before any > of the non- connected handles are connected. This is obviously against > the intent of doing a connect-all and enumerating all devices, and so this is a bug in UiApp. > > > EfiBootManagerConnectAll() feels quite heavy-weight; I'd *vaguely* > > prefer adding it elsewhere, instead of a "UI lib"'s constructor. I > > don't have a particular suggestion however. > > > > I guess part of the issue must be that the > > EfiBootManagerConnectAll() calls in BootManagerMenuApp / UiApp are > > not reached, somehow. Maybe those are the calls that DeviceManagerUiLib takes for granted. > > > > I agree that this approach is not the most elegant, but the way UiApp > is constructed makes it very hard to find a better way without some > major refactoring. > > > >> diff --git > a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > >> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > >> index 0540e6fa8a44..3bc13d340775 100644 > >> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > >> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c > >> @@ -892,6 +892,13 @@ DeviceManagerUiLibConstructor ( > >> ); > >> ASSERT (gDeviceManagerPrivate.HiiHandle != NULL); > >> > >> + // > >> + // The device manager form contains a page listing all the > >> + network // controllers in the system. This list can only be > >> + populated if all // handles have been connected, so do it here. > >> + // > >> + EfiBootManagerConnectAll (); > >> + > >> // > >> // Update boot manager page > >> // > >> diff --git > a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h > >> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h > >> index 22fe12d2a5e8..c53c2a1a0e1a 100644 > >> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h > >> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h > >> @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > >> #include <Library/BaseLib.h> > >> #include <Library/HiiLib.h> > >> #include <Library/DevicePathLib.h> > >> +#include <Library/UefiBootManagerLib.h> > >> #include <Library/UefiHiiServicesLib.h> > >> > >> // > >> diff --git > >> a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > >> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > >> index cb01b3b85180..d7f833d8b23a 100644 > >> --- > a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > >> +++ > b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf > >> @@ -40,6 +40,7 @@ [LibraryClasses] > >> DebugLib > >> PrintLib > >> HiiLib > >> + UefiBootManagerLib > >> UefiHiiServicesLib > >> > >> [Guids] > >> > > > > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page 2020-04-12 8:11 ` Hot Tian @ 2020-04-14 10:02 ` Laszlo Ersek 0 siblings, 0 replies; 11+ messages in thread From: Laszlo Ersek @ 2020-04-14 10:02 UTC (permalink / raw) To: Tian, Hot, devel@edk2.groups.io, Bi, Dandan, ard.biesheuvel@arm.com Cc: Gao, Zhichao, Ni, Ray, Wang, Jian J, Wu, Hao A On 04/12/20 10:11, Tian, Hot wrote: > EfiBootManagerConnectAll is kind of BDS policy. Should it be controlled by Ui App or Ui Lib? I think platform BDS policy applies to normal (non-interactive, non-interrupted) boot. I agree that connect-all should not be forced into that. But, if the user intentionally enters the setup TUI, or else we fall back to the setup TUI because there's nothing to boot -- see commit range cef7ecf6cdb4..1010873becc5 --, then compatibility (i.e., connect everything we can) is arguably more important than "speed". Because, we're not (immediately) booting an OS anyway, at that point. IOW, I think connect-all should stay in UiApp. Thanks Laszlo > > Thanks, > Hot > > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dandan Bi > Sent: Sunday, April 12, 2020 15:56 > To: devel@edk2.groups.io; ard.biesheuvel@arm.com; lersek@redhat.com > Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page > > Hi Ard, > > It seems that the root cause is the 'Network Device List' in the device manager menu is crated before EfiBootManagerConnectAll () is called in UiEntry function. > If we choose to add the EfiBootManagerConnectAll() in DeviceManagerUiLib with this patch, could we don't call the EfiBootManagerConnectAll() in UiEntry to avoid it's called twice when enter UiApp? > > > Thanks, > Dandan >> -----Original Message----- >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >> Ard Biesheuvel >> Sent: Thursday, April 9, 2020 5:37 PM >> To: devel@edk2.groups.io; lersek@redhat.com >> Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; >> Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; >> Bi, Dandan <dandan.bi@intel.com> >> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: >> connect all before creating menu page >> >> On 4/9/20 11:29 AM, Laszlo Ersek via groups.io wrote: >>> On 04/08/20 19:28, Ard Biesheuvel wrote: >>>> The device manager UI library creates a UiApp submenu that contains >>>> a list of network devices in the system. The logic that creates >>>> this menu assumes that all handles have been connected to their >>>> drivers, but this is not guaranteed in the general case. >>>> >>>> So work around this by doing an explicit ConnectAll() before >>>> populating the pages. >>>> >>>> Cc: Zhichao Gao <zhichao.gao@intel.com> >>>> Cc: Ray Ni <ray.ni@intel.com> >>>> Cc: Jian J Wang <jian.j.wang@intel.com> >>>> Cc: Hao A Wu <hao.a.wu@intel.com> >>>> Cc: Dandan Bi <dandan.bi@intel.com> >>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com> >>>> --- >>>> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c | 7 >> +++++++ >>>> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h | 1 + >>>> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf | >> 1 + >>>> 3 files changed, 9 insertions(+) >>> >>> Can you describe one example that's (a) relevant to you and (b) >>> affected by this issue? >>> >> >> Yes. The SynQuacer platform has a NIC driver that waits up to 5 >> seconds for the link to come up after it resets the PHY, which never >> happens if you boot without a network cable connected. But in the >> general case, connecting all network drivers on a typical BootOrder based boot should be be necessary. >> >> Currently, ArmPkg's PlatformBootManagerLib does a >> EfiBootManagerConnectAll () in its AfterConsole() callback, and so >> this delay is always induced. I intend to 'fix' this platform (and >> this library in general) to only do a >> EfiBootManagerConnectAllDefaultConsoles >> () at this point, which is sufficient to interrupt the boot if >> necessary, and otherwise, boot as fast as possible, without connecting >> any devices that are not needed for this. >> >> This works surprisingly well, with the exception of the 'Network >> Device List' in the device manager UiApp menu, which no longer lists >> the network interface, even though UiApp does a connect-all as it starts up. >> As you guessed, the constructor ordering versus the connect-all call >> results in the 'network device list' menu to be populated before any >> of the non- connected handles are connected. This is obviously against >> the intent of doing a connect-all and enumerating all devices, and so this is a bug in UiApp. >> >>> EfiBootManagerConnectAll() feels quite heavy-weight; I'd *vaguely* >>> prefer adding it elsewhere, instead of a "UI lib"'s constructor. I >>> don't have a particular suggestion however. >>> >>> I guess part of the issue must be that the >>> EfiBootManagerConnectAll() calls in BootManagerMenuApp / UiApp are >>> not reached, somehow. Maybe those are the calls that DeviceManagerUiLib takes for granted. >>> >> >> I agree that this approach is not the most elegant, but the way UiApp >> is constructed makes it very hard to find a better way without some >> major refactoring. >> >> >>>> diff --git >> a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c >>>> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c >>>> index 0540e6fa8a44..3bc13d340775 100644 >>>> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c >>>> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c >>>> @@ -892,6 +892,13 @@ DeviceManagerUiLibConstructor ( >>>> ); >>>> ASSERT (gDeviceManagerPrivate.HiiHandle != NULL); >>>> >>>> + // >>>> + // The device manager form contains a page listing all the >>>> + network // controllers in the system. This list can only be >>>> + populated if all // handles have been connected, so do it here. >>>> + // >>>> + EfiBootManagerConnectAll (); >>>> + >>>> // >>>> // Update boot manager page >>>> // >>>> diff --git >> a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h >>>> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h >>>> index 22fe12d2a5e8..c53c2a1a0e1a 100644 >>>> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h >>>> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h >>>> @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >>>> #include <Library/BaseLib.h> >>>> #include <Library/HiiLib.h> >>>> #include <Library/DevicePathLib.h> >>>> +#include <Library/UefiBootManagerLib.h> >>>> #include <Library/UefiHiiServicesLib.h> >>>> >>>> // >>>> diff --git >>>> a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf >>>> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf >>>> index cb01b3b85180..d7f833d8b23a 100644 >>>> --- >> a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf >>>> +++ >> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf >>>> @@ -40,6 +40,7 @@ [LibraryClasses] >>>> DebugLib >>>> PrintLib >>>> HiiLib >>>> + UefiBootManagerLib >>>> UefiHiiServicesLib >>>> >>>> [Guids] >>>> >>> >>> >>> >>> >> >> >> > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page 2020-04-12 7:55 ` Dandan Bi 2020-04-12 8:11 ` Hot Tian @ 2020-04-12 8:21 ` Ard Biesheuvel 2020-04-19 8:06 ` Dandan Bi 1 sibling, 1 reply; 11+ messages in thread From: Ard Biesheuvel @ 2020-04-12 8:21 UTC (permalink / raw) To: Bi, Dandan, devel@edk2.groups.io, lersek@redhat.com Cc: Gao, Zhichao, Ni, Ray, Wang, Jian J, Wu, Hao A On 4/12/20 9:55 AM, Bi, Dandan wrote: > Hi Ard, > > It seems that the root cause is the 'Network Device List' in the device manager menu is crated before EfiBootManagerConnectAll () is called in UiEntry function. > If we choose to add the EfiBootManagerConnectAll() in DeviceManagerUiLib with this patch, could we don't call the EfiBootManagerConnectAll() in UiEntry to avoid it's called twice when enter UiApp? > DeviceManagerUiLib is optional - this seems to be the purpose of the modular nature of UiApp with the NULL library class resolution. Removing EfiBootManagerConnectAll() from UiApp itself means it may not ever be called. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page 2020-04-12 8:21 ` Ard Biesheuvel @ 2020-04-19 8:06 ` Dandan Bi 2020-04-24 17:59 ` Ard Biesheuvel [not found] ` <1608D342B47ED79B.10837@groups.io> 0 siblings, 2 replies; 11+ messages in thread From: Dandan Bi @ 2020-04-19 8:06 UTC (permalink / raw) To: devel@edk2.groups.io, ard.biesheuvel@arm.com, lersek@redhat.com Cc: Gao, Zhichao, Ni, Ray, Wang, Jian J, Wu, Hao A Ok, thanks Ard! Reviewed-by: Dandan Bi <dandan.bi@intel.com> for this patch. Thanks, Dandan > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Ard Biesheuvel > Sent: Sunday, April 12, 2020 4:22 PM > To: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io; > lersek@redhat.com > Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; > Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: > connect all before creating menu page > > On 4/12/20 9:55 AM, Bi, Dandan wrote: > > Hi Ard, > > > > It seems that the root cause is the 'Network Device List' in the device > manager menu is crated before EfiBootManagerConnectAll () is called in > UiEntry function. > > If we choose to add the EfiBootManagerConnectAll() in > DeviceManagerUiLib with this patch, could we don't call the > EfiBootManagerConnectAll() in UiEntry to avoid it's called twice when enter > UiApp? > > > > DeviceManagerUiLib is optional - this seems to be the purpose of the > modular nature of UiApp with the NULL library class resolution. > > Removing EfiBootManagerConnectAll() from UiApp itself means it may not > ever be called. > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page 2020-04-19 8:06 ` Dandan Bi @ 2020-04-24 17:59 ` Ard Biesheuvel [not found] ` <1608D342B47ED79B.10837@groups.io> 1 sibling, 0 replies; 11+ messages in thread From: Ard Biesheuvel @ 2020-04-24 17:59 UTC (permalink / raw) To: Bi, Dandan, devel@edk2.groups.io, lersek@redhat.com Cc: Gao, Zhichao, Ni, Ray, Wang, Jian J, Wu, Hao A On 4/19/20 10:06 AM, Bi, Dandan wrote: > Ok, thanks Ard! > Reviewed-by: Dandan Bi <dandan.bi@intel.com> for this patch. > Hao, Jian, I intend to merge this patch next week, unless you have any objections. Thanks, Ard. >> -----Original Message----- >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >> Ard Biesheuvel >> Sent: Sunday, April 12, 2020 4:22 PM >> To: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io; >> lersek@redhat.com >> Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; >> Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com> >> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: >> connect all before creating menu page >> >> On 4/12/20 9:55 AM, Bi, Dandan wrote: >>> Hi Ard, >>> >>> It seems that the root cause is the 'Network Device List' in the device >> manager menu is crated before EfiBootManagerConnectAll () is called in >> UiEntry function. >>> If we choose to add the EfiBootManagerConnectAll() in >> DeviceManagerUiLib with this patch, could we don't call the >> EfiBootManagerConnectAll() in UiEntry to avoid it's called twice when enter >> UiApp? >>> >> >> DeviceManagerUiLib is optional - this seems to be the purpose of the >> modular nature of UiApp with the NULL library class resolution. >> >> Removing EfiBootManagerConnectAll() from UiApp itself means it may not >> ever be called. >> >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <1608D342B47ED79B.10837@groups.io>]
* Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page [not found] ` <1608D342B47ED79B.10837@groups.io> @ 2020-04-30 7:52 ` Ard Biesheuvel 0 siblings, 0 replies; 11+ messages in thread From: Ard Biesheuvel @ 2020-04-30 7:52 UTC (permalink / raw) To: devel, Bi, Dandan, lersek@redhat.com Cc: Gao, Zhichao, Ni, Ray, Wang, Jian J, Wu, Hao A On 4/24/20 7:59 PM, Ard Biesheuvel via groups.io wrote: > On 4/19/20 10:06 AM, Bi, Dandan wrote: >> Ok, thanks Ard! >> Reviewed-by: Dandan Bi <dandan.bi@intel.com> for this patch. >> > > Hao, Jian, > > I intend to merge this patch next week, unless you have any objections. > Merged as https://github.com/tianocore/edk2/pull/566, with Dandan's R-b, and the clarification added that Laszlo requested. Thanks all, ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-04-30 7:52 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-08 17:28 [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page Ard Biesheuvel
2020-04-09 9:29 ` [edk2-devel] " Laszlo Ersek
2020-04-09 9:37 ` Ard Biesheuvel
2020-04-09 13:30 ` Laszlo Ersek
2020-04-12 7:55 ` Dandan Bi
2020-04-12 8:11 ` Hot Tian
2020-04-14 10:02 ` Laszlo Ersek
2020-04-12 8:21 ` Ard Biesheuvel
2020-04-19 8:06 ` Dandan Bi
2020-04-24 17:59 ` Ard Biesheuvel
[not found] ` <1608D342B47ED79B.10837@groups.io>
2020-04-30 7:52 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox