[PATCH 0/3] rpi: Tidy up booting

[Simon Glass]

Hi Tom,

On Sun, 8 Dec 2024 at 09:15, Tom Rini <trini at konsulko.com> wrote:
>
> On Sat, Dec 07, 2024 at 04:51:34PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 6 Dec 2024 at 17:52, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Fri, Dec 06, 2024 at 05:10:39PM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Fri, 6 Dec 2024 at 17:00, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > On Fri, Dec 06, 2024 at 04:56:31PM -0700, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Fri, 6 Dec 2024 at 16:52, Tom Rini <trini at konsulko.com> wrote:
> > > > > > >
> > > > > > > On Fri, Dec 06, 2024 at 04:41:37PM -0700, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Fri, 6 Dec 2024 at 12:41, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Fri, Dec 06, 2024 at 12:17:49PM -0700, Simon Glass wrote:
> > > > > > > > > > Hi Tom,
> > > > > > > > > >
> > > > > > > > > > On Fri, 6 Dec 2024 at 07:08, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Fri, Dec 06, 2024 at 06:11:10AM -0700, Simon Glass wrote:
> > > > > > > > > > >
> > > > > > > > > > > > This series allows rpi to boot a compressed Ubuntu kernel with ~100MB
> > > > > > > > > > > > ramdisk, by expanding the available space.
> > > > > > > > > > > >
> > > > > > > > > > > > It also tidies up some strange behaviour with the provided FDT, where a
> > > > > > > > > > > > separate pointer is maintained to it, even though U-Boot has copied it
> > > > > > > > > > > > and placed it in its own space. This avoids strange bugs where it
> > > > > > > > > > > > accidentally gets overwritten when loading a file into memory.
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > Simon Glass (3):
> > > > > > > > > > > >   rpi: Update environment to support booti and large initrd
> > > > > > > > > > > >   fdt: Allow expanding the devicetree during relocation
> > > > > > > > > > > >   rpi: Use the U-Boot control FDT for fdt_addr
> > > > > > > > > > > >
> > > > > > > > > > > >  board/raspberrypi/rpi/rpi.c   | 20 ++++++++------------
> > > > > > > > > > > >  board/raspberrypi/rpi/rpi.env | 10 ++++++----
> > > > > > > > > > > >  common/board_f.c              |  6 ++++--
> > > > > > > > > > > >  dts/Kconfig                   | 11 +++++++++++
> > > > > > > > > > > >  4 files changed, 29 insertions(+), 18 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > My feedback here is the same feedback I gave the last person that wanted
> > > > > > > > > > > to update the Pi addresses, and I forget if they came back and did that
> > > > > > > > > > > (and it's in Peter / Matthias' queue) or not. Disabling device tree
> > > > > > > > > > > relocation is a bug and must be removed.
> > > > > > > > > > >
> > > > > > > > > > > After that, given the range of memory sizes available on Pi platforms,
> > > > > > > > > > > allocating the kernel / initrd / kernel decompression buffer at run
> > > > > > > > > > > time, ala mach-apple would make life easier in the long run.
> > > > > > > > > >
> > > > > > > > > > Yes, of course, but for now, this resolves the problem and I don't
> > > > > > > > > > believe it creates any other problem.
> > > > > > > > >
> > > > > > > > > Yes. I think you missed that:
> > > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20240723193430.3050251-1-walter.lozano@collabora.com/
> > > > > > > > > is also outstanding and should solve the problem.
> > > > > > > >
> > > > > > > > No, it doesn't support decompression, although I have to wonder why
> > > > > > > > that patch was never applied.
> > > > > > >
> > > > > > > See Peter's response elsewhere about having a lot of things to do and
> > > > > > > not a lot of time. I know he's intended to pick up and post a Pi PR for
> > > > > > > a while.
> > > > > >
> > > > > > OK, but let's drop that patch since it doesn't support decompression, right?
> > > > >
> > > > > No? No one has done all of the work required (remove fdt_high /
> > > > > initrd_high) and this also solves the larger kernel issue.
> > > >
> > > > No, that is a separate issue. We need kernel_comp_addr and kernel_comp_size
> > >
> > > Yes, those are required for automatic decompression, which is handy. But
> > > the highest priority is removing fdt_high / initrd_high because those
> > > lead to so much time being wasted by everyone. Your time digging in to
> > > and making patch #2 for example.
> >
> > Can you remind me why fdt_high is so bad, or even what it is supposed
> > to help with? Perhaps file an issue? I'm not necessarily offering to
> > do it, but if I understand it better, then it is more likely that I
> > will take it on.
>
> There's nothing to take on? The fdt_high and initrd_high environment
> variables are documented, along with the other relevant bootm variables.
> In general, at least bootm_low / bootm_size are useful and the correct
> way to solve the problem of "ensure device tree and initrd are where the
> OS can see them", when we are NOT in the use case of "we have an API to
> cooperate on this task".  The problem is that for in hind sight
> extremely bad reasons, we allowed for the magic value of -1 to mean "Do
> not move the device tree" (or initrd). In the latter case, it's a boot
> time optimization that's quite useful and handy at least on the
> processors of the time. In the case of the former, man years (I'm not
> exaggerating at this point) of engineering time has been wasted over the
> device tree being in an unaligned or about to be overwritten location
> and we (U-Boot) don't fix it. The only remaining problem really is to
> remove fdt_high / initrd_high from the platforms that disabled
> relocation before I finally got all of my tooling right such that when a
> new platform did set it, I would see it and go off on an
> explanatory-rant about why that needs to be fixed.

OK, yes it seems like fdt_high is a hack. But 'git grep fdt_high |grep
fffff |wc' shows 74 matches, so it is still there.

Going back to my patch, do you have any comments about it? I am not
about to boil the fdt_high ocean, sorry, but I will happily drop the
fdt_high on rpi, since it seems unnecessary, at least for 64-bit
kernels. I do see that kernels <4.2 require fdt_high on arm so that
the kernel and FDT are within the same 512MB section. Is that still a
concern?

I'll also add myself as a rpi maintainer since the current ones seem
pretty busy.

I am planning to continue with the bootstd work, part of which will be
to drop the need for these variables. If there are constraints, we can
specify those either in the kernel FIT or in U-Boot's config, and
implement them in C code (with tests), not scripts, variables, etc.

Regards,
Simon