From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mx.groups.io with SMTP id smtpd.web10.3045.1570611027224192656 for ; Wed, 09 Oct 2019 01:50:27 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.24, mailfrom: maciej.rabeda@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Oct 2019 01:50:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,273,1566889200"; d="scan'208";a="277363648" Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96]) by orsmga001.jf.intel.com with ESMTP; 09 Oct 2019 01:50:24 -0700 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.103]) by IRSMSX154.ger.corp.intel.com ([169.254.12.160]) with mapi id 14.03.0439.000; Wed, 9 Oct 2019 09:50:24 +0100 From: "Rabeda, Maciej" To: "Fu, Siyuan" , "devel@edk2.groups.io" CC: "Wu, Jiaxin" Subject: Re: [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove ExitBootServices event Thread-Topic: [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove ExitBootServices event Thread-Index: AQHVffPBBqOFhuQL+0i7oYbgQy73C6dRjx8AgABvIMA= Date: Wed, 9 Oct 2019 08:50:23 +0000 Message-ID: <40FBBD5C52E8B4429773266A45BDCC174C665110@IRSMSX104.ger.corp.intel.com> References: <20191008161629.14668-1-maciej.rabeda@intel.com> <20191008161629.14668-2-maciej.rabeda@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTgzMzQwYmItMWYyNS00ODIxLWE4OWYtZWRiZjA1NmQ5ZWJkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUW9vWU9LUDZZZlpiXC93T283eTJpSm9iZUFqeWdXTFA4K1dXaFwvMEFLSDIzazJ4OHhyNUkxd0ZLOFZQaWdjcHRyIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [163.33.239.181] MIME-Version: 1.0 Content-Language: en-US Content-Type: text/plain; charset="iso-2022-jp" Content-Transfer-Encoding: quoted-printable Hi Siyuan, This change has no effect to Intel Ethernet Server UNDI drivers. They already handle ExitBootServices event and configure the Ethernet adapt= ers not to perform any DMA at this point. As per whitepaper (https://firmware.intel.com/sites/default/files/Intel_Whi= tePaper_Using_IOMMU_for_DMA_Protection_in_UEFI.pdf), section Call to Action= , UEFI device drivers (including UNDI) should put the controller to halt st= ate and disable bus mastering in ExitBootServices stage - which obliges UND= I drivers to have ExitBootServices event handlers. To me, this means shutti= ng down DMA on the controllers to which UNDI driver is bound. As for second question, not disabling DMA will make our adapters continue w= riting Rx packet data to RAM in DXE Runtime stage. I believe that this behavior is as far from desired as it can. Thanks! Maciej Rabeda -----Original Message----- From: Fu, Siyuan = Sent: Wednesday, October 9, 2019 04:08 To: Rabeda, Maciej ; devel@edk2.groups.io Cc: Wu, Jiaxin Subject: RE: [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove ExitBootServices event Hi, Maciej Have you tested what will happen if this SNP co-work with those UNDI driver= s which don't have an ExitBootService event callback to shut down its DMA a= ctivity? And what's the impact to OS if UNDI's DMA is not shut down? Best Regards Siyuan = > -----Original Message----- > From: Rabeda, Maciej > Sent: 2019=1B$BG/=1B(B10=1B$B7n=1B(B9=1B$BF|=1B(B 0:16 > To: devel@edk2.groups.io > Cc: Fu, Siyuan ; Wu, Jiaxin > Subject: [PATCH v1 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. > = > 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->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 =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 > -- > 2.17.0.windows.1 -------------------------------------------------------------------- Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydz= ial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-31= 6 | Kapital zakladowy 200.000 PLN. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata= i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wi= adomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiek= olwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the s= ole use of the intended recipient(s). If you are not the intended recipient= , please contact the sender and delete all copies; any review or distributi= on by others is strictly prohibited.