[edk2-devel] [edk2][PATCH 1/1] BcmGenetDxe: don't consume RX buffer until it's actually copied

[Andrei Warkentin]

This was originally a bit sloppy, and could hypothetically under heavy
load result in a buffer being overwritten by hardware before the received
buffer is copied.

Signed-off-by: Andrei Warkentin <andrey.warkentin at gmail.com>
---
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h   | 15 +++++
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c     | 59 +++++++++++++++-----
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 26 ++++++---
 3 files changed, 77 insertions(+), 23 deletions(-)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
index 1a117b25..b39a1326 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
@@ -358,6 +358,16 @@ GenetTxIntr (
   OUT VOID              **TxBuf
   );
 
+UINT32
+GenetRxPending (
+  IN  GENET_PRIVATE_DATA *Genet
+  );
+
+UINT32
+GenetTxPending (
+  IN  GENET_PRIVATE_DATA *Genet
+  );
+
 EFI_STATUS
 GenetRxIntr (
   IN GENET_PRIVATE_DATA *Genet,
@@ -365,4 +375,9 @@ GenetRxIntr (
   OUT UINTN             *FrameLength
   );
 
+VOID
+GenetRxComplete (
+  IN GENET_PRIVATE_DATA *Genet
+  );
+
 #endif /* GENET_UTIL_H__ */
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c
index 1c4c8527..a0097b0d 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c
@@ -661,6 +661,7 @@ GenetDmaMapRxDescriptor (
     Genet->RxBufferMap[DescIndex].PhysAddress & 0xFFFFFFFF);
   GenetMmioWrite (Genet, GENET_RX_DESC_ADDRESS_HI (DescIndex),
     (Genet->RxBufferMap[DescIndex].PhysAddress >> 32) & 0xFFFFFFFF);
+  GenetMmioWrite (Genet, GENET_RX_DESC_STATUS (DescIndex), 0);
 
   return EFI_SUCCESS;
 }
