Re: [PATCH] libfdt: fdt_check_full: Add can_assume(PERFECT) check

David Gibson <[email protected]> Wed, 17 Jun 2026 15:38:19 +1000
Newsgroups org.kernel.vger.devicetree-compiler
Message-ID <ajIyy7hesDaTlvc5@zatzit>
--qEWYAeTfPpI/WGA0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Jun 05, 2026 at 04:20:54PM -0600, Tom Rini wrote:
> On Tue, May 26, 2026 at 10:38:59PM -0600, Tom Rini wrote:
> > On Wed, May 27, 2026 at 02:23:20PM +1000, David Gibson wrote:
> > > On Tue, May 26, 2026 at 02:30:22PM -0600, Tom Rini wrote:
> > > > In this function from fdt_check.c we have (reasonably and as the na=
me
> > > > implies) a number of checks on the DTB. However, there are cases wh=
ere
> > > > we may wish to assume that we have been given a perfect DTB already=
 and
> > > > do nothing here. Add a test for can_assume(PERFECT) as the first ch=
eck
> > > > in this function and if true, perform no checks.
> > > >=20
> > > > Signed-off-by: Tom Rini <[email protected]>
> > > > ---
> > > > Along the lines of the patches I posted back in December, in U-Boot=
 SPL
> > > > we just don't have the space for this check much of the time and so=
 have
> > > > always omitted it (going back to at least when Simon posted the ini=
tial
> > > > patch to make libfdt/fdt_check.c here). This is another case where =
it's
> > > > a noticeable size win for us. I had missed this change in particular
> > > > because we had in turn missed catching up on fdt_check_full being m=
oved
> > > > out of fdt_ro.c and in to fdt_check.c.
> > >=20
> > > I'm not necessarily against this, but I have some misgivings.
> > >=20
> > > fdt_check_full() is (deliberately) not called from anywhere else in
> > > libfdt - it's intended to allow the user to explicitly do a full
> > > validity check on the tree.  Given that meaning, I'm not sure it's
> > > wise to turn it into a no-op based on the assume flags.
> > >=20
> > > Your comment seems to imply that the issue here is size - simply
> > > having this function compiled - rather than being too expensive when
> > > (explicitly) called.  That's a little surprising to me - it's in its
> > > own compilation unit, specifically so that the linker can omit it if
> > > it's not used.  Is there something unusual about your build
> > > environment that's not letting that happen?
> >=20
> > So, we have code like this:
> >         /* Get the total space reserved for FDT in blob */
> >         live_fdt =3D bloblist_get_blob(BLOBLISTT_CONTROL_FDT, &blob_siz=
e);
> >         if (live_fdt !=3D gd->fdt_blob)
> >                 return -ENOENT;
> >=20
> >         ret =3D fdt_check_full(live_fdt, blob_size);
> >         if (ret)
> >                 return fdtdec_ret_to_errno(ret);
> >=20
> > And this is compiled on TPL, SPL and full U-Boot builds. On the first
> > two, we're just too space constrained to do this check. So it's not the
> > linker doing the right thing or not, it's avoiding having to #if the
> > code directly (or rather, CONFIG_VAL(...)). My line of thinking was that
> > since ASSUME_PERFECT is that everything is really perfect, this is the
> > way to go. Yes, it's a little odd to have both "call the validation
> > function" and "the validation function does not validate" but that's
> > just because in the second case, we explicitly configured ourself to not
> > validate anything.
> >=20
> > And FWIW, that's really how we use the assume mask in U-Boot, either
> > 0xff or 0x0. It's a case where we're either passing along the tree we
> > bundled with ourself (and so we can assume it's fine) or it's passed
> > along (and we verify).
>=20
> Hey, just checking if there's any further discussion here? I want to try
> and get the delta between U-Boot and upstream down to zero if we can.

No further discussion, I just have approximately zero time for dtc
maintenance :(.

--=20
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

--qEWYAeTfPpI/WGA0
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmoyMsoACgkQzQJF27ox
2GdfZw//eB2i+h5rPxF07cx20Dsz0dCunCUy3khpodGj/4l90C9tunWPY4pHt6yU
s3aUEZuDgo5dH1noytMPmsIsOenUELmpUOpfgGaw2+ItILuT7IPRto4V2162DCPo
4rQk7kSuHDmHLWeBNXka1yOrPSxgnNz7bXNdEwTUcxJEHuANPLKteH8z9krzgVEq
H+J1M7seKej05JW37PDm+/n1apq2q75XRCN9vDp0115/+CLXtimKvrlypnKrAXS0
T+/Wv4kDWVJpyYvHLh6YOHsftNKbzzOmU1KzWB8/8K94wyYyXY9yBcVy7bIZqStp
XLE0qVJ93uptFks2AyMlcGMqhL+gTg4wVRR44sDtMqJSoIsNSVXz3mLwjtOCHpHe
o9xVZRvo8sNOTZcsREXn83sCtJM52VyLwIcKxYmmDWKOXJTdikRAHCovM8BVwimD
TrT1J6yDZuaeSdGusF5nFRjqAkVqk5/WFXIpkTK4FVc/XzrJ9jzL2VgvUEA1AKgk
ZurUz9Vg+Z4/dx8IA1vhRqI+eGAMETHuh/d9khkVta+J9CXX8ZoXXVNzCAO1bWeb
/af3nCOJviu8pWxDAvfVeji/GngdcTwpDHBJSsLpaxDmQwMvVnbZyj5ehxDcpuIs
sV+UQYt8rx4GM+z+2xyN1NO5qw2uyeeKYVqGqUFUsqm6/wI5aSQ=
=tWeG
-----END PGP SIGNATURE-----

--qEWYAeTfPpI/WGA0--