[RFC PATCH] rpi: copy the EMMC controller configuration from firmware-supplied DT

[RFC PATCH] rpi: copy the EMMC controller configuration from firmware-supplied DT

[K900]

We need this to boot with a custom DTB on BCM2711C0-based Pi 4s

Signed-off-by: Ilya Katsnelson <me@0upti.me>
---
 board/raspberrypi/rpi/rpi.c | 63 +++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 372b26b6f2..f074540091 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -495,6 +495,67 @@ void *board_fdt_blob_setup(void)
 	return (void *)fw_dtb_pointer;
 }
 
+void copy_emmc_config(void *our_fdt)
+{
+	/*
+	 * As of 2021-09-28, the Pi 4 has two different revisions, one using a
+	 * B0 stepping of the BCM2711 SoC, and one using a C0 stepping.
+	 *
+	 * The two SoC versions have different, incompatible DMA mappings for
+	 * the on-board eMMC controller, which would normally make them require
+	 * two different DTs.
+	 *
+	 * Unfortunately for us, the different revisions don't actually _use_
+	 * different DTs - instead, the proprietary stage0 bootloader reads the DT,
+	 * patches it in-memory, then passes the corrected DT to the OS.
+	 *
+	 * In our case, the OS is actually U-Boot, and U-Boot can choose to
+	 * completely disregard the firmware-supplied DT and load a custom one
+	 * instead, which is used by, e.g., NixOS.
+	 *
+	 * When that happens, the DT patches applied by the firmware are also
+	 * thrown out, which leads to BCM2711C0 boards being unable to boot
+	 * due to them trying to use the hardcoded DMA mappings in the DT
+	 * (which are for the B0 revision).
+	 *
+	 * Work around that by manually copying the DMA region setup from the
+	 * firmware-provided DT into whatever DT we're actually being asked
+	 * to load.
+	 A*/
+	void *fw_fdt = (void *)fw_dtb_pointer;
+	int fw_emmc_node;
+	int our_emmc_node;
+	int length;
+	const void *fw_value;
+	int result;
+
+	fw_emmc_node = fdt_path_offset(fw_fdt, "emmc2bus");
+	if (fw_emmc_node < 0) {
+		printf("RPi: Failed to find EMMC config in FW DT: %d\n", fw_emmc_node);
+		return;
+	}
+
+	our_emmc_node = fdt_path_offset(our_fdt, "emmc2bus");
+	if (our_emmc_node < 0) {
+		printf("RPi: Failed to find EMMC config in our DT: %d\n", our_emmc_node);
+		return;
+	}
+
+	*fw_value = fdt_getprop(fw_fdt, fw_emmc_node, "dma-ranges", &length);
+	if (!fw_value) {
+		printf("RPi: Failed to get EMMC DMA ranges property from FW DT: %d\n", length);
+		return;
+	}
+
+	result = fdt_setprop(our_fdt, our_emmc_node, "dma-ranges", fw_value, length);
+	if (result != 0) {
+		printf("RPi: Failed to set EMMC DMA ranges property in our DT: %d\n", result);
+		return;
+	}
+
+	printf("RPi: successfully copied FW DT EMMC configuration to our DT!\n");
+}
+
 int ft_board_setup(void *blob, struct bd_info *bd)
 {
 	int node;
@@ -509,5 +570,7 @@ int ft_board_setup(void *blob, struct bd_info *bd)
 			   EFI_RESERVED_MEMORY_TYPE);
 #endif
 
+	copy_emmc_config(blob);
+
 	return 0;
 }
-- 
2.33.0

Re: [RFC PATCH] rpi: copy the EMMC controller configuration from firmware-supplied DT

[François Ozog]

Hi

It looks real strange to ignore the authoritative entity and try to patch a
wrong DTB embedded in U-Boot.

Arm SystemReady is definitively aligning to the authoritative entities in
the platform to give U-Boot the right basis on which it can apply
additional overlays (providing it has a way to verify origin and integrity).

Couldn’t you find a cleaner way to just leverage the previous boot DTB ?
There is another discussion thread on similar thing with RISCV.

Le mer. 29 sept. 2021 à 13:30, K900 <me@0upti.me> a écrit :

