public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: "Alex G." <mr.nuke.me@gmail.com>
To: Patrick DELAUNAY <patrick.delaunay@foss.st.com>,
	"u-boot@lists.denx.de" <u-boot@lists.denx.de>
Cc: Marek Vasut <marex@denx.de>
Subject: Massive stm32mp1 breakage with v2021.10-rc2
Date: Tue, 24 Aug 2021 15:30:03 -0500	[thread overview]
Message-ID: <966fc974-8331-aeac-ba15-5b2ab19c0eaf@gmail.com> (raw)

Hi Patrick,

I'm having issues with some of the recent changes centered around FIP 
support and CONFIG_STM32MP15x_STM32IMAGE. and commit f91783edf224 ("arm: 
stm32mp: handle the OP-TEE nodes in DT with FIP support")

## Problem description

 > +#ifdef CONFIG_STM32MP15x_STM32IMAGE
 > +       /* only needed for boot with TF-A, witout FIP support */
 >         firmware {
 >                 optee {
 >                         compatible = "linaro,optee-tz";
 > @@ -33,6 +35,7 @@
 >                         no-map;
 >                 };
 >         };
 > +#endif

This removes the "optee" and "reserved-memory" nodes. These nodes are 
required by SPL for setting up TZC memory regions, as introduced in 
commit 8533263c8512 ("stm32mp1: spl: Configure TrustZone controller for 
OP-TEE").


## Further details

We now have several boot flows:

1) BL1 -> SPL -> u-boot
2) BL1 -> SPL -> OP-TEE  (this path is now broken)
3) BL1 -> TF-A -> u-boot (use case for STM32IMAGE)
4) BL1 -> TF-A -> OP-TEE (use case for STM32IMAGE)
5) BL1 -> TF-A -> FIP container

So it seems that STM32IMAGE only makes sense for (3) and (4), but 
shouldn't affect the others. The fact that OP-TEE is mixed into this is 
the first red flag.


 > INPUTS-$(CONFIG_STM32MP15x_STM32IMAGE) += u-boot.stm32
 > MKIMAGEFLAGS_u-boot.stm32 = -T stm32image ...

This tells me we use _STM32IMAGE just to enable another output image 
format. The build could give me both u-boot.img, and u-boot.stm32, and I 
would use the correct file. SO in this case, I would expect _STM32IMAGE 
to not change the code behavior. This is the second red flag.


 > $ git grep -c 'ifdef CONFIG_STM32MP15x_STM32IMAGE'
 > arch/arm/mach-stm32mp/cmd_stm32prog/cmd_stm32prog.c:1
 > arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.c:2
 > arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.h:1
 > arch/arm/mach-stm32mp/include/mach/stm32prog.h:1

We're actually trying to move away from ifdefs, so this intense reliance 
on _STM32IMAGE raises the third red flag.

 > board/st/common/stm32mp_mtdparts.c:9

I'm not sure I fully understand why there are so many ifdefs in 
mtdparts.c. MTD layout seems to me like something that is intended to be 
devicetree-driven. This is the fourth red flag.

Let's take a deeper look at one of those:

 > stm32mp_mtdparts.c:#ifdef CONFIG_STM32MP15x_STM32IMAGE
 > stm32mp_mtdparts.c:                      tee = > 
stm32prog_get_tee_partitions();
 > stm32mp_mtdparts.c-#endif


This makes no sense to me. What does OP-TEE presence have to do with the 
image format of u-boot? If TF-A requires a different layout in FIP vs 
non-FIP mode, then it's the responsibility of TF-A to supply a corrected 
devicetree to u-boot. It's not u-boot's role.

## Proposal

I propose we remove CONFIG_STM32MP15x_STM32IMAGE. Always build 
u-boot.stm32, and don't mix it with code behavior.

Alex



             reply	other threads:[~2021-08-24 20:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 20:30 Alex G. [this message]
2021-08-26 21:47 ` [RFC PATCH] stm32mp1: Replace STM32IMAGE config with TFABOOT_FIP Alexandru Gagniuc
2021-08-31 14:54   ` Patrick DELAUNAY
2021-08-31 16:42     ` Marek Vasut
2021-09-01  9:07       ` Patrick DELAUNAY
2021-09-03 15:32         ` Marek Vasut
2021-09-03 17:47           ` Alex G.
2021-09-06  6:16             ` Etienne Carriere

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=966fc974-8331-aeac-ba15-5b2ab19c0eaf@gmail.com \
    --to=mr.nuke.me@gmail.com \
    --cc=marex@denx.de \
    --cc=patrick.delaunay@foss.st.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox