Thanks Ting, version2 patch will be sent out later. After that, I will highlight it with your comments again.
Thanks. Jiaxin > -----Original Message----- > From: Ye, Ting > Sent: Thursday, May 26, 2016 2:52 PM > To: Wu, Jiaxin <[email protected]>; [email protected] > Cc: Fu, Siyuan <[email protected]> > Subject: RE: [Patch] MdeModulePkg: Fix SNP.Initialize() spec conformance > issue > > Jiaxin, > > It looks to me the three places of below code can be merged together. > Please double check. > > + Snp->Mode.State = EfiSimpleNetworkInitialized; > + Status = EFI_SUCCESS; > > Also, I would like to highlight that the patch might bring interoperability > issues to the NIC cards which do not comply with the UNDI definitions in UEFI > spec. We validated some Intel NICs and they worked probably with this patch. > If anyone in the mailing list meet the interoperability issue please raise to > us. > > Thanks, > Ye Ting > > -----Original Message----- > From: Wu, Jiaxin > Sent: Thursday, May 26, 2016 9:10 AM > To: [email protected] > Cc: Ye, Ting <[email protected]>; Fu, Siyuan <[email protected]> > Subject: [Patch] MdeModulePkg: Fix SNP.Initialize() spec conformance issue > > Current SNP UNDI Initialize command does not follow the UEFI Spec to > update the SNP MediaPresent field. The result for the Initialize command > execution check should be: > StatFlags: (1) Monitor the upper two bits (14 & 15) in the field to know > whether the command has been executed by the UNDI (Not started, > Queued, Error, Complete). (2) Check the other field to see if there is an > active connection to this network device (used to update MediaPresent). > StatCode: After command execution completes, either successfully or not, > this field contains the result of the command execution (success or failure). > This patch is used to fix it. > > NOTE: If any UNDI driver does not follow the UEFI Spec for the media status > update, it may meet failure with this more conditions check (StatFlags). > > Cc: Ye Ting <[email protected]> > Cc: Fu Siyuan <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiaxin Wu <[email protected]> > --- > MdeModulePkg/Universal/Network/SnpDxe/Initialize.c | 37 > ++++++++++++++++++---- > 1 file changed, 31 insertions(+), 6 deletions(-) > > diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c > b/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c > index 2151375..0c292a5 100644 > --- a/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c > +++ b/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c > @@ -1,9 +1,9 @@ > /** @file > Implementation of initializing a network adapter. > > -Copyright (c) 2004 - 2008, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR> > This program and the accompanying materials are licensed and made > available under the terms and conditions of the BSD License which > accompanies this distribution. The full text of the license may be found at > http://opensource.org/licenses/bsd-license.php > > @@ -35,10 +35,12 @@ PxeInit ( > { > PXE_CPB_INITIALIZE *Cpb; > VOID *Addr; > EFI_STATUS Status; > > + Status = EFI_SUCCESS; > + > Cpb = Snp->Cpb; > if (Snp->TxRxBufferSize != 0) { > Status = Snp->PciIo->AllocateBuffer ( > Snp->PciIo, > AllocateAnyPages, @@ -97,14 +99,38 @@ PxeInit ( > > DEBUG ((EFI_D_NET, "\nSnp->undi.initialize() ")); > > (*Snp->IssueUndi32Command) ((UINT64)(UINTN) &Snp->Cdb); > > - if (Snp->Cdb.StatCode == PXE_STATCODE_SUCCESS) { > - Snp->Mode.State = EfiSimpleNetworkInitialized; > - > - Status = EFI_SUCCESS; > + // > + // There are two fields need to be checked here: > + // First is the upper two bits (14 & 15) in the CDB.StatFlags field. > + Until these bits change to report // > PXE_STATFLAGS_COMMAND_COMPLETE or > PXE_STATFLAGS_COMMAND_FAILED, the command has not been executed > by the UNDI. > + // Second is the CDB.StatCode field. After command execution > + completes, either successfully or not, // the CDB.StatCode field contains > the result of the command execution. > + // > + if ((((Snp->Cdb.StatFlags) & PXE_STATFLAGS_STATUS_MASK) == > PXE_STATFLAGS_COMMAND_COMPLETE) && > + (Snp->Cdb.StatCode == PXE_STATCODE_SUCCESS)) { > + // > + // If cable detect feature is enabled in CDB.OpFlags, check the > CDB.StatFlags to see if there is an > + // active connection to this network device. If the no media StatFlag is > set, > the UNDI and network > + // device are still initialized. > + // > + if (CableDetectFlag == PXE_OPFLAGS_INITIALIZE_DETECT_CABLE) { > + if(((Snp->Cdb.StatFlags) & PXE_STATFLAGS_INITIALIZED_NO_MEDIA) != > PXE_STATFLAGS_INITIALIZED_NO_MEDIA) { > + Snp->Mode.MediaPresent = TRUE; > + Snp->Mode.State = EfiSimpleNetworkInitialized; > + Status = EFI_SUCCESS; > + } else { > + Snp->Mode.MediaPresent = FALSE; > + Snp->Mode.State = EfiSimpleNetworkInitialized; > + Status = EFI_SUCCESS; > + } > + } else { > + Snp->Mode.State = EfiSimpleNetworkInitialized; > + Status = EFI_SUCCESS; > + } > } else { > DEBUG ( > (EFI_D_WARN, > "\nSnp->undi.initialize() %xh:%xh\n", > Snp->Cdb.StatFlags, > @@ -232,11 +258,10 @@ SnpUndi32Initialize ( > // > // If UNDI support cable detect for INITIALIZE command, try it first. > // > if (Snp->CableDetectSupported) { > if (PxeInit (Snp, PXE_OPFLAGS_INITIALIZE_DETECT_CABLE) == > EFI_SUCCESS) { > - Snp->Mode.MediaPresent = TRUE; > goto ON_EXIT; > } > } > > Snp->Mode.MediaPresent = FALSE; > -- > 1.9.5.msysgit.1 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
