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"? 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 (#48710): https://edk2.groups.io/g/devel/message/48710 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] -=-=-=-=-=-=-=-=-=-=-=-
