[PATCH edk2-platforms 0/3] Platform/RaspberryPi: omit ConnectAll() on regular boot

[PATCH edk2-platforms 0/3] Platform/RaspberryPi: omit ConnectAll() on regular boot

[Ard Biesheuvel]

Some changes that allow RPi to boot without having to connect all controllers
on every boot. This is a step towards unifying the bespoke RPi PlatformBmLib
and the generic ArmPkg one.

Note that the 'recent change' that patch #2 refers to has not been merged
as of yet.

Cc: Jared McNeill <jmcneill@invisible.ca>
Cc: Andrei Warkentin <awarkentin@vmware.com>
Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Jeremy Linton <lintonrjeremy@gmail.com>

Ard Biesheuvel (3):
  Silicon/Broadcom/BcmGenetDxe: program MAC also when not connected
  Platform/RaspberryPi: add UEFI Shell to boot manager menu
  Platform/RaspberryPi: don't connect all devices on an ordinary boot

 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c |  7 +--
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c         | 66 ++++++++++++++++++++
 2 files changed, 67 insertions(+), 6 deletions(-)

-- 
2.26.2

[PATCH edk2-platforms 1/3] Silicon/Broadcom/BcmGenetDxe: program MAC also when not connected

[Ard Biesheuvel]

In preparation of removing the EfiBootManagerConnectAll() from the
ordinary boot path, ensure that the MAC programming of the GENET
occurs even if the SNP driver for it is never invoked by the BDS.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c | 66 ++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
index f37da3b65511..7f93c68cd608 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
@@ -12,6 +12,7 @@
 #include <Uefi.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
+#include <Library/IoLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/NetLib.h>
 #include <Library/UefiBootServicesTableLib.h>
@@ -20,6 +21,9 @@
 
 #include "BcmGenetDxe.h"
 
