[edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove ExitBootServices event
Siyuan, Fu
siyuan.fu at intel.com
Thu Oct 10 09:29:31 UTC 2019
> -----Original Message-----
> From: Laszlo Ersek <lersek at redhat.com>
> Sent: 2019年10月10日 16:06
> To: Fu, Siyuan <siyuan.fu at intel.com>; devel at edk2.groups.io; Rabeda,
> Maciej <maciej.rabeda at intel.com>
> Cc: Wu, Jiaxin <jiaxin.wu at 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Laszlo
> >> Ersek
> >> Sent: 2019年10月10日 6:10
> >> To: devel at edk2.groups.io; Rabeda, Maciej <maciej.rabeda at intel.com>
> >> Cc: Fu, Siyuan <siyuan.fu at intel.com>; Wu, Jiaxin <jiaxin.wu at 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.rabeda at intel.com>
> >>> Cc: Siyuan Fu <siyuan.fu at intel.com>
> >>> Cc: Jiaxin Wu <jiaxin.wu at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list