On Tue, Apr 30, 2019 at 03:06:35PM +0200, Alexander Graf wrote: > > Right, so with a brain that's actually awake: > > > >> --- > >> util/grub-mkimagexx.c | 29 +++++++++++++---------------- > >> 1 file changed, 13 insertions(+), 16 deletions(-) > >> > >> diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c > >> index 2059890c3..af23fae52 100644 > >> --- a/util/grub-mkimagexx.c > >> +++ b/util/grub-mkimagexx.c > >> @@ -2197,25 +2197,10 @@ SUFFIX (locate_sections) (Elf_Ehdr *e, const char > >> *kernel_path, > >> } > >> } > >> > >> - layout->kernel_size = ALIGN_UP (layout->kernel_size + > >> image_target->vaddr_offset, > >> - image_target->section_align) > >> - - image_target->vaddr_offset; > >> - layout->exec_size = layout->kernel_size; > >> - > >> - /* .data */ > >> - for (i = 0, s = smd->sections; > >> - i < smd->num_sections; > >> - i++, s = (Elf_Shdr *) ((char *) s + smd->section_entsize)) > >> - if (SUFFIX (is_data_section) (s, image_target)) > >> - layout->kernel_size = SUFFIX (put_section) (s, i, > >> layout->kernel_size, smd, > >> - image_target); > >> - > > > > This patch only moves the below ifdef/conditional before the above > > stanza, which remains unchanged. So this does not affect !armhf at > > all. The generated diff is less than helpful here. > > > >> #ifdef MKIMAGE_ELF32 > >> if (image_target->elf_target == EM_ARM) > >> { > >> grub_size_t tramp; > >> - layout->kernel_size = ALIGN_UP (layout->kernel_size + > >> image_target->vaddr_offset, > >> - image_target->section_align) - > >> image_target->vaddr_offset; > > > > *boggle*, so we were double adjusting these on arm? That explains why > > things were confused/confusing. > > > >> > >> layout->kernel_size = ALIGN_UP (layout->kernel_size, 16); > >> > >> @@ -2223,10 +2208,22 @@ SUFFIX (locate_sections) (Elf_Ehdr *e, const char > >> *kernel_path, > > > > However, the line just left out of context here > > # tramp = arm_get_trampoline_size (e, smd->sections, smd->section_entsize, > > > >> smd->num_sections, image_target); > > > > now looks a bit weird. We set "tramp" but never use it. > > > >> > >> layout->tramp_off = layout->kernel_size; > >> - layout->kernel_size += ALIGN_UP (tramp, 16); > > > > Because we delete this adjustment. > > Why is that no longer needed? > > Because it was 2am for me too :). It obviously is needed - otherwise > we blindly rely on the section padding to fit our trampoline.
Ah, that makes more sense then :) Well, if you put that adjustment in, and turn this into a 2-patch set with the offset adjustment one, I think we're good to go. I think this ought to be 1/2 with the offset adjustment 2/2, to emphasise this fixes a problem alrady present. / Leif _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel