[edk2-devel][RESEND][PATCH 0/2] Resend of the 2 previous RPi patches

[edk2-devel][RESEND][PATCH 0/2] Resend of the 2 previous RPi patches

[Pete Batard]

As requested due to mangling, this is an unmodified resend of the 2 previous patches:
* https://edk2.groups.io/g/devel/message/55159
* https://edk2.groups.io/g/devel/message/55160

Andrei Warkentin (2):
  Platform/RPi4: allow overriding TF-A binaries during build
  Platform/RPi4/Library/PlatformBootManagerLib: remove dead logo code

 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c               | 18 ------------------
 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  1 -
 Platform/RaspberryPi/RPi4/RPi4.dsc                                             | 14 ++++++++++++++
 Platform/RaspberryPi/RPi4/RPi4.fdf                                             |  2 +-
 4 files changed, 15 insertions(+), 20 deletions(-)

-- 
2.21.0.windows.1

[edk2-devel][RESEND][PATCH 1/2] Platform/RPi4: allow overriding TF-A binaries during build

[Pete Batard]

From: Andrei Warkentin <awarkentin@vmware.com>

For PFTF developers working on the firmware, being able to use a
local TF-A build without extra extra copy operations ends up being
very helpful.

This can be accomplished via a TFA_BUILD_ARTIFACTS option passed
to the edk2 build tool.

If/when the Pi 3 and 4 DSC/FDFs become unified, this will be even
more important to trivially perform a full clean upstream build
for either platform, without having to worry about different TF-A
deliverables - Pi 4 uses bl31.bin, while Pi 3 uses fip.bin and bl1.bin.

A similar Pi 3 patch to follow.