> We need this to boot with a custom DTB on BCM2711C0-based Pi 4s
>
> Signed-off-by: Ilya Katsnelson <me@0upti.me>
> ---
>  board/raspberrypi/rpi/rpi.c | 63 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>
> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> index 372b26b6f2..f074540091 100644
> --- a/board/raspberrypi/rpi/rpi.c
> +++ b/board/raspberrypi/rpi/rpi.c
> @@ -495,6 +495,67 @@ void *board_fdt_blob_setup(void)
>         return (void *)fw_dtb_pointer;
>  }
>
> +void copy_emmc_config(void *our_fdt)
> +{
> +       /*
> +        * As of 2021-09-28, the Pi 4 has two different revisions, one
> using a
> +        * B0 stepping of the BCM2711 SoC, and one using a C0 stepping.
> +        *
> +        * The two SoC versions have different, incompatible DMA mappings
> for
> +        * the on-board eMMC controller, which would normally make them
> require
> +        * two different DTs.
> +        *
> +        * Unfortunately for us, the different revisions don't actually
> _use_
> +        * different DTs - instead, the proprietary stage0 bootloader
> reads the DT,
> +        * patches it in-memory, then passes the corrected DT to the OS.
> +        *
> +        * In our case, the OS is actually U-Boot, and U-Boot can choose to
> +        * completely disregard the firmware-supplied DT and load a custom
> one
> +        * instead, which is used by, e.g., NixOS.
> +        *
> +        * When that happens, the DT patches applied by the firmware are
> also
> +        * thrown out, which leads to BCM2711C0 boards being unable to boot
> +        * due to them trying to use the hardcoded DMA mappings in the DT
> +        * (which are for the B0 revision).
> +        *
> +        * Work around that by manually copying the DMA region setup from
> the
> +        * firmware-provided DT into whatever DT we're actually being asked
> +        * to load.
> +        A*/
> +       void *fw_fdt = (void *)fw_dtb_pointer;
> +       int fw_emmc_node;
> +       int our_emmc_node;
> +       int length;
> +       const void *fw_value;
> +       int result;
> +
> +       fw_emmc_node = fdt_path_offset(fw_fdt, "emmc2bus");
> +       if (fw_emmc_node < 0) {
> +               printf("RPi: Failed to find EMMC config in FW DT: %d\n",
> fw_emmc_node);
> +               return;
> +       }
> +
> +       our_emmc_node = fdt_path_offset(our_fdt, "emmc2bus");
> +       if (our_emmc_node < 0) {
> +               printf("RPi: Failed to find EMMC config in our DT: %d\n",
> our_emmc_node);
> +               return;
> +       }
> +
> +       *fw_value = fdt_getprop(fw_fdt, fw_emmc_node, "dma-ranges",
> &length);
> +       if (!fw_value) {
> +               printf("RPi: Failed to get EMMC DMA ranges property from
> FW DT: %d\n", length);
> +               return;
> +       }
> +
> +       result = fdt_setprop(our_fdt, our_emmc_node, "dma-ranges",
> fw_value, length);
> +       if (result != 0) {
> +               printf("RPi: Failed to set EMMC DMA ranges property in our
> DT: %d\n", result);
> +               return;
> +       }
> +
> +       printf("RPi: successfully copied FW DT EMMC configuration to our
> DT!\n");
> +}
> +
>  int ft_board_setup(void *blob, struct bd_info *bd)
>  {
>         int node;
> @@ -509,5 +570,7 @@ int ft_board_setup(void *blob, struct bd_info *bd)
>                            EFI_RESERVED_MEMORY_TYPE);
>  #endif
>
> +       copy_emmc_config(blob);
> +
>         return 0;
>  }
> --
> 2.33.0
>
> --
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

Re: [RFC PATCH] rpi: copy the EMMC controller configuration from firmware-supplied DT

[François Ozog]

On Wed, 29 Sept 2021 at 14:29, Ilya K <me@0upti.me> wrote:

> There is no DTB embedded in U-Boot here - the DTB comes from the
> distribution that U-Boot is booting (in my case NixOS, but it's likely
> other similar setups are affected as well).
>
That's exactly what we (Arm/Linaro) try to avoid from now on with
SystemReady: the DTB shall never come booted OS but is a trustable
"property" of the board. You can look are
https://arm-software.github.io/ebbr/ for more details. RISC-V community
also adopted the specification to allow effective decoupling between the
booted payloads and the firmware.
Of course, non SystemReady systems can use whatever scheme they see fit.
And that may be your case. But at the same time, I think there is a path to
properly leverage the DTB provided by the Videocore firmware.

> The goal here is to have a generic image that can boot on both revisions
> of the board, and this is basically the only way to do that (outside of
> maybe adding a way to load an entire separate DT for the C0 revision, which
> will require lots of work on the distribution/kernel side as well). We
> could potentially just keep the firmware-provided DTB, and not load the
> distro-provided one, but this will be messy with mainline kernels, as those
> have diverged to use a somewhat different DT layout by now.
>
Taking the firmware DTB is the new norm and I think Peter from Red Hat, who
is in the Linaro group that deals with this firmware/OS interface may share
his views (may be different from mine though ;-)
Now for your particular case, it seems you may need to do what you
described. I guess it is a call for Arm, Linaro, RPI foundation, Beagle
foundation and 96Boards to get together on the SystemReady/DTB front.

>
>
29.09.2021, 14:43, "François Ozog" <francois.ozog@linaro.org>:
>
> Hi
>
> It looks real strange to ignore the authoritative entity and try to patch
> a wrong DTB embedded in U-Boot.
>
> Arm SystemReady is definitively aligning to the authoritative entities in
> the platform to give U-Boot the right basis on which it can apply
> additional overlays (providing it has a way to verify origin and integrity).
>
> Couldn’t you find a cleaner way to just leverage the previous boot DTB ?
> There is another discussion thread on similar thing with RISCV.
>
> Le mer. 29 sept. 2021 à 13:30, K900 <me@0upti.me> a écrit :
>
> We need this to boot with a custom DTB on BCM2711C0-based Pi 4s
>
> Signed-off-by: Ilya Katsnelson <me@0upti.me>
> ---
>  board/raspberrypi/rpi/rpi.c | 63 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>
> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> index 372b26b6f2..f074540091 100644
> --- a/board/raspberrypi/rpi/rpi.c
> +++ b/board/raspberrypi/rpi/rpi.c
> @@ -495,6 +495,67 @@ void *board_fdt_blob_setup(void)
>         return (void *)fw_dtb_pointer;
>  }
>
> +void copy_emmc_config(void *our_fdt)
> +{
> +       /*
> +        * As of 2021-09-28, the Pi 4 has two different revisions, one
> using a
> +        * B0 stepping of the BCM2711 SoC, and one using a C0 stepping.
> +        *
> +        * The two SoC versions have different, incompatible DMA mappings
> for
> +        * the on-board eMMC controller, which would normally make them
> require
> +        * two different DTs.
> +        *
> +        * Unfortunately for us, the different revisions don't actually
> _use_
> +        * different DTs - instead, the proprietary stage0 bootloader
> reads the DT,
> +        * patches it in-memory, then passes the corrected DT to the OS.
> +        *
> +        * In our case, the OS is actually U-Boot, and U-Boot can choose to
> +        * completely disregard the firmware-supplied DT and load a custom
> one
> +        * instead, which is used by, e.g., NixOS.
> +        *
> +        * When that happens, the DT patches applied by the firmware are
> also
> +        * thrown out, which leads to BCM2711C0 boards being unable to boot
> +        * due to them trying to use the hardcoded DMA mappings in the DT
> +        * (which are for the B0 revision).
> +        *
> +        * Work around that by manually copying the DMA region setup from
> the
> +        * firmware-provided DT into whatever DT we're actually being asked
> +        * to load.
> +        A*/
> +       void *fw_fdt = (void *)fw_dtb_pointer;
> +       int fw_emmc_node;
> +       int our_emmc_node;
> +       int length;
> +       const void *fw_value;
> +       int result;
> +
> +       fw_emmc_node = fdt_path_offset(fw_fdt, "emmc2bus");
> +       if (fw_emmc_node < 0) {
> +               printf("RPi: Failed to find EMMC config in FW DT: %d\n",
> fw_emmc_node);
> +               return;
> +       }
> +
> +       our_emmc_node = fdt_path_offset(our_fdt, "emmc2bus");
> +       if (our_emmc_node < 0) {
> +               printf("RPi: Failed to find EMMC config in our DT: %d\n",
> our_emmc_node);
> +               return;
> +       }
> +
> +       *fw_value = fdt_getprop(fw_fdt, fw_emmc_node, "dma-ranges",
> &length);
> +       if (!fw_value) {
> +               printf("RPi: Failed to get EMMC DMA ranges property from
> FW DT: %d\n", length);
> +               return;
> +       }
> +
> +       result = fdt_setprop(our_fdt, our_emmc_node, "dma-ranges",
> fw_value, length);
> +       if (result != 0) {
> +               printf("RPi: Failed to set EMMC DMA ranges property in our
> DT: %d\n", result);
> +               return;
> +       }
> +
> +       printf("RPi: successfully copied FW DT EMMC configuration to our
> DT!\n");
> +}
> +
>  int ft_board_setup(void *blob, struct bd_info *bd)
>  {
>         int node;
> @@ -509,5 +570,7 @@ int ft_board_setup(void *blob, struct bd_info *bd)
>                            EFI_RESERVED_MEMORY_TYPE);
>  #endif
>
> +       copy_emmc_config(blob);
> +
>         return 0;
>  }
> --
> 2.33.0
>
>
> --
> *François-Frédéric Ozog* | *Director Business Development*
> T: +33.67221.6485
> francois.ozog@linaro.org | Skype: ffozog
>
>
>

-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

Re: [RFC PATCH] rpi: copy the EMMC controller configuration from firmware-supplied DT

[François Ozog]

On Wed, 29 Sept 2021 at 14:51, Ilya K <me@0upti.me> wrote:

> Now that you mention EBBR: I've actually looked at what TianoCore does
> here, and they report different ACPI tables based on the SoC revision as
> well. It's probably a better option in the long run, but we already have
> people running NixOS on Pi4s the way it's currently set up, with u-boot and
> custom DTBs, so this might be an acceptable short term solution...
>
I am afraid so... The long run goal shall be clear though, hence I seized
the opportunity to re-state SystemReady guidelines.

>
> 15:46, September 29, 2021, "François Ozog" <francois.ozog@linaro.org>:
>
>
>
> On Wed, 29 Sept 2021 at 14:29, Ilya K <me@0upti.me> wrote:
>
> There is no DTB embedded in U-Boot here - the DTB comes from the
> distribution that U-Boot is booting (in my case NixOS, but it's likely
> other similar setups are affected as well).
>
> That's exactly what we (Arm/Linaro) try to avoid from now on with
> SystemReady: the DTB shall never come booted OS but is a trustable
> "property" of the board. You can look are
> https://arm-software.github.io/ebbr/ for more details. RISC-V community
> also adopted the specification to allow effective decoupling between the
> booted payloads and the firmware.
> Of course, non SystemReady systems can use whatever scheme they see fit.
> And that may be your case. But at the same time, I think there is a path to
> properly leverage the DTB provided by the Videocore firmware.
>
> The goal here is to have a generic image that can boot on both revisions
> of the board, and this is basically the only way to do that (outside of
> maybe adding a way to load an entire separate DT for the C0 revision, which
> will require lots of work on the distribution/kernel side as well). We
> could potentially just keep the firmware-provided DTB, and not load the
> distro-provided one, but this will be messy with mainline kernels, as those
> have diverged to use a somewhat different DT layout by now.
>
> Taking the firmware DTB is the new norm and I think Peter from Red Hat,
> who is in the Linaro group that deals with this firmware/OS interface may
> share his views (may be different from mine though ;-)
> Now for your particular case, it seems you may need to do what you
> described. I guess it is a call for Arm, Linaro, RPI foundation, Beagle
> foundation and 96Boards to get together on the SystemReady/DTB front.
>
>
>
> 29.09.2021, 14:43, "François Ozog" <francois.ozog@linaro.org>:
>
> Hi
>
> It looks real strange to ignore the authoritative entity and try to patch
> a wrong DTB embedded in U-Boot.
>
> Arm SystemReady is definitively aligning to the authoritative entities in
> the platform to give U-Boot the right basis on which it can apply
> additional overlays (providing it has a way to verify origin and integrity).
>
> Couldn’t you find a cleaner way to just leverage the previous boot DTB ?
> There is another discussion thread on similar thing with RISCV.
>
> Le mer. 29 sept. 2021 à 13:30, K900 <me@0upti.me> a écrit :
>
> We need this to boot with a custom DTB on BCM2711C0-based Pi 4s
>
> Signed-off-by: Ilya Katsnelson <me@0upti.me>
> ---
>  board/raspberrypi/rpi/rpi.c | 63 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>
> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> index 372b26b6f2..f074540091 100644
> --- a/board/raspberrypi/rpi/rpi.c
> +++ b/board/raspberrypi/rpi/rpi.c
> @@ -495,6 +495,67 @@ void *board_fdt_blob_setup(void)
>         return (void *)fw_dtb_pointer;
>  }
>
> +void copy_emmc_config(void *our_fdt)
> +{
> +       /*
> +        * As of 2021-09-28, the Pi 4 has two different revisions, one
> using a
> +        * B0 stepping of the BCM2711 SoC, and one using a C0 stepping.
> +        *
> +        * The two SoC versions have different, incompatible DMA mappings
> for
> +        * the on-board eMMC controller, which would normally make them
> require
> +        * two different DTs.
> +        *
> +        * Unfortunately for us, the different revisions don't actually
> _use_
> +        * different DTs - instead, the proprietary stage0 bootloader
> reads the DT,
> +        * patches it in-memory, then passes the corrected DT to the OS.
> +        *
> +        * In our case, the OS is actually U-Boot, and U-Boot can choose to
> +        * completely disregard the firmware-supplied DT and load a custom
> one
> +        * instead, which is used by, e.g., NixOS.
> +        *
> +        * When that happens, the DT patches applied by the firmware are
> also
> +        * thrown out, which leads to BCM2711C0 boards being unable to boot
> +        * due to them trying to use the hardcoded DMA mappings in the DT
> +        * (which are for the B0 revision).
> +        *
> +        * Work around that by manually copying the DMA region setup from
> the
> +        * firmware-provided DT into whatever DT we're actually being asked
> +        * to load.
> +        A*/
> +       void *fw_fdt = (void *)fw_dtb_pointer;
> +       int fw_emmc_node;
> +       int our_emmc_node;
> +       int length;
> +       const void *fw_value;
> +       int result;
> +
> +       fw_emmc_node = fdt_path_offset(fw_fdt, "emmc2bus");
> +       if (fw_emmc_node < 0) {
> +               printf("RPi: Failed to find EMMC config in FW DT: %d\n",
> fw_emmc_node);
> +               return;
> +       }
> +
> +       our_emmc_node = fdt_path_offset(our_fdt, "emmc2bus");
> +       if (our_emmc_node < 0) {
> +               printf("RPi: Failed to find EMMC config in our DT: %d\n",
> our_emmc_node);
> +               return;
> +       }
> +
> +       *fw_value = fdt_getprop(fw_fdt, fw_emmc_node, "dma-ranges",
> &length);
> +       if (!fw_value) {
> +               printf("RPi: Failed to get EMMC DMA ranges property from
> FW DT: %d\n", length);
> +               return;
> +       }
> +
> +       result = fdt_setprop(our_fdt, our_emmc_node, "dma-ranges",
> fw_value, length);
> +       if (result != 0) {
> +               printf("RPi: Failed to set EMMC DMA ranges property in our
> DT: %d\n", result);
> +               return;
> +       }
> +
> +       printf("RPi: successfully copied FW DT EMMC configuration to our
> DT!\n");
> +}
> +
>  int ft_board_setup(void *blob, struct bd_info *bd)
>  {
>         int node;
> @@ -509,5 +570,7 @@ int ft_board_setup(void *blob, struct bd_info *bd)
>                            EFI_RESERVED_MEMORY_TYPE);
>  #endif
>
> +       copy_emmc_config(blob);
> +
>         return 0;
>  }
> --
> 2.33.0
>
>
> --
> *François-Frédéric Ozog* | *Director Business Development*
> T: +33.67221.6485
> francois.ozog@linaro.org | Skype: ffozog
>
>
>
>
> --
> François-Frédéric Ozog | *Director Business Development*
> T: +33.67221.6485
> francois.ozog@linaro.org | Skype: ffozog
>
>
>
> --
> Sent from Yandex.Mail for mobile



-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog