One very minor formatting issue inline (that can be sorted during integration):

On 2020.07.12 05:28, Andrei Warkentin wrote:
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.warken...@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;

If we want to be pedantic about EDK2 formatting, this should be indented to start after the "Ge" of GenetMmioRead above, like the previous calls.

+
+  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;
  }


Reviewed-by: Pete Batard <p...@akeo.ie>

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

View/Reply Online (#62421): https://edk2.groups.io/g/devel/message/62421
Mute This Topic: https://groups.io/mt/75452213/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to