Re: [PATCH v2 00/14] Introduce STM32MP1 RCC in secured mode

[Marek Vasut]

On 3/11/21 2:15 PM, Alexandre TORGUE wrote:
> Hi Marek

Hello Alexandre,

> On 3/11/21 12:43 PM, Marek Vasut wrote:
> > On 3/11/21 9:08 AM, Alexandre TORGUE wrote:
> > > Hi ALex
> >
> > Hello everyone,
> >
> > [...]
> >
> > > > Subject: Re: [PATCH v2 00/14] Introduce STM32MP1 RCC in secured mode
> > > >
> > > > On 1/26/21 3:01 AM, gabriel.fernandez@xxxxxxxxxxx wrote:
> > > > > From: Gabriel Fernandez <gabriel.fernandez@xxxxxxxxxxx>
> > > > >
> > > > > Platform STM32MP1 can be used in configuration where some clocks and
> > > > > IP resets can relate as secure resources.
> > > > > These resources are moved from a RCC clock/reset handle to a SCMI
> > > > > clock/reset_domain handle.
> > > > >
> > > > > The RCC clock driver is now dependent of the SCMI driver, then we have
> > > > > to manage now the probe defering.
> > > > >
> > > > > v1 -> v2:
> > > > >     - fix yamllint warnings.
> > > >
> > > > Hi Gabriel,
> > > >
> > > > I don't have much clout with the maintainers, but I have to NAK this 
> > > > series
> > > > after finding major breakage.
> > > >
> > > > The problem with series is that it breaks pretty much every board it 
> > > > touches.
> > > > I have a DK2 here that I'm using for development, which no longer 
> > > > boots with
> > > > this series applied.
> > > >
> > > > The crux of the matter is that this series assumes all boards will 
> > > > boot with an
> > > > FSBL that implements a very specific SCMI clock tree. This is major ABI
> > > > breakage for anyone not using TF-A as the first stage bootloader. 
> > > > Anyone
> > > > using u-boot SPL is screwed.
> > > >
> > > > This series imposes a SOC-wide change via the dtsi files. So even 
> > > > boards that
> > > > you don't intend to convert to SCMI will get broken this way.
> > > > Adding a -no-scmi file that isn't used anywhere doesn't help things.
> > >
> > > You are right. We mainly take care about NO ST (DH/...) boards, but 
> > > not really about current usage
> > > Of our stm32 boards. Several options exist:
> >
> > Since a lot of people benefit from the good upstream support for the 
> > MP1 _and_ keep updating their machines to get the latest fixes, it is 
> > very important to keep the current usage working.
> >
> > > 1- Break the current ABI: as soon as those patches are merged, 
> > > stm32mp157c-dk2.dtb will impose to use
> > > A tf-a for scmi clocks. For people using u-boot spl, the will have to 
> > > create their own "no-secure" devicetree.
> >
> > NAK, this breaks existing boards and existing setups, e.g. DK2 that 
> > does not use ATF. >
> > > 2-As you suggest, create a new "secure" dtb per boards (Not my wish 
> > > for maintenance perspectives).
> >
> > I agree with Alex (G) that the "secure" option should be opt-in.
> > That way existing setups remain working and no extra requirements are 
> > imposed on MP1 users. Esp. since as far as I understand this, the 
> > "secure" part isn't really about security, but rather about moving 
> > clock configuration from Linux to some firmware blob.
> >
> > > 3- Keep kernel device tree as they are and applied this secure layer 
> > > (scmi clocks phandle) thanks to dtbo in
> > > U-boot.
> >
> > Is this really better than
> > #include "stm32mp15xx-enable-secure-stuff.dtsi"
> > in a board DT ? Because that is how I imagine the opt-in "secure" 
> > option could work.
>
> The dtbo usage could avoid to add another st board (actually a secure 
> config) in arch/arm/boot/dts.

It isn't even a board, it is a configuration. Could you detect this 
secure/non-secure state at runtime, have both clock options in the DT, 
and handle it accordingly ? That might be even better option.