Re: [edk2-devel] [PATCH edk2-platforms] Platform/OverdriveBoard: work around network ConnectAll() dependency

["Leif Lindholm" <leif@nuviainc.com>]

On Fri, May 29, 2020 at 23:40:19 +0200, Ard Biesheuvel wrote:
> The AMD Seattle based platforms have been kept up to date in recent
> years, even though the hardware is obsolete and was never available
> that widely in the first place.
> 
> However, one aspect that has sadly been left behind is the support for
> the builtin network controllers. These are only wired up and enabled
> on the Overdrive board to begin with, and the driver was only made
> available as two separate binary blobs implementing the SNP protocol
> for each controller separately, without taking the UEFI driver model
> into account.
> 
> Even worse, the PHY initialization code that needs to run at boot in
> order for the OS to be able to use the device never executes unless
> the upper networking layers start the SNP protocol, which doesn't
> happen on a fast boot (one that does not use ConnectAll()) unless the
> boot target is a network device path.
> 
> We cannot fix the driver, but fortunately, there is another way out:
> protocols that are installed on a handle during the execution of the
> entrypoint of a driver will be connected by the DXE core, and so we
> can ensure that the old behavior is retained regardless of whether
> ConnectAll() is ever invoked, by reordering the load sequence so that
> the upper layer drivers have all been registered by the time the
> entrypoints of the SNP drivers are called.
> 
> This relies on FV contents to be dispatched in the order they appear
> in the .FDF file. The AMD SNP driver as well as the upper layer
> drivers in NetworkPkg are UEFI_DRIVER modules, which means their
> DEPEXes are implicitly defined as the full set of architectural
> PI protocols. This means that all these modules become available for
> dispatch at the same time, and their dispatch order is fully defined
> by the order of appearance in the FV. Unfortunately, this is an
> implementation detail rather than something that is supported by the
> PI spec, but this is unlikely to ever change since other platforms
> undoubtedly exist that depend on this behavior as well.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

I'm holding my nose a bit, but:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>

> ---
> 
> This is a preparatory patch to ensure that this platform does not break
> when core EDK2 drops the EfiBootManagerConnectAll() from the ArmPkg
> version of PlatformBootManagerLib.
> 
>  Platform/AMD/OverdriveBoard/OverdriveBoard.fdf | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Platform/AMD/OverdriveBoard/OverdriveBoard.fdf b/Platform/AMD/OverdriveBoard/OverdriveBoard.fdf
> index 851ae65b5be9..15b5b1bc317f 100644
> --- a/Platform/AMD/OverdriveBoard/OverdriveBoard.fdf
> +++ b/Platform/AMD/OverdriveBoard/OverdriveBoard.fdf
> @@ -178,18 +178,18 @@ [FV.FvMain]
>    INF MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>    INF MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
>  
> -  #
> -  # SNP support
> -  #
> -  INF Silicon/AMD/Styx/AmdModulePkg/SnpDxe/SnpDxePort0.inf
> -  INF Silicon/AMD/Styx/AmdModulePkg/SnpDxe/SnpDxePort1.inf
> -
>    #
>    # Networking stack
>    #
>  !include NetworkPkg/Network.fdf.inc
>    INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
>  
> +  #
> +  # SNP support
> +  #
> +  INF Silicon/AMD/Styx/AmdModulePkg/SnpDxe/SnpDxePort0.inf
> +  INF Silicon/AMD/Styx/AmdModulePkg/SnpDxe/SnpDxePort1.inf
> +
>    #
>    # Core Info
>    #
> -- 
> 2.26.2
> 
> 
> 
>