[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 at 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 at linaro.org>:
>
>
>
> On Wed, 29 Sept 2021 at 14:29, Ilya K <me at 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 at 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 at 0upti.me> a écrit :
>
> We need this to boot with a custom DTB on BCM2711C0-based Pi 4s
>
> Signed-off-by: Ilya Katsnelson <me at 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 at linaro.org | Skype: ffozog
>
>
>
>
> --
> François-Frédéric Ozog | *Director Business Development*
> T: +33.67221.6485
> francois.ozog at 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 at linaro.org | Skype: ffozog