@@ -753,12 +754,9 @@ GenetTxIntr (
   OUT VOID               **TxBuf
   )
 {
-  UINT32  ConsIndex, Total;
+  UINT32 Total;
 
-  ConsIndex = GenetMmioRead (Genet,
-                GENET_TX_DMA_CONS_INDEX (GENET_DMA_DEFAULT_QUEUE)) & 0xFFFF;
-
-  Total = (ConsIndex - Genet->TxConsIndex) & 0xFFFF;
+  Total = GenetTxPending (Genet);
   if (Genet->TxQueued > 0 && Total > 0) {
     DmaUnmap (Genet->TxBufferMap[Genet->TxNext]);
     *TxBuf = Genet->TxBuffer[Genet->TxNext];
@@ -770,6 +768,46 @@ GenetTxIntr (
   }
 }
 
+UINT32
+GenetRxPending (
+  IN  GENET_PRIVATE_DATA *Genet
+  )
+{
+  UINT32 ProdIndex;
+  UINT32 ConsIndex;
+
+  ConsIndex = GenetMmioRead (Genet,
+                GENET_RX_DMA_CONS_INDEX (GENET_DMA_DEFAULT_QUEUE)) & 0xFFFF;
+  ASSERT (ConsIndex == Genet->RxConsIndex);
+
+  ProdIndex = GenetMmioRead (Genet,
+                GENET_RX_DMA_PROD_INDEX (GENET_DMA_DEFAULT_QUEUE)) & 0xFFFF;
+  return (ProdIndex - Genet->RxConsIndex) & 0xFFFF;
+}
+
+UINT32
+GenetTxPending (
+  IN  GENET_PRIVATE_DATA *Genet
+  )
+{
+  UINT32 ConsIndex;
+
+  ConsIndex = GenetMmioRead (Genet,
+     GENET_TX_DMA_CONS_INDEX (GENET_DMA_DEFAULT_QUEUE)) & 0xFFFF;
+
+  return (ConsIndex - Genet->TxConsIndex) & 0xFFFF;
+}
+
+VOID
+GenetRxComplete (
+  IN GENET_PRIVATE_DATA *Genet
+  )
+{
+  Genet->RxConsIndex = (Genet->RxConsIndex + 1) & 0xFFFF;
+  GenetMmioWrite (Genet, GENET_RX_DMA_CONS_INDEX (GENET_DMA_DEFAULT_QUEUE),
+                  Genet->RxConsIndex);
+}
+
 /**
   Simulate an "RX interrupt", returning the index of a completed RX buffer and
   corresponding frame length.
@@ -790,21 +828,14 @@ GenetRxIntr (
   )
 {
   EFI_STATUS    Status;
-  UINT32        ProdIndex, Total;
+  UINT32        Total;
   UINT32        DescStatus;
 
-  ProdIndex = GenetMmioRead (Genet,
-                GENET_RX_DMA_PROD_INDEX (GENET_DMA_DEFAULT_QUEUE)) & 0xFFFF;
-
-  Total = (ProdIndex - Genet->RxConsIndex) & 0xFFFF;
+  Total = GenetRxPending (Genet);
   if (Total > 0) {
     *DescIndex = Genet->RxConsIndex % GENET_DMA_DESC_COUNT;
     DescStatus = GenetMmioRead (Genet, GENET_RX_DESC_STATUS (*DescIndex));
     *FrameLength = SHIFTOUT (DescStatus, GENET_RX_DESC_STATUS_BUFLEN);
-
-    Genet->RxConsIndex = (Genet->RxConsIndex + 1) & 0xFFFF;
-    GenetMmioWrite (Genet, GENET_RX_DMA_CONS_INDEX (GENET_DMA_DEFAULT_QUEUE),
-      Genet->RxConsIndex);
     Status = EFI_SUCCESS;
   } else {
     Status = EFI_NOT_READY;
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
index 371216ca..1bda18f1 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
@@ -502,9 +502,19 @@ GenetSimpleNetworkGetStatus (
     Genet->SnpMode.MediaPresent = FALSE;
   } else {
     Genet->SnpMode.MediaPresent = TRUE;
+  }
+
+  if (TxBuf != NULL) {
+    GenetTxIntr (Genet, TxBuf);
+  }
 
-    if (TxBuf != NULL) {
-      GenetTxIntr (Genet, TxBuf);
+  if (InterruptStatus != NULL) {
+    *InterruptStatus = 0;
+    if (GenetRxPending (Genet) > 0) {
+      *InterruptStatus |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
+    }
+    if (GenetTxPending (Genet) > 0) {
+      *InterruptStatus |= EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
     }
   }
 
@@ -741,13 +751,8 @@ GenetSimpleNetworkReceive (
       DEBUG ((DEBUG_ERROR,
         "%a: Buffer size (0x%X) is too small for frame (0x%X)\n",
         __FUNCTION__, *BufferSize, FrameLength));
-      Status = GenetDmaMapRxDescriptor (Genet, DescIndex);
-      if (EFI_ERROR (Status)) {
-        DEBUG ((DEBUG_ERROR, "%a: Failed to remap RX descriptor!\n",
-          __FUNCTION__));
-      }
-      EfiReleaseLock (&Genet->Lock);
-      return EFI_BUFFER_TOO_SMALL;
+      Status = EFI_BUFFER_TOO_SMALL;
+      goto out;
     }
 
     if (DestAddr != NULL) {
@@ -773,11 +778,14 @@ GenetSimpleNetworkReceive (
     Status = EFI_NOT_READY;
   }
 
+out:
   Status = GenetDmaMapRxDescriptor (Genet, DescIndex);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a: Failed to remap RX descriptor!\n", __FUNCTION__));
   }
 
+  GenetRxComplete (Genet);
+
   EfiReleaseLock (&Genet->Lock);
   return Status;
 }
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62372): https://edk2.groups.io/g/devel/message/62372
Mute This Topic: https://groups.io/mt/75452213/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-