OpenPlatformPkg/Lan91xDxe: Fixed driver crash and TX recycle buffer16.06-daniil-001

Fixed driver crash on ARP packets. Fixed TX recycle buffer overflow.
Added prototype for SnpWaitForPacketNotify SNP call and notify event.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>

1 files changed, 118 insertions, 61 deletions

diff --git a/Drivers/Net/Lan91xDxe/Lan91xDxe.c b/Drivers/Net/Lan91xDxe/Lan91xDxe.c
index 3f857a5..7c903f7 100644
--- a/Drivers/Net/Lan91xDxe/Lan91xDxe.c
+++ b/Drivers/Net/Lan91xDxe/Lan91xDxe.c
@@ -65,10 +65,7 @@ typedef struct _LAN91X_DRIVER {
   EFI_NETWORK_STATISTICS Stats;
 
   // Transmit Buffer recycle queue
-#define TX_QUEUE_DEPTH 16
-  VOID              *TxQueue[TX_QUEUE_DEPTH];
-  UINTN             TxQueHead;
-  UINTN             TxQueTail;
+  LIST_ENTRY TransmitQueueHead;
 
   // Register access variables
   UINTN             IoBase;             // I/O Base Address
@@ -137,47 +134,18 @@ STATIC CHAR16 CONST * CONST ChipIds[ 16 ] =  {
   NULL, NULL, NULL
 };
 
+typedef struct {
+  VOID            *Buf;
+  UINTN           Length;
+} MSK_SYSTEM_BUF;
 
-/* ------------------ TxBuffer Queue functions ------------------- */
-
-#define TxQueNext(off)  ((((off) + 1) >= TX_QUEUE_DEPTH) ? 0 : ((off) + 1))
-
-STATIC
-BOOLEAN
-TxQueInsert (
-    IN    LAN91X_DRIVER *LanDriver,
-    IN    VOID          *Buffer
-    )
-{
-
-  if (TxQueNext (LanDriver->TxQueTail) == LanDriver->TxQueHead) {
-    return FALSE;
-  }
-
-  LanDriver->TxQueue[LanDriver->TxQueTail] = Buffer;
-  LanDriver->TxQueTail = TxQueNext (LanDriver->TxQueTail);
-
-  return TRUE;
-}
-
-STATIC
-VOID
-*TxQueRemove (
-    IN    LAN91X_DRIVER *LanDriver
-    )
-{
-  VOID *Buffer;
-
-  if (LanDriver->TxQueTail == LanDriver->TxQueHead) {
-    return NULL;
-  }
-
-  Buffer = LanDriver->TxQueue[LanDriver->TxQueHead];
-  LanDriver->TxQueue[LanDriver->TxQueHead] = NULL;
-  LanDriver->TxQueHead = TxQueNext (LanDriver->TxQueHead);
+typedef struct {
+  UINTN           Signature;
+  LIST_ENTRY      Link;
+  MSK_SYSTEM_BUF  SystemBuf;
+} MSK_LINKED_SYSTEM_BUF;
 
-  return Buffer;
-}
+#define TX_MBUF_SIGNATURE  SIGNATURE_32 ('t','x','m','b')
 
 /* ------------------ MAC Address Hash Calculations ------------------- */
 
@@ -1105,6 +1073,42 @@ exit_unlock:
   return Status;
 }
 
