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

--TyBL2h1I8DdC0IMw
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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 name
> > implies) a number of checks on the DTB. However, there are cases where
> > 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 check
> > 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 initial
> > 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 moved
> > 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?

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_size);
        if (live_fdt !=3D gd->fdt_blob)
                return -ENOENT;

        ret =3D fdt_check_full(live_fdt, blob_size);
        if (ret)
                return fdtdec_ret_to_errno(ret);

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.

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
Tom

--TyBL2h1I8DdC0IMw
Content-Type: application/pgp-signature; name="signature.asc"

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

iHUEABYKAB0WIQTzzqh0PWDgGS+bTHor4qD1Cr/kCgUCahZ1XQAKCRAr4qD1Cr/k
CoDeAQCO9bSyqTi1SDHjR1cXnRetJBtGBuIHNEsfvA+D2kOTRAD+KHZggh+IuCNn
vMIbI4zaDyx8sd3j2+4KvtArtC4naQk=
=s0b5
-----END PGP SIGNATURE-----

--TyBL2h1I8DdC0IMw--