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):
Will do, thanks for highlighting (and review). / Leif > 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 (#62495): https://edk2.groups.io/g/devel/message/62495 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] -=-=-=-=-=-=-=-=-=-=-=-