From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM02-SN1-obe.outbound.protection.outlook.com (NAM02-SN1-obe.outbound.protection.outlook.com [40.107.77.57]) by mx.groups.io with SMTP id smtpd.web12.396.1589214765643478662 for ; Mon, 11 May 2020 09:32:45 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@vmware.com header.s=selector2 header.b=T7OSPMw1; spf=pass (domain: vmware.com, ip: 40.107.77.57, mailfrom: awarkentin@vmware.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ot6t71fRLMKFwtpXLHkChjIjavdpdITOF1gsZG6jkJ1x0C78mw7zpuCD1uRpvd4/pfO1+VFV0aYw262V5ea+laVoJ+hgG3RhLFzIiTRduqNabeqnBqnUeHqnDXKdkatTXfJYIVbWnou7jFKhhf7I4ehThcGQf24/FlKWkCLBBUZQsa0lYHGOwN36h1YJmb4rKs3a/oyoXC4Dbbkfo++Ttqsyb8nbhz9y6gN6FYQpxY9EQAFtt9ACjc+C5gBCYkcGnlTHn/Lj+u/liaT2kjePh9Jf9q7KDNZxJbOaUp0CP3gq5fNhDdcfA0Wo37SeP3LIdmCsMwMBO3NRdNsX4/te7A== 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=dLdX5MrsCRJ5KJKKdFHUBaKqio1/orDfS425X/KRRO4=; b=gbe3anIyHuguWqguUcnDRpdXbuFjo0qRVTwtqEsJWypdYOrQowp/8lJYs77OAqtH4KE9it+F5zdIIx3Ogm/zFq1kjMHsGOve2hxcuCvdUhar6G1RYCNFWqjl4Luz2TcZbIRVSTSu3hgJo/BbVquCAT2HUIqQUHLCoKERpIVYXgSpb1epXeX4SOUef6gbADVeZLs6/CawlKrq+99QldRMulYYT1B8JeOfv2Y10vpQwgZbTp10JK+VW0BZKIyR/P/ztQo7E9yYcm9ssy/lEq0RAzUQ5dpq57MneuyYDBZHCpuAq/IPb7l18hMAj4NgcmqdqCVqJ6FbO/fzq1ze624K9g== 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=dLdX5MrsCRJ5KJKKdFHUBaKqio1/orDfS425X/KRRO4=; b=T7OSPMw1+BU/3ptNYRnT11NZfEvIe+rKMf6U901Gu6EuDCeViCNHOY0FsHeOC6/ehLK4w2GqslFJ2nkCWy2ikcBCwpy0iq0DS23g3kCfR0qFOlaq+9aHzsvIWisZOBzidKCl1/VC+ST3TkZIwudpUaVLVgRyhcCUhOENKKxhgnY= Received: from BN6PR05MB3411.namprd05.prod.outlook.com (2603:10b6:405:43::23) by BN6PR05MB3555.namprd05.prod.outlook.com (2603:10b6:405:3e::34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3000.11; Mon, 11 May 2020 16:32:43 +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:32:42 +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 5/9] Silicon/Broadcom/BcmGenetDxe: shut down devices on ExitBootServices() Thread-Topic: [PATCH edk2-platforms v4 5/9] Silicon/Broadcom/BcmGenetDxe: shut down devices on ExitBootServices() Thread-Index: AQHWJ6RD5Lex8FoS30GTiZv0jbhLKqijEk29 Date: Mon, 11 May 2020 16:32:42 +0000 Message-ID: References: <20200511145527.23453-1-ard.biesheuvel@arm.com>,<20200511145527.23453-6-ard.biesheuvel@arm.com> In-Reply-To: <20200511145527.23453-6-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: 76f6e307-4c58-42cb-b047-08d7f5c8ede8 x-ms-traffictypediagnostic: BN6PR05MB3555: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:6430; x-forefront-prvs: 04004D94E2 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: VfsjierwiHLpLWmV2pLFsLmR240UWZzmkc1HzlOR/u0awXEYnH40XwN2YWkknygcE/p4+4fiZt914SlF/cfRLoxwYupmJkusCHbHDVapJZT3Ec7DaAKqU7OyMA03kNlIL4t1dNoq/HEPi436Ype3DDhjx32jFfXgRirq9mRlLCfxL5CKNfY7vKFd79iUZJZkjjm1g517Va479eAr9xTF2v0aF9dozfVjHWl8YkAcTLQf3F/BnwNn6oOjFvrBhg/PxZxpwjdtlQknUMRXr+4+rii6ZhdLkUjiDGuYC/+RW0lfFF0PtSHspWWS0OPgaS2Jlbyx5ImlUWhF5ZGRNnlDQKs8o41MSoAB0pldR4pEXEofx7x1lTg7SVZevYiU91s9g1o5Ejq6Vw+4kTE0v8Bhle9z4Fw0cbfHnfPeu5d6vGhyA/UBemwmrxxwX2gUtAv+YqxVKjyMia1pkCPIlmJ4rjMXzpg+Auc634Ci6WqKVQgGK54YPsHLJX++21Q6FslzjXhMPjtiGPvdIVuYbF3iZbGQ8VaERlEgPJWXt5ofruDGd8Pq1iH0j09LcoX5hVvD 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)(376002)(366004)(346002)(39860400002)(136003)(396003)(33430700001)(52536014)(53546011)(316002)(86362001)(5660300002)(33440700001)(110136005)(2906002)(6506007)(19627405001)(54906003)(66556008)(76116006)(478600001)(7696005)(66946007)(71200400001)(4326008)(64756008)(186003)(66476007)(9686003)(55016002)(33656002)(8936002)(66446008)(8676002)(26005)(44824005);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata: 2rvcVwB8HeRqEpevVJY9P7vmtLubgNgZZi76TW+rqFoSlKX0o7c3XHpd4gmTLn91G/12daX4iCwsgLnLv9SElDdVc05QRglqe7PQaG9Vv4N0FpoKdfjwlYnIqP0Td3OrXde4sxWNEDuos18h9MCs9TeDcs5e/IEIJy5Q2DDVpIdYBg24dGC1D+I80ZyvEgHsA+MFTE7hs2jzHXST5aRAi56UM0qO+t5xkc16fBAZrniqwVaykne//hjWaJh8lN21Et1LKe355hZofoH5lmfqexJK2DtNlUDj42oZL1YcjcrKUGxBgbsKk6FMqyPujzfmEppc4yRWmWc/DcI1L19eXVHbjiuutZsMvt6q7+7VbzMqypD3dq8QV8dPBj+ng6EPSs6AoaIbxxvXCbnEDNJSy7fCtoBFAgY2bwjFe9KS1pPwqSVaoNmG7ObZP7hMT59Oc3Cj6AN7C/Pwik2aQf36BWZwTTTYGJtzhAPulIn8gdI= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: vmware.com X-MS-Exchange-CrossTenant-Network-Message-Id: 76f6e307-4c58-42cb-b047-08d7f5c8ede8 X-MS-Exchange-CrossTenant-originalarrivaltime: 11 May 2020 16:32:42.9588 (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: SGGzEP2kne70hZzMdBcluaMfv3IrSXgRqoHmg1ImVxGmE0oxnb2v/+41WmG5i7vrVetjlA/64TMLjapfPpr7KA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR05MB3555 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_BN6PR05MB3411A7C32C950A64D3426135B9A10BN6PR05MB3411namp_" --_000_BN6PR05MB3411A7C32C950A64D3426135B9A10BN6PR05MB3411namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable LGTM. I agree that we just need to stop TX/RX. We do not want to reset cont= roller (as that will nuke the programmed-in MAC address). 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 5/9] Silicon/Broadcom/BcmGenetDxe: shut d= own devices on ExitBootServices() When the OS takes over the machine, it calls ExitBootServices() in order to shut down all resident services and event notifications so that all asynchronous activity is stopped. At this point, any DMA masters that are still active should be shut down. This is especially important for network controllers, since any activity on the network will trigger DMA writes into memory, which will no longer be reserved for this purpose once the OS takes over. So register for the ExitBootServices event, and shut down the controller at this point if it was started before. Signed-off-by: Ard Biesheuvel --- Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf | 4 +++ Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h | 1 + Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c | 35 ++++++++++++= +++++++- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf b/Sil= icon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf index 248164249c6e..9b3dc5e62ecf 100644 --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf @@ -44,6 +44,7 @@ [LibraryClasses] IoLib MemoryAllocationLib NetLib + UefiBootServicesTableLib UefiDriverEntryPoint UefiLib @@ -52,6 +53,9 @@ [Protocols] gEfiDevicePathProtocolGuid ## BY_START gEfiSimpleNetworkProtocolGuid ## BY_START +[Guids] + gEfiEventExitBootServicesGuid + [FixedPcd] gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h b/Silicon= /Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h index ddfbc0806c07..c43bdbe1d6da 100644 --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h @@ -203,6 +203,7 @@ typedef struct { EFI_HANDLE ControllerHandle; EFI_LOCK Lock; + EFI_EVENT ExitBootServicesEvent; EFI_SIMPLE_NETWORK_PROTOCOL Snp; EFI_SIMPLE_NETWORK_MODE SnpMode; diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c b/Sil= icon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c index dacb3ac7d762..00fbfbc109bb 100644 --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c @@ -74,6 +74,23 @@ GenetDriverBindingSupported ( return EFI_SUCCESS; } +/** + Callback function to shut down the network device at ExitBootServices + + @param Event Pointer to this event + @param Context Event handler private data + +**/ +STATIC +VOID +EFIAPI +GenetNotifyExitBootServices ( + EFI_EVENT Event, + VOID *Context + ) +{ + GenetDisableTxRx ((GENET_PRIVATE_DATA *)Context); +} /** Starts a device controller or a bus controller. @@ -171,6 +188,17 @@ GenetDriverBindingStart ( CopyMem (&Genet->SnpMode.CurrentAddress, &Genet->Dev->MacAddress, sizeof(EFI_MAC_ADDRESS)); + Status =3D gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, + GenetNotifyExitBootServices, Genet, + &gEfiEventExitBootServicesGuid, + &Genet->ExitBootServicesEvent); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_WARN, + "GenetDriverBindingStart: failed to register for ExitBootServices ev= ent - %r\n", + Status)); + goto FreeDevice; + } + Status =3D gBS->InstallMultipleProtocolInterfaces (&ControllerHandle, &gEfiSimpleNetworkProtocolGuid, &Genet->Snp, NULL); @@ -182,12 +210,14 @@ GenetDriverBindingStart ( &gBcmGenetPlatformDeviceProtocolGuid, This->DriverBindingHandle, ControllerHandle); - goto FreeDevice; + goto FreeEvent; } Genet->ControllerHandle =3D ControllerHandle; return EFI_SUCCESS; +FreeEvent: + gBS->CloseEvent (Genet->ExitBootServicesEvent); FreeDevice: DEBUG ((DEBUG_WARN, "%a: Returning %r\n", __FUNCTION__, Status)); FreePool (Genet); @@ -248,6 +278,9 @@ GenetDriverBindingStop ( return Status; } + Status =3D gBS->CloseEvent (Genet->ExitBootServicesEvent); + ASSERT_EFI_ERROR (Status); + GenetDmaFree (Genet); Status =3D gBS->CloseProtocol (ControllerHandle, -- 2.17.1 --_000_BN6PR05MB3411A7C32C950A64D3426135B9A10BN6PR05MB3411namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable
LGTM. I agree that we just need to stop TX/RX. We do not want to rese= t controller (as that will nuke the programmed-in MAC address).

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 5/9] Silicon/Broadcom/BcmGenetDxe:= shut down devices on ExitBootServices()
 
