[edk2] [PATCH v2] MdeModulePkg: Fix SNP.Initialize() spec conformance issue

2016-05-30 Thread Jiaxin Wu
v2:
*Refine the coding style according edk2 community's feedback.

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 | 33 ++
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
index 2151375..63bdf92 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,34 @@ 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;
+  } else {
+Snp->Mode.MediaPresent = FALSE;
+  }
+}
+
+Snp->Mode.State   = EfiSimpleNetworkInitialized;
+Status= EFI_SUCCESS;
   } else {
 DEBUG (
   (EFI_D_WARN,
   "\nSnp->undi.initialize()  %xh:%xh\n",
   Snp->Cdb.StatFlags,
@@ -232,11 +254,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


Re: [edk2] [PATCH v2] MdeModulePkg: Fix SNP.Initialize() spec conformance issue

2016-05-30 Thread Wu, Jiaxin
Hi All,
This patch may bring  interoperability issues to the NIC cards which do not 
comply with the UNDI definitions in UEFI spec (Appendix E). We have validated 
some Intel NICs and they worked probably with this patch. If anyone in the 
mailing list meet the interoperability issue with this patch. Please raise to 
us.

Thanks and Best Regards!
Jiaxin

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Tuesday, May 31, 2016 10:34 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan 
> Subject: [edk2] [PATCH v2] MdeModulePkg: Fix SNP.Initialize() spec
> conformance issue
> 
> v2:
> *Refine the coding style according edk2 community's feedback.
> 
> 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 | 33
> ++
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
> b/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
> index 2151375..63bdf92 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,34 @@ 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;
> +  } else {
> +Snp->Mode.MediaPresent = FALSE;
> +  }
> +}
> +
> +Snp->Mode.State   = EfiSimpleNetworkInitialized;
> +Status= E

Re: [edk2] [PATCH v2] MdeModulePkg: Fix SNP.Initialize() spec conformance issue

2016-05-31 Thread Ye, Ting
Reviewed-by: Ye Ting  

-Original Message-
From: Wu, Jiaxin 
Sent: Tuesday, May 31, 2016 10:34 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan 
Subject: [PATCH v2] MdeModulePkg: Fix SNP.Initialize() spec conformance issue

v2:
*Refine the coding style according edk2 community's feedback.

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 | 33 ++
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
index 2151375..63bdf92 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,34 @@ 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;
+  } else {
+Snp->Mode.MediaPresent = FALSE;
+  }
+}
+
+Snp->Mode.State   = EfiSimpleNetworkInitialized;
+Status= EFI_SUCCESS;
   } else {
 DEBUG (
   (EFI_D_WARN,
   "\nSnp->undi.initialize()  %xh:%xh\n",
   Snp->Cdb.StatFlags,
@@ -232,11 +254,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