v2023.07-rc5 regression: Image overlaps SPL

[Simon Glass]

Hi,

On Wed, 5 Jul 2023 at 09:35, Tom Rini <trini at konsulko.com> wrote:
>
> On Tue, Jul 04, 2023 at 02:13:01PM -0300, Fabio Estevam wrote:
> > On 04/07/2023 14:04, Francesco Dolcini wrote:
> >
> > > The boards that do not check the return value might start to behave
> > > wrongly without an obvious error to help the debugging.
> >
> > Yes, the current implementation of fdt_status_disabled() is fragile, but
> > there's not so much we can do for the upcoming 2023.07.
> >
> > I can try to prepare a patch to improve it after 2023.07 is out next week.
>
> It's also been fragile-as-designed since inception. The trigger here was
> that renaming a ton of properties _reduced_ the overall dtb size itself
> and so some edge cases got pushed over the edge. I'm honestly not sure
> if it's better to:
> - Give everyone a small padding by default
> - Make the platforms which may call one of these functions a small
>   padding by default

One of the above seems good to me. Perhaps we could add a default
padding, with the ability to reduce it to 0, or increase it. I suspect
that not many boards update the FDT in SPL.

> - Audit the callers and make them handle -FDT_ERR_NOSPACE like the other
>   cases where we grow the dtb size do. Even if yes, many of those other
>   cases are for non-trivial growth rather than just adding 4 more
>   letters.
> - Make fdt_set_node_status() handle the disabled case when out of space
>   and grow/retry.

I don't like that since:

- it adds to complexity in that a 'leaf' function is messing with
stuff it shouldn't
- In generate we don't know if there is space to increase the FDT size
- It likely increase code size for a case that cannot happen if people
have tested it properly
- It affects all boards

If boards are ignoring errors, that is up to the boards. We do
encourage people (in core review) to check all errors. This is an
example of why.

Regards,
Simon