On Mon, Jul 11, 2022 at 03:10:36PM +0200, Heinrich Schuchardt wrote:
> On 7/11/22 14:41, Tom Rini wrote:
> > On Mon, Jul 11, 2022 at 08:42:08AM +0200, Heinrich Schuchardt wrote:
> > > On 7/10/22 18:17, Tom Rini wrote:
> > > > On Sun, Jul 10, 2022 at 02:26:04PM +0200, Heinrich Schuchardt wrote:
> > > > > On 6/30/22 12:06, Simon Glass wrote:
> > > > > > On Mon, 20 Jun 2022 at 08:32, Tom Rini <tr...@konsulko.com> wrote:
> > > > > > >
> > > > > > > Start by elaborating on what some of our constraints tend to be
> > > > > > > with
> > > > > > > image location values, and document where these external
> > > > > > > constraints
> > > > > > > come from. Provide a new subsection, an example based on the TI
> > > > > > > ARMv7
> > > > > > > OMAP2PLUS families of chips, that gives sample values and
> > > > > > > explains why
> > > > > > > we use these particular values. This is based on what is in
> > > > > > > include/configs/ti_armv7_common.h as of fb3ad9bd923d ("TI: Add,
> > > > > > > use a
> > > > > > > DEFAULT_LINUX_BOOT_ENV environment string") as this contains just
> > > > > > > the
> > > > > > > values referenced in this document now and not some of the further
> > > > > > > additions that are less generic.
> > > > > > >
> > > > > > > Signed-off-by: Tom Rini <tr...@konsulko.com>
> > > > > > > ---
> > > > > > > doc/usage/environment.rst | 39
> > > > > > > +++++++++++++++++++++++++++++++++++++++
> > > > > > > 1 file changed, 39 insertions(+)
> > > > > >
> > > > > > Reviewed-by: Simon Glass <s...@chromium.org>
> > > > >
> > > > > Below you want a change?
> > > >
> > > > Yes, often Simon does that (and it's fine) to both offer a tag but if
> > > > another iteration is needed to make a minor adjustment to some wording
> > > > or another, or to make when applying. Which is fine with me.
> > > >
> > > > > > > diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
> > > > > > > index a9a4702632d2..f70ccd6a58ee 100644
> > > > > > > --- a/doc/usage/environment.rst
> > > > > > > +++ b/doc/usage/environment.rst
> > > > > > > @@ -404,6 +404,42 @@ device tree blob fdtfile fdt_addr_r
> > > > > > > fdt_addr
> > > > > > > ramdisk ramdiskfile ramdisk_addr_r ramdisk_addr
> > > > > > > ================= ============== ================
> > > > > > > ==============
> > > > > > >
> > > > > > > +When setting the RAM addresses for `kernel_addr_r`, `fdt_addr_r`
> > > > > > > and
> > > > > > > +`ramdisk_addr_r` there are several constraints to keep in mind.
> > > > > > > When booting
> > > > > > > +Linux, the `Booting ARM Linux`_ and `Booting AArch64 Linux`_
> > > > > > > documents lay out
> > > > > > > +the requirements for booting all ARM platforms, including both
> > > > > > > alignment and
> > > > > > > +where within memory various things must be. These guidelines
> > > > > > > tend to also be
> > > > > > > +correct for other OSes and unless specifically contradicted by
> > > > > > > documentation
> > > > >
> > > > > What makes you think that BSD or Haiku have the same constraints as
> > > > > Linux?
> > > >
> > > > Because of what I said, and experience? Now, one may be a subset of
> > > > another, but that still means it will work for both. This is intended
> > > > to be general best practices. If you follow this then it's likely
> > > > anything else will work too. The danger comes from trying to optimize
> > > > the sizes to be as small as possible, rather than as large/flexible as
> > > > will likely work anywhere. I will try and expand on that idea in the
> > > > next iteration.
> > > >
> > > > > > > +specific to another architecture, are good rules to follow for
> > > > > > > other
> > > > > > > +architectures as well.
> > > > >
> > > > > No. RISC-V does not have the same requirements as ARM. E.g. the initrd
> > > > > can be located anywhere in memory.
> > > >
> > > > Please point to documentation that confirms that, and some otherwise bad
> > > > examples that actually work.
> > >
> > > [PATCH 1/1] RISC-V: load initrd wherever it fits into memory
> > > https://lore.kernel.org/all/20210629134018.62859-1-xypron.g...@gmx.de/
> >
> > Looks like someone ran in to the first common case of "oops, overwrote
> > the ramdisk with the kernel bss" or something along those lines.
>
> Not at all. The ramdisk was relocated by U-Boot unnecessarily to above
> 256 MiB after start of RAM and the EFI stub before the patch did not
> accept this address for no reason.
Then bootm_size / bootm_low should have been set appropriately in
U-Boot.
> > Which is why I'm asking for more architectures to add good examples of
> > where to load each payload in to memory, with explanations of why and
> > how big of a gap to have. I _think_ in Linux RISC-V (and hopefully for
> > 32bit and 64bit) used the arm64 Image format and so BSS size is
> > available in the header and so we can safely check for that overlap and
> > relocate rather than fail to boot. Checking for, and avoiding to start
> > with, these types of problems is why I want to add the examples.
>
> I am not aware of any restrictions for the placement of kernel, initrd,
> fdt for RISC-V. Therefore there is no need to relocate anything after
> loading (without overlap).
>
> The bootefi command will never relocate a kernel or an fdt on any
> architecture. You just pass the original load addresses.
Which is all the more reason we need to ensure that the fdt and initrd
are placed in appropriately aligned and non-overlapping locations.
Because if you put the initrd too close to where the kernel ends up, the
BSS will eat your ramdisk before it can go "oh, there's a ramdisk
there". That's a practical, not architectural, problem. Same with the
device tree.
> > > Please, have a look at efi_get_max_initrd_addr() in these files:
> > >
> > > arch/arm/include/asm/efi.h:73
> > > arch/arm64/include/asm/efi.h:77
> > > arch/loongarch/include/asm/efi.h:36
> > > arch/riscv/include/asm/efi.h:31
> > >
> > > MAX_UNCOMP_KERNEL_SIZE = 32 MiB is only enforced in
> > > drivers/firmware/efi/libstub/arm32-stub.c.
> >
> > This isn't an EFI thing however. The max uncompressed Linux kernel
> > image for arm32 is something along the lines of 96MiB I recall rmk
> > telling me when I asked about it at the time. The base+32MiB in the
> > example here is for optimal (but not REQUIRED) decompressor location.
>
> The decompressor is what follows the EFI stub in the image?
I'm not talking about EFI at all in this example, as it's 32bit ARM, not
64bit ARM or RISC-V.
> > > > > > > +Example Image locations
> > > > > > > +^^^^^^^^^^^^^^^^^^^^^^^
> > > > >
> > > > > You seem not to refer to a file 'Image'.
> > > > >
> > > > > %s/Image/image/
> > > >
> > > > OK.
> > > >
> > > > > > > +
> > > > > > > +If we take the Texas Instruments OMAP2PLUS family of ARMv7
> > > > > > > processors as an
> > > > > > > +example for the above listed variables, we would do::
> > > > >
> > > > > %s/we would do/we chose/ ?
> > > >
> > > > Either? I don't see it mattering either way.
> > > >
> > > > > > > +
> > > > > > > + loadaddr=0x82000000
> > > > > > > + kernel_addr_r=${loadaddr}
> > > > > > > + fdt_addr_r=0x88000000
> > > > > > > + ramdisk_addr_r=0x88080000
> > > > > > > + bootm_size=0x10000000
> > > > > > > +
> > > > > > > +To explain this, we start by noting that DRAM starts at
> > > > > > > 0x80000000. A 32MiB
> > > > > >
> > > > > > Should it say 'We use a 32MiB' ?
> > > > >
> > > > > Please, mention that MAX_UNCOMP_KERNEL_SIZE = 32 MiB is ARMv7
> > > > > specific.
> > > >
> > > > Sorry? As I understood it last, the maximum size was something like
> > > > 96MiB before you have to employ some funky tricks.
> > >
> > > Look at the use of MAX_UNCOMP_KERNEL_SIZE in handle_kernel_image() of
> > > the EFI stub (drivers/firmware/efi/libstub/arm32-stub.c).
> > >
> > > >
> > > > > > > +buffer from the start of memory as our default load address, and
> > > > > > > so where the
> > > > > > > +kernel would also be loaded to. This will hopefully allow for
> > > > > > > us to have the
> > > > >
> > > > > %s/allow for us/allow/
> > > > >
> > > > > > > +whole of the compressed kernel image exist in memory above where
> > > > > > > the whole of
> > > > > > > +the decompressed kernel image will be, and allow for a quicker
> > > > > > > boot. Next, we
> > > >
> > > > We use 32MiB for the reason I said here. Which is only a slight
> > > > rewording of the arm32 Linux booting document, and the section starts
> > > > out by saying this is an example for ARMv7 platforms.
> > >
> > > You ask all other architectures to follow this example?
> >
> > I could have sworn that somewhere within the comments of this series I
> > asked for more examples to be added, yes. And I know I intended to
> > (since we _need_ them, and I think I've expressed me desire to have them
> > before) and I am asking now.
> >
> > > > > Please, mention that decompressor code otherwise will have to relocate
> > > > > the compressed kernel.
> > > >
> > > > I'm not sure. Perhaps it would be good to also link to some of the
> > > > articles expanding upon how Linux on ARM32 boots, as part of more
> > > > general documentation, rather than a specific example here.
> > >
> > > Just look at the comment above the definition of MAX_UNCOMP_KERNEL_SIZE
> > > in arch/arm/include/asm/efi.h.
> >
> > Keep in mind this is only vaguely EFI-related. Given how long ago edkII
> > for beagleboard was done, it doesn't quite predate EFI on ARM, but this
> > example has been in long use for the common non-EFI case on 32bit ARM.
>
> The EFI stub is using the value due to (assumed) restrictions in the
> decompressor and main kernel.
OK, but EFI is largely irrelevant to the 32bit ARM case, which is what
this example is for.
--
Tom
signature.asc
Description: PGP signature
