From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM11-DM6-obe.outbound.protection.outlook.com (NAM11-DM6-obe.outbound.protection.outlook.com [40.107.223.76]) by mx.groups.io with SMTP id smtpd.web12.104.1589213988613826121 for ; Mon, 11 May 2020 09:19:48 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@vmware.com header.s=selector2 header.b=TCEwf/rn; spf=pass (domain: vmware.com, ip: 40.107.223.76, mailfrom: awarkentin@vmware.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=G5hqKQ61uGQBGPY+zoF4VcM5v5L7ywsScORTsArwrKfR4p5TqAKgTnwNowEbmu2THK37qqrisigAu/BN2yKoYBsRgpBarlWZJ6G3oFtArsblzzo0KjgA0A8M2F3/EKBBHONpI/3w/jct5nj1DLao5y61QZzhu4HYxXD+9VdxrBgmoqexqPfcPCZ7Y3bgGy6L32H9o3WOCbXzDFP2jZm7lGQIRq1eUn5t4C4uI6ithBtGSHUN10ifo0ydWI7OM2BQMVmzpotciKylV94u/R+thg9QaKETf68f8JDH0fbh9iaJ5R0WUxvyKr8iSIk7kZSzd02I9YA4cRS3sf1BEzkKQA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=CruLoFlsy8pYFXAc+dyw1w2S94G5L31cS9jJlJb60FU=; b=mxgKjBjAtf4F34OKOIuMSPi2DBOgGTNB1w8939SJJcI075OdB1bDUgMXZCr4knd2ZmCoJ8/Zu7jpKifrg/vqFWrnq60cPKtyGt83kqCDd+a2BWRsM0I/6IRy7/W1u5eZDefdiJPOpHlN8yuiAEKLkG4yeSlHBB48P5VFFSHrdleIvQGcY1tRO2hQ94zA1MGUpWhiuUPLgfqkWH/DJkwWLInIr4DC0mRkdA5opeIgPYw6kCvSYwAtqzgB/ax1MajTwhROkbqvtvpAr0IEWM7tygkemoLTt+Tpq1Bcj++vdIY8MfVSdtBgbQQRNBpprnUMKW7sujvu+jjwkferP/Nghw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=vmware.com; dmarc=pass action=none header.from=vmware.com; dkim=pass header.d=vmware.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vmware.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=CruLoFlsy8pYFXAc+dyw1w2S94G5L31cS9jJlJb60FU=; b=TCEwf/rnHn2v8NdWXWU4Ck+JM8DPcHTtG/3ezFm3kinptrOqgFjiGenM56vIXPxW7jrVq0yBPbD7cXjvdl4IR7z6/+bSNLAeY1POX+pOmUliWyVEhTmytmve/Z46widlLaFkrmo58biSuNswJa+1z04RLoCmbN+ubd/lYUnvrpY= Received: from BN6PR05MB3411.namprd05.prod.outlook.com (2603:10b6:405:43::23) by BN6PR05MB3538.namprd05.prod.outlook.com (2603:10b6:405:46::32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3000.13; Mon, 11 May 2020 16:19:44 +0000 Received: from BN6PR05MB3411.namprd05.prod.outlook.com ([fe80::f463:db64:43d8:5a0f]) by BN6PR05MB3411.namprd05.prod.outlook.com ([fe80::f463:db64:43d8:5a0f%3]) with mapi id 15.20.3000.016; Mon, 11 May 2020 16:19:44 +0000 From: "Andrei Warkentin" To: Ard Biesheuvel , "devel@edk2.groups.io" CC: Pete Batard , Jared McNeill , Samer El-Haj-Mahmoud , Jeremy Linton Subject: Re: [PATCH edk2-platforms v4 6/9] Silicon/Broadcom/BcmGenetDxe: keep TX buffer mapped during DMA transfer Thread-Topic: [PATCH edk2-platforms v4 6/9] Silicon/Broadcom/BcmGenetDxe: keep TX buffer mapped during DMA transfer Thread-Index: AQHWJ6REzOrl+ZmuYU2V+SVLnQxZCqijEGJB Date: Mon, 11 May 2020 16:19:44 +0000 Message-ID: References: <20200511145527.23453-1-ard.biesheuvel@arm.com>,<20200511145527.23453-7-ard.biesheuvel@arm.com> In-Reply-To: <20200511145527.23453-7-ard.biesheuvel@arm.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=vmware.com; x-originating-ip: [98.214.99.181] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 83eb7e1d-b9fb-414c-4772-08d7f5c71e17 x-ms-traffictypediagnostic: BN6PR05MB3538: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8882; x-forefront-prvs: 04004D94E2 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: B2uaiTtF+frpfyqCfdt61UlwyxxA8wP9rsSAer031ImGpJBIABNJZcY24ZeKnTSpXEfF9p25iW+vfe1jIHxhQIqgvVbekdU3pcxXzTfF7JAIXMzyfzXcV32P3vh7BEM6xQ9YGGpX8KErCEll6q9CdUcDSyg/k6vi7aXzpaY6/PMpnYfenFHRzasABWJkAl39mEqFRk0/5dVkNTcWvqWZXyYSrsVmcRjheiPlBe7Sx7HErVKWwroPbooZzUZHu7/InqSsrgTKKiGLb5K2LkdzO32zTx7OQxQxFUkbz7REXePBQ7ZZdBB53OFv/wCL8us//FelP626G/8gtuhHM+IOvDREQuC5vOzu9FYIzSy0rmfhr6FYBy5OvXR9VnpwW7S+5DAKW1lhTId8oUxnDFAzkeKZg3fpzq5HsogpDzJ8MlgvLsdx0EqGnj/5g9Hl9fXWRKI1Rvt7mZ32gJw4nEFtP2oiNiFNR14OuHikIf6VgSzubo3lfczxhYa3FK6hHjnpIq4sA1ajTOCG66liZODEyg== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BN6PR05MB3411.namprd05.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(39860400002)(346002)(366004)(396003)(136003)(376002)(33430700001)(86362001)(4326008)(71200400001)(52536014)(110136005)(66476007)(478600001)(76116006)(66946007)(54906003)(316002)(66556008)(66446008)(53546011)(33656002)(2906002)(33440700001)(55016002)(6506007)(8936002)(186003)(5660300002)(64756008)(19627405001)(26005)(8676002)(9686003)(7696005);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata: NBKF1Pi1ROV1xm5kOINWhTYTNV9jW8hI0Nq3ZX0N1o1GaUEzvrC2WDsa9OWxA6ZKrKriMdvnWYDL5KlNunrPjN5rJBIkbTEP44K4OpgK3hWr3oLzNRxNmkWoSEPtLlLfXp5jsWfVIhrsxHha/suAx5H8X5aYdV2re0T3BuKurOv3uZbr0ciSXt59FTCYNSVGxq6GLkS60yPQ/MWQgtDSi9BYvfvwVgp+Ek00ECPhXAO4vEFwsvMNnfStwt3Aw+MkeFOjZ4I0nwVL6LoRnCCtE0cDJqkYJ5bJvOQATP9P5I58RDQxtRjd+6E164B3UGQ3/ZcCJKVDItd9C7RhLvryuGJSCLGQ7QW2P1UJjzwkZzq31ezZug505kmRi7+PS0aBtYA8TGKFZcoSa8vKQeQ6hSZU96Aj149X4nwmy5mux/HJhLMSBWHcNHvkoGXlaKGFtlAqDWxuIhHxCnUWXX0LAvXf9yPT/dar+4BkPmKgDT0= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: vmware.com X-MS-Exchange-CrossTenant-Network-Message-Id: 83eb7e1d-b9fb-414c-4772-08d7f5c71e17 X-MS-Exchange-CrossTenant-originalarrivaltime: 11 May 2020 16:19:44.7624 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: UphKVMirNugWPdL1uicOGgi/UJr4ASTOEyG8Er3yUduBa264h7U61uVSuG96kXu/TDbg1+RBjnv3XiKaVBNYcg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR05MB3538 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_BN6PR05MB34113C4C5721CCDF38627C00B9A10BN6PR05MB3411namp_" --_000_BN6PR05MB34113C4C5721CCDF38627C00B9A10BN6PR05MB3411namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 v= alid during hw access?) Reviewed-by: Andrei Warkentin ________________________________ From: Ard Biesheuvel Sent: Monday, May 11, 2020 9:55 AM To: devel@edk2.groups.io Cc: Ard Biesheuvel ; Pete Batard ; Ja= red McNeill ; Andrei Warkentin ; Samer El-Haj-Mahmoud ; Jeremy Linton Subject: [PATCH edk2-platforms v4 6/9] Silicon/Broadcom/BcmGenetDxe: keep T= X 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 --- 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 =3D (ConsIndex - Genet->TxConsIndex) & 0xFFFF; if (Genet->TxQueued > 0 && Total > 0) { + DmaUnmap (Genet->TxBufferMap[Genet->TxNext]); *TxBuf =3D Genet->TxBuffer[Genet->TxNext]; Genet->TxQueued--; Genet->TxNext =3D (Genet->TxNext + 1) % GENET_DMA_DESC_COUNT; diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Sil= icon/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 =3D=3D NULL || Buffer =3D=3D 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 =3D (Genet->TxProdIndex + 1) % 0xFFFF; Genet->TxQueued++; - DmaUnmap (DmaMapping); - EfiReleaseLock (&Genet->Lock); return EFI_SUCCESS; -- 2.17.1 --_000_BN6PR05MB34113C4C5721CCDF38627C00B9A10BN6PR05MB3411namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable
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 v= alid during hw access?)

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

From: Ard Biesheuvel <ar= d.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 <p= ete@akeo.ie>; Jared McNeill <jmcneill@invisible.ca>; Andrei Warken= tin <awarkentin@vmware.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Ma= hmoud@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      &n= bsp;     Phy;
 
   UINT8         &nb= sp;            =          *TxBuffer[GENET_DMA_DESC_C= OUNT];
+  VOID          = ;            &n= bsp;         *TxBufferMap[GENET_DMA= _DESC_COUNT];
   UINT8         &nb= sp;            =          TxQueued;
   UINT16         &n= bsp;            = ;        TxNext;
   UINT16         &n= bsp;            = ;        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 =3D (ConsIndex - Genet->TxConsIndex) & 0xFFFF;    if (Genet->TxQueued > 0 && Total > 0) {
+    DmaUnmap (Genet->TxBufferMap[Genet->TxNext]);=
     *TxBuf =3D Genet->TxBuffer[Genet->TxNext];      Genet->TxQueued--;
     Genet->TxNext =3D (Genet->TxNext + 1) % = GENET_DMA_DESC_COUNT;
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Sil= icon/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<= br> @@ -559,7 +559,6 @@ GenetSimpleNetworkTransmit (
   UINT8         &nb= sp;     Desc;
   PHYSICAL_ADDRESS    DmaDeviceAddress;
   UINTN         &nb= sp;     DmaNumberOfBytes;
-  VOID          &nb= sp;     *DmaMapping;
 
   if (This =3D=3D NULL || Buffer =3D=3D NULL) {
     DEBUG ((DEBUG_ERROR, "%a: Invalid parameter (= missing handle or buffer)\n",
@@ -631,7 +630,7 @@ GenetSimpleNetworkTransmit (
            &nb= sp;       (VOID *)(UINTN)Frame,
            &nb= sp;       &DmaNumberOfBytes,
            &nb= sp;       &DmaDeviceAddress,
-            &n= bsp;      &DmaMapping);
+           &nbs= p;       &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 =3D (Genet->TxProdIndex + 1) % 0x= FFFF;
   Genet->TxQueued++;
 
-  DmaUnmap (DmaMapping);
-
   EfiReleaseLock (&Genet->Lock);
 
   return EFI_SUCCESS;
--
2.17.1

--_000_BN6PR05MB34113C4C5721CCDF38627C00B9A10BN6PR05MB3411namp_--