[PATCH v2 0/2] rpi: Fix PXE issues with grub

[PATCH v2 0/2] rpi: Fix PXE issues with grub

[Jeremy Linton]

When PXE booting with grub the network link
isn't given a chance to resume so grub's transmit
calls fail. This results in failed boots. Similarly
the DMA range for the adapter isn't right since it
doesn't have a 32-bit restriction. Again this keeps
grub from failing on 8G devices,

v1-v2:
	Update commit message to note solution
	Add Review-by's

Jeremy Linton (2):
  Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit
  Platform/RaspberryPi: Increase genet dma window

 Platform/RaspberryPi/RPi4/RPi4.dsc                 |  2 +-
 .../Drivers/Net/BcmGenetDxe/SimpleNetwork.c        | 24 ++++++++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

-- 
2.13.7

[PATCH v2 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit

[Jeremy Linton]

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://github.com/pftf/RPi4/issues/113

This patch fixes that by polling the link state via
GenericPhyUpdateConfig() for ten seconds in the transmit path
when the link is down. If the link recovers within this timeout
the state machine is transitioned and we continue data
transmission. If the 10 seconds expires without the link
resuming we will fail as before. While full link negotiation
can be fast, it frequently can take a second or two, or longer
depending on the remote peer on the other end of the
Ethernet cable. It seems auto MDX can slow this down,
and certain vendors products seem to be slower than the
norm. Ten seconds may not cover some of these possibilities,
but the user should validate cabling and the switch/peer's
port configuration if resuming the link is taking > 10
seconds. Picking a longer timeout is a tradeoff between the
machine appearing to hang for extended periods of time
(due to grub retries) if the link is actually down vs
generally providing enough time for most endpoints to
complete the negotiation.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Reviewed-by: Jared McNeill <jmcneill@invisible.ca>
Reviewed-by: Andrei Warkentin <awarkentin@vmware.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

[PATCH v2 2/2] Platform/RaspberryPi: Increase genet dma window

[Jeremy Linton]

The genet is capable of addressing the entire memory space
on the RPI4. Lets allow it to dma into those regions.
This solves intermittent issues with grub/etc being able
to communicate when the 3G limit is lifted on 8G boards.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Reviewed-by: Jared McNeill <jmcneill@invisible.ca>
---
 Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 5c6783eae7..cf796acf6a 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -717,7 +717,7 @@
   Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf {
     <PcdsFixedAtBuild>
       gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset|0x00000000
-      gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit|0xffffffff
+      gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit|0xffffffffff
   }
 
   #
-- 
2.13.7

Re: [PATCH v2 2/2] Platform/RaspberryPi: Increase genet dma window

[Ard Biesheuvel]

On Wed, 12 May 2021 at 00:37, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> The genet is capable of addressing the entire memory space
> on the RPI4. Lets allow it to dma into those regions.
> This solves intermittent issues with grub/etc being able
> to communicate when the 3G limit is lifted on 8G boards.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> Reviewed-by: Jared McNeill <jmcneill@invisible.ca>
> ---
>  Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index 5c6783eae7..cf796acf6a 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -717,7 +717,7 @@
>    Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf {
>      <PcdsFixedAtBuild>
>        gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset|0x00000000
> -      gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit|0xffffffff
> +      gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit|0xffffffffff
>    }
>
>    #
> --
> 2.13.7
>

As I said before, I am fine with this patch, but I would still like to
understand why this causes a failure. The device limit is used by the
DMA layer to decide whether bounce buffering is needed or not, but the
API should work as expected either way, with perhaps only an impact on
performance if bounce buffering is done unnecessarily.

What kind of errors are you seeing (and fixing) with this change?

Re: [PATCH v2 0/2] rpi: Fix PXE issues with grub

[Ard Biesheuvel]

On Wed, 12 May 2021 at 00:37, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> When PXE booting with grub the network link
> isn't given a chance to resume so grub's transmit
> calls fail. This results in failed boots. Similarly
> the DMA range for the adapter isn't right since it
> doesn't have a 32-bit restriction. Again this keeps
> grub from failing on 8G devices,
>
> v1-v2:
>         Update commit message to note solution
>         Add Review-by's
>

Pushed as 9466dad3ae87..b291607503dc

Thanks all,

> Jeremy Linton (2):
>   Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit
>   Platform/RaspberryPi: Increase genet dma window
>
>  Platform/RaspberryPi/RPi4/RPi4.dsc                 |  2 +-
>  .../Drivers/Net/BcmGenetDxe/SimpleNetwork.c        | 24 ++++++++++++++++++++--
>  2 files changed, 23 insertions(+), 3 deletions(-)
>
> --
> 2.13.7
>