+STATIC EFI_EVENT      mProtocolNotifyEvent;
+STATIC VOID           *mProtocolNotifyEventRegistration;
+
 /**
   Tests to see if this driver supports a given controller.
 
@@ -305,6 +309,57 @@ STATIC EFI_DRIVER_BINDING_PROTOCOL mGenetDriverBinding = {
   NULL
 };
 
+STATIC
+VOID
+EFIAPI
+OnProtocolNotify (
+  IN  EFI_EVENT       Event,
+  IN  VOID            *Context
+  )
+{
+  BCM_GENET_PLATFORM_DEVICE_PROTOCOL  *GenetDevice;
+  EFI_STATUS                          Status;
+  EFI_HANDLE                          *HandleBuffer;
+  UINTN                               HandleCount;
+  UINTN                               Index;
+  UINT32                              Mac0, Mac1;
+
+  while (TRUE) {
+    Status = gBS->LocateHandleBuffer (ByRegisterNotify, NULL,
+                    mProtocolNotifyEventRegistration, &HandleCount,
+                    &HandleBuffer);
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+
+    for (Index = 0; Index < HandleCount; Index++) {
+      Status = gBS->HandleProtocol (HandleBuffer[Index],
+                      &gBcmGenetPlatformDeviceProtocolGuid,
+                      (VOID **)&GenetDevice);
+      ASSERT_EFI_ERROR (Status);
+
+      Mac0 = (GenetDevice->MacAddress.Addr[3])       |
+             (GenetDevice->MacAddress.Addr[2] << 8)  |
+             (GenetDevice->MacAddress.Addr[1] << 16) |
+             (GenetDevice->MacAddress.Addr[0] << 24);
+      Mac1 = (GenetDevice->MacAddress.Addr[5])       |
+             (GenetDevice->MacAddress.Addr[4] << 8);
+
+      MmioOr32 (GenetDevice->BaseAddress + GENET_SYS_RBUF_FLUSH_CTRL,
+        GENET_SYS_RBUF_FLUSH_RESET);
+      gBS->Stall (10);
+      MmioAnd32 (GenetDevice->BaseAddress + GENET_SYS_RBUF_FLUSH_CTRL,
+        ~GENET_SYS_RBUF_FLUSH_RESET);
+
+      MemoryFence ();
+
+      MmioWrite32 (GenetDevice->BaseAddress + GENET_UMAC_MAC0, Mac0);
+      MmioWrite32 (GenetDevice->BaseAddress + GENET_UMAC_MAC1, Mac1);
+    }
+    FreePool (HandleBuffer);
+  }
+}
+
 /**
   The entry point of GENET UEFI Driver.
 
@@ -336,6 +391,17 @@ GenetEntryPoint (
 
   ASSERT_EFI_ERROR (Status);
 
+  //
+  // We need to program the MAC address into each existing controller,
+  // regardless of whether UEFI ever makes use of the interface, so that
+  // the OS driver will not have to care about this.
+  //
+  mProtocolNotifyEvent = EfiCreateProtocolNotifyEvent (
+                           &gBcmGenetPlatformDeviceProtocolGuid,
+                           TPL_CALLBACK, OnProtocolNotify, NULL,
+                           &mProtocolNotifyEventRegistration);
+  ASSERT (mProtocolNotifyEvent != NULL);
+
   DEBUG ((DEBUG_INIT | DEBUG_INFO, "Installed GENET UEFI driver!\n"));
 
   return EFI_SUCCESS;
-- 
2.26.2

[PATCH edk2-platforms 2/3] Platform/RaspberryPi: add UEFI Shell to boot manager menu

[Ard Biesheuvel]

Take advantage of a recent change to the core EDK2 BDS code that
makes boot options with the LOAD_OPTION_ACTIVE flag cleared visible
in the boot manager menu, so that they can be selected manually
while never being considered for automatic booting (unless selected
specifically via BootNext)

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
index 996ba8f39938..2bd625ad7e7c 100644
--- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
@@ -460,7 +460,7 @@ PlatformRegisterOptionsAndKeys (
   RemoveStaleBootOptions ();
 
   ShellOption = PlatformRegisterFvBootOption (&gUefiShellFileGuid,
-                  L"UEFI Shell", LOAD_OPTION_CATEGORY_APP);
+                  L"UEFI Shell", 0);
   if (ShellOption != -1) {
     //
     // F1 boots Shell.
-- 
2.26.2

[PATCH edk2-platforms 3/3] Platform/RaspberryPi: don't connect all devices on an ordinary boot

[Ard Biesheuvel]

The BDS will connect device paths that are considered as boot options,
so there is really no reason to always connect absolutely everything.
So now that all the drivers have been updated to play nice in this
case, remove the ConnectAll() call from the RPi BDS code.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
index 2bd625ad7e7c..253614a646c1 100644
--- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
@@ -655,11 +655,6 @@ PlatformBootManagerAfterConsole (
     Print (BOOT_PROMPT);
   }
 
-  //
-  // Connect the rest of the devices.
-  //
-  EfiBootManagerConnectAll ();
-
   Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL, (VOID**)&EsrtManagement);
   if (!EFI_ERROR (Status)) {
     EsrtManagement->SyncEsrtFmp ();
-- 
2.26.2

Re: [PATCH edk2-platforms 1/3] Silicon/Broadcom/BcmGenetDxe: program MAC also when not connected

[Pete Batard]

Note: For some reason, I am getting extra empty lines in this patch 
series, which makes applying those patches problematic.

Apart from that, I have no notes on the proposal.

On 2020.06.04 10:50, Ard Biesheuvel wrote:
> In preparation of removing the EfiBootManagerConnectAll() from the
> ordinary boot path, ensure that the MAC programming of the GENET
> occurs even if the SNP driver for it is never invoked by the BDS.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>   Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c | 66 ++++++++++++++++++++
>   1 file changed, 66 insertions(+)
> 
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> index f37da3b65511..7f93c68cd608 100644
> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> @@ -12,6 +12,7 @@
>   #include <Uefi.h>
> 
>   #include <Library/BaseMemoryLib.h>
> 
>   #include <Library/DebugLib.h>
> 
> +#include <Library/IoLib.h>
> 
>   #include <Library/MemoryAllocationLib.h>
> 
>   #include <Library/NetLib.h>
> 
>   #include <Library/UefiBootServicesTableLib.h>
> 
> @@ -20,6 +21,9 @@
>   
> 
>   #include "BcmGenetDxe.h"
> 
>   
> 
> +STATIC EFI_EVENT      mProtocolNotifyEvent;
> 
> +STATIC VOID           *mProtocolNotifyEventRegistration;
> 
> +
> 
>   /**
> 
>     Tests to see if this driver supports a given controller.
> 
>   
> 
> @@ -305,6 +309,57 @@ STATIC EFI_DRIVER_BINDING_PROTOCOL mGenetDriverBinding = {
>     NULL
> 
>   };
> 
>   
> 
> +STATIC
> 
> +VOID
> 
> +EFIAPI
> 
> +OnProtocolNotify (
> 
> +  IN  EFI_EVENT       Event,
> 
> +  IN  VOID            *Context
> 
> +  )
> 
> +{
> 
> +  BCM_GENET_PLATFORM_DEVICE_PROTOCOL  *GenetDevice;
> 
> +  EFI_STATUS                          Status;
> 
> +  EFI_HANDLE                          *HandleBuffer;
> 
> +  UINTN                               HandleCount;
> 
> +  UINTN                               Index;
> 
> +  UINT32                              Mac0, Mac1;
> 
> +
> 
> +  while (TRUE) {
> 
> +    Status = gBS->LocateHandleBuffer (ByRegisterNotify, NULL,
> 
> +                    mProtocolNotifyEventRegistration, &HandleCount,
> 
> +                    &HandleBuffer);
> 
> +    if (EFI_ERROR (Status)) {
> 
> +      break;
> 
> +    }
> 
> +
> 
> +    for (Index = 0; Index < HandleCount; Index++) {
> 
> +      Status = gBS->HandleProtocol (HandleBuffer[Index],
> 
> +                      &gBcmGenetPlatformDeviceProtocolGuid,
> 
> +                      (VOID **)&GenetDevice);
> 
> +      ASSERT_EFI_ERROR (Status);
> 
> +
> 
> +      Mac0 = (GenetDevice->MacAddress.Addr[3])       |
> 
> +             (GenetDevice->MacAddress.Addr[2] << 8)  |
> 
> +             (GenetDevice->MacAddress.Addr[1] << 16) |
> 
> +             (GenetDevice->MacAddress.Addr[0] << 24);
> 
> +      Mac1 = (GenetDevice->MacAddress.Addr[5])       |
> 
> +             (GenetDevice->MacAddress.Addr[4] << 8);
> 
> +
> 
> +      MmioOr32 (GenetDevice->BaseAddress + GENET_SYS_RBUF_FLUSH_CTRL,
> 
> +        GENET_SYS_RBUF_FLUSH_RESET);
> 
> +      gBS->Stall (10);
> 
> +      MmioAnd32 (GenetDevice->BaseAddress + GENET_SYS_RBUF_FLUSH_CTRL,
> 
> +        ~GENET_SYS_RBUF_FLUSH_RESET);
> 
> +
> 
> +      MemoryFence ();
> 
> +
> 
> +      MmioWrite32 (GenetDevice->BaseAddress + GENET_UMAC_MAC0, Mac0);
> 
> +      MmioWrite32 (GenetDevice->BaseAddress + GENET_UMAC_MAC1, Mac1);
> 
> +    }
> 
> +    FreePool (HandleBuffer);
> 
> +  }
> 
> +}
> 
> +
> 
>   /**
> 
>     The entry point of GENET UEFI Driver.
> 
>   
> 
> @@ -336,6 +391,17 @@ GenetEntryPoint (
>   
> 
>     ASSERT_EFI_ERROR (Status);
> 
>   
> 
> +  //
> 
> +  // We need to program the MAC address into each existing controller,
> 
> +  // regardless of whether UEFI ever makes use of the interface, so that
> 
> +  // the OS driver will not have to care about this.
> 
> +  //
> 
> +  mProtocolNotifyEvent = EfiCreateProtocolNotifyEvent (
> 
> +                           &gBcmGenetPlatformDeviceProtocolGuid,
> 
> +                           TPL_CALLBACK, OnProtocolNotify, NULL,
> 
> +                           &mProtocolNotifyEventRegistration);
> 
> +  ASSERT (mProtocolNotifyEvent != NULL);
> 
> +
> 
>     DEBUG ((DEBUG_INIT | DEBUG_INFO, "Installed GENET UEFI driver!\n"));
> 
>   
> 
>     return EFI_SUCCESS;
> 

Reviewed-By: Pete Batard <pete@akeo.ie>

Re: [PATCH edk2-platforms 2/3] Platform/RaspberryPi: add UEFI Shell to boot manager menu

[Pete Batard]

On 2020.06.04 10:50, Ard Biesheuvel wrote:
> Take advantage of a recent change to the core EDK2 BDS code that
> makes boot options with the LOAD_OPTION_ACTIVE flag cleared visible
> in the boot manager menu, so that they can be selected manually
> while never being considered for automatic booting (unless selected
> specifically via BootNext)
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>   Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> index 996ba8f39938..2bd625ad7e7c 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -460,7 +460,7 @@ PlatformRegisterOptionsAndKeys (
>     RemoveStaleBootOptions ();
> 
>   
> 
>     ShellOption = PlatformRegisterFvBootOption (&gUefiShellFileGuid,
> 
> -                  L"UEFI Shell", LOAD_OPTION_CATEGORY_APP);
> 
> +                  L"UEFI Shell", 0);
> 
>     if (ShellOption != -1) {
> 
>       //
> 
>       // F1 boots Shell.
> 

Reviewed-by: Pete Batard <pete@akeo.ie>

Re: [PATCH edk2-platforms 3/3] Platform/RaspberryPi: don't connect all devices on an ordinary boot

[Pete Batard]

On 2020.06.04 10:50, Ard Biesheuvel wrote:
> The BDS will connect device paths that are considered as boot options,
> so there is really no reason to always connect absolutely everything.
> So now that all the drivers have been updated to play nice in this
> case, remove the ConnectAll() call from the RPi BDS code.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>   Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 5 -----
>   1 file changed, 5 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> index 2bd625ad7e7c..253614a646c1 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -655,11 +655,6 @@ PlatformBootManagerAfterConsole (
>       Print (BOOT_PROMPT);
> 
>     }
> 
>   
> 
> -  //
> 
> -  // Connect the rest of the devices.
> 
> -  //
> 
> -  EfiBootManagerConnectAll ();
> 
> -
> 
>     Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL, (VOID**)&EsrtManagement);
> 
>     if (!EFI_ERROR (Status)) {
> 
>       EsrtManagement->SyncEsrtFmp ();
> 

Reviewed-by: Pete Batard <pete@akeo.ie>
For the series (after application of the planned edk2 commits to confirm 
that UEFI Shell is now added as an option in Boot Manager):
Tested-by: Pete Batard <pete@akeo.ie>

Re: [PATCH edk2-platforms 1/3] Silicon/Broadcom/BcmGenetDxe: program MAC also when not connected

[Leif Lindholm]

On Thu, Jun 04, 2020 at 11:50:05 +0200, Ard Biesheuvel wrote:
> In preparation of removing the EfiBootManagerConnectAll() from the
> ordinary boot path, ensure that the MAC programming of the GENET
> occurs even if the SNP driver for it is never invoked by the BDS.

Does this not cause a behaviour change if the driver *has* been
invoked, and the MAC address has been modified with
SNP.StationAddress()?

/
    Leif

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>  Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c | 66 ++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> index f37da3b65511..7f93c68cd608 100644
> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> @@ -12,6 +12,7 @@
>  #include <Uefi.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
> +#include <Library/IoLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/NetLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
> @@ -20,6 +21,9 @@
>  
>  #include "BcmGenetDxe.h"
>  
> +STATIC EFI_EVENT      mProtocolNotifyEvent;
> +STATIC VOID           *mProtocolNotifyEventRegistration;
> +
>  /**
>    Tests to see if this driver supports a given controller.
>  
> @@ -305,6 +309,57 @@ STATIC EFI_DRIVER_BINDING_PROTOCOL mGenetDriverBinding = {
>    NULL
>  };
>  
> +STATIC
> +VOID
> +EFIAPI
> +OnProtocolNotify (
> +  IN  EFI_EVENT       Event,
> +  IN  VOID            *Context
> +  )
> +{
> +  BCM_GENET_PLATFORM_DEVICE_PROTOCOL  *GenetDevice;
> +  EFI_STATUS                          Status;
> +  EFI_HANDLE                          *HandleBuffer;
> +  UINTN                               HandleCount;
> +  UINTN                               Index;
> +  UINT32                              Mac0, Mac1;
> +
> +  while (TRUE) {
> +    Status = gBS->LocateHandleBuffer (ByRegisterNotify, NULL,
> +                    mProtocolNotifyEventRegistration, &HandleCount,
> +                    &HandleBuffer);
> +    if (EFI_ERROR (Status)) {
> +      break;
> +    }
> +
> +    for (Index = 0; Index < HandleCount; Index++) {
> +      Status = gBS->HandleProtocol (HandleBuffer[Index],
> +                      &gBcmGenetPlatformDeviceProtocolGuid,
> +                      (VOID **)&GenetDevice);
> +      ASSERT_EFI_ERROR (Status);
> +
> +      Mac0 = (GenetDevice->MacAddress.Addr[3])       |
> +             (GenetDevice->MacAddress.Addr[2] << 8)  |
> +             (GenetDevice->MacAddress.Addr[1] << 16) |
> +             (GenetDevice->MacAddress.Addr[0] << 24);
> +      Mac1 = (GenetDevice->MacAddress.Addr[5])       |
> +             (GenetDevice->MacAddress.Addr[4] << 8);
> +
> +      MmioOr32 (GenetDevice->BaseAddress + GENET_SYS_RBUF_FLUSH_CTRL,
> +        GENET_SYS_RBUF_FLUSH_RESET);
> +      gBS->Stall (10);
> +      MmioAnd32 (GenetDevice->BaseAddress + GENET_SYS_RBUF_FLUSH_CTRL,
> +        ~GENET_SYS_RBUF_FLUSH_RESET);
> +
> +      MemoryFence ();
> +
> +      MmioWrite32 (GenetDevice->BaseAddress + GENET_UMAC_MAC0, Mac0);
> +      MmioWrite32 (GenetDevice->BaseAddress + GENET_UMAC_MAC1, Mac1);
> +    }
> +    FreePool (HandleBuffer);
> +  }
> +}
> +
>  /**
>    The entry point of GENET UEFI Driver.
>  
> @@ -336,6 +391,17 @@ GenetEntryPoint (
>  
>    ASSERT_EFI_ERROR (Status);
>  
> +  //
> +  // We need to program the MAC address into each existing controller,
> +  // regardless of whether UEFI ever makes use of the interface, so that
> +  // the OS driver will not have to care about this.
> +  //
> +  mProtocolNotifyEvent = EfiCreateProtocolNotifyEvent (
> +                           &gBcmGenetPlatformDeviceProtocolGuid,
> +                           TPL_CALLBACK, OnProtocolNotify, NULL,
> +                           &mProtocolNotifyEventRegistration);
> +  ASSERT (mProtocolNotifyEvent != NULL);
> +
>    DEBUG ((DEBUG_INIT | DEBUG_INFO, "Installed GENET UEFI driver!\n"));
>  
>    return EFI_SUCCESS;
> -- 
> 2.26.2
> 

Re: [PATCH edk2-platforms 1/3] Silicon/Broadcom/BcmGenetDxe: program MAC also when not connected

[Ard Biesheuvel]

On 6/4/20 4:53 PM, Leif Lindholm wrote:
> On Thu, Jun 04, 2020 at 11:50:05 +0200, Ard Biesheuvel wrote:
>> In preparation of removing the EfiBootManagerConnectAll() from the
>> ordinary boot path, ensure that the MAC programming of the GENET
>> occurs even if the SNP driver for it is never invoked by the BDS.
> 
> Does this not cause a behaviour change if the driver *has* been
> invoked, and the MAC address has been modified with
> SNP.StationAddress()?
> 

No, the MAC is programmed when the device is registered. If SNP takes 
control, it does so afterwards, and will be able to supersede the MAC as 
before.

Re: [PATCH edk2-platforms 1/3] Silicon/Broadcom/BcmGenetDxe: program MAC also when not connected

[Leif Lindholm]

On Thu, Jun 04, 2020 at 16:57:00 +0200, Ard Biesheuvel wrote:
> On 6/4/20 4:53 PM, Leif Lindholm wrote:
> > On Thu, Jun 04, 2020 at 11:50:05 +0200, Ard Biesheuvel wrote:
> > > In preparation of removing the EfiBootManagerConnectAll() from the
> > > ordinary boot path, ensure that the MAC programming of the GENET
> > > occurs even if the SNP driver for it is never invoked by the BDS.
> > 
> > Does this not cause a behaviour change if the driver *has* been
> > invoked, and the MAC address has been modified with
> > SNP.StationAddress()?
> 
> No, the MAC is programmed when the device is registered. If SNP takes
> control, it does so afterwards, and will be able to supersede the MAC as
> before.

Ah, good.

Then, for the series:
Acked-by: Leif Lindholm <leif@nuviainc.com>

Re: [PATCH edk2-platforms 3/3] Platform/RaspberryPi: don't connect all devices on an ordinary boot

[Ard Biesheuvel]

On 6/4/20 2:27 PM, Pete Batard wrote:
> On 2020.06.04 10:50, Ard Biesheuvel wrote:
>> The BDS will connect device paths that are considered as boot options,
>> so there is really no reason to always connect absolutely everything.
>> So now that all the drivers have been updated to play nice in this
>> case, remove the ConnectAll() call from the RPi BDS code.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> ---
>>   Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 5 
>> -----
>>   1 file changed, 5 deletions(-)
>>
>> diff --git 
>> a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c 
>> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
>> index 2bd625ad7e7c..253614a646c1 100644
>> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
>> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
>> @@ -655,11 +655,6 @@ PlatformBootManagerAfterConsole (
>>       Print (BOOT_PROMPT);
>>
>>     }
>>
>>
>> -  //
>>
>> -  // Connect the rest of the devices.
>>
>> -  //
>>
>> -  EfiBootManagerConnectAll ();
>>
>> -
>>
>>     Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL, 
>> (VOID**)&EsrtManagement);
>>
>>     if (!EFI_ERROR (Status)) {
>>
>>       EsrtManagement->SyncEsrtFmp ();
>>
> 
> Reviewed-by: Pete Batard <pete@akeo.ie>
> For the series (after application of the planned edk2 commits to confirm 
> that UEFI Shell is now added as an option in Boot Manager):
> Tested-by: Pete Batard <pete@akeo.ie>

Series pushed as 4a937016142e..c8000ecccc83

Thanks all