Re: [PATCH v2 1/1] NetworkPkg/SnpDxe: Remove ExitBootServices event

["Siyuan, Fu" <siyuan.fu@intel.com>]

Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>

> -----Original Message-----
> From: Rabeda, Maciej <maciej.rabeda@intel.com>
> Sent: 2019年10月14日 20:37
> To: devel@edk2.groups.io
> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH v2 1/1] NetworkPkg/SnpDxe: Remove ExitBootServices
> event
> 
> Patch addresses Bugzilla #1972.
> During ExitBootServices stage, drivers should not call any
> functions known to use Memory Allocation Services. One of such
> functions (as per UEFI spec) is UNDI->Shutdown().
> 
> Since UNDI drivers during ExitBootServices phase are expected
> to put the adapter to such a state that it will not perform any DMA
> operations, there is no need to interface UNDI by SNP driver during
> that phase.
> 
> Finally, since ExitBootServices event notification function in SNP
> only calls UNDI->Shutdown() and Stop() functions, there is no need
> to create this event at all. Adding PCD to control creation of event
> reacting to ExitBootServices() call so that systems with UNDIs relying
> on SNP to call their Shutdown() and Stop() can still work.
> 
> Signed-off-by: Maciej Rabeda <maciej.rabeda@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
> 
> Notes:
>     v2:
>     - modified commit message
>     - added PCD to control ExitBootServices() creation event in SnpDxe
> 
>  NetworkPkg/SnpDxe/Snp.c      | 38 +++++++++++---------
>  NetworkPkg/NetworkPkg.dec    |  7 ++++
>  NetworkPkg/SnpDxe/Snp.h      |  1 +
>  NetworkPkg/SnpDxe/SnpDxe.inf |  3 ++
>  4 files changed, 32 insertions(+), 17 deletions(-)
> 
> diff --git a/NetworkPkg/SnpDxe/Snp.c b/NetworkPkg/SnpDxe/Snp.c
> index a23af05078bc..03317ffa26f1 100644
> --- a/NetworkPkg/SnpDxe/Snp.c
> +++ b/NetworkPkg/SnpDxe/Snp.c
> @@ -647,19 +647,21 @@ SimpleNetworkDriverStart (
>    PxeShutdown (Snp);
>    PxeStop (Snp);
> 
> -  //
> -  // Create EXIT_BOOT_SERIVES Event
> -  //
> -  Status = gBS->CreateEventEx (
> -                  EVT_NOTIFY_SIGNAL,
> -                  TPL_NOTIFY,
> -                  SnpNotifyExitBootServices,
> -                  Snp,
> -                  &gEfiEventExitBootServicesGuid,
> -                  &Snp->ExitBootServicesEvent
> -                  );
> -  if (EFI_ERROR (Status)) {
> -    goto Error_DeleteSNP;
> +  if (FixedPcdGetBool (PcdSnpCreateExitBootServicesEvent)) {
> +    //
> +    // Create EXIT_BOOT_SERIVES Event
> +    //
> +    Status = gBS->CreateEventEx (
> +                    EVT_NOTIFY_SIGNAL,
> +                    TPL_NOTIFY,
> +                    SnpNotifyExitBootServices,
> +                    Snp,
> +                    &gEfiEventExitBootServicesGuid,
> +                    &Snp->ExitBootServicesEvent
> +                    );
> +    if (EFI_ERROR (Status)) {
> +      goto Error_DeleteSNP;
> +    }
>    }
> 
>    //
> @@ -778,10 +780,12 @@ SimpleNetworkDriverStop (
>      return Status;
>    }
> 
> -  //
> -  // Close EXIT_BOOT_SERIVES Event
> -  //
> -  gBS->CloseEvent (Snp->ExitBootServicesEvent);
> +  if (FixedPcdGetBool (PcdSnpCreateExitBootServicesEvent)) {
> +    //
> +    // Close EXIT_BOOT_SERIVES Event
> +    //
> +    gBS->CloseEvent (Snp->ExitBootServicesEvent);
> +  }
> 
>    Status = gBS->CloseProtocol (
>                    Controller,
> diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec
> index 944b1d1501c0..66e500cbeaf7 100644
> --- a/NetworkPkg/NetworkPkg.dec
> +++ b/NetworkPkg/NetworkPkg.dec
> @@ -109,6 +109,13 @@
>    # @Prompt TFTP block size.
> 
> gEfiNetworkPkgTokenSpaceGuid.PcdTftpBlockSize|0x0|UINT64|0x1000000B
> 
> +  ## Indicates whether SnpDxe driver will create an event that will be
> notified
> +  # upon gBS->ExitBootServices() call.
> +  # TRUE - Event being triggered upon ExitBootServices call will be created
> +  # FALSE - Event being triggered upon ExitBootServices call will NOT be
> created
> +  # @Prompt Indicates whether SnpDxe creates event for ExitBootServices()
> call.
> +
> gEfiNetworkPkgTokenSpaceGuid.PcdSnpCreateExitBootServicesEvent|TRUE|
> BOOLEAN|0x1000000C
> +
>  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>    ## IPv6 DHCP Unique Identifier (DUID) Type configuration (From RFCs 3315
> and 6355).
>    # 01 = DUID Based on Link-layer Address Plus Time [DUID-LLT]
> diff --git a/NetworkPkg/SnpDxe/Snp.h b/NetworkPkg/SnpDxe/Snp.h
> index e6b62930397d..4f64c525e357 100644
> --- a/NetworkPkg/SnpDxe/Snp.h
> +++ b/NetworkPkg/SnpDxe/Snp.h
> @@ -26,6 +26,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/UefiLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/PrintLib.h>
> +#include <Library/PcdLib.h>
> 
>  #include <IndustryStandard/Pci.h>
>  #include <IndustryStandard/Acpi.h>
> diff --git a/NetworkPkg/SnpDxe/SnpDxe.inf
> b/NetworkPkg/SnpDxe/SnpDxe.inf
> index afeb1526cc10..79ea789c0837 100644
> --- a/NetworkPkg/SnpDxe/SnpDxe.inf
> +++ b/NetworkPkg/SnpDxe/SnpDxe.inf
> @@ -73,5 +73,8 @@
>    gEfiNetworkInterfaceIdentifierProtocolGuid_31 ## TO_START
>    gEfiPciIoProtocolGuid                         ## TO_START
> 
> +[Pcd]
> +  gEfiNetworkPkgTokenSpaceGuid.PcdSnpCreateExitBootServicesEvent   ##
> CONSUMES
> +
>  [UserExtensions.TianoCore."ExtraFiles"]
>    SnpDxeExtra.uni
> --
> 2.17.0.windows.1