[edk2-devel] [PATCH edk2-platforms v4 6/9] Silicon/Broadcom/BcmGenetDxe: keep TX buffer mapped during DMA transfer

[Andrei Warkentin]

Yeah if this code ever runs on a system with some kind of SMMU, definitely want the fix. (could we mention SMMU as a reason for why mappings must be valid during hw access?)

Reviewed-by: Andrei Warkentin <andrey.warkentin at gmail.com>

________________________________
From: Ard Biesheuvel <ard.biesheuvel at arm.com>
Sent: Monday, May 11, 2020 9:55 AM
To: devel at edk2.groups.io <devel at edk2.groups.io>
Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>; Pete Batard <pete at akeo.ie>; Jared McNeill <jmcneill at invisible.ca>; Andrei Warkentin <awarkentin at vmware.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud at arm.com>; Jeremy Linton <jeremy.linton at arm.com>
Subject: [PATCH edk2-platforms v4 6/9] Silicon/Broadcom/BcmGenetDxe: keep TX buffer mapped during DMA transfer

The DMA api requires that DMA mappings are kept in place during the
time the device may access the memory. This may not matter for the
way GENET is used on the RPi4, but it could break bounce buffering on
platforms that impose DMA memory limits, since the bounce buffer may
have been reused by the time the device gets to look at the data.

So defer the DmaUnmap() of TX buffers to the point where they are
recycled and returned to the caller.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel at arm.com>
---
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h     | 1 +
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c     | 1 +
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 5 +----
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h
index c43bdbe1d6da..16890f723cb2 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h
@@ -213,6 +213,7 @@ typedef struct {
   GENERIC_PHY_PRIVATE_DATA            Phy;

   UINT8                               *TxBuffer[GENET_DMA_DESC_COUNT];
+  VOID                                *TxBufferMap[GENET_DMA_DESC_COUNT];
   UINT8                               TxQueued;
   UINT16                              TxNext;
   UINT16                              TxConsIndex;
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c
index 94b578a10aa1..35a3c7abdf1e 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.c
@@ -760,6 +760,7 @@ GenetTxIntr (

   Total = (ConsIndex - Genet->TxConsIndex) & 0xFFFF;
   if (Genet->TxQueued > 0 && Total > 0) {
+    DmaUnmap (Genet->TxBufferMap[Genet->TxNext]);
     *TxBuf = Genet->TxBuffer[Genet->TxNext];
     Genet->TxQueued--;
     Genet->TxNext = (Genet->TxNext + 1) % GENET_DMA_DESC_COUNT;
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
index 9746f210d1f2..b2cae687b3d4 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
@@ -559,7 +559,6 @@ GenetSimpleNetworkTransmit (
   UINT8               Desc;
   PHYSICAL_ADDRESS    DmaDeviceAddress;
   UINTN               DmaNumberOfBytes;
-  VOID                *DmaMapping;

   if (This == NULL || Buffer == NULL) {
     DEBUG ((DEBUG_ERROR, "%a: Invalid parameter (missing handle or buffer)\n",
@@ -631,7 +630,7 @@ GenetSimpleNetworkTransmit (
                    (VOID *)(UINTN)Frame,
                    &DmaNumberOfBytes,
                    &DmaDeviceAddress,
-                   &DmaMapping);
+                   &Genet->TxBufferMap[Desc]);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a: DmaMap failed: %r\n", __FUNCTION__, Status));
     EfiReleaseLock (&Genet->Lock);
@@ -643,8 +642,6 @@ GenetSimpleNetworkTransmit (
   Genet->TxProdIndex = (Genet->TxProdIndex + 1) % 0xFFFF;
   Genet->TxQueued++;

-  DmaUnmap (DmaMapping);
-
   EfiReleaseLock (&Genet->Lock);

   return EFI_SUCCESS;
--
2.17.1


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

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20200511/b3e0e49b/attachment.htm>