It was <2020-06-01 pon 21:41>, when Russell King - ARM Linux admin wrote: > On Mon, Jun 01, 2020 at 10:27:45PM +0200, Lukasz Stelmach wrote: >> It was <2020-06-01 pon 19:25>, when Russell King - ARM Linux admin wrote: >>> On Mon, Jun 01, 2020 at 06:19:52PM +0200, Lukasz Stelmach wrote: >>>> It was <2020-06-01 pon 15:55>, when Russell King - ARM Linux admin wrote: >>>>> On Mon, Jun 01, 2020 at 04:27:52PM +0200, Łukasz Stelmach wrote: >>>>>> Add DCSZ tag which holds dynamic memory (stack, bss, malloc pool) >>>>>> requirements of the decompressor code. >>>>> >>>>> Why do we need to know the stack and BSS size, when the userspace >>>>> kexec tool doesn't need to know this to perform the same function. >>>> >>>> >>>> kexec-tools load zImage as low in DRAM as possible and rely on two >>>> assumptions: >>>> >>>> + the zImage will copy itself to make enough room for the kernel, >>>> + sizeof(zImage+mem) < sizeof(kernel+mem), which is true because >>>> of compression. >>>> >>>> DRAM start >>>> + 0x8000 >>>> >>>> zImage |-----------|-----|-------| >>>> text+data bss stack >>>> >>>> text+data bss >>>> kernel |---------------------|-------------------| >>>> >>>> >>>> initrd |-initrd-|-dtb-| >>> >>> This is actually incorrect, because the decompressor will self- >>> relocate itself to avoid the area that it is going to decompress >>> into. >> >> I described the state right after kexec(8) invocation. > > Actually, you haven't, because this is _not_ how kexec(8) lays it > out, as I attempted to detail further down in my reply.
Let me try to describe how I understand the code in kexec-tools (commit 74c7c369). --8<---------------cut here---------------start------------->8--- int zImage_arm_load(…, const char *buf, off_t len, …) // buf - zImage // len - size of zImage unsigned int extra_size = 0x8000; /* TEXT_OFFSET */ kernel_mem_size = len + 4; // locate a hole to fit zImage + 32kB as low as possible, base = locate_hole(info, len + extra_size, 0, 0, ULONG_MAX, INT_MAX); kernel_base = base + extra_size; add_segment(info, buf, len, kernel_base, kernel_mem_size); --8<---------------cut here---------------end--------------->8--- Therefore, zImage is loaded low and always requires relocation. ram |-------------------------------------------------------------- zImage |----k_m_s----| ^ | kernel_base — TEXT_OFFSET or higher Next goes initrd --8<---------------cut here---------------start------------->8--- kexec_arm_image_size = len * 5; // or passed on command line // if the tag exists kexec_arm_image_size = max(edata_size + bss_size, edata_size + len); // len - zImage size + 64 kB initrd_base = kernel_base + _ALIGN(kexec_arm_image_size, page_size); add_segment(info, ramdisk_buf, initrd_size, initrd_base, initrd_size); --8<---------------cut here---------------end--------------->8--- above whatever is bigger (kernel + kernel bss) OR (kernel + zImage + zImage mem). ram |--------------------------------------------------------------- zImage |----k_m_s----| Where kexec loads zImage @kernel_base |.............len * 5....................| Fallback kernel |.....edata.....|...bss...| These are just calculations zImage |.....len+....| zImage will copy itself here WHEN it runs initrd |--initrd_size--| dtb ^ |---| | initrd_base DTB, of course, goes next dtb_offset = initrd_base + initrd_size + page_size; Stuff marked with "-" is actually loaded, "." are just calculations to establish initrd_base. >>> So, while the decompressor runs, in the above situation it >>> ends up as: >>> >>> >>> ram |------------------------------------------------------... >>> text+data bss >>> kernel |---------------------|-------------------| >>> zImage |-----------|-----|-------| >>> text+data bss stack+malloc > > Note here - if the initrd was placed as _you_ describe at the end > of where the zImage ends up including its BSS, it would be > corrupted by the stack and malloc space of the decompressor while > running. Ergo, your description of how kexec(8) lays stuff out > is incorrect. Is my analysis above accurate now? Do I understand this? As you noted, my intention is to load zImage after edata (dotted len+ above). >>>>>> +struct arm_zimage_tag_dc { >>>>>> + struct tag_header hdr; >>>>>> + union { >>>>>> +#define ZIMAGE_TAG_DECOMP_SIZE ARM_ZIMAGE_MAGIC4 >>>>>> + struct zimage_decomp_size { >>>>>> + __le32 bss_size; >>>>>> + __le32 stack_size; >>>>>> + __le32 malloc_size; >>>>>> + } decomp_size; >>> >>> You certainly don't need to know all this. All you need to know is >>> how much space the decompressor requires after the end of the image, >>> encompassing the BSS size, stack size and malloc size, which is one >>> value. >> >> I agree. However, since we are not fighting here for every single byte, >> I'd rather add them as separate values and make the tag, even if only >> slightly, more future-proof. > > It doesn't make it more future-proof. What happens if we add something > else, do we need to list it separately, and then change the kernel to > accept the new value and maybe also kexec(8)? Or do we just say "the > decompressor needs X many bytes after the image" and be done with it? > The latter sounds way more future-proof to me. You are right. I changed it to a single value. Done. -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics
signature.asc
Description: PGP signature