> -----Original Message----- > From: Laszlo Ersek <ler...@redhat.com> > Sent: 2019年10月10日 16:06 > To: Fu, Siyuan <siyuan...@intel.com>; devel@edk2.groups.io; Rabeda, > Maciej <maciej.rab...@intel.com> > Cc: Wu, Jiaxin <jiaxin...@intel.com> > Subject: Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove > ExitBootServices event > > On 10/10/19 05:32, Fu, Siyuan wrote: > > Hi, Maciej > > > > Considering that this patch has to co-work with corresponding UNDI device > driver > > bug fix, in order to avoid potential compatibility problem, please add a PCD > to > > NetworkPkg for this fix, and set the default value to disable state (no > behavior > > change). The platform which need this fix could set the PCD to enable in > its > > platform DSC. > > Should the new PCD go into "NetworkPkg/NetworkPcds.dsc.inc", and be > gated with a new build flag defined in "NetworkPkg/NetworkDefines.dsc.inc"?
Currently not all the network package PCDs are listed in the NetworkPcds.dsc.inc, But I do not oppose it if you think this PCD should be controlled by a build flag. Thanks, Siyuan > > Thanks > Laszlo > > > > > Please add clear description for the problem, new PCD, the SNP fix, and > also the > > expected UNDI driver fix in Bugzilla 1974. So 3rd party UNDI device vendor > could > > know how to fix the problem if they meet same issue. > > > > And please correct the Bugzilla number in patch description as Laszlo > pointed out. > > > > Thanks. > > > > Best Regards > > Siyuan > > > >> -----Original Message----- > >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo > >> Ersek > >> Sent: 2019年10月10日 6:10 > >> To: devel@edk2.groups.io; Rabeda, Maciej <maciej.rab...@intel.com> > >> Cc: Fu, Siyuan <siyuan...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com> > >> Subject: Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove > >> ExitBootServices event > >> > >> 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.rab...@intel.com> > >>> Cc: Siyuan Fu <siyuan...@intel.com> > >>> Cc: Jiaxin Wu <jiaxin...@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 > >>> > >> > >> > >> > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48717): https://edk2.groups.io/g/devel/message/48717 Mute This Topic: https://groups.io/mt/34444145/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