+/**
+  Notification call back function for WaitForPacket event.
+
+   <at> param  Event       EFI Event.
+   <at> param  SnpPtr      Pointer to YUKON_DRIVER structure.
+
+**/
+VOID
+EFIAPI
+SnpWaitForPacketNotify (
+    EFI_EVENT     Event,
+    VOID          *SnpPtr
+    )
+{
+  DEBUG ((EFI_D_NET, "LAN91x: SnpWaitForPacketNotify()\n"));
+  //
+  // Do nothing if either parameter is a NULL pointer.
+  //
+  if (Event == NULL || SnpPtr == NULL) {
+    return ;
+  }
+  //
+  // Do nothing if the SNP interface is not initialized.
+  //
+  switch (((LAN91X_DRIVER *) SnpPtr)->SnpMode.State) {
+    case EfiSimpleNetworkInitialized:
+      break;
+
+    case EfiSimpleNetworkStopped:
+    case EfiSimpleNetworkStarted:
+    default:
+      return ;
+  }
+
+}
+
 /*
 **  UEFI Initialize() function
 **
@@ -1148,10 +1152,25 @@ SnpInitialize (
   // Find the LanDriver structure
   LanDriver = INSTANCE_FROM_SNP_THIS(Snp);
 
+  Status = gBS->CreateEvent (
+        EVT_NOTIFY_WAIT,
+        TPL_NOTIFY,
+        &SnpWaitForPacketNotify,
+        LanDriver,
+        &LanDriver->Snp.WaitForPacket
+        );
+
+  if (EFI_ERROR (Status)) {
+    LanDriver->Snp.WaitForPacket = NULL;
+    Status = EFI_DEVICE_ERROR;
+    ReturnUnlock (EFI_DEVICE_ERROR);
+  }
+
   // Initiate a software reset
   Status = SoftReset (LanDriver);
   if (EFI_ERROR(Status)) {
     DEBUG((EFI_D_WARN, "LAN91x: Soft reset failed\n"));
+    gBS->CloseEvent (LanDriver->Snp.WaitForPacket);
     ReturnUnlock (EFI_DEVICE_ERROR);
   }
 
@@ -1159,6 +1178,7 @@ SnpInitialize (
   if (PhySoftReset (LanDriver) < 0) {
     Snp->Mode->State = EfiSimpleNetworkStopped;
     DEBUG((EFI_D_WARN, "LAN91x: PHY soft reset timeout\n"));
+    gBS->CloseEvent (LanDriver->Snp.WaitForPacket);
     ReturnUnlock (EFI_NOT_STARTED);
   }
 
@@ -1643,11 +1663,12 @@ SnpGetStatus (
       OUT   VOID    **TxBuff    OPTIONAL
   )
 {
-  LAN91X_DRIVER   *LanDriver;
-  EFI_TPL          SavedTpl;
-  EFI_STATUS       Status;
-  BOOLEAN          MediaPresent;
-  UINT8            IstReg;
+  LAN91X_DRIVER         *LanDriver;
+  EFI_TPL               SavedTpl;
+  EFI_STATUS            Status;
+  BOOLEAN               MediaPresent;
+  UINT8                 IstReg;
+  MSK_LINKED_SYSTEM_BUF *m_head;
 
   // Check preliminaries
   if (Snp == NULL) {
@@ -1689,8 +1710,21 @@ SnpGetStatus (
   }
 
   // Pass back the completed buffer address
+  // The transmit buffer status is not read when TxBuf is NULL
   if (TxBuff != NULL) {
-    *TxBuff = TxQueRemove (LanDriver);
+    *((UINT8 **) TxBuff) = (UINT8 *) 0;
+    if( !IsListEmpty (&LanDriver->TransmitQueueHead))
+    {
+      m_head = CR (GetFirstNode (&LanDriver->TransmitQueueHead), MSK_LINKED_SYSTEM_BUF, Link, TX_MBUF_SIGNATURE);
+      if(m_head != NULL) {
+        *TxBuff = m_head->SystemBuf.Buf;
+        RemoveEntryList (&m_head->Link);
+        FreePool (m_head);
+      }
+    }
+
+  //  if (TxBuff != NULL) {
+  //  *TxBuff = TxQueRemove (LanDriver);
   }
 
   // Update the media status
@@ -1733,6 +1767,8 @@ SnpTransmit (
   UINTN            Retries;
   UINT16           Proto;
   UINT8            PktNum;
+  MSK_LINKED_SYSTEM_BUF   *LinkedSystemBuf;
+
 
   // Check preliminaries
   if ((Snp == NULL) || (BufAddr == NULL)) {
@@ -1818,12 +1854,18 @@ SnpTransmit (
   PktNum &= ARR_PACKET;
 
   // Check for the nature of the frame
-  if (DstAddr->Addr[0] == 0xFF) {
-    LanDriver->Stats.TxBroadcastFrames += 1;
-  } else if ((DstAddr->Addr[0] & 0x1) == 1) {
-    LanDriver->Stats.TxMulticastFrames += 1;
+  // If no destination address, it's ARP broadcast
+  if(DstAddr != NULL)
+  {
+    if (DstAddr->Addr[0] == 0xFF) {
+      LanDriver->Stats.TxBroadcastFrames += 1;
+    } else if ((DstAddr->Addr[0] & 0x1) == 1) {
+      LanDriver->Stats.TxMulticastFrames += 1;
+    } else {
+      LanDriver->Stats.TxUnicastFrames += 1;
+    }
   } else {
-    LanDriver->Stats.TxUnicastFrames += 1;
+    LanDriver->Stats.TxBroadcastFrames += 1;
   }
 
   // Set the Packet Number and Pointer registers
@@ -1877,14 +1919,27 @@ SnpTransmit (
     ReturnUnlock (EFI_DEVICE_ERROR);
   }
 
-  // Update the Rx statistics
+  // Update the Tx statistics
   LanDriver->Stats.TxTotalBytes += BufSize;
   LanDriver->Stats.TxGoodFrames += 1;
 
   // Update the Tx Buffer cache
-  if (!TxQueInsert (LanDriver, BufAddr)) {
-    DEBUG((EFI_D_WARN, "LAN91x: SnpTransmit(): TxQueue insert failure.\n"));
-  }
+  LinkedSystemBuf = AllocateZeroPool (sizeof (MSK_LINKED_SYSTEM_BUF));
+  if (LinkedSystemBuf == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+  LinkedSystemBuf->Signature = TX_MBUF_SIGNATURE;
+  //
+  // Add the passed Buffer to the transmit queue. Don't copy.
+  //
+  LinkedSystemBuf->SystemBuf.Buf = BufAddr;
+  LinkedSystemBuf->SystemBuf.Length = BufSize;
+  InsertTailList (&LanDriver->TransmitQueueHead, &LinkedSystemBuf->Link);
+
+ // if (!TxQueInsert (LanDriver, BufAddr)) {
+ //   DEBUG((EFI_D_WARN, "LAN91x: SnpTransmit(): TxQueue insert failure.\n"));
+ // }
+
   Status = EFI_SUCCESS;
 
   // Dump the packet header
@@ -1941,7 +1996,7 @@ SnpReceive (
   // Serialize access to data and registers
   SavedTpl = gBS->RaiseTPL (LAN91X_TPL);
 
-  // Check that driver was started and initialised
+  // Check that driver was started and initialized
   switch (Snp->Mode->State) {
   case EfiSimpleNetworkInitialized:
     break;
@@ -2104,7 +2159,6 @@ exit_unlock:
   return Status;
 }
 
-
 /*------------------ Driver Execution Environment main entry point ------------------*/
 
 /*
@@ -2155,9 +2209,12 @@ Lan91xDxeEntry (
   PrintPhyRegisters (LanDriver);
 #endif
 
+  // Initialize transmit queue
+  InitializeListHead (&LanDriver->TransmitQueueHead);
+
   // Assign fields and func pointers
   Snp->Revision = EFI_SIMPLE_NETWORK_PROTOCOL_REVISION;
-  Snp->WaitForPacket = NULL;
+  Snp->WaitForPacket = SnpWaitForPacketNotify;
   Snp->Initialize = SnpInitialize;
   Snp->Start = SnpStart;
   Snp->Stop = SnpStop;