On 2020.07.14 14:36, Leif Lindholm wrote:
On Mon, Jul 13, 2020 at 12:25:18 +0100, Pete Batard wrote:
One very minor formatting issue inline (that can be sorted during
integration):
I would also like to prepend "Silicon/" to the subject line.
Please do. It should have been there in the first place.
Regards,
/Pete
If I can do those two things:
Reviewed-by: Leif Lindholm <l...@nuviainc.com>
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 (#62502): https://edk2.groups.io/g/devel/message/62502
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]
-=-=-=-=-=-=-=-=-=-=-=-