The context for this is the community Raspberry Pi 4 firmware
project (https://https://rpi4-uefi.dev, https://github.com/pftf),
which you might have heard already about from Pete Batard, who
upstreamed much of my previous Pi 3 and Pi 4 enablement patches.

Signed-off-by: Andrei Warkentin <awarkentin@vmware.com>
---
 Platform/RaspberryPi/RPi4/RPi4.dsc | 14 ++++++++++++++
 Platform/RaspberryPi/RPi4/RPi4.fdf |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index c039f6df2eb4..1d38f4ab051d 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -40,6 +40,20 @@ [Defines]
   DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
   DEFINE ACPI_BASIC_MODE_ENABLE  = FALSE
 
+!ifndef TFA_BUILD_ARTIFACTS
+  #
+  # Default TF-A binary checked into edk2-non-osi.
+  #
+  DEFINE TFA_BUILD_BL31 = Platform/RaspberryPi/$(PLATFORM_NAME)/TrustedFirmware/bl31_pl011.bin
+!else
+  #
+  # Usually we use the checked-in binaries, but for developers working
+  # on the firmware, being able to use a local TF-A build without extra copy
+  # operations ends up being very helpful.
+  #
+  DEFINE TFA_BUILD_BL31 = $(TFA_BUILD_ARTIFACTS)/bl31.bin
+!endif
+
 ################################################################################
 #
 # Library Class section - list of all Library Classes needed by this Platform.
diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RPi4/RPi4.fdf
index b2a6ac9e6c66..8e2d6fd49a9d 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.fdf
+++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
@@ -51,7 +51,7 @@ [FD.RPI_EFI]
 # ATF primary boot image
 #
 0x00000000|0x00020000
-FILE = Platform/RaspberryPi/$(PLATFORM_NAME)/TrustedFirmware/bl31_pl011.bin
+FILE = $(TFA_BUILD_BL31)
 
 #
 # DTB.
-- 
2.21.0.windows.1

[edk2-devel][RESEND][PATCH 2/2] Platform/RPi4/Library/PlatformBootManagerLib: remove dead logo code

[Pete Batard]

From: Andrei Warkentin <awarkentin@vmware.com>

Back in RaspberryPiPkg (before upstream Pi 3) support, I wrote
some extra code in PlatformBootManagerLib and BootGraphicsResourceTableDxe
to clear out the logo/BGRT, so that Windows would always show its own
logo instead of the platform logo. It kind of made sense back in the day,
when they only portion of Windows that "ran" on Pi 3 was the part that
could display a logo before BSODing...

The code in PlatformBootManagerLib (that this patch is removing) only
worked with the matching BootGraphicsResourceTableDxe change*** that
never got upstreamed. Moreover, Windows (for logo/cert) requires BGRT
so these kinds of shennigans aren't worth the effort.

So, remove the dead code.

***https://github.com/andreiw/RaspberryPiPkg/blob/master/edk2Patches/0003-BootGraphicsResourceTableDxe-properly-handle-SetBoot.patch

Signed-off-by: Andrei Warkentin <awarkentin@vmware.com>
---
 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c               | 18 ------------------
 Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  1 -
 2 files changed, 19 deletions(-)

diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
index 12c3829d28f5..996ba8f39938 100644
--- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
@@ -25,7 +25,6 @@
 #include <Protocol/LoadedImage.h>
 #include <Guid/EventGroup.h>
 #include <Guid/TtyTerm.h>
-#include <Protocol/BootLogo.h>
 
 #include "PlatformBm.h"
 
@@ -691,7 +690,6 @@ PlatformBootManagerWaitCallback (
   EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION White;
   UINT16                              Timeout;
   EFI_STATUS                          Status;
-  EFI_BOOT_LOGO_PROTOCOL *BootLogo;
 
   Timeout = PcdGet16 (PcdPlatformBootTimeOut);
 
@@ -711,22 +709,6 @@ PlatformBootManagerWaitCallback (
   } else {
     Print (L".");
   }
-
-  if (TimeoutRemain == 0) {
-    BootLogo = NULL;
-
-    //
-    // Clear out the boot logo so that Windows displays its own logo
-    // instead of ours.
-    //
-    Status = gBS->LocateProtocol (&gEfiBootLogoProtocolGuid, NULL, (VOID**)&BootLogo);
-    if (!EFI_ERROR (Status) && (BootLogo != NULL)) {
-      Status = BootLogo->SetBootLogo (BootLogo, NULL, 0, 0, 0, 0);
-      ASSERT_EFI_ERROR (Status);
-    };
-
-    gST->ConOut->ClearScreen (gST->ConOut);
-  }
 }
 
 /**
diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 79b75d667f55..e40b3f096a4f 100644
--- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -81,4 +81,3 @@ [Protocols]
   gEfiSimpleFileSystemProtocolGuid
   gEsrtManagementProtocolGuid
   gEfiUsb2HcProtocolGuid
-  gEfiBootLogoProtocolGuid
-- 
2.21.0.windows.1

Re: [edk2-devel][RESEND][PATCH 2/2] Platform/RPi4/Library/PlatformBootManagerLib: remove dead logo code

[Ard Biesheuvel]

On Mon, 2 Mar 2020 at 15:42, Pete Batard <pete@akeo.ie> wrote:
>
> From: Andrei Warkentin <awarkentin@vmware.com>
>
> Back in RaspberryPiPkg (before upstream Pi 3) support, I wrote
> some extra code in PlatformBootManagerLib and BootGraphicsResourceTableDxe
> to clear out the logo/BGRT, so that Windows would always show its own
> logo instead of the platform logo. It kind of made sense back in the day,
> when they only portion of Windows that "ran" on Pi 3 was the part that
> could display a logo before BSODing...
>
> The code in PlatformBootManagerLib (that this patch is removing) only
> worked with the matching BootGraphicsResourceTableDxe change*** that
> never got upstreamed. Moreover, Windows (for logo/cert) requires BGRT
> so these kinds of shennigans aren't worth the effort.
>
> So, remove the dead code.
>
> ***https://github.com/andreiw/RaspberryPiPkg/blob/master/edk2Patches/0003-BootGraphicsResourceTableDxe-properly-handle-SetBoot.patch
>
> Signed-off-by: Andrei Warkentin <awarkentin@vmware.com>

Thanks

Pushed as f57b439e5610..30324eefb3a4 (with Pete's ack)

> ---
>  Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c               | 18 ------------------
>  Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  1 -
>  2 files changed, 19 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> index 12c3829d28f5..996ba8f39938 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -25,7 +25,6 @@
>  #include <Protocol/LoadedImage.h>
>  #include <Guid/EventGroup.h>
>  #include <Guid/TtyTerm.h>
> -#include <Protocol/BootLogo.h>
>
>  #include "PlatformBm.h"
>
> @@ -691,7 +690,6 @@ PlatformBootManagerWaitCallback (
>    EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION White;
>    UINT16                              Timeout;
>    EFI_STATUS                          Status;
> -  EFI_BOOT_LOGO_PROTOCOL *BootLogo;
>
>    Timeout = PcdGet16 (PcdPlatformBootTimeOut);
>
> @@ -711,22 +709,6 @@ PlatformBootManagerWaitCallback (
>    } else {
>      Print (L".");
>    }
> -
> -  if (TimeoutRemain == 0) {
> -    BootLogo = NULL;
> -
> -    //
> -    // Clear out the boot logo so that Windows displays its own logo
> -    // instead of ours.
> -    //
> -    Status = gBS->LocateProtocol (&gEfiBootLogoProtocolGuid, NULL, (VOID**)&BootLogo);
> -    if (!EFI_ERROR (Status) && (BootLogo != NULL)) {
> -      Status = BootLogo->SetBootLogo (BootLogo, NULL, 0, 0, 0, 0);
> -      ASSERT_EFI_ERROR (Status);
> -    };
> -
> -    gST->ConOut->ClearScreen (gST->ConOut);
> -  }
>  }
>
>  /**
> diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 79b75d667f55..e40b3f096a4f 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -81,4 +81,3 @@ [Protocols]
>    gEfiSimpleFileSystemProtocolGuid
>    gEsrtManagementProtocolGuid
>    gEfiUsb2HcProtocolGuid
> -  gEfiBootLogoProtocolGuid
> --
> 2.21.0.windows.1
>

Re: [edk2-devel][RESEND][PATCH 1/2] Platform/RPi4: allow overriding TF-A binaries during build

[Ard Biesheuvel]

On Mon, 2 Mar 2020 at 15:42, Pete Batard <pete@akeo.ie> wrote:
>
> From: Andrei Warkentin <awarkentin@vmware.com>
>
> For PFTF developers working on the firmware, being able to use a
> local TF-A build without extra extra copy operations ends up being
> very helpful.
>
> This can be accomplished via a TFA_BUILD_ARTIFACTS option passed
> to the edk2 build tool.
>
> If/when the Pi 3 and 4 DSC/FDFs become unified, this will be even
> more important to trivially perform a full clean upstream build
> for either platform, without having to worry about different TF-A
> deliverables - Pi 4 uses bl31.bin, while Pi 3 uses fip.bin and bl1.bin.
>
> A similar Pi 3 patch to follow.
>
> The context for this is the community Raspberry Pi 4 firmware
> project (https://https://rpi4-uefi.dev, https://github.com/pftf),
> which you might have heard already about from Pete Batard, who
> upstreamed much of my previous Pi 3 and Pi 4 enablement patches.
>
> Signed-off-by: Andrei Warkentin <awarkentin@vmware.com>

Do we really need this patch? For development, you can put anything
you want here. For doing releases, I'd expect edk2-platforms to be in
sync with edk2-non-osi, given that there are more blobs there than
TF-A, right?



> ---
>  Platform/RaspberryPi/RPi4/RPi4.dsc | 14 ++++++++++++++
>  Platform/RaspberryPi/RPi4/RPi4.fdf |  2 +-
>  2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index c039f6df2eb4..1d38f4ab051d 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -40,6 +40,20 @@ [Defines]
>    DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
>    DEFINE ACPI_BASIC_MODE_ENABLE  = FALSE
>
> +!ifndef TFA_BUILD_ARTIFACTS
> +  #
> +  # Default TF-A binary checked into edk2-non-osi.
> +  #
> +  DEFINE TFA_BUILD_BL31 = Platform/RaspberryPi/$(PLATFORM_NAME)/TrustedFirmware/bl31_pl011.bin
> +!else
> +  #
> +  # Usually we use the checked-in binaries, but for developers working
> +  # on the firmware, being able to use a local TF-A build without extra copy
> +  # operations ends up being very helpful.
> +  #
> +  DEFINE TFA_BUILD_BL31 = $(TFA_BUILD_ARTIFACTS)/bl31.bin
> +!endif
> +
>  ################################################################################
>  #
>  # Library Class section - list of all Library Classes needed by this Platform.
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RPi4/RPi4.fdf
> index b2a6ac9e6c66..8e2d6fd49a9d 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.fdf
> +++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
> @@ -51,7 +51,7 @@ [FD.RPI_EFI]
>  # ATF primary boot image
>  #
>  0x00000000|0x00020000
> -FILE = Platform/RaspberryPi/$(PLATFORM_NAME)/TrustedFirmware/bl31_pl011.bin
> +FILE = $(TFA_BUILD_BL31)
>
>  #
>  # DTB.
> --
> 2.21.0.windows.1
>

Re: [edk2-devel][RESEND][PATCH 1/2] Platform/RPi4: allow overriding TF-A binaries during build

[Andrei Warkentin]

Hi Ard,

> Do we really need this patch? For development, you can put anything
> you want here. For doing releases, I'd expect edk2-platforms to be in
> sync with edk2-non-osi, given that there are more blobs there than
> TF-A, right?

This is a developer productivity improvement that I've found to be useful and it has no impact on others for the default case of building code for release.

Sure, you can copy things around and stage/unstage files, but I like automating my work and I tend to track the upstream TF-A (for regressions, rpi4-uefi.dev work),
so being able to quickly generate builds both with internal and external TF-A and without extra hoopla has been key. The time I avoid dealing with dirty trees is time I can spend elsewhere else making Pi 4 implementation better.

There aren't really any other blobs that matter for Pi 4...The embedded DTB is going to go away as part of fixing the regression seen with  https://github.com/pftf/RPi4/issues/17 (on the Pi 4 you can't really boot anyway without passing a DTB in config.txt).

A

Re: [edk2-devel][RESEND][PATCH 1/2] Platform/RPi4: allow overriding TF-A binaries during build

[Ard Biesheuvel]

On Mon, 2 Mar 2020 at 18:40, Andrei Warkentin <awarkentin@vmware.com> wrote:
>
> Hi Ard,
>
> > Do we really need this patch? For development, you can put anything
> > you want here. For doing releases, I'd expect edk2-platforms to be in
> > sync with edk2-non-osi, given that there are more blobs there than
> > TF-A, right?
>
> This is a developer productivity improvement that I've found to be useful and it has no impact on others for the default case of building code for release.
>
> Sure, you can copy things around and stage/unstage files, but I like automating my work and I tend to track the upstream TF-A (for regressions, rpi4-uefi.dev work),
> so being able to quickly generate builds both with internal and external TF-A and without extra hoopla has been key. The time I avoid dealing with dirty trees is time I can spend elsewhere else making Pi 4 implementation better.
>
> There aren't really any other blobs that matter for Pi 4...The embedded DTB is going to go away as part of fixing the regression seen with  https://github.com/pftf/RPi4/issues/17 (on the Pi 4 you can't really boot anyway without passing a DTB in config.txt).
>

Fair enough. I still don't quite see the point, but I don't really mind either.

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Pushed as 30324eefb3a4..07cc442f7212