From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.groups.io with SMTP id smtpd.web12.3451.1570694774307726346 for ; Thu, 10 Oct 2019 01:06:14 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C3C4DA26664; Thu, 10 Oct 2019 08:06:13 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-48.rdu2.redhat.com [10.10.120.48]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8BC6E60BE1; Thu, 10 Oct 2019 08:06:12 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove ExitBootServices event To: "Fu, Siyuan" , "devel@edk2.groups.io" , "Rabeda, Maciej" Cc: "Wu, Jiaxin" References: <20191008161629.14668-1-maciej.rabeda@intel.com> <20191008161629.14668-2-maciej.rabeda@intel.com> From: "Laszlo Ersek" Message-ID: Date: Thu, 10 Oct 2019 10:06:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.68]); Thu, 10 Oct 2019 08:06:13 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 10/10/19 05:32, Fu, Siyuan wrote: > Hi, Maciej >=20 > Considering that this patch has to co-work with corresponding UNDI devic= e driver > bug fix, in order to avoid potential compatibility problem, please add a= PCD to=20 > 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 i= n its=20 > 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 >=20 > 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 ven= dor could > know how to fix the problem if they meet same issue. >=20 > And please correct the Bugzilla number in patch description as Laszlo po= inted out. >=20 > Thanks. >=20 > Best Regards > Siyuan=20 >=20 >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Laszlo >> Ersek >> Sent: 2019=E5=B9=B410=E6=9C=8810=E6=97=A5 6:10 >> To: devel@edk2.groups.io; Rabeda, Maciej >> Cc: Fu, Siyuan ; Wu, Jiaxin >> 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 >> . (The cover let= ter >> 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 >>> Cc: Siyuan Fu >>> Cc: Jiaxin Wu >>> --- >>> 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->ExitBootService= s() >> 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 =3D (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 =3D 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 =3D 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 >>> >> >> >>=20 >=20