Re: [edk2] [Patch] MdeModulePkg: Fix SNP.Initialize() spec conformance issue
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; edk2-devel@lists.01.org > Cc: Fu, Siyuan > 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: edk2-devel@lists.01.org > Cc: Ye, Ting ; Fu, Siyuan > 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 > Cc: Fu Siyuan > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiaxin Wu > --- > 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. > +Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved. > 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) >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 =
Re: [edk2] [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: edk2-devel@lists.01.org Cc: Ye, Ting; Fu, Siyuan 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 Cc: Fu Siyuan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jiaxin Wu --- 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. +Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved. 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) >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
[edk2] [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 TingCc: Fu Siyuan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jiaxin Wu --- 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. +Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved. 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) >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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel