From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f67.google.com (mail-pj1-f67.google.com [209.85.216.67]) by mx.groups.io with SMTP id smtpd.web11.3402.1594528138945895918 for ; Sat, 11 Jul 2020 21:28:59 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=p6yzXrtj; spf=pass (domain: gmail.com, ip: 209.85.216.67, mailfrom: andrey.warkentin@gmail.com) Received: by mail-pj1-f67.google.com with SMTP id k5so4573402pjg.3 for ; Sat, 11 Jul 2020 21:28:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=xab+IZ/iy9bRYwJlaeHVaNt5jZZJRWVGEmmWB5a4Lh8=; b=p6yzXrtjSuziRWcDk6taC+SNvL46Yxn7Y44cY1nY1A6/YQNKo0NPDNUC1mKG7meTAm vxBm50WFKW3Myli4Mc5ZsxUke2SHf3Ow6vufGrv3OVDhqNTFoO4TY/gWU3sOJmjOZcUY z1UiTCg6kIGqfW+21xTbHCjc/S89SzcqxKnQ/T5Nt04lu87KHU3m/fGO/Bll/o3Ma4qM fTpf+xHHJk24Ss0hY40Jbszt0oQi8qceOPPKmVwmE81PBwrbMk7NQVUtPRmDLoW9uZZF K1wDlM76+NPrG9O9E6ZhpoMmDf4f8bR25/OSaP2kJgClOnImWS3SKNHbS9N8cJgfrYm/ xjBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=xab+IZ/iy9bRYwJlaeHVaNt5jZZJRWVGEmmWB5a4Lh8=; b=MVlyuAqfg3OPUPS/n2QrQoTYEtOIQ5DAuuGuqQsxqU+JZco8YpCyKOmLYFAuLOs1if s2zscz2YdgiVgcDkzeNdR5rH9MbxeRiooe3IDLJgkQ7Yk5CZLRhn/PYJ0UjVYPQRTQF1 jTZiRzd2l1HQOo1V59StEdHZT/UcSBqcNrbaHoh0762d49GhFWBQ06H3XceT/4DAaNFm tCdbEDSC8PR9GE7MTLazVzSczJfATkKlyJeJyB1h6WPpqoQaKPB03OB59XvX+G+zLMiu EgMdMBKQFRP7A/dpPgjQKJlmYcOVeJv2AwUszlr0U79iqOckX3oIa0NIZxDW3vpmZ3Tp 6AVw== X-Gm-Message-State: AOAM532uaUbexzz3N5BeMN31oPAcdiazW+qdSxM0a+WoL12hxjYAzy3E Sdktf69uIaG12Sfdeiq0j/3GIWqa X-Google-Smtp-Source: ABdhPJx0cfjoYhotoHDgW1lNmFJRPZXiif0YM7FXTX5TeOYHV6knlNGqtUkfBz4ds21GeKS+mMEc7w== X-Received: by 2002:a17:90a:240a:: with SMTP id h10mr14083536pje.225.1594528138159; Sat, 11 Jul 2020 21:28:58 -0700 (PDT) Return-Path: Received: from ubuntu.localdomain (c-98-214-99-181.hsd1.il.comcast.net. [98.214.99.181]) by smtp.gmail.com with ESMTPSA id o18sm11815310pfu.138.2020.07.11.21.28.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 11 Jul 2020 21:28:57 -0700 (PDT) From: "Andrei Warkentin" To: devel@edk2.groups.io Cc: ard.biesheuvel@arm.com, leif@nuviainc.com, pete@akeo.ie, philmd@redhat.com Subject: [edk2][PATCH 1/1] BcmGenetDxe: don't consume RX buffer until it's actually copied Date: Sat, 11 Jul 2020 21:28:50 -0700 Message-Id: <20200712042850.117715-1-andrey.warkentin@gmail.com> X-Mailer: git-send-email 2.17.1 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 --- 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