Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page

["Laszlo Ersek" <lersek@redhat.com>]

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]
>