On Mon, Jun 19, 2023 at 1:40 PM Tim Harvey <thar...@gateworks.com> wrote:
>
> On Fri, Jun 16, 2023 at 4:52 PM Tim Harvey <thar...@gateworks.com> wrote:
> >
> > On Thu, Jun 15, 2023 at 8:31 PM Peng Fan <peng....@oss.nxp.com> wrote:
> > >
> > >
> > >
> > > On 6/16/2023 9:56 AM, Tim Harvey wrote:
> > > > Greetings,
> > > >
> > > > I've seen several IMX8M boards include a firmware/optee node in the
> > > > U-Boot dt (git grep optee arch/arm/dts/imx8m*.dtsi) yet but none that
> > > > I see configure binman to actually add the binary in the u-boot.dtsi,
> > > > do anything to keep U-Boot from accessing the OPTEE memory, or
> > > > document how to configure and build OPTEE for imx8m. I would like to
> > > > add such support but I find it odd that OPTEE needs to be built
> > > > differently depending on the dram size.
> > > >
> > > > Prior to switching to binman (v2021.10)
> > > > arch/arm/mach-imx/mkimage_fit_atf.sh [1] would include tee.bin in the
> > > > FIT image if it was found in the current directory and it was expected
> > > > that you provide a proper TEE_LOAD_ADDR via the env (commit
> > > > a9eed6e1b8d8 ("imx: imx8m: introduce script to generate fit image").
> > > >
> > > > According to the IMX OPTEE documentation [2] the size and location of
> > > > OPTEE is hard coded (CFG_TZDRAM_START and CFG_TZDRAM_SIZE) but looking
> > > > at the imx-optee source [3] this is calculated based off of
> > > > CFG_DDR_SIZE (which you can provide via env at build time or just
> > > > provide CFG_TZDRAM_START and CFG_TZDRAM_SIZE via env directly).
> > >
> > > optee does not support PIE, it could only run at the address that built
> > > time defined.
> >
> > Hi Peng,
> >
> > I wasn't thinking about PIE but just building it at a location other
> > than the top of DRAM in order to come up with a generalized location
> > instead of having to customize u-boot.dtsi for different RAM sizes and
> > to workaround the 4GiB boundary issue.
> >
> > If my understanding is correct optee's load address needs to match between:
> > 1. atf: BL32_BASE at build time defines the address in secure memory
> > where BL2 loads the BL32 binary image (tee.bin) (must be aligned on a
> > page-size boundary)
> > 2. optee: CFG_TZDRAM_START at build time (calculated based off of
> > CFG_DDR_SIZE) defines the address it is built to run at. Note you must
> > still define CFG_DDR_SIZE properly as this is passed to
> > imx_tzc_auto_configure()
> > 3. uboot: tee.bin is contained in u-boot.itb FIT image and specifies
> > the address that U-Boot SPL loads tee.bin into
> >
> > I'm really trying to understand the imx8m 4GiB limit issue as that
> > seems to be a huge limiation.
>
> Peng,
>
> I've made good progress and now have a 4GiB imx8mm board working with
> optee (with 1 hack):
>
> >
> > On an imx8mm board with 4GiB of DRAM configured with
> > atf:BL32_BASE=0x13e000000
> > optee:CFG_DDR_SIZE=0x100000000,CFG_TZDRAM_START=0x13e000000 and
> > u-boot.dtsi tee.bin load/entry=<0x1 0x3e000000> we get this:
> >
> > U-Boot SPL 2023.04-00027-g578c653cbafa (Jun 16 2023 - 09:22:40 -0700)
> > GSCv3 : v58 0xf098 RST:VIN Thermal protection:disabled
> > RTC : 1970-01-01 0:05:40 UTC
> > Model : GW7201-01-EB
> > Serial : 852455
> > MFGDate : 11-10-2020
> > PMIC : MP5416 (IMX8MM)
> > DRAM : LPDDR4 4 GiB 3000MT/s 1500MHz
> > WDT: Started watchdog@30280000 with servicing every 1000ms (60s timeout)
> > Trying to boot from eMMC
> > DTB : imx8mm-venice-gw72xx-0x
> > Cannot use 64 bit addresses with SDMA
> > ^^^ This is the imx mmc driver warning that it can't load data into
> > dram across 32bit boundary
> >
> > Authenticate image from DDR location 0x401fcdc0...
> > NOTICE: Do not release JR0 to NS as it can be used by HAB
> > ERROR: mmap_add_region_check() failed. error -34
> > ASSERT: lib/xlat_tables_v2/xlat_tables_core.c:793
> > BACKTRACE: START: assert
> > 0: EL3: 0x9289cc
> > 1: EL3: 0x929efc
> > 2: EL3: 0x92469c
> > 3: EL3: 0x9248f4
> > 4: EL3: 0x928850
> > 5: EL3: 0x927594
> > 6: EL3: 0x920128
> > 7: EL3: 0x7eb19c
> > 8: EL3: 0x7f3de0
> > BACKTRACE: END: assert
> >
> > This is where the atf jumps to optee so I can understand why the above
> > doesn't work as U-Boot can't load tee.bin from the FIT to 0x13e000000
> > (not sure where it gets loaded... the 64bit address probably wraps).
> > I've also tried setting CFG_CORE_LARGE_PHYS_ADDR=y and
> > CFG_CORE_ARM64_PA_BITS=36 in optee as this is what the imx8mp-evk
> > config with 6GiB does and it ended with the same result but I do
> > notice that doing this changes the link address
>
> The issue above appears to be a result of not setting
> CFG_CORE_LARGE_PHYS_ADDR=y and CFG_CORE_ARM64_PA_BITS=36 in optee when
> CFG_DDR_SIZE exceeds 3GiB.
>
> >
> > The imx8mp-evk has 6GiB of dram on it. Does NXP not have a solution
> > for secure boot on boards with >3GiB of dram? Is there some support
> > in their downstream U-Boot that I'm maybe missing? Personally I can't
> > even get NXP's lf_v2022.04 u-boot branch to build flash.bin for
> > imx8mp_evk so I can try it out.
>
> The optee configuration for imx8mp-evk in
> core/arch/arm/plat-imx/conf.mk is misleading as it does not override
> the default for CFG_TZDRAM_START which will get defaulted to above a
> 32bit address boundary and won't work. Instead it should set it below
> the 32bit address boundary in order for uboot to load it from a FIT
> image, like the mx8mp_rsb3720_6g PLATFORM_FLAVOR does:
> @@ -379,6 +391,7 @@ endif
> ifneq (,$(filter $(PLATFORM_FLAVOR),mx8mpevk))
> CFG_DDR_SIZE ?= UL(0x180000000)
> CFG_UART_BASE ?= UART2_BASE
> +CFG_TZDRAM_START ?= 0x56000000
> $(call force,CFG_CORE_LARGE_PHYS_ADDR,y)
> $(call force,CFG_CORE_ARM64_PA_BITS,36)
> endif
>
> >
> > Also, if I look at the latest NXP Yocto BSP I don't even see where
> > they configure BL32_BASE when building the atf which leaves it to
> > default to something SOC-specific that does not depend on dram size
> > and does not match the dram sizes for imx8mn_evk and imx8mp_evk
> > (https://github.com/nxp-imx/meta-imx/blob/kirkstone-5.15.71-2.2.0/meta-bsp/recipes-bsp/imx-atf/imx-atf_2.6.bb).
> >
> > >
> > > And the TZASC needs following it to protect the region properly.
> > >
> > > >
> > > > This results in the OPTEE location of 0x7e000000 for 1GiB systems,
> > > > 0xbe000000 for 2GiB systems, and 0x13e000000 for 4GiB systems for
> > > > example. This dependence on dram size causes a couple of issues:
> > > > - OPTEE location depends on build-time configuration and several
> > > > boards have run-time identification of their memory size
> > > > - for 4GiB systems the address 0x13e000000 crosses the 32bit address
> > > > boundary and U-Boot hangs
> > >
> > > imx optee seems has this. But for overlapping with U-Boot, I doubt that.
> > >
> > > >
> > > > Is there any reason OPTEE can't be located wherever you want and not
> > > > depend on dram size?
> > >
> > > You could , just modify the imx *.mk file per my recall.
> >
> > Right. So, for example on the same imx8mm board with 4GiB I can
> > configure uboot/atf/optee to load tee at top of 3GiB with
> > atf:BL32_BASE=0xfe000000
> > optee:CFG_DDR_SIZE=0xc0000000,CFG_TZDRAM_START=0xfe000000 and
> > u-boot.dtsi tee.bin load/entry=<0xfe000000>.
> >
> > I finally found that this boots only if I also adjust the size
> > configured by board_phys_sdram_size() down to 3GiB otherwise we hang
> > somewhere before dram_init_banksize(). I am guessing this still has
> > something to do with missing a uboot carevout for the memory tee is
> > loaded in (and I'm not clear how to add a carve-out yet). I still
> > would like to understand this hang as while this is somewhat of a
> > work-around but it does cripple the board's memory size that gets
> > passed along to Linux.
>
> This hang between init_dram() and dram_init_banksize() appears to be
> caused by my U-Boot dram bank configuration which was essentially:
> /* SDRAM configuration: 4GiB */
> #define CFG_SYS_SDRAM_BASE 0x40000000
> #define PHYS_SDRAM 0x40000000
> #define PHYS_SDRAM_SIZE 0x100000000 /* 4 GB */
>
> This appears to be incorrect as it allows U-Boot to go over the 32bit
> address boundary and should instead use a 2 bank configuration:
> /* SDRAM configuration: 4GiB */
> #define CFG_SYS_SDRAM_BASE 0x40000000
> #define PHYS_SDRAM 0x40000000
> #define PHYS_SDRAM_SIZE 0x80000000 /* 2 GB */
> #define PHYS_SDRAM_2 0xC0000000
> #define PHYS_SDRAM_2_SIZE 0x80000000 /* 2 GB */
>
> Even with the above 2-bank configuration, I still find I hang in
> get_ram_size() called from my board_phys_sdram_size() override if I
> pass get_ram_size() a full 4GiB size: get_ram_size((void *)PHYS_SDRAM,
> PHYS_SDRAM_SIZE + PHYS_SDRAM_2_SIZE);
>
> If I avoid calling get_ram_size() and hard-code the dram size U-Boot
> boots fine: '*size = SZ_4G;'
>
> I don't think the get_ram_size() hang is a U-Boot carve-out issue
> because if I configure optee's load address for something like
> 0x7e000000 (below 2GiB DRAM) and use get_ram_size(PHYS_SDRAM, SZ_2G)
> it does not hang (but a memtest from 40000000 to 80000000 will, which
> makes sense).
>
> Do you have any ideas what could be causing the hang in get_ram_size()
> when optee is enabled?
I managed to figure out my remaining issue - I had a typecasting issue
which was causing the value passed to get_ram_size() to overflow and
truncate:
-*size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE + PHYS_SDRAM_2_SIZE);
+*size = get_ram_size((void *)PHYS_SDRAM, (long)PHYS_SDRAM_SIZE +
(long)PHYS_SDRAM_2_SIZE);
Can anyone explain why I need to change my dram config in order to
work with secure-boot/optee on a 4GiB board?:
diff --git a/board/gateworks/venice/venice.c b/board/gateworks/venice/venice.c
index b57fa681804f..9aac38d5e628 100644
--- a/board/gateworks/venice/venice.c
+++ b/board/gateworks/venice/venice.c
@@ -20,7 +20,7 @@ int board_phys_sdram_size(phys_size_t *size)
if (!size)
return -EINVAL;
- *size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
+ *size = get_ram_size((void *)PHYS_SDRAM, (long)PHYS_SDRAM_SIZE
+ (long)PHYS_SDRAM_2_SIZE);
return 0;
}
diff --git a/include/configs/imx8mm_venice.h b/include/configs/imx8mm_venice.h
index b33b8283085f..046d5685d04d 100644
--- a/include/configs/imx8mm_venice.h
+++ b/include/configs/imx8mm_venice.h
@@ -31,10 +31,11 @@
#define CFG_SYS_INIT_RAM_ADDR 0x40000000
#define CFG_SYS_INIT_RAM_SIZE SZ_2M
+/* SDRAM configuration: 4GiB */
#define CFG_SYS_SDRAM_BASE 0x40000000
-
-/* SDRAM configuration */
-#define PHYS_SDRAM 0x40000000
-#define PHYS_SDRAM_SIZE SZ_4G
+#define PHYS_SDRAM 0x40000000
+#define PHYS_SDRAM_SIZE 0x80000000 /* 2 GB */
+#define PHYS_SDRAM_2 0xC0000000
+#define PHYS_SDRAM_2_SIZE 0x80000000 /* 2 GB */
#endif
Best regards,
Tim
