grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] arm: Move trampolines into code section


From: Leif Lindholm
Subject: Re: [PATCH] arm: Move trampolines into code section
Date: Tue, 30 Apr 2019 14:45:56 +0100
User-agent: NeoMutt/20170113 (1.7.2)

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]