Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove ExitBootServices event

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

On 10/08/19 18:16, Rabeda, Maciej wrote:
> Patch addresses Bugzilla #1972.

I think the BZ reference should be
<https://bugzilla.tianocore.org/show_bug.cgi?id=1974>. (The cover letter
has it right.)

Thanks
Laszlo

> 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.
> 
> Signed-off-by: Maciej Rabeda <maciej.rabeda@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  NetworkPkg/SnpDxe/Snp.c      | 45 --------------------
>  NetworkPkg/SnpDxe/Snp.h      |  2 -
>  NetworkPkg/SnpDxe/SnpDxe.inf |  3 --
>  3 files changed, 50 deletions(-)
> 
> diff --git a/NetworkPkg/SnpDxe/Snp.c b/NetworkPkg/SnpDxe/Snp.c
> index a23af05078bc..7646a3ce0293 100644
> --- a/NetworkPkg/SnpDxe/Snp.c
> +++ b/NetworkPkg/SnpDxe/Snp.c
> @@ -8,31 +8,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  #include "Snp.h"
>  
> -/**
> -  One notified function to stop UNDI device when gBS->ExitBootServices() called.
> -
> -  @param  Event                   Pointer to this event
> -  @param  Context                 Event handler private data
> -
> -**/
> -VOID
> -EFIAPI
> -SnpNotifyExitBootServices (
> -  EFI_EVENT Event,
> -  VOID      *Context
> -  )
> -{
> -  SNP_DRIVER             *Snp;
> -
> -  Snp  = (SNP_DRIVER *)Context;
> -
> -  //
> -  // Shutdown and stop UNDI driver
> -  //
> -  PxeShutdown (Snp);
> -  PxeStop (Snp);
> -}
> -
>  /**
>    Send command to UNDI. It does nothing currently.
>  
> @@ -647,21 +622,6 @@ 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;
> -  }
> -
>    //
>    //  add SNP to the undi handle
>    //
> @@ -778,11 +738,6 @@ SimpleNetworkDriverStop (
>      return Status;
>    }
>  
> -  //
> -  // Close EXIT_BOOT_SERIVES Event
> -  //
> -  gBS->CloseEvent (Snp->ExitBootServicesEvent);
> -
>    Status = gBS->CloseProtocol (
>                    Controller,
>                    &gEfiNetworkInterfaceIdentifierProtocolGuid_31,
> diff --git a/NetworkPkg/SnpDxe/Snp.h b/NetworkPkg/SnpDxe/Snp.h
> index e6b62930397d..f83a4f075adc 100644
> --- a/NetworkPkg/SnpDxe/Snp.h
> +++ b/NetworkPkg/SnpDxe/Snp.h
> @@ -120,8 +120,6 @@ typedef struct {
>      VOID                  *MapCookie;
>    } MapList[MAX_MAP_LENGTH];
>  
> -  EFI_EVENT              ExitBootServicesEvent;
> -
>    //
>    // Whether UNDI support reporting media status from GET_STATUS command,
>    // i.e. PXE_STATFLAGS_GET_STATUS_NO_MEDIA_SUPPORTED or
> diff --git a/NetworkPkg/SnpDxe/SnpDxe.inf b/NetworkPkg/SnpDxe/SnpDxe.inf
> index afeb1526cc10..8d045cfcf4ca 100644
> --- a/NetworkPkg/SnpDxe/SnpDxe.inf
> +++ b/NetworkPkg/SnpDxe/SnpDxe.inf
> @@ -64,9 +64,6 @@
>    DebugLib
>    NetLib
>  
> -[Guids]
> -  gEfiEventExitBootServicesGuid                 ## SOMETIMES_CONSUMES ## Event
> -
>  [Protocols]
>    gEfiSimpleNetworkProtocolGuid                 ## BY_START
>    gEfiDevicePathProtocolGuid                    ## TO_START
>