[PATCH 3/3] doc: environment: Further expand on Image locations and provide example

Mon Jul 11 14:41:12 CEST 2022

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 <trini at 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 <trini at konsulko.com>
> > > > > ---
> > > > >    doc/usage/environment.rst | 39 +++++++++++++++++++++++++++++++++++++++
> > > > >    1 file changed, 39 insertions(+)
> > > > 
> > > > Reviewed-by: Simon Glass <sjg at 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.glpk@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.

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.

> 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.

> > > > > +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.

> > > > > +say that the device tree will be placed at 128MiB offset from the start of
> > > 
> > > Please, mention that initrd must be
> > > 
> > > * within 512 MiB (0x20000000) of the memory start on arm
> > >    (which restricts initrd_high)
> > > * in a a 1 GB aligned region of size '1UL << (VA_BITS_MIN - 1)' that
> > >    includes the kernel on arm64
> > 
> > No, because this is not intended to list every constraint on ARM32 (nor
> > arm64, which would benefit from an example that's not TI, as TI arm64
> > platforms share the same base address for memory).
> 
> You ask above to follow the example of ARMv7 on all architectures. Hence
> it is necessary to point out the differences.

No, I'm asking for more examples to be added for each architecture.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220711/1a165e96/attachment.sig>