Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Dmitry Rokosov <DDRokosov@sberdevices.ru>
To:     Jerome Brunet <jbrunet@baylibre.com>
CC:     "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        "narmstrong@baylibre.com" <narmstrong@baylibre.com>,
        "khilman@baylibre.com" <khilman@baylibre.com>,
        "martin.blumenstingl@googlemail.com" 
        <martin.blumenstingl@googlemail.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-amlogic@lists.infradead.org" 
        <linux-amlogic@lists.infradead.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        kernel <kernel@sberdevices.ru>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1] arm64: dts: meson-axg: reserve memory region for
 Amlogic TrustOS
Thread-Topic: [PATCH v1] arm64: dts: meson-axg: reserve memory region for
 Amlogic TrustOS
Thread-Index: AQHYqCKmlgn31IcXkUK0U/+U7ZMA562fwScAgABe64A=
Date:   Fri, 5 Aug 2022 13:42:53 +0000
Message-ID: <20220805134317.y6lnpgefolfq4iim@CAB-WSD-L081021.sigma.sbrf.ru>
References: <20220804165317.29086-1-ddrokosov@sberdevices.ru>
 <1jfsibqfez.fsf@starbuckisacylon.baylibre.com>
In-Reply-To: <1jfsibqfez.fsf@starbuckisacylon.baylibre.com>
Accept-Language: ru-RU, en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1C39E89F7937BD45A6D243AD02E27093@sberdevices.ru>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Precedence: bulk

Hello Jerome,

Thank you for the feedback.

On Fri, Aug 05, 2022 at 10:03:34AM +0200, Jerome Brunet wrote:
>=20
> On Thu 04 Aug 2022 at 16:52, Dmitry Rokosov <DDRokosov@sberdevices.ru> wr=
ote:
>=20
> > For the all AXG SoC based boards, which run Amlogic vendor ATF and
> > TrustOS this memory region 0x5300000-0x6300000 is reserved by BL32,
> > so tag it as no-map in the kernel iomem.
>=20
> This may be true for the boards you have seen so far but not all ship
> with this specific AML TEE. Some don't have TEE at all, other may have
> different TEE regions.
>=20
> 16 MB may be a significant part of the available memory on some AXG
> devices. Reserving that memory on all AXG devices, regardless of what is
> actually running does not seem appropriate.
>=20
> I know the same has been done for other devices but I don't think we shou=
ld
> continue to do so. This should be set either
> * per device if it is fixed
> * dynamically by the bootloader depending on the ATF (which is probably b=
etter)
>=20

I agree with you, *.dtsi is a common device tree base file which is
included in the all board trees. But looks like I don't understand meson
dtsi policy about TEE reserved memory regions. I mean g12 and gx dtsi
have statically defined TrustOS regions inside, and all meson dtsi have
hardcoded ATF regions. All of these regions are aligned with Amlogic ATF
and Amlogic Trust OS reserved memory addresses. And if I want to use
upstream ATF or optee build for Amlogic board, I need to patch dtsi file,
which is not right way as you mentioned.

If we want to use per-board TEE memory regions definitions I suppose we
need to move secos reserved ranges from gx and g12 dtsi files and move
all secmon definitions from all meson dtsi files to appropriate board
dtses (or mark them with status =3D "disable").

What do you think?

> >
> > Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> > ---
> >  arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/bo=
ot/dts/amlogic/meson-axg.dtsi
> > index 3f5254eeb47b..1fa0d3805969 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> > +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> > @@ -142,6 +142,12 @@ secmon_reserved: secmon@5000000 {
> >  			reg =3D <0x0 0x05000000 0x0 0x300000>;
> >  			no-map;
> >  		};
> > +
> > +		/* 16 MiB reserved for Amlogic Trust OS (BL32) */
> > +		secos_reserved: secos@5300000 {
> > +			reg =3D <0x0 0x05300000 0x0 0x1000000>;
> > +			no-map;
> > +		};
> >  	};
> > =20
> >  	scpi {
>=20

--=20
Thank you,
Dmitry=