public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Andrei Warkentin" <awarkentin@vmware.com>
To: Ard Biesheuvel <ard.biesheuvel@arm.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Pete Batard <pete@akeo.ie>, Jared McNeill <jmcneill@invisible.ca>,
	Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>,
	Jeremy Linton <jeremy.linton@arm.com>
Subject: Re: [PATCH edk2-platforms v4 6/9] Silicon/Broadcom/BcmGenetDxe: keep TX buffer mapped during DMA transfer
Date: Mon, 11 May 2020 16:19:44 +0000	[thread overview]
Message-ID: <BN6PR05MB34113C4C5721CCDF38627C00B9A10@BN6PR05MB3411.namprd05.prod.outlook.com> (raw)
In-Reply-To: <20200511145527.23453-7-ard.biesheuvel@arm.com>

[-- Attachment #1: Type: text/plain, Size: 4076 bytes --]

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@gmail.com>

________________________________
From: Ard Biesheuvel <ard.biesheuvel@arm.com>
Sent: Monday, May 11, 2020 9:55 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; Pete Batard <pete@akeo.ie>; Jared McNeill <jmcneill@invisible.ca>; Andrei Warkentin <awarkentin@vmware.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Jeremy Linton <jeremy.linton@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@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


[-- Attachment #2: Type: text/html, Size: 7527 bytes --]

  reply	other threads:[~2020-05-11 16:19 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-11 14:55 [PATCH edk2-platforms v4 0/9] BCM genet fixes Ard Biesheuvel
2020-05-11 14:55 ` [PATCH edk2-platforms v4 1/9] Silicon/Broadcom/BcmGenetDxe: whitespace/cosmetic cleanup Ard Biesheuvel
2020-05-11 16:36   ` Samer El-Haj-Mahmoud
2020-05-11 21:14   ` [edk2-devel] " Philippe Mathieu-Daudé
2020-05-11 14:55 ` [PATCH edk2-platforms v4 2/9] Silicon/Broadcom/BcmGenetDxe: add support for broadcast filtering Ard Biesheuvel
2020-05-11 16:44   ` Andrei Warkentin
2020-05-11 20:34   ` Jeremy Linton
2020-05-11 21:21     ` Ard Biesheuvel
2020-05-11 14:55 ` [PATCH edk2-platforms v4 3/9] Silicon/Broadcom/BcmGenetDxe: fix multicast/broadcast handling Ard Biesheuvel
2020-05-11 14:55 ` [PATCH edk2-platforms v4 4/9] Silicon/Broadcom/BcmGenetDxe: avoid uncached memory for streaming DMA Ard Biesheuvel
2020-05-11 16:42   ` Andrei Warkentin
2020-05-11 16:53     ` Ard Biesheuvel
2020-05-11 14:55 ` [PATCH edk2-platforms v4 5/9] Silicon/Broadcom/BcmGenetDxe: shut down devices on ExitBootServices() Ard Biesheuvel
2020-05-11 16:32   ` Andrei Warkentin
2020-05-11 14:55 ` [PATCH edk2-platforms v4 6/9] Silicon/Broadcom/BcmGenetDxe: keep TX buffer mapped during DMA transfer Ard Biesheuvel
2020-05-11 16:19   ` Andrei Warkentin [this message]
2020-05-11 14:55 ` [PATCH edk2-platforms v4 7/9] Silicon/Broadcom/BcmGenetDxe: use MemoryFence() for MMIO write ordering Ard Biesheuvel
2020-05-11 16:55   ` [edk2-devel] " Andrei Warkentin
2020-05-11 21:11   ` Philippe Mathieu-Daudé
2020-05-11 14:55 ` [PATCH edk2-platforms v4 8/9] Silicon/Broadcom/BcmGenetDxe: add unload support Ard Biesheuvel
2020-05-11 16:17   ` Andrei Warkentin
2020-05-11 14:55 ` [PATCH edk2-platforms v4 9/9] Platform/RaspberryPi4: remove ASIX 88772b driver Ard Biesheuvel
2020-05-11 16:06   ` Andrei Warkentin
2020-05-11 16:24   ` Samer El-Haj-Mahmoud
2020-05-11 23:20 ` [PATCH edk2-platforms v4 0/9] BCM genet fixes Jeremy Linton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BN6PR05MB34113C4C5721CCDF38627C00B9A10@BN6PR05MB3411.namprd05.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox