From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (NAM10-BN7-obe.outbound.protection.outlook.com [40.107.92.44]) by mx.groups.io with SMTP id smtpd.web12.3877.1619735411373905801 for ; Thu, 29 Apr 2021 15:30:11 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@vmware.com header.s=selector2 header.b=w6wJ+tbC; spf=pass (domain: vmware.com, ip: 40.107.92.44, mailfrom: awarkentin@vmware.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=i4z06LGB1LuACYwkLrEWKy4glZKGry2bwAPHGuoW8eRLThpGzmvS8FIHij/vZMwYR+cZD13XQUu4tMfQOv7EE8ufitblhpnkIAcmQTv8LC8hbpc8YewgrQM5gSF0YbFOQjTxAN4nuRiZABmB6DzUOVHNk5g4RATr8+Yj3iLKb+6WcgBhXUy6vEYYCIxhazO9jZUFWfdrNtrDL9+vNBlvmYiKGzKe0ARfRRS8hUzyEZ21X8i9touzKiu3GO/iwI/h0doBWxPPpl0q3aUrA4oWViGg/MueSOPPQA33V7aDImeqDzreRwOVPigHKtEr9s2zf7NUhyYv2e1dcMQ1s+kBgg== 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=xHZrBGdZALQ2AaqxHsDVIDP/5JET/vfOqcd7GHcKLZ8=; b=l+MOHG3ETzxXZPkjypMvTK25CTzMvmNVSRH4MB0Y5Z5ePQhiFSj2z0ywWabvYcHOOy0dlPnZT67O6Qh5XRQT7XStInMj47SzKOer1OcNWmjwgeAUxSvtzjOa4+vn9haDUQFOrxBho2I2oFnEvV2q3zbCCmqmMiNkmRLXGbQz+6x0y/7L4qR5UryRC9GGi6TNG6aNR8WhAFG20q5fDedlxurWxbtwVtL6JmBx8nqGzYROWtu6HHF05LIt+eIXv2UZ5S8YTYgQ0ht9L6KZRRmkra0F1v/JNH0JquFEjVYMdERe0dAgNokOVobZDNoQII2RvP53b4gLnVOfJBMZrMmxEg== 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=xHZrBGdZALQ2AaqxHsDVIDP/5JET/vfOqcd7GHcKLZ8=; b=w6wJ+tbCSzFUXQg5FzyzPs3K11DJlCODWpHT3xfc0W2DPI1rC1Ww0om0VUfAABTe10w5xH2Jv+xPXpecqz4Zj7A04N5wDp2ObhxuwbzrfyLOku7LNHlaa+pxEE5hqfo8asbuOw+ZBJGriloDY/fpdhRRCepqoiF4VqgaLhpzHPc= Received: from SN7PR05MB7582.namprd05.prod.outlook.com (2603:10b6:806:f7::16) by SN6PR05MB5054.namprd05.prod.outlook.com (2603:10b6:805:f0::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4087.22; Thu, 29 Apr 2021 22:30:07 +0000 Received: from SN7PR05MB7582.namprd05.prod.outlook.com ([fe80::a199:b8b:c31a:cd0e]) by SN7PR05MB7582.namprd05.prod.outlook.com ([fe80::a199:b8b:c31a:cd0e%7]) with mapi id 15.20.4087.021; Thu, 29 Apr 2021 22:30:07 +0000 From: "Andrei Warkentin" To: Jeremy Linton , "devel@edk2.groups.io" CC: "ard.biesheuvel@arm.com" , "leif@nuviainc.com" , "pete@akeo.ie" , "samer.el-haj-mahmoud@arm.com" Subject: Re: [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit Thread-Topic: [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit Thread-Index: AQHXMiyl39TBpRf7/US70vrxYIbzAKrMKfHm Date: Thu, 29 Apr 2021 22:30:07 +0000 Message-ID: References: <20210415192207.3257790-1-jeremy.linton@arm.com>,<20210415192207.3257790-2-jeremy.linton@arm.com> In-Reply-To: <20210415192207.3257790-2-jeremy.linton@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: [69.174.145.79] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: ce3ca5ae-ad95-44ef-68b5-08d90b5e5771 x-ms-traffictypediagnostic: SN6PR05MB5054: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: NuwRc56AbPIKmM5SsYQuBWTsYaW7tzc2p0PMhp5Hjwja8C4YD11txmluQ92gE+Yesn5mUbKre8qt7iZzkVH4dSlvbfRGOK7LkTZLxyuAunDDVqk4BT3ArAQAZrV8k6sy5EEcLP4loE/U9yozkMf4y59VTWfKSRGjwHDTEc3/fSzYY8YxOWgwonpiG89KpQzCI4/TdGFmB7MZ4IAiozhXXN5K7jlu/F7kWrrBMIeZ/tA48BIb8moY+YkobUjxWleohnMzapVyz81xWsoraInQlhN6XwdgINn9Pzt9Av1scP7rFRi8zVURU7rWnUESY0R/Q+ztlEwvT+UQ+1B6qI/dUfymrHZCd4To6BPzHjJCSmJ/DrBzo2uOs4+lMkTRtuSx0X2Q/Ef85R2mtlMdeR4kjP6pNIZwsxxrXTj6IvIV3J4eAb0WZwl5w4APV3wGoXrtVpjWPaG7WhlQAXbqjDFHPnr9icv9qzhr+GtiD6C4v2DH6rrJAz3Q3vb2l4U8/VZpD7X3vqpNCysvO0cn2PlFL5gufPw5W3/yTJQu6OF/6pXivo8slt1+Am42bNTbAH1EnMzVFz3yLMpT1Nwo4xAc+jt0XhLUnf0E+PTrlXpIe5PRVajgt48wsdUpTd9Qjf2yrQ+/+VTTXkk+Ew8++5fCm94s8DQpaknqFDhN0UjGt2MsjS0yXw/CCwGBjQHtZM6p x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:SN7PR05MB7582.namprd05.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(366004)(346002)(136003)(39860400002)(376002)(396003)(9686003)(38100700002)(2906002)(71200400001)(316002)(166002)(7696005)(6506007)(53546011)(26005)(186003)(8676002)(54906003)(8936002)(5660300002)(110136005)(86362001)(55016002)(33656002)(66476007)(66556008)(64756008)(966005)(66946007)(66446008)(4326008)(478600001)(76116006)(52536014)(122000001)(19627405001)(45080400002)(83380400001);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?LXKdoVNtaxJQ/4VTBaVIOZ2L0Hb/DJpgM3G8FuJOD+A+rQeMCwB7J5O+4Q3i?= =?us-ascii?Q?lk/8f2ek9Ie8Lfn8B388JTehisEeUQgGPkxIK8UVCDyVWxt9kzh28fhG0ccf?= =?us-ascii?Q?Tu96ablz1t+RTzaZYRKOJzIxrRnz8mi5mnXxwb2rbEycMGIO4KlJ9fmKR8Go?= =?us-ascii?Q?fi+Os/BDHib1CSzIzimXhhaAoNxeUy8ioA+PopJyGsogXXSwqHk6vQ6k7HGu?= =?us-ascii?Q?yMSKGYQ3f2TbzafC/dNnDof2cmPywnkbDVbUUffcoF/zvlwbkOstUJEkna2t?= =?us-ascii?Q?O45e8xAk+C6SgoteoTAOYW1s5iHYZLqZHXnrK+jDaop2JVdGeHUSXkJue74+?= =?us-ascii?Q?1hr7cN8NJS7QuMqrtNXFZnDz4GXcZBMNvahhStbUuFzCGIQ8wGf39mKNfAv/?= =?us-ascii?Q?EUMtGXm+p3eg4jIg43NHRw0+kcFhxcW2DMrSVN/ZJjlsFKCXvkNdcvkqfcDz?= =?us-ascii?Q?M8bw0FqrVr313PSUGQa6QM67LD8OuwG4Eja2pm9dEg44UMhKxK3dGLEz0pr1?= =?us-ascii?Q?cn22nenWRzWZ7dtl1Fi+8jb/gz0+Ek2NK8ILv6lCCowSWHkxMbjiwF1NJNSA?= =?us-ascii?Q?EAtXTsBFK7YXBe9UsrXlyUAQM4KaUXlvN2PnGnIpWQUuDj0Y4uurc+xyWlHM?= =?us-ascii?Q?CFpFzdebVFa4xbr3+SK1Vjk7GwHlNVEcb8xSHOv2e6O4/2x8dBZa52aqeY/Q?= =?us-ascii?Q?4A8SCgA5cSdvgORL7cxjEeMpJNZcfAhQazQutOGep7P2Kwh1xvL0NeunHV0R?= =?us-ascii?Q?swx+SV3HtzNCxfue/b8vToymtJwPEq37gskUeUmDfp2kOF56Wtvjtfe98ym+?= =?us-ascii?Q?5hx/9dq+1X/43UoolwJ1XDeJsUSxVp29bEKsP0HqpumuCNXJVonZG3rFM/mh?= =?us-ascii?Q?8oJ3vZlupyr8yfExIJesNcMtc52FcZVRhzTomWBpaUBA7Jyfh9sqW6LGkjUP?= =?us-ascii?Q?vqMDmUnWG9aK57qgAopvhQZ5XIeDTCKwL3xwdFyceWDyn5wi8+uxjly7tg3L?= =?us-ascii?Q?mJbQGz5FcIJQmc75Vz/6FI0nsmFjGheeauZv2qSGI8iTHMosVtpm/rXEzKol?= =?us-ascii?Q?okkty5MAvXZT0ADPz9oxDpzqoDOHES3X1ouIiI0UPk3W3hEUbqS6psXJMgEf?= =?us-ascii?Q?Jyokc05LR/h5S5oTPq36SIdWrAcmebOEnQBobuJbOSHXyqVqho9oe/jYjFNl?= =?us-ascii?Q?UZSzIFjrX15FmB2GWG9ahNYvcuMMw0c383b/CDHe1mpUGO+oyHIWOcE1NIQt?= =?us-ascii?Q?wblb9Kxa6n0e699h1kXQCV4LK1t8iRamLowgOce1Bvk0UJWNbDmSyrRBdACi?= =?us-ascii?Q?N3A=3D?= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: vmware.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: SN7PR05MB7582.namprd05.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: ce3ca5ae-ad95-44ef-68b5-08d90b5e5771 X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Apr 2021 22:30:07.0681 (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: UpQYaJnTWnsNITy7Iv0jSt4YNYIVEg3VLO8EraWjvy+3y9ELYQmLDzBFPNMHN9vI//ft7fuB3jhgd22zLgtyHw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR05MB5054 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_SN7PR05MB75822A9A0C631D0DF85CFBCEB95F9SN7PR05MB7582namp_" --_000_SN7PR05MB75822A9A0C631D0DF85CFBCEB95F9SN7PR05MB7582namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Looks fine to me Reviewed-by: Andrei Warkentin ________________________________ From: Jeremy Linton Sent: Thursday, April 15, 2021 2:22 PM To: devel@edk2.groups.io Cc: ard.biesheuvel@arm.com ; leif@nuviainc.com ; pete@akeo.ie ; samer.el-haj-mahmoud@arm.com = ; Andrei Warkentin ; J= eremy Linton Subject: [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in tran= smit Under normal circumstances GenetSimpleNetworkTransmit won't be called unless the rest of the network stack detects the link is up. So, during normal operation when the adapter is initialized the link naturally transitions to link up, and then is ready for activity later in the boot sequence. If that hasn't happened by the time PXE runs then it will itself wait for the link. OTOH, the normal distro PXE sequence involves PXE loading shim which in turn loads grub, which tries to read machine specific configs, modules, and grub.cfg in order to prepare the boot menu. Then, once a grub selection is picked, it might try to load the kernel+initrd. In this sequence the network stack is shutdown and restarted multiple times. Grub though, starts up, notices its been network booted, reads saved network parameters and immediately tries to transmit data assuming the link is still up. When that happens grub will print "couldn't send network packet" and if that lasts long enough it fails to load grub.cfg and the user gets dropped to the grub prompt because no one in the path bothers to assure the link state has transitioned back up. For reference: https://nam04.safelinks.protection.outlook.com/?url=3Dhttps%= 3A%2F%2Fgithub.com%2Fpftf%2FRPi4%2Fissues%2F113&data=3D04%7C01%7Cawarke= ntin%40vmware.com%7Cadf7e9a53f9b4c6cff9608d90043c562%7Cb39138ca3cee4b4aa4d6= cd83d9dd62f0%7C0%7C0%7C637541113365821315%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM= C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata= =3DGdg67x%2Fv75%2B5r9bWhz8SvB%2B4VBTgZ6sEZ9SeYor%2B02E%3D&reserved=3D0 Signed-off-by: Jeremy Linton --- .../Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 24 ++++++++++++++++++= ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Sil= icon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c index 1bda18f157..29c76d8495 100644 --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include "BcmGenetDxe.h" @@ -590,9 +591,28 @@ GenetSimpleNetworkTransmit ( if (!Genet->SnpMode.MediaPresent) { // - // Don't bother transmitting if there's no link. + // We should only really get here if the link was up + // and is now down due to a stop/shutdown sequence, and + // the app (grub) doesn't bother to check link state + // because it was up a moment before. + // Lets wait a bit for the link to resume, rather than + // failing to send. In the case of grub it works either way + // but we can't be sure that is universally true, and + // hanging for a couple seconds is nicer than a screen of + // grub send failure messages. // - return EFI_NOT_READY; + int retries =3D 1000; + DEBUG ((DEBUG_INFO, "%a: Waiting 10s for link\n", __FUNCTION__)); + do { + gBS->Stall (10000); + Status =3D GenericPhyUpdateConfig (&Genet->Phy); + } while (EFI_ERROR (Status) && retries--); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "%a: no link\n", __FUNCTION__)); + return EFI_NOT_READY; + } else { + Genet->SnpMode.MediaPresent =3D TRUE; + } } if (HeaderSize !=3D 0) { -- 2.13.7 --_000_SN7PR05MB75822A9A0C631D0DF85CFBCEB95F9SN7PR05MB7582namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable
Looks fine to me

Reviewed-by: Andrei Warkentin <awarkentin@vmware.com>
From: Jeremy Linton <jer= emy.linton@arm.com>
Sent: Thursday, April 15, 2021 2:22 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ard.biesheuvel@arm.com <ard.biesheuvel@arm.com>; leif@nuvi= ainc.com <leif@nuviainc.com>; pete@akeo.ie <pete@akeo.ie>; same= r.el-haj-mahmoud@arm.com <samer.el-haj-mahmoud@arm.com>; Andrei Warke= ntin <awarkentin@vmware.com>; Jeremy Linton <jeremy.linton@arm.com= >
Subject: [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup = in transmit
 
Under normal circumstances GenetSimpleNetworkTrans= mit won't be
called unless the rest of the network stack detects the link is
up. So, during normal operation when the adapter is initialized
the link naturally transitions to link up, and then is ready for
activity later in the boot sequence. If that hasn't happened by
the time PXE runs then it will itself wait for the link.

OTOH, the normal distro PXE sequence involves PXE loading shim
which in turn loads grub, which tries to read machine specific
configs, modules, and grub.cfg in order to prepare the boot menu.
Then, once a grub selection is picked, it might try to load the
kernel+initrd.

In this sequence the network stack is shutdown and restarted
multiple times. Grub though, starts up, notices its been network
booted, reads saved network parameters and immediately tries to
transmit data assuming the link is still up.

When that happens grub will print "couldn't send network packet"<= br> and if that lasts long enough it fails to load grub.cfg and the
user gets dropped to the grub prompt because no one in the path
bothers to assure the link state has transitioned back up.

For reference: https://nam04.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fgithub.= com%2Fpftf%2FRPi4%2Fissues%2F113&amp;data=3D04%7C01%7Cawarkentin%40vmwa= re.com%7Cadf7e9a53f9b4c6cff9608d90043c562%7Cb39138ca3cee4b4aa4d6cd83d9dd62f= 0%7C0%7C0%7C637541113365821315%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi= LCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=3DGdg67= x%2Fv75%2B5r9bWhz8SvB%2B4VBTgZ6sEZ9SeYor%2B02E%3D&amp;reserved=3D0<= br>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 .../Drivers/Net/BcmGenetDxe/SimpleNetwork.c    &n= bsp;   | 24 ++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Sil= icon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
index 1bda18f157..29c76d8495 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
@@ -13,6 +13,7 @@
 #include <Library/DebugLib.h>
 #include <Library/DmaLib.h>
 #include <Library/NetLib.h>
+#include <Library/UefiBootServicesTableLib.h>
 #include <Protocol/SimpleNetwork.h>
 
 #include "BcmGenetDxe.h"
@@ -590,9 +591,28 @@ GenetSimpleNetworkTransmit (
 
   if (!Genet->SnpMode.MediaPresent) {
     //
-    // Don't bother transmitting if there's no link.
+    // We should only really get here if the link was up +    // and is now down due to a stop/shutdown sequence, and=
+    // the app (grub) doesn't bother to check link state +    // because it was up a moment before.
+    // Lets wait a bit for the link to resume, rather than<= br> +    // failing to send. In the case of grub it works either= way
+    // but we can't be sure that is universally true, and +    // hanging for a couple seconds is nicer than a screen = of
+    // grub send failure messages.
     //
-    return EFI_NOT_READY;
+    int retries =3D 1000;
+    DEBUG ((DEBUG_INFO, "%a: Waiting 10s for link\n&qu= ot;, __FUNCTION__));
+    do {
+      gBS->Stall (10000);
+      Status =3D GenericPhyUpdateConfig (&Gen= et->Phy);
+    } while (EFI_ERROR (Status) && retries--);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "%a: no link\n&qu= ot;, __FUNCTION__));
+      return EFI_NOT_READY;
+    } else {
+      Genet->SnpMode.MediaPresent =3D TRUE; +    }
   }
 
   if (HeaderSize !=3D 0) {
--
2.13.7

--_000_SN7PR05MB75822A9A0C631D0DF85CFBCEB95F9SN7PR05MB7582namp_--