When the OS takes over the machine, it calls ExitB= ootServices() in
order to shut down all resident services and event notifications so
that all asynchronous activity is stopped.

At this point, any DMA masters that are still active should be shut
down. This is especially important for network controllers, since
any activity on the network will trigger DMA writes into memory, which
will no longer be reserved for this purpose once the OS takes over.

So register for the ExitBootServices event, and shut down the controller at this point if it was started before.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf |  4 &#= 43;++
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h   = ;  |  1 +
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c | 35 +&#= 43;++++++++++++++&#= 43;++-
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf b/Sil= icon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
index 248164249c6e..9b3dc5e62ecf 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf<= br> @@ -44,6 +44,7 @@ [LibraryClasses]
   IoLib
   MemoryAllocationLib
   NetLib
+  UefiBootServicesTableLib
   UefiDriverEntryPoint
   UefiLib
 
@@ -52,6 +53,9 @@ [Protocols]
   gEfiDevicePathProtocolGuid      =             ## BY_ST= ART
   gEfiSimpleNetworkProtocolGuid     &nb= sp;         ## BY_START
 
+[Guids]
+  gEfiEventExitBootServicesGuid
+
 [FixedPcd]
   gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset
   gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h b/Silicon= /Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h
index ddfbc0806c07..c43bdbe1d6da 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenetUtil.h
@@ -203,6 +203,7 @@ typedef struct {
   EFI_HANDLE        &nbs= p;            &= nbsp;    ControllerHandle;
 
   EFI_LOCK         =             &nb= sp;      Lock;
+  EFI_EVENT         =             &nb= sp;     ExitBootServicesEvent;
 
   EFI_SIMPLE_NETWORK_PROTOCOL      = ;   Snp;
   EFI_SIMPLE_NETWORK_MODE      &nb= sp;      SnpMode;
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c b/Sil= icon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
index dacb3ac7d762..00fbfbc109bb 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c<= br> @@ -74,6 +74,23 @@ GenetDriverBindingSupported (
   return EFI_SUCCESS;
 }
 
+/**
+  Callback function to shut down the network device at ExitBootSe= rvices
+
+  @param  Event       &nb= sp;           Pointer to = this event
+  @param  Context       &= nbsp;         Event handler private= data
+
+**/
+STATIC
+VOID
+EFIAPI
+GenetNotifyExitBootServices (
+  EFI_EVENT     Event,
+  VOID          *Con= text
+  )
+{
+  GenetDisableTxRx ((GENET_PRIVATE_DATA *)Context);
+}
 
 /**
   Starts a device controller or a bus controller.
@@ -171,6 +188,17 @@ GenetDriverBindingStart (
   CopyMem (&Genet->SnpMode.CurrentAddress, &Genet->= ;Dev->MacAddress,
     sizeof(EFI_MAC_ADDRESS));
 
+  Status =3D gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBA= CK,
+           &nbs= p;      GenetNotifyExitBootServices, Genet,
+           &nbs= p;      &gEfiEventExitBootServicesGuid,
+           &nbs= p;      &Genet->ExitBootServicesEvent);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_WARN,
+      "GenetDriverBindingStart: failed t= o register for ExitBootServices event - %r\n",
+      Status));
+    goto FreeDevice;
+  }
+
   Status =3D gBS->InstallMultipleProtocolInterfaces (&Con= trollerHandle,
            &nb= sp;      &gEfiSimpleNetworkProtocolGuid, =   &Genet->Snp,
            &nb= sp;      NULL);
@@ -182,12 +210,14 @@ GenetDriverBindingStart (
            &nb= sp;            &= gBcmGenetPlatformDeviceProtocolGuid,
            &nb= sp;            This-= >DriverBindingHandle,
            &nb= sp;            Contr= ollerHandle);
-    goto FreeDevice;
+    goto FreeEvent;
   }
 
   Genet->ControllerHandle =3D ControllerHandle;
   return EFI_SUCCESS;
 
+FreeEvent:
+  gBS->CloseEvent (Genet->ExitBootServicesEvent);
 FreeDevice:
   DEBUG ((DEBUG_WARN, "%a: Returning %r\n", __FUNCTION= __, Status));
   FreePool (Genet);
@@ -248,6 +278,9 @@ GenetDriverBindingStop (
     return Status;
   }
 
+  Status =3D gBS->CloseEvent (Genet->ExitBootServicesEvent)= ;
+  ASSERT_EFI_ERROR (Status);
+
   GenetDmaFree (Genet);
 
   Status =3D gBS->CloseProtocol (ControllerHandle,
--
2.17.1

--_000_BN6PR05MB3411A7C32C950A64D3426135B9A10BN6PR05MB3411namp_--