<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);"> Looks fine to me</div> <div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);"> Reviewed-by: Andrei Warkentin <awarkentin@vmware.com></div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Jeremy Linton <jeremy.linton@arm.com><br> <b>Sent:</b> Thursday, April 15, 2021 2:22 PM<br> <b>To:</b> devel@edk2.groups.io <devel@edk2.groups.io><br> <b>Cc:</b> ard.biesheuvel@arm.com <ard.biesheuvel@arm.com>; leif@nuviainc.com <leif@nuviainc.com>; pete@akeo.ie <pete@akeo.ie>; samer.el-haj-mahmoud@arm.com <samer.el-haj-mahmoud@arm.com>; Andrei Warkentin <awarkentin@vmware.com>; Jeremy Linton <jeremy.linton@arm.com><br> <b>Subject:</b> [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit</font> <div> </div> </div> <div class="BodyFragment"><font size="2"><span style="font-size:11pt;"> <div class="PlainText">Under normal circumstances GenetSimpleNetworkTransmit won't be<br> called unless the rest of the network stack detects the link is<br> up. So, during normal operation when the adapter is initialized<br> the link naturally transitions to link up, and then is ready for<br> activity later in the boot sequence. If that hasn't happened by<br> the time PXE runs then it will itself wait for the link.<br> <br> OTOH, the normal distro PXE sequence involves PXE loading shim<br> which in turn loads grub, which tries to read machine specific<br> configs, modules, and grub.cfg in order to prepare the boot menu.<br> Then, once a grub selection is picked, it might try to load the<br> kernel+initrd.<br> <br> In this sequence the network stack is shutdown and restarted<br> multiple times. Grub though, starts up, notices its been network<br> booted, reads saved network parameters and immediately tries to<br> transmit data assuming the link is still up.<br> <br> 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<br> user gets dropped to the grub prompt because no one in the path<br> bothers to assure the link state has transitioned back up.<br> <br> For reference: <a href="https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpftf%2FRPi4%2Fissues%2F113&data=04%7C01%7Cawarkentin%40vmware.com%7Cadf7e9a53f9b4c6cff9608d90043c562%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637541113365821315%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Gdg67x%2Fv75%2B5r9bWhz8SvB%2B4VBTgZ6sEZ9SeYor%2B02E%3D&reserved=0"> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpftf%2FRPi4%2Fissues%2F113&data=04%7C01%7Cawarkentin%40vmware.com%7Cadf7e9a53f9b4c6cff9608d90043c562%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637541113365821315%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Gdg67x%2Fv75%2B5r9bWhz8SvB%2B4VBTgZ6sEZ9SeYor%2B02E%3D&reserved=0</a><br> <br> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com><br> ---<br> .../Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 24 ++++++++++++++++++++--<br> 1 file changed, 22 insertions(+), 2 deletions(-)<br> <br> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c<br> index 1bda18f157..29c76d8495 100644<br> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c<br> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c<br> @@ -13,6 +13,7 @@<br> #include <Library/DebugLib.h><br> #include <Library/DmaLib.h><br> #include <Library/NetLib.h><br> +#include <Library/UefiBootServicesTableLib.h><br> #include <Protocol/SimpleNetwork.h><br> <br> #include "BcmGenetDxe.h"<br> @@ -590,9 +591,28 @@ GenetSimpleNetworkTransmit (<br> <br> if (!Genet->SnpMode.MediaPresent) {<br> //<br> - // Don't bother transmitting if there's no link.<br> + // We should only really get here if the link was up<br> + // and is now down due to a stop/shutdown sequence, and<br> + // the app (grub) doesn't bother to check link state<br> + // because it was up a moment before.<br> + // Lets wait a bit for the link to resume, rather than<br> + // failing to send. In the case of grub it works either way<br> + // but we can't be sure that is universally true, and<br> + // hanging for a couple seconds is nicer than a screen of<br> + // grub send failure messages.<br> //<br> - return EFI_NOT_READY;<br> + int retries = 1000;<br> + DEBUG ((DEBUG_INFO, "%a: Waiting 10s for link\n", __FUNCTION__));<br> + do {<br> + gBS->Stall (10000);<br> + Status = GenericPhyUpdateConfig (&Genet->Phy);<br> + } while (EFI_ERROR (Status) && retries--);<br> + if (EFI_ERROR (Status)) {<br> + DEBUG ((DEBUG_ERROR, "%a: no link\n", __FUNCTION__));<br> + return EFI_NOT_READY;<br> + } else {<br> + Genet->SnpMode.MediaPresent = TRUE;<br> + }<br> }<br> <br> if (HeaderSize != 0) {<br> -- <br> 2.13.7<br> <br> </div> </span></font></div> </body> </html> <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links:<p> You receive all messages sent to this group. <p> <a target="_blank" href="https://edk2.groups.io/g/devel/message/74618">View/Reply Online (#74618)</a> | | <a target="_blank" href="https://groups.io/mt/82125863/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a><br> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com]<br> <div width="1" style="color:white;clear:both">_._,_._,_</div>