[edk2-devel] [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit
Jeremy Linton
jeremy.linton at arm.com
Mon May 10 17:26:23 UTC 2021
Hi,
On 5/10/21 11:56 AM, Ard Biesheuvel wrote:
> On Thu, 15 Apr 2021 at 21:22, Jeremy Linton <jeremy.linton at arm.com> wrote:
>>
>> 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.
>>
>
> This is an excellent problem description. Could we dedicate a couple
> of paragraphs to the solution as well? Especially given that 'wait for
> random # of seconds' seems to be the taken approach here, which seems
> a bit sloppy to me, given that we should be able to detect whether the
> link is up, no?
Isn't that what GenericPhyUpdateConfig is doing? Checking for linkup,
and updating the phy state? So once the link goes up, it return
EFI_SUCCESS and the loop terminates.
I was actually a bit concerned that the 10s wasn't enough since AFAIK a
large part of the delay is the remote side.
>
>
>
>> For reference: https://github.com/pftf/RPi4/issues/113
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton at arm.com>
>> ---
>> .../Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 24 ++++++++++++++++++++--
>> 1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Silicon/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
>> + // 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 = 1000;
>> + DEBUG ((DEBUG_INFO, "%a: Waiting 10s for link\n", __FUNCTION__));
>> + do {
>> + gBS->Stall (10000);
>> + Status = 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 = TRUE;
>> + }
>> }
>>
>> if (HeaderSize != 0) {
>> --
>> 2.13.7
>>
>>
>>
>>
>>
>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74904): https://edk2.groups.io/g/devel/message/74904
Mute This Topic: https://groups.io/mt/82125863